From 5812c9b1b90485caca272ffb52c97dd071ba9466 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 05:57:50 +0300 Subject: [PATCH 01/13] fix: parse server dates the same way whatever the 12/24-hour setting is Date(iso8601:) parsed fixed-format API timestamps with the user's locale. When the 24-Hour Time switch is overridden in iOS settings, DateFormatter reads `HH` as a 12-hour field, so any timestamp after 12:59 failed to parse and silently became "now". Due dates showed as "Today" and past-due work was sorted as due today. Parse with en_US_POSIX, as Apple QA1480 recommends and as Date(subtitleTime:) already does. --- Core/Core/Extensions/DateExtension.swift | 6 ++++-- .../Unit/CourseDateViewModelTests.swift | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/Core/Core/Extensions/DateExtension.swift b/Core/Core/Extensions/DateExtension.swift index a07b68335..3769b1f72 100644 --- a/Core/Core/Extensions/DateExtension.swift +++ b/Core/Core/Extensions/DateExtension.swift @@ -14,8 +14,10 @@ public extension Date { var date: Date var dateFormatter: DateFormatter? dateFormatter = DateFormatter() - dateFormatter?.locale = .current - + // A fixed-format API date must not follow the user's locale: with the 24-hour + // time switch overridden, `HH` stops matching hours past noon (Apple QA1480). + dateFormatter?.locale = Locale(identifier: "en_US_POSIX") + date = formats.compactMap { format -> Date? in dateFormatter?.dateFormat = format guard let formattedDate = dateFormatter?.date(from: iso8601) else { return nil } diff --git a/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift b/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift index cd7ad9d69..6f327299f 100644 --- a/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift +++ b/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift @@ -529,4 +529,19 @@ final class CourseDateViewModelTests: XCTestCase { XCTAssertEqual(BlockStatus.status(of: "assignment-due-date"), .assignment, "Incorrect mapping for 'assignment-due-date'") XCTAssertEqual(BlockStatus.status(of: ""), .event, "Incorrect mapping for 'event'") } + + func testDateWithFractionalSecondsAfterNoon_isParsedExactly() { + var utc = Calendar(identifier: .gregorian) + utc.timeZone = TimeZone(identifier: "UTC")! + + let date = Date(iso8601: "2026-07-30T16:46:03.445838Z") + let components = utc.dateComponents([.year, .month, .day, .hour, .minute, .second], from: date) + + XCTAssertEqual(components.year, 2026) + XCTAssertEqual(components.month, 7) + XCTAssertEqual(components.day, 30) + XCTAssertEqual(components.hour, 16) + XCTAssertEqual(components.minute, 46) + XCTAssertEqual(components.second, 3) + } } From 60f186597e32d55b85f6557ea990eeac3b21e9ee Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 05:58:09 +0300 Subject: [PATCH 02/13] fix: open the Dates tab once, without a reload flash or a false error Fixes #438. Opening a course on the Dates tab (for example from "Past due assignments" on the dashboard) loaded the dates twice: once when the tab appeared and again when the course outline finished loading and posted .getCourseDates. The second load swapped the whole list for a spinner. Every load also read the cached outline into a property nothing used, and on a course opened for the first time that read failed and showed "Course component not found, please reload". The tab now skips a refresh while its own request is still running, refreshes loaded dates in the background without blanking them, ignores refresh requests for other courses, and no longer reads the outline. --- .../Dates/CourseDatesViewModel.swift | 35 ++++----- .../Unit/CourseDateViewModelTests.swift | 77 +++++++++++++++++++ 2 files changed, 91 insertions(+), 21 deletions(-) diff --git a/Course/Course/Presentation/Dates/CourseDatesViewModel.swift b/Course/Course/Presentation/Dates/CourseDatesViewModel.swift index c2473700c..36140d610 100644 --- a/Course/Course/Presentation/Dates/CourseDatesViewModel.swift +++ b/Course/Course/Presentation/Dates/CourseDatesViewModel.swift @@ -26,6 +26,7 @@ public class CourseDatesViewModel { var courseDates: CourseDates? var isOn: Bool = false var eventState: EventState? + private var isLoadingDates = false var errorMessage: String? @@ -40,7 +41,6 @@ public class CourseDatesViewModel { let config: ConfigProtocol let courseID: String let courseName: String - var courseStructure: CourseStructure? let analytics: CourseAnalytics let calendarManager: CalendarManagerProtocol @@ -84,21 +84,19 @@ public class CourseDatesViewModel { } @MainActor - func getCourseDates(courseID: String) async { - isShowProgress = true + func getCourseDates(courseID: String, withProgress: Bool = true) async { + isLoadingDates = true + isShowProgress = withProgress do { courseDates = try await interactor.getCourseDates(courseID: courseID) - await getCourseStructure(courseID: courseID) - if courseDates?.courseDateBlocks == nil { - isShowProgress = false + } catch { + // A background refresh must not wipe dates the learner is already looking at. + if withProgress { courseDates = nil - return } - isShowProgress = false - } catch { - isShowProgress = false - courseDates = nil } + isShowProgress = false + isLoadingDates = false } func showCourseDetails(componentID: String, blockLink: String) async { @@ -114,15 +112,6 @@ public class CourseDatesViewModel { } } - @MainActor - func getCourseStructure(courseID: String) async { - do { - courseStructure = try await interactor.getLoadedCourseBlocks(courseID: courseID) - } catch _ { - errorMessage = CourseLocalization.Error.componentNotFount - } - } - func syncStatus() async -> SyncStatus { return await calendarManager.courseStatus(courseID: courseID) } @@ -181,8 +170,12 @@ extension CourseDatesViewModel { } @objc private func getCourseDates(_ notification: Notification) { + // The course container asks for fresh dates every time it reloads the outline. + // The tab already loads its own dates when it appears, so skip a request that is + // already in flight and refresh loaded dates quietly instead of blanking the list. + guard notification.object as? String == courseID, !isLoadingDates else { return } Task { - await getCourseDates(courseID: courseID) + await getCourseDates(courseID: courseID, withProgress: courseDates == nil) } } diff --git a/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift b/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift index 6f327299f..6c791d2f4 100644 --- a/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift +++ b/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift @@ -530,6 +530,83 @@ final class CourseDateViewModelTests: XCTestCase { XCTAssertEqual(BlockStatus.status(of: ""), .event, "Incorrect mapping for 'event'") } + // MARK: - Dates tab refresh + + private func makeDatesViewModel(interactor: CourseInteractorProtocolMock) -> CourseDatesViewModel { + // A course opened straight on the Dates tab has no outline cached yet. + interactor.getLoadedCourseBlocksHandler = { _ in throw NoCachedDataError() } + return CourseDatesViewModel( + interactor: interactor, + router: CourseRouterMock(), + cssInjector: CSSInjectorMock(), + connectivity: ConnectivityProtocolMock(), + config: ConfigProtocolMock(), + courseID: "1", + courseName: "a", + analytics: CourseAnalyticsMock(), + calendarManager: CalendarManagerProtocolMock() + ) + } + + private var pastDueDates: CourseDates { + CourseDates( + datesBannerInfo: DatesBannerInfo( + missedDeadlines: true, + contentTypeGatingEnabled: false, + missedGatedContent: false, + verifiedUpgradeLink: nil, + status: .resetDatesBanner + ), + courseDateBlocks: [], + hasEnded: false, + learnerIsFullAccess: true, + userTimezone: nil + ) + } + + private func waitUntil(_ condition: @MainActor () -> Bool) async { + let deadline = Date().addingTimeInterval(2) + while !condition() && Date() < deadline { + try? await Task.sleep(for: .milliseconds(20)) + } + } + + func testGetCourseDates_whenCourseOutlineIsNotCachedYet_doesNotShowError() async { + let interactor = CourseInteractorProtocolMock() + let dates = pastDueDates + interactor.getCourseDatesHandler = { _ in dates } + let viewModel = makeDatesViewModel(interactor: interactor) + + await viewModel.getCourseDates(courseID: "1") + + XCTAssertNotNil(viewModel.courseDates) + XCTAssertNil(viewModel.errorMessage) + XCTAssertFalse(viewModel.showError) + } + + func testRefreshRequestedByCourse_whenDatesAreShown_keepsThemOnScreen() async { + let interactor = CourseInteractorProtocolMock() + let dates = pastDueDates + interactor.getCourseDatesHandler = { _ in dates } + let viewModel = makeDatesViewModel(interactor: interactor) + await viewModel.getCourseDates(courseID: "1") + + let refreshed = expectation(description: "dates refreshed") + let blanked = expectation(description: "list replaced by a spinner") + blanked.isInverted = true + interactor.getCourseDatesHandler = { [weak viewModel] _ in + if await MainActor.run(body: { viewModel?.isShowProgress == true || viewModel?.courseDates == nil }) { + blanked.fulfill() + } + refreshed.fulfill() + return dates + } + NotificationCenter.default.post(name: .getCourseDates, object: "1") + + await fulfillment(of: [refreshed, blanked], timeout: 1) + XCTAssertEqual(interactor.getCourseDatesCallCount, 2) + } + func testDateWithFractionalSecondsAfterNoon_isParsedExactly() { var utc = Calendar(identifier: .gregorian) utc.timeZone = TimeZone(identifier: "UTC")! From c8ae17b17cbf65324d15be3e24021b3de4bdb1d7 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 05:58:20 +0300 Subject: [PATCH 03/13] fix: refresh the Dates tab and confirm when due dates are shifted Fixes #161. Addresses #419. "Shift due dates" on a course's Dates tab moved the deadlines on the server but the tab kept showing "Missed some deadlines?" and the old past-due list, with no confirmation, so learners tapped it again. The tab posts .shiftCourseDates with a (courseID, courseName) tuple, like the app-level Dates screen and the calendar sync expect, but the tab and the course container only listened for a plain course ID and dropped it. The container also posted a plain ID, so calendar events never updated after a shift there. All senders now post the tuple, the listeners read it and react only to their own course, and the tab reloads its dates and shows the success message. While the request runs, the banner now shows a spinner in place of its button, as the app-level Dates screen does. Before, the whole list was replaced by a full-screen spinner, which on a slow connection looked like the screen had reset (#419). Now that the list refreshes in place, the Dates list also takes its section order from the dates it is drawing. It read the order from the view model, whose dates a refresh can replace while the previous value is still on screen; after a shift the order could name a section (Next Week) that the drawn dates did not have, and the force-unwrapped lookup crashed. --- .../Container/CourseContainerViewModel.swift | 9 ++++-- .../Dates/CourseDatesViewModel.swift | 19 ++++++------ .../Dates/DatesStatusInfoView.swift | 12 ++++++-- .../Dates/Elements/CourseDateListView.swift | 2 +- .../Unit/CourseDateViewModelTests.swift | 29 +++++++++++++++++++ 5 files changed, 56 insertions(+), 15 deletions(-) diff --git a/Course/Course/Presentation/Container/CourseContainerViewModel.swift b/Course/Course/Presentation/Container/CourseContainerViewModel.swift index 1c5b40012..f0cfef250 100644 --- a/Course/Course/Presentation/Container/CourseContainerViewModel.swift +++ b/Course/Course/Presentation/Container/CourseContainerViewModel.swift @@ -312,7 +312,10 @@ extension CourseTab { do { try await interactor.shiftDueDates(courseID: courseID) - NotificationCenter.default.post(name: .shiftCourseDates, object: courseID) + NotificationCenter.default.post( + name: .shiftCourseDates, + object: (courseID, courseStructure?.displayName ?? "") + ) isShowProgress = false isShowRefresh = false @@ -1775,7 +1778,9 @@ extension CourseTab { extension CourseContainerViewModel { @objc private func handleShiftDueDates(_ notification: Notification) { - if let courseID = notification.object as? String { + // Every sender posts `(courseID, courseName)`; calendar sync needs the name too. + if let (courseID, _) = notification.object as? (String, String), + courseID == courseStructure?.id { Task { await withTaskGroup(of: Void.self) { group in group.addTask { diff --git a/Course/Course/Presentation/Dates/CourseDatesViewModel.swift b/Course/Course/Presentation/Dates/CourseDatesViewModel.swift index 36140d610..d00715040 100644 --- a/Course/Course/Presentation/Dates/CourseDatesViewModel.swift +++ b/Course/Course/Presentation/Dates/CourseDatesViewModel.swift @@ -67,7 +67,10 @@ public class CourseDatesViewModel { addObservers() } - var sortedStatuses: [CompletionStatus] { + /// Statuses present in `courseDates`, in display order. Takes the dates the list is + /// drawing rather than reading `self.courseDates`, which a background refresh can + /// replace while the list still renders the previous value. + func sortedStatuses(in courseDates: CourseDates) -> [CompletionStatus] { let desiredSequence = [ CompletionStatus.completed, CompletionStatus.pastDue, @@ -79,7 +82,7 @@ public class CourseDatesViewModel { // Filter out keys that don't exist in the dictionary let filteredKeys = desiredSequence.filter { - courseDates?.statusDatesBlocks.keys.contains($0) ?? false } + courseDates.statusDatesBlocks.keys.contains($0) } return filteredKeys } @@ -180,13 +183,11 @@ extension CourseDatesViewModel { } @objc private func handleShiftDueDates(_ notification: Notification) { - if let courseID = notification.object as? String { - Task { - await getCourseDates(courseID: courseID) - await MainActor.run { [weak self] in - self?.eventState = .shiftedDueDates - } - } + guard let (courseID, _) = notification.object as? (String, String), + courseID == self.courseID else { return } + Task { + await getCourseDates(courseID: courseID, withProgress: false) + eventState = .shiftedDueDates } } diff --git a/Course/Course/Presentation/Dates/DatesStatusInfoView.swift b/Course/Course/Presentation/Dates/DatesStatusInfoView.swift index b193dae22..a554e3831 100644 --- a/Course/Course/Presentation/Dates/DatesStatusInfoView.swift +++ b/Course/Course/Presentation/Dates/DatesStatusInfoView.swift @@ -43,7 +43,13 @@ struct DatesStatusInfoView: View { .foregroundColor(Theme.Colors.textPrimary) .padding(.horizontal, 16) - if !button.isEmpty { + if isLoading { + // Keep the banner and the dates in place while the request runs, the way the + // app-level Dates screen does, instead of swapping the whole list for a spinner. + ProgressBar(size: 40, lineWidth: 8) + .frame(maxWidth: .infinity, maxHeight: 42) + .accessibilityLabel(CourseLocalization.Accessibility.loadingSection) + } else if !button.isEmpty { UnitButtonView(type: .custom(button)) { guard !isLoading else { return } isLoading = true @@ -58,6 +64,7 @@ struct DatesStatusInfoView: View { if courseDatesViewModel != nil { await courseDatesViewModel?.shiftDueDates( courseID: courseID, + withProgress: false, screen: screen, type: datesBannerInfo.status?.analyticsBannerType ?? "" ) @@ -68,11 +75,10 @@ struct DatesStatusInfoView: View { type: datesBannerInfo.status?.analyticsBannerType ?? "" ) } - isLoading = false + isLoading = false } } .padding([.leading, .trailing], 16) - .disabled(isLoading) } Spacer() } diff --git a/Course/Course/Presentation/Dates/Elements/CourseDateListView.swift b/Course/Course/Presentation/Dates/Elements/CourseDateListView.swift index bb374f73d..0cdd5743a 100644 --- a/Course/Course/Presentation/Dates/Elements/CourseDateListView.swift +++ b/Course/Course/Presentation/Dates/Elements/CourseDateListView.swift @@ -49,7 +49,7 @@ struct CourseDateListView: View { .padding(.bottom, 16) } - ForEach(Array(viewModel.sortedStatuses), id: \.self) { status in + ForEach(viewModel.sortedStatuses(in: courseDates), id: \.self) { status in let courseDateBlockDict = courseDates.statusDatesBlocks[status]! if status == .completed { CompletedBlocks( diff --git a/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift b/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift index 6c791d2f4..ca46d2fda 100644 --- a/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift +++ b/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift @@ -607,6 +607,35 @@ final class CourseDateViewModelTests: XCTestCase { XCTAssertEqual(interactor.getCourseDatesCallCount, 2) } + // MARK: - Shifting due dates + + func testShiftDueDates_reloadsDatesAndConfirmsTheShift() async { + let interactor = CourseInteractorProtocolMock() + let dates = pastDueDates + interactor.shiftDueDatesHandler = { _ in } + interactor.getCourseDatesHandler = { _ in dates } + let viewModel = makeDatesViewModel(interactor: interactor) + + await viewModel.shiftDueDates(courseID: "1", withProgress: false, screen: .courseDates, type: "") + await waitUntil { viewModel.eventState == .shiftedDueDates } + + XCTAssertEqual(interactor.shiftDueDatesCallCount, 1) + XCTAssertEqual(interactor.getCourseDatesCallCount, 1) + XCTAssertEqual(viewModel.eventState, .shiftedDueDates) + XCTAssertFalse(viewModel.isShowProgress) + } + + func testShiftDueDatesInAnotherCourse_doesNotTouchThisCourse() async { + let interactor = CourseInteractorProtocolMock() + let viewModel = makeDatesViewModel(interactor: interactor) + + NotificationCenter.default.post(name: .shiftCourseDates, object: ("2", "Another course")) + try? await Task.sleep(for: .milliseconds(200)) + + XCTAssertEqual(interactor.getCourseDatesCallCount, 0) + XCTAssertNil(viewModel.eventState) + } + func testDateWithFractionalSecondsAfterNoon_isParsedExactly() { var utc = Calendar(identifier: .gregorian) utc.timeZone = TimeZone(identifier: "UTC")! From 36fccbac9daa6365895c275c21dd67b17d449038 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:10:21 +0300 Subject: [PATCH 04/13] fix: point calendar event links at the course the event belongs to Fixes #271. Links added to synced calendar events carry a course_id that the app uses to open the course (DeepLinkManager.showCourseScreen). Since the calendar synchronization rework in #466, that field held the calendar's name instead of the course ID, so tapping a link in an event could not find the course. Pass the course ID down to where the link is built, as before #466. --- .../DatesAndCalendar/CalendarManager.swift | 56 ++++++++++++++----- .../ProfileTests/CalendarManagerTests.swift | 24 ++++++++ 2 files changed, 66 insertions(+), 14 deletions(-) diff --git a/Profile/Profile/Presentation/DatesAndCalendar/CalendarManager.swift b/Profile/Profile/Presentation/DatesAndCalendar/CalendarManager.swift index 8c6fdeb93..a64dc85e3 100644 --- a/Profile/Profile/Presentation/DatesAndCalendar/CalendarManager.swift +++ b/Profile/Profile/Presentation/DatesAndCalendar/CalendarManager.swift @@ -188,7 +188,7 @@ public final class CalendarManager: CalendarManagerProtocol { courseName: String, calendar: EKCalendar ) -> Bool { - let events = generateEvents(for: dateBlocks, courseName: courseName, calendar: calendar) + let events = generateEvents(for: dateBlocks, courseID: courseID, courseName: courseName, calendar: calendar) var saveSuccessful = true events.forEach { event in if !eventExists(event, in: calendar) { @@ -251,6 +251,7 @@ public final class CalendarManager: CalendarManagerProtocol { private func generateEvents( for dateBlocks: [Date: [CourseDateBlock]], + courseID: String, courseName: String, calendar: EKCalendar ) -> [EKEvent] { @@ -258,12 +259,22 @@ public final class CalendarManager: CalendarManagerProtocol { dateBlocks.forEach { item in let blocks = item.value if blocks.count > 1 { - if let generatedEvent = calendarEvent(for: blocks, courseName: courseName, calendar: calendar) { + if let generatedEvent = calendarEvent( + for: blocks, + courseID: courseID, + courseName: courseName, + calendar: calendar + ) { events.append(generatedEvent) } } else { if let block = blocks.first { - if let generatedEvent = calendarEvent(for: block, courseName: courseName, calendar: calendar) { + if let generatedEvent = calendarEvent( + for: block, + courseID: courseID, + courseName: courseName, + calendar: calendar + ) { events.append(generatedEvent) } } @@ -272,7 +283,12 @@ public final class CalendarManager: CalendarManagerProtocol { return events } - private func calendarEvent(for block: CourseDateBlock, courseName: String, calendar: EKCalendar) -> EKEvent? { + private func calendarEvent( + for block: CourseDateBlock, + courseID: String, + courseName: String, + calendar: EKCalendar + ) -> EKEvent? { guard !block.title.isEmpty else { return nil } let title = block.title @@ -281,7 +297,7 @@ public final class CalendarManager: CalendarManagerProtocol { let endDate = block.date var notes = "\(calendar.title)\n\n\(block.title)" - if let link = generateDeeplink(componentBlockID: block.firstComponentBlockID) { + if let link = generateDeeplink(componentBlockID: block.firstComponentBlockID, courseID: courseID) { notes += "\n\(link)" } @@ -296,7 +312,12 @@ public final class CalendarManager: CalendarManagerProtocol { ) } - private func calendarEvent(for blocks: [CourseDateBlock], courseName: String, calendar: EKCalendar) -> EKEvent? { + private func calendarEvent( + for blocks: [CourseDateBlock], + courseID: String, + courseName: String, + calendar: EKCalendar + ) -> EKEvent? { guard let block = blocks.first, !block.title.isEmpty else { return nil } let title = block.title @@ -304,7 +325,7 @@ public final class CalendarManager: CalendarManagerProtocol { let secondAlert = startDate.addingTimeInterval(Double(alertOffset) * 86400) let endDate = block.date let notes = "\(calendar.title)\n\n" + blocks.compactMap { block -> String in - if let link = generateDeeplink(componentBlockID: block.firstComponentBlockID) { + if let link = generateDeeplink(componentBlockID: block.firstComponentBlockID, courseID: courseID) { return "\(block.title)\n\(link)" } else { return block.title @@ -348,26 +369,33 @@ public final class CalendarManager: CalendarManagerProtocol { } } - private func generateDeeplink(componentBlockID: String) -> String? { + private func generateDeeplink(componentBlockID: String, courseID: String) -> String? { guard !componentBlockID.isEmpty else { return nil } let branchUniversalObject = BranchUniversalObject( canonicalIdentifier: "\(CalendarDeepLinkType.courseComponent.rawValue)/\(componentBlockID)" ) - let dictionary: NSMutableDictionary = [ - CalendarDeepLinkKeys.screenName.rawValue: CalendarDeepLinkType.courseComponent.rawValue, - CalendarDeepLinkKeys.courseID.rawValue: profileStorage.calendarSettings?.calendarName ?? "", - CalendarDeepLinkKeys.componentID.rawValue: componentBlockID - ] let metadata = BranchContentMetadata() - metadata.customMetadata = dictionary + metadata.customMetadata = NSMutableDictionary( + dictionary: deepLinkMetadata(componentBlockID: componentBlockID, courseID: courseID) + ) branchUniversalObject.contentMetadata = metadata let properties = BranchLinkProperties() let shortUrl = branchUniversalObject.getShortUrl(with: properties) return shortUrl } + /// What a calendar event's link carries. The app opens the course by `course_id` + /// (DeepLinkManager.showCourseScreen), so it must be the course ID, not the calendar name. + func deepLinkMetadata(componentBlockID: String, courseID: String) -> [String: String] { + [ + CalendarDeepLinkKeys.screenName.rawValue: CalendarDeepLinkType.courseComponent.rawValue, + CalendarDeepLinkKeys.courseID.rawValue: courseID, + CalendarDeepLinkKeys.componentID.rawValue: componentBlockID + ] + } + private func generateEvent( title: String, startDate: Date, diff --git a/Profile/ProfileTests/CalendarManagerTests.swift b/Profile/ProfileTests/CalendarManagerTests.swift index 4e3db38ef..ae84c387e 100644 --- a/Profile/ProfileTests/CalendarManagerTests.swift +++ b/Profile/ProfileTests/CalendarManagerTests.swift @@ -290,4 +290,28 @@ final class CalendarManagerTests: XCTestCase { XCTAssertEqual(manager.colorSelection?.color, Color.accentColor) } + + func testCalendarEventLink_opensTheCourseTheEventBelongsTo() { + let profileStorage = ProfileStorageMock() + profileStorage.calendarSettings = CalendarSettings( + colorSelection: "accent", + calendarName: "My Courses", + accountSelection: "iCloud", + courseCalendarSync: true + ) + let manager = CalendarManager( + persistence: ProfilePersistenceProtocolMock(), + interactor: ProfileInteractorProtocolMock(), + profileStorage: profileStorage + ) + + let metadata = manager.deepLinkMetadata( + componentBlockID: "block-v1:OpenedX+DemoX+DemoCourse+type@sequential+block@homework", + courseID: "course-v1:OpenedX+DemoX+DemoCourse" + ) + + XCTAssertEqual(metadata["course_id"], "course-v1:OpenedX+DemoX+DemoCourse") + XCTAssertEqual(metadata["component_id"], "block-v1:OpenedX+DemoX+DemoCourse+type@sequential+block@homework") + XCTAssertEqual(metadata["screen_name"], "course_component") + } } From 91ddc0e2b37191a2ad508e80d27b72ba7925c578 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:10:28 +0300 Subject: [PATCH 05/13] fix: keep figures with transparent backgrounds readable in dark mode Fixes #531. Dark mode inverts course HTML and inverts images back, so photos keep their colours. Figures exported as PNGs with dark text and lines on a transparent background then show that dark ink straight on the dark page and become nearly invisible. Give images a white backing in dark mode; the double inversion keeps it white, so the figure looks as it does on the web. Opaque images are unchanged. --- .../View/Base/Webview/Models/ColorInversionInjection.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Core/Core/View/Base/Webview/Models/ColorInversionInjection.swift b/Core/Core/View/Base/Webview/Models/ColorInversionInjection.swift index cf842acf7..705c22598 100644 --- a/Core/Core/View/Base/Webview/Models/ColorInversionInjection.swift +++ b/Core/Core/View/Base/Webview/Models/ColorInversionInjection.swift @@ -22,6 +22,12 @@ public struct ColorInversionInjection: WebViewScriptInjectionProtocol, CSSInject img, video, iframe { filter: invert(100%) hue-rotate(180deg) !important; } + img { + /* Course figures are often PNGs with dark text on a transparent + background. Give them the white page they were drawn for; the + double inversion above keeps it white. */ + background-color: #ffffff; + } } """ return cssScript(with: css) From ff6c850771296b05f1475ed177aa8de98f258c52 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:10:37 +0300 Subject: [PATCH 06/13] fix: mark an assignment done once every graded problem is attempted Fixes #650. The Assignments views treated a graded subsection as done only when the whole subsection was complete, so an unwatched video or unread text page kept an assignment "incomplete" after the learner had submitted every problem in it. Count the assignment as done when each graded problem in it has been attempted. The server marks a problem complete as soon as it is submitted, whatever the score, so block completion is the attempt signal. --- .../Container/CourseContainerViewModel.swift | 13 ++- .../CourseContainerViewModelTests.swift | 104 ++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/Course/Course/Presentation/Container/CourseContainerViewModel.swift b/Course/Course/Presentation/Container/CourseContainerViewModel.swift index f0cfef250..f2c9aa07e 100644 --- a/Course/Course/Presentation/Container/CourseContainerViewModel.swift +++ b/Course/Course/Presentation/Container/CourseContainerViewModel.swift @@ -1445,7 +1445,7 @@ extension CourseTab { for chapter in courseStructure.childs { for sequential in chapter.childs { if sequential.blockId == blockKey || sequential.id == blockKey { - if sequential.completion >= 1.0 { + if sequential.completion >= 1.0 || isEveryGradedProblemAttempted(in: sequential) { return .completed } @@ -1460,6 +1460,17 @@ extension CourseTab { return nil } + /// An assignment is done once every graded problem in it has been attempted. A problem is + /// marked complete when it is submitted, whatever the score, while the subsection itself + /// stays incomplete until its videos and text are viewed too. + private func isEveryGradedProblemAttempted(in sequential: CourseSequential) -> Bool { + let scoredTypes: Set = [.problem, .dragAndDropV2, .openassessment, .peerInstructionTool] + let gradedProblems = sequential.childs + .flatMap { $0.childs } + .filter { $0.graded && scoredTypes.contains($0.type) } + return !gradedProblems.isEmpty && gradedProblems.allSatisfy { $0.completion >= 1.0 } + } + private func createUIModels(from subsections: [CourseProgressSubsection]) -> [CourseProgressSubsectionUI] { return subsections.map { subsection in let shortLabel = getSequentialShortLabel(for: subsection.blockKey) ?? "" diff --git a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift index 6381232fa..68d01a965 100644 --- a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift +++ b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift @@ -616,4 +616,108 @@ final class CourseContainerViewModelTests: XCTestCase { viewModel.trackSelectedTab(selection: .handounds, courseId: "1", courseName: "name") XCTAssertEqual(analytics.courseOutlineHandoutsTabClickedCallCount, 1) } + + private func makeViewModel() -> CourseContainerViewModel { + let connectivity = ConnectivityProtocolMock() + connectivity.isInternetAvaliable = true + connectivity.internetReachableSubject = .init(.reachable) + return CourseContainerViewModel( + interactor: CourseInteractorProtocolMock(), + authInteractor: AuthInteractorProtocolMock(), + router: CourseRouterMock(), + analytics: CourseAnalyticsMock(), + config: ConfigMock(), + connectivity: connectivity, + manager: DownloadManagerMock(), + storage: CourseStorageMock(), + isActive: true, + courseStart: Date(), + courseEnd: nil, + enrollmentStart: nil, + enrollmentEnd: nil, + lastVisitedBlockID: nil, + coreAnalytics: CoreAnalyticsMock(), + courseHelper: courseHelperMock + ) + } + + private func block(_ id: String, _ type: BlockType, completion: Double) -> CourseBlock { + CourseBlock( + blockId: id, + id: id, + courseId: "course", + graded: true, + due: nil, + completion: completion, + type: type, + displayName: id, + studentUrl: "", + webUrl: "", + encodedVideo: nil, + multiDevice: true, + offlineDownload: nil + ) + } + + private func course(withAssignment blocks: [CourseBlock]) -> CourseStructure { + let unit = CourseVertical( + blockId: "unit", + id: "unit", + courseId: "course", + displayName: "Unit", + type: .vertical, + completion: 0, + childs: blocks, + webUrl: "" + ) + let assignment = CourseSequential( + blockId: "homework", + id: "homework", + displayName: "Homework", + type: .sequential, + completion: 0.75, + childs: [unit], + sequentialProgress: nil, + due: nil + ) + return CourseStructure( + id: "course", + graded: true, + completion: 0, + viewYouTubeUrl: "", + encodedVideo: "", + displayName: "Course", + topicID: nil, + childs: [ + CourseChapter(blockId: "section", id: "section", displayName: "Section", type: .chapter, childs: [assignment]) + ], + media: CourseMedia(image: CourseImage(raw: "", small: "", large: "")), + certificate: nil, + org: "", + isSelfPaced: true, + courseProgress: nil + ) + } + + func testAssignmentWithEveryGradedProblemAttempted_isCompleted_evenWithAnUnwatchedVideo() { + let viewModel = makeViewModel() + viewModel.courseStructure = course(withAssignment: [ + block("setup", .html, completion: 1), + block("video", .video, completion: 0), + block("problem1", .problem, completion: 1), + block("problem2", .problem, completion: 1) + ]) + + XCTAssertEqual(viewModel.getSequentialAssignmentStatus(for: "homework"), .completed) + } + + func testAssignmentWithAnUnattemptedGradedProblem_isNotCompleted() { + let viewModel = makeViewModel() + viewModel.courseStructure = course(withAssignment: [ + block("problem1", .problem, completion: 1), + block("problem2", .problem, completion: 0) + ]) + + XCTAssertEqual(viewModel.getSequentialAssignmentStatus(for: "homework"), .incomplete) + } } From 185318bd2c5eff4172050f798035aa27573e5250 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:10:45 +0300 Subject: [PATCH 07/13] fix: open the downloads list from a section's progress indicator Fixes #455. While a section's videos were downloading, tapping its progress indicator on the Home, Content and Videos screens cancelled every download in the section at once, with no way to follow progress per video. The course-level download bar already opens the downloads list, where each video shows its progress and can be cancelled on its own. Section indicators now open the same list while downloading, as the previous app did. --- .../Container/CourseContainerView.swift | 3 +++ .../Container/CourseContainerViewModel.swift | 11 +++++++++++ .../CourseCompletionCarouselSlideView.swift | 4 ++++ .../Subviews/CustomDisclosureGroup.swift | 4 ++++ .../Container/CourseContainerViewModelTests.swift | 15 +++++++++++++-- 5 files changed, 35 insertions(+), 2 deletions(-) diff --git a/Course/Course/Presentation/Container/CourseContainerView.swift b/Course/Course/Presentation/Container/CourseContainerView.swift index 84a83298a..647e5e5ab 100644 --- a/Course/Course/Presentation/Container/CourseContainerView.swift +++ b/Course/Course/Presentation/Container/CourseContainerView.swift @@ -95,6 +95,9 @@ public struct CourseContainerView: View { .onChange(of: viewModel.selection, perform: didSelect) .onChange(of: coordinate, perform: collapseHeader) .background(Theme.Colors.background) + .sheet(isPresented: $viewModel.isDownloadsListPresented) { + DownloadsView(router: viewModel.router, courseHelper: viewModel.courseHelper) + } } @ViewBuilder diff --git a/Course/Course/Presentation/Container/CourseContainerViewModel.swift b/Course/Course/Presentation/Container/CourseContainerViewModel.swift index f2c9aa07e..e8c0c5673 100644 --- a/Course/Course/Presentation/Container/CourseContainerViewModel.swift +++ b/Course/Course/Presentation/Container/CourseContainerViewModel.swift @@ -89,6 +89,7 @@ extension CourseTab { var downloadedFilesSize: Int = 0 var largestDownloadBlocks: [CourseBlock] = [] var downloadAllButtonState: OfflineView.DownloadAllState = .start + var isDownloadsListPresented = false var expandedSections: [String: Bool] = [:] var courseDeadlines: CourseDates? private(set) var assignmentSectionsData: [AssignmentSection] = [] @@ -382,8 +383,18 @@ extension CourseTab { } } + /// A section's progress indicator opens the list of videos being downloaded, where each one + /// can be cancelled, instead of cancelling the whole section on a single tap. + func showDownloadsList() { + isDownloadsListPresented = true + } + @MainActor func onDownloadViewTap(chapter: CourseChapter, state: DownloadViewState) async { + guard state != .downloading else { + showDownloadsList() + return + } let blocks = chapter.childs .flatMap { $0.childs } .flatMap { $0.childs } diff --git a/Course/Course/Presentation/Subviews/CourseCarouselView/CourseCompletionCarouselSlideView.swift b/Course/Course/Presentation/Subviews/CourseCarouselView/CourseCompletionCarouselSlideView.swift index 3b2c7bf82..13a4d004f 100644 --- a/Course/Course/Presentation/Subviews/CourseCarouselView/CourseCompletionCarouselSlideView.swift +++ b/Course/Course/Presentation/Subviews/CourseCarouselView/CourseCompletionCarouselSlideView.swift @@ -242,6 +242,10 @@ struct CourseCompletionCarouselSlideView: View { } private func downloadAllSubsections(in chapter: CourseChapter, state: DownloadViewState) { + guard state != .downloading else { + viewModelContainer.showDownloadsList() + return + } Task { var allBlocks: [CourseBlock] = [] var sequentialsToDownload: [CourseSequential] = [] diff --git a/Course/Course/Presentation/Subviews/CustomDisclosureGroup.swift b/Course/Course/Presentation/Subviews/CustomDisclosureGroup.swift index 4efc03974..dfc2ac3f0 100644 --- a/Course/Course/Presentation/Subviews/CustomDisclosureGroup.swift +++ b/Course/Course/Presentation/Subviews/CustomDisclosureGroup.swift @@ -161,6 +161,10 @@ private struct ChapterRowView: View { } private func downloadAllSubsections(state: DownloadViewState) { + guard state != .downloading else { + viewModel.showDownloadsList() + return + } Task { var allBlocks: [CourseBlock] = [] var sequentialsToDownload: [CourseSequential] = [] diff --git a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift index 68d01a965..542b339a1 100644 --- a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift +++ b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift @@ -617,7 +617,7 @@ final class CourseContainerViewModelTests: XCTestCase { XCTAssertEqual(analytics.courseOutlineHandoutsTabClickedCallCount, 1) } - private func makeViewModel() -> CourseContainerViewModel { + private func makeViewModel(manager: DownloadManagerProtocol = DownloadManagerMock()) -> CourseContainerViewModel { let connectivity = ConnectivityProtocolMock() connectivity.isInternetAvaliable = true connectivity.internetReachableSubject = .init(.reachable) @@ -628,7 +628,7 @@ final class CourseContainerViewModelTests: XCTestCase { analytics: CourseAnalyticsMock(), config: ConfigMock(), connectivity: connectivity, - manager: DownloadManagerMock(), + manager: manager, storage: CourseStorageMock(), isActive: true, courseStart: Date(), @@ -699,6 +699,17 @@ final class CourseContainerViewModelTests: XCTestCase { ) } + func testSectionIndicatorWhileDownloading_opensDownloadsListInsteadOfCancelling() async { + let downloadManager = DownloadManagerProtocolMock() + let viewModel = makeViewModel(manager: downloadManager) + let section = CourseChapter(blockId: "", id: "section", displayName: "Section", type: .chapter, childs: []) + + await viewModel.onDownloadViewTap(chapter: section, state: .downloading) + + XCTAssertTrue(viewModel.isDownloadsListPresented) + XCTAssertEqual(downloadManager.cancelDownloadingCallCount, 0) + } + func testAssignmentWithEveryGradedProblemAttempted_isCompleted_evenWithAnUnwatchedVideo() { let viewModel = makeViewModel() viewModel.courseStructure = course(withAssignment: [ From 3906ce4ab7ae5d25be9206c8dbeca7ad2108e500 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:13:16 +0300 Subject: [PATCH 08/13] fix: don't open course content the learner has no access to Fixes #337. The enrollments API reports courseware_access.has_access = false, for example when an audit learner's access has expired, and the dashboard already shows a lock on such courses, but tapping one still opened the course and loaded its content and videos. The course screen now shows a "You don't currently have access to this course" state with a back button and doesn't request the course content. A course that hasn't started yet keeps its existing "hasn't started" screen. Note: on current edx-platform master the blocks API answers 500 for such a learner (get_assignments_grades returns [] and the view unpacks two values), so before this change the screen spun forever instead. --- .../Container/CourseContainerView.swift | 20 +++++++++++++++++++ Course/Course/SwiftGen/Strings.swift | 2 ++ Course/Course/en.lproj/Localizable.strings | 1 + 3 files changed, 23 insertions(+) diff --git a/Course/Course/Presentation/Container/CourseContainerView.swift b/Course/Course/Presentation/Container/CourseContainerView.swift index 647e5e5ab..be28a132f 100644 --- a/Course/Course/Presentation/Container/CourseContainerView.swift +++ b/Course/Course/Presentation/Container/CourseContainerView.swift @@ -70,6 +70,8 @@ public struct CourseContainerView: View { self.courseDatesViewModel = courseDatesViewModel self.courseProgressViewModel = courseProgressViewModel Task { + // Without access the server has no content to give; don't ask for it. + guard viewModel.isActive != false else { return } await withTaskGroup(of: Void.self) { group in group.addTask { await viewModel.getCourseBlocks(courseID: courseID) @@ -115,6 +117,8 @@ public struct CourseContainerView: View { viewHeight: $viewHeight, dateTabIndex: CourseTab.dates.rawValue ) + } else if viewModel.isActive == false { + noAccessContent } else { ZStack(alignment: .top) { tabs @@ -164,6 +168,22 @@ public struct CourseContainerView: View { } } + private var noAccessContent: some View { + VStack(spacing: 0) { + NavigationBar( + title: title, + leftButtonAction: { viewModel.router.back() } + ) + FullScreenErrorView( + type: .noContent( + CourseLocalization.Error.courseAccessUnavailable, + image: CoreAssets.lockIcon.swiftUIImage + ) + ) + } + .background(Theme.Colors.background.ignoresSafeArea()) + } + private func showDatesSuccessView(title: String, message: String) -> some View { return DatesSuccessView( title: title, diff --git a/Course/Course/SwiftGen/Strings.swift b/Course/Course/SwiftGen/Strings.swift index e92c3f84c..bbe9b9fca 100644 --- a/Course/Course/SwiftGen/Strings.swift +++ b/Course/Course/SwiftGen/Strings.swift @@ -559,6 +559,8 @@ public enum CourseLocalization { public static let assignmentsUnavailable = CourseLocalization.tr("Localizable", "ERROR.ASSIGNMENTS_UNAVAILABLE", fallback: "No assignments available for this course.") /// Course component not found, please reload public static let componentNotFount = CourseLocalization.tr("Localizable", "ERROR.COMPONENT_NOT_FOUNT", fallback: "Course component not found, please reload") + /// You don't currently have access to this course. + public static let courseAccessUnavailable = CourseLocalization.tr("Localizable", "ERROR.COURSE_ACCESS_UNAVAILABLE", fallback: "You don't currently have access to this course.") /// Course dates are not currently available. public static let courseDateUnavailable = CourseLocalization.tr("Localizable", "ERROR.COURSE_DATE_UNAVAILABLE", fallback: "Course dates are not currently available.") /// No course content is currently available. diff --git a/Course/Course/en.lproj/Localizable.strings b/Course/Course/en.lproj/Localizable.strings index 37ad21cb8..521f2f23e 100644 --- a/Course/Course/en.lproj/Localizable.strings +++ b/Course/Course/en.lproj/Localizable.strings @@ -34,6 +34,7 @@ "ERROR.ASSIGNMENTS_UNAVAILABLE" = "No assignments available for this course."; "ERROR.COURSE_DATE_UNAVAILABLE" = "Course dates are not currently available."; "ERROR.COURSEWARE_UNAVAILABLE" = "No course content is currently available."; +"ERROR.COURSE_ACCESS_UNAVAILABLE" = "You don't currently have access to this course."; "ALERT.ROTATE_DEVICE" = "Rotate your device to view this video in full screen."; "ALERT.DELETE_ALL_VIDEOS" = "Are you sure you want to delete all video(s) for"; From b018fae98c2455f5b7208b455d5d42396a1fb2f8 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:25:40 +0300 Subject: [PATCH 09/13] fix: refresh course download state at the caller's priority Fixes #576. CourseDownloadHelper refreshed its value from a Task(priority: .background), added in #562. A background task can wait seconds for a thread while the device or the CI runner is busy, so the download buttons lagged behind the download manager, and the tests waiting for that refresh went over the time limit on GitHub Actions. They were commented out in #575 and deleted in #652. On this PR's first CI run the restored tests showed it: awaiting the async refresh directly took 0.004 s, the sync refresh through the background task 7.9 s, and the refresh triggered by a download event did not start within 15 s. The refresh now runs at the priority of whoever asks for it. The tests are back in a form that does not block the main thread: every test is async and waits with fulfillment(of:) instead of wait(for:), so the helper's main-actor hop can always run, and the checks use the Mockolo call counters that replaced SwiftyMocky's Verify. --- .../Container/CourseDownloadHelper.swift | 4 +- .../Container/CourseDownloadHelperTests.swift | 95 +++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/Course/Course/Presentation/Container/CourseDownloadHelper.swift b/Course/Course/Presentation/Container/CourseDownloadHelper.swift index 065bf7856..51f844736 100644 --- a/Course/Course/Presentation/Container/CourseDownloadHelper.swift +++ b/Course/Course/Presentation/Container/CourseDownloadHelper.swift @@ -107,7 +107,9 @@ public final class CourseDownloadHelper: CourseDownloadHelperProtocol, @unchecke } public func refreshValue() { - Task(priority: .background) { + // Runs at the caller's priority: the result drives the download buttons on screen, + // and a background task can wait seconds for a thread while the device is busy. + Task { await refreshValue() } } diff --git a/Course/CourseTests/Presentation/Container/CourseDownloadHelperTests.swift b/Course/CourseTests/Presentation/Container/CourseDownloadHelperTests.swift index 7d2a04934..9ba711eeb 100644 --- a/Course/CourseTests/Presentation/Container/CourseDownloadHelperTests.swift +++ b/Course/CourseTests/Presentation/Container/CourseDownloadHelperTests.swift @@ -128,6 +128,101 @@ final class CourseDownloadHelperTests: XCTestCase { cancellables = [] } + func testPublisher_whenRefresh_ShouldSendValue() async { + // given + var valueReceived: CourseDownloadValue? + let published = expectation(description: "value published") + helper.publisher() + .sink { value in + valueReceived = value + published.fulfill() + } + .store(in: &cancellables) + + // when + let refreshInBackground: () -> Void = helper.refreshValue + refreshInBackground() + + // then + await fulfillment(of: [published], timeout: timeout) + XCTAssertEqual(downloadManagerMock.getDownloadTasksCallCount, 1) + XCTAssertEqual(downloadManagerMock.getCurrentDownloadTaskCallCount, 1) + XCTAssertEqual(valueReceived, value) + } + + func testPublisher_whenAsyncRefresh_ShouldSendValue() async { + // given + var valueReceived: CourseDownloadValue? + let published = expectation(description: "value published") + helper.publisher() + .sink { value in + valueReceived = value + published.fulfill() + } + .store(in: &cancellables) + + // when + await helper.refreshValue() + + // then + await fulfillment(of: [published], timeout: timeout) + XCTAssertEqual(downloadManagerMock.getDownloadTasksCallCount, 1) + XCTAssertEqual(downloadManagerMock.getCurrentDownloadTaskCallCount, 1) + XCTAssertEqual(valueReceived, value) + } + + func testPublisher_whenReceivedNotProgressEvent_ShouldSendValue() async { + // given + var valueReceived: CourseDownloadValue? + var receivedCount = 0 + let published = expectation(description: "value published after the event") + helper.publisher() + .sink { value in + receivedCount += 1 + valueReceived = value + published.fulfill() + } + .store(in: &cancellables) + + // when + downloadPublisher.send(.added) + + // then + await fulfillment(of: [published], timeout: timeout) + XCTAssertEqual(downloadManagerMock.getDownloadTasksCallCount, 1) + XCTAssertEqual(downloadManagerMock.getCurrentDownloadTaskCallCount, 1) + XCTAssertEqual(receivedCount, 1) + XCTAssertEqual(valueReceived, value) + } + + func testEventPublisher_whenReceivedProgressEvent_ShouldSendEvent() async { + // given + var valueReceived: DownloadDataTask? + var countOfEvents = 0 + task.progress = 0.5 + let progressed = expectation(description: "progress event published") + helper.progressPublisher() + .sink { value in + countOfEvents += 1 + valueReceived = value + progressed.fulfill() + } + .store(in: &cancellables) + helper.value = value + + // when + downloadPublisher.send(.progress(task)) + + // then + await fulfillment(of: [progressed], timeout: timeout) + value.currentDownloadTask = task + XCTAssertEqual(helper.value, value) + XCTAssertEqual(valueReceived, task) + XCTAssertEqual(countOfEvents, 1) + XCTAssertEqual(downloadManagerMock.getDownloadTasksCallCount, 0) + XCTAssertEqual(downloadManagerMock.getCurrentDownloadTaskCallCount, 0) + } + func testSizeForBlock_whenCalled_ShouldReturnSize() { // when let size = helper.sizeFor(block: block) From b00146990f8f564402d205acf498d0344858a50e Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 06:42:09 +0300 Subject: [PATCH 10/13] fix: keep saved courses reachable when the network is up but not working Fixes #581. On a network that connects but doesn't carry traffic (captive portal, in-flight Wi-Fi) the Learn screen showed a spinner until its request timed out after 60 s and then "No Courses" with "Something went wrong", even though the courses and their outlines were saved from the last visit. The connectivity check from #635 notices such a network after about 30 s and shows the offline banner, but the screens only looked at it when a request started. - Both dashboards now show the saved courses as soon as the app knows it is offline while a request is still hanging, and fall back to them when a request fails instead of emptying the list. A late successful response still replaces them. The update-required (426) path is unchanged. - The course screen opens the outline saved on the last visit when loading it fails on the network, instead of an empty course. Saved data is only used where the app already uses it in offline mode; nothing is shown ahead of a working network. --- .../Container/CourseContainerViewModel.swift | 11 +++- .../CourseContainerViewModelTests.swift | 21 ++++++- .../Presentation/ListDashboardViewModel.swift | 29 +++++++++- .../PrimaryCourseDashboardViewModel.swift | 32 +++++++++- .../DashboardViewModelTests.swift | 43 ++++++++++++++ ...PrimaryCourseDashboardViewModelTests.swift | 58 +++++++++++++++++++ 6 files changed, 189 insertions(+), 5 deletions(-) diff --git a/Course/Course/Presentation/Container/CourseContainerViewModel.swift b/Course/Course/Presentation/Container/CourseContainerViewModel.swift index e8c0c5673..e407e0a4b 100644 --- a/Course/Course/Presentation/Container/CourseContainerViewModel.swift +++ b/Course/Course/Presentation/Container/CourseContainerViewModel.swift @@ -220,7 +220,16 @@ extension CourseTab { @MainActor func getCourseStructure(courseID: String) async throws -> CourseStructure? { if isInternetAvaliable { - return try await interactor.getCourseBlocks(courseID: courseID) + do { + return try await interactor.getCourseBlocks(courseID: courseID) + } catch let error where error.isInternetError { + // A network that is up but not working fails only after the request times + // out; open the copy saved on the last visit rather than an empty course. + if let saved = try? await interactor.getLoadedCourseBlocks(courseID: courseID) { + return saved + } + throw error + } } else { return try await interactor.getLoadedCourseBlocks(courseID: courseID) } diff --git a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift index 542b339a1..245f3e400 100644 --- a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift +++ b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift @@ -333,6 +333,7 @@ final class CourseContainerViewModelTests: XCTestCase { ) interactor.getCourseBlocksHandler = { _ in throw noInternetError } + interactor.getLoadedCourseBlocksHandler = { _ in throw NoCachedDataError() } interactor.getCourseAssignmentBlocksHandler = { _ in courseStructure } let mockCourseProgress = CourseProgressDetails( @@ -617,12 +618,15 @@ final class CourseContainerViewModelTests: XCTestCase { XCTAssertEqual(analytics.courseOutlineHandoutsTabClickedCallCount, 1) } - private func makeViewModel(manager: DownloadManagerProtocol = DownloadManagerMock()) -> CourseContainerViewModel { + private func makeViewModel( + manager: DownloadManagerProtocol = DownloadManagerMock(), + interactor: CourseInteractorProtocolMock = CourseInteractorProtocolMock() + ) -> CourseContainerViewModel { let connectivity = ConnectivityProtocolMock() connectivity.isInternetAvaliable = true connectivity.internetReachableSubject = .init(.reachable) return CourseContainerViewModel( - interactor: CourseInteractorProtocolMock(), + interactor: interactor, authInteractor: AuthInteractorProtocolMock(), router: CourseRouterMock(), analytics: CourseAnalyticsMock(), @@ -731,4 +735,17 @@ final class CourseContainerViewModelTests: XCTestCase { XCTAssertEqual(viewModel.getSequentialAssignmentStatus(for: "homework"), .incomplete) } + + func testCourseStructure_whenTheNetworkFails_opensTheSavedCopy() async throws { + let interactor = CourseInteractorProtocolMock() + let saved = course(withAssignment: [block("problem", .problem, completion: 0)]) + interactor.getCourseBlocksHandler = { _ in throw AFError.sessionTaskFailed(error: URLError(.timedOut)) } + interactor.getLoadedCourseBlocksHandler = { _ in saved } + let viewModel = makeViewModel(interactor: interactor) + + let structure = try await viewModel.getCourseStructure(courseID: "course") + + XCTAssertEqual(structure?.id, saved.id) + XCTAssertEqual(interactor.getLoadedCourseBlocksCallCount, 1) + } } diff --git a/Dashboard/Dashboard/Presentation/ListDashboardViewModel.swift b/Dashboard/Dashboard/Presentation/ListDashboardViewModel.swift index 773859414..1b3668bd3 100644 --- a/Dashboard/Dashboard/Presentation/ListDashboardViewModel.swift +++ b/Dashboard/Dashboard/Presentation/ListDashboardViewModel.swift @@ -9,6 +9,7 @@ import Foundation import Core import SwiftUI import Combine +import OEXFoundation @MainActor @Observable @@ -34,6 +35,7 @@ public class ListDashboardViewModel { let storage: CoreStorage private var onCourseEnrolledCancellable: AnyCancellable? private var refreshEnrollmentsCancellable: AnyCancellable? + private var offlineCancellable: AnyCancellable? public init(interactor: DashboardInteractorProtocol, connectivity: ConnectivityProtocol, @@ -60,6 +62,14 @@ public class ListDashboardViewModel { await self.getMyCourses(page: 1, refresh: true) } } + offlineCancellable = connectivity.internetReachableSubject + .receive(on: DispatchQueue.main) + .sink { [weak self] state in + guard state == .notReachable else { return } + Task { @MainActor in + await self?.showSavedCoursesIfStillLoading() + } + } } @MainActor @@ -89,10 +99,27 @@ public class ListDashboardViewModel { if error is NoCachedDataError { errorMessage = CoreLocalization.Error.noCachedData } else { - errorMessage = CoreLocalization.Error.unknownError + // A failed refresh must not take the learner's courses away: fall back to the + // ones saved by the last successful load instead of an empty list. + if courses.isEmpty, let saved = try? await interactor.getEnrollmentsOffline() { + courses = saved + } + errorMessage = error.isInternetError + ? CoreLocalization.Error.slowOrNoInternetConnection + : CoreLocalization.Error.unknownError } } } + + /// On a network that is up but doesn't work (captive portal, in-flight Wi-Fi) the request + /// hangs until it times out. As soon as the app knows it is offline, show the saved + /// courses instead of the spinner; a late response still replaces them. + private func showSavedCoursesIfStillLoading() async { + guard fetchInProgress, courses.isEmpty, + let saved = try? await interactor.getEnrollmentsOffline(), !saved.isEmpty else { return } + courses = saved + fetchInProgress = false + } @MainActor public func getMyCoursesPagination(index: Int) async { diff --git a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift index a0f43855f..e8577f336 100644 --- a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift +++ b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift @@ -7,7 +7,9 @@ import Foundation import Core +import Combine import SwiftUI +import OEXFoundation @MainActor @Observable @@ -36,6 +38,7 @@ public class PrimaryCourseDashboardViewModel { var storage: CoreStorage let router: DashboardRouter @ObservationIgnored nonisolated(unsafe) private var observers: [NSObjectProtocol] = [] + @ObservationIgnored private var cancellables = Set() private let ipadPageSize = 7 private let iphonePageSize = 5 @@ -89,6 +92,16 @@ public class PrimaryCourseDashboardViewModel { } observers.append(contentsOf: [enrollmentObserver, completionObserver, refreshObserver]) + + connectivity.internetReachableSubject + .receive(on: DispatchQueue.main) + .sink { [weak self] state in + guard state == .notReachable else { return } + Task { @MainActor in + await self?.showSavedEnrollmentsIfStillLoading() + } + } + .store(in: &cancellables) } func setupNotifications() { @@ -148,10 +161,27 @@ public class PrimaryCourseDashboardViewModel { storage.updateAppRequired = true self.router.showUpdateRequiredView(showAccountLink: true) } else { - errorMessage = CoreLocalization.Error.unknownError + // A failed refresh must not take the learner's courses away: fall back to the + // ones saved by the last successful load instead of showing "No Courses". + if enrollments == nil { + enrollments = try? await interactor.getPrimaryEnrollmentOffline() + } + errorMessage = error.isInternetError + ? CoreLocalization.Error.slowOrNoInternetConnection + : CoreLocalization.Error.unknownError } } } + + /// On a network that is up but doesn't work (captive portal, in-flight Wi-Fi) the request + /// hangs until it times out. As soon as the app knows it is offline, show the saved + /// courses instead of the spinner; a late response still replaces them. + private func showSavedEnrollmentsIfStillLoading() async { + guard fetchInProgress, enrollments == nil, + let saved = try? await interactor.getPrimaryEnrollmentOffline() else { return } + enrollments = saved + fetchInProgress = false + } func trackDashboardCourseClicked(courseID: String, courseName: String) { analytics.dashboardCourseClicked(courseID: courseID, courseName: courseName) diff --git a/Dashboard/DashboardTests/Presentation/DashboardViewModelTests.swift b/Dashboard/DashboardTests/Presentation/DashboardViewModelTests.swift index c4c6cc5d0..8cb95ae29 100644 --- a/Dashboard/DashboardTests/Presentation/DashboardViewModelTests.swift +++ b/Dashboard/DashboardTests/Presentation/DashboardViewModelTests.swift @@ -17,6 +17,7 @@ final class ListDashboardViewModelTests: XCTestCase { func testGetMyCoursesSuccess() async throws { let interactor = DashboardInteractorProtocolMock() let connectivity = ConnectivityProtocolMock() + connectivity.internetReachableSubject = .init(.reachable) let analytics = DashboardAnalyticsMock() let viewModel = ListDashboardViewModel( interactor: interactor, @@ -72,6 +73,7 @@ final class ListDashboardViewModelTests: XCTestCase { func testGetMyCoursesOfflineSuccess() async throws { let interactor = DashboardInteractorProtocolMock() let connectivity = ConnectivityProtocolMock() + connectivity.internetReachableSubject = .init(.reachable) let analytics = DashboardAnalyticsMock() let viewModel = ListDashboardViewModel( interactor: interactor, @@ -127,6 +129,7 @@ final class ListDashboardViewModelTests: XCTestCase { func testGetMyCoursesNoCacheError() async throws { let interactor = DashboardInteractorProtocolMock() let connectivity = ConnectivityProtocolMock() + connectivity.internetReachableSubject = .init(.reachable) let analytics = DashboardAnalyticsMock() let viewModel = ListDashboardViewModel( interactor: interactor, @@ -149,6 +152,7 @@ final class ListDashboardViewModelTests: XCTestCase { func testGetMyCoursesUnknownError() async throws { let interactor = DashboardInteractorProtocolMock() let connectivity = ConnectivityProtocolMock() + connectivity.internetReachableSubject = .init(.reachable) let analytics = DashboardAnalyticsMock() let viewModel = ListDashboardViewModel( interactor: interactor, @@ -159,6 +163,7 @@ final class ListDashboardViewModelTests: XCTestCase { connectivity.isInternetAvaliable = true interactor.getEnrollmentsHandler = { _ in throw NSError(domain: "error", code: -1, userInfo: nil) } + interactor.getEnrollmentsOfflineHandler = { throw NoCachedDataError() } await viewModel.getMyCourses(page: 1) @@ -167,4 +172,42 @@ final class ListDashboardViewModelTests: XCTestCase { XCTAssertEqual(viewModel.errorMessage, CoreLocalization.Error.unknownError) XCTAssertTrue(viewModel.showError) } + + func testGetMyCourses_whenTheRequestFails_showsTheSavedCourses() async { + let interactor = DashboardInteractorProtocolMock() + let connectivity = ConnectivityProtocolMock() + connectivity.internetReachableSubject = .init(.reachable) + let viewModel = ListDashboardViewModel( + interactor: interactor, + connectivity: connectivity, + analytics: DashboardAnalyticsMock(), + storage: CoreStorageMock() + ) + let saved = [ + CourseItem(name: "Saved", + org: "org", + shortDescription: "", + imageURL: "", + hasAccess: true, + courseStart: Date(), + courseEnd: nil, + enrollmentStart: Date(), + enrollmentEnd: Date(), + courseID: "123", + numPages: 1, + coursesCount: 1, + courseRawImage: nil, + progressEarned: 0, + progressPossible: 0) + ] + connectivity.isInternetAvaliable = true + interactor.getEnrollmentsHandler = { _ in throw URLError(.timedOut) } + interactor.getEnrollmentsOfflineHandler = { saved } + + await viewModel.getMyCourses(page: 1) + + XCTAssertTrue(viewModel.courses == saved) + XCTAssertTrue(viewModel.showError) + XCTAssertFalse(viewModel.fetchInProgress) + } } diff --git a/Dashboard/DashboardTests/Presentation/PrimaryCourseDashboardViewModelTests.swift b/Dashboard/DashboardTests/Presentation/PrimaryCourseDashboardViewModelTests.swift index 87393fd26..feff29a7e 100644 --- a/Dashboard/DashboardTests/Presentation/PrimaryCourseDashboardViewModelTests.swift +++ b/Dashboard/DashboardTests/Presentation/PrimaryCourseDashboardViewModelTests.swift @@ -25,6 +25,7 @@ final class PrimaryCourseDashboardViewModelTests: XCTestCase { super.setUp() interactor = DashboardInteractorProtocolMock() connectivity = ConnectivityProtocolMock() + connectivity.internetReachableSubject = .init(.reachable) analytics = DashboardAnalyticsMock() storage = CoreStorageMock() config = ConfigMock() @@ -158,6 +159,7 @@ final class PrimaryCourseDashboardViewModelTests: XCTestCase { connectivity.isInternetAvaliable = true interactor.getPrimaryEnrollmentHandler = { _ in throw NSError(domain: "error", code: -1, userInfo: nil) } + interactor.getPrimaryEnrollmentOfflineHandler = { throw NoCachedDataError() } // When await viewModel.getEnrollments() @@ -170,6 +172,62 @@ final class PrimaryCourseDashboardViewModelTests: XCTestCase { XCTAssertFalse(viewModel.fetchInProgress) } + func testGetEnrollments_whenTheRequestFails_showsTheSavedCourses() async { + // Given + let viewModel = PrimaryCourseDashboardViewModel( + interactor: interactor, + connectivity: connectivity, + analytics: analytics, + config: config, + storage: storage, + router: router + ) + connectivity.isInternetAvaliable = true + interactor.getPrimaryEnrollmentHandler = { _ in throw URLError(.timedOut) } + interactor.getPrimaryEnrollmentOfflineHandler = { self.enrollment } + + // When + await viewModel.getEnrollments() + + // Then + XCTAssertEqual(viewModel.enrollments, enrollment) + XCTAssertTrue(viewModel.showError) + XCTAssertFalse(viewModel.fetchInProgress) + } + + func testGoingOfflineWhileLoading_showsTheSavedCoursesBeforeTheRequestTimesOut() async { + // Given + let viewModel = PrimaryCourseDashboardViewModel( + interactor: interactor, + connectivity: connectivity, + analytics: analytics, + config: config, + storage: storage, + router: router + ) + connectivity.isInternetAvaliable = true + interactor.getPrimaryEnrollmentHandler = { _ in + try await Task.sleep(for: .seconds(1)) + throw URLError(.timedOut) + } + interactor.getPrimaryEnrollmentOfflineHandler = { self.enrollment } + let loading = Task { await viewModel.getEnrollments() } + try? await Task.sleep(for: .milliseconds(100)) + + // When + connectivity.internetReachableSubject.send(.notReachable) + let deadline = Date().addingTimeInterval(0.8) + while viewModel.enrollments == nil && Date() < deadline { + try? await Task.sleep(for: .milliseconds(20)) + } + + // Then: the saved courses are on screen while the request is still hanging + XCTAssertEqual(viewModel.enrollments, enrollment) + XCTAssertFalse(viewModel.fetchInProgress) + await loading.value + XCTAssertEqual(viewModel.enrollments, enrollment) + } + func testTrackDashboardCourseClicked() { // Given let viewModel = PrimaryCourseDashboardViewModel( From 5465f0c58fe3623d185edd64de077ec19762cf47 Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 07:27:35 +0300 Subject: [PATCH 11/13] fix: show a section's completion mark as soon as it is completed Fixes #262. After the last block of a section was completed, the app reloaded the course outline and the server already reported the section complete, but the outline kept the old marks and progress bar until the course was closed and opened again. CourseStructure, CourseChapter and CourseSequential compared equal by id alone, and SwiftUI skips redrawing a view whose inputs compare equal, so the refreshed outline was never drawn. They now use memberwise equality, as CourseVertical already did, so new completion, due dates and assignment progress redraw the outline right away. --- Core/Core/Domain/Model/CourseBlockModel.swift | 21 +++++-------------- .../CourseContainerViewModelTests.swift | 10 +++++++++ 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/Core/Core/Domain/Model/CourseBlockModel.swift b/Core/Core/Domain/Model/CourseBlockModel.swift index 6bff06608..82e07ee46 100644 --- a/Core/Core/Domain/Model/CourseBlockModel.swift +++ b/Core/Core/Domain/Model/CourseBlockModel.swift @@ -7,11 +7,10 @@ import Foundation +// Equality is memberwise on purpose. SwiftUI skips redrawing a view whose inputs compare +// equal, so an id-only `==` kept the outline showing stale completion, due dates and +// progress until the course screen was opened again. public struct CourseStructure: Equatable, Sendable { - public static func == (lhs: CourseStructure, rhs: CourseStructure) -> Bool { - return lhs.id == rhs.id - } - public let id: String public let graded: Bool public let completion: Double @@ -101,7 +100,7 @@ public struct CourseImage: Decodable, Sendable, Equatable { } } -public struct CourseProgress: Sendable { +public struct CourseProgress: Sendable, Equatable { public let totalAssignmentsCount: Int? public let assignmentsCompleted: Int? @@ -112,11 +111,6 @@ public struct CourseProgress: Sendable { } public struct CourseChapter: Identifiable, Sendable, Equatable { - public static func == (lhs: CourseChapter, rhs: CourseChapter) -> Bool { - lhs.id == rhs.id && - lhs.blockId == rhs.blockId - } - public let blockId: String public let id: String public let displayName: String @@ -139,11 +133,6 @@ public struct CourseChapter: Identifiable, Sendable, Equatable { } public struct CourseSequential: Identifiable, Sendable, Equatable { - public static func == (lhs: CourseSequential, rhs: CourseSequential) -> Bool { - lhs.id == rhs.id && - lhs.blockId == rhs.blockId - } - public let blockId: String public let id: String public let displayName: String @@ -231,7 +220,7 @@ public struct SubtitleUrl: Equatable, Sendable { } } -public struct SequentialProgress: Sendable { +public struct SequentialProgress: Sendable, Equatable { public let assignmentType: String? public let numPointsEarned: Int? public let numPointsPossible: Int? diff --git a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift index 245f3e400..1f0077a07 100644 --- a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift +++ b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift @@ -703,6 +703,16 @@ final class CourseContainerViewModelTests: XCTestCase { ) } + func testOutlineInputs_withNewCompletion_doNotCompareEqual() { + // SwiftUI redraws the outline only when its inputs compare unequal. + let started = course(withAssignment: [block("problem", .problem, completion: 0)]) + let finished = course(withAssignment: [block("problem", .problem, completion: 1)]) + + XCTAssertNotEqual(started, finished) + XCTAssertNotEqual(started.childs[0], finished.childs[0]) + XCTAssertNotEqual(started.childs[0].childs[0], finished.childs[0].childs[0]) + } + func testSectionIndicatorWhileDownloading_opensDownloadsListInsteadOfCancelling() async { let downloadManager = DownloadManagerProtocolMock() let viewModel = makeViewModel(manager: downloadManager) From 9d327fbdefaf458cf1de1e0ad340d9b6e58fc88a Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 10:11:20 +0300 Subject: [PATCH 12/13] fix: show the calendar sync status on a course's Dates tab The card at the top of a course's Dates tab always said "Offline", also for a course synced to the calendar, while Settings > Dates & Calendar said "Synced". Its status was a @State declared as a local variable inside body. SwiftUI keeps state only for a view's stored properties, so the value the .task wrote was dropped and the card kept its initial .offline. The status is now a property of the view. --- .../Presentation/Dates/Elements/CourseDateListView.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Course/Course/Presentation/Dates/Elements/CourseDateListView.swift b/Course/Course/Presentation/Dates/Elements/CourseDateListView.swift index 0cdd5743a..5aed2374b 100644 --- a/Course/Course/Presentation/Dates/Elements/CourseDateListView.swift +++ b/Course/Course/Presentation/Dates/Elements/CourseDateListView.swift @@ -14,6 +14,7 @@ struct CourseDateListView: View { var viewModel: CourseDatesViewModel @State private var isExpanded = false + @State private var syncStatus: SyncStatus = .offline @Binding var coordinate: CGFloat @Binding var collapsed: Bool @Binding var viewHeight: CGFloat @@ -30,13 +31,10 @@ struct CourseDateListView: View { viewHeight: $viewHeight ) VStack(alignment: .leading, spacing: 0) { - - @State var status: SyncStatus = .offline - - CalendarSyncStatusView(status: status, router: viewModel.router) + CalendarSyncStatusView(status: syncStatus, router: viewModel.router) .padding(.bottom, 16) .task { - status = await viewModel.syncStatus() + syncStatus = await viewModel.syncStatus() } if !courseDates.hasEnded { From 053fa3b19108d46944cf75e2bf6e5068c67f780e Mon Sep 17 00:00:00 2001 From: IvanStepanok <128456094+IvanStepanok@users.noreply.github.com> Date: Wed, 7 Oct 2026 10:49:54 +0300 Subject: [PATCH 13/13] fix: report a video download the server refused instead of marking it downloaded Downloads did not check the HTTP status. When the server answered with an error, such as a 403 from a CDN for an expired or removed link, the error page was saved under the video's file name and the video showed as downloaded; playing it offline then failed. Downloads are now validated. A download that fails for a reason other than the connection is dropped from the queue, its file removed, and reported with the other failed downloads once the queue is done, the way a download that started offline already is. Before, it stayed in the queue and the next pass picked it up again straight away. The Alamofire session can be passed in, so a test can answer with an error. --- Core/Core/Network/DownloadManager.swift | 36 +++++++---- .../DownloadManagerTests.swift | 59 +++++++++++++++++++ 2 files changed, 84 insertions(+), 11 deletions(-) diff --git a/Core/Core/Network/DownloadManager.swift b/Core/Core/Network/DownloadManager.swift index 7a9a14409..39df8b7df 100644 --- a/Core/Core/Network/DownloadManager.swift +++ b/Core/Core/Network/DownloadManager.swift @@ -204,6 +204,7 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable { private let appStorage: CoreStorage private let connectivity: ConnectivityProtocol private let filePathProvider: InstanceFilePathProvider + private let session: Session private var downloadRequest: DownloadRequest? nonisolated private let currentDownloadEventPublisher: PassthroughSubject = .init() @@ -230,12 +231,14 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable { persistence: CorePersistenceProtocol, appStorage: CoreStorage, connectivity: ConnectivityProtocol, - filePathProvider: InstanceFilePathProvider + filePathProvider: InstanceFilePathProvider, + session: Session = AF ) { self.persistence = persistence self.appStorage = appStorage self.connectivity = connectivity self.filePathProvider = filePathProvider + self.session = session if let userId = appStorage.user?.id { persistence.set(userId: userId) } @@ -606,9 +609,9 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable { } if let resumeData = download.resumeData { - downloadRequest = AF.download(resumingWith: resumeData, to: destination) + downloadRequest = session.download(resumingWith: resumeData, to: destination).validate() } else { - downloadRequest = AF.download(url, to: destination) + downloadRequest = session.download(url, to: destination).validate() } downloadRequest?.downloadProgress { @Sendable [weak self] prog in @@ -636,6 +639,21 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable { currentDownloadEventPublisher.send(.started(download)) } + /// Drops a download the server refused or that broke for a reason other than the + /// connection, so `newDownload()` moves on instead of picking it up again, and leaves + /// it to be reported with the others once the queue is done. `file` is whatever the + /// server sent instead, such as an error page, which must not stay on disk. + private func failDownload(_ download: DownloadDataTask, file: URL?) { + if let file { + try? FileManager.default.removeItem(at: file) + } + failedDownloads.append(download) + try? cancelDownloading(task: download) + Task { + try? await self.newDownload() + } + } + private func setCurrentTask(with task: DownloadDataTask?) async { currentDownloadTask = task } @@ -646,10 +664,7 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable { if error.isInternetError { state = .waiting } else if error.asAFError?.isExplicitlyCancelledError == false { - self.failedDownloads.append(download) - Task { - try? await self.newDownload() - } + failDownload(download, file: response.fileURL) return } } @@ -697,9 +712,9 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable { currentDownloadTask = download currentDownloadTask?.state = .inProgress if let resumeData = download.resumeData { - downloadRequest = AF.download(resumingWith: resumeData, to: destination) + downloadRequest = session.download(resumingWith: resumeData, to: destination).validate() } else { - downloadRequest = AF.download(url, to: destination) + downloadRequest = session.download(url, to: destination).validate() } downloadRequest?.downloadProgress { [weak self] prog in guard let self else { return } @@ -729,8 +744,7 @@ public actor DownloadManager: DownloadManagerProtocol, @unchecked Sendable { private func completeHTMLDownload(with response: AFDownloadResponse, for download: DownloadDataTask) async { if let error = response.error { if error.asAFError?.isExplicitlyCancelledError == false { - failedDownloads.append(download) - try? await self.newDownload() + failDownload(download, file: response.fileURL) return } } diff --git a/Core/CoreTests/DownloadManager/DownloadManagerTests.swift b/Core/CoreTests/DownloadManager/DownloadManagerTests.swift index 3b8bc4252..9c5ed6d2c 100644 --- a/Core/CoreTests/DownloadManager/DownloadManagerTests.swift +++ b/Core/CoreTests/DownloadManager/DownloadManagerTests.swift @@ -7,6 +7,7 @@ import XCTest import Combine +import Alamofire @testable import Core @MainActor @@ -15,6 +16,7 @@ final class DownloadManagerTests: XCTestCase { var persistence: CorePersistenceProtocolMock! var storage: CoreStorageMock! var connectivity: ConnectivityProtocolMock! + var cancellables = Set() override func setUp() { super.setUp() @@ -120,6 +122,42 @@ final class DownloadManagerTests: XCTestCase { XCTAssertEqual(currentDownloadTask?.id, mockTask.id) } + func testNewDownload_WhenServerRefusesTheFile_ReportsItAsFailedNotDownloaded() async throws { + // Given + let task = createMockDownloadTask() + persistence.getDownloadDataTasksHandler = { [task] } + connectivity.isMobileData = false + let configuration = URLSessionConfiguration.af.default + configuration.protocolClasses = [ForbiddenURLProtocol.self] + + let downloadManager = DownloadManager( + persistence: persistence, + appStorage: storage, + connectivity: connectivity, + filePathProvider: InstanceFilePathProvider(instanceStore: InstanceProviderMock()), + session: Session(configuration: configuration) + ) + var finishedCount = 0 + await downloadManager.eventPublisher() + .sink { event in + if case .finished = event { + finishedCount += 1 + } + } + .store(in: &cancellables) + let reported = expectation(forNotification: .showDownloadFailed, object: nil) { notification in + (notification.object as? [DownloadDataTask])?.map(\.id) == [task.id] + } + + // When + try await downloadManager.resumeDownloading() + + // Then + await fulfillment(of: [reported], timeout: 10) + XCTAssertEqual(finishedCount, 0) + XCTAssertEqual(persistence.deleteDownloadDataTasksCallCount, 1) + } + // MARK: - Test Cancel Downloads func testCancelDownloading_ForSpecificTask_ShouldRemoveFileAndTask() async throws { @@ -366,3 +404,24 @@ final class DownloadManagerTests: XCTestCase { ) } } + +/// Answers every request the way a CDN answers a link it refuses: 403 with an XML body. +private final class ForbiddenURLProtocol: URLProtocol { + override class func canInit(with request: URLRequest) -> Bool { true } + override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + + override func startLoading() { + guard let url = request.url, + let response = HTTPURLResponse( + url: url, + statusCode: 403, + httpVersion: "HTTP/1.1", + headerFields: ["Content-Type": "application/xml"] + ) else { return } + client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) + client?.urlProtocol(self, didLoad: Data("AccessDenied".utf8)) + client?.urlProtocolDidFinishLoading(self) + } + + override func stopLoading() {} +}