Use first_attempted value from database

TNL-5930

lms/djangoapps/grades/models.py

@@ -244,6 +244,35 @@ class PersistentSubsectionGrade(TimeStampedModel):
     # track which blocks were visible at the time of grade calculation
     visible_blocks = models.ForeignKey(VisibleBlocks, db_column='visible_blocks_hash', to_field='hashed')
 
+    def _is_unattempted_with_score(self):
+        """
+        Return True if the object has a non-zero score, but has not been
+        attempted.  This is an inconsistent state, and needs to be cleaned up.
+        """
+        return self.first_attempted is None and any(field != 0.0 for field in (self.earned_all, self.earned_graded))
+
+    def enforce_unattempted(self, save=True):
+        """
+        If an grade has not been attempted, but was given a non-zero score,
+        reset the score to 0.0.
+
+        Params:
+
+        save (bool, default: True):
+            By default, this method saves the model if and only if there was an
+            inconsistency.  If the caller needs to save the model regardless of
+            the result, or will be saving the model later after making other
+            changes, this may be an unwanted database request.  It can be
+            disabled by passing ``save=False``.
+
+        Return value: None
+        """
+        if self._is_unattempted_with_score():
+            self.earned_all = 0.0
+            self.earned_graded = 0.0
+            if save:
+                self.save()
+
     @property
     def full_usage_key(self):
         """
@@ -313,6 +342,7 @@ class PersistentSubsectionGrade(TimeStampedModel):
 
         user_id = kwargs.pop('user_id')
         usage_key = kwargs.pop('usage_key')
+        attempted = kwargs.pop('attempted')
 
         grade, _ = cls.objects.update_or_create(
             user_id=user_id,
@@ -320,6 +350,11 @@ class PersistentSubsectionGrade(TimeStampedModel):
             usage_key=usage_key,
             defaults=kwargs,
         )
+        if attempted and not grade.first_attempted:
+            grade.first_attempted = now()
+            grade.save()
+        else:
+            grade.enforce_unattempted()
         return grade
 
     @classmethod
@@ -328,7 +363,15 @@ class PersistentSubsectionGrade(TimeStampedModel):
         Wrapper for objects.create.
         """
         cls._prepare_params_and_visible_blocks(kwargs)
-        return cls.objects.create(**kwargs)
+
+        attempted = kwargs.pop('attempted')
+
+        grade = cls(**kwargs)
+        if attempted:
+            grade.first_attempted = now()
+        grade.enforce_unattempted(save=False)
+        grade.save()
+        return grade
 
     @classmethod
     def bulk_create_grades(cls, grade_params_iter, course_key):
@@ -342,6 +385,10 @@ class PersistentSubsectionGrade(TimeStampedModel):
         VisibleBlocks.bulk_get_or_create([params['visible_blocks'] for params in grade_params_iter], course_key)
         map(cls._prepare_params_visible_blocks_id, grade_params_iter)
 
+        first_attempt_timestamp = now()
+        for params in grade_params_iter:
+            if params.pop('attempted'):
+                params['first_attempted'] = first_attempt_timestamp
         return cls.objects.bulk_create([PersistentSubsectionGrade(**params) for params in grade_params_iter])
 
     @classmethod
@@ -376,6 +423,14 @@ class PersistentSubsectionGrade(TimeStampedModel):
         params['visible_blocks_id'] = params['visible_blocks'].hash_value
         del params['visible_blocks']
 
+    def remove_attempts(self):
+        """
+        Explicitly mark a subsection as unattempted
+        """
+        self.first_attempted = None
+        self.enforce_unattempted(save=False)
+        self.save()
+
 
 class PersistentCourseGrade(TimeStampedModel):
     """
@@ -422,7 +477,7 @@ class PersistentCourseGrade(TimeStampedModel):
             u"grading policy: {}".format(self.grading_policy_hash),
             u"percent grade: {}%".format(self.percent_grade),
             u"letter grade: {}".format(self.letter_grade),
-            u"passed_timestamp: {}".format(self.passed_timestamp),
+            u"passed timestamp: {}".format(self.passed_timestamp),
         ])
 
     @classmethod

lms/djangoapps/grades/new/subsection_grade.py

@@ -51,6 +51,11 @@ class SubsectionGrade(object):
         Returns whether any problem in this subsection
         was attempted by the student.
         """
+
+        assert self.all_total is not None, (
+            "SubsectionGrade not fully populated yet.  Call init_from_structure or init_from_model "
+            "before use."
+        )
         return self.all_total.attempted
 
     def init_from_structure(self, student, course_structure, submissions_scores, csm_scores):
