[NT-783] Simplify internal/external link behaviour in updates (#1062)

* Simplify internal/external link behaviour * Use same naming convention for signals
kickstarter · Feb 10, 2020 · ef3b7a0 · ef3b7a0
1 parent 18c679d
commit ef3b7a0
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 52 deletions.
diff --git a/Kickstarter-iOS/ViewModels/UpdateViewModel.swift b/Kickstarter-iOS/ViewModels/UpdateViewModel.swift
@@ -94,34 +94,30 @@ internal final class UpdateViewModel: UpdateViewModelType, UpdateViewModelInputs
           : .cancel
       }
 
-    let possiblyGoToComments = currentUpdate
-      .takePairWhen(navigationAction)
-      .map { update, action -> Update? in
-        if action.navigationType == .linkActivated,
-          Navigation.Project.updateCommentsWithRequest(action.request) != nil {
-          return update
-        }
-        return nil
-      }
+    let externalNavigationAction = navigationAction
+      .filter { action in action.navigationType == .linkActivated }
+      .filter(isExternalNavigation)
 
-    let possiblyGoToProject = navigationAction
-      .map { action in
-        action.navigationType == .linkActivated
-          ? Navigation.Project.withRequest(action.request)
-          : nil
-      }
+    let internalNavigationAction = navigationAction
+      .filter { action in action.navigationType == .linkActivated }
+      .filter(isExternalNavigation >>> isFalse)
 
-    let possiblyGoToUpdate = navigationAction
-      .map { action in
-        action.navigationType == .linkActivated
-          ? Navigation.Project.updateWithRequest(action.request)
-          : nil
+    let goToComments = currentUpdate
+      .takePairWhen(internalNavigationAction)
+      .map { update, action -> Update? in
+        Navigation.Project.updateCommentsWithRequest(action.request) != nil ? update : nil
       }
+      .skipNil()
 
-    let projectAndRefTag = project
-      .takePairWhen(possiblyGoToProject)
-      .switchMap { (project, projectParamAndRefTag) -> SignalProducer<(Project, RefTag), Never> in
+    let goToProject = internalNavigationAction
+      .map { action in (action, Navigation.Project.withRequest(action.request)) }
+      .filter(second >>> isNotNil)
 
+    let projectAndRefTag = project
+      .takePairWhen(goToProject)
+      .map(unpack)
+      .switchMap { (project, action, projectParamAndRefTag)
+        -> SignalProducer<(WKNavigationActionData, Project, RefTag), Never> in
         guard let (projectParam, refTag) = projectParamAndRefTag else { return .empty }
 
         let producer: SignalProducer<Project, Never>
@@ -133,33 +129,20 @@ internal final class UpdateViewModel: UpdateViewModelType, UpdateViewModelInputs
             .demoteErrors()
         }
 
-        return producer.map { ($0, refTag ?? RefTag.update) }
+        return producer.map { (action, $0, refTag ?? RefTag.update) }
       }
 
-    self.goToComments = possiblyGoToComments.skipNil()
-    self.goToProject = projectAndRefTag.filter { project, _ in project.prelaunchActivated != .some(true) }
+    self.goToComments = goToComments
+    self.goToProject = projectAndRefTag.filter { _, project, _ in project.prelaunchActivated != .some(true) }
+      .map { _, project, refTag in (project, refTag) }
 
-    let projectIsPrelaunch = projectAndRefTag.filter { project, _ in project.prelaunchActivated == true }
-    let notProjectNotCommentsNotUpdate = Signal.zip(
-      possiblyGoToProject,
-      possiblyGoToComments,
-      possiblyGoToUpdate
-    )
-    .filter { goToProject, goToComments, goToUpdate in
-      goToProject == nil && goToComments == nil && goToUpdate == nil
-    }
-
-    let goToExternalLink = Signal.zip(navigationAction, notProjectNotCommentsNotUpdate).map(first)
-    let goToPrelaunchPage = Signal.zip(navigationAction, projectIsPrelaunch).map(first)
-
-    self.goToSafariBrowser = Signal.merge(goToExternalLink, goToPrelaunchPage)
-      .filterMap { action in
-        guard action.navigationType == .linkActivated else {
-          return nil
-        }
+    let goToPrelaunchPage = projectAndRefTag
+      .filter { _, project, _ in project.prelaunchActivated == true }
+      .map(first)
 
-        return action.request.url
-      }
+    self.goToSafariBrowser = Signal.merge(externalNavigationAction, goToPrelaunchPage)
+      .map { action in action.request.url }
+      .skipNil()
 
     project
       .takeWhen(self.goToSafariBrowser)
@@ -195,3 +178,11 @@ internal final class UpdateViewModel: UpdateViewModelType, UpdateViewModelInputs
   internal var inputs: UpdateViewModelInputs { return self }
   internal var outputs: UpdateViewModelOutputs { return self }
 }
+
+func isExternalNavigation(_ action: WKNavigationActionData) -> Bool {
+  return ([
+    Navigation.Project.withRequest(action.request),
+    Navigation.Project.updateWithRequest(action.request),
+    Navigation.Project.updateCommentsWithRequest(action.request)
+  ] as [Any?]).compactMap { $0 }.isEmpty
+}
diff --git a/Kickstarter-iOS/ViewModels/UpdateViewModelTests.swift b/Kickstarter-iOS/ViewModels/UpdateViewModelTests.swift
@@ -205,18 +205,47 @@ final class UpdateViewModelTests: TestCase {
     self.vm.inputs.viewDidLoad()
 
     withEnvironment(apiService: MockService(fetchProjectResponse: prelaunchProject)) {
-      let request = URLRequest(url: prelaunchProjectURL)
+      guard let google = URL(string: "https://www.google.com") else {
+        XCTFail("Should have a URL")
+        return
+      }
+
+      let request1 = URLRequest(url: google)
+
+      let navigationAction1 = WKNavigationActionData(
+        navigationType: .other,
+        request: request1,
+        sourceFrame: WKFrameInfoData(mainFrame: true, request: request1),
+        targetFrame: WKFrameInfoData(mainFrame: true, request: request1)
+      )
 
-      let navigationAction = WKNavigationActionData(
+      XCTAssertEqual(
+        WKNavigationActionPolicy.allow.rawValue,
+        self.vm.inputs.decidePolicyFor(navigationAction: navigationAction1).rawValue
+      )
+
+      self.scheduler.run()
+
+      self.goToProject.assertDidNotEmitValue()
+      self.goToComments.assertDidNotEmitValue()
+      self.webViewLoadRequest.assertValueCount(1, "Initial update load request")
+      self.goToSafariBrowser.assertValues([])
+
+      XCTAssertEqual([], self.trackingClient.events)
+      XCTAssertEqual(nil, self.trackingClient.properties.last?["context"] as? String)
+
+      let request2 = URLRequest(url: prelaunchProjectURL)
+
+      let navigationAction2 = WKNavigationActionData(
         navigationType: .linkActivated,
-        request: request,
-        sourceFrame: WKFrameInfoData(mainFrame: true, request: request),
-        targetFrame: WKFrameInfoData(mainFrame: true, request: request)
+        request: request2,
+        sourceFrame: WKFrameInfoData(mainFrame: true, request: request2),
+        targetFrame: WKFrameInfoData(mainFrame: true, request: request2)
       )
 
       XCTAssertEqual(
         WKNavigationActionPolicy.cancel.rawValue,
-        self.vm.inputs.decidePolicyFor(navigationAction: navigationAction).rawValue
+        self.vm.inputs.decidePolicyFor(navigationAction: navigationAction2).rawValue
       )
 
       self.scheduler.run()