Merge pull request #15791 from edx/EDUCATOR-926

EDUCATOR-1127 Override grade to zero when exam attempt is rejected Final

lms/djangoapps/grades/models.py

@@ -412,7 +412,22 @@ class PersistentSubsectionGrade(DeleteGradesMixin, TimeStampedModel):
         usage_key = params.pop('usage_key')
 
         # apply grade override if one exists before saving model
-        # EDUCTATOR-1127: remove override until this behavior is verified in production
+        try:
+            override = PersistentSubsectionGradeOverride.objects.get(
+                grade__user_id=user_id,
+                grade__course_id=usage_key.course_key,
+                grade__usage_key=usage_key,
+            )
+            if override.earned_all_override is not None:
+                params['earned_all'] = override.earned_all_override
+            if override.possible_all_override is not None:
+                params['possible_all'] = override.possible_all_override
+            if override.earned_graded_override is not None:
+                params['earned_graded'] = override.earned_graded_override
+            if override.possible_graded_override is not None:
+                params['possible_graded'] = override.possible_graded_override
+        except PersistentSubsectionGradeOverride.DoesNotExist:
+            pass
 
         grade, _ = cls.objects.update_or_create(
             user_id=user_id,

lms/djangoapps/grades/services.py

 from datetime import datetime
 
-import logging
 import pytz
 
 from opaque_keys.edx.keys import CourseKey, UsageKey
@@ -12,8 +11,6 @@ from .constants import ScoreDatabaseTableEnum
 from .models import PersistentSubsectionGrade, PersistentSubsectionGradeOverride
 from .signals.signals import SUBSECTION_OVERRIDE_CHANGED
 
-log = logging.getLogger(__name__)
-
 
 def _get_key(key_or_id, key_cls):
     """
@@ -73,21 +70,40 @@ class GradesService(object):
         Fires off a recalculate_subsection_grade async task to update the PersistentSubsectionGrade table. Will not
         override earned_all or earned_graded value if they are None. Both default to None.
         """
+        # prevent circular imports:
+        from .signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE
+
         course_key = _get_key(course_key_or_id, CourseKey)
         usage_key = _get_key(usage_key_or_id, UsageKey)
 
-        log.info(
-            u"EDUCATOR-1127: Subsection grade override for user {user_id} on subsection {usage_key} in course "
-            u"{course_key} would be created with params: {params}"
-            .format(
-                user_id=unicode(user_id),
-                usage_key=unicode(usage_key),
-                course_key=unicode(course_key),
-                params=unicode({
-                    'earned_all': earned_all,
-                    'earned_graded': earned_graded,
-                })
-            )
+        grade = PersistentSubsectionGrade.objects.get(
+            user_id=user_id,
+            course_id=course_key,
+            usage_key=usage_key
+        )
+
+        # Create override that will prevent any future updates to grade
+        override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(
+            grade=grade,
+            earned_all_override=earned_all,
+            earned_graded_override=earned_graded
+        )
+
+        # Cache a new event id and event type which the signal handler will use to emit a tracking log event.
+        create_new_event_transaction_id()
+        set_event_transaction_type(SUBSECTION_OVERRIDE_EVENT_TYPE)
+
+        # Signal will trigger subsection recalculation which will call PersistentSubsectionGrade.update_or_create_grade
+        # which will use the above override to update the grade before writing to the table.
+        SUBSECTION_OVERRIDE_CHANGED.send(
+            sender=None,
+            user_id=user_id,
+            course_id=unicode(course_key),
+            usage_id=unicode(usage_key),
+            only_if_higher=False,
+            modified=override.modified,
+            score_deleted=False,
+            score_db_table=ScoreDatabaseTableEnum.overrides
         )
 
     def undo_override_subsection_grade(self, user_id, course_key_or_id, usage_key_or_id):
@@ -97,17 +113,33 @@ class GradesService(object):
         Fires off a recalculate_subsection_grade async task to update the PersistentSubsectionGrade table. If the
         override does not exist, no error is raised, it just triggers the recalculation.
         """
+        # prevent circular imports:
+        from .signals.handlers import SUBSECTION_OVERRIDE_EVENT_TYPE
+
         course_key = _get_key(course_key_or_id, CourseKey)
         usage_key = _get_key(usage_key_or_id, UsageKey)
 
-        log.info(
-            u"EDUCATOR-1127: Subsection grade override for user {user_id} on subsection {usage_key} in course "
-            u"{course_key} would be deleted"
-            .format(
-                user_id=unicode(user_id),
-                usage_key=unicode(usage_key),
-                course_key=unicode(course_key)
-            )
+        override = self.get_subsection_grade_override(user_id, course_key, usage_key)
+        # Older rejected exam attempts that transition to verified might not have an override created
+        if override is not None:
+            override.delete()
+
+        # Cache a new event id and event type which the signal handler will use to emit a tracking log event.
+        create_new_event_transaction_id()
+        set_event_transaction_type(SUBSECTION_OVERRIDE_EVENT_TYPE)
+
+        # Signal will trigger subsection recalculation which will call PersistentSubsectionGrade.update_or_create_grade
+        # which will no longer use the above deleted override, and instead return the grade to the original score from
+        # the actual problem responses before writing to the table.
+        SUBSECTION_OVERRIDE_CHANGED.send(
+            sender=None,
+            user_id=user_id,
+            course_id=unicode(course_key),
+            usage_id=unicode(usage_key),
+            only_if_higher=False,
+            modified=datetime.now().replace(tzinfo=pytz.UTC),  # Not used when score_deleted=True
+            score_deleted=True,
+            score_db_table=ScoreDatabaseTableEnum.overrides
         )
 
     def should_override_grade_on_rejected_exam(self, course_key_or_id):

lms/djangoapps/grades/tests/test_models.py

@@ -312,9 +312,8 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
         override = PersistentSubsectionGradeOverride(grade=grade, earned_all_override=0.0, earned_graded_override=0.0)
         override.save()
         grade = PersistentSubsectionGrade.update_or_create_grade(**self.params)
-        # EDUCATOR-1127 Override is not enabled yet, change to 0.0 when enabled
-        self.assertEqual(grade.earned_all, 6.0)
-        self.assertEqual(grade.earned_graded, 6.0)
+        self.assertEqual(grade.earned_all, 0.0)
+        self.assertEqual(grade.earned_graded, 0.0)
 
     def _assert_tracker_emitted_event(self, tracker_mock, grade):
         """

lms/djangoapps/grades/tests/test_services.py

@@ -171,10 +171,28 @@ class GradesServiceTests(ModuleStoreTestCase):
             self.course.id,
             self.subsection.location
         )
-        self.assertIsNone(override_obj)
+        self.assertIsNotNone(override_obj)
+        self.assertEqual(override_obj.earned_all_override, override['earned_all'])
+        self.assertEqual(override_obj.earned_graded_override, override['earned_graded'])
+
+        self.assertEqual(
+            self.mock_signal.call_args,
+            call(
+                sender=None,
+                user_id=self.user.id,
+                course_id=unicode(self.course.id),
+                usage_id=unicode(self.subsection.location),
+                only_if_higher=False,
+                modified=override_obj.modified,
+                score_deleted=False,
+                score_db_table=ScoreDatabaseTableEnum.overrides
+            )
+        )
 
     @freeze_time('2017-01-01')
     def test_undo_override_subsection_grade(self):
+        override, _ = PersistentSubsectionGradeOverride.objects.update_or_create(grade=self.grade)
+
         self.service.undo_override_subsection_grade(
             user_id=self.user.id,
             course_key_or_id=self.course.id,
@@ -184,6 +202,20 @@ class GradesServiceTests(ModuleStoreTestCase):
         override = self.service.get_subsection_grade_override(self.user.id, self.course.id, self.subsection.location)
         self.assertIsNone(override)
 
+        self.assertEqual(
+            self.mock_signal.call_args,
+            call(
+                sender=None,
+                user_id=self.user.id,
+                course_id=unicode(self.course.id),
+                usage_id=unicode(self.subsection.location),
+                only_if_higher=False,
+                modified=datetime.now().replace(tzinfo=pytz.UTC),
+                score_deleted=True,
+                score_db_table=ScoreDatabaseTableEnum.overrides
+            )
+        )
+
     @ddt.data(
         ['edX/DemoX/Demo_Course', CourseKey.from_string('edX/DemoX/Demo_Course'), CourseKey],
         ['course-v1:edX+DemoX+Demo_Course', CourseKey.from_string('course-v1:edX+DemoX+Demo_Course'), CourseKey],

lms/djangoapps/grades/tests/test_tasks.py

@@ -163,10 +163,10 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEquals(mock_block_structure_create.call_count, 1)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 29, True),
-        (ModuleStoreEnum.Type.mongo, 1, 25, False),
-        (ModuleStoreEnum.Type.split, 3, 29, True),
-        (ModuleStoreEnum.Type.split, 3, 25, False),
+        (ModuleStoreEnum.Type.mongo, 1, 30, True),
+        (ModuleStoreEnum.Type.mongo, 1, 26, False),
+        (ModuleStoreEnum.Type.split, 3, 30, True),
+        (ModuleStoreEnum.Type.split, 3, 26, False),
     )
     @ddt.unpack
     def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
@@ -178,8 +178,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
                     self._apply_recalculate_subsection_grade()
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 29),
-        (ModuleStoreEnum.Type.split, 3, 29),
+        (ModuleStoreEnum.Type.mongo, 1, 30),
+        (ModuleStoreEnum.Type.split, 3, 30),
     )
     @ddt.unpack
     def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
