Skip to content

Commit 4dd4abc

Browse files
suhaibabsi-instsuhaibabsi-inst
authored
Sync students order in SpeedGrader with that shown in Submissions list (#3481)
refs: MBL-18890 affects: Teacher release note: Fixed student ordering in SpeedGrader to match that used in submissions list.
1 parent 4173301 commit 4dd4abc

File tree

9 files changed

+120
-59
lines changed

9 files changed

+120
-59
lines changed

‎Core/Core/Features/Submissions/Submission.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,21 @@ extension Submission {
439439
}
440440
}
441441

442+
extension Submission: Comparable {
443+
444+
/// This nearly matching ordering used in GetSubmissions use case.
445+
public static func < (lhs: Submission, rhs: Submission) -> Bool {
446+
let lhsSortableName = lhs.sortableName ?? lhs.user?.sortableName
447+
let rhsSortableName = rhs.sortableName ?? rhs.user?.sortableName
448+
449+
if let name1 = lhsSortableName, let name2 = rhsSortableName {
450+
return name1 < name2
451+
}
452+
453+
return lhs.userID < rhs.userID
454+
}
455+
}
456+
442457
/// This is merely used to properly describe the state of submission in certain contexts.
443458
/// It is not strictly matching `SubmissionStatus` in all cases. And it is not
444459
/// meant to replace status cases, or be used in all related areas of the apps.

‎Teacher/Teacher.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
089AACBA2DD5F7FB0022F341 /* TeacherAssignmentDetailsScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = 089AACB92DD5F7F80022F341 /* TeacherAssignmentDetailsScreen.swift */; };
2424
089AACBE2DD5FAAE0022F341 /* TeacherDateSection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 089AACBD2DD5FAAA0022F341 /* TeacherDateSection.swift */; };
2525
089AACC02DD5FAF20022F341 /* TeacherSubmissionBreakdownView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 089AACBF2DD5FAEC0022F341 /* TeacherSubmissionBreakdownView.swift */; };
26+
08BFD0B92E24611D0086DD69 /* SubmissionsSortComparator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 08BFD0B82E24611B0086DD69 /* SubmissionsSortComparator.swift */; };
2627
13B07FBF1A68108700A75B9A /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 13B07FB51A68108700A75B9A /* Assets.xcassets */; };
2728
2C50EDE11E735CA50096592C /* AssetsLibrary.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 2C50EDE01E735CA50096592C /* AssetsLibrary.framework */; };
2829
2C50EDFD1E735CAC0096592C /* CoreTelephony.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 2C50EDFC1E735CAC0096592C /* CoreTelephony.framework */; };
@@ -277,6 +278,7 @@
277278
089AACB92DD5F7F80022F341 /* TeacherAssignmentDetailsScreen.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TeacherAssignmentDetailsScreen.swift; sourceTree = "<group>"; };
278279
089AACBD2DD5FAAA0022F341 /* TeacherDateSection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TeacherDateSection.swift; sourceTree = "<group>"; };
279280
089AACBF2DD5FAEC0022F341 /* TeacherSubmissionBreakdownView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TeacherSubmissionBreakdownView.swift; sourceTree = "<group>"; };
281+
08BFD0B82E24611B0086DD69 /* SubmissionsSortComparator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubmissionsSortComparator.swift; sourceTree = "<group>"; };
280282
13B07F961A680F5B00A75B9A /* Teacher.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = Teacher.app; sourceTree = BUILT_PRODUCTS_DIR; };
281283
13B07FB51A68108700A75B9A /* Assets.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = Assets.xcassets; sourceTree = "<group>"; };
282284
2C3DE5091F21494D00AE1D59 /* Info.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
@@ -522,6 +524,7 @@
522524
0856FCFF2DCB09CE009067CF /* SubmissionListItem.swift */,
523525
0856FCD12DB64663009067CF /* SubmissionListInteractorLive.swift */,
524526
0856FCCF2DB644EC009067CF /* SubmissionListInteractor.swift */,
527+
08BFD0B82E24611B0086DD69 /* SubmissionsSortComparator.swift */,
525528
);
526529
path = Model;
527530
sourceTree = "<group>";
@@ -1810,6 +1813,7 @@
18101813
0856FCCC2DB641EC009067CF /* SubmissionListScreen.swift in Sources */,
18111814
CFFEADAC2E00703000277F69 /* RollCallSession.swift in Sources */,
18121815
7D742F382554D01400FD6CF1 /* RubricsView.swift in Sources */,
1816+
08BFD0B92E24611D0086DD69 /* SubmissionsSortComparator.swift in Sources */,
18131817
CF13E550299D0E0B00F57F69 /* QuizSubmissionListInteractor.swift in Sources */,
18141818
494AE0BA1F843CB5001A8F31 /* Routes.swift in Sources */,
18151819
CF13E559299D1AEC00F57F69 /* QuizSubmissionListView.swift in Sources */,

