diff --git a/lms/djangoapps/grades/scores.py b/lms/djangoapps/grades/scores.py index 27abc562cf..86832ef156 100644 --- a/lms/djangoapps/grades/scores.py +++ b/lms/djangoapps/grades/scores.py @@ -124,6 +124,23 @@ def get_score(submissions_scores, csm_scores, persisted_block, block): ) +def weighted_score(raw_earned, raw_possible, weight): + """ + Returns a tuple that represents the weighted (earned, possible) score. + If weight is None or raw_possible is 0, returns the original values. + + When weight is used, it defines the weighted_possible. This allows + course authors to specify the exact maximum value for a problem when + they provide a weight. + """ + assert raw_possible is not None + cannot_compute_with_weight = weight is None or raw_possible == 0 + if cannot_compute_with_weight: + return raw_earned, raw_possible + else: + return float(raw_earned) * weight / raw_possible, float(weight) + + def _get_score_from_submissions(submissions_scores, block): """ Returns the score values from the submissions API if found. @@ -157,7 +174,7 @@ def _get_score_from_csm(csm_scores, block, weight): if has_valid_score: raw_earned = score.correct if score.correct is not None else 0.0 raw_possible = score.total - return (raw_earned, raw_possible) + _weighted_score(raw_earned, raw_possible, weight) + return (raw_earned, raw_possible) + weighted_score(raw_earned, raw_possible, weight) def _get_score_from_persisted_or_latest_block(persisted_block, block, weight): @@ -174,7 +191,7 @@ def _get_score_from_persisted_or_latest_block(persisted_block, block, weight): else: raw_possible = block.transformer_data[GradesTransformer].max_score - return (raw_earned, raw_possible) + _weighted_score(raw_earned, raw_possible, weight) + return (raw_earned, raw_possible) + weighted_score(raw_earned, raw_possible, weight) def _get_weight_from_block(persisted_block, block): @@ -216,23 +233,6 @@ def _get_explicit_graded(block): return True if field_value is None else field_value -def _weighted_score(raw_earned, raw_possible, weight): - """ - Returns a tuple that represents the weighted (earned, possible) score. - If weight is None or raw_possible is 0, returns the original values. - - When weight is used, it defines the weighted_possible. This allows - course authors to specify the exact maximum value for a problem when - they provide a weight. - """ - assert raw_possible is not None - cannot_compute_with_weight = weight is None or raw_possible == 0 - if cannot_compute_with_weight: - return raw_earned, raw_possible - else: - return float(raw_earned) * weight / raw_possible, float(weight) - - @memoized def _block_types_possibly_scored(): """ diff --git a/lms/djangoapps/grades/tests/test_scores.py b/lms/djangoapps/grades/tests/test_scores.py index a4002fbef2..f7d6e34055 100644 --- a/lms/djangoapps/grades/tests/test_scores.py +++ b/lms/djangoapps/grades/tests/test_scores.py @@ -162,9 +162,9 @@ class TestGetScore(TestCase): @ddt.ddt -class TestInternalWeightedScore(TestCase): +class TestWeightedScore(TestCase): """ - Tests the internal helper method: _weighted_score + Tests the helper method: weighted_score """ @ddt.data( (0, 0, 1), @@ -177,7 +177,7 @@ class TestInternalWeightedScore(TestCase): @ddt.unpack def test_cannot_compute(self, raw_earned, raw_possible, weight): self.assertEquals( - scores._weighted_score(raw_earned, raw_possible, weight), + scores.weighted_score(raw_earned, raw_possible, weight), (raw_earned, raw_possible), ) @@ -192,13 +192,13 @@ class TestInternalWeightedScore(TestCase): @ddt.unpack def test_computed(self, raw_earned, raw_possible, weight, expected_score): self.assertEquals( - scores._weighted_score(raw_earned, raw_possible, weight), + scores.weighted_score(raw_earned, raw_possible, weight), expected_score, ) def test_assert_on_invalid_r_possible(self): with self.assertRaises(AssertionError): - scores._weighted_score(raw_earned=1, raw_possible=None, weight=1) + scores.weighted_score(raw_earned=1, raw_possible=None, weight=1) @ddt.ddt diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 3540593483..becae8f2ba 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -4,6 +4,7 @@ Enrollment operations for use by instructor APIs. Does not include any access control, be sure to check access before calling. """ +import crum import json import logging from django.contrib.auth.models import User @@ -13,11 +14,14 @@ from django.core.mail import send_mail from django.utils.translation import override as override_language from course_modes.models import CourseMode -from student.models import CourseEnrollment, CourseEnrollmentAllowed +from courseware.model_data import FieldDataCache +from courseware.module_render import get_module_for_descriptor from courseware.models import StudentModule from edxmako.shortcuts import render_to_string +from grades.scores import weighted_score +from grades.signals.signals import SCORE_CHANGED from lang_pref import LANGUAGE_KEY - +from student.models import CourseEnrollment, CourseEnrollmentAllowed from submissions import api as sub_api # installed from the edx-submissions repository from student.models import anonymous_id_for_user from openedx.core.djangoapps.user_api.models import UserPreference @@ -245,6 +249,7 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user ) submission_cleared = True except ItemNotFoundError: + block = None log.warning("Could not find %s in modulestore when attempting to reset attempts.", module_state_key) # Reset the student's score in the submissions API, if xblock.clear_student_state has not done so already. @@ -267,6 +272,7 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user if delete_module: module_to_reset.delete() + _fire_score_changed_for_block(course_id, student, block, module_state_key) else: _reset_module_attempts(module_to_reset) @@ -287,6 +293,43 @@ def _reset_module_attempts(studentmodule): studentmodule.save() +def _fire_score_changed_for_block(course_id, student, block, module_state_key): + """ + Fires a SCORE_CHANGED event for the given module. The earned points are + always zero. We must retrieve the possible points from the XModule, as + noted below. + """ + if block and block.has_score: + cache = FieldDataCache.cache_for_descriptor_descendents( + course_id=course_id, + user=student, + descriptor=block, + depth=0 + ) + # For implementation reasons, we need to pull the max_score from the XModule, + # even though the data is not user-specific. Here we bind the data to the + # current user. + request = crum.get_current_request() + module = get_module_for_descriptor( + user=student, + request=request, + descriptor=block, + field_data_cache=cache, + course_key=course_id + ) + points_earned, points_possible = weighted_score(0, module.max_score(), getattr(module, 'weight', None)) + else: + points_earned, points_possible = 0, 0 + SCORE_CHANGED.send( + sender=None, + points_possible=points_possible, + points_earned=points_earned, + user=student, + course_id=course_id, + usage_id=module_state_key + ) + + def get_email_params(course, auto_enroll, secure=True, course_key=None, display_name=None): """ Generate parameters used when parsing email templates. diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 01559dfd8f..0b65f1c8a4 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -8,6 +8,7 @@ import mock from mock import patch from abc import ABCMeta from courseware.models import StudentModule +from courseware.tests.helpers import get_request_for_user from django.conf import settings from django.utils.translation import get_language from django.utils.translation import override as override_language @@ -16,7 +17,11 @@ from ccx_keys.locator import CCXLocator from student.tests.factories import UserFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory +from grades.new.subsection_grade import SubsectionGradeFactory +from grades.tests.utils import answer_problem from lms.djangoapps.ccx.tests.factories import CcxFactory +from lms.djangoapps.course_blocks.api import get_course_blocks from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from student.models import CourseEnrollment, CourseEnrollmentAllowed from student.roles import CourseCcxCoachRole @@ -494,6 +499,80 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): self.assertEqual(unrelated_state['brains'], 'zombie') +class TestStudentModuleGrading(SharedModuleStoreTestCase): + """ + Tests the effects of student module manipulations + on student grades. + """ + @classmethod + def setUpClass(cls): + super(TestStudentModuleGrading, cls).setUpClass() + cls.course = CourseFactory.create() + cls.chapter = ItemFactory.create( + parent=cls.course, + category="chapter", + display_name="Test Chapter" + ) + cls.sequence = ItemFactory.create( + parent=cls.chapter, + category='sequential', + display_name="Test Sequential 1", + graded=True + ) + cls.vertical = ItemFactory.create( + parent=cls.sequence, + category='vertical', + display_name='Test Vertical 1' + ) + problem_xml = MultipleChoiceResponseXMLFactory().build_xml( + question_text='The correct answer is Choice 3', + choices=[False, False, True, False], + choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3'] + ) + cls.problem = ItemFactory.create( + parent=cls.vertical, + category="problem", + display_name="Test Problem", + data=problem_xml + ) + cls.request = get_request_for_user(UserFactory()) + cls.user = cls.request.user + + def _get_subsection_grade_and_verify(self, all_earned, all_possible, graded_earned, graded_possible): + """ + Retrieves the subsection grade and verifies that + its scores match those expected. + """ + subsection_grade_factory = SubsectionGradeFactory( + self.user, + self.course, + get_course_blocks(self.user, self.course.location) + ) + grade = subsection_grade_factory.update(self.sequence) + self.assertEqual(grade.all_total.earned, all_earned) + self.assertEqual(grade.graded_total.earned, graded_earned) + self.assertEqual(grade.all_total.possible, all_possible) + self.assertEqual(grade.graded_total.possible, graded_possible) + + @patch('crum.get_current_request') + def test_delete_student_state(self, _crum_mock): + problem_location = self.problem.location + self._get_subsection_grade_and_verify(0, 1, 0, 1) + answer_problem(course=self.course, request=self.request, problem=self.problem, score=1, max_value=1) + self._get_subsection_grade_and_verify(1, 1, 1, 1) + + # Delete student state using the instructor dash + reset_student_attempts( + self.course.id, + self.user, + problem_location, + requesting_user=self.user, + delete_module=True, + ) + # Verify that the student's grades are reset + self._get_subsection_grade_and_verify(0, 1, 0, 1) + + class EnrollmentObjects(object): """ Container for enrollment objects.