refactor: address review feedback

openedx · Dec 15, 2023 · 83d62bd · 83d62bd
1 parent bf42b25
commit 83d62bd
Show file tree

Hide file tree

Showing 14 changed files with 88 additions and 50 deletions.
diff --git a/Authorization/Authorization/Presentation/Login/SignInView.swift b/Authorization/Authorization/Presentation/Login/SignInView.swift
@@ -192,7 +192,8 @@ struct SignInView_Previews: PreviewProvider {
             router: AuthorizationRouterMock(),
             config: ConfigMock(),
             analytics: AuthorizationAnalyticsMock(),
-            validator: Validator()
+            validator: Validator(),
+            sourceScreen: .default
         )
 
         SignInView(viewModel: vm)

diff --git a/Authorization/Authorization/Presentation/Login/SignInViewModel.swift b/Authorization/Authorization/Presentation/Login/SignInViewModel.swift
@@ -19,7 +19,7 @@ public class SignInViewModel: ObservableObject {
     @Published private(set) var isShowProgress = false
     @Published private(set) var showError: Bool = false
     @Published private(set) var showAlert: Bool = false
-    public var sourceScreen: LogistrationSourceScreen = .default
+    public var sourceScreen: LogistrationSourceScreen
 
     var errorMessage: String? {
         didSet {
@@ -47,13 +47,15 @@ public class SignInViewModel: ObservableObject {
         router: AuthorizationRouter,
         config: ConfigProtocol,
         analytics: AuthorizationAnalytics,
-        validator: Validator
+        validator: Validator,
+        sourceScreen: LogistrationSourceScreen
     ) {
         self.interactor = interactor
         self.router = router
         self.config = config
         self.analytics = analytics
         self.validator = validator
+        self.sourceScreen = sourceScreen
     }
 
     var socialAuthEnabled: Bool {

diff --git a/Authorization/Authorization/Presentation/Registration/SignUpView.swift b/Authorization/Authorization/Presentation/Registration/SignUpView.swift
@@ -177,7 +177,8 @@ struct SignUpView_Previews: PreviewProvider {
             analytics: AuthorizationAnalyticsMock(),
             config: ConfigMock(),
             cssInjector: CSSInjectorMock(),
-            validator: Validator()
+            validator: Validator(),
+            sourceScreen: .default
         )
 
         SignUpView(viewModel: vm)

diff --git a/Authorization/Authorization/Presentation/Registration/SignUpViewModel.swift b/Authorization/Authorization/Presentation/Registration/SignUpViewModel.swift
@@ -19,7 +19,7 @@ public class SignUpViewModel: ObservableObject {
     @Published var scrollTo: Int?
     @Published var showError: Bool = false
     @Published var thirdPartyAuthSuccess: Bool = false
-    public var sourceScreen: LogistrationSourceScreen = .default
+    public var sourceScreen: LogistrationSourceScreen
 
     var errorMessage: String? {
         didSet {
@@ -45,14 +45,16 @@ public class SignUpViewModel: ObservableObject {
         analytics: AuthorizationAnalytics,
         config: ConfigProtocol,
         cssInjector: CSSInjector,
-        validator: Validator
+        validator: Validator,
+        sourceScreen: LogistrationSourceScreen
     ) {
         self.interactor = interactor
         self.router = router
         self.analytics = analytics
         self.config = config
         self.cssInjector = cssInjector
         self.validator = validator
+        self.sourceScreen = sourceScreen
     }
 
     var socialAuthEnabled: Bool {

diff --git a/Authorization/Authorization/Presentation/Startup/StartupView.swift b/Authorization/Authorization/Presentation/Startup/StartupView.swift
@@ -55,7 +55,7 @@ public struct StartupView: View {
                                 if searchQuery.isEmpty { return }
                                 viewModel.router.showDiscoveryScreen(
                                     searchQuery: searchQuery,
-                                    sourceScreen: LogistrationSourceScreen.startup
+                                    sourceScreen: .startup
                                 )
                             })
                             .autocapitalization(.none)
@@ -76,7 +76,7 @@ public struct StartupView: View {
                         Button {
                             viewModel.router.showDiscoveryScreen (
                                 searchQuery: searchQuery,
-                                sourceScreen: LogistrationSourceScreen.startup
+                                sourceScreen: .startup
                             )
                         } label: {
                             Text(AuthLocalization.Startup.exploreAllCourses)

diff --git a/Course/Course/Presentation/Details/CourseDetailsView.swift b/Course/Course/Presentation/Details/CourseDetailsView.swift
@@ -225,17 +225,21 @@ private struct CourseStateView: View {
                 .padding(.vertical, 24)
         case .alreadyEnrolled:
             StyledButton(CourseLocalization.Details.viewCourse, action: {
-                viewModel.viewCourseClicked(courseId: courseDetails.courseID,
-                                            courseName: courseDetails.courseTitle)
-                viewModel.router.showCourseScreens(
-                    courseID: courseDetails.courseID,
-                    isActive: nil,
-                    courseStart: courseDetails.courseStart,
-                    courseEnd: courseDetails.courseEnd,
-                    enrollmentStart: courseDetails.enrollmentStart,
-                    enrollmentEnd: courseDetails.enrollmentEnd,
-                    title: title
-                )
+                if !viewModel.userloggedIn {
+                    viewModel.router.showRegisterScreen(sourceScreen: .courseDetail(courseDetails.courseID, courseDetails.courseTitle))
+                } else {
+                    viewModel.viewCourseClicked(courseId: courseDetails.courseID,
+                                                courseName: courseDetails.courseTitle)
+                    viewModel.router.showCourseScreens(
+                        courseID: courseDetails.courseID,
+                        isActive: nil,
+                        courseStart: courseDetails.courseStart,
+                        courseEnd: courseDetails.courseEnd,
+                        enrollmentStart: courseDetails.enrollmentStart,
+                        enrollmentEnd: courseDetails.enrollmentEnd,
+                        title: title
+                    )
+                }
             })
             .padding(16)
         }

diff --git a/Discovery/Discovery/Presentation/DiscoveryView.swift b/Discovery/Discovery/Presentation/DiscoveryView.swift
@@ -195,7 +195,8 @@ struct DiscoveryView_Previews: PreviewProvider {
                                     config: ConfigMock(),
                                     interactor: DiscoveryInteractor.mock,
                                     connectivity: Connectivity(),
-                                    analytics: DiscoveryAnalyticsMock())
+                                    analytics: DiscoveryAnalyticsMock(),
+                                    storage: CoreStorageMock())
         let router = DiscoveryRouterMock()
 
         DiscoveryView(viewModel: vm, router: router)

diff --git a/Discovery/Discovery/Presentation/DiscoveryViewModel.swift b/Discovery/Discovery/Presentation/DiscoveryViewModel.swift
@@ -23,8 +23,7 @@ public class DiscoveryViewModel: ObservableObject {
     @Published var showError: Bool = false
 
     var userloggedIn: Bool {
-        guard let container = Container.shared.resolve(CoreStorage.self) else { return false }
-        return !(container.user?.username?.isEmpty ?? true)
+        return !(storage.user?.username?.isEmpty ?? true)
     }
 
     var errorMessage: String? {
@@ -40,19 +39,22 @@ public class DiscoveryViewModel: ObservableObject {
     let connectivity: ConnectivityProtocol
     private let interactor: DiscoveryInteractorProtocol
     private let analytics: DiscoveryAnalytics
+    private let storage: CoreStorage
 
     public init(
         router: DiscoveryRouter,
         config: ConfigProtocol,
         interactor: DiscoveryInteractorProtocol,
         connectivity: ConnectivityProtocol,
-        analytics: DiscoveryAnalytics
+        analytics: DiscoveryAnalytics,
+        storage: CoreStorage
     ) {
         self.router = router
         self.config = config
         self.interactor = interactor
         self.connectivity = connectivity
         self.analytics = analytics
+        self.storage = storage
     }
 
     @MainActor

diff --git a/Discovery/DiscoveryTests/Presentation/DiscoveryViewModelTests.swift b/Discovery/DiscoveryTests/Presentation/DiscoveryViewModelTests.swift
@@ -26,11 +26,12 @@ final class DiscoveryViewModelTests: XCTestCase {
         let interactor = DiscoveryInteractorProtocolMock()
         let connectivity = Connectivity()
         let analytics = DiscoveryAnalyticsMock()
-        let viewModel = DiscoveryViewModel(router: DiscoveryRouterMock(), 
+        let viewModel = DiscoveryViewModel(router: DiscoveryRouterMock(),
                                            config: ConfigMock(),
                                            interactor: interactor,
                                            connectivity: connectivity,
-                                           analytics: analytics)
+                                           analytics: analytics,
+                                           storage: CoreStorageMock())
 
         let items = [
             CourseItem(name: "Test",
@@ -79,7 +80,8 @@ final class DiscoveryViewModelTests: XCTestCase {
                                            config: ConfigMock(),
                                            interactor: interactor,
                                            connectivity: connectivity,
-                                           analytics: analytics)
+                                           analytics: analytics,
+                                           storage: CoreStorageMock())
         let items = [
             CourseItem(name: "Test",
                        org: "org",
@@ -126,7 +128,8 @@ final class DiscoveryViewModelTests: XCTestCase {
                                            config: ConfigMock(),
                                            interactor: interactor,
                                            connectivity: connectivity,
-                                           analytics: analytics)
+                                           analytics: analytics,
+                                           storage: CoreStorageMock())
         let items = [
             CourseItem(name: "Test",
                        org: "org",
@@ -175,7 +178,8 @@ final class DiscoveryViewModelTests: XCTestCase {
                                            config: ConfigMock(),
                                            interactor: interactor,
                                            connectivity: connectivity,
-                                           analytics: analytics)
+                                           analytics: analytics,
+                                           storage: CoreStorageMock())
 
         let noInternetError = AFError.sessionInvalidated(error: URLError(.notConnectedToInternet))
 
@@ -198,7 +202,8 @@ final class DiscoveryViewModelTests: XCTestCase {
                                            config: ConfigMock(),
                                            interactor: interactor,
                                            connectivity: connectivity,
-                                           analytics: analytics)
+                                           analytics: analytics,
+                                           storage: CoreStorageMock())
 
         let noInternetError = AFError.sessionInvalidated(error: NSError())
 

diff --git a/OpenEdX/DI/ScreenAssembly.swift b/OpenEdX/DI/ScreenAssembly.swift
@@ -34,11 +34,12 @@ class ScreenAssembly: Assembly {
         }
 
         // MARK: MainScreenView
-        container.register(MainScreenViewModel.self) { r in
+        container.register(MainScreenViewModel.self) { r, sourceScreen in
             MainScreenViewModel(
                 analytics: r.resolve(MainScreenAnalytics.self)!,
                 config: r.resolve(ConfigProtocol.self)!,
-                profileInteractor: r.resolve(ProfileInteractorProtocol.self)!
+                profileInteractor: r.resolve(ProfileInteractorProtocol.self)!,
+                sourceScreen: sourceScreen
             )
         }
         // MARK: Startup screen
@@ -51,23 +52,25 @@ class ScreenAssembly: Assembly {
         }
 
         // MARK: SignIn
-        container.register(SignInViewModel.self) { r in
+        container.register(SignInViewModel.self) { r, sourceScreen in
             SignInViewModel(
                 interactor: r.resolve(AuthInteractorProtocol.self)!,
                 router: r.resolve(AuthorizationRouter.self)!,
                 config: r.resolve(ConfigProtocol.self)!,
                 analytics: r.resolve(AuthorizationAnalytics.self)!,
-                validator: r.resolve(Validator.self)!
+                validator: r.resolve(Validator.self)!,
+                sourceScreen: sourceScreen
             )
         }
-        container.register(SignUpViewModel.self) { r in
+        container.register(SignUpViewModel.self) { r, sourceScreen in
             SignUpViewModel(
                 interactor: r.resolve(AuthInteractorProtocol.self)!,
                 router: r.resolve(AuthorizationRouter.self)!,
                 analytics: r.resolve(AuthorizationAnalytics.self)!,
                 config: r.resolve(ConfigProtocol.self)!,
                 cssInjector: r.resolve(CSSInjector.self)!,
-                validator: r.resolve(Validator.self)!
+                validator: r.resolve(Validator.self)!,
+                sourceScreen: sourceScreen
             )
         }
         container.register(ResetPasswordViewModel.self) { r in
@@ -103,7 +106,8 @@ class ScreenAssembly: Assembly {
                 config: r.resolve(ConfigProtocol.self)!,
                 interactor: r.resolve(DiscoveryInteractorProtocol.self)!,
                 connectivity: r.resolve(ConnectivityProtocol.self)!,
-                analytics: r.resolve(DiscoveryAnalytics.self)!
+                analytics: r.resolve(DiscoveryAnalytics.self)!,
+                storage: r.resolve(CoreStorage.self)!
             )
         }
 

diff --git a/OpenEdX/RouteController.swift b/OpenEdX/RouteController.swift
@@ -74,7 +74,10 @@ class RouteController: UIViewController {
             let controller = UIHostingController(rootView: whatsNewView)
             navigation.viewControllers = [controller]
         } else {
-            let viewModel = Container.shared.resolve(MainScreenViewModel.self)!
+            let viewModel = Container.shared.resolve(
+                MainScreenViewModel.self,
+                argument: LogistrationSourceScreen.default
+            )!
             let controller = UIHostingController(rootView: MainScreenView(viewModel: viewModel))
             navigation.viewControllers = [controller]
         }

diff --git a/OpenEdX/Router.swift b/OpenEdX/Router.swift
@@ -63,8 +63,7 @@ public class Router: AuthorizationRouter,
         var storage = Container.shared.resolve(WhatsNewStorage.self)!
         let config = Container.shared.resolve(ConfigProtocol.self)!
 
-        let viewModel = WhatsNewViewModel(storage: storage)
-        viewModel.sourceScreen = sourceScreen
+        let viewModel = WhatsNewViewModel(storage: storage, sourceScreen: sourceScreen)
         let whatsNew = WhatsNewView(router: Container.shared.resolve(WhatsNewRouter.self)!, viewModel: viewModel)
         let shouldShowWhatsNew = viewModel.shouldShowWhatsNew()
 
@@ -76,18 +75,24 @@ public class Router: AuthorizationRouter,
             navigationController.viewControllers = [controller]
             navigationController.setViewControllers([controller], animated: true)
         } else {
-            let viewModel = Container.shared.resolve(MainScreenViewModel.self)!
-            viewModel.sourceScreen = sourceScreen
+            let viewModel = Container.shared.resolve(
+                MainScreenViewModel.self,
+                argument: sourceScreen
+            )!
+
+
             let controller = UIHostingController(rootView: MainScreenView(viewModel: viewModel))
             navigationController.viewControllers = [controller]
             navigationController.setViewControllers([controller], animated: true)
         }
     }
 
     public func showLoginScreen(sourceScreen: LogistrationSourceScreen) {
-        guard let viewModel = Container.shared.resolve(SignInViewModel.self) else { return }
-
-        viewModel.sourceScreen = sourceScreen
+        guard let viewModel = Container.shared.resolve(
+            SignInViewModel.self,
+            argument: sourceScreen
+        ) else { return }
+
         let view = SignInView(viewModel: viewModel)
         let controller = UIHostingController(rootView: view)
         navigationController.pushViewController(controller, animated: true)
@@ -171,9 +176,11 @@ public class Router: AuthorizationRouter,
     }
 
     public func showRegisterScreen(sourceScreen: LogistrationSourceScreen) {
-        guard let viewModel = Container.shared.resolve(SignUpViewModel.self) else { return }
+        guard let viewModel = Container.shared.resolve(
+            SignUpViewModel.self,
+            argument: sourceScreen
+        ) else { return }
 
-        viewModel.sourceScreen = sourceScreen
         let view = SignUpView(viewModel: viewModel)
         let controller = UIHostingController(rootView: view)
         navigationController.pushViewController(controller, animated: true)

diff --git a/OpenEdX/View/MainScreenViewModel.swift b/OpenEdX/View/MainScreenViewModel.swift
@@ -14,12 +14,17 @@ class MainScreenViewModel: ObservableObject {
     private let analytics: MainScreenAnalytics
     let config: ConfigProtocol
     let profileInteractor: ProfileInteractorProtocol
-    var sourceScreen: LogistrationSourceScreen = .default
+    var sourceScreen: LogistrationSourceScreen
 
-    init(analytics: MainScreenAnalytics, config: ConfigProtocol, profileInteractor: ProfileInteractorProtocol) {
+    init(analytics: MainScreenAnalytics,
+         config: ConfigProtocol,
+         profileInteractor: ProfileInteractorProtocol,
+         sourceScreen: LogistrationSourceScreen = .default
+    ) {
         self.analytics = analytics
         self.config = config
         self.profileInteractor = profileInteractor
+        self.sourceScreen = sourceScreen
     }
 
     func trackMainDiscoveryTabClicked() {

diff --git a/WhatsNew/WhatsNew/Presentation/WhatsNewViewModel.swift b/WhatsNew/WhatsNew/Presentation/WhatsNewViewModel.swift
@@ -13,10 +13,11 @@ public class WhatsNewViewModel: ObservableObject {
     @Published var index: Int = 0
     @Published var newItems: [WhatsNewPage] = []
     private let storage: WhatsNewStorage
-    public var sourceScreen: LogistrationSourceScreen = .default
+    var sourceScreen: LogistrationSourceScreen
 
-    public init(storage: WhatsNewStorage) {
+    public init(storage: WhatsNewStorage, sourceScreen: LogistrationSourceScreen = .default) {
         self.storage = storage
+        self.sourceScreen = sourceScreen
         newItems = loadWhatsNew()
     }