Skip to content

E
edx-platform
Commit 5a98a90e by Eric Fischer
Options

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
parent d0be0ae8
Showing with 163 additions and 65 deletions
lms/djangoapps/courseware/model_data.py
...@@ -1008,6 +1008,7 @@ def set_score(user_id, usage_key, score, max_score): ...@@ -1008,6 +1008,7 @@ def set_score(user_id, usage_key, score, max_score):
student_module.grade = score student_module.grade = score
student_module.max_grade = max_score student_module.max_grade = max_score
student_module.save() student_module.save()
return student_module.modified
def get_score(user_id, usage_key): def get_score(user_id, usage_key):
...@@ -1024,4 +1025,4 @@ def get_score(user_id, usage_key): ...@@ -1024,4 +1025,4 @@ def get_score(user_id, usage_key):
except StudentModule.DoesNotExist: except StudentModule.DoesNotExist:
return None return None
else: else:
return student_module.grade, student_module.max_grade return student_module
lms/djangoapps/courseware/tests/test_module_render.py
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
""" """
Test for lms courseware app, module render unit Test for lms courseware app, module render unit
""" """
from datetime import datetime
import ddt import ddt
import itertools import itertools
import json import json
...@@ -15,10 +16,12 @@ from django.conf import settings ...@@ -15,10 +16,12 @@ from django.conf import settings
from django.test.client import RequestFactory from django.test.client import RequestFactory
from django.test.utils import override_settings from django.test.utils import override_settings
from django.contrib.auth.models import AnonymousUser from django.contrib.auth.models import AnonymousUser
from freezegun import freeze_time
from mock import MagicMock, patch, Mock from mock import MagicMock, patch, Mock
from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.keys import UsageKey, CourseKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from pyquery import PyQuery from pyquery import PyQuery
import pytz
from xblock.field_data import FieldData from xblock.field_data import FieldData
from xblock.runtime import Runtime from xblock.runtime import Runtime
from xblock.fields import ScopeIds from xblock.fields import ScopeIds
...@@ -1831,6 +1834,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): ...@@ -1831,6 +1834,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems):
self.assertIsNone(student_module.grade) self.assertIsNone(student_module.grade)
self.assertIsNone(student_module.max_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') @patch('lms.djangoapps.grades.signals.handlers.PROBLEM_RAW_SCORE_CHANGED.send')
def test_score_change_signal(self, send_mock): def test_score_change_signal(self, send_mock):
"""Test that a Django signal is generated when a score changes""" """Test that a Django signal is generated when a score changes"""
...@@ -1844,6 +1848,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): ...@@ -1844,6 +1848,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems):
'course_id': unicode(self.course.id), 'course_id': unicode(self.course.id),
'usage_id': unicode(self.problem.location), 'usage_id': unicode(self.problem.location),
'only_if_higher': None, 'only_if_higher': None,
'modified': datetime.now().replace(tzinfo=pytz.UTC)
} }
send_mock.assert_called_with(**expected_signal_kwargs) send_mock.assert_called_with(**expected_signal_kwargs)
......
lms/djangoapps/grades/signals/handlers.py
...@@ -4,9 +4,11 @@ Grades related signals. ...@@ -4,9 +4,11 @@ Grades related signals.
from logging import getLogger from logging import getLogger
from courseware.model_data import get_score, set_score
from django.dispatch import receiver from django.dispatch import receiver
from openedx.core.lib.grade_utils import is_score_higher from openedx.core.lib.grade_utils import is_score_higher
from submissions.models import score_set, score_reset 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 courseware.model_data import get_score, set_score
from eventtracking import tracker from eventtracking import tracker
...@@ -25,7 +27,7 @@ from .signals import ( ...@@ -25,7 +27,7 @@ from .signals import (
) )
from ..new.course_grade import CourseGradeFactory from ..new.course_grade import CourseGradeFactory
from ..scores import weighted_score from ..scores import weighted_score
from ..tasks import recalculate_subsection_grade from ..tasks import recalculate_subsection_grade_v2
log = getLogger(__name__) log = getLogger(__name__)
...@@ -62,7 +64,8 @@ def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-a ...@@ -62,7 +64,8 @@ def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-a
weighted_possible=points_possible, weighted_possible=points_possible,
user_id=user.id, user_id=user.id,
course_id=course_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 ...@@ -92,7 +95,8 @@ def submissions_score_reset_handler(sender, **kwargs): # pylint: disable=unused
weighted_possible=0, weighted_possible=0,
user_id=user.id, user_id=user.id,
course_id=course_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_ ...@@ -107,7 +111,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_
previous_score = get_score(user.id, block.location) previous_score = get_score(user.id, block.location)
if previous_score is not None: 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): if not is_score_higher(prev_raw_earned, prev_raw_possible, raw_earned, raw_possible):
update_score = False update_score = False
...@@ -119,7 +123,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ ...@@ -119,7 +123,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_
) )
if update_score: 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( PROBLEM_RAW_SCORE_CHANGED.send(
sender=None, sender=None,
raw_earned=raw_earned, raw_earned=raw_earned,
...@@ -129,6 +133,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_ ...@@ -129,6 +133,7 @@ def score_published_handler(sender, block, user, raw_earned, raw_possible, only_
course_id=unicode(block.location.course_key), course_id=unicode(block.location.course_key),
usage_id=unicode(block.location), usage_id=unicode(block.location),
only_if_higher=only_if_higher, only_if_higher=only_if_higher,
modified=score_modified_time,
) )
return update_score return update_score
...@@ -157,6 +162,7 @@ def problem_raw_score_changed_handler(sender, **kwargs): # pylint: disable=unus ...@@ -157,6 +162,7 @@ def problem_raw_score_changed_handler(sender, **kwargs): # pylint: disable=unus
usage_id=kwargs['usage_id'], usage_id=kwargs['usage_id'],
only_if_higher=kwargs['only_if_higher'], only_if_higher=kwargs['only_if_higher'],
score_deleted=kwargs.get('score_deleted', False), score_deleted=kwargs.get('score_deleted', False),
modified=kwargs['modified'],
) )
...@@ -167,14 +173,13 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum ...@@ -167,14 +173,13 @@ def enqueue_subsection_update(sender, **kwargs): # pylint: disable=unused-argum
enqueueing a subsection update operation to occur asynchronously. enqueueing a subsection update operation to occur asynchronously.
""" """
_emit_problem_submitted_event(kwargs) _emit_problem_submitted_event(kwargs)
result = recalculate_subsection_grade.apply_async( result = recalculate_subsection_grade_v2.apply_async(
kwargs=dict( kwargs=dict(
user_id=kwargs['user_id'], user_id=kwargs['user_id'],
course_id=kwargs['course_id'], course_id=kwargs['course_id'],
usage_id=kwargs['usage_id'], usage_id=kwargs['usage_id'],
only_if_higher=kwargs.get('only_if_higher'), only_if_higher=kwargs.get('only_if_higher'),
weighted_earned=kwargs.get('weighted_earned'), expected_modified_time=to_timestamp(kwargs['modified']),
weighted_possible=kwargs.get('weighted_possible'),
score_deleted=kwargs.get('score_deleted', False), score_deleted=kwargs.get('score_deleted', False),
event_transaction_id=unicode(get_event_transaction_id()), event_transaction_id=unicode(get_event_transaction_id()),
event_transaction_type=unicode(get_event_transaction_type()), event_transaction_type=unicode(get_event_transaction_type()),
......
lms/djangoapps/grades/signals/signals.py
...@@ -20,6 +20,8 @@ PROBLEM_RAW_SCORE_CHANGED = Signal( ...@@ -20,6 +20,8 @@ PROBLEM_RAW_SCORE_CHANGED = Signal(
'weight', # Weight of the problem 'weight', # Weight of the problem
'only_if_higher', # Boolean indicating whether updates should be 'only_if_higher', # Boolean indicating whether updates should be
# made only if the new score is higher than previous. # 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( ...@@ -39,6 +41,8 @@ PROBLEM_WEIGHTED_SCORE_CHANGED = Signal(
'weighted_possible', # Maximum score available for the exercise 'weighted_possible', # Maximum score available for the exercise
'only_if_higher', # Boolean indicating whether updates should be 'only_if_higher', # Boolean indicating whether updates should be
# made only if the new score is higher than previous. # 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.
] ]
) )
......
lms/djangoapps/grades/tasks.py
...@@ -8,15 +8,19 @@ from django.contrib.auth.models import User ...@@ -8,15 +8,19 @@ from django.contrib.auth.models import User
from django.db.utils import DatabaseError from django.db.utils import DatabaseError
from logging import getLogger from logging import getLogger
from courseware.model_data import get_score
from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.course_blocks.api import get_course_blocks
from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.keys import UsageKey
from opaque_keys.edx.locator import CourseLocator 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 ( from track.event_transaction_utils import (
set_event_transaction_type, set_event_transaction_type,
set_event_transaction_id, set_event_transaction_id,
get_event_transaction_type, get_event_transaction_type,
get_event_transaction_id get_event_transaction_id
) )
from util.date_utils import from_timestamp
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from .config.models import PersistentGradesEnabledFlag from .config.models import PersistentGradesEnabledFlag
...@@ -29,9 +33,27 @@ log = getLogger(__name__) ...@@ -29,9 +33,27 @@ log = getLogger(__name__)
@task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY) @task(default_retry_delay=30, routing_key=settings.RECALCULATE_GRADES_ROUTING_KEY)
def recalculate_subsection_grade( def recalculate_subsection_grade(
# pylint: disable=unused-argument
user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, **kwargs 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. Updates a saved subsection grade.
Arguments: Arguments:
...@@ -41,10 +63,8 @@ def recalculate_subsection_grade( ...@@ -41,10 +63,8 @@ def recalculate_subsection_grade(
only_if_higher (boolean): indicating whether grades should only_if_higher (boolean): indicating whether grades should
be updated only if the new raw_earned is higher than the be updated only if the new raw_earned is higher than the
previous value. previous value.
weighted_earned (float): the weighted points the learner earned on the expected_modified_time (serialized timestamp): indicates when the task
problem that triggered the update. was queued so that we can verify the underlying data update.
weighted_possible (float): the max weighted points the leaner could have
earned on the problem.
score_deleted (boolean): indicating whether the grade change is score_deleted (boolean): indicating whether the grade change is
a result of the problem's score being deleted. a result of the problem's score being deleted.
event_transaction_id(string): uuid identifying the current event_transaction_id(string): uuid identifying the current
...@@ -52,56 +72,74 @@ def recalculate_subsection_grade( ...@@ -52,56 +72,74 @@ def recalculate_subsection_grade(
event_transaction_type(string): human-readable type of the event_transaction_type(string): human-readable type of the
event at the root of the current event transaction. 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): if not PersistentGradesEnabledFlag.feature_enabled(course_key):
return return
score_deleted = kwargs['score_deleted'] 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, # The request cache is not maintained on celery workers,
# where this code runs. So we take the values from the # where this code runs. So we take the values from the
# main request cache and store them in the local request # main request cache and store them in the local request
# cache. This correlates model-level grading events with # cache. This correlates model-level grading events with
# higher-level ones. # higher-level ones.
set_event_transaction_id(kwargs.get('event_transaction_id', None)) set_event_transaction_id(kwargs.pop('event_transaction_id', None))
set_event_transaction_type(kwargs.get('event_transaction_type', None)) set_event_transaction_type(kwargs.pop('event_transaction_type', None))
scored_block_usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key)
# Verify the database has been updated with the scores when the task was # Verify the database has been updated with the scores when the task was
# created. This race condition occurs if the transaction in the task # created. This race condition occurs if the transaction in the task
# creator's process hasn't committed before the task initiates in the worker # creator's process hasn't committed before the task initiates in the worker
# process. # process.
if not _has_database_updated_with_new_score( 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( raise _retry_recalculate_subsection_grade(**kwargs)
user_id, course_id, usage_id, only_if_higher, weighted_earned, weighted_possible, score_deleted,
)
_update_subsection_grades( _update_subsection_grades(
course_key, course_key,
scored_block_usage_key, scored_block_usage_key,
only_if_higher, kwargs['only_if_higher'],
course_id, kwargs['course_id'],
user_id, kwargs['user_id'],
usage_id, kwargs['usage_id'],
weighted_earned, kwargs['expected_modified_time'],
weighted_possible,
score_deleted, score_deleted,
) )
def _has_database_updated_with_new_score( 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 Returns whether the database has been updated with the
expected new score values for the given problem and user. 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( def _update_subsection_grades(
...@@ -111,8 +149,7 @@ def _update_subsection_grades( ...@@ -111,8 +149,7 @@ def _update_subsection_grades(
course_id, course_id,
user_id, user_id,
usage_id, usage_id,
weighted_earned, expected_modified_time,
weighted_possible,
score_deleted, score_deleted,
): ):
""" """
...@@ -153,8 +190,7 @@ def _update_subsection_grades( ...@@ -153,8 +190,7 @@ def _update_subsection_grades(
course_id, course_id,
usage_id, usage_id,
only_if_higher, only_if_higher,
weighted_earned, expected_modified_time,
weighted_possible,
score_deleted, score_deleted,
exc, exc,
) )
...@@ -165,8 +201,7 @@ def _retry_recalculate_subsection_grade( ...@@ -165,8 +201,7 @@ def _retry_recalculate_subsection_grade(
course_id, course_id,
usage_id, usage_id,
only_if_higher, only_if_higher,
weighted_earned, expected_modified_time,
weighted_possible,
score_deleted, score_deleted,
exc=None, exc=None,
): ):
...@@ -174,14 +209,13 @@ def _retry_recalculate_subsection_grade( ...@@ -174,14 +209,13 @@ def _retry_recalculate_subsection_grade(
Calls retry for the recalculate_subsection_grade task with the Calls retry for the recalculate_subsection_grade task with the
given inputs. given inputs.
""" """
recalculate_subsection_grade.retry( recalculate_subsection_grade_v2.retry(
kwargs=dict( kwargs=dict(
user_id=user_id, user_id=user_id,
course_id=course_id, course_id=course_id,
usage_id=usage_id, usage_id=usage_id,
only_if_higher=only_if_higher, only_if_higher=only_if_higher,
weighted_earned=weighted_earned, expected_modified_time=expected_modified_time,
weighted_possible=weighted_possible,
score_deleted=score_deleted, score_deleted=score_deleted,
event_transaction_id=unicode(get_event_transaction_id()), event_transaction_id=unicode(get_event_transaction_id()),
event_transaction_type=unicode(get_event_transaction_type()), event_transaction_type=unicode(get_event_transaction_type()),
......
lms/djangoapps/grades/tests/integration/test_events.py
...@@ -156,6 +156,9 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe ...@@ -156,6 +156,9 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe
'course_version': unicode(course.course_version), '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.instructor_task.tasks_helper.tracker')
@patch('lms.djangoapps.grades.signals.handlers.tracker') @patch('lms.djangoapps.grades.signals.handlers.tracker')
...@@ -223,3 +226,6 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe ...@@ -223,3 +226,6 @@ class GradesEventIntegrationTest(ProblemSubmissionTestMixin, SharedModuleStoreTe
'course_edited_timestamp': unicode(course.subtree_edited_on), 'course_edited_timestamp': unicode(course.subtree_edited_on),
} }
) )
instructor_task_tracker.reset_mock()
models_tracker.reset_mock()
handlers_tracker.reset_mock()
lms/djangoapps/grades/tests/test_signals.py
...@@ -4,9 +4,12 @@ Tests for the score change signals defined in the courseware models module. ...@@ -4,9 +4,12 @@ Tests for the score change signals defined in the courseware models module.
import re import re
from datetime import datetime
import ddt import ddt
from django.test import TestCase from django.test import TestCase
from mock import patch, MagicMock from mock import patch, MagicMock
import pytz
from util.date_utils import to_timestamp
from ..signals.handlers import ( from ..signals.handlers import (
enqueue_subsection_update, enqueue_subsection_update,
...@@ -17,19 +20,24 @@ from ..signals.handlers import ( ...@@ -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]'}) 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 = { SUBMISSION_SET_KWARGS = {
'points_possible': 10, 'points_possible': 10,
'points_earned': 5, 'points_earned': 5,
'anonymous_user_id': 'anonymous_id', 'anonymous_user_id': 'anonymous_id',
'course_id': 'CourseID', '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 = { SUBMISSION_RESET_KWARGS = {
'anonymous_user_id': 'anonymous_id', 'anonymous_user_id': 'anonymous_id',
'course_id': 'CourseID', '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 = { PROBLEM_RAW_SCORE_CHANGED_KWARGS = {
...@@ -41,6 +49,7 @@ PROBLEM_RAW_SCORE_CHANGED_KWARGS = { ...@@ -41,6 +49,7 @@ PROBLEM_RAW_SCORE_CHANGED_KWARGS = {
'usage_id': 'i4x://org/course/usage/123456', 'usage_id': 'i4x://org/course/usage/123456',
'only_if_higher': False, 'only_if_higher': False,
'score_deleted': True, 'score_deleted': True,
'modified': FROZEN_NOW_TIMESTAMP,
} }
...@@ -102,7 +111,8 @@ class ScoreChangedSignalRelayTest(TestCase): ...@@ -102,7 +111,8 @@ class ScoreChangedSignalRelayTest(TestCase):
'weighted_earned': earned, 'weighted_earned': earned,
'user_id': self.user_mock.id, 'user_id': self.user_mock.id,
'course_id': 'CourseID', '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.signal_mock.assert_called_once_with(**expected_set_kwargs)
self.get_user_mock.assert_called_once_with(kwargs['anonymous_user_id']) self.get_user_mock.assert_called_once_with(kwargs['anonymous_user_id'])
...@@ -152,6 +162,7 @@ class ScoreChangedSignalRelayTest(TestCase): ...@@ -152,6 +162,7 @@ class ScoreChangedSignalRelayTest(TestCase):
'usage_id': 'i4x://org/course/usage/123456', 'usage_id': 'i4x://org/course/usage/123456',
'only_if_higher': False, 'only_if_higher': False,
'score_deleted': True, 'score_deleted': True,
'modified': FROZEN_NOW_TIMESTAMP
} }
self.signal_mock.assert_called_with(**expected_set_kwargs) self.signal_mock.assert_called_with(**expected_set_kwargs)
...@@ -168,6 +179,7 @@ class ScoreChangedSignalRelayTest(TestCase): ...@@ -168,6 +179,7 @@ class ScoreChangedSignalRelayTest(TestCase):
'usage_id': 'i4x://org/course/usage/123456', 'usage_id': 'i4x://org/course/usage/123456',
'only_if_higher': False, 'only_if_higher': False,
'score_deleted': False, 'score_deleted': False,
'modified': FROZEN_NOW_TIMESTAMP
} }
self.signal_mock.assert_called_with(**expected_set_kwargs) self.signal_mock.assert_called_with(**expected_set_kwargs)
...@@ -178,6 +190,7 @@ class ScoreChangedSignalRelayTest(TestCase): ...@@ -178,6 +190,7 @@ class ScoreChangedSignalRelayTest(TestCase):
user_id=1, user_id=1,
course_id=u'course-v1:edX+Demo_Course+DemoX', course_id=u'course-v1:edX+Demo_Course+DemoX',
usage_id=u'block-v1:block-key', usage_id=u'block-v1:block-key',
modified=FROZEN_NOW_DATETIME,
) )
log_statement = mocklog.call_args[0][0] log_statement = mocklog.call_args[0][0]
log_statement = UUID_REGEX.sub(u'*UUID*', log_statement) log_statement = UUID_REGEX.sub(u'*UUID*', log_statement)
...@@ -185,7 +198,7 @@ class ScoreChangedSignalRelayTest(TestCase): ...@@ -185,7 +198,7 @@ class ScoreChangedSignalRelayTest(TestCase):
log_statement, log_statement,
( (
u'Grades: Request async calculation of subsection grades with args: ' 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'course_id:course-v1:edX+Demo_Course+DemoX, modified:{time}, '
u'user_id:1. Task [*UUID*]' u'usage_id:block-v1:block-key, user_id:1. Task [*UUID*]'
) ).format(time=FROZEN_NOW_DATETIME)
) )
lms/djangoapps/grades/tests/test_tasks.py
...@@ -4,10 +4,13 @@ Tests for the functionality and infrastructure of grades tasks. ...@@ -4,10 +4,13 @@ Tests for the functionality and infrastructure of grades tasks.
from collections import OrderedDict from collections import OrderedDict
from contextlib import contextmanager from contextlib import contextmanager
from datetime import datetime, timedelta
import ddt import ddt
from django.conf import settings from django.conf import settings
from django.db.utils import IntegrityError 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 unittest import skip
from student.models import anonymous_id_for_user from student.models import anonymous_id_for_user
...@@ -23,7 +26,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, chec ...@@ -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.config.models import PersistentGradesEnabledFlag
from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED 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}) @patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False})
...@@ -54,6 +57,9 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ...@@ -54,6 +57,9 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
self.sequential = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Sequential1") 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 = 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([ self.problem_weighted_score_changed_kwargs = OrderedDict([
('weighted_earned', 1.0), ('weighted_earned', 1.0),
('weighted_possible', 2.0), ('weighted_possible', 2.0),
...@@ -61,6 +67,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ...@@ -61,6 +67,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
('course_id', unicode(self.course.id)), ('course_id', unicode(self.course.id)),
('usage_id', unicode(self.problem.location)), ('usage_id', unicode(self.problem.location)),
('only_if_higher', None), ('only_if_higher', None),
('modified', self.frozen_now_datetime),
]) ])
create_new_event_transaction_id() create_new_event_transaction_id()
...@@ -70,8 +77,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ...@@ -70,8 +77,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
('course_id', unicode(self.course.id)), ('course_id', unicode(self.course.id)),
('usage_id', unicode(self.problem.location)), ('usage_id', unicode(self.problem.location)),
('only_if_higher', None), ('only_if_higher', None),
('weighted_earned', 1.0), ('expected_modified_time', self.frozen_now_timestamp),
('weighted_possible', 2.0),
('score_deleted', False), ('score_deleted', False),
('event_transaction_id', unicode(get_event_transaction_id())), ('event_transaction_id', unicode(get_event_transaction_id())),
('event_transaction_type', u'edx.grades.problem.submitted'), ('event_transaction_type', u'edx.grades.problem.submitted'),
...@@ -81,6 +87,15 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ...@@ -81,6 +87,15 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
_ = anonymous_id_for_user(self.user, self.course.id) _ = anonymous_id_for_user(self.user, self.course.id)
# pylint: enable=attribute-defined-outside-init,no-member # 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): def test_problem_weighted_score_changed_queues_task(self):
""" """
Ensures that the PROBLEM_WEIGHTED_SCORE_CHANGED signal enqueues the correct task. Ensures that the PROBLEM_WEIGHTED_SCORE_CHANGED signal enqueues the correct task.
...@@ -89,8 +104,8 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ...@@ -89,8 +104,8 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
send_args = self.problem_weighted_score_changed_kwargs send_args = self.problem_weighted_score_changed_kwargs
local_task_args = self.recalculate_subsection_grade_kwargs.copy() local_task_args = self.recalculate_subsection_grade_kwargs.copy()
local_task_args['event_transaction_type'] = u'edx.grades.problem.submitted' local_task_args['event_transaction_type'] = u'edx.grades.problem.submitted'
with patch( with self.mock_get_score() and patch(
'lms.djangoapps.grades.tasks.recalculate_subsection_grade.apply_async', 'lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.apply_async',
return_value=None return_value=None
) as mock_task_apply: ) as mock_task_apply:
PROBLEM_WEIGHTED_SCORE_CHANGED.send(sender=None, **send_args) PROBLEM_WEIGHTED_SCORE_CHANGED.send(sender=None, **send_args)
...@@ -186,9 +201,9 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ...@@ -186,9 +201,9 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
with self.store.default_store(default_store): with self.store.default_store(default_store):
self.set_up_course() self.set_up_course()
with check_mongo_calls(0) and self.assertNumQueries(3 if feature_flag else 2): 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') @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update')
def test_retry_subsection_update_on_integrity_error(self, mock_update, mock_retry): def test_retry_subsection_update_on_integrity_error(self, mock_update, mock_retry):
""" """
...@@ -199,18 +214,18 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ...@@ -199,18 +214,18 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
self._apply_recalculate_subsection_grade() self._apply_recalculate_subsection_grade()
self._assert_retry_called(mock_retry) self._assert_retry_called(mock_retry)
@skip # Pending completion of TNL-5995 @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
@patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade.retry')
def test_retry_subsection_grade_on_update_not_complete(self, mock_retry): def test_retry_subsection_grade_on_update_not_complete(self, mock_retry):
self.set_up_course() 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) self._assert_retry_called(mock_retry)
@skip # Pending completion of TNL-5995 @patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade_v2.retry')
@patch('lms.djangoapps.grades.tasks.recalculate_subsection_grade.retry')
def test_retry_subsection_grade_on_no_score(self, mock_retry): def test_retry_subsection_grade_on_no_score(self, mock_retry):
self.set_up_course() self.set_up_course()
self._apply_recalculate_subsection_grade() self._apply_recalculate_subsection_grade(mock_score=None)
self._assert_retry_called(mock_retry) self._assert_retry_called(mock_retry)
@patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send')
...@@ -224,12 +239,16 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): ...@@ -224,12 +239,16 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase):
self._apply_recalculate_subsection_grade() self._apply_recalculate_subsection_grade()
self.assertEquals(mock_course_signal.call_count, 1) 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 Calls the recalculate_subsection_grade task with necessary
mocking in place. 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): def _assert_retry_called(self, mock_retry):
""" """
......
lms/djangoapps/instructor/enrollment.py
...@@ -7,12 +7,18 @@ Does not include any access control, be sure to check access before calling. ...@@ -7,12 +7,18 @@ Does not include any access control, be sure to check access before calling.
import json import json
import logging import logging
from django.conf import settings from datetime import datetime
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.conf import settings
from django.core.mail import send_mail from django.core.mail import send_mail
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.utils.translation import override as override_language from django.utils.translation import override as override_language
from eventtracking import tracker 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 lms.djangoapps.grades.signals.signals import PROBLEM_RAW_SCORE_CHANGED
from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
...@@ -321,7 +327,7 @@ def _fire_score_changed_for_block( ...@@ -321,7 +327,7 @@ def _fire_score_changed_for_block(
""" """
Fires a PROBLEM_RAW_SCORE_CHANGED event for the given module. Fires a PROBLEM_RAW_SCORE_CHANGED event for the given module.
The earned points are always zero. We must retrieve the possible points 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: if block and block.has_score and block.max_score() is not None:
PROBLEM_RAW_SCORE_CHANGED.send( PROBLEM_RAW_SCORE_CHANGED.send(
...@@ -334,6 +340,7 @@ def _fire_score_changed_for_block( ...@@ -334,6 +340,7 @@ def _fire_score_changed_for_block(
usage_id=unicode(module_state_key), usage_id=unicode(module_state_key),
score_deleted=True, score_deleted=True,
only_if_higher=False, only_if_higher=False,
modified=datetime.now().replace(tzinfo=pytz.UTC),
) )
......
openedx/tests/xblock_integration/xblock_testcase.py
...@@ -37,9 +37,11 @@ Our next steps would be to: ...@@ -37,9 +37,11 @@ Our next steps would be to:
import HTMLParser import HTMLParser
import collections import collections
from datetime import datetime, timedelta
import json import json
import mock import mock
import sys import sys
import pytz
import unittest import unittest
from bs4 import BeautifulSoup from bs4 import BeautifulSoup
...@@ -202,6 +204,8 @@ class GradePublishTestMixin(object): ...@@ -202,6 +204,8 @@ class GradePublishTestMixin(object):
'usage': usage_key, 'usage': usage_key,
'score': score, 'score': score,
'max_score': max_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 = [] self.scores = []
patcher = mock.patch("lms.djangoapps.grades.signals.handlers.set_score", capture_score) patcher = mock.patch("lms.djangoapps.grades.signals.handlers.set_score", capture_score)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment