Skip to content

Commit f61c4dd

Browse files
authored
Fix file parsing failing when filename is null. Fix offline content selector screen not reacting to cancel when in error or loading states. (#2926)
refs: MBL-17877 affects: Student release note: Fixed cancel not working on offline sync picker screen while the screen is loading or displaying an error. Fixed offline sync picker failing to load in some cases. test plan: See ticket.
1 parent 00c1531 commit f61c4dd

File tree

4 files changed

+90
-5
lines changed

4 files changed

+90
-5
lines changed

Core/Core/CourseSync/CourseSyncSelector/ViewModel/CourseSyncSelectorViewModel.swift

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,40 @@ class CourseSyncSelectorViewModel: ObservableObject {
101101
updateSelectAllButtonTitle(selectorInteractor)
102102
updateNavBarSubtitle(selectorInteractor)
103103

104-
handleCancelButtonTap(cancelConfirmAlert: cancelConfirmAlert)
105104
handleLeftNavBarTap(selectorInteractor)
106105
handleSyncButtonTap(
107106
selectorInteractor: selectorInteractor,
108107
syncConfirmAlert: syncConfirmAlert
109108
)
110109

110+
dismissScreenInLoadingAndErrorState(on: cancelButtonDidTap)
111+
showConfirmationAlertInDataState(on: cancelButtonDidTap, cancelConfirmAlert: cancelConfirmAlert)
112+
111113
syncButtonDidTap.logReceiveOutput(
112114
"offline_sync_button_tapped",
113115
storeIn: &subscriptions
114116
)
115117
}
116118

117-
private func handleCancelButtonTap(cancelConfirmAlert: ConfirmationAlertViewModel) {
118-
cancelButtonDidTap
119+
private func dismissScreenInLoadingAndErrorState(on publisher: PassthroughRelay<WeakViewController>) {
120+
publisher
121+
.filter { [unowned self] _ in
122+
state == .loading || state == .error
123+
}
124+
.sink { [unowned router] viewController in
125+
router.dismiss(viewController)
126+
}
127+
.store(in: &subscriptions)
128+
}
129+
130+
private func showConfirmationAlertInDataState(
131+
on publisher: PassthroughRelay<WeakViewController>,
132+
cancelConfirmAlert: ConfirmationAlertViewModel
133+
) {
134+
publisher
135+
.filter { [unowned self] _ in
136+
state == .data
137+
}
119138
.handleEvents(receiveOutput: { [unowned self] _ in
120139
isShowingCancelConfirmationDialog = true
121140
})

Core/Core/Files/Model/API/APIFile.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ public struct APIFile: Codable, Equatable {
140140
uuid = try container.decode(String.self, forKey: .uuid)
141141
folder_id = try container.decode(ID.self, forKey: .folder_id)
142142
display_name = try container.decode(String.self, forKey: .display_name)
143-
filename = try container.decode(String.self, forKey: .filename)
144143
contentType = try container.decode(String.self, forKey: .contentType)
145144
url = try container.decodeURLIfPresent(forKey: .url)
146145
size = try container.decodeIfPresent(Int.self, forKey: .size)
@@ -161,6 +160,14 @@ public struct APIFile: Codable, Equatable {
161160
avatar = try container.decodeIfPresent(APIFileToken.self, forKey: .avatar)
162161
usage_rights = try container.decodeIfPresent(APIUsageRights.self, forKey: .usage_rights)
163162
visibility_level = try container.decodeIfPresent(String.self, forKey: .visibility_level)
163+
164+
do {
165+
/// We've seen instances where filename is `null` in the JSON.
166+
/// We fall back to `display_name` in this case since these two are usually the same.
167+
filename = try container.decode(String.self, forKey: .filename)
168+
} catch {
169+
filename = display_name
170+
}
164171
}
165172
}
166173

Core/CoreTests/CourseSync/CourseSyncSelector/ViewModel/CourseSyncSelectorViewModelTests.swift

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,57 @@ class CourseSyncSelectorViewModelTests: XCTestCase {
135135
XCTAssertEqual(testee.navBarSubtitle, "Test Name")
136136
}
137137

138-
func testCancelTap() {
138+
func testCancelTapInLoadingState() {
139139
let controller = UIViewController()
140140
let weakController = WeakViewController(controller)
141+
XCTAssertEqual(testee.state, .loading)
142+
143+
// WHEN
144+
testee.cancelButtonDidTap.accept(weakController)
145+
146+
// THEN
147+
XCTAssertEqual(testee.isShowingCancelConfirmationDialog, false)
148+
XCTAssertEqual(router.dismissed, controller)
149+
}
150+
151+
func testCancelTapInErrorState() {
152+
let controller = UIViewController()
153+
let weakController = WeakViewController(controller)
154+
mockSelectorInteractor.courseSyncEntriesSubject.send(
155+
completion: .failure(NSError.internalError())
156+
)
157+
waitUntil(1, shouldFail: true) {
158+
testee.state == .error
159+
}
160+
161+
// WHEN
141162
testee.cancelButtonDidTap.accept(weakController)
163+
164+
// THEN
165+
XCTAssertEqual(testee.isShowingCancelConfirmationDialog, false)
166+
XCTAssertEqual(router.dismissed, controller)
167+
}
168+
169+
func testCancelTapInDataState() {
170+
let controller = UIViewController()
171+
let weakController = WeakViewController(controller)
172+
mockSelectorInteractor.courseSyncEntriesSubject.send([])
173+
174+
waitUntil(1, shouldFail: true) {
175+
testee.state == .data
176+
}
177+
178+
// WHEN
179+
testee.cancelButtonDidTap.accept(weakController)
180+
181+
// THEN
182+
XCTAssertEqual(testee.isShowingCancelConfirmationDialog, true)
183+
XCTAssertEqual(router.dismissed, nil)
184+
185+
// WHEN
142186
testee.cancelConfirmAlert.notifyCompletion(isConfirmed: true)
187+
188+
// THEN
143189
XCTAssertEqual(router.dismissed, controller)
144190
}
145191

Core/CoreTests/Files/Model/API/APIFileTests.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ private let decoder = APIJSONDecoder()
2323
private let encoder = APIJSONEncoder()
2424

2525
class APIFileTests: XCTestCase {
26+
2627
func testAPIFileDecode() throws {
2728
var fixture = validFixture
2829
var data = try serialize(json: fixture)
@@ -39,6 +40,18 @@ class APIFileTests: XCTestCase {
3940
XCTAssertNil(file.thumbnail_url)
4041
}
4142

43+
func testAPIFileDecodeWithNullFileName() throws {
44+
var fixture = validFixture
45+
fixture["filename"] = nil
46+
let data = try serialize(json: fixture)
47+
48+
// WHEN
49+
let testee = try decoder.decode(APIFile.self, from: data)
50+
51+
// THEN
52+
XCTAssertEqual(testee.filename, testee.display_name)
53+
}
54+
4255
func testAPIFolderItem() throws {
4356
let data = try encoder.encode(APIFolder.make())
4457
XCTAssertThrowsError(try decoder.decode(APIFolderItem.self, from: data), "Not for actual api decoding")

0 commit comments

Comments
 (0)