Sort unassessed exercises by most recent submission #1167
Conversation
EerikSaksi
force-pushed
the
chronological-next-submission
branch
from
99c0ba9
to
a6e5972
Compare
| if submissions.first(): | ||
| submitter = submissions.first().submitters.first() |
There was a problem hiding this comment.
Could do submission = submissions.first() instead of calling submissions.first() twice.
There was a problem hiding this comment.
We have found bugs in the implementation.
- The selected next submitter may be such a student who has already received manual grading on a submission other than the latest one. A student should be considered manually graded once any one of his/her submissions has been manually graded. The "next submitter" link should not repeat a student who has been manually graded.
- The "next submitter" link may repeat the same student who was already being inspected. This happens when the student's earlier submission (but not the latest) has been manually graded even though other students have not received any manual grading.
- When all students have received manual grading, the "next submitter" link must stop cycling between students like the old implementation had done. The old code has an if statement with the comment "There are no more unassessed submitters". It redirects back to the submission list.
| submissions = (submissions.annotate(latest=Max('submission_time')) | ||
| .filter(submission_time=F('latest')) | ||
| .order_by('latest')) |
There was a problem hiding this comment.
Since this filters only the latest submissions, the query for the "manually assessed submissions" does not check any other submissions than the latest ones. However, usually only one submission from the student is manually assessed and it is not always the latest submission.
If there are students who have not received any manual grading, those should be prioritized. Currently, the implementation may first select students that have received manual grading on an earlier submission.
| submissions = Submission.objects.filter(exercise=self.exercise) | ||
|
|
||
| # filter only the latest submissions | ||
| submissions = (submissions.annotate(latest=Max('submission_time')) |
There was a problem hiding this comment.
Does the annotate call really query the latest submission for each submitter separately? I didn't try it, but usually GROUP BY queries include a values() invocation.
Useful links:
| # filter only the latest submissions | ||
| submissions = (submissions.annotate(latest=Max('submission_time')) | ||
| .filter(submission_time=F('latest')) | ||
| .order_by('latest')) | ||
|
|
||
| previous_user_id = request.GET.get('prev') |
There was a problem hiding this comment.
Just a thought, since the old code is changed to order by submission times, would it be more useful to set the previous timestamp to the prev GET query parameter instead of the user id?
This reverts commit 0a5d7f5. The feature for chronological sorting of the submissions in the `NextUnassessedSubmitterView` is still not working correctly. Thus, I revert this commit before making a new stable release. See issues apluslms#1196 and apluslms#1110 and pull requests apluslms#1199 and apluslms#1167.
Fixes #1110
Description
What?
Sort unassessed submissions by time when manually assessing submissions
Why?
Teachers can start grading the earliest submissions before all have been submitted
How?
Sort submissions by submission time, and if the prev URL param exists, then get the next submission after prev
Fixes #1110
Testing
Remember to add or update unit tests for new features and changes.
What type of test did you run?
Manually ensured that sorted by submission time, and order changes as submission time is changed in the database and newer submissions are added.
Did you test the changes in
Think of what is affected by these changes and could become broken
Translation
Programming style
Have you updated the README or other relevant documentation?
Is it Done?
Clean up your git commit history before submitting the pull request!