Merge pull request #12 from edx/muzaffar/removed-sub-with-0-score-tnl1125

Ignore the submissions that have 0 score.

submissions/api.py

@@ -377,6 +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.
     A score is only calculated for a student item if it has completed the workflow for
     a particular assessment module.
 
@@ -398,7 +399,7 @@ def get_top_submissions(course_id, item_id, item_type, number_of_top_scores, use
 
     Returns:
         topscores (dict): The top scores for the assessment for the student item.
-            An empty array if there are no scores.
+            An empty array if there are no scores or all scores are 0.
 
     Raises:
         SubmissionNotFoundError: Raised when a submission cannot be found for
@@ -446,6 +447,7 @@ def get_top_submissions(course_id, item_id, item_type, number_of_top_scores, use
                 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")
 
             if read_replica:
@@ -467,8 +469,8 @@ def get_top_submissions(course_id, item_id, item_type, number_of_top_scores, use
             for score in scores
         ]
 
-    # Always store the retrieved list in the cache
-    cache.set(cache_key, top_submissions, TOP_SUBMISSIONS_CACHE_TIMEOUT)
+        # Always store the retrieved list in the cache
+        cache.set(cache_key, top_submissions, TOP_SUBMISSIONS_CACHE_TIMEOUT)
 
     return top_submissions
 

submissions/tests/test_api.py

@@ -370,6 +370,42 @@ 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_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")
+
+        api.set_score(student_1['uuid'], 8, 10)
+        api.set_score(student_2['uuid'], 4, 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"],
+                "Peer_Submission", 3,
+                use_cache=False,
+                read_replica=False,
+            )
+            self.assertEqual(
+                top_scores,
+                [
+                    {
+                        'content': "Hello World",
+                        'score': 8
+                    },
+                    {
+                        'content': "Hello World",
+                        'score': 4
+                    }
+                ]
+            )
+
     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")
@@ -404,6 +440,40 @@ 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")
+
+        api.set_score(student_1['uuid'], 8, 10)
+        api.set_score(student_2['uuid'], 4, 10)
+        api.set_score(student_3['uuid'], 0, 10)
+
+        # The first call should hit the database
+        with self.assertNumQueries(1):
+            scores = api.get_top_submissions(
+                STUDENT_ITEM["course_id"],
+                STUDENT_ITEM["item_id"],
+                STUDENT_ITEM["item_type"], 3,
+                use_cache=True,
+                read_replica=False
+            )
+        self.assertEqual(scores, [
+            {"content": "Hello World", "score": 8},
+            {"content": "Hello World", "score": 4},
+        ])
+
+        # The second call should use the cache
+        with self.assertNumQueries(0):
+            cached_scores = api.get_top_submissions(
+                STUDENT_ITEM["course_id"],
+                STUDENT_ITEM["item_id"],
+                STUDENT_ITEM["item_type"], 3,
+                use_cache=True,
+                read_replica=False
+            )
+        self.assertEqual(cached_scores, scores)
+
     @raises(api.SubmissionRequestError)
     def test_error_on_get_top_submissions_too_few(self):
         student_item = copy.deepcopy(STUDENT_ITEM)