From 6a47311479c11021de728808a04eb6d64e86f3b2 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Fri, 14 Oct 2016 15:54:03 -0400 Subject: [PATCH 1/2] Update Persisted Course Grade on Subsection Grade update Makes use of the new SUBSECTION_SCORE_CHANGED signal to trigger a task that updates persisted course grade values. We've also renamed SCORE_CHANGED to PROBLEM_SCORE_CHANGED to head off any issues with unclear signal names. TNL-5740 --- .../courseware/tests/test_module_render.py | 2 +- .../tests/test_submitting_problems.py | 4 +- lms/djangoapps/gating/signals.py | 6 +- lms/djangoapps/grades/new/course_grade.py | 9 +- lms/djangoapps/grades/signals/handlers.py | 36 +++-- lms/djangoapps/grades/signals/signals.py | 6 +- lms/djangoapps/grades/tasks.py | 21 ++- lms/djangoapps/grades/tests/test_new.py | 2 + lms/djangoapps/grades/tests/test_signals.py | 4 +- lms/djangoapps/grades/tests/test_tasks.py | 123 +++++++++++++++--- lms/djangoapps/instructor/enrollment.py | 6 +- 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 | 6 +- 15 files changed, 182 insertions(+), 51 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 7b89397c40..750e56f544 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1831,7 +1831,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): self.assertIsNone(student_module.grade) self.assertIsNone(student_module.max_grade) - @patch('lms.djangoapps.grades.signals.handlers.SCORE_CHANGED.send') + @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_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) diff --git a/lms/djangoapps/courseware/tests/test_submitting_problems.py b/lms/djangoapps/courseware/tests/test_submitting_problems.py index 046f356041..b92a3d1ff5 100644 --- a/lms/djangoapps/courseware/tests/test_submitting_problems.py +++ b/lms/djangoapps/courseware/tests/test_submitting_problems.py @@ -153,7 +153,7 @@ 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.SCORE_CHANGED.send') + self.score_changed_signal_patch = patch('lms.djangoapps.grades.signals.handlers.PROBLEM_SCORE_CHANGED.send') self.score_changed_signal_patch.start() def tearDown(self): @@ -162,7 +162,7 @@ class TestSubmittingProblems(ModuleStoreTestCase, LoginEnrollmentTestCase, Probl def _stop_signal_patch(self): """ - Stops the signal patch for the SCORE_CHANGED event. + Stops the signal patch for the PROBLEM_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 796e0ef756..554ab642ab 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 SCORE_CHANGED, SUBSECTION_SCORE_CHANGED +from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED, SUBSECTION_SCORE_CHANGED from opaque_keys.edx.keys import CourseKey, UsageKey from xmodule.modulestore.django import modulestore -@receiver(SCORE_CHANGED) +@receiver(PROBLEM_SCORE_CHANGED) def handle_score_changed(**kwargs): """ - Receives the SCORE_CHANGED signal sent by LMS when a student's score has changed + Receives the PROBLEM_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/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index f3aa5c0581..5af49dec85 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/lms/djangoapps/grades/new/course_grade.py @@ -260,7 +260,14 @@ class CourseGradeFactory(object): self._compute_and_update_grade(course, course_structure, read_only) ) - def _compute_and_update_grade(self, course, course_structure, read_only): + def update(self, course): + """ + Updates the CourseGrade for this Factory's student. + """ + course_structure = get_course_blocks(self.student, course.location) + self._compute_and_update_grade(course, course_structure) + + def _compute_and_update_grade(self, course, course_structure, read_only=False): """ Freshly computes and updates the grade for the student and course. diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index 00c090b2da..bebb14456b 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -2,6 +2,7 @@ Grades related signals. """ +from celery import Task from django.dispatch import receiver from logging import getLogger @@ -10,8 +11,8 @@ 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 SCORE_CHANGED, SCORE_PUBLISHED -from ..tasks import recalculate_subsection_grade +from .signals import PROBLEM_SCORE_CHANGED, SUBSECTION_SCORE_CHANGED, SCORE_PUBLISHED +from ..tasks import recalculate_subsection_grade, recalculate_course_grade log = getLogger(__name__) @@ -21,9 +22,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 SCORE_CHANGED signal defined in this module. Converts the unicode keys + 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 - SCORE_CHANGED signal. + PROBLEM_SCORE_CHANGED signal. This method expects that the kwargs dictionary will contain the following entries (See the definition of score_set): @@ -41,7 +42,7 @@ def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-a if user is None: return - SCORE_CHANGED.send( + PROBLEM_SCORE_CHANGED.send( sender=None, points_earned=points_earned, points_possible=points_possible, @@ -55,9 +56,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 SCORE_CHANGED signal indicating that the score has been set to 0/0. + 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 SCORE_CHANGED signal. + representation for the PROBLEM_SCORE_CHANGED signal. This method expects that the kwargs dictionary will contain the following entries (See the definition of score_reset): @@ -71,7 +72,7 @@ def submissions_score_reset_handler(sender, **kwargs): # pylint: disable=unused if user is None: return - SCORE_CHANGED.send( + PROBLEM_SCORE_CHANGED.send( sender=None, points_earned=0, points_possible=0, @@ -106,7 +107,7 @@ 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) - SCORE_CHANGED.send( + PROBLEM_SCORE_CHANGED.send( sender=None, points_earned=raw_earned, points_possible=raw_possible, @@ -118,10 +119,10 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ return update_score -@receiver(SCORE_CHANGED) -def enqueue_grade_update(sender, **kwargs): # pylint: disable=unused-argument +@receiver(PROBLEM_SCORE_CHANGED) +def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argument """ - Handles the SCORE_CHANGED signal by enqueueing an update operation to occur asynchronously. + Handles the PROBLEM_SCORE_CHANGED signal by enqueueing a subsection update operation to occur asynchronously. """ recalculate_subsection_grade.apply_async( args=( @@ -131,3 +132,14 @@ def enqueue_grade_update(sender, **kwargs): # pylint: disable=unused-argument kwargs.get('only_if_higher'), ) ) + + +@receiver(SUBSECTION_SCORE_CHANGED) +def enqueue_course_update(sender, **kwargs): # pylint: disable=unused-argument + """ + Handles the SUBSECTION_SCORE_CHANGED signal by enqueueing a course update operation to occur asynchronously. + """ + if isinstance(sender, Task): # We're already in a async worker, just do the task + recalculate_course_grade.apply(args=(kwargs['user'].id, unicode(kwargs['course'].id))) + else: # Otherwise, queue the work to be done asynchronously + recalculate_course_grade.apply_async(args=(kwargs['user'].id, unicode(kwargs['course'].id))) diff --git a/lms/djangoapps/grades/signals/signals.py b/lms/djangoapps/grades/signals/signals.py index 774de107f8..8418f90ca5 100644 --- a/lms/djangoapps/grades/signals/signals.py +++ b/lms/djangoapps/grades/signals/signals.py @@ -10,7 +10,7 @@ from django.dispatch import Signal # 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). -SCORE_CHANGED = Signal( +PROBLEM_SCORE_CHANGED = Signal( providing_args=[ 'user_id', # Integer User ID 'course_id', # Unicode string representing the course @@ -25,7 +25,7 @@ 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 SCORE_CHANGED signal instead, since that is signalled only after the +# to the PROBLEM_SCORE_CHANGED signal instead, since that is signalled only after the # problem's score is changed. SCORE_PUBLISHED = Signal( providing_args=[ @@ -40,7 +40,7 @@ SCORE_PUBLISHED = Signal( # Signal that indicates that a user's score for a subsection has been updated. -# This is a downstream signal of SCORE_CHANGED sent for each affected containing +# This is a downstream signal of PROBLEM_SCORE_CHANGED sent for each affected containing # subsection. SUBSECTION_SCORE_CHANGED = Signal( providing_args=[ diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index b5798a8cb7..f65815389f 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -13,6 +13,7 @@ from opaque_keys.edx.locator import CourseLocator from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache from xmodule.modulestore.django import modulestore +from .new.course_grade import CourseGradeFactory from .new.subsection_grade import SubsectionGradeFactory from .signals.signals import SUBSECTION_SCORE_CHANGED from .transformer import GradesTransformer @@ -57,7 +58,7 @@ def recalculate_subsection_grade(user_id, course_id, usage_id, only_if_higher): only_if_higher, ) SUBSECTION_SCORE_CHANGED.send( - sender=None, + sender=recalculate_subsection_grade, course=course, user=student, subsection_grade=subsection_grade, @@ -65,3 +66,21 @@ def recalculate_subsection_grade(user_id, course_id, usage_id, only_if_higher): except IntegrityError as exc: raise recalculate_subsection_grade.retry(args=[user_id, course_id, usage_id], exc=exc) + + +@task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) +def recalculate_course_grade(user_id, course_id): + """ + Updates a saved course grade. + This method expects the following parameters: + - user_id: serialized id of applicable User object + - course_id: Unicode string representing the course + """ + student = User.objects.get(id=user_id) + course_key = CourseLocator.from_string(course_id) + course = modulestore().get_course(course_key, depth=0) + + try: + CourseGradeFactory(student).update(course) + except IntegrityError as exc: + raise recalculate_course_grade.retry(args=[user_id, course_id], exc=exc) diff --git a/lms/djangoapps/grades/tests/test_new.py b/lms/djangoapps/grades/tests/test_new.py index e5b4399ae3..133302ffc2 100644 --- a/lms/djangoapps/grades/tests/test_new.py +++ b/lms/djangoapps/grades/tests/test_new.py @@ -574,6 +574,8 @@ class TestCourseGradeLogging(SharedModuleStoreTestCase): enabled_for_course=True ): with patch('lms.djangoapps.grades.new.course_grade.log') as log_mock: + # TODO: once merged with the "glue code" PR, update expected logging to include the relevant new info + # the course grade has not been created, so we expect each grade to be created self._create_course_grade_and_check_logging( grade_factory, diff --git a/lms/djangoapps/grades/tests/test_signals.py b/lms/djangoapps/grades/tests/test_signals.py index a79e951ec2..13afaeb326 100644 --- a/lms/djangoapps/grades/tests/test_signals.py +++ b/lms/djangoapps/grades/tests/test_signals.py @@ -42,7 +42,7 @@ class SubmissionSignalRelayTest(TestCase): 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.SCORE_CHANGED.send', None) + self.signal_mock = self.setup_patch('lms.djangoapps.grades.signals.signals.PROBLEM_SCORE_CHANGED.send', None) self.user_mock = MagicMock() self.user_mock.id = 42 self.get_user_mock = self.setup_patch( @@ -68,7 +68,7 @@ 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 SCORE_CHANGED signal. + the signal handler correctly converts it to a PROBLEM_SCORE_CHANGED signal. Also ensures that the handler calls user_by_anonymous_id correctly. """ diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 861e4672fe..8066c17dd9 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -7,17 +7,20 @@ import ddt from django.conf import settings from django.db.utils import IntegrityError from mock import patch +from uuid import uuid4 from unittest import skip +from opaque_keys.edx.locator import CourseLocator from student.models import anonymous_id_for_user from student.tests.factories import UserFactory +from xmodule.modulestore.django import modulestore from xmodule.modulestore import ModuleStoreEnum 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 SCORE_CHANGED -from lms.djangoapps.grades.tasks import recalculate_subsection_grade +from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED, SUBSECTION_SCORE_CHANGED +from lms.djangoapps.grades.tasks import recalculate_course_grade, recalculate_subsection_grade @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @@ -59,24 +62,91 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): _ = anonymous_id_for_user(self.user, self.course.id) # pylint: enable=attribute-defined-outside-init,no-member - def test_score_changed_signal_queues_task(self): + @ddt.data( + ('lms.djangoapps.grades.tasks.recalculate_subsection_grade.apply_async', PROBLEM_SCORE_CHANGED), + ('lms.djangoapps.grades.tasks.recalculate_course_grade.apply_async', SUBSECTION_SCORE_CHANGED) + ) + @ddt.unpack + def test_signal_queues_task(self, enqueue_op, test_signal): """ - Ensures that the SCORE_CHANGED signal enqueues a recalculate subsection grade task. + Ensures that the PROBLEM_SCORE_CHANGED and SUBSECTION_SCORE_CHANGED signals enqueue the correct tasks. """ self.set_up_course() + if test_signal == PROBLEM_SCORE_CHANGED: + send_args = self.score_changed_kwargs + expected_args = tuple(self.score_changed_kwargs.values()) + else: + send_args = {'user': self.user, 'course': self.course} + expected_args = (self.score_changed_kwargs['user_id'], self.score_changed_kwargs['course_id']) with patch( - 'lms.djangoapps.grades.tasks.recalculate_subsection_grade.apply_async', + enqueue_op, return_value=None ) as mock_task_apply: - SCORE_CHANGED.send(sender=None, **self.score_changed_kwargs) - mock_task_apply.assert_called_once_with(args=tuple(self.score_changed_kwargs.values())) + test_signal.send(sender=None, **send_args) + mock_task_apply.assert_called_once_with(args=expected_args) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_subsection_grade_updated(self, default_store): + @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') + def test_subsection_update_triggers_course_update(self, mock_course_signal): + """ + Ensures that the subsection update operation also updates the course grade. + """ + self.set_up_course() + mock_return = uuid4() + course_key = CourseLocator.from_string(unicode(self.course.id)) + course = modulestore().get_course(course_key, depth=0) + with patch( + 'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update', + return_value=mock_return + ): + recalculate_subsection_grade.apply(args=tuple(self.score_changed_kwargs.values())) + mock_course_signal.assert_called_once_with( + sender=recalculate_subsection_grade, + course=course, + user=self.user, + subsection_grade=mock_return, + ) + + @ddt.data(True, False) + def test_course_update_enqueuing(self, should_be_async): + """ + Ensures that the course update operation is enqueued on an async queue (or not) as expected. + """ + base = 'lms.djangoapps.grades.tasks.recalculate_course_grade' + if should_be_async: + executed = base + '.apply_async' + other = base + '.apply' + sender = None + else: + executed = base + '.apply' + other = base + '.apply_async' + sender = recalculate_subsection_grade + self.set_up_course() + + with patch(executed) as executed_task: + with patch(other) as other_task: + SUBSECTION_SCORE_CHANGED.send( + sender=sender, + course=self.course, + user=self.user, + ) + other_task.assert_not_called() + executed_task.assert_called_once_with( + args=( + self.score_changed_kwargs['user_id'], + self.score_changed_kwargs['course_id'], + ) + ) + + @ddt.data( + (ModuleStoreEnum.Type.mongo, 1), + (ModuleStoreEnum.Type.split, 0), + ) + @ddt.unpack + def test_subsection_grade_updated(self, default_store, added_queries): with self.store.default_store(default_store): self.set_up_course() self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) - with check_mongo_calls(2) and self.assertNumQueries(13): + with check_mongo_calls(2) and self.assertNumQueries(21 + added_queries): recalculate_subsection_grade.apply(args=tuple(self.score_changed_kwargs.values())) def test_single_call_to_create_block_structure(self): @@ -87,16 +157,20 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): return_value=None, ) as mock_block_structure_create: recalculate_subsection_grade.apply(args=tuple(self.score_changed_kwargs.values())) - self.assertEquals(mock_block_structure_create.call_count, 1) + self.assertEquals(mock_block_structure_create.call_count, 2) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_query_count_does_not_change_with_more_problems(self, default_store): + @ddt.data( + (ModuleStoreEnum.Type.mongo, 1), + (ModuleStoreEnum.Type.split, 0), + ) + @ddt.unpack + def test_query_count_does_not_change_with_more_problems(self, default_store, added_queries): with self.store.default_store(default_store): self.set_up_course() self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) ItemFactory.create(parent=self.sequential, category='problem', display_name='problem2') ItemFactory.create(parent=self.sequential, category='problem', display_name='problem3') - with check_mongo_calls(2) and self.assertNumQueries(13): + with check_mongo_calls(2) and self.assertNumQueries(21 + added_queries): recalculate_subsection_grade.apply(args=tuple(self.score_changed_kwargs.values())) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) @@ -104,7 +178,8 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): with self.store.default_store(default_store): self.set_up_course(enable_subsection_grades=False) self.assertFalse(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) - with check_mongo_calls(2) and self.assertNumQueries(5): + additional_queries = 1 if default_store == ModuleStoreEnum.Type.mongo else 0 + with check_mongo_calls(2) and self.assertNumQueries(12 + additional_queries): recalculate_subsection_grade.apply(args=tuple(self.score_changed_kwargs.values())) @skip("Pending completion of TNL-5089") @@ -124,7 +199,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade.retry') @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') - def test_retry_on_integrity_error(self, mock_update, mock_retry): + def test_retry_subsection_update_on_integrity_error(self, mock_update, mock_retry): """ Ensures that tasks will be retried if IntegrityErrors are encountered. """ @@ -132,3 +207,19 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): mock_update.side_effect = IntegrityError("WHAMMY") recalculate_subsection_grade.apply(args=tuple(self.score_changed_kwargs.values())) self.assertTrue(mock_retry.called) + + @patch('lms.djangoapps.grades.tasks.recalculate_course_grade.retry') + @patch('lms.djangoapps.grades.new.course_grade.CourseGradeFactory.update') + def test_retry_course_update_on_integrity_error(self, mock_update, mock_retry): + """ + Ensures that tasks will be retried if IntegrityErrors are encountered. + """ + self.set_up_course() + mock_update.side_effect = IntegrityError("WHAMMY") + recalculate_course_grade.apply( + args=( + self.score_changed_kwargs['user_id'], + self.score_changed_kwargs['course_id'], + ) + ) + self.assertTrue(mock_retry.called) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 732a674803..6e7a62f6e9 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -19,7 +19,7 @@ from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor from edxmako.shortcuts import render_to_string from lms.djangoapps.grades.scores import weighted_score -from lms.djangoapps.grades.signals.signals import SCORE_CHANGED +from lms.djangoapps.grades.signals.signals import PROBLEM_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 @@ -293,7 +293,7 @@ def _reset_module_attempts(studentmodule): 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 + 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. """ @@ -322,7 +322,7 @@ def _fire_score_changed_for_block(course_id, student, block, module_state_key): points_earned, points_possible = weighted_score(0, max_score, getattr(module, 'weight', None)) else: points_earned, points_possible = 0, 0 - SCORE_CHANGED.send( + PROBLEM_SCORE_CHANGED.send( sender=None, points_possible=points_possible, points_earned=points_earned, diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 660612b419..9c46d7491d 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.SCORE_CHANGED.send') + @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_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 779dd54ae7..0b574acaca 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -378,7 +378,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.SCORE_CHANGED.send') + @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_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'}) @@ -404,7 +404,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.SCORE_CHANGED.send') + @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_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 4cc8ab504c..a460ad9566 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.SCORE_CHANGED.send') + @mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_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 692dff1e36..4d48a55ce4 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 SCORE_CHANGED +from lms.djangoapps.grades.signals.signals import PROBLEM_SCORE_CHANGED from lms import CELERY_APP from lti_provider.models import GradedAssignment import lti_provider.outcomes as outcomes @@ -19,11 +19,11 @@ from xmodule.modulestore.django import modulestore log = logging.getLogger("edx.lti_provider") -@receiver(SCORE_CHANGED) +@receiver(PROBLEM_SCORE_CHANGED) def score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument """ Consume signals that indicate score changes. See the definition of - SCORE_CHANGED for a description of the signal. + PROBLEM_SCORE_CHANGED for a description of the signal. """ points_possible = kwargs.get('points_possible', None) points_earned = kwargs.get('points_earned', None) From 563122fe8c3c49d522f82f821041eedb599b8895 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Tue, 25 Oct 2016 11:12:51 -0400 Subject: [PATCH 2/2] Mark Flaky Test --- .../test/acceptance/tests/lms/test_lms_instructor_dashboard.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py index c911d96b6b..19219d7a6a 100644 --- a/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py +++ b/common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py @@ -7,6 +7,7 @@ import ddt from nose.plugins.attrib import attr from bok_choy.promise import EmptyPromise +from flaky import flaky from common.test.acceptance.tests.helpers import UniqueCourseTest, get_modal_alert, EventsTestMixin from common.test.acceptance.pages.common.logout import LogoutPage @@ -377,6 +378,7 @@ class ProctoredExamsTest(BaseInstructorDashboardTest): # Then, the added record should be visible self.assertTrue(allowance_section.is_allowance_record_visible) + @flaky # TNL-5832 def test_can_reset_attempts(self): """ Make sure that Exam attempts are visible and can be reset.