From d0be0ae8ec30c8eb4a758da377afc4f34f459901 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Wed, 7 Dec 2016 14:41:13 -0500 Subject: [PATCH 1/2] ORA2 and edx-submissions releases --- requirements/edx/github.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index dd74515429..e38fbc4b63 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -76,8 +76,8 @@ git+https://github.com/edx/XBlock.git@xblock-0.4.12#egg=XBlock==0.4.12 -e git+https://github.com/edx/event-tracking.git@0.2.1#egg=event-tracking==0.2.1 -e git+https://github.com/edx/django-splash.git@v0.2#egg=django-splash==0.2 -e git+https://github.com/edx/acid-block.git@e46f9cda8a03e121a00c7e347084d142d22ebfb7#egg=acid-xblock -git+https://github.com/edx/edx-ora2.git@1.1.11#egg=ora2==1.1.11 --e git+https://github.com/edx/edx-submissions.git@1.1.1#egg=edx-submissions==1.1.1 +git+https://github.com/edx/edx-ora2.git@1.1.12#egg=ora2==1.1.12 +-e git+https://github.com/edx/edx-submissions.git@1.1.4#egg=edx-submissions==1.1.4 git+https://github.com/edx/ease.git@release-2015-07-14#egg=ease==0.1.3 git+https://github.com/edx/i18n-tools.git@v0.3.2#egg=i18n-tools==v0.3.2 git+https://github.com/edx/edx-val.git@0.0.11#egg=edxval==0.0.11 From 5a98a90eba72c7233d780064d93f87b4c97a8de3 Mon Sep 17 00:00:00 2001 From: Eric Fischer Date: Wed, 7 Dec 2016 14:41:20 -0500 Subject: [PATCH 2/2] Use timestamps in _has_database_updated_with_new_score check Also fixes issue where ORA grades were never updated, by including them in this check. TNL-5994 TNL-5995 --- lms/djangoapps/courseware/model_data.py | 3 +- .../courseware/tests/test_module_render.py | 5 + lms/djangoapps/grades/signals/handlers.py | 21 ++-- lms/djangoapps/grades/signals/signals.py | 4 + lms/djangoapps/grades/tasks.py | 98 +++++++++++++------ .../grades/tests/integration/test_events.py | 6 ++ lms/djangoapps/grades/tests/test_signals.py | 25 +++-- lms/djangoapps/grades/tests/test_tasks.py | 51 +++++++--- lms/djangoapps/instructor/enrollment.py | 11 ++- .../xblock_integration/xblock_testcase.py | 4 + 10 files changed, 163 insertions(+), 65 deletions(-) diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 58e32ca8b0..fe2e88c89e 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -1008,6 +1008,7 @@ def set_score(user_id, usage_key, score, max_score): student_module.grade = score student_module.max_grade = max_score student_module.save() + return student_module.modified def get_score(user_id, usage_key): @@ -1024,4 +1025,4 @@ def get_score(user_id, usage_key): except StudentModule.DoesNotExist: return None else: - return student_module.grade, student_module.max_grade + return student_module diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index c3c817a5f7..d6cf8a5420 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -2,6 +2,7 @@ """ Test for lms courseware app, module render unit """ +from datetime import datetime import ddt import itertools import json @@ -15,10 +16,12 @@ from django.conf import settings from django.test.client import RequestFactory from django.test.utils import override_settings from django.contrib.auth.models import AnonymousUser +from freezegun import freeze_time from mock import MagicMock, patch, Mock from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey from pyquery import PyQuery +import pytz from xblock.field_data import FieldData from xblock.runtime import Runtime from xblock.fields import ScopeIds @@ -1831,6 +1834,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): self.assertIsNone(student_module.grade) self.assertIsNone(student_module.max_grade) + @freeze_time(datetime.now().replace(tzinfo=pytz.UTC)) @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""" @@ -1844,6 +1848,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): 'course_id': unicode(self.course.id), 'usage_id': unicode(self.problem.location), 'only_if_higher': None, + 'modified': datetime.now().replace(tzinfo=pytz.UTC) } send_mock.assert_called_with(**expected_signal_kwargs) diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index e80ec389e7..fa05873c45 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -4,9 +4,11 @@ Grades related signals. from logging import getLogger +from courseware.model_data import get_score, set_score from django.dispatch import receiver from openedx.core.lib.grade_utils import is_score_higher from submissions.models import score_set, score_reset +from util.date_utils import to_timestamp from courseware.model_data import get_score, set_score from eventtracking import tracker @@ -25,7 +27,7 @@ from .signals import ( ) from ..new.course_grade import CourseGradeFactory from ..scores import weighted_score -from ..tasks import recalculate_subsection_grade +from ..tasks import recalculate_subsection_grade_v2 log = getLogger(__name__) @@ -62,7 +64,8 @@ def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-a weighted_possible=points_possible, user_id=user.id, course_id=course_id, - usage_id=usage_id + usage_id=usage_id, + modified=kwargs['created_at'], ) @@ -92,7 +95,8 @@ def submissions_score_reset_handler(sender, **kwargs): # pylint: disable=unused weighted_possible=0, user_id=user.id, course_id=course_id, - usage_id=usage_id + usage_id=usage_id, + modified=kwargs['created_at'], ) @@ -107,7 +111,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ previous_score = get_score(user.id, block.location) if previous_score is not None: - prev_raw_earned, prev_raw_possible = previous_score # pylint: disable=unpacking-non-sequence + prev_raw_earned, prev_raw_possible = (previous_score.grade, previous_score.max_grade) if not is_score_higher(prev_raw_earned, prev_raw_possible, raw_earned, raw_possible): update_score = False @@ -119,7 +123,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_modified_time = set_score(user.id, block.location, raw_earned, raw_possible) PROBLEM_RAW_SCORE_CHANGED.send( sender=None, raw_earned=raw_earned, @@ -129,6 +133,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ course_id=unicode(block.location.course_key), usage_id=unicode(block.location), only_if_higher=only_if_higher, + modified=score_modified_time, ) return update_score @@ -157,6 +162,7 @@ def problem_raw_score_changed_handler(sender, **kwargs): # pylint: disable=unus usage_id=kwargs['usage_id'], only_if_higher=kwargs['only_if_higher'], score_deleted=kwargs.get('score_deleted', False), + modified=kwargs['modified'], ) @@ -167,14 +173,13 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum enqueueing a subsection update operation to occur asynchronously. """ _emit_problem_submitted_event(kwargs) - result = recalculate_subsection_grade.apply_async( + result = recalculate_subsection_grade_v2.apply_async( kwargs=dict( user_id=kwargs['user_id'], course_id=kwargs['course_id'], usage_id=kwargs['usage_id'], only_if_higher=kwargs.get('only_if_higher'), - weighted_earned=kwargs.get('weighted_earned'), - weighted_possible=kwargs.get('weighted_possible'), + expected_modified_time=to_timestamp(kwargs['modified']), score_deleted=kwargs.get('score_deleted', False), event_transaction_id=unicode(get_event_transaction_id()), event_transaction_type=unicode(get_event_transaction_type()), diff --git a/lms/djangoapps/grades/signals/signals.py b/lms/djangoapps/grades/signals/signals.py index eba96e15f2..a2c3c21a00 100644 --- a/lms/djangoapps/grades/signals/signals.py +++ b/lms/djangoapps/grades/signals/signals.py @@ -20,6 +20,8 @@ PROBLEM_RAW_SCORE_CHANGED = Signal( 'weight', # Weight of the problem 'only_if_higher', # Boolean indicating whether updates should be # made only if the new score is higher than previous. + 'modified', # A datetime indicating when the database representation of + # this the problem score was saved. ] ) @@ -39,6 +41,8 @@ PROBLEM_WEIGHTED_SCORE_CHANGED = Signal( '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. + 'modified', # A datetime indicating when the database representation of + # this the problem score was saved. ] ) diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 238fd21334..36583450f7 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -8,15 +8,19 @@ from django.contrib.auth.models import User from django.db.utils import DatabaseError from logging import getLogger +from courseware.model_data import get_score from lms.djangoapps.course_blocks.api import get_course_blocks from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import CourseLocator +from submissions import api as sub_api +from student.models import anonymous_id_for_user from track.event_transaction_utils import ( set_event_transaction_type, set_event_transaction_id, get_event_transaction_type, get_event_transaction_id ) +from util.date_utils import from_timestamp from xmodule.modulestore.django import modulestore from .config.models import PersistentGradesEnabledFlag @@ -29,8 +33,26 @@ log = getLogger(__name__) @task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) def recalculate_subsection_grade( + # pylint: disable=unused-argument user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, **kwargs ): + """ + Shim to allow us to modify this task's signature without blowing up production on deployment. + """ + recalculate_subsection_grade_v2.apply( + kwargs=dict( + user_id=user_id, + course_id=course_id, + usage_id=usage_id, + only_if_higher=only_if_higher, + expected_modified_time=kwargs.get('expected_modified_time', 0), # Use the unix epoch as a default + score_deleted=kwargs['score_deleted'], + ) + ) + + +@task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) +def recalculate_subsection_grade_v2(**kwargs): """ Updates a saved subsection grade. @@ -41,10 +63,8 @@ def recalculate_subsection_grade( only_if_higher (boolean): indicating whether grades should be updated only if the new raw_earned is higher than the previous value. - weighted_earned (float): the weighted points the learner earned on the - problem that triggered the update. - weighted_possible (float): the max weighted points the leaner could have - earned on the problem. + expected_modified_time (serialized timestamp): indicates when the task + was queued so that we can verify the underlying data update. score_deleted (boolean): indicating whether the grade change is a result of the problem's score being deleted. event_transaction_id(string): uuid identifying the current @@ -52,56 +72,74 @@ def recalculate_subsection_grade( event_transaction_type(string): human-readable type of the event at the root of the current event transaction. """ - course_key = CourseLocator.from_string(course_id) + course_key = CourseLocator.from_string(kwargs['course_id']) if not PersistentGradesEnabledFlag.feature_enabled(course_key): return score_deleted = kwargs['score_deleted'] + scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) + expected_modified_time = from_timestamp(kwargs['expected_modified_time']) # The request cache is not maintained on celery workers, # where this code runs. So we take the values from the # main request cache and store them in the local request # cache. This correlates model-level grading events with # higher-level ones. - set_event_transaction_id(kwargs.get('event_transaction_id', None)) - set_event_transaction_type(kwargs.get('event_transaction_type', None)) - scored_block_usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key) + set_event_transaction_id(kwargs.pop('event_transaction_id', None)) + set_event_transaction_type(kwargs.pop('event_transaction_type', None)) # Verify the database has been updated with the scores when the task was # created. This race condition occurs if the transaction in the task # 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, weighted_earned, weighted_possible, score_deleted, + kwargs['user_id'], scored_block_usage_key, expected_modified_time, score_deleted, ): - raise _retry_recalculate_subsection_grade( - user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, score_deleted, - ) + raise _retry_recalculate_subsection_grade(**kwargs) _update_subsection_grades( course_key, scored_block_usage_key, - only_if_higher, - course_id, - user_id, - usage_id, - weighted_earned, - weighted_possible, + kwargs['only_if_higher'], + kwargs['course_id'], + kwargs['user_id'], + kwargs['usage_id'], + kwargs['expected_modified_time'], score_deleted, ) def _has_database_updated_with_new_score( - user_id, scored_block_usage_key, expected_raw_earned, expected_raw_possible, score_deleted, + user_id, scored_block_usage_key, expected_modified_time, score_deleted, ): """ Returns whether the database has been updated with the expected new score values for the given problem and user. - - Just here to let tests run while Eric updates his PR to go back - to timestamp-based comparison. """ - return True + score = get_score(user_id, scored_block_usage_key) + + if score is None: + # score should be None only if it was deleted. + # Otherwise, it hasn't yet been saved. + return score_deleted + elif score.module_type == 'openassessment': + anon_id = anonymous_id_for_user(User.objects.get(id=user_id), scored_block_usage_key.course_key) + course_id = unicode(scored_block_usage_key.course_key) + item_id = unicode(scored_block_usage_key) + + api_score = sub_api.get_score( + { + "student_id": anon_id, + "course_id": course_id, + "item_id": item_id, + "item_type": "openassessment" + } + ) + reported_modified_time = api_score.created_at + else: + reported_modified_time = score.modified + + return reported_modified_time >= expected_modified_time def _update_subsection_grades( @@ -111,8 +149,7 @@ def _update_subsection_grades( course_id, user_id, usage_id, - weighted_earned, - weighted_possible, + expected_modified_time, score_deleted, ): """ @@ -153,8 +190,7 @@ def _update_subsection_grades( course_id, usage_id, only_if_higher, - weighted_earned, - weighted_possible, + expected_modified_time, score_deleted, exc, ) @@ -165,8 +201,7 @@ def _retry_recalculate_subsection_grade( course_id, usage_id, only_if_higher, - weighted_earned, - weighted_possible, + expected_modified_time, score_deleted, exc=None, ): @@ -174,14 +209,13 @@ def _retry_recalculate_subsection_grade( Calls retry for the recalculate_subsection_grade task with the given inputs. """ - recalculate_subsection_grade.retry( + recalculate_subsection_grade_v2.retry( kwargs=dict( user_id=user_id, course_id=course_id, usage_id=usage_id, only_if_higher=only_if_higher, - weighted_earned=weighted_earned, - weighted_possible=weighted_possible, + expected_modified_time=expected_modified_time, score_deleted=score_deleted, event_transaction_id=unicode(get_event_transaction_id()), event_transaction_type=unicode(get_event_transaction_type()), diff --git a/lms/djangoapps/grades/tests/integration/test_events.py b/lms/djangoapps/grades/tests/integration/test_events.py index 9f59a6f68f..3c579f3fb9 100644 --- a/lms/djangoapps/grades/tests/integration/test_events.py +++ b/lms/djangoapps/grades/tests/integration/test_events.py @@ -156,6 +156,9 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe 'course_version': unicode(course.course_version), } ) + enrollment_tracker.reset_mock() + models_tracker.reset_mock() + handlers_tracker.reset_mock() @patch('lms.djangoapps.instructor_task.tasks_helper.tracker') @patch('lms.djangoapps.grades.signals.handlers.tracker') @@ -223,3 +226,6 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe 'course_edited_timestamp': unicode(course.subtree_edited_on), } ) + instructor_task_tracker.reset_mock() + models_tracker.reset_mock() + handlers_tracker.reset_mock() diff --git a/lms/djangoapps/grades/tests/test_signals.py b/lms/djangoapps/grades/tests/test_signals.py index 59acf800ac..26f61e1180 100644 --- a/lms/djangoapps/grades/tests/test_signals.py +++ b/lms/djangoapps/grades/tests/test_signals.py @@ -4,9 +4,12 @@ Tests for the score change signals defined in the courseware models module. import re +from datetime import datetime import ddt from django.test import TestCase from mock import patch, MagicMock +import pytz +from util.date_utils import to_timestamp from ..signals.handlers import ( enqueue_subsection_update, @@ -17,19 +20,24 @@ from ..signals.handlers import ( 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]'}) +FROZEN_NOW_DATETIME = datetime.now().replace(tzinfo=pytz.UTC) +FROZEN_NOW_TIMESTAMP = to_timestamp(FROZEN_NOW_DATETIME) + SUBMISSION_SET_KWARGS = { 'points_possible': 10, 'points_earned': 5, 'anonymous_user_id': 'anonymous_id', 'course_id': 'CourseID', - 'item_id': 'i4x://org/course/usage/123456' + 'item_id': 'i4x://org/course/usage/123456', + 'created_at': FROZEN_NOW_TIMESTAMP, } SUBMISSION_RESET_KWARGS = { 'anonymous_user_id': 'anonymous_id', 'course_id': 'CourseID', - 'item_id': 'i4x://org/course/usage/123456' + 'item_id': 'i4x://org/course/usage/123456', + 'created_at': FROZEN_NOW_TIMESTAMP, } PROBLEM_RAW_SCORE_CHANGED_KWARGS = { @@ -41,6 +49,7 @@ PROBLEM_RAW_SCORE_CHANGED_KWARGS = { 'usage_id': 'i4x://org/course/usage/123456', 'only_if_higher': False, 'score_deleted': True, + 'modified': FROZEN_NOW_TIMESTAMP, } @@ -102,7 +111,8 @@ class ScoreChangedSignalRelayTest(TestCase): 'weighted_earned': earned, 'user_id': self.user_mock.id, 'course_id': 'CourseID', - 'usage_id': 'i4x://org/course/usage/123456' + 'usage_id': 'i4x://org/course/usage/123456', + 'modified': FROZEN_NOW_TIMESTAMP, } self.signal_mock.assert_called_once_with(**expected_set_kwargs) self.get_user_mock.assert_called_once_with(kwargs['anonymous_user_id']) @@ -152,6 +162,7 @@ class ScoreChangedSignalRelayTest(TestCase): 'usage_id': 'i4x://org/course/usage/123456', 'only_if_higher': False, 'score_deleted': True, + 'modified': FROZEN_NOW_TIMESTAMP } self.signal_mock.assert_called_with(**expected_set_kwargs) @@ -168,6 +179,7 @@ class ScoreChangedSignalRelayTest(TestCase): 'usage_id': 'i4x://org/course/usage/123456', 'only_if_higher': False, 'score_deleted': False, + 'modified': FROZEN_NOW_TIMESTAMP } self.signal_mock.assert_called_with(**expected_set_kwargs) @@ -178,6 +190,7 @@ class ScoreChangedSignalRelayTest(TestCase): user_id=1, course_id=u'course-v1:edX+Demo_Course+DemoX', usage_id=u'block-v1:block-key', + modified=FROZEN_NOW_DATETIME, ) log_statement = mocklog.call_args[0][0] log_statement = UUID_REGEX.sub(u'*UUID*', log_statement) @@ -185,7 +198,7 @@ class ScoreChangedSignalRelayTest(TestCase): log_statement, ( u'Grades: Request async calculation of subsection grades with args: ' - u'course_id:course-v1:edX+Demo_Course+DemoX, usage_id:block-v1:block-key, ' - u'user_id:1. Task [*UUID*]' - ) + u'course_id:course-v1:edX+Demo_Course+DemoX, modified:{time}, ' + u'usage_id:block-v1:block-key, user_id:1. Task [*UUID*]' + ).format(time=FROZEN_NOW_DATETIME) ) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 78a8984bcb..6e29f753cc 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -4,10 +4,13 @@ Tests for the functionality and infrastructure of grades tasks. from collections import OrderedDict from contextlib import contextmanager +from datetime import datetime, timedelta import ddt from django.conf import settings from django.db.utils import IntegrityError -from mock import patch +from mock import patch, MagicMock +import pytz +from util.date_utils import to_timestamp from unittest import skip from student.models import anonymous_id_for_user @@ -23,7 +26,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, chec from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED -from lms.djangoapps.grades.tasks import recalculate_subsection_grade +from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v2 @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) @@ -54,6 +57,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.frozen_now_datetime = datetime.now().replace(tzinfo=pytz.UTC) + self.frozen_now_timestamp = to_timestamp(self.frozen_now_datetime) + self.problem_weighted_score_changed_kwargs = OrderedDict([ ('weighted_earned', 1.0), ('weighted_possible', 2.0), @@ -61,6 +67,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ('course_id', unicode(self.course.id)), ('usage_id', unicode(self.problem.location)), ('only_if_higher', None), + ('modified', self.frozen_now_datetime), ]) create_new_event_transaction_id() @@ -70,8 +77,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ('course_id', unicode(self.course.id)), ('usage_id', unicode(self.problem.location)), ('only_if_higher', None), - ('weighted_earned', 1.0), - ('weighted_possible', 2.0), + ('expected_modified_time', self.frozen_now_timestamp), ('score_deleted', False), ('event_transaction_id', unicode(get_event_transaction_id())), ('event_transaction_type', u'edx.grades.problem.submitted'), @@ -81,6 +87,15 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): _ = anonymous_id_for_user(self.user, self.course.id) # pylint: enable=attribute-defined-outside-init,no-member + @contextmanager + def mock_get_score(self, score=MagicMock(grade=1.0, max_grade=2.0)): + """ + Mocks the scores needed by the SCORE_PUBLISHED signal + handler. By default, sets the returned score to 1/2. + """ + with patch("lms.djangoapps.grades.tasks.get_score", return_value=score): + yield + def test_problem_weighted_score_changed_queues_task(self): """ Ensures that the PROBLEM_WEIGHTED_SCORE_CHANGED signal enqueues the correct task. @@ -89,8 +104,8 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): send_args = self.problem_weighted_score_changed_kwargs local_task_args = self.recalculate_subsection_grade_kwargs.copy() local_task_args['event_transaction_type'] = u'edx.grades.problem.submitted' - with patch( - 'lms.djangoapps.grades.tasks.recalculate_subsection_grade.apply_async', + with self.mock_get_score() and patch( + 'lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.apply_async', return_value=None ) as mock_task_apply: PROBLEM_WEIGHTED_SCORE_CHANGED.send(sender=None, **send_args) @@ -186,9 +201,9 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): with self.store.default_store(default_store): self.set_up_course() with check_mongo_calls(0) and self.assertNumQueries(3 if feature_flag else 2): - recalculate_subsection_grade.apply(kwargs=self.recalculate_subsection_grade_kwargs) + self._apply_recalculate_subsection_grade() - @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade.retry') + @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry') @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') def test_retry_subsection_update_on_integrity_error(self, mock_update, mock_retry): """ @@ -199,18 +214,18 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): self._apply_recalculate_subsection_grade() self._assert_retry_called(mock_retry) - @skip # Pending completion of TNL-5995 - @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade.retry') + @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry') def test_retry_subsection_grade_on_update_not_complete(self, mock_retry): self.set_up_course() - self._apply_recalculate_subsection_grade() + self._apply_recalculate_subsection_grade( + mock_score=MagicMock(modified=datetime.utcnow().replace(tzinfo=pytz.UTC) - timedelta(days=1)) + ) self._assert_retry_called(mock_retry) - @skip # Pending completion of TNL-5995 - @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade.retry') + @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry') def test_retry_subsection_grade_on_no_score(self, mock_retry): self.set_up_course() - self._apply_recalculate_subsection_grade() + self._apply_recalculate_subsection_grade(mock_score=None) self._assert_retry_called(mock_retry) @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') @@ -224,12 +239,16 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): self._apply_recalculate_subsection_grade() self.assertEquals(mock_course_signal.call_count, 1) - def _apply_recalculate_subsection_grade(self): + def _apply_recalculate_subsection_grade( + self, + mock_score=MagicMock(modified=datetime.utcnow().replace(tzinfo=pytz.UTC) + timedelta(days=1)) + ): """ Calls the recalculate_subsection_grade task with necessary mocking in place. """ - recalculate_subsection_grade.apply(kwargs=self.recalculate_subsection_grade_kwargs) + with self.mock_get_score(mock_score): + recalculate_subsection_grade_v2.apply(kwargs=self.recalculate_subsection_grade_kwargs) def _assert_retry_called(self, mock_retry): """ diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index c8eec5f61a..168754899b 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -7,12 +7,18 @@ Does not include any access control, be sure to check access before calling. import json import logging -from django.conf import settings +from datetime import datetime from django.contrib.auth.models import User +from django.conf import settings from django.core.mail import send_mail from django.core.urlresolvers import reverse from django.utils.translation import override as override_language from eventtracking import tracker +import pytz + +from course_modes.models import CourseMode +from courseware.models import StudentModule +from edxmako.shortcuts import render_to_string 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 @@ -321,7 +327,7 @@ def _fire_score_changed_for_block( """ 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. + from the XModule, as noted below. The effective time is now(). """ if block and block.has_score and block.max_score() is not None: PROBLEM_RAW_SCORE_CHANGED.send( @@ -334,6 +340,7 @@ def _fire_score_changed_for_block( usage_id=unicode(module_state_key), score_deleted=True, only_if_higher=False, + modified=datetime.now().replace(tzinfo=pytz.UTC), ) diff --git a/openedx/tests/xblock_integration/xblock_testcase.py b/openedx/tests/xblock_integration/xblock_testcase.py index 1b602e2999..77c48d03ba 100644 --- a/openedx/tests/xblock_integration/xblock_testcase.py +++ b/openedx/tests/xblock_integration/xblock_testcase.py @@ -37,9 +37,11 @@ Our next steps would be to: import HTMLParser import collections +from datetime import datetime, timedelta import json import mock import sys +import pytz import unittest from bs4 import BeautifulSoup @@ -202,6 +204,8 @@ class GradePublishTestMixin(object): 'usage': usage_key, 'score': score, 'max_score': max_score}) + # Shim a return time, defaults to 1 hour before now + return datetime.now().replace(tzinfo=pytz.UTC) - timedelta(hours=1) self.scores = [] patcher = mock.patch("lms.djangoapps.grades.signals.handlers.set_score", capture_score)