Merge pull request #15908 from edx/sstudent/state-deletion

account for delete state in grading code

lms/djangoapps/grades/new/subsection_grade.py

@@ -148,35 +148,41 @@ class SubsectionGrade(SubsectionGradeBase):
         """
         Saves the subsection grade in a persisted model.
         """
-        subsection_grades = filter(lambda subs_grade: subs_grade._should_persist_per_attempted, subsection_grades)
-        return PersistentSubsectionGrade.bulk_create_grades(
-            [subsection_grade._persisted_model_params(student) for subsection_grade in subsection_grades],  # pylint: disable=protected-access
-            course_key,
-        )
+        params = [
+            subsection_grade._persisted_model_params(student) for subsection_grade in subsection_grades  # pylint: disable=protected-access
+            if subsection_grade._should_persist_per_attempted()  # pylint: disable=protected-access
+        ]
+        return PersistentSubsectionGrade.bulk_create_grades(params, course_key)
 
     def create_model(self, student):
         """
         Saves the subsection grade in a persisted model.
         """
-        if self._should_persist_per_attempted:
+        if self._should_persist_per_attempted():
             self._log_event(log.debug, u"create_model", student)
             return PersistentSubsectionGrade.create_grade(**self._persisted_model_params(student))
 
-    def update_or_create_model(self, student):
+    def update_or_create_model(self, student, score_deleted):
         """
         Saves or updates the subsection grade in a persisted model.
         """
-        if self._should_persist_per_attempted:
+        if self._should_persist_per_attempted(score_deleted):
             self._log_event(log.debug, u"update_or_create_model", student)
             return PersistentSubsectionGrade.update_or_create_grade(**self._persisted_model_params(student))
 
-    @property
-    def _should_persist_per_attempted(self):
+    def _should_persist_per_attempted(self, score_deleted=False):
         """
         Returns whether the SubsectionGrade's model should be
         persisted based on settings and attempted status.
+
+        If the learner's score was just deleted, they will have
+        no attempts but the grade should still be persisted.
         """
-        return not waffle().is_enabled(WRITE_ONLY_IF_ENGAGED) or self.all_total.first_attempted is not None
+        return (
+            not waffle().is_enabled(WRITE_ONLY_IF_ENGAGED) or
+            self.all_total.first_attempted is not None or
+            score_deleted
+        )
 
     def _compute_block_score(
             self,

lms/djangoapps/grades/new/subsection_grade_factory.py

@@ -63,7 +63,7 @@ class SubsectionGradeFactory(object):
         )
         self._unsaved_subsection_grades.clear()
 
-    def update(self, subsection, only_if_higher=None):
+    def update(self, subsection, only_if_higher=None, score_deleted=False):
         """
         Updates the SubsectionGrade object for the student and subsection.
         """
@@ -93,7 +93,7 @@ class SubsectionGradeFactory(object):
                     ):
                         return orig_subsection_grade
 
-            grade_model = calculated_grade.update_or_create_model(self.student)
+            grade_model = calculated_grade.update_or_create_model(self.student, score_deleted)
             self._update_saved_subsection_grade(subsection.location, grade_model)
 
         return calculated_grade

lms/djangoapps/grades/signals/handlers.py

@@ -40,6 +40,7 @@ log = getLogger(__name__)
 GRADES_RESCORE_EVENT_TYPE = 'edx.grades.problem.rescored'
 PROBLEM_SUBMITTED_EVENT_TYPE = 'edx.grades.problem.submitted'
 SUBSECTION_OVERRIDE_EVENT_TYPE = 'edx.grades.subsection.score_overridden'
+STATE_DELETED_EVENT_TYPE = 'edx.grades.problem.state_deleted'
 
 
 @receiver(score_set)

lms/djangoapps/grades/tasks.py

@@ -187,6 +187,7 @@ def _recalculate_subsection_grade(self, **kwargs):
             scored_block_usage_key,
             kwargs['only_if_higher'],
             kwargs['user_id'],
+            kwargs['score_deleted'],
         )
     except Exception as exc:   # pylint: disable=broad-except
         if not isinstance(exc, KNOWN_RETRY_ERRORS):
