sync: Part 7 sync to upstream (#548)

* 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 <[email protected]> * chore: delete unneeded MainActor instructions * 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 <[email protected]> * chore: analytics screen events improvements and name fixes (#76) * chore: fix tests --------- Co-authored-by: Anton Yarmolenko <[email protected]> Co-authored-by: Saeed Bashir <[email protected]>
openedx · Dec 9, 2024 · 17f9376 · 17f9376
1 parent 0f16019
commit 17f9376
Show file tree

Hide file tree

Showing 24 changed files with 178 additions and 64 deletions.
diff --git 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/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/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))

diff --git 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
@@ -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/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/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<Double, Never>
     func getErrorPublisher() -> AnyPublisher<Error, Never>
@@ -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<Error, Never>()
     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
@@ -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<Error, Never>()
     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)
         }
@@ -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()
@@ -182,7 +184,6 @@ extension YoutubePlayerViewControllerHolder {
             blockID: "",
             courseID: "",
             selectedCourseTab: 0,
-            videoResolution: .zero,
             pipManager: PipManagerProtocolMock(),
             playerTracker: PlayerTrackerProtocolMock(url: URL(string: "")),
             playerDelegate: nil,
@@ -191,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
@@ -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/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
@@ -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
@@ -214,10 +214,6 @@ public struct PrimaryCourseDashboardView<ProgramView: View>: 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<ProgramView: View>: View {
                     }
                     if showDropdown {
                         HStack(alignment: .center) {
-                            DropDownMenu(selectedOption: $selectedMenu)
+                            DropDownMenu(selectedOption: $selectedMenu, analytics: viewModel.analytics)
                             Spacer()
                         }
                     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -294,6 +294,7 @@ public struct CourseContainerView: View { @@
                 Task {
                     await viewModel.tryToRefreshCookies()
                 }
+                viewModel.analytics.courseOutlineCourseTabClicked(courseId: courseID, courseName: title)
             }
         }
@@ Expand Down @@