From 3d5d7bb97d4373b129b9d7f0e2af055ef02bec23 Mon Sep 17 00:00:00 2001 From: Anton Yarmolenko Date: Mon, 29 Jul 2024 10:44:27 +0200 Subject: [PATCH 1/5] fix: video tab with youtube (#52) * fix: don't show error if only youtube videos * fix: call UI updates from main queue * fix: don't send completion at youtube video start --------- Co-authored-by: Anton Yarmolenko <37253+rnr@users.noreply.github.com> --- .../Presentation/Container/CourseContainerViewModel.swift | 1 + Course/Course/Presentation/Outline/CourseOutlineView.swift | 2 +- .../Video/YoutubePlayerViewControllerHolder.swift | 4 +++- .../Presentation/PrimaryCourseDashboardViewModel.swift | 5 ++++- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Course/Course/Presentation/Container/CourseContainerViewModel.swift b/Course/Course/Presentation/Container/CourseContainerViewModel.swift index 54d7dca4..f7ee8452 100644 --- a/Course/Course/Presentation/Container/CourseContainerViewModel.swift +++ b/Course/Course/Presentation/Container/CourseContainerViewModel.swift @@ -154,6 +154,7 @@ public final class CourseContainerViewModel: BaseCourseViewModel { addObservers() } + @MainActor func updateCourseIfNeeded(courseID: String) async { if updateCourseProgress { await getCourseBlocks(courseID: courseID, withProgress: false) diff --git a/Course/Course/Presentation/Outline/CourseOutlineView.swift b/Course/Course/Presentation/Outline/CourseOutlineView.swift index 3d04e8d0..5061fc24 100644 --- a/Course/Course/Presentation/Outline/CourseOutlineView.swift +++ b/Course/Course/Presentation/Outline/CourseOutlineView.swift @@ -261,7 +261,7 @@ public struct CourseOutlineView: View { } } } - } else { + } else if viewModel.courseVideosStructure == nil { FullScreenErrorView( type: .noContent( CourseLocalization.Error.videosUnavailable, diff --git a/Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift b/Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift index f1273708..44ba088f 100644 --- a/Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift +++ b/Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift @@ -78,7 +78,9 @@ public final class YoutubePlayerViewControllerHolder: PlayerViewControllerHolder timePublisher .sink {[weak self] _ in guard let self else { return } - if self.playerTracker.progress > 0.8 && !self.isViewedOnce { + if self.playerTracker.progress != .infinity + && self.playerTracker.progress > 0.8 + && !self.isViewedOnce { self.isViewedOnce = true Task { await self.sendCompletion() diff --git a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift index 5159bc8c..fae9a0fe 100644 --- a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift +++ b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift @@ -66,7 +66,9 @@ public class PrimaryCourseDashboardViewModel: ObservableObject { completionPublisher .sink { [weak self] _ in guard let self = self else { return } - updateEnrollmentsIfNeeded() + DispatchQueue.main.async { + self.updateEnrollmentsIfNeeded() + } } .store(in: &cancellables) @@ -80,6 +82,7 @@ public class PrimaryCourseDashboardViewModel: ObservableObject { .store(in: &cancellables) } + @MainActor private func updateEnrollmentsIfNeeded() { guard updateNeeded else { return } Task { From a806e0407f268141e4d59a6fb7c90f92104cbc1e Mon Sep 17 00:00:00 2001 From: Anton Yarmolenko <37253+rnr@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:08:27 +0100 Subject: [PATCH 2/5] chore: delete unneeded MainActor instructions --- .../Course/Presentation/Container/CourseContainerViewModel.swift | 1 - .../Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift | 1 - 2 files changed, 2 deletions(-) diff --git a/Course/Course/Presentation/Container/CourseContainerViewModel.swift b/Course/Course/Presentation/Container/CourseContainerViewModel.swift index f7ee8452..54d7dca4 100644 --- a/Course/Course/Presentation/Container/CourseContainerViewModel.swift +++ b/Course/Course/Presentation/Container/CourseContainerViewModel.swift @@ -154,7 +154,6 @@ public final class CourseContainerViewModel: BaseCourseViewModel { addObservers() } - @MainActor func updateCourseIfNeeded(courseID: String) async { if updateCourseProgress { await getCourseBlocks(courseID: courseID, withProgress: false) diff --git a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift index fae9a0fe..0fd05294 100644 --- a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift +++ b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift @@ -82,7 +82,6 @@ public class PrimaryCourseDashboardViewModel: ObservableObject { .store(in: &cancellables) } - @MainActor private func updateEnrollmentsIfNeeded() { guard updateNeeded else { return } Task { From 108044ad50dc64510b619e826a761c3da0dc483e Mon Sep 17 00:00:00 2001 From: Anton Yarmolenko Date: Thu, 22 Aug 2024 11:16:16 +0200 Subject: [PATCH 3/5] feat: save playback speed (#67) * chore: save playback speed in CourseStorage * chore: add full screen for youtube on iPad * chore: save value in userdefaults * chore: little refactor * fix: fix warning * chore: added CourseStorage as injection * chore: moved playback speed to user settings --------- Co-authored-by: Anton Yarmolenko <37253+rnr@users.noreply.github.com> --- Core/Core/Data/Model/UserSettings.swift | 5 ++- .../Video/PlayerViewControllerHolder.swift | 41 +++++++++++++++---- .../YoutubePlayerViewControllerHolder.swift | 12 +++--- .../Unit/VideoPlayerViewModelTests.swift | 14 +++---- OpenEdX/DI/ScreenAssembly.swift | 12 +++--- OpenEdX/Data/AppStorage.swift | 13 ++++-- Profile/Profile/Data/ProfileRepository.swift | 4 +- .../Settings/SettingsViewModelTests.swift | 18 +++++--- 8 files changed, 79 insertions(+), 40 deletions(-) diff --git a/Core/Core/Data/Model/UserSettings.swift b/Core/Core/Data/Model/UserSettings.swift index ec6decdf..fdd2cf27 100644 --- a/Core/Core/Data/Model/UserSettings.swift +++ b/Core/Core/Data/Model/UserSettings.swift @@ -11,15 +11,18 @@ public struct UserSettings: Codable, Hashable, Sendable { public var wifiOnly: Bool public var streamingQuality: StreamingQuality public var downloadQuality: DownloadQuality + public var videoPlaybackSpeed: Float public init( wifiOnly: Bool, streamingQuality: StreamingQuality, - downloadQuality: DownloadQuality + downloadQuality: DownloadQuality, + playbackSpeed: Float ) { self.wifiOnly = wifiOnly self.streamingQuality = streamingQuality self.downloadQuality = downloadQuality + self.videoPlaybackSpeed = playbackSpeed } } diff --git a/Course/Course/Presentation/Video/PlayerViewControllerHolder.swift b/Course/Course/Presentation/Video/PlayerViewControllerHolder.swift index 787423c4..e6d99428 100644 --- a/Course/Course/Presentation/Video/PlayerViewControllerHolder.swift +++ b/Course/Course/Presentation/Video/PlayerViewControllerHolder.swift @@ -7,6 +7,7 @@ @preconcurrency import AVKit @preconcurrency import Combine +import Core @MainActor public protocol PlayerViewControllerHolderProtocol: AnyObject, Sendable { @@ -24,11 +25,11 @@ public protocol PlayerViewControllerHolderProtocol: AnyObject, Sendable { blockID: String, courseID: String, selectedCourseTab: Int, - videoResolution: CGSize, pipManager: PipManagerProtocol, playerTracker: any PlayerTrackerProtocol, playerDelegate: PlayerDelegateProtocol?, - playerService: PlayerServiceProtocol + playerService: PlayerServiceProtocol, + appStorage: CoreStorage? ) func getTimePublisher() -> AnyPublisher func getErrorPublisher() -> AnyPublisher @@ -71,10 +72,10 @@ public final class PlayerViewControllerHolder: PlayerViewControllerHolderProtoco private let playerTracker: any PlayerTrackerProtocol private let playerDelegate: PlayerDelegateProtocol? private let playerService: PlayerServiceProtocol - private let videoResolution: CGSize private let errorPublisher = PassthroughSubject() private var isViewedOnce: Bool = false private var cancellations: [AnyCancellable] = [] + private var appStorage: CoreStorage? let pipManager: PipManagerProtocol @@ -85,7 +86,21 @@ public final class PlayerViewControllerHolder: PlayerViewControllerHolderProtoco playerController.canStartPictureInPictureAutomaticallyFromInline = true playerController.delegate = playerDelegate playerController.player = playerTracker.player as? AVPlayer - playerController.player?.currentItem?.preferredMaximumResolution = videoResolution + playerController.player?.currentItem?.preferredMaximumResolution = ( + appStorage?.userSettings?.streamingQuality ?? .auto + ).resolution + + if let speed = appStorage?.userSettings?.videoPlaybackSpeed { + if #available(iOS 16.0, *) { + if let playbackSpeed = playerController.speeds.first(where: { $0.rate == speed }) { + playerController.selectSpeed(playbackSpeed) + } + } else { + // Fallback on earlier versions + playerController.player?.rate = speed + } + } + return playerController }() @@ -94,21 +109,21 @@ public final class PlayerViewControllerHolder: PlayerViewControllerHolderProtoco blockID: String, courseID: String, selectedCourseTab: Int, - videoResolution: CGSize, pipManager: PipManagerProtocol, playerTracker: any PlayerTrackerProtocol, playerDelegate: PlayerDelegateProtocol?, - playerService: PlayerServiceProtocol + playerService: PlayerServiceProtocol, + appStorage: CoreStorage? ) { self.url = url self.blockID = blockID self.courseID = courseID self.selectedCourseTab = selectedCourseTab - self.videoResolution = videoResolution self.pipManager = pipManager self.playerTracker = playerTracker self.playerDelegate = playerDelegate self.playerService = playerService + self.appStorage = appStorage addObservers() } @@ -139,6 +154,7 @@ public final class PlayerViewControllerHolder: PlayerViewControllerHolderProtoco guard let self else { return } MainActor.assumeIsolated { self.pausePipIfNeed() + self.saveSelectedRate(rate: rate) } } .store(in: &cancellations) @@ -153,6 +169,13 @@ public final class PlayerViewControllerHolder: PlayerViewControllerHolderProtoco .store(in: &cancellations) } + private func saveSelectedRate(rate: Float) { + if var storage = appStorage, var userSettings = storage.userSettings, userSettings.videoPlaybackSpeed != rate { + userSettings.videoPlaybackSpeed = rate + storage.userSettings = userSettings + } + } + public func pausePipIfNeed() { if !isPlayingInPip { pipManager.pauseCurrentPipVideo() @@ -218,7 +241,6 @@ extension PlayerViewControllerHolder { blockID: "", courseID: "", selectedCourseTab: 0, - videoResolution: .zero, pipManager: PipManagerProtocolMock(), playerTracker: PlayerTrackerProtocolMock(url: URL(string: "")), playerDelegate: nil, @@ -227,7 +249,8 @@ extension PlayerViewControllerHolder { blockID: "", interactor: CourseInteractor.mock, router: CourseRouterMock() - ) + ), + appStorage: CoreStorageMock() ) } } diff --git a/Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift b/Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift index 44ba088f..52e65540 100644 --- a/Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift +++ b/Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift @@ -8,6 +8,7 @@ @preconcurrency import Combine import Foundation @preconcurrency import YouTubePlayerKit +import Core @MainActor public final class YoutubePlayerViewControllerHolder: PlayerViewControllerHolderProtocol { @@ -34,7 +35,6 @@ public final class YoutubePlayerViewControllerHolder: PlayerViewControllerHolder } private let playerTracker: any PlayerTrackerProtocol private let playerService: PlayerServiceProtocol - private let videoResolution: CGSize private let errorPublisher = PassthroughSubject() private var isViewedOnce: Bool = false private var cancellations: [AnyCancellable] = [] @@ -50,23 +50,23 @@ public final class YoutubePlayerViewControllerHolder: PlayerViewControllerHolder blockID: String, courseID: String, selectedCourseTab: Int, - videoResolution: CGSize, pipManager: PipManagerProtocol, playerTracker: any PlayerTrackerProtocol, playerDelegate: PlayerDelegateProtocol?, - playerService: PlayerServiceProtocol + playerService: PlayerServiceProtocol, + appStorage: CoreStorage? ) { self.url = url self.blockID = blockID self.courseID = courseID self.selectedCourseTab = selectedCourseTab - self.videoResolution = videoResolution self.pipManager = pipManager self.playerTracker = playerTracker self.playerService = playerService let youtubePlayer = playerTracker.player as? YouTubePlayer var configuration = youtubePlayer?.configuration configuration?.autoPlay = !pipManager.isPipActive + configuration?.fullscreenMode = .web if let configuration = configuration { youtubePlayer?.update(configuration: configuration) } @@ -184,7 +184,6 @@ extension YoutubePlayerViewControllerHolder { blockID: "", courseID: "", selectedCourseTab: 0, - videoResolution: .zero, pipManager: PipManagerProtocolMock(), playerTracker: PlayerTrackerProtocolMock(url: URL(string: "")), playerDelegate: nil, @@ -193,7 +192,8 @@ extension YoutubePlayerViewControllerHolder { blockID: "", interactor: CourseInteractor.mock, router: CourseRouterMock() - ) + ), + appStorage: nil ) } } diff --git a/Course/CourseTests/Presentation/Unit/VideoPlayerViewModelTests.swift b/Course/CourseTests/Presentation/Unit/VideoPlayerViewModelTests.swift index 9c787122..a4b1f803 100644 --- a/Course/CourseTests/Presentation/Unit/VideoPlayerViewModelTests.swift +++ b/Course/CourseTests/Presentation/Unit/VideoPlayerViewModelTests.swift @@ -36,7 +36,7 @@ final class VideoPlayerViewModelTests: XCTestCase { let tracker = PlayerTrackerProtocolMock(url: nil) let service = PlayerService(courseID: "", blockID: "", interactor: interactor, router: router) - let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, videoResolution: .zero, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service) + let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service, appStorage: CoreStorageMock()) let viewModel = VideoPlayerViewModel(languages: [], connectivity: connectivity, playerHolder: playerHolder) await viewModel.getSubtitles(subtitlesUrl: "url") @@ -60,7 +60,7 @@ final class VideoPlayerViewModelTests: XCTestCase { let tracker = PlayerTrackerProtocolMock(url: nil) let service = PlayerService(courseID: "", blockID: "", interactor: interactor, router: router) - let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, videoResolution: .zero, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service) + let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service, appStorage: CoreStorageMock()) let viewModel = VideoPlayerViewModel(languages: [], connectivity: connectivity, playerHolder: playerHolder) await viewModel.getSubtitles(subtitlesUrl: "url") @@ -79,7 +79,7 @@ final class VideoPlayerViewModelTests: XCTestCase { let tracker = PlayerTrackerProtocolMock(url: nil) let service = PlayerService(courseID: "", blockID: "", interactor: interactor, router: router) - let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, videoResolution: .zero, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service) + let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service, appStorage: CoreStorageMock()) let viewModel = VideoPlayerViewModel(languages: [], connectivity: connectivity, playerHolder: playerHolder) viewModel.languages = [ @@ -108,11 +108,11 @@ final class VideoPlayerViewModelTests: XCTestCase { blockID: "", courseID: "", selectedCourseTab: 0, - videoResolution: .zero, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, - playerService: service + playerService: service, + appStorage: CoreStorageMock() ) Given(interactor, .blockCompletionRequest(courseID: .any, blockID: .any, willProduce: {_ in})) @@ -131,7 +131,7 @@ final class VideoPlayerViewModelTests: XCTestCase { let tracker = PlayerTrackerProtocolMock(url: nil) let service = PlayerService(courseID: "", blockID: "", interactor: interactor, router: router) - let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, videoResolution: .zero, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service) + let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service, appStorage: CoreStorageMock()) let viewModel = VideoPlayerViewModel(languages: [], connectivity: connectivity, playerHolder: playerHolder) Given(interactor, .blockCompletionRequest(courseID: .any, blockID: .any, willThrow: NSError())) @@ -154,7 +154,7 @@ final class VideoPlayerViewModelTests: XCTestCase { let tracker = PlayerTrackerProtocolMock(url: nil) let service = PlayerService(courseID: "", blockID: "", interactor: interactor, router: router) - let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, videoResolution: .zero, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service) + let playerHolder = PlayerViewControllerHolder(url: nil, blockID: "", courseID: "", selectedCourseTab: 0, pipManager: PipManagerProtocolMock(), playerTracker: tracker, playerDelegate: nil, playerService: service, appStorage: CoreStorageMock()) let viewModel = VideoPlayerViewModel(languages: [], connectivity: connectivity, playerHolder: playerHolder) Given(interactor, .blockCompletionRequest(courseID: .any, blockID: .any, willThrow: noInternetError)) diff --git a/OpenEdX/DI/ScreenAssembly.swift b/OpenEdX/DI/ScreenAssembly.swift index e46faaa9..d08c923d 100644 --- a/OpenEdX/DI/ScreenAssembly.swift +++ b/OpenEdX/DI/ScreenAssembly.swift @@ -459,11 +459,11 @@ class ScreenAssembly: Assembly { blockID: blockID, courseID: courseID, selectedCourseTab: selectedCourseTab, - videoResolution: .zero, pipManager: r.resolve(PipManagerProtocol.self)!, playerTracker: r.resolve(YoutubePlayerTracker.self, argument: url)!, playerDelegate: nil, - playerService: r.resolve(PlayerServiceProtocol.self, arguments: courseID, blockID)! + playerService: r.resolve(PlayerServiceProtocol.self, arguments: courseID, blockID)!, + appStorage: nil ) } @@ -481,7 +481,6 @@ class ScreenAssembly: Assembly { } let storage = r.resolve(CoreStorage.self)! - let quality = storage.userSettings?.streamingQuality ?? .auto let tracker = r.resolve(PlayerTracker.self, argument: url)! let delegate = r.resolve(PlayerDelegateProtocol.self)! let holder = PlayerViewControllerHolder( @@ -489,15 +488,16 @@ class ScreenAssembly: Assembly { blockID: blockID, courseID: courseID, selectedCourseTab: selectedCourseTab, - videoResolution: quality.resolution, pipManager: pipManager, playerTracker: tracker, playerDelegate: delegate, - playerService: r.resolve(PlayerServiceProtocol.self, arguments: courseID, blockID)! + playerService: r.resolve(PlayerServiceProtocol.self, arguments: courseID, blockID)!, + appStorage: storage ) delegate.playerHolder = holder return holder - }) + } + ) container.register(PlayerServiceProtocol.self) { @MainActor r, courseID, blockID in let interactor = r.resolve(CourseInteractorProtocol.self)! diff --git a/OpenEdX/Data/AppStorage.swift b/OpenEdX/Data/AppStorage.swift index e5076de1..ec2bbab5 100644 --- a/OpenEdX/Data/AppStorage.swift +++ b/OpenEdX/Data/AppStorage.swift @@ -170,15 +170,22 @@ public final class AppStorage: CoreStorage, ProfileStorage, WhatsNewStorage, Cou public var userSettings: UserSettings? { get { - guard let userSettings = userDefaults.data(forKey: KEY_SETTINGS) else { - let defaultSettings = UserSettings(wifiOnly: true, streamingQuality: .auto, downloadQuality: .auto) + if let userSettings = userDefaults.data(forKey: KEY_SETTINGS), + let settings = try? JSONDecoder().decode(UserSettings.self, from: userSettings) { + return settings + } else { + let defaultSettings = UserSettings( + wifiOnly: true, + streamingQuality: .auto, + downloadQuality: .auto, + playbackSpeed: 1.0 + ) let encoder = JSONEncoder() if let encoded = try? encoder.encode(defaultSettings) { userDefaults.set(encoded, forKey: KEY_SETTINGS) } return defaultSettings } - return try? JSONDecoder().decode(UserSettings.self, from: userSettings) } set(newValue) { if let settings = newValue { diff --git a/Profile/Profile/Data/ProfileRepository.swift b/Profile/Profile/Data/ProfileRepository.swift index a51859f6..f5c3bf04 100644 --- a/Profile/Profile/Data/ProfileRepository.swift +++ b/Profile/Profile/Data/ProfileRepository.swift @@ -146,7 +146,7 @@ public actor ProfileRepository: ProfileRepositoryProtocol { if let userSettings = storage.userSettings { return userSettings } else { - return UserSettings(wifiOnly: true, streamingQuality: .auto, downloadQuality: .auto) + return UserSettings(wifiOnly: true, streamingQuality: .auto, downloadQuality: .auto, playbackSpeed: 1.0) } } @@ -254,7 +254,7 @@ actor ProfileRepositoryMock: ProfileRepositoryProtocol { public func deleteAccount(password: String) async throws -> Bool { return false } nonisolated public func getSettings() -> UserSettings { - return UserSettings(wifiOnly: true, streamingQuality: .auto, downloadQuality: .auto) + return UserSettings(wifiOnly: true, streamingQuality: .auto, downloadQuality: .auto, playbackSpeed: 1.0) } public func saveSettings(_ settings: UserSettings) async {} diff --git a/Profile/ProfileTests/Presentation/Settings/SettingsViewModelTests.swift b/Profile/ProfileTests/Presentation/Settings/SettingsViewModelTests.swift index 23264dcc..fa985688 100644 --- a/Profile/ProfileTests/Presentation/Settings/SettingsViewModelTests.swift +++ b/Profile/ProfileTests/Presentation/Settings/SettingsViewModelTests.swift @@ -27,7 +27,8 @@ final class SettingsViewModelTests: XCTestCase { willReturn: UserSettings( wifiOnly: true, streamingQuality: .auto, - downloadQuality: .auto + downloadQuality: .auto, + playbackSpeed: 1.0 ) ) ) @@ -61,7 +62,8 @@ final class SettingsViewModelTests: XCTestCase { willReturn: UserSettings( wifiOnly: true, streamingQuality: .auto, - downloadQuality: .auto + downloadQuality: .auto, + playbackSpeed: 1.0 ) ) ) @@ -94,7 +96,8 @@ final class SettingsViewModelTests: XCTestCase { willReturn: UserSettings( wifiOnly: true, streamingQuality: .auto, - downloadQuality: .auto + downloadQuality: .auto, + playbackSpeed: 1.0 ) ) ) @@ -127,7 +130,8 @@ final class SettingsViewModelTests: XCTestCase { willReturn: UserSettings( wifiOnly: true, streamingQuality: .auto, - downloadQuality: .auto + downloadQuality: .auto, + playbackSpeed: 1.0 ) ) ) @@ -160,7 +164,8 @@ final class SettingsViewModelTests: XCTestCase { willReturn: UserSettings( wifiOnly: true, streamingQuality: .auto, - downloadQuality: .auto + downloadQuality: .auto, + playbackSpeed: 1.0 ) ) ) @@ -193,7 +198,8 @@ final class SettingsViewModelTests: XCTestCase { willReturn: UserSettings( wifiOnly: true, streamingQuality: .auto, - downloadQuality: .auto + downloadQuality: .auto, + playbackSpeed: 1.0 ) ) ) From 721bc86864cea2abdcc295618553c5ee676d0ecd Mon Sep 17 00:00:00 2001 From: Saeed Bashir Date: Fri, 13 Sep 2024 10:01:55 +0500 Subject: [PATCH 4/5] chore: analytics screen events improvements and name fixes (#76) --- Core/Core/Analytics/CoreAnalytics.swift | 10 ++++-- .../Container/CourseContainerView.swift | 1 + .../Presentation/Handouts/HandoutsView.swift | 2 +- .../Presentation/DashboardAnalytics.swift | 4 +++ .../Presentation/Elements/DropDownMenu.swift | 7 +++++ .../PrimaryCourseDashboardView.swift | 6 +--- .../PrimaryCourseDashboardViewModel.swift | 2 +- .../DashboardMock.generated.swift | 31 +++++++++++++++++++ .../DiscoveryWebviewViewModel.swift | 1 + .../AnalyticsManager/AnalyticsManager.swift | 12 +++++-- .../MainScreenAnalytics.swift | 12 ++++--- OpenEdX/Router.swift | 4 +++ OpenEdX/View/MainScreenView.swift | 4 ++- OpenEdX/View/MainScreenViewModel.swift | 11 +++++-- 14 files changed, 88 insertions(+), 19 deletions(-) diff --git a/Core/Core/Analytics/CoreAnalytics.swift b/Core/Core/Analytics/CoreAnalytics.swift index 006b41c7..b74ec00c 100644 --- a/Core/Core/Analytics/CoreAnalytics.swift +++ b/Core/Core/Analytics/CoreAnalytics.swift @@ -73,9 +73,11 @@ public enum AnalyticsEvent: String { case resetPasswordClicked = "Logistration:Reset Password Clicked" case resetPasswordSuccess = "Logistration:Reset Password Success" case mainDiscoveryTabClicked = "MainDashboard:Discover" - case mainDashboardTabClicked = "MainDashboard:My Courses" - case mainProgramsTabClicked = "MainDashboard:My Programs" + case mainDashboardLearnTabClicked = "MainDashboard:Learn" case mainProfileTabClicked = "MainDashboard:Profile" + case mainProgramsTabClicked = "MainDashboard:My Programs" + case mainDashboardCoursesClicked = "Learn:My Courses" + case mainDashboardProgramsClicked = "Learn:My Programs" case discoverySearchBarClicked = "Discovery:Search Bar Clicked" case discoveryCoursesSearch = "Discovery:Courses Search" case discoveryCourseClicked = "Discovery:Course Clicked" @@ -163,7 +165,9 @@ public enum EventBIValue: String { case viewCourseClicked = "edx.bi.app.course.info" case resumeCourseClicked = "edx.bi.app.course.resume_course.clicked" case mainDiscoveryTabClicked = "edx.bi.app.main_dashboard.discover" - case mainDashboardTabClicked = "edx.bi.app.main_dashboard.my_course" + case mainDashboardLearnTabClicked = "edx.bi.app.main_dashboard.learn" + case mainDashboardCoursesClicked = "edx.bi.app.main_dashboard.learn.my_course" + case mainDashboardProgramsClicked = "edx.bi.app.main_dashboard.learn.my_programs" case mainProgramsTabClicked = "edx.bi.app.main_dashboard.my_program" case mainProfileTabClicked = "edx.bi.app.main_dashboard.profile" case profileEditClicked = "edx.bi.app.profile.edit.clicked" diff --git a/Course/Course/Presentation/Container/CourseContainerView.swift b/Course/Course/Presentation/Container/CourseContainerView.swift index 7f773561..0043f3fe 100644 --- a/Course/Course/Presentation/Container/CourseContainerView.swift +++ b/Course/Course/Presentation/Container/CourseContainerView.swift @@ -294,6 +294,7 @@ public struct CourseContainerView: View { Task { await viewModel.tryToRefreshCookies() } + viewModel.analytics.courseOutlineCourseTabClicked(courseId: courseID, courseName: title) } } diff --git a/Course/Course/Presentation/Handouts/HandoutsView.swift b/Course/Course/Presentation/Handouts/HandoutsView.swift index e3998963..d04be11a 100644 --- a/Course/Course/Presentation/Handouts/HandoutsView.swift +++ b/Course/Course/Presentation/Handouts/HandoutsView.swift @@ -78,7 +78,7 @@ struct HandoutsView: View { cssInjector: viewModel.cssInjector, type: type ) - viewModel.analytics.trackCourseEvent( + viewModel.analytics.trackCourseScreenEvent( .courseAnnouncement, biValue: .courseAnnouncement, courseID: courseID diff --git a/Dashboard/Dashboard/Presentation/DashboardAnalytics.swift b/Dashboard/Dashboard/Presentation/DashboardAnalytics.swift index d5edf28d..2dab5cf3 100644 --- a/Dashboard/Dashboard/Presentation/DashboardAnalytics.swift +++ b/Dashboard/Dashboard/Presentation/DashboardAnalytics.swift @@ -10,10 +10,14 @@ import Foundation //sourcery: AutoMockable public protocol DashboardAnalytics { func dashboardCourseClicked(courseID: String, courseName: String) + func mainProgramsClicked() + func mainCoursesClicked() } #if DEBUG class DashboardAnalyticsMock: DashboardAnalytics { public func dashboardCourseClicked(courseID: String, courseName: String) {} + public func mainProgramsClicked() {} + public func mainCoursesClicked() {} } #endif diff --git a/Dashboard/Dashboard/Presentation/Elements/DropDownMenu.swift b/Dashboard/Dashboard/Presentation/Elements/DropDownMenu.swift index beaf3bec..3c6d678a 100644 --- a/Dashboard/Dashboard/Presentation/Elements/DropDownMenu.swift +++ b/Dashboard/Dashboard/Presentation/Elements/DropDownMenu.swift @@ -26,6 +26,7 @@ enum MenuOption: String, CaseIterable { struct DropDownMenu: View { @Binding var selectedOption: MenuOption @State private var expanded: Bool = false + var analytics: DashboardAnalytics var body: some View { VStack(alignment: .leading, spacing: 2) { @@ -53,6 +54,12 @@ struct DropDownMenu: View { action: { selectedOption = option expanded = false + switch selectedOption { + case .courses: + analytics.mainCoursesClicked() + case .programs: + analytics.mainProgramsClicked() + } }, label: { HStack { Text(option.text) diff --git a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardView.swift b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardView.swift index e7af096b..8b04aa91 100644 --- a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardView.swift +++ b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardView.swift @@ -214,10 +214,6 @@ public struct PrimaryCourseDashboardView: View { id: \.offset ) { _, course in Button(action: { - viewModel.trackDashboardCourseClicked( - courseID: course.courseID, - courseName: course.name - ) router.showCourseScreens( courseID: course.courseID, hasAccess: course.hasAccess, @@ -308,7 +304,7 @@ public struct PrimaryCourseDashboardView: View { } if showDropdown { HStack(alignment: .center) { - DropDownMenu(selectedOption: $selectedMenu) + DropDownMenu(selectedOption: $selectedMenu, analytics: viewModel.analytics) Spacer() } } diff --git a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift index 0fd05294..b064907b 100644 --- a/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift +++ b/Dashboard/Dashboard/Presentation/PrimaryCourseDashboardViewModel.swift @@ -29,7 +29,7 @@ public class PrimaryCourseDashboardViewModel: ObservableObject { let connectivity: ConnectivityProtocol private let interactor: DashboardInteractorProtocol - private let analytics: DashboardAnalytics + let analytics: DashboardAnalytics let config: ConfigProtocol let storage: CoreStorage private var cancellables = Set() diff --git a/Dashboard/DashboardTests/DashboardMock.generated.swift b/Dashboard/DashboardTests/DashboardMock.generated.swift index 4fc3fb8d..1508c25a 100644 --- a/Dashboard/DashboardTests/DashboardMock.generated.swift +++ b/Dashboard/DashboardTests/DashboardMock.generated.swift @@ -3459,9 +3459,23 @@ open class DashboardAnalyticsMock: DashboardAnalytics, Mock { perform?(`courseID`, `courseName`) } + open func mainProgramsClicked() { + addInvocation(.m_mainProgramsClicked) + let perform = methodPerformValue(.m_mainProgramsClicked) as? () -> Void + perform?() + } + + open func mainCoursesClicked() { + addInvocation(.m_mainCoursesClicked) + let perform = methodPerformValue(.m_mainCoursesClicked) as? () -> Void + perform?() + } + fileprivate enum MethodType { case m_dashboardCourseClicked__courseID_courseIDcourseName_courseName(Parameter, Parameter) + case m_mainProgramsClicked + case m_mainCoursesClicked static func compareParameters(lhs: MethodType, rhs: MethodType, matcher: Matcher) -> Matcher.ComparisonResult { switch (lhs, rhs) { @@ -3470,17 +3484,26 @@ open class DashboardAnalyticsMock: DashboardAnalytics, Mock { results.append(Matcher.ParameterComparisonResult(Parameter.compare(lhs: lhsCourseid, rhs: rhsCourseid, with: matcher), lhsCourseid, rhsCourseid, "courseID")) results.append(Matcher.ParameterComparisonResult(Parameter.compare(lhs: lhsCoursename, rhs: rhsCoursename, with: matcher), lhsCoursename, rhsCoursename, "courseName")) return Matcher.ComparisonResult(results) + + case (.m_mainProgramsClicked, .m_mainProgramsClicked): return .match + + case (.m_mainCoursesClicked, .m_mainCoursesClicked): return .match + default: return .none } } func intValue() -> Int { switch self { case let .m_dashboardCourseClicked__courseID_courseIDcourseName_courseName(p0, p1): return p0.intValue + p1.intValue + case .m_mainProgramsClicked: return 0 + case .m_mainCoursesClicked: return 0 } } func assertionName() -> String { switch self { case .m_dashboardCourseClicked__courseID_courseIDcourseName_courseName: return ".dashboardCourseClicked(courseID:courseName:)" + case .m_mainProgramsClicked: return ".mainProgramsClicked()" + case .m_mainCoursesClicked: return ".mainCoursesClicked()" } } } @@ -3500,6 +3523,8 @@ open class DashboardAnalyticsMock: DashboardAnalytics, Mock { fileprivate var method: MethodType public static func dashboardCourseClicked(courseID: Parameter, courseName: Parameter) -> Verify { return Verify(method: .m_dashboardCourseClicked__courseID_courseIDcourseName_courseName(`courseID`, `courseName`))} + public static func mainProgramsClicked() -> Verify { return Verify(method: .m_mainProgramsClicked)} + public static func mainCoursesClicked() -> Verify { return Verify(method: .m_mainCoursesClicked)} } public struct Perform { @@ -3509,6 +3534,12 @@ open class DashboardAnalyticsMock: DashboardAnalytics, Mock { public static func dashboardCourseClicked(courseID: Parameter, courseName: Parameter, perform: @escaping (String, String) -> Void) -> Perform { return Perform(method: .m_dashboardCourseClicked__courseID_courseIDcourseName_courseName(`courseID`, `courseName`), performs: perform) } + public static func mainProgramsClicked(perform: @escaping () -> Void) -> Perform { + return Perform(method: .m_mainProgramsClicked, performs: perform) + } + public static func mainCoursesClicked(perform: @escaping () -> Void) -> Perform { + return Perform(method: .m_mainCoursesClicked, performs: perform) + } } public func given(_ method: Given) { diff --git a/Discovery/Discovery/Presentation/WebDiscovery/DiscoveryWebviewViewModel.swift b/Discovery/Discovery/Presentation/WebDiscovery/DiscoveryWebviewViewModel.swift index 86f17ef5..aed09eff 100644 --- a/Discovery/Discovery/Presentation/WebDiscovery/DiscoveryWebviewViewModel.swift +++ b/Discovery/Discovery/Presentation/WebDiscovery/DiscoveryWebviewViewModel.swift @@ -169,6 +169,7 @@ extension DiscoveryWebviewViewModel: WebViewNavigationDelegate { } case .courseDetail: guard let pathID = detailPathID(from: url) else { return false } + analytics.discoveryScreenEvent(event: .viewCourseClicked, biValue: .viewCourseClicked) router.showWebDiscoveryDetails( pathID: pathID, discoveryType: .courseDetail(pathID), diff --git a/OpenEdX/Managers/AnalyticsManager/AnalyticsManager.swift b/OpenEdX/Managers/AnalyticsManager/AnalyticsManager.swift index 2487a8c7..5e3ca3ab 100644 --- a/OpenEdX/Managers/AnalyticsManager/AnalyticsManager.swift +++ b/OpenEdX/Managers/AnalyticsManager/AnalyticsManager.swift @@ -146,8 +146,8 @@ class AnalyticsManager: AuthorizationAnalytics, trackScreenEvent(.mainDiscoveryTabClicked, biValue: .mainDiscoveryTabClicked) } - public func mainDashboardTabClicked() { - trackEvent(.mainDashboardTabClicked, biValue: .mainDashboardTabClicked) + public func mainLearnTabClicked() { + trackScreenEvent(.mainDashboardLearnTabClicked, biValue: .mainDashboardLearnTabClicked) } public func mainProgramsTabClicked() { @@ -158,6 +158,14 @@ class AnalyticsManager: AuthorizationAnalytics, trackScreenEvent(.mainProfileTabClicked, biValue: .mainProfileTabClicked) } + public func mainCoursesClicked() { + trackScreenEvent(.mainDashboardCoursesClicked, biValue: .mainDashboardCoursesClicked) + } + + public func mainProgramsClicked() { + trackScreenEvent(.mainDashboardProgramsClicked, biValue: .mainDashboardProgramsClicked) + } + // MARK: Discovery public func discoverySearchBarClicked() { diff --git a/OpenEdX/Managers/AnalyticsManager/MainScreenAnalytics.swift b/OpenEdX/Managers/AnalyticsManager/MainScreenAnalytics.swift index fc7bdb38..f65a7658 100644 --- a/OpenEdX/Managers/AnalyticsManager/MainScreenAnalytics.swift +++ b/OpenEdX/Managers/AnalyticsManager/MainScreenAnalytics.swift @@ -10,16 +10,20 @@ import Foundation //sourcery: AutoMockable public protocol MainScreenAnalytics { func mainDiscoveryTabClicked() - func mainDashboardTabClicked() - func mainProgramsTabClicked() + func mainLearnTabClicked() func mainProfileTabClicked() + func mainProgramsTabClicked() + func mainCoursesClicked() + func mainProgramsClicked() } #if DEBUG public class MainScreenAnalyticsMock: MainScreenAnalytics { public func mainDiscoveryTabClicked() {} - public func mainDashboardTabClicked() {} - public func mainProgramsTabClicked() {} + public func mainLearnTabClicked() {} public func mainProfileTabClicked() {} + public func mainProgramsTabClicked() {} + public func mainProgramsClicked() {} + public func mainCoursesClicked() {} } #endif diff --git a/OpenEdX/Router.swift b/OpenEdX/Router.swift index 721dac5d..e238568d 100644 --- a/OpenEdX/Router.swift +++ b/OpenEdX/Router.swift @@ -387,6 +387,10 @@ public class Router: AuthorizationRouter, try? await Task.sleep(for: .seconds(1)) await Container.shared.resolve(PushNotificationsManager.self)?.performRegistration() } + + if let analytics = Container.shared.resolve(DashboardAnalytics.self) { + analytics.dashboardCourseClicked(courseID: courseID, courseName: title) + } } public func getCourseScreensController( diff --git a/OpenEdX/View/MainScreenView.swift b/OpenEdX/View/MainScreenView.swift index f430a466..f8d4a620 100644 --- a/OpenEdX/View/MainScreenView.swift +++ b/OpenEdX/View/MainScreenView.swift @@ -179,7 +179,7 @@ struct MainScreenView: View { case .discovery: viewModel.trackMainDiscoveryTabClicked() case .dashboard: - viewModel.trackMainDashboardTabClicked() + viewModel.trackMainDashboardLearnTabClicked() case .programs: viewModel.trackMainProgramsTabClicked() case .profile: @@ -191,6 +191,8 @@ struct MainScreenView: View { await viewModel.prefetchDataForOffline() await viewModel.loadCalendar() } + viewModel.trackMainDashboardLearnTabClicked() + viewModel.trackMainDashboardMyCoursesClicked() } .accentColor(Theme.Colors.accentXColor) } diff --git a/OpenEdX/View/MainScreenViewModel.swift b/OpenEdX/View/MainScreenViewModel.swift index 84fe435a..8cc55f59 100644 --- a/OpenEdX/View/MainScreenViewModel.swift +++ b/OpenEdX/View/MainScreenViewModel.swift @@ -73,12 +73,15 @@ final class MainScreenViewModel: ObservableObject { func trackMainDiscoveryTabClicked() { analytics.mainDiscoveryTabClicked() } - func trackMainDashboardTabClicked() { - analytics.mainDashboardTabClicked() + + func trackMainDashboardLearnTabClicked() { + analytics.mainLearnTabClicked() } + func trackMainProgramsTabClicked() { analytics.mainProgramsTabClicked() } + func trackMainProfileTabClicked() { analytics.mainProfileTabClicked() } @@ -114,6 +117,10 @@ final class MainScreenViewModel: ObservableObject { } } + func trackMainDashboardMyCoursesClicked() { + analytics.mainCoursesClicked() + } + @MainActor func prefetchDataForOffline() async { if await profileInteractor.getMyProfileOffline() == nil { From f0bfeca0e5843d6af098a289f1e67390c7eaf7c8 Mon Sep 17 00:00:00 2001 From: Anton Yarmolenko <37253+rnr@users.noreply.github.com> Date: Thu, 5 Dec 2024 17:44:19 +0100 Subject: [PATCH 5/5] chore: fix tests --- Core/CoreTests/DownloadManager/DownloadManagerTests.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Core/CoreTests/DownloadManager/DownloadManagerTests.swift b/Core/CoreTests/DownloadManager/DownloadManagerTests.swift index 5eb8d8b6..5c1b2ec5 100644 --- a/Core/CoreTests/DownloadManager/DownloadManagerTests.swift +++ b/Core/CoreTests/DownloadManager/DownloadManagerTests.swift @@ -40,7 +40,8 @@ final class DownloadManagerTests: XCTestCase { Given(storage, .userSettings(getter: UserSettings( wifiOnly: true, streamingQuality: .auto, - downloadQuality: .auto + downloadQuality: .auto, + playbackSpeed: 1.0 ))) let blocks = [createMockCourseBlock()] @@ -57,7 +58,8 @@ final class DownloadManagerTests: XCTestCase { Given(storage, .userSettings(getter: UserSettings( wifiOnly: true, streamingQuality: .auto, - downloadQuality: .auto + downloadQuality: .auto, + playbackSpeed: 1.0 ))) Given(connectivity, .isInternetAvaliable(getter: true)) Given(connectivity, .isMobileData(getter: true))