@@ -239,8 +239,8 @@ class RecalculateSubsectionGradeTest(HasCourseWithProblemsMixin, ModuleStoreTest
             self.assertEqual(len(PersistentSubsectionGrade.bulk_read_grades(self.user.id, self.course.id)), 0)
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 1, 26),
-        (ModuleStoreEnum.Type.split, 3, 26),
+        (ModuleStoreEnum.Type.mongo, 1, 27),
+        (ModuleStoreEnum.Type.split, 3, 27),
     )
     @ddt.unpack
     def test_persistent_grades_enabled_on_course(self, default_store, num_mongo_queries, num_sql_queries):

lms/templates/courseware/progress.html

@@ -183,18 +183,15 @@ from django.utils.http import urlquote_plus
                                                 <em class="localized-datetime" data-datetime="${section.due}" data-string="${_('due {date}')}" data-timezone="${user_timezone}" data-language="${user_language}"></em>
                                             %endif
                                         </p>
-                                        <%doc>
-                                            EDUCATOR-1127: Do not display override notice until override is enabled
+                                        <p class="override-notice">
                                             %if section.override is not None:
-                                                <p class="override-notice">
-                                                    %if section.format is not None and section.format == "Exam":
-                                                        ${_("Exam grade has been overridden due to a failed proctoring review.")}
-                                                    %else:
-                                                        ${_("Section grade has been overridden.")}
-                                                    %endif
-                                                </p>
+                                                %if section.format is not None and section.format == "Exam":
+                                                    ${_("Suspicious activity detected during proctored exam review. Exam score 0.")}
+                                                %else:
+                                                    ${_("Section grade has been overridden.")}
+                                                %endif
                                             %endif
-                                        </%doc>
+                                        </p>
                                         %if len(section.problem_scores.values()) > 0:
                                           %if section.show_grades(staff_access):
                                           <dl class="scores">