diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 8588eef7b1..147ef1ae84 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -540,7 +540,7 @@ def get_module_system_for_user(user, student_data, # TODO # pylint: disable=to sender=None, points_possible=event['max_value'], points_earned=event['value'], - user=user, + user_id=user.id, course_id=unicode(course_id), usage_id=unicode(descriptor.location) ) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 57dbc46aa9..6ec12729e8 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1847,7 +1847,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): 'sender': None, 'points_possible': self.grade_dict['max_value'], 'points_earned': self.grade_dict['value'], - 'user': self.student_user, + 'user_id': self.student_user.id, 'course_id': unicode(self.course.id), 'usage_id': unicode(self.problem.location) } diff --git a/lms/djangoapps/gating/signals.py b/lms/djangoapps/gating/signals.py index bfa5b974a7..16eed7371b 100644 --- a/lms/djangoapps/gating/signals.py +++ b/lms/djangoapps/gating/signals.py @@ -26,5 +26,5 @@ def handle_score_changed(**kwargs): gating_api.evaluate_prerequisite( course, UsageKey.from_string(kwargs.get('usage_id')), - kwargs.get('user').id, + kwargs.get('user_id'), ) diff --git a/lms/djangoapps/gating/tests/test_signals.py b/lms/djangoapps/gating/tests/test_signals.py index 16116da608..6e16498d87 100644 --- a/lms/djangoapps/gating/tests/test_signals.py +++ b/lms/djangoapps/gating/tests/test_signals.py @@ -19,7 +19,7 @@ class TestHandleScoreChanged(ModuleStoreTestCase): def setUp(self): super(TestHandleScoreChanged, self).setUp() self.course = CourseFactory.create(org='TestX', number='TS01', run='2016_Q1') - self.user = UserFactory() + self.user = UserFactory.create() self.test_usage_key = UsageKey.from_string('i4x://the/content/key/12345678') @patch('gating.signals.gating_api.evaluate_prerequisite') @@ -31,7 +31,7 @@ class TestHandleScoreChanged(ModuleStoreTestCase): sender=None, points_possible=1, points_earned=1, - user=self.user, + user_id=self.user.id, course_id=unicode(self.course.id), usage_id=unicode(self.test_usage_key) ) @@ -44,7 +44,7 @@ class TestHandleScoreChanged(ModuleStoreTestCase): sender=None, points_possible=1, points_earned=1, - user=self.user, + user_id=self.user.id, course_id=unicode(self.course.id), usage_id=unicode(self.test_usage_key) ) diff --git a/lms/djangoapps/grades/signals/handlers.py b/lms/djangoapps/grades/signals/handlers.py index b13fefd7c9..0b60a47fe0 100644 --- a/lms/djangoapps/grades/signals/handlers.py +++ b/lms/djangoapps/grades/signals/handlers.py @@ -1,22 +1,15 @@ """ Grades related signals. """ -from logging import getLogger from django.dispatch import receiver +from logging import getLogger -from lms.djangoapps.course_blocks.api import get_course_blocks -from lms.djangoapps.courseware.courses import get_course_by_id -from opaque_keys.edx.locator import CourseLocator -from opaque_keys.edx.keys import UsageKey -from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache from student.models import user_by_anonymous_id from submissions.models import score_set, score_reset from .signals import SCORE_CHANGED -from ..config.models import PersistentGradesEnabledFlag -from ..transformer import GradesTransformer -from ..new.subsection_grade import SubsectionGradeFactory +from ..tasks import recalculate_subsection_grade log = getLogger(__name__) @@ -42,12 +35,14 @@ def submissions_score_set_handler(sender, **kwargs): # pylint: disable=unused-a course_id = kwargs['course_id'] usage_id = kwargs['item_id'] user = user_by_anonymous_id(kwargs['anonymous_user_id']) + if user is None: + return SCORE_CHANGED.send( sender=None, points_possible=points_possible, points_earned=points_earned, - user=user, + user_id=user.id, course_id=course_id, usage_id=usage_id ) @@ -70,51 +65,22 @@ def submissions_score_reset_handler(sender, **kwargs): # pylint: disable=unused course_id = kwargs['course_id'] usage_id = kwargs['item_id'] user = user_by_anonymous_id(kwargs['anonymous_user_id']) + if user is None: + return SCORE_CHANGED.send( sender=None, points_possible=0, points_earned=0, - user=user, + user_id=user.id, course_id=course_id, usage_id=usage_id ) @receiver(SCORE_CHANGED) -def recalculate_subsection_grade_handler(sender, **kwargs): # pylint: disable=unused-argument +def enqueue_update(sender, **kwargs): # pylint: disable=unused-argument """ - Consume the SCORE_CHANGED signal and trigger an update. - This method expects that the kwargs dictionary will contain the following - entries (See the definition of SCORE_CHANGED): - - points_possible: Maximum score available for the exercise - - points_earned: Score obtained by the user - - user: User object - - course_id: Unicode string representing the course - - usage_id: Unicode string indicating the courseware instance + Handles the SCORE_CHANGED signal by enqueueing an update operation to occur asynchronously. """ - student = kwargs['user'] - course_key = CourseLocator.from_string(kwargs['course_id']) - if not PersistentGradesEnabledFlag.feature_enabled(course_key): - return - - scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) - collected_block_structure = get_course_in_cache(course_key) - course = get_course_by_id(course_key, depth=0) - - subsections_to_update = collected_block_structure.get_transformer_block_field( - scored_block_usage_key, - GradesTransformer, - 'subsections', - set() - ) - subsection_grade_factory = SubsectionGradeFactory(student, course, collected_block_structure) - for subsection_usage_key in subsections_to_update: - transformed_subsection_structure = get_course_blocks( - student, - subsection_usage_key, - collected_block_structure=collected_block_structure, - ) - subsection_grade_factory.update( - transformed_subsection_structure[subsection_usage_key], transformed_subsection_structure - ) + recalculate_subsection_grade.apply_async(args=(kwargs['user_id'], kwargs['course_id'], kwargs['usage_id'])) diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py new file mode 100644 index 0000000000..22c067fb47 --- /dev/null +++ b/lms/djangoapps/grades/tasks.py @@ -0,0 +1,53 @@ +""" +This module contains tasks for asynchronous execution of grade updates. +""" + +from celery import task +from django.contrib.auth.models import User + +from lms.djangoapps.course_blocks.api import get_course_blocks +from lms.djangoapps.courseware.courses import get_course_by_id +from opaque_keys.edx.keys import UsageKey +from opaque_keys.edx.locator import CourseLocator +from openedx.core.djangoapps.content.block_structure.api import get_course_in_cache + +from .config.models import PersistentGradesEnabledFlag +from .transformer import GradesTransformer +from .new.subsection_grade import SubsectionGradeFactory + + +@task() +def recalculate_subsection_grade(user_id, course_id, usage_id): + """ + Updates a saved subsection grade. + This method expects the following parameters: + - user_id: serialized id of applicable User object + - course_id: Unicode string representing the course + - usage_id: Unicode string indicating the courseware instance + """ + course_key = CourseLocator.from_string(course_id) + if not PersistentGradesEnabledFlag.feature_enabled(course_key): + return + + student = User.objects.get(id=user_id) + scored_block_usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key) + + collected_block_structure = get_course_in_cache(course_key) + course = get_course_by_id(course_key, depth=0) + subsection_grade_factory = SubsectionGradeFactory(student, course, collected_block_structure) + subsections_to_update = collected_block_structure.get_transformer_block_field( + scored_block_usage_key, + GradesTransformer, + 'subsections', + set() + ) + + for subsection_usage_key in subsections_to_update: + transformed_subsection_structure = get_course_blocks( + student, + subsection_usage_key, + collected_block_structure=collected_block_structure, + ) + subsection_grade_factory.update( + transformed_subsection_structure[subsection_usage_key], transformed_subsection_structure + ) diff --git a/lms/djangoapps/grades/tests/test_signals.py b/lms/djangoapps/grades/tests/test_signals.py index 2054f967f5..a79e951ec2 100644 --- a/lms/djangoapps/grades/tests/test_signals.py +++ b/lms/djangoapps/grades/tests/test_signals.py @@ -3,24 +3,13 @@ Tests for the score change signals defined in the courseware models module. """ import ddt -from django.conf import settings from django.test import TestCase from mock import patch, MagicMock -from unittest import skip - -from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag -from student.models import anonymous_id_for_user -from student.tests.factories import UserFactory -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 ..signals.handlers import ( submissions_score_set_handler, submissions_score_reset_handler, - recalculate_subsection_grade_handler, ) -from ..signals.signals import SCORE_CHANGED SUBMISSION_SET_KWARGS = { @@ -39,6 +28,7 @@ SUBMISSION_RESET_KWARGS = { } +@ddt.ddt class SubmissionSignalRelayTest(TestCase): """ Tests to ensure that the courseware module correctly catches score_set and @@ -70,202 +60,60 @@ class SubmissionSignalRelayTest(TestCase): self.addCleanup(new_patch.stop) return mock - def test_score_set_signal_handler(self): + @ddt.data( + [submissions_score_set_handler, SUBMISSION_SET_KWARGS, 5, 10], + [submissions_score_reset_handler, SUBMISSION_RESET_KWARGS, 0, 0], + ) + @ddt.unpack + def test_score_set_signal_handler(self, handler, kwargs, earned, possible): """ - Ensure that, on receipt of a score_set signal from the Submissions API, - the courseware model correctly converts it to a score_changed signal + 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. + + Also ensures that the handler calls user_by_anonymous_id correctly. """ - submissions_score_set_handler(None, **SUBMISSION_SET_KWARGS) + handler(None, **kwargs) expected_set_kwargs = { 'sender': None, - 'points_possible': 10, - 'points_earned': 5, - 'user': self.user_mock, + 'points_possible': possible, + 'points_earned': earned, + 'user_id': self.user_mock.id, 'course_id': 'CourseID', 'usage_id': 'i4x://org/course/usage/123456' } self.signal_mock.assert_called_once_with(**expected_set_kwargs) + self.get_user_mock.assert_called_once_with(kwargs['anonymous_user_id']) - def test_score_set_user_conversion(self): - """ - Ensure that the score_set handler properly calls the - user_by_anonymous_id method to convert from an anonymized ID to a user - object - """ - submissions_score_set_handler(None, **SUBMISSION_SET_KWARGS) - self.get_user_mock.assert_called_once_with('anonymous_id') - - def test_score_set_missing_kwarg(self): - """ - Ensure that, on receipt of a score_set signal from the Submissions API - that does not have the correct kwargs, the courseware model does not - generate a signal. - """ - for missing in SUBMISSION_SET_KWARGS: - kwargs = SUBMISSION_SET_KWARGS.copy() - del kwargs[missing] - - with self.assertRaises(KeyError): - submissions_score_set_handler(None, **kwargs) - self.signal_mock.assert_not_called() - - def test_score_set_bad_user(self): - """ - Ensure that, on receipt of a score_set signal from the Submissions API - that has an invalid user ID, the courseware model does not generate a - signal. - """ - self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.handlers.user_by_anonymous_id', None) - submissions_score_set_handler(None, **SUBMISSION_SET_KWARGS) - self.signal_mock.assert_not_called() - - def test_score_reset_signal_handler(self): - """ - Ensure that, on receipt of a score_reset signal from the Submissions - API, the courseware model correctly converts it to a score_changed - signal - """ - submissions_score_reset_handler(None, **SUBMISSION_RESET_KWARGS) - expected_reset_kwargs = { - 'sender': None, - 'points_possible': 0, - 'points_earned': 0, - 'user': self.user_mock, - 'course_id': 'CourseID', - 'usage_id': 'i4x://org/course/usage/123456' - } - self.signal_mock.assert_called_once_with(**expected_reset_kwargs) - - def test_score_reset_user_conversion(self): - """ - Ensure that the score_reset handler properly calls the - user_by_anonymous_id method to convert from an anonymized ID to a user - object - """ - submissions_score_reset_handler(None, **SUBMISSION_RESET_KWARGS) - self.get_user_mock.assert_called_once_with('anonymous_id') - - def test_score_reset_missing_kwarg(self): - """ - Ensure that, on receipt of a score_reset signal from the Submissions API - that does not have the correct kwargs, the courseware model does not - generate a signal. - """ - for missing in SUBMISSION_RESET_KWARGS: - kwargs = SUBMISSION_RESET_KWARGS.copy() - del kwargs[missing] - - with self.assertRaises(KeyError): - submissions_score_reset_handler(None, **kwargs) - self.signal_mock.assert_not_called() - - def test_score_reset_bad_user(self): - """ - Ensure that, on receipt of a score_reset signal from the Submissions API - that has an invalid user ID, the courseware model does not generate a - signal. - """ - self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.handlers.user_by_anonymous_id', None) - submissions_score_reset_handler(None, **SUBMISSION_RESET_KWARGS) - self.signal_mock.assert_not_called() - - -@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) -@ddt.ddt -class ScoreChangedUpdatesSubsectionGradeTest(ModuleStoreTestCase): - """ - Ensures that upon SCORE_CHANGED signals, the handler - initiates an update to the affected subsection grade. - """ - def setUp(self): - super(ScoreChangedUpdatesSubsectionGradeTest, self).setUp() - self.user = UserFactory() - PersistentGradesEnabledFlag.objects.create(enabled_for_all_courses=True, enabled=True) - - def set_up_course(self, enable_subsection_grades=True): - """ - Configures the course for this test. - """ - # pylint: disable=attribute-defined-outside-init,no-member - self.course = CourseFactory.create( - org='edx', - name='course', - run='run', - ) - if not enable_subsection_grades: - PersistentGradesEnabledFlag.objects.create(enabled=False) - - self.chapter = ItemFactory.create(parent=self.course, category="chapter", display_name="Chapter") - self.sequential = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Open Sequential") - self.problem = ItemFactory.create(parent=self.sequential, category='problem', display_name='problem') - - self.score_changed_kwargs = { - 'points_possible': 10, - 'points_earned': 5, - 'user': self.user, - 'course_id': unicode(self.course.id), - 'usage_id': unicode(self.problem.location), - } - - # this call caches the anonymous id on the user object, saving 4 queries in all happy path tests - _ = anonymous_id_for_user(self.user, self.course.id) - # pylint: enable=attribute-defined-outside-init,no-member - - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_subsection_grade_updated_on_signal(self, default_store): - 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(11): - recalculate_subsection_grade_handler(None, **self.score_changed_kwargs) - - def test_single_call_to_create_block_structure(self): - self.set_up_course() - self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) - with patch( - 'openedx.core.lib.block_structure.factory.BlockStructureFactory.create_from_cache', - return_value=None, - ) as mock_block_structure_create: - recalculate_subsection_grade_handler(None, **self.score_changed_kwargs) - self.assertEquals(mock_block_structure_create.call_count, 1) - - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_query_count_does_not_change_with_more_problems(self, default_store): - 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(11): - recalculate_subsection_grade_handler(None, **self.score_changed_kwargs) - - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_subsection_grades_not_enabled_on_course(self, default_store): - 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(0): - recalculate_subsection_grade_handler(None, **self.score_changed_kwargs) - - @skip("Pending completion of TNL-5089") @ddt.data( - (ModuleStoreEnum.Type.mongo, True), - (ModuleStoreEnum.Type.split, True), - (ModuleStoreEnum.Type.mongo, False), - (ModuleStoreEnum.Type.split, False), + [submissions_score_set_handler, SUBMISSION_SET_KWARGS], + [submissions_score_reset_handler, SUBMISSION_RESET_KWARGS] ) @ddt.unpack - def test_score_changed_sent_with_feature_flag(self, default_store, feature_flag): - PersistentGradesEnabledFlag.objects.create(enabled=feature_flag) - with self.store.default_store(default_store): - self.set_up_course() - with check_mongo_calls(0) and self.assertNumQueries(15 if feature_flag else 1): - SCORE_CHANGED.send(sender=None, **self.score_changed_kwargs) + def test_score_set_missing_kwarg(self, handler, kwargs): + """ + Ensure that, on receipt of a score_(re)set signal from the Submissions API + that does not have the correct kwargs, the courseware model does not + generate a signal. + """ + for missing in kwargs: + local_kwargs = kwargs.copy() + del local_kwargs[missing] - @ddt.data('user', 'course_id', 'usage_id') - def test_missing_kwargs(self, kwarg): - self.set_up_course() - self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) - del self.score_changed_kwargs[kwarg] - with self.assertRaises(KeyError): - recalculate_subsection_grade_handler(None, **self.score_changed_kwargs) + with self.assertRaises(KeyError): + handler(None, **local_kwargs) + self.signal_mock.assert_not_called() + + @ddt.data( + [submissions_score_set_handler, SUBMISSION_SET_KWARGS], + [submissions_score_reset_handler, SUBMISSION_RESET_KWARGS] + ) + @ddt.unpack + def test_score_set_bad_user(self, handler, kwargs): + """ + Ensure that, on receipt of a score_(re)set signal from the Submissions API + that has an invalid user ID, the courseware model does not generate a + signal. + """ + self.get_user_mock = self.setup_patch('lms.djangoapps.grades.signals.handlers.user_by_anonymous_id', None) + handler(None, **kwargs) + self.signal_mock.assert_not_called() diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py new file mode 100644 index 0000000000..579cf5109a --- /dev/null +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -0,0 +1,156 @@ +""" +Tests for the functionality and infrastructure of grades tasks. +""" + +import ddt +from django.conf import settings +from mock import patch +from unittest import skip + +from student.models import anonymous_id_for_user +from student.tests.factories import UserFactory +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 + + +@patch.dict(settings.FEATURES, {'PERSISTENT_GRADES_ENABLED_FOR_ALL_TESTS': False}) +@ddt.ddt +class RecalculateSubsectionGradeTest(ModuleStoreTestCase): + """ + Ensures that the recalculate subsection grade task functions as expected when run. + """ + def setUp(self): + super(RecalculateSubsectionGradeTest, self).setUp() + self.user = UserFactory() + PersistentGradesEnabledFlag.objects.create(enabled_for_all_courses=True, enabled=True) + + def set_up_course(self, enable_subsection_grades=True): + """ + Configures the course for this test. + """ + # pylint: disable=attribute-defined-outside-init,no-member + self.course = CourseFactory.create( + org='edx', + name='course', + run='run', + ) + if not enable_subsection_grades: + PersistentGradesEnabledFlag.objects.create(enabled=False) + + self.chapter = ItemFactory.create(parent=self.course, category="chapter", display_name="Chapter") + self.sequential = ItemFactory.create(parent=self.chapter, category='sequential', display_name="Open Sequential") + self.problem = ItemFactory.create(parent=self.sequential, category='problem', display_name='problem') + + self.score_changed_kwargs = { + 'user_id': self.user.id, + 'course_id': unicode(self.course.id), + 'usage_id': unicode(self.problem.location), + } + + # this call caches the anonymous id on the user object, saving 4 queries in all happy path tests + _ = 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): + """ + Ensures that the SCORE_CHANGED signal enqueues a recalculate subsection grade task. + """ + self.set_up_course() + with patch( + 'lms.djangoapps.grades.tasks.recalculate_subsection_grade.apply_async', + return_value=None + ) as mock_task_apply: + SCORE_CHANGED.send(sender=None, **self.score_changed_kwargs) + mock_task_apply.assert_called_once_with( + args=( + self.score_changed_kwargs['user_id'], + self.score_changed_kwargs['course_id'], + self.score_changed_kwargs['usage_id'], + ) + ) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_subsection_grade_updated(self, default_store): + 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): + recalculate_subsection_grade.apply( + args=( + self.score_changed_kwargs['user_id'], + self.score_changed_kwargs['course_id'], + self.score_changed_kwargs['usage_id'], + ) + ) + + def test_single_call_to_create_block_structure(self): + self.set_up_course() + self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) + with patch( + 'openedx.core.lib.block_structure.factory.BlockStructureFactory.create_from_cache', + return_value=None, + ) as mock_block_structure_create: + recalculate_subsection_grade.apply( + args=( + self.score_changed_kwargs['user_id'], + self.score_changed_kwargs['course_id'], + self.score_changed_kwargs['usage_id'], + ) + ) + self.assertEquals(mock_block_structure_create.call_count, 1) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_query_count_does_not_change_with_more_problems(self, default_store): + 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): + recalculate_subsection_grade.apply( + args=( + self.score_changed_kwargs['user_id'], + self.score_changed_kwargs['course_id'], + self.score_changed_kwargs['usage_id'], + ) + ) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_subsection_grades_not_enabled_on_course(self, default_store): + 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(0): + recalculate_subsection_grade.apply( + args=( + self.score_changed_kwargs['user_id'], + self.score_changed_kwargs['course_id'], + self.score_changed_kwargs['usage_id'], + ) + ) + + @skip("Pending completion of TNL-5089") + @ddt.data( + (ModuleStoreEnum.Type.mongo, True), + (ModuleStoreEnum.Type.split, True), + (ModuleStoreEnum.Type.mongo, False), + (ModuleStoreEnum.Type.split, False), + ) + @ddt.unpack + def test_query_counts_with_feature_flag(self, default_store, feature_flag): + PersistentGradesEnabledFlag.objects.create(enabled=feature_flag) + 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( + args=( + self.score_changed_kwargs['user_id'], + self.score_changed_kwargs['course_id'], + self.score_changed_kwargs['usage_id'], + ) + ) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index f1e3b0b436..732a674803 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -326,7 +326,7 @@ def _fire_score_changed_for_block(course_id, student, block, module_state_key): sender=None, points_possible=points_possible, points_earned=points_earned, - user=student, + user_id=student.id, course_id=unicode(course_id), usage_id=unicode(module_state_key) ) diff --git a/lms/djangoapps/lti_provider/tasks.py b/lms/djangoapps/lti_provider/tasks.py index 7980a68f22..692dff1e36 100644 --- a/lms/djangoapps/lti_provider/tasks.py +++ b/lms/djangoapps/lti_provider/tasks.py @@ -27,13 +27,13 @@ def score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument """ points_possible = kwargs.get('points_possible', None) points_earned = kwargs.get('points_earned', None) - user = kwargs.get('user', None) + user_id = kwargs.get('user_id', None) course_id = kwargs.get('course_id', None) usage_id = kwargs.get('usage_id', None) - if None not in (points_earned, points_possible, user.id, course_id, user.id): + if None not in (points_earned, points_possible, user_id, course_id): course_key, usage_key = parse_course_and_usage_keys(course_id, usage_id) - assignments = increment_assignment_versions(course_key, usage_key, user.id) + assignments = increment_assignment_versions(course_key, usage_key, user_id) for assignment in assignments: if assignment.usage_key == usage_key: send_leaf_outcome.delay( @@ -41,15 +41,15 @@ def score_changed_handler(sender, **kwargs): # pylint: disable=unused-argument ) else: send_composite_outcome.apply_async( - (user.id, course_id, assignment.id, assignment.version_number), + (user_id, course_id, assignment.id, assignment.version_number), countdown=settings.LTI_AGGREGATE_SCORE_PASSBACK_DELAY ) else: log.error( "Outcome Service: Required signal parameter is None. " - "points_possible: %s, points_earned: %s, user: %s, " + "points_possible: %s, points_earned: %s, user_id: %s, " "course_id: %s, usage_id: %s", - points_possible, points_earned, unicode(user), course_id, usage_id + points_possible, points_earned, user_id, course_id, usage_id )