Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync: Part 7 sync to upstream #548

Merged
merged 6 commits into from
Dec 9, 2024
Merged

sync: Part 7 sync to upstream #548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3d5d7bb
fix: video tab with youtube (#52)
rnr Jul 29, 2024
a806e04
chore: delete unneeded MainActor instructions
rnr Dec 5, 2024
108044a
feat: save playback speed (#67)
rnr Aug 22, 2024
f911de0
Merge branch 'develop' into part-7-sync-to-upstream
rnr Dec 5, 2024
721bc86
chore: analytics screen events improvements and name fixes (#76)
saeedbashir Sep 13, 2024
f0bfeca
chore: fix tests
rnr Dec 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions Core/Core/Analytics/CoreAnalytics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion Core/Core/Data/Model/UserSettings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
6 changes: 4 additions & 2 deletions Core/CoreTests/DownloadManager/DownloadManagerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()]
Expand All @@ -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))
Expand Down
1 change: 1 addition & 0 deletions Course/Course/Presentation/Container/CourseContainerView.swift
View file
Open in desktop
Original file line number Diff 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
2 changes: 1 addition & 1 deletion Course/Course/Presentation/Handouts/HandoutsView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ struct HandoutsView: View {
cssInjector: viewModel.cssInjector,
type: type
)
viewModel.analytics.trackCourseEvent(
viewModel.analytics.trackCourseScreenEvent(
.courseAnnouncement,
biValue: .courseAnnouncement,
courseID: courseID
Expand Down
2 changes: 1 addition & 1 deletion Course/Course/Presentation/Outline/CourseOutlineView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ public struct CourseOutlineView: View {
}
}
}
} else {
} else if viewModel.courseVideosStructure == nil {
FullScreenErrorView(
type: .noContent(
CourseLocalization.Error.videosUnavailable,
Expand Down
41 changes: 32 additions & 9 deletions Course/Course/Presentation/Video/PlayerViewControllerHolder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

@preconcurrency import AVKit
@preconcurrency import Combine
import Core

@MainActor
public protocol PlayerViewControllerHolderProtocol: AnyObject, Sendable {
Expand All @@ -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>
Expand Down Expand Up @@ -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

Expand All @@ -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
}()

Expand All @@ -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()
}

Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand Down Expand Up @@ -218,7 +241,6 @@ extension PlayerViewControllerHolder {
blockID: "",
courseID: "",
selectedCourseTab: 0,
videoResolution: .zero,
pipManager: PipManagerProtocolMock(),
playerTracker: PlayerTrackerProtocolMock(url: URL(string: "")),
playerDelegate: nil,
Expand All @@ -227,7 +249,8 @@ extension PlayerViewControllerHolder {
blockID: "",
interactor: CourseInteractor.mock,
router: CourseRouterMock()
)
),
appStorage: CoreStorageMock()
)
}
}
Expand Down
16 changes: 9 additions & 7 deletions Course/Course/Presentation/Video/YoutubePlayerViewControllerHolder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
@preconcurrency import Combine
import Foundation
@preconcurrency import YouTubePlayerKit
import Core

@MainActor
public final class YoutubePlayerViewControllerHolder: PlayerViewControllerHolderProtocol {
Expand All @@ -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] = []
Expand All @@ -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)
}
Expand All @@ -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()
Expand Down Expand Up @@ -182,7 +184,6 @@ extension YoutubePlayerViewControllerHolder {
blockID: "",
courseID: "",
selectedCourseTab: 0,
videoResolution: .zero,
pipManager: PipManagerProtocolMock(),
playerTracker: PlayerTrackerProtocolMock(url: URL(string: "")),
playerDelegate: nil,
Expand All @@ -191,7 +192,8 @@ extension YoutubePlayerViewControllerHolder {
blockID: "",
interactor: CourseInteractor.mock,
router: CourseRouterMock()
)
),
appStorage: nil
)
}
}
Expand Down
14 changes: 7 additions & 7 deletions Course/CourseTests/Presentation/Unit/VideoPlayerViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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 = [
Expand Down Expand Up @@ -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}))
Expand All @@ -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()))
Expand All @@ -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))
Expand Down
4 changes: 4 additions & 0 deletions Dashboard/Dashboard/Presentation/DashboardAnalytics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions Dashboard/Dashboard/Presentation/Elements/DropDownMenu.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 1 addition & 5 deletions Dashboard/Dashboard/Presentation/PrimaryCourseDashboardView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
}
}
Expand Down
Loading
Loading