@@ -80,7 +85,7 @@ class SubsectionGrade(object):
             graded=True,
             display_name=self.display_name,
             module_id=self.location,
-            attempted=True,  # TODO TNL-5930
+            attempted=model.first_attempted is not None,
         )
         self.all_total = AggregatedScore(
             tw_earned=model.earned_all,
@@ -88,7 +93,7 @@ class SubsectionGrade(object):
             graded=False,
             display_name=self.display_name,
             module_id=self.location,
-            attempted=True,  # TODO TNL-5930
+            attempted=model.first_attempted is not None,
         )
         self._log_event(log.debug, u"init_from_model", student)
         return self
@@ -156,6 +161,7 @@ class SubsectionGrade(object):
             earned_graded=self.graded_total.earned,
             possible_graded=self.graded_total.possible,
             visible_blocks=self._get_visible_blocks,
+            attempted=self.attempted
         )
 
     @property

lms/djangoapps/grades/tests/test_models.py

@@ -210,7 +210,7 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
             "earned_graded": 6.0,
             "possible_graded": 8.0,
             "visible_blocks": self.block_records,
-            "first_attempted": "2016-08-01 18:53:24.354741",
+            "attempted": True,
         }
 
     def test_create(self):
@@ -228,17 +228,8 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
         with self.assertRaises(IntegrityError):
             PersistentSubsectionGrade.create_grade(**self.params)
 
-    def test_create_bad_params(self):
-        """
-        Confirms create will fail if params are missing.
-        """
-        del self.params["earned_graded"]
-        with self.assertRaises(IntegrityError):
-            PersistentSubsectionGrade.create_grade(**self.params)
-
-    @ddt.data("course_version", "first_attempted")
-    def test_optional_fields(self, field):
-        del self.params[field]
+    def test_optional_fields(self):
+        del self.params["course_version"]
         PersistentSubsectionGrade.create_grade(**self.params)
 
     @ddt.data(
@@ -250,6 +241,7 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
         ("earned_graded", IntegrityError),
         ("possible_graded", IntegrityError),
         ("visible_blocks", KeyError),
+        ("attempted", KeyError),
     )
     @ddt.unpack
     def test_non_optional_fields(self, field, error):
@@ -268,6 +260,42 @@ class PersistentSubsectionGradeTest(GradesModelTestCase):
             self.assertEqual(created_grade.id, updated_grade.id)
             self.assertEqual(created_grade.earned_all, 6)
 
+    def test_update_or_create_with_implicit_attempted(self):
+        grade = PersistentSubsectionGrade.update_or_create_grade(**self.params)
+        self.assertIsInstance(grade.first_attempted, datetime)
+
+    def test_create_inconsistent_unattempted(self):
+        self.params['attempted'] = False
+        grade = PersistentSubsectionGrade.create_grade(**self.params)
+        self.assertEqual(grade.earned_all, 0.0)
+
+    def test_update_inconsistent_unattempted(self):
+        self.params['attempted'] = False
+        PersistentSubsectionGrade.create_grade(**self.params)
+        grade = PersistentSubsectionGrade.update_or_create_grade(**self.params)
+        self.assertEqual(grade.earned_all, 0.0)
+
+    def test_first_attempted_not_changed_on_update(self):
+        PersistentSubsectionGrade.create_grade(**self.params)
+        moment = now()
+        grade = PersistentSubsectionGrade.update_or_create_grade(**self.params)
+        self.assertLess(grade.first_attempted, moment)
+
+    def test_unattempted_save_does_not_remove_attempt(self):
+        PersistentSubsectionGrade.create_grade(**self.params)
+        self.params['unattempted'] = False
+        grade = PersistentSubsectionGrade.update_or_create_grade(**self.params)
+        self.assertIsInstance(grade.first_attempted, datetime)
+        self.assertEqual(grade.earned_all, 6.0)
+
+    def test_explicitly_remove_attempts(self):
+        grade = PersistentSubsectionGrade.create_grade(**self.params)
+        self.assertIsInstance(grade.first_attempted, datetime)
+        self.assertEqual(grade.earned_all, 6.0)
+        grade.remove_attempts()
+        self.assertIsNone(grade.first_attempted)
+        self.assertEqual(grade.earned_all, 0.0)
+
 
 @ddt.ddt
 class PersistentCourseGradesTest(GradesModelTestCase):