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/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/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/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) 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() {} +} diff --git a/Course/Course/Presentation/Container/CourseContainerView.swift b/Course/Course/Presentation/Container/CourseContainerView.swift index 84a83298a..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) @@ -95,6 +97,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 @@ -112,6 +117,8 @@ public struct CourseContainerView: View { viewHeight: $viewHeight, dateTabIndex: CourseTab.dates.rawValue ) + } else if viewModel.isActive == false { + noAccessContent } else { ZStack(alignment: .top) { tabs @@ -161,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/Presentation/Container/CourseContainerViewModel.swift b/Course/Course/Presentation/Container/CourseContainerViewModel.swift index 1c5b40012..e407e0a4b 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] = [] @@ -219,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) } @@ -312,7 +322,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 @@ -379,8 +392,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 } @@ -1442,7 +1465,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 } @@ -1457,6 +1480,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) ?? "" @@ -1775,7 +1809,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/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/Course/Presentation/Dates/CourseDatesViewModel.swift b/Course/Course/Presentation/Dates/CourseDatesViewModel.swift index c2473700c..d00715040 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 @@ -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,26 +82,24 @@ 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 } @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 +115,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,19 +173,21 @@ 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) } } @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..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 { @@ -49,7 +47,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/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/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"; diff --git a/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift b/Course/CourseTests/Presentation/Container/CourseContainerViewModelTests.swift index 6381232fa..1f0077a07 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( @@ -616,4 +617,145 @@ final class CourseContainerViewModelTests: XCTestCase { viewModel.trackSelectedTab(selection: .handounds, courseId: "1", courseName: "name") XCTAssertEqual(analytics.courseOutlineHandoutsTabClickedCallCount, 1) } + + private func makeViewModel( + manager: DownloadManagerProtocol = DownloadManagerMock(), + interactor: CourseInteractorProtocolMock = CourseInteractorProtocolMock() + ) -> CourseContainerViewModel { + let connectivity = ConnectivityProtocolMock() + connectivity.isInternetAvaliable = true + connectivity.internetReachableSubject = .init(.reachable) + return CourseContainerViewModel( + interactor: interactor, + authInteractor: AuthInteractorProtocolMock(), + router: CourseRouterMock(), + analytics: CourseAnalyticsMock(), + config: ConfigMock(), + connectivity: connectivity, + manager: manager, + 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 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) + 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: [ + 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) + } + + 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/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) diff --git a/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift b/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift index cd7ad9d69..ca46d2fda 100644 --- a/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift +++ b/Course/CourseTests/Presentation/Unit/CourseDateViewModelTests.swift @@ -529,4 +529,125 @@ 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'") } + + // 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) + } + + // 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")! + + 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) + } } 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( 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") + } }