From 4eb5311ecebad7aad428fbdca4bcf9138fe9d780 Mon Sep 17 00:00:00 2001 From: Sanford Student Date: Mon, 28 Nov 2016 16:30:45 -0500 Subject: [PATCH] Separating problem raw score changed and problem weighted score changed signals For TNL-5993 --- .../courseware/tests/test_module_render.py | 7 +- .../tests/test_submitting_problems.py | 6 +- lms/djangoapps/gating/signals.py | 6 +- lms/djangoapps/grades/signals/handlers.py | 75 ++++++++++++++----- lms/djangoapps/grades/signals/signals.py | 40 +++++++--- lms/djangoapps/grades/tasks.py | 28 +++---- lms/djangoapps/grades/tests/test_signals.py | 69 ++++++++++++++--- lms/djangoapps/grades/tests/test_tasks.py | 20 ++--- lms/djangoapps/instructor/enrollment.py | 39 ++++------ lms/djangoapps/instructor/tests/test_api.py | 2 +- .../instructor/tests/test_enrollment.py | 4 +- .../instructor/tests/test_services.py | 2 +- lms/djangoapps/lti_provider/tasks.py | 10 +-- 13 files changed, 205 insertions(+), 103 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 750e56f544..c3c817a5f7 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1831,14 +1831,15 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): self.assertIsNone(student_module.grade) self.assertIsNone(student_module.max_grade) - @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_SCORE_CHANGED.send') + @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_RAW_SCORE_CHANGED.send') def test_score_change_signal(self, send_mock): """Test that a Django signal is generated when a score changes""" self.set_module_grade_using_publish(self.grade_dict) expected_signal_kwargs = { 'sender': None, - 'points_possible': self.grade_dict['max_value'], - 'points_earned': self.grade_dict['value'], + 'raw_possible': self.grade_dict['max_value'], + 'raw_earned': self.grade_dict['value'], + 'weight': None, 'user_id': self.student_user.id, 'course_id': unicode(self.course.id), 'usage_id': unicode(self.problem.location), diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index b92a3d1ff5..81f3864a35 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -153,7 +153,9 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl self.student_user = User.objects.get(email=self.student) self.factory = RequestFactory() # Disable the score change signal to prevent other components from being pulled into tests. - self.score_changed_signal_patch = patch('lms.djangoapps.grades.signals.handlers.PROBLEM_SCORE_CHANGED.send') + self.score_changed_signal_patch = patch( + 'lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send' + ) self.score_changed_signal_patch.start() def tearDown(self): @@ -162,7 +164,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl def _stop_signal_patch(self): """ - Stops the signal patch for the PROBLEM_SCORE_CHANGED event. + Stops the signal patch for the PROBLEM_WEIGHTED_SCORE_CHANGED event. In case a test wants to test with the event actually firing. """ diff --git a/lms/djangoapps/gating/signals.py b/lms/djangoapps/gating/signals.py index 368092dd4a..88bad17149 100644 --- a/lms/djangoapps/gating/signals.py +++ b/lms/djangoapps/gating/signals.py @@ -4,15 +4,15 @@ Signal handlers for the gating djangoapp from django.dispatch import receiver from gating import api as gating_api -from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED +from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED from opaque_keys.edx.keys import CourseKey, UsageKey from xmodule.modulestore.django import modulestore -@receiver(PROBLEM_SCORE_CHANGED) +@receiver(PROBLEM_WEIGHTED_SCORE_CHANGED) def handle_score_changed(**kwargs): """ - Receives the PROBLEM_SCORE_CHANGED signal sent by LMS when a student's score has changed + Receives the PROBLEM_WEIGHTED_SCORE_CHANGED signal sent by LMS when a student's score has changed for a given component and triggers the evaluation of any milestone relationships which are attached to the updated content. diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 88a7d7faa3..d0c9c5f744 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -10,8 +10,14 @@ from openedx.core.lib.grade_utils import is_score_higher from student.models import user_by_anonymous_id from submissions.models import score_set, score_reset -from .signals import PROBLEM_SCORE_CHANGED, SUBSECTION_SCORE_CHANGED, SCORE_PUBLISHED +from .signals import ( + PROBLEM_RAW_SCORE_CHANGED, + PROBLEM_WEIGHTED_SCORE_CHANGED, + SUBSECTION_SCORE_CHANGED, + SCORE_PUBLISHED, +) from ..new.course_grade import CourseGradeFactory +from ..scores import weighted_score from ..tasks import recalculate_subsection_grade @@ -22,9 +28,9 @@ log = getLogger(__name__) def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-argument """ Consume the score_set signal defined in the Submissions API, and convert it - to a PROBLEM_SCORE_CHANGED signal defined in this module. Converts the unicode keys - for user, course and item into the standard representation for the - PROBLEM_SCORE_CHANGED signal. + to a PROBLEM_WEIGHTED_SCORE_CHANGED signal defined in this module. Converts the + unicode keys for user, course and item into the standard representation for the + PROBLEM_WEIGHTED_SCORE_CHANGED signal. This method expects that the kwargs dictionary will contain the following entries (See the definition of score_set): @@ -42,10 +48,10 @@ def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-a if user is None: return - PROBLEM_SCORE_CHANGED.send( + PROBLEM_WEIGHTED_SCORE_CHANGED.send( sender=None, - points_earned=points_earned, - points_possible=points_possible, + weighted_earned=points_earned, + weighted_possible=points_possible, user_id=user.id, course_id=course_id, usage_id=usage_id @@ -56,9 +62,9 @@ def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-a def submissions_score_reset_handler(sender, **kwargs): # pylint: disable=unused-argument """ Consume the score_reset signal defined in the Submissions API, and convert - it to a PROBLEM_SCORE_CHANGED signal indicating that the score has been set to 0/0. - Converts the unicode keys for user, course and item into the standard - representation for the PROBLEM_SCORE_CHANGED signal. + it to a PROBLEM_WEIGHTED_SCORE_CHANGED signal indicating that the score + has been set to 0/0. Converts the unicode keys for user, course and item + into the standard representation for the PROBLEM_WEIGHTED_SCORE_CHANGED signal. This method expects that the kwargs dictionary will contain the following entries (See the definition of score_reset): @@ -72,10 +78,10 @@ def submissions_score_reset_handler(sender, **kwargs): # pylint: disable=unused if user is None: return - PROBLEM_SCORE_CHANGED.send( + PROBLEM_WEIGHTED_SCORE_CHANGED.send( sender=None, - points_earned=0, - points_possible=0, + weighted_earned=0, + weighted_possible=0, user_id=user.id, course_id=course_id, usage_id=usage_id @@ -106,10 +112,11 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ if update_score: set_score(user.id, block.location, raw_earned, raw_possible) - PROBLEM_SCORE_CHANGED.send( + PROBLEM_RAW_SCORE_CHANGED.send( sender=None, - points_earned=raw_earned, - points_possible=raw_possible, + raw_earned=raw_earned, + raw_possible=raw_possible, + weight=getattr(block, 'weight', None), user_id=user.id, course_id=unicode(block.location.course_key), usage_id=unicode(block.location), @@ -118,10 +125,38 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ return update_score -@receiver(PROBLEM_SCORE_CHANGED) +@receiver(PROBLEM_RAW_SCORE_CHANGED) +def problem_raw_score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument + """ + Handles the raw score changed signal, converting the score to a + weighted score and firing the PROBLEM_WEIGHTED_SCORE_CHANGED signal. + """ + if kwargs['raw_possible'] is not None: + weighted_earned, weighted_possible = weighted_score( + kwargs['raw_earned'], + kwargs['raw_possible'], + kwargs['weight'], + ) + else: # TODO: remove as part of TNL-5982 + weighted_earned, weighted_possible = kwargs['raw_earned'], kwargs['raw_possible'] + + PROBLEM_WEIGHTED_SCORE_CHANGED.send( + sender=None, + weighted_earned=weighted_earned, + weighted_possible=weighted_possible, + user_id=kwargs['user_id'], + course_id=kwargs['course_id'], + usage_id=kwargs['usage_id'], + only_if_higher=kwargs['only_if_higher'], + score_deleted=kwargs.get('score_deleted', False), + ) + + +@receiver(PROBLEM_WEIGHTED_SCORE_CHANGED) def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argument """ - Handles the PROBLEM_SCORE_CHANGED signal by enqueueing a subsection update operation to occur asynchronously. + Handles the PROBLEM_WEIGHTED_SCORE_CHANGED signal by + enqueueing a subsection update operation to occur asynchronously. """ result = recalculate_subsection_grade.apply_async( kwargs=dict( @@ -129,8 +164,8 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum course_id=kwargs['course_id'], usage_id=kwargs['usage_id'], only_if_higher=kwargs.get('only_if_higher'), - raw_earned=kwargs.get('points_earned'), - raw_possible=kwargs.get('points_possible'), + weighted_earned=kwargs.get('weighted_earned'), + weighted_possible=kwargs.get('weighted_possible'), score_deleted=kwargs.get('score_deleted', False), ) ) diff --git a/lms/djangoapps/grades/signals/signals.py b/lms/djangoapps/grades/signals/signals.py index 0f0465f62b..eba96e15f2 100644 --- a/lms/djangoapps/grades/signals/signals.py +++ b/lms/djangoapps/grades/signals/signals.py @@ -4,19 +4,39 @@ Grades related signals. from django.dispatch import Signal -# Signal that indicates that a user's score for a problem has been updated. -# This signal is generated when a scoring event occurs either within the core -# platform or in the Submissions module. Note that this signal will be triggered +# Signal that indicates that a user's raw score for a problem has been updated. +# This signal is generated when a scoring event occurs within the core +# platform. Note that this signal will be triggered # regardless of the new and previous values of the score (i.e. it may be the # case that this signal is generated when a user re-attempts a problem but # receives the same score). -PROBLEM_SCORE_CHANGED = Signal( +PROBLEM_RAW_SCORE_CHANGED = Signal( providing_args=[ 'user_id', # Integer User ID 'course_id', # Unicode string representing the course 'usage_id', # Unicode string indicating the courseware instance - 'points_earned', # Score obtained by the user - 'points_possible', # Maximum score available for the exercise + 'raw_earned', # Score obtained by the user + 'raw_possible', # Maximum score available for the exercise + 'weight', # Weight of the problem + 'only_if_higher', # Boolean indicating whether updates should be + # made only if the new score is higher than previous. + ] +) + +# Signal that indicates that a user's weighted score for a problem has been updated. +# This signal is generated when a scoring event occurs in the Submissions module +# or a PROBLEM_RAW_SCORE_CHANGED event is handled in the core platform. +# Note that this signal will be triggered +# regardless of the new and previous values of the score (i.e. it may be the +# case that this signal is generated when a user re-attempts a problem but +# receives the same score). +PROBLEM_WEIGHTED_SCORE_CHANGED = Signal( + providing_args=[ + 'user_id', # Integer User ID + 'course_id', # Unicode string representing the course + 'usage_id', # Unicode string indicating the courseware instance + 'weighted_earned', # Score obtained by the user + 'weighted_possible', # Maximum score available for the exercise 'only_if_higher', # Boolean indicating whether updates should be # made only if the new score is higher than previous. ] @@ -25,8 +45,8 @@ PROBLEM_SCORE_CHANGED = Signal( # Signal that indicates that a user's score for a problem has been published # for possible persistence and update. Typically, most clients should listen -# to the PROBLEM_SCORE_CHANGED signal instead, since that is signalled only after the -# problem's score is changed. +# to the PROBLEM_WEIGHTED_SCORE_CHANGED signal instead, since that is signalled +# only after the problem's score is changed. SCORE_PUBLISHED = Signal( providing_args=[ 'block', # Course block object @@ -40,8 +60,8 @@ SCORE_PUBLISHED = Signal( # Signal that indicates that a user's score for a subsection has been updated. -# This is a downstream signal of PROBLEM_SCORE_CHANGED sent for each affected containing -# subsection. +# This is a downstream signal of PROBLEM_WEIGHTED_SCORE_CHANGED sent for each +# affected containing subsection. SUBSECTION_SCORE_CHANGED = Signal( providing_args=[ 'course', # Course object diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 052f9ed556..83171e4522 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -23,7 +23,9 @@ log = getLogger(__name__) @task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) -def recalculate_subsection_grade(user_id, course_id, usage_id, only_if_higher, raw_earned, raw_possible, **kwargs): +def recalculate_subsection_grade( + user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, **kwargs +): """ Updates a saved subsection grade. @@ -34,9 +36,9 @@ def recalculate_subsection_grade(user_id, course_id, usage_id, only_if_higher, r only_if_higher (boolean): indicating whether grades should be updated only if the new raw_earned is higher than the previous value. - raw_earned (float): the raw points the learner earned on the + weighted_earned (float): the weighted points the learner earned on the problem that triggered the update. - raw_possible (float): the max raw points the leaner could have + weighted_possible (float): the max weighted points the leaner could have earned on the problem. score_deleted (boolean): indicating whether the grade change is a result of the problem's score being deleted. @@ -53,10 +55,10 @@ def recalculate_subsection_grade(user_id, course_id, usage_id, only_if_higher, r # creator's process hasn't committed before the task initiates in the worker # process. if not _has_database_updated_with_new_score( - user_id, scored_block_usage_key, raw_earned, raw_possible, score_deleted, + user_id, scored_block_usage_key, weighted_earned, weighted_possible, score_deleted, ): raise _retry_recalculate_subsection_grade( - user_id, course_id, usage_id, only_if_higher, raw_earned, raw_possible, score_deleted, + user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, score_deleted, ) _update_subsection_grades( @@ -66,8 +68,8 @@ def recalculate_subsection_grade(user_id, course_id, usage_id, only_if_higher, r course_id, user_id, usage_id, - raw_earned, - raw_possible, + weighted_earned, + weighted_possible, score_deleted, ) @@ -100,8 +102,8 @@ def _update_subsection_grades( course_id, user_id, usage_id, - raw_earned, - raw_possible, + weighted_earned, + weighted_possible, score_deleted, ): """ @@ -138,12 +140,12 @@ def _update_subsection_grades( except DatabaseError as exc: raise _retry_recalculate_subsection_grade( - user_id, course_id, usage_id, only_if_higher, raw_earned, raw_possible, score_deleted, exc, + user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, score_deleted, exc, ) def _retry_recalculate_subsection_grade( - user_id, course_id, usage_id, only_if_higher, raw_earned, raw_possible, score_deleted, exc=None, + user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, score_deleted, exc=None, ): """ Calls retry for the recalculate_subsection_grade task with the @@ -155,8 +157,8 @@ def _retry_recalculate_subsection_grade( course_id=course_id, usage_id=usage_id, only_if_higher=only_if_higher, - raw_earned=raw_earned, - raw_possible=raw_possible, + weighted_earned=weighted_earned, + weighted_possible=weighted_possible, score_deleted=score_deleted, ), exc=exc, diff --git a/lms/djangoapps/grades/tests/test_signals.py b/lms/djangoapps/grades/tests/test_signals.py index f280bd76f6..59acf800ac 100644 --- a/lms/djangoapps/grades/tests/test_signals.py +++ b/lms/djangoapps/grades/tests/test_signals.py @@ -12,6 +12,7 @@ from ..signals.handlers import ( enqueue_subsection_update, submissions_score_set_handler, submissions_score_reset_handler, + problem_raw_score_changed_handler, ) UUID_REGEX = re.compile(ur'%(hex)s{8}-%(hex)s{4}-%(hex)s{4}-%(hex)s{4}-%(hex)s{12}' % {'hex': u'[0-9a-f]'}) @@ -31,22 +32,39 @@ SUBMISSION_RESET_KWARGS = { 'item_id': 'i4x://org/course/usage/123456' } +PROBLEM_RAW_SCORE_CHANGED_KWARGS = { + 'raw_earned': 1.0, + 'raw_possible': 2.0, + 'weight': 4, + 'user_id': 'UserID', + 'course_id': 'CourseID', + 'usage_id': 'i4x://org/course/usage/123456', + 'only_if_higher': False, + 'score_deleted': True, +} + @ddt.ddt -class SubmissionSignalRelayTest(TestCase): +class ScoreChangedSignalRelayTest(TestCase): """ - Tests to ensure that the courseware module correctly catches score_set and - score_reset signals from the Submissions API and recasts them as LMS - signals. This ensures that listeners in the LMS only have to handle one type - of signal for all scoring events. + Tests to ensure that the courseware module correctly catches + (a) score_set and score_reset signals from the Submissions API + (b) LMS PROBLEM_RAW_SCORE_CHANGED signals + and recasts them as LMS PROBLEM_WEIGHTED_SCORE_CHANGED signals. + + This ensures that listeners in the LMS only have to handle one type + of signal for all scoring events regardless of their origin. """ def setUp(self): """ Configure mocks for all the dependencies of the render method """ - super(SubmissionSignalRelayTest, self).setUp() - self.signal_mock = self.setup_patch('lms.djangoapps.grades.signals.signals.PROBLEM_SCORE_CHANGED.send', None) + super(ScoreChangedSignalRelayTest, self).setUp() + self.signal_mock = self.setup_patch( + 'lms.djangoapps.grades.signals.signals.PROBLEM_WEIGHTED_SCORE_CHANGED.send', + None, + ) self.user_mock = MagicMock() self.user_mock.id = 42 self.get_user_mock = self.setup_patch( @@ -72,15 +90,16 @@ class SubmissionSignalRelayTest(TestCase): def test_score_set_signal_handler(self, handler, kwargs, earned, possible): """ Ensure that on receipt of a score_(re)set signal from the Submissions API, - the signal handler correctly converts it to a PROBLEM_SCORE_CHANGED signal. + the signal handler correctly converts it to a PROBLEM_WEIGHTED_SCORE_CHANGED + signal. Also ensures that the handler calls user_by_anonymous_id correctly. """ handler(None, **kwargs) expected_set_kwargs = { 'sender': None, - 'points_possible': possible, - 'points_earned': earned, + 'weighted_possible': possible, + 'weighted_earned': earned, 'user_id': self.user_mock.id, 'course_id': 'CourseID', 'usage_id': 'i4x://org/course/usage/123456' @@ -122,6 +141,36 @@ class SubmissionSignalRelayTest(TestCase): handler(None, **kwargs) self.signal_mock.assert_not_called() + def test_raw_score_changed_signal_handler(self): + problem_raw_score_changed_handler(None, **PROBLEM_RAW_SCORE_CHANGED_KWARGS) + expected_set_kwargs = { + 'sender': None, + 'weighted_earned': 2.0, + 'weighted_possible': 4.0, + 'user_id': 'UserID', + 'course_id': 'CourseID', + 'usage_id': 'i4x://org/course/usage/123456', + 'only_if_higher': False, + 'score_deleted': True, + } + self.signal_mock.assert_called_with(**expected_set_kwargs) + + def test_raw_score_changed_score_deleted_optional(self): + local_kwargs = PROBLEM_RAW_SCORE_CHANGED_KWARGS.copy() + del local_kwargs['score_deleted'] + problem_raw_score_changed_handler(None, **local_kwargs) + expected_set_kwargs = { + 'sender': None, + 'weighted_earned': 2.0, + 'weighted_possible': 4.0, + 'user_id': 'UserID', + 'course_id': 'CourseID', + 'usage_id': 'i4x://org/course/usage/123456', + 'only_if_higher': False, + 'score_deleted': False, + } + self.signal_mock.assert_called_with(**expected_set_kwargs) + @patch('lms.djangoapps.grades.signals.handlers.log.info') def test_problem_score_changed_logging(self, mocklog): enqueue_subsection_update( diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index f205ddf9b6..3b8d520aea 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -18,7 +18,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag -from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED +from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED from lms.djangoapps.grades.tasks import recalculate_subsection_grade @@ -50,9 +50,9 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): self.sequential = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Sequential1") self.problem = ItemFactory.create(parent=self.sequential, category='problem', display_name='Problem') - self.problem_score_changed_kwargs = OrderedDict([ - ('points_earned', 1.0), - ('points_possible', 2.0), + self.problem_weighted_score_changed_kwargs = OrderedDict([ + ('weighted_earned', 1.0), + ('weighted_possible', 2.0), ('user_id', self.user.id), ('course_id', unicode(self.course.id)), ('usage_id', unicode(self.problem.location)), @@ -64,8 +64,8 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ('course_id', unicode(self.course.id)), ('usage_id', unicode(self.problem.location)), ('only_if_higher', None), - ('raw_earned', 1.0), - ('raw_possible', 2.0), + ('weighted_earned', 1.0), + ('weighted_possible', 2.0), ('score_deleted', False), ]) @@ -82,17 +82,17 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): with patch("lms.djangoapps.grades.tasks.get_score", return_value=score): yield - def test_problem_score_changed_queues_task(self): + def test_problem_weighted_score_changed_queues_task(self): """ - Ensures that the PROBLEM_SCORE_CHANGED signal enqueues the correct task. + Ensures that the PROBLEM_WEIGHTED_SCORE_CHANGED signal enqueues the correct task. """ self.set_up_course() - send_args = self.problem_score_changed_kwargs + send_args = self.problem_weighted_score_changed_kwargs with self.mock_get_score() and patch( 'lms.djangoapps.grades.tasks.recalculate_subsection_grade.apply_async', return_value=None ) as mock_task_apply: - PROBLEM_SCORE_CHANGED.send(sender=None, **send_args) + PROBLEM_WEIGHTED_SCORE_CHANGED.send(sender=None, **send_args) mock_task_apply.assert_called_once_with(kwargs=self.recalculate_subsection_grade_kwargs) @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index fc0428d729..cd0b4327a8 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -15,8 +15,7 @@ from django.utils.translation import override as override_language from course_modes.models import CourseMode from courseware.models import StudentModule from edxmako.shortcuts import render_to_string -from lms.djangoapps.grades.scores import weighted_score -from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED +from lms.djangoapps.grades.signals.signals import PROBLEM_RAW_SCORE_CHANGED from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.user_api.models import UserPreference @@ -290,28 +289,22 @@ def _reset_module_attempts(studentmodule): def _fire_score_changed_for_block(course_id, student, block, module_state_key): """ - Fires a PROBLEM_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. + Fires a PROBLEM_RAW_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: - max_score = block.max_score() - if max_score is None: - return - else: - points_earned, points_possible = weighted_score(0, max_score, getattr(block, 'weight', None)) - else: - points_earned, points_possible = 0, 0 - - PROBLEM_SCORE_CHANGED.send( - sender=None, - points_possible=points_possible, - points_earned=points_earned, - user_id=student.id, - course_id=unicode(course_id), - usage_id=unicode(module_state_key), - score_deleted=True, - ) + if block and block.has_score and block.max_score() is not None: + PROBLEM_RAW_SCORE_CHANGED.send( + sender=None, + raw_possible=0, + raw_earned=block.max_score(), + weight=getattr(block, 'weight', None), + user_id=student.id, + course_id=unicode(course_id), + usage_id=unicode(module_state_key), + score_deleted=True, + only_if_higher=False, + ) def get_email_params(course, auto_enroll, secure=True, course_key=None, display_name=None): diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 9c46d7491d..e0bb7e21a9 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -3190,7 +3190,7 @@ class TestInstructorAPIRegradeTask(SharedModuleStoreTestCase, LoginEnrollmentTes }) self.assertEqual(response.status_code, 400) - @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_SCORE_CHANGED.send') + @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send') def test_reset_student_attempts_delete(self, _mock_signal): """ Test delete single student state. """ url = reverse('reset_student_attempts', kwargs={'course_id': self.course.id.to_deprecated_string()}) diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 97b7a7f703..20805a90bd 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -374,7 +374,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): reset_student_attempts(self.course_key, self.user, msk, requesting_user=self.user) self.assertEqual(json.loads(module().state)['attempts'], 0) - @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_SCORE_CHANGED.send') + @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send') def test_delete_student_attempts(self, _mock_signal): msk = self.course_key.make_usage_key('dummy', 'module') original_state = json.dumps({'attempts': 32, 'otherstuff': 'alsorobots'}) @@ -400,7 +400,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): # Disable the score change signal to prevent other components from being # pulled into tests. - @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_SCORE_CHANGED.send') + @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send') def test_delete_submission_scores(self, _mock_signal): user = UserFactory() problem_location = self.course_key.make_usage_key('dummy', 'module') diff --git a/lms/djangoapps/instructor/tests/test_services.py b/lms/djangoapps/instructor/tests/test_services.py index a460ad9566..09f824a154 100644 --- a/lms/djangoapps/instructor/tests/test_services.py +++ b/lms/djangoapps/instructor/tests/test_services.py @@ -49,7 +49,7 @@ class InstructorServiceTests(SharedModuleStoreTestCase): state=json.dumps({'attempts': 2}), ) - @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_SCORE_CHANGED.send') + @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send') def test_reset_student_attempts_delete(self, _mock_signal): """ Test delete student state. diff --git a/lms/djangoapps/lti_provider/tasks.py b/lms/djangoapps/lti_provider/tasks.py index 4d48a55ce4..396e462d2b 100644 --- a/lms/djangoapps/lti_provider/tasks.py +++ b/lms/djangoapps/lti_provider/tasks.py @@ -8,7 +8,7 @@ from django.dispatch import receiver import logging from lms.djangoapps.grades import progress -from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED +from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED from lms import CELERY_APP from lti_provider.models import GradedAssignment import lti_provider.outcomes as outcomes @@ -19,14 +19,14 @@ from xmodule.modulestore.django import modulestore log = logging.getLogger("edx.lti_provider") -@receiver(PROBLEM_SCORE_CHANGED) +@receiver(PROBLEM_WEIGHTED_SCORE_CHANGED) def score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument """ Consume signals that indicate score changes. See the definition of - PROBLEM_SCORE_CHANGED for a description of the signal. + PROBLEM_WEIGHTED_SCORE_CHANGED for a description of the signal. """ - points_possible = kwargs.get('points_possible', None) - points_earned = kwargs.get('points_earned', None) + points_possible = kwargs.get('weighted_possible', None) + points_earned = kwargs.get('weighted_earned', None) user_id = kwargs.get('user_id', None) course_id = kwargs.get('course_id', None) usage_id = kwargs.get('usage_id', None)