-
Notifications
You must be signed in to change notification settings - Fork 122
Sync students order in SpeedGrader with that shown in Submissions list #3481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sync students order in SpeedGrader with that shown in Submissions list #3481
Conversation
refs: MBL-18890 affects: Teacher release note: Fixed student ordering in SpeedGrader to match that used in submissions list.
Release Note:Fixed student ordering in SpeedGrader to match that used in submissions list. Affected Apps: Teacher
|
@@ -182,16 +182,30 @@ public enum SpeedGraderUserInfoKey { | |||
private extension SpeedGraderInteractorLive { | |||
|
|||
static let needsGradingFirstSortingStrategy: (Submission, Submission) -> Bool = { sub1, sub2 in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had a discussion on this with @vargaat and would it be possible to extract this strategy somewhere and use that as the single source of truth for grouping and ordering both in SubmissionList and in SpeedGrader?
Please put the extension Submission: Comparable
logic there as well to have all related logic in one place.
That strategy could handle the grouping, so SubmissionList could use that for it's sections, and SpeedGrader could use its flatmapped version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rh12 We need to keep Submission
conformance to Comparable
in Core
as this is considered more safe according to Swift. Not complying with that will produce this warning:
Extension declares a conformance of imported type 'Submission' to imported protocol 'Comparable'; this will not behave correctly if the owners of 'Core' introduce this conformance in the future.
- Add '@retrospective' to silence the warning.
Teacher/Teacher/Routes.swift
Outdated
let sortNeedsGradingFirst = ( | ||
userInfo?[SpeedGraderUserInfoKey.sortNeedsGradingSubmissionsFirst] as? Bool | ||
) ?? false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed with product:
The SubmissionList ordering should be applied to all entry points (not just Submission List & AssignmentDetails SG button). It should be fixed to the currently used logic (grouped by status and alphabetical within each group)
Could you please remove the parameter and condition from the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QA + 1
Thanks for the changes!
refs: MBL-18890
affects: Teacher
release note: Fixed student ordering in SpeedGrader to match that used in submissions list.
Test Plan
By opening SpeedGrader in Teacher app, either by;
Students order as you navigate through it now must match the order used in Submissions list.
Checklist