Return top submissions without the cancelled ones.

TNL-1455

submissions/api.py

@@ -377,7 +377,7 @@ def get_top_submissions(course_id, item_id, item_type, number_of_top_scores, use
     """Get a number of top scores for an assessment based on a particular student item
 
     This function will return top scores for the piece of assessment.
-    It will ignore the submissions that have 0 score/points_earned.
+    It will consider only the latest and greater than 0 score for a piece of assessment.
     A score is only calculated for a student item if it has completed the workflow for
     a particular assessment module.
 
@@ -443,18 +443,18 @@ def get_top_submissions(course_id, item_id, item_type, number_of_top_scores, use
     # By default, prefer the read-replica.
     if top_submissions is None:
         try:
-            query = Score.objects.filter(
+            query = ScoreSummary.objects.filter(
                 student_item__course_id=course_id,
                 student_item__item_id=item_id,
                 student_item__item_type=item_type,
-                points_earned__gt=0
-            ).select_related("submission").order_by("-points_earned")
+                latest__points_earned__gt=0
+            ).select_related('latest', 'latest__submission').order_by("-latest__points_earned")
 
             if read_replica:
                 query = _use_read_replica(query)
-            scores = query[:number_of_top_scores]
+            score_summaries = query[:number_of_top_scores]
         except DatabaseError:
-            msg = u"Could not fetch top scores for course {}, item {} of type {}".format(
+            msg = u"Could not fetch top score summaries for course {}, item {} of type {}".format(
                 course_id, item_id, item_type
             )
             logger.exception(msg)
@@ -463,10 +463,10 @@ def get_top_submissions(course_id, item_id, item_type, number_of_top_scores, use
         # Retrieve the submission content for each top score
         top_submissions = [
             {
-                "score": score.points_earned,
-                "content": SubmissionSerializer(score.submission).data['answer']
+                "score": score_summary.latest.points_earned,
+                "content": SubmissionSerializer(score_summary.latest.submission).data['answer']
             }
-            for score in scores
+            for score_summary in score_summaries
         ]
 
         # Always store the retrieved list in the cache

submissions/tests/test_api.py

@@ -283,13 +283,18 @@ class TestSubmissionsApi(TestCase):
             )
 
     def test_get_top_submissions(self):
-        student_item = copy.deepcopy(STUDENT_ITEM)
-        student_item["course_id"] = "get_scores_course"
-        student_item["item_id"] = "i4x://a/b/c/s1"
+        student_item_1 = copy.deepcopy(STUDENT_ITEM)
+        student_item_1['student_id'] = 'Tim'
+
+        student_item_2 = copy.deepcopy(STUDENT_ITEM)
+        student_item_2['student_id'] = 'Bob'
+
+        student_item_3 = copy.deepcopy(STUDENT_ITEM)
+        student_item_3['student_id'] = 'Li'
 
-        student_1 = api.create_submission(student_item, "Hello World")
-        student_2 = api.create_submission(student_item, "Hello World")
-        student_3 = api.create_submission(student_item, "Hello World")
+        student_1 = api.create_submission(student_item_1, "Hello World")
+        student_2 = api.create_submission(student_item_2, "Hello World")
+        student_3 = api.create_submission(student_item_3, "Hello World")
 
         api.set_score(student_1['uuid'], 8, 10)
         api.set_score(student_2['uuid'], 4, 10)
@@ -298,8 +303,8 @@ class TestSubmissionsApi(TestCase):
         # Get top scores works correctly
         with self.assertNumQueries(1):
             top_scores = api.get_top_submissions(
-                student_item["course_id"],
-                student_item["item_id"],
+                STUDENT_ITEM["course_id"],
+                STUDENT_ITEM["item_id"],
                 "Peer_Submission", 3,
                 use_cache=False,
                 read_replica=False,
@@ -324,8 +329,8 @@ class TestSubmissionsApi(TestCase):
 
         # Fewer top scores available than the number requested.
         top_scores = api.get_top_submissions(
-            student_item["course_id"],
-            student_item["item_id"],
+            STUDENT_ITEM["course_id"],
+            STUDENT_ITEM["item_id"],
             "Peer_Submission", 10,
             use_cache=False,
             read_replica=False
@@ -350,8 +355,8 @@ class TestSubmissionsApi(TestCase):
 
         # More top scores available than the number requested.
         top_scores = api.get_top_submissions(
-            student_item["course_id"],
-            student_item["item_id"],
+            STUDENT_ITEM["course_id"],
+            STUDENT_ITEM["item_id"],
             "Peer_Submission", 2,
             use_cache=False,
             read_replica=False
@@ -371,23 +376,32 @@ class TestSubmissionsApi(TestCase):
         )
 
     def test_get_top_submissions_with_score_greater_than_zero(self):
-        student_item = copy.deepcopy(STUDENT_ITEM)
-        student_item["course_id"] = "get_scores_course"
-        student_item["item_id"] = "i4x://a/b/c/s1"
+        student_item_1 = copy.deepcopy(STUDENT_ITEM)
+        student_item_1['student_id'] = 'Tim'
+
+        student_item_2 = copy.deepcopy(STUDENT_ITEM)
+        student_item_2['student_id'] = 'Bob'
 
-        student_1 = api.create_submission(student_item, "Hello World")
-        student_2 = api.create_submission(student_item, "Hello World")
-        student_3 = api.create_submission(student_item, "Hello World")
+        student_item_3 = copy.deepcopy(STUDENT_ITEM)
+        student_item_3['student_id'] = 'Li'
+
+        student_1 = api.create_submission(student_item_1, "Hello World")
+        student_2 = api.create_submission(student_item_2, "Hello World")
+        student_3 = api.create_submission(student_item_3, "Hello World")
 
         api.set_score(student_1['uuid'], 8, 10)
         api.set_score(student_2['uuid'], 4, 10)
+        # These scores should not appear in top submissions.
+        # because we are considering the scores which are
+        # latest and greater than 0.
+        api.set_score(student_3['uuid'], 5, 10)
         api.set_score(student_3['uuid'], 0, 10)
 
         # Get greater than 0 top scores works correctly
         with self.assertNumQueries(1):
             top_scores = api.get_top_submissions(
-                student_item["course_id"],
-                student_item["item_id"],
+                STUDENT_ITEM["course_id"],
+                STUDENT_ITEM["item_id"],
                 "Peer_Submission", 3,
                 use_cache=False,
                 read_replica=False,
@@ -407,9 +421,18 @@ class TestSubmissionsApi(TestCase):
             )
 
     def test_get_top_submissions_from_cache(self):
-        student_1 = api.create_submission(STUDENT_ITEM, "Hello World")
-        student_2 = api.create_submission(STUDENT_ITEM, "Hello World")
-        student_3 = api.create_submission(STUDENT_ITEM, "Hello World")
+        student_item_1 = copy.deepcopy(STUDENT_ITEM)
+        student_item_1['student_id'] = 'Tim'
+
+        student_item_2 = copy.deepcopy(STUDENT_ITEM)
+        student_item_2['student_id'] = 'Bob'
+
+        student_item_3 = copy.deepcopy(STUDENT_ITEM)
+        student_item_3['student_id'] = 'Li'
+
+        student_1 = api.create_submission(student_item_1, "Hello World")
+        student_2 = api.create_submission(student_item_2, "Hello World")
+        student_3 = api.create_submission(student_item_3, "Hello World")
 
         api.set_score(student_1['uuid'], 8, 10)
         api.set_score(student_2['uuid'], 4, 10)
@@ -425,8 +448,8 @@ class TestSubmissionsApi(TestCase):
                 read_replica=False
             )
             self.assertEqual(scores, [
-                { "content": "Hello World", "score": 8 },
-                { "content": "Hello World", "score": 4 },
+                {"content": "Hello World", "score": 8},
+                {"content": "Hello World", "score": 4},
             ])
 
         # The second call should use the cache
@@ -441,9 +464,18 @@ class TestSubmissionsApi(TestCase):
             self.assertEqual(cached_scores, scores)
 
     def test_get_top_submissions_from_cache_having_greater_than_0_score(self):
-        student_1 = api.create_submission(STUDENT_ITEM, "Hello World")
-        student_2 = api.create_submission(STUDENT_ITEM, "Hello World")
-        student_3 = api.create_submission(STUDENT_ITEM, "Hello World")
+        student_item_1 = copy.deepcopy(STUDENT_ITEM)
+        student_item_1['student_id'] = 'Tim'
+
+        student_item_2 = copy.deepcopy(STUDENT_ITEM)
+        student_item_2['student_id'] = 'Bob'
+
+        student_item_3 = copy.deepcopy(STUDENT_ITEM)
+        student_item_3['student_id'] = 'Li'
+
+        student_1 = api.create_submission(student_item_1, "Hello World")
+        student_2 = api.create_submission(student_item_2, "Hello World")
+        student_3 = api.create_submission(student_item_3, "Hello World")
 
         api.set_score(student_1['uuid'], 8, 10)
         api.set_score(student_2['uuid'], 4, 10)
@@ -499,7 +531,7 @@ class TestSubmissionsApi(TestCase):
             read_replica=False
         )
 
-    @patch.object(Score.objects, 'filter')
+    @patch.object(ScoreSummary.objects, 'filter')
     @raises(api.SubmissionInternalError)
     def test_error_on_get_top_submissions_db_error(self, mock_filter):
         mock_filter.side_effect = DatabaseError("Bad things happened")

submissions/tests/test_read_replica.py

@@ -42,9 +42,18 @@ class ReadReplicaTest(TransactionTestCase):
         self.assertEqual(retrieved['points_earned'], self.SCORE['points_earned'])
 
     def test_get_top_submissions(self):
-        student_1 = sub_api.create_submission(self.STUDENT_ITEM, "Hello World")
-        student_2 = sub_api.create_submission(self.STUDENT_ITEM, "Hello World")
-        student_3 = sub_api.create_submission(self.STUDENT_ITEM, "Hello World")
+        student_item_1 = copy.deepcopy(self.STUDENT_ITEM)
+        student_item_1['student_id'] = 'Tim'
+
+        student_item_2 = copy.deepcopy(self.STUDENT_ITEM)
+        student_item_2['student_id'] = 'Bob'
+
+        student_item_3 = copy.deepcopy(self.STUDENT_ITEM)
+        student_item_3['student_id'] = 'Li'
+
+        student_1 = sub_api.create_submission(student_item_1, "Hello World")
+        student_2 = sub_api.create_submission(student_item_2, "Hello World")
+        student_3 = sub_api.create_submission(student_item_3, "Hello World")
 
         sub_api.set_score(student_1['uuid'], 8, 10)
         sub_api.set_score(student_2['uuid'], 4, 10)