[MBL-2855][2/2] Bypass pledge manager decision policy (#2645)

* Refactor the pledge manager native navigation so all cases are represented in an enum

* Add feature flag for bypassing the decision policy

* Allow unrecognized urls to render when bypass flag is on

* Add basic error handling

Author: ifosli

Library/RemoteConfig/RemoteConfigFeature+Helpers.swift

@@ -15,6 +15,10 @@ func featureEnabled(feature: RemoteConfigFeature) -> Bool {
   return false
 }
 
+public func featureBypassPledgeManagerDecisionPolicyEnabled() -> Bool {
+  return featureEnabled(feature: .bypassPledgeManagerDecisionPolicy)
+}
+
 public func featureEditPledgeOverTimeEnabled() -> Bool {
   return featureEnabled(feature: .editPledgeOverTimeEnabled)
 }

Library/RemoteConfig/RemoteConfigFeature.swift

@@ -1,6 +1,7 @@
 import Foundation
 
 public enum RemoteConfigFeature: String, CaseIterable {
+  case bypassPledgeManagerDecisionPolicy = "bypass_pledge_manager_decision_policy"
   case editPledgeOverTimeEnabled = "edit_pledge_over_time"
   case useKeychainForOAuthToken = "use_keychain_for_oauth_token"
   case pledgedProjectsOverviewV2Enabled = "pledged_projects_overview_v2"
@@ -12,6 +13,7 @@ public enum RemoteConfigFeature: String, CaseIterable {
 extension RemoteConfigFeature: CustomStringConvertible {
   public var description: String {
     switch self {
+    case .bypassPledgeManagerDecisionPolicy: return "Bypass Pledge Manager Decision Policy"
     case .editPledgeOverTimeEnabled: return "Edit Pledge Over Time"
     case .useKeychainForOAuthToken: return "Use Keychain for OAuth token"
     case .pledgedProjectsOverviewV2Enabled: return "Pledged Projects Overview V2"

Library/ViewModels/PledgeManagerWebViewModel.swift

@@ -1,3 +1,4 @@
+import FirebaseCrashlytics
 import KsApi
 import Prelude
 import ReactiveExtensions
@@ -145,13 +146,31 @@ public final class PledgeManagerWebViewModel: PledgeManagerWebViewModelType {
           return true
         }
 
+        // A supported request will be prepared by logic elsewhere in the class and loaded into
+        // the web view from there. Never allow the unprepared version of these to render.
         let request = action.request
+        if isSupportedRequest(request: request) {
+          return AppEnvironment.current.apiService.isPrepared(request: request)
+        }
 
-        if !AppEnvironment.current.apiService.isPrepared(request: request) {
+        // If the corresponding native navigation request exists,
+        // this request will be handled elsewhere.
+        if nativeNavigationRequestForURLRequest(request) != nil {
           return false
         }
 
-        return isSupportedRequest(request: request)
+        // Log unrecognized urls.
+        if let error = errorForUnrecognizedUrl(request: request) {
+          Crashlytics.crashlytics().record(error: error)
+        }
+
+        // Never show unsupported kickstarter navigation requests, since these
+        // can get the user into bad/weird states.
+        if isKickstarterRequest(request) {
+          return false
+        }
+
+        return featureBypassPledgeManagerDecisionPolicyEnabled()
       }
       .map { $0 ? .allow : .cancel }
 
@@ -218,6 +237,11 @@ private func nativeNavigationRequestForURLRequest(_ request: URLRequest)
   }
 }
 
+private func isKickstarterRequest(_ request: URLRequest) -> Bool {
+  guard let host = request.url?.host() else { return false }
+  return host == AppEnvironment.current.apiService.serverConfig.webBaseUrl.host()
+}
+
 private func isUnpreparedSupportedRequest(request: URLRequest) -> Bool {
   guard !AppEnvironment.current.apiService.isPrepared(request: request) else { return false }
   return isSupportedRequest(request: request)
@@ -247,3 +271,35 @@ private func isStripeHost(_ host: String) -> Bool {
   let withoutSubdomain = host.lowercased().split(separator: ".").suffix(2).joined(separator: ".")
   return stripeDomains.contains(withoutSubdomain)
 }
+
+private func errorForUnrecognizedUrl(request: URLRequest) -> NSError? {
+  let errorDomain = "Kickstarter.PledgeManagerWebView"
+  enum ErrorCode: Int {
+    case unhandledKickstarterUrl = 1
+    case unrecognizedUrl = 2
+  }
+
+  // If there's no url present, don't log an error. The "about:blank" happens
+  // every time the web view loads, so logging these would be too noisy.
+  guard let url = request.url, url.absoluteString != "about:blank" else {
+    return nil
+  }
+
+  if isKickstarterRequest(request) {
+    return NSError(
+      domain: errorDomain,
+      code: ErrorCode.unhandledKickstarterUrl.rawValue,
+      userInfo: [
+        NSLocalizedDescriptionKey: "Found unhandled kickstarter url"
+      ]
+    )
+  }
+
+  return NSError(
+    domain: errorDomain,
+    code: ErrorCode.unrecognizedUrl.rawValue,
+    userInfo: [
+      NSLocalizedDescriptionKey: "Found unrecongnized url request with host: \(url.host() ?? "Unknown")"
+    ]
+  )
+}

Library/ViewModels/PledgeManagerWebViewModelTests.swift

@@ -267,6 +267,21 @@ final class PledgeManagerWebViewModelTests: TestCase {
 
   // MARK: - Decision policy tests
 
+  func testDecisionPolicyBypass() {
+    let mockConfigClient = MockRemoteConfigClient()
+    mockConfigClient.features = [
+      RemoteConfigFeature.bypassPledgeManagerDecisionPolicy.rawValue: true
+    ]
+
+    withEnvironment(remoteConfigClient: mockConfigClient) {
+      let navigationData = navigationData("https://www.fake.com/unrecognized-url")
+      XCTAssertEqual(
+        self.vm.decidePolicyFor(navigationAction: navigationData),
+        WKNavigationActionPolicy.allow
+      )
+    }
+  }
+
   func testBadRequest() {
     let navigationData = navigationData("https://www.fake.com/bad-url")
     XCTAssertEqual(

Library/ViewModels/RemoteConfigFeatureFlagToolsViewModelTests.swift

@@ -60,47 +60,50 @@ final class RemoteConfigFlagToolsViewModelTests: TestCase {
     }
   }
 
-  func testUpdateUserDefaultsWithFeature_FeatureIsEnabled() {
+  func testUpdateUserDefaultsWithFeature_FeatureIsDisabledAndEnabled() {
     let mockRemoteConfigClient = MockRemoteConfigClient()
-      |> \.features .~ [
-        RemoteConfigFeature.editPledgeOverTimeEnabled.rawValue: false
-      ]
 
     withEnvironment(remoteConfigClient: mockRemoteConfigClient) {
       self.vm.inputs.viewDidLoad()
-
       self.scheduler.advance()
-
       self.updateUserDefaultsWithFeatures.assertDidNotEmitValue()
 
-      self.vm.inputs.setFeatureAtIndexEnabled(index: 0, isEnabled: true)
-
+      self.vm.inputs.setFeatureAtIndexEnabled(index: 0, isEnabled: false)
       self.scheduler.advance()
+      let (_, isEnabled1) = self.updateUserDefaultsWithFeatures.lastValue![0]
+      XCTAssertFalse(isEnabled1)
 
-      let (_, isEnabled1) = self.updateUserDefaultsWithFeatures.values[0][0]
-
-      XCTAssertTrue(isEnabled1)
+      self.vm.inputs.setFeatureAtIndexEnabled(index: 0, isEnabled: true)
+      self.scheduler.advance()
+      let (_, isEnabled2) = self.updateUserDefaultsWithFeatures.lastValue![0]
+      XCTAssertTrue(isEnabled2)
     }
   }
 
   func testUpdateUserDefaultsWithFeatures_ReloadWithData_UserDefaultsIsUpdated() {
+    let feature = RemoteConfigFeature.editPledgeOverTimeEnabled
     let mockRemoteConfigClient = MockRemoteConfigClient()
       |> \.features .~ [
-        RemoteConfigFeature.editPledgeOverTimeEnabled.rawValue: false
+        feature.rawValue: false
       ]
 
     withEnvironment(remoteConfigClient: mockRemoteConfigClient, userDefaults: userDefaults) {
       self.vm.inputs.viewDidLoad()
 
       self.scheduler.advance()
 
+      guard let index = (self.reloadWithData.lastValue?.firstIndex { $0.0 == feature }) else {
+        XCTFail("Expected to find feature \(feature) in the list of features.")
+        return
+      }
+
       XCTAssertEqual(
         userDefaults
           .dictionary(forKey: "com.kickstarter.KeyValueStoreType.remoteConfigFeatureFlags") as? [String: Bool],
         nil
       )
 
-      self.vm.inputs.setFeatureAtIndexEnabled(index: 0, isEnabled: true)
+      self.vm.inputs.setFeatureAtIndexEnabled(index: index, isEnabled: true)
 
       self.scheduler.advance()
 
@@ -112,7 +115,7 @@ final class RemoteConfigFlagToolsViewModelTests: TestCase {
         userDefaults
           .dictionary(forKey: "com.kickstarter.KeyValueStoreType.remoteConfigFeatureFlags") as? [String: Bool],
         [
-          RemoteConfigFeature.editPledgeOverTimeEnabled.rawValue: true
+          feature.rawValue: true
         ]
       )
     }