[SAK-42980] Assignment submission list - comparator has errors when sorting by content review - Sakai

Details

Type: Bug
Status: CLOSED
Priority: Major
Resolution: Fixed
Affects Version/s: 20.0, 21.0 [Tentative]
Fix Version/s: 20.0, 21.0 [Tentative]
Component/s: Assignments
Labels:
None

20 status:
Resolved

Test Plan:
Hide

If a job trigger exists for "Process Content Review Queue" / "Process Content Review Reports", disable it

Create an assignment using content review that accepts inline and multiple attachments.

Identify cases for your content review service that will result in an error status, e.g. submitting a paper with fewer than 20 words, submitting an image file, etc.

As various students, create submissions containing multiple attachments with a diverse amount of plagiarism (I like to use http://enneagon.org/phrases which scrambles french literature; also useful to reveal encoding issues).

Create submissions where some attachments will have scores, and others will be in an error status

Create submissions where all attachments are in an error status

As admin, run the "Process Content Review Queue" and "Process Content Review Reports" jobs as appropriate until all the scores / error statuses return

As various students, create some more submissions such that their content review items are in a pending state

Resubmit to existing submissions adding new attachments, so we get a mix with pending states as well

As the instructor, view the submission list, and sort by the content-review column both ascending and descending; ensure that in descending order, the submissions are sorted by their maximum content review item's score, followed by Error status only, followed by Pending status only, followed by all the No Submission entries.
Show
If a job trigger exists for "Process Content Review Queue" / "Process Content Review Reports", disable it Create an assignment using content review that accepts inline and multiple attachments. Identify cases for your content review service that will result in an error status, e.g. submitting a paper with fewer than 20 words, submitting an image file, etc. As various students, create submissions containing multiple attachments with a diverse amount of plagiarism (I like to use http://enneagon.org/phrases which scrambles french literature; also useful to reveal encoding issues). Create submissions where some attachments will have scores, and others will be in an error status Create submissions where all attachments are in an error status As admin, run the "Process Content Review Queue" and "Process Content Review Reports" jobs as appropriate until all the scores / error statuses return As various students, create some more submissions such that their content review items are in a pending state Resubmit to existing submissions adding new attachments, so we get a mix with pending states as well As the instructor, view the submission list, and sort by the content-review column both ascending and descending; ensure that in descending order, the submissions are sorted by their maximum content review item's score, followed by Error status only, followed by Pending status only, followed by all the No Submission entries.

Description

The desired order is defined in ~~SAK-40425~~:

If this column is clicked, the sort order should be by the submissions' maximum plagiarism match score descending. As for submissions with other statuses, etc., our comparison should order them: items with a score > error status > pending status > no submission. That is, if you have:
submission 1 with 3 content review items with scores 3%, 2%, 8%
submission 2 with 3 content review items: 50%, 5%, 3%
submission 3 with 1 content review item: 7%
submission 4 with 2 content review items in pending status and error status
submission 5 with 2 content review items, both in pending status
submission 6 with no submission

The descending order should be submissions 2, 1, 3, 4, 5, 6

Here are some issues I've found so far:

1)

result = score1 > score2 ? 1 : -1;

If we have two submissions, s1 and s2, with equal scores, then compare(s1, s2) == compare(s2, s1) == -1. If the scores are equal, compare should return 0.

2)

This might be a result of 1), but when I sort a particular assignment's submissions in ascending order, it behaves as expected (scores ascend after all the "No Submission" entries); however, if I sort the submissions in descending order, I have the following:
10%,
3%,
0%,
..."No Submission" entries...,
6%,
..."No Submission" entries...,
100%,
..."No Submission" entries...

3)

This algorithm:

                             // Find the highest score in all of the possible submissions
                            for (ContentReviewResult crr1 : r1) {
                                String reviewReport = crr1.getReviewReport();
                                // Yes, "Error" appears to be magic throughout the review code
                                // Error should appear before pending
                                if (StringUtils.equals(reviewReport, "Error")) {
                                    score1 = -1;
                                } else if (crr1.getReviewScore() > score1) {
                                    score1 = crr1.getReviewScore(); // This will return -2 for pending
                                }
                            }

Consider if a submission has 3 content review items: Error, 100%, 10%.
Error <- score1 is -1
100% <- score1 is 100%
10% <- score1 stays 100%
All good!
Now consider if it's iterated: 100%, Error, 10%
100% <- score1 is 100%
Error <- Oh no, score1 is -1!
10% <- score1 is now 10%

Likely as a result of 1), I get the following in my logs:

org.sakaiproject.assignment.tool.AssignmentAction.sizeResources sorting mode = Assignment.mode_instructor_grade_assignment, sort = sorted_grade_submission_by_contentreview, ascending 
= false, Comparison method violates its general contract!

Assignment submission list - comparator has errors when sorting by content review

Details

Description

Gliffy Diagrams

Zeplin

Attachments

Issue Links

Activity

People

Dates

Git Integration