@@ -246,7 +247,7 @@ def _has_db_updated_with_new_score(self, scored_block_usage_key, **kwargs):
     return db_is_updated
 
 
-def _update_subsection_grades(course_key, scored_block_usage_key, only_if_higher, user_id):
+def _update_subsection_grades(course_key, scored_block_usage_key, only_if_higher, user_id, score_deleted):
     """
     A helper function to update subsection grades in the database
     for each subsection containing the given block, and to signal
@@ -271,6 +272,7 @@ def _update_subsection_grades(course_key, scored_block_usage_key, only_if_higher
                 subsection_grade = subsection_grade_factory.update(
                     course_structure[subsection_usage_key],
                     only_if_higher,
+                    score_deleted
                 )
                 SUBSECTION_SCORE_CHANGED.send(
                     sender=None,

lms/djangoapps/grades/tests/test_new.py

@@ -328,6 +328,24 @@ class TestSubsectionGradeFactory(ProblemSubmissionTestMixin, GradeTestBase):
             grade = self.subsection_grade_factory.update(self.sequence)
         self.assert_grade(grade, 1, 2)
 
+    def test_write_only_if_engaged(self):
+        """
+        Test that scores are not persisted when a learner has
+        never attempted a problem, but are persisted if the
+        learner's state has been deleted.
+        """
+        with waffle().override(WRITE_ONLY_IF_ENGAGED):
+            with mock_get_score(0, 0, None):
+                self.subsection_grade_factory.update(self.sequence)
+        # ensure no grades have been persisted
+        self.assertEqual(0, len(PersistentSubsectionGrade.objects.all()))
+
+        with waffle().override(WRITE_ONLY_IF_ENGAGED):
+            with mock_get_score(0, 0, None):
+                self.subsection_grade_factory.update(self.sequence, score_deleted=True)
+        # ensure a grade has been persisted
+        self.assertEqual(1, len(PersistentSubsectionGrade.objects.all()))
+
     def test_update_if_higher(self):
         def verify_update_if_higher(mock_score, expected_grade):
             """

lms/djangoapps/grades/tests/utils.py

@@ -24,7 +24,7 @@ def mock_passing_grade(grade_pass='Pass', percent=0.75, ):
 
 
 @contextmanager
-def mock_get_score(earned=0, possible=1):
+def mock_get_score(earned=0, possible=1, first_attempted=datetime(2000, 1, 1, 0, 0, 0)):
     """
     Mocks the get_score function to return a valid grade.
     """
@@ -36,7 +36,7 @@ def mock_get_score(earned=0, possible=1):
             weighted_possible=possible,
             weight=1,
             graded=True,
-            first_attempted=datetime(2000, 1, 1, 0, 0, 0)
+            first_attempted=first_attempted
         )
         yield mock_score
 

lms/djangoapps/instructor/enrollment.py

@@ -20,7 +20,7 @@ from courseware.models import StudentModule
 from edxmako.shortcuts import render_to_string
 from eventtracking import tracker
 from lms.djangoapps.grades.constants import ScoreDatabaseTableEnum
-from lms.djangoapps.grades.signals.handlers import disconnect_submissions_signal_receiver
+from lms.djangoapps.grades.signals.handlers import disconnect_submissions_signal_receiver, STATE_DELETED_EVENT_TYPE
 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
@@ -274,17 +274,16 @@ def reset_student_attempts(course_id, student, module_state_key, requesting_user
     if delete_module:
         module_to_reset.delete()
         create_new_event_transaction_id()
-        grade_update_root_type = 'edx.grades.problem.state_deleted'
-        set_event_transaction_type(grade_update_root_type)
+        set_event_transaction_type(STATE_DELETED_EVENT_TYPE)
         tracker.emit(
-            unicode(grade_update_root_type),
+            unicode(STATE_DELETED_EVENT_TYPE),
             {
                 'user_id': unicode(student.id),
                 'course_id': unicode(course_id),
                 'problem_id': unicode(module_state_key),
                 'instructor_id': unicode(requesting_user.id),
                 'event_transaction_id': unicode(get_event_transaction_id()),
-                'event_transaction_type': unicode(grade_update_root_type),
+                'event_transaction_type': unicode(STATE_DELETED_EVENT_TYPE),
             }
         )
         if not submission_cleared: