Skip to content

E
edx-submissions
Commit 8fb070d2 by Muzaffar yousaf
Options

Merge pull request #13 from edx/muzaffar/fix-top-submissions-tnl1455 

The Top submissions should not be including the cancelled ones.
parents ff6a76e4 85bab344
Showing with 82 additions and 41 deletions
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_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)
......@@ -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_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)
# 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)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment