From 634ac8a825dfb7b59da5ccb8bfe7e9bbeb995565 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Thu, 16 Mar 2017 11:38:35 -0400 Subject: [PATCH] Still compute grades asynchronously when PersistentGrades is enabled --- lms/djangoapps/grades/new/course_grade.py | 30 +++++++------- lms/djangoapps/grades/new/subsection_grade.py | 41 +++++++++---------- lms/djangoapps/grades/tasks.py | 4 -- lms/djangoapps/grades/tests/test_tasks.py | 31 +++++++++++--- .../tests/test_tasks_helper.py | 2 +- 5 files changed, 63 insertions(+), 45 deletions(-) diff --git a/lms/djangoapps/grades/new/course_grade.py b/lms/djangoapps/grades/new/course_grade.py index 319a3b8156..25b597139b 100644 --- a/lms/djangoapps/grades/new/course_grade.py +++ b/lms/djangoapps/grades/new/course_grade.py @@ -158,21 +158,23 @@ class CourseGrade(object): subsections_created = len(self._subsection_grade_factory._unsaved_subsection_grades) # pylint: disable=protected-access subsections_read = subsections_total - subsections_created blocks_total = len(self.locations_to_scores) - if not read_only: - self._subsection_grade_factory.bulk_create_unsaved() - grading_policy_hash = self.get_grading_policy_hash(self.course.location, self.course_structure) - PersistentCourseGrade.update_or_create_course_grade( - user_id=self.student.id, - course_id=self.course.id, - course_version=self.course_version, - course_edited_timestamp=self.course_edited_timestamp, - grading_policy_hash=grading_policy_hash, - percent_grade=self.percent, - letter_grade=self.letter_grade or "", - passed=self.passed, - ) - self._signal_listeners_when_grade_computed() + if not read_only: + if PersistentGradesEnabledFlag.feature_enabled(self.course.id): + self._subsection_grade_factory.bulk_create_unsaved() + grading_policy_hash = self.get_grading_policy_hash(self.course.location, self.course_structure) + PersistentCourseGrade.update_or_create_course_grade( + user_id=self.student.id, + course_id=self.course.id, + course_version=self.course_version, + course_edited_timestamp=self.course_edited_timestamp, + grading_policy_hash=grading_policy_hash, + percent_grade=self.percent, + letter_grade=self.letter_grade or "", + passed=self.passed, + ) + self._signal_listeners_when_grade_computed() + self._log_event( log.warning, u"compute_and_update, read_only: {0}, subsections read/created: {1}/{2}, blocks accessed: {3}, total " diff --git a/lms/djangoapps/grades/new/subsection_grade.py b/lms/djangoapps/grades/new/subsection_grade.py index 31a004b292..2c2df98d9f 100644 --- a/lms/djangoapps/grades/new/subsection_grade.py +++ b/lms/djangoapps/grades/new/subsection_grade.py @@ -247,34 +247,33 @@ class SubsectionGradeFactory(object): """ # Save ourselves the extra queries if the course does not persist # subsection grades. - if not PersistentGradesEnabledFlag.feature_enabled(self.course.id): - return - self._log_event(log.warning, u"update, subsection: {}".format(subsection.location), subsection) calculated_grade = SubsectionGrade(subsection).init_from_structure( self.student, self.course_structure, self._submissions_scores, self._csm_scores, ) - if only_if_higher: - try: - grade_model = PersistentSubsectionGrade.read_grade(self.student.id, subsection.location) - except PersistentSubsectionGrade.DoesNotExist: - pass - else: - orig_subsection_grade = SubsectionGrade(subsection).init_from_model( - self.student, grade_model, self.course_structure, self._submissions_scores, self._csm_scores, - ) - if not is_score_higher_or_equal( - orig_subsection_grade.graded_total.earned, - orig_subsection_grade.graded_total.possible, - calculated_grade.graded_total.earned, - calculated_grade.graded_total.possible, - ): - return orig_subsection_grade + if PersistentGradesEnabledFlag.feature_enabled(self.course.id): + if only_if_higher: + try: + grade_model = PersistentSubsectionGrade.read_grade(self.student.id, subsection.location) + except PersistentSubsectionGrade.DoesNotExist: + pass + else: + orig_subsection_grade = SubsectionGrade(subsection).init_from_model( + self.student, grade_model, self.course_structure, self._submissions_scores, self._csm_scores, + ) + if not is_score_higher_or_equal( + orig_subsection_grade.graded_total.earned, + orig_subsection_grade.graded_total.possible, + calculated_grade.graded_total.earned, + calculated_grade.graded_total.possible, + ): + return orig_subsection_grade + + grade_model = calculated_grade.update_or_create_model(self.student) + self._update_saved_subsection_grade(subsection.location, grade_model) - grade_model = calculated_grade.update_or_create_model(self.student) - self._update_saved_subsection_grade(subsection.location, grade_model) return calculated_grade @lazy diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 0359deed1d..263be346cf 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -25,7 +25,6 @@ from track.event_transaction_utils import ( from util.date_utils import from_timestamp from xmodule.modulestore.django import modulestore -from .config.models import PersistentGradesEnabledFlag from .constants import ScoreDatabaseTableEnum from .new.subsection_grade import SubsectionGradeFactory from .signals.signals import SUBSECTION_SCORE_CHANGED @@ -91,9 +90,6 @@ def _recalculate_subsection_grade(self, **kwargs): """ try: 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) newrelic.agent.add_custom_parameter('course_id', unicode(course_key)) diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index bb1c99b6fa..33fdd27708 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -27,6 +27,7 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, chec from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum +from lms.djangoapps.grades.models import PersistentCourseGrade, PersistentSubsectionGrade from lms.djangoapps.grades.signals.signals import PROBLEM_WEIGHTED_SCORE_CHANGED from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3, RECALCULATE_GRADE_DELAY @@ -203,14 +204,34 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): {self.sequential.location, accessible_seq.location}, ) - @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_persistent_grades_not_enabled_on_course(self, default_store): + @ddt.data( + (ModuleStoreEnum.Type.mongo, 1, 9), + (ModuleStoreEnum.Type.split, 3, 8), + ) + @ddt.unpack + def test_persistent_grades_not_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): with self.store.default_store(default_store): self.set_up_course(enable_persistent_grades=False) - self.assertFalse(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) - with check_mongo_calls(0): - with self.assertNumQueries(0): + with check_mongo_calls(num_mongo_queries): + with self.assertNumQueries(num_sql_queries): self._apply_recalculate_subsection_grade() + with self.assertRaises(PersistentCourseGrade.DoesNotExist): + PersistentCourseGrade.read_course_grade(self.user.id, self.course.id) + self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) + + @ddt.data( + (ModuleStoreEnum.Type.mongo, 1, 22), + (ModuleStoreEnum.Type.split, 3, 21), + ) + @ddt.unpack + def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries): + with self.store.default_store(default_store): + self.set_up_course(enable_persistent_grades=True) + with check_mongo_calls(num_mongo_queries): + with self.assertNumQueries(num_sql_queries): + self._apply_recalculate_subsection_grade() + self.assertIsNotNone(PersistentCourseGrade.read_course_grade(self.user.id, self.course.id)) + self.assertGreater(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0) @patch('lms.djangoapps.grades.signals.signals.SUBSECTION_SCORE_CHANGED.send') @patch('lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory.update') diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index 4e7e8d5d4a..24d517d7e6 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -1775,7 +1775,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase): 'failed': 3, 'skipped': 2 } - with self.assertNumQueries(169): + with self.assertNumQueries(168): self.assertCertificatesGenerated(task_input, expected_results) expected_results = {