‎Teacher/Teacher/Assignments/AssignmentDetails/Views/TeacherAssignmentDetailsScreen.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,6 @@ extension TeacherAssignmentDetailsScreen {
333333
actionHandler: {
334334
env.router.route(
335335
to: "/courses/\(courseID)/gradebook/speed_grader?assignment_id=\(assignmentID)",
336-
userInfo: [SpeedGraderUserInfoKey.sortNeedsGradingSubmissionsFirst: true],
337336
from: controller,
338337
options: .modal(.fullScreen, isDismissable: false, embedInNav: true)
339338
)

‎Teacher/Teacher/Routes.swift

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,20 +181,17 @@ let router = Router(routes: [
181181
)
182182
},
183183

184-
RouteHandler("/courses/:courseID/gradebook/speed_grader") { url, _, userInfo, env in
184+
RouteHandler("/courses/:courseID/gradebook/speed_grader") { url, _, _, env in
185185
guard
186186
let context = Context(path: url.path),
187187
let assignmentId = url.queryValue(for: "assignment_id")
188188
else { return nil }
189189

190-
let sortNeedsGradingFirst = (userInfo?[SpeedGraderUserInfoKey.sortNeedsGradingSubmissionsFirst] as? Bool) ?? false
191-
192190
return SpeedGraderAssembly.makeSpeedGraderViewController(
193191
context: context,
194192
assignmentId: assignmentId,
195193
userId: url.queryValue(for: "student_id"),
196194
filter: [],
197-
sortNeedsGradingSubmissionsFirst: sortNeedsGradingFirst,
198195
env: env
199196
)
200197
},
@@ -217,7 +214,6 @@ let router = Router(routes: [
217214
assignmentId: assignmentId,
218215
userId: userId,
219216
filter: filter,
220-
sortNeedsGradingSubmissionsFirst: false,
221217
env: env
222218
)
223219
},

‎Teacher/Teacher/SpeedGrader/SpeedGraderAssembly.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ enum SpeedGraderAssembly {
2626
assignmentId: String,
2727
userId: String?,
2828
filter: [GetSubmissions.Filter],
29-
sortNeedsGradingSubmissionsFirst: Bool,
3029
env: AppEnvironment
3130
) -> UIViewController {
3231
let normalizedUserId = SpeedGraderUserIdNormalization.normalizeUserId(userId)
@@ -40,7 +39,6 @@ enum SpeedGraderAssembly {
4039
assignmentID: assignmentId,
4140
userID: normalizedUserId,
4241
filter: filter,
43-
sortNeedsGradingSubmissionsFirst: sortNeedsGradingSubmissionsFirst,
4442
gradeStatusInteractor: gradeStatusInteractor,
4543
env: env
4644
)

‎Teacher/Teacher/SpeedGrader/SpeedGraderScreen/Model/SpeedGraderInteractorLive.swift

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,12 @@ class SpeedGraderInteractorLive: SpeedGraderInteractor {
3434
private let env: AppEnvironment
3535
private let filter: [GetSubmissions.Filter]
3636
private var subscriptions = Set<AnyCancellable>()
37-
private let sortNeedsGradingSubmissionsFirst: Bool
3837

3938
init(
4039
context: Context,
4140
assignmentID: String,
4241
userID: String,
4342
filter: [GetSubmissions.Filter],
44-
sortNeedsGradingSubmissionsFirst: Bool,
4543
gradeStatusInteractor: GradeStatusInteractor,
4644
env: AppEnvironment
4745
) {
@@ -50,7 +48,6 @@ class SpeedGraderInteractorLive: SpeedGraderInteractor {
5048
self.assignmentID = assignmentID
5149
self.userID = userID
5250
self.filter = filter
53-
self.sortNeedsGradingSubmissionsFirst = sortNeedsGradingSubmissionsFirst
5451
self.gradeStatusInteractor = gradeStatusInteractor
5552
}
5653

@@ -94,9 +91,8 @@ class SpeedGraderInteractorLive: SpeedGraderInteractor {
9491
} receiveValue: { [weak self] (assignment: Assignment, fetchedSubmissions: [Submission]) in
9592
guard let self else { return }
9693

97-
let submissions = sortNeedsGradingSubmissionsFirst
98-
? fetchedSubmissions.sorted(by: Self.needsGradingFirstSortingStrategy)
99-
: fetchedSubmissions
94+
let submissions = fetchedSubmissions
95+
.sorted(using: .submissionsSortComparator)
10096

10197
if submissions.isEmpty {
10298
state.send(.error(.submissionNotFound))
@@ -172,26 +168,3 @@ class SpeedGraderInteractorLive: SpeedGraderInteractor {
172168
.eraseToAnyPublisher()
173169
}
174170
}
175-
176-
// MARK: - Grading-Based Sorting Strategy
177-
178-
public enum SpeedGraderUserInfoKey {
179-
static let sortNeedsGradingSubmissionsFirst = "sortNeedsGradingSubmissionsFirst"
180-
}
181-
182-
private extension SpeedGraderInteractorLive {
183-
184-
static let needsGradingFirstSortingStrategy: (Submission, Submission) -> Bool = { sub1, sub2 in
185-
/// Put 'Needs Grading' first
186-
if sub1.needsGrading != sub2.needsGrading {
187-
return sub1.needsGrading
188-
}
189-
190-
/// Put 'Graded' last
191-
if sub1.isGraded != sub2.isGraded {
192-
return sub2.isGraded
193-
}
194-
195-
return false
196-
}
197-
}

‎Teacher/Teacher/Submissions/SubmissionsList/Model/SubmissionsSortComparator.swift

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
//
2+
// This file is part of Canvas.
3+
// Copyright (C) 2025-present Instructure, Inc.
4+
//
5+
// This program is free software: you can redistribute it and/or modify
6+
// it under the terms of the GNU Affero General Public License as
7+
// published by the Free Software Foundation, either version 3 of the
8+
// License, or (at your option) any later version.
9+
//
10+
// This program is distributed in the hope that it will be useful,
11+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
// GNU Affero General Public License for more details.
14+
//
15+
// You should have received a copy of the GNU Affero General Public License
16+
// along with this program. If not, see <https://www.gnu.org/licenses/>.
17+
//
18+
19+
import Core
20+
import Foundation
21+
22+
struct SubmissionsSortComparator: SortComparator {
23+
24+
var order: SortOrder
25+
26+
func compare(_ lhs: Core.Submission, _ rhs: Core.Submission) -> ComparisonResult {
27+
let sub1KindOrder = lhs.listSectionKind.order
28+
let sub2KindOrder = rhs.listSectionKind.order
29+
30+
if sub1KindOrder != sub2KindOrder {
31+
return sub1KindOrder < sub2KindOrder ? inOrder : againstOrder
32+
}
33+
34+
// This is to match ordering of submission as
35+
// they are being fetched to submission list
36+
// via GetSubmissions use case.
37+
if lhs != rhs {
38+
return lhs < rhs ? inOrder : againstOrder
39+
}
40+
41+
return .orderedSame
42+
}
43+
44+
private var inOrder: ComparisonResult {
45+
order == .forward ? .orderedAscending : .orderedDescending
46+
}
47+
48+
private var againstOrder: ComparisonResult {
49+
order == .forward ? .orderedDescending : .orderedAscending
50+
}
51+
}
52+
53+
// MARK: Convenience Extensions
54+
55+
extension Array where Element == Submission {
56+
57+
func toSectionedItems(assignment: Assignment? = nil) -> [SubmissionListSection] {
58+
let list = sorted(using: .submissionsSortComparator)
59+
60+
var displayIndex = 0
61+
return Dictionary(grouping: list, by: { $0.listSectionKind })
62+
.sorted(by: { $0.key.order < $1.key.order })
63+
.filter { $0.value.isNotEmpty }
64+
.map { (kind, submissions) in
65+
SubmissionListSection(
66+
kind: kind,
67+
items: submissions.map { sub in
68+
displayIndex += 1
69+
return SubmissionListItem(submission: sub, assignment: assignment, displayIndex: displayIndex)
70+
}
71+
)
72+
}
73+
}
74+
}
75+
76+
extension SortComparator where Self == SubmissionsSortComparator {
77+
static var submissionsSortComparator: Self {
78+
SubmissionsSortComparator(order: .forward)
79+
}
80+
}
81+
82+
// MARK: Sorting Helper Extensions
83+
84+
private extension SubmissionListSection.Kind {
85+
var order: Int { return Self.allCases.firstIndex(of: self) ?? -1 }
86+
}
87+
88+
extension Submission {
89+
var listSectionKind: SubmissionListSection.Kind {
90+
SubmissionListSection.Kind
91+
.allCases
92+
.first(where: { $0.filter(self) }) ?? .others
93+
}
94+
}

‎Teacher/Teacher/Submissions/SubmissionsList/ViewModel/SubmissionListViewModel.swift

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,28 +56,13 @@ class SubmissionListViewModel: ObservableObject {
5656
interactor.submissions.receive(on: scheduler),
5757
$searchText.throttle(for: 1, scheduler: scheduler, latest: true)
5858
)
59-
.map({ (list, searchText) in
60-
59+
.map({ [weak self] (list, searchText) in
6160
let searchTerm = searchText.lowercased()
6261
var curatedList: [Submission] = list
6362
if searchTerm.isNotEmpty {
6463
curatedList = curatedList.filter { $0.user?.nameContains(searchTerm) ?? false }
6564
}
66-
67-
var itemIndex = 0
68-
let sections = SubmissionListSection.Kind.allCases
69-
.map { [weak self] kind in
70-
let items = curatedList
71-
.filter { kind.filter($0) }
72-
.map {
73-
itemIndex += 1
74-
return SubmissionListItem(submission: $0, assignment: self?.assignment, displayIndex: itemIndex)
75-
}
76-
return SubmissionListSection(kind: kind, items: items)
77-
}
78-
.filter { $0.items.isNotEmpty }
79-
80-
return sections
65+
return curatedList.toSectionedItems(assignment: self?.assignment)
8166
})
8267
.assign(to: &$sections)
8368

‎Teacher/TeacherTests/SpeedGrader/SpeedGraderScreen/Model/SpeedGraderInteractorLiveTests.swift

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@ class SpeedGraderInteractorLiveTests: TeacherTestCase {
4545
assignmentID: testData.assignmentId,
4646
userID: testData.userId,
4747
filter: [],
48-
sortNeedsGradingSubmissionsFirst: false,
4948
gradeStatusInteractor: gradeStatusInteractorMock,
5049
env: environment
5150
)
@@ -110,10 +109,10 @@ class SpeedGraderInteractorLiveTests: TeacherTestCase {
110109
assignmentID: testData.assignmentId
111110
)
112111
api.mock(getSubmission, value: [
113-
.make(id: "1", submission_history: [], submission_type: .online_upload, user_id: "1", workflow_state: .unsubmitted),
112+
.make(id: "1", submission_history: [], submission_type: .online_upload, submitted_at: nil, user_id: "1", workflow_state: .unsubmitted),
114113
.make(id: "2", submission_history: [], submission_type: .online_upload, user_id: "2", workflow_state: .pending_review),
115114
.make(id: "3", score: 98, submission_history: [], submission_type: .online_upload, user_id: "3", workflow_state: .graded),
116-
.make(id: "4", submission_history: [], submission_type: .online_upload, user_id: "4", workflow_state: .unsubmitted),
115+
.make(id: "4", submission_history: [], submission_type: .online_upload, submitted_at: nil, user_id: "4", workflow_state: .unsubmitted),
117116
.make(id: "5", submission_history: [], submission_type: .online_upload, user_id: "5")
118117
])
119118

@@ -123,7 +122,6 @@ class SpeedGraderInteractorLiveTests: TeacherTestCase {
123122
assignmentID: testData.assignmentId,
124123
userID: "1",
125124
filter: [],
126-
sortNeedsGradingSubmissionsFirst: true,
127125
gradeStatusInteractor: GradeStatusInteractorMock(),
128126
env: environment
129127
)
@@ -188,7 +186,6 @@ class SpeedGraderInteractorLiveTests: TeacherTestCase {
188186
assignmentID: testData.assignmentId,
189187
userID: testData.invalidUserId,
190188
filter: [],
191-
sortNeedsGradingSubmissionsFirst: false,
192189
gradeStatusInteractor: GradeStatusInteractorMock(),
193190
env: environment
194191
)

0 commit comments

Comments
 (0)