VPN ship review changes (#2630)

Task/Issue URL: https://app.asana.com/0/414235014887631/1206914003023279/f Tech Design URL: CC: Description: This PR fixes a number of ship review issues: The option to show the VPN screen after purchase was pushing the VC multiple times - this is now fixed The VPN is now fully removed after entitlements expire The VPN disconnected notification is now only displayed if the VPN is installed This PR also pulls in a crash fix from @miasma13 It also cleans up some waitlist code that is no longer really desired in the app.
duckduckgo · Mar 25, 2024 · d0ee9c3 · d0ee9c3
1 parent c5550c5
commit d0ee9c3
Show file tree

Hide file tree

Showing 10 changed files with 50 additions and 53 deletions.
diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj
@@ -10033,7 +10033,7 @@
 			repositoryURL = "https://github.com/DuckDuckGo/BrowserServicesKit";
 			requirement = {
 				kind = exactVersion;
-				version = 129.1.8;
+				version = 129.2.0;
 			};
 		};
 		B6F997C22B8F374300476735 /* XCRemoteSwiftPackageReference "apple-toolbox" */ = {

diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -32,8 +32,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/DuckDuckGo/BrowserServicesKit",
       "state" : {
-        "revision" : "0509e0cf15f1ff8b0f32ac39c262da131fba7f3e",
-        "version" : "129.1.8"
+        "revision" : "284c328a097132a12e8abcf94d8f4d369063dcb4",
+        "version" : "129.2.0"
       }
     },
     {

diff --git a/DuckDuckGo/AppDelegate+Waitlists.swift b/DuckDuckGo/AppDelegate+Waitlists.swift
@@ -37,7 +37,7 @@ extension AppDelegate {
     func checkWaitlists() {
 
 #if NETWORK_PROTECTION
-        if vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist() {
+        if vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist() {
             checkNetworkProtectionWaitlist()
         }
 #endif
@@ -54,35 +54,7 @@ extension AppDelegate {
 
         VPNWaitlist.shared.fetchInviteCodeIfAvailable { [weak self] error in
             guard error == nil else {
-
-                if error == .alreadyHasInviteCode, UIApplication.shared.applicationState == .active {
-                    // If the user already has an invite code but their auth token has gone missing, attempt to redeem it again.
-                    let tokenStore = NetworkProtectionKeychainTokenStore()
-                    let waitlistStorage = VPNWaitlist.shared.waitlistStorage
-                    let configManager = ContentBlocking.shared.privacyConfigurationManager
-                    let waitlistBetaActive = configManager.privacyConfig.isSubfeatureEnabled(NetworkProtectionSubfeature.waitlistBetaActive)
-
-                    if let inviteCode = waitlistStorage.getWaitlistInviteCode(),
-                       !tokenStore.isFeatureActivated,
-                       waitlistBetaActive {
-                        let pixel: Pixel.Event = .networkProtectionWaitlistRetriedInviteCodeRedemption
-
-                        do {
-                            if let token = try tokenStore.fetchToken() {
-                                DailyPixel.fireDailyAndCount(pixel: pixel, withAdditionalParameters: [ "tokenState": "found" ])
-                            } else {
-                                DailyPixel.fireDailyAndCount(pixel: pixel, withAdditionalParameters: [ "tokenState": "nil" ])
-                            }
-                        } catch {
-                            DailyPixel.fireDailyAndCount(pixel: pixel, error: error, withAdditionalParameters: [ "tokenState": "error" ])
-                        }
-
-                        self?.fetchVPNWaitlistAuthToken(inviteCode: inviteCode)
-                    }
-                }
-
                 return
-
             }
 
             guard let inviteCode = VPNWaitlist.shared.waitlistStorage.getWaitlistInviteCode() else {
@@ -95,7 +67,7 @@ extension AppDelegate {
 #endif
 
     private func checkWaitlistBackgroundTasks() {
-        guard vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist() else { return }
+        guard vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist() else { return }
 
         BGTaskScheduler.shared.getPendingTaskRequests { tasks in
 

diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift
@@ -75,7 +75,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
 #if NETWORK_PROTECTION
     private let widgetRefreshModel = NetworkProtectionWidgetRefreshModel()
     private let tunnelDefaults = UserDefaults.networkProtectionGroupDefaults
-    lazy var vpnFeatureVisibilty = DefaultNetworkProtectionVisibility()
+    lazy var vpnFeatureVisibility = DefaultNetworkProtectionVisibility()
 #endif
 
     private var autoClear: AutoClear?
@@ -304,7 +304,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
         AppConfigurationFetch.registerBackgroundRefreshTaskHandler()
 
 #if NETWORK_PROTECTION
-        if vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist() {
+        if vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist() {
             VPNWaitlist.shared.registerBackgroundRefreshTaskHandler()
         }
 #endif
@@ -335,7 +335,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
         setupSubscriptionsEnvironment()
 #endif
 
-        if vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist() {
+        if vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist() {
             clearDebugWaitlistState()
         }
 
@@ -490,7 +490,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
 #if NETWORK_PROTECTION
         widgetRefreshModel.refreshVPNWidget()
 
-        if vpnFeatureVisibilty.shouldShowThankYouMessaging() && !tunnelDefaults.vpnEarlyAccessOverAlertAlreadyShown {
+        if vpnFeatureVisibility.shouldShowThankYouMessaging() && !tunnelDefaults.vpnEarlyAccessOverAlertAlreadyShown {
             presentVPNEarlyAccessOverAlert()
         }
 
@@ -817,7 +817,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
 
 #if NETWORK_PROTECTION
             if shortcutItem.type == ShortcutKey.openVPNSettings {
-                presentNetworkProtectionStatusSettingsModal()
+                let visibility = DefaultNetworkProtectionVisibility()
+                if visibility.shouldShowVPNShortcut() {
+                    presentNetworkProtectionStatusSettingsModal()
+                }
             }
 #endif
 
@@ -852,7 +855,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
 
     func refreshShortcuts() {
 #if NETWORK_PROTECTION
-        guard vpnFeatureVisibilty.shouldShowVPNShortcut() else {
+        guard vpnFeatureVisibility.shouldShowVPNShortcut() else {
             UIApplication.shared.shortcutItems = nil
             return
         }
@@ -929,7 +932,7 @@ extension AppDelegate: UNUserNotificationCenterDelegate {
                 presentNetworkProtectionStatusSettingsModal()
             }
 
-            if vpnFeatureVisibilty.shouldKeepVPNAccessViaWaitlist(), identifier == VPNWaitlist.notificationIdentifier {
+            if vpnFeatureVisibility.shouldKeepVPNAccessViaWaitlist(), identifier == VPNWaitlist.notificationIdentifier {
                 presentNetworkProtectionWaitlistModal()
                 DailyPixel.fire(pixel: .networkProtectionWaitlistNotificationLaunched)
             }

diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift
@@ -1427,10 +1427,14 @@ class MainViewController: UIViewController {
         Task {
             guard case .success(false) = await AccountManager().hasEntitlement(for: .networkProtection) else { return }
 
-            tunnelDefaults.enableEntitlementMessaging()
-
             let controller = NetworkProtectionTunnelController()
+
+            if await controller.isInstalled {
+                tunnelDefaults.enableEntitlementMessaging()
+            }
+
             await controller.stop()
+            await controller.removeVPN()
         }
     }
 

diff --git a/DuckDuckGo/NetworkProtectionAccessController.swift b/DuckDuckGo/NetworkProtectionAccessController.swift
@@ -143,7 +143,6 @@ struct NetworkProtectionAccessController: NetworkProtectionAccess {
     }
 
     func revokeNetworkProtectionAccess() {
-        networkProtectionWaitlistStorage.deleteWaitlistState()
         try? NetworkProtectionKeychainTokenStore().deleteToken()
 
         Task {

diff --git a/DuckDuckGo/NetworkProtectionFeatureVisibility.swift b/DuckDuckGo/NetworkProtectionFeatureVisibility.swift
@@ -18,6 +18,7 @@
 //
 
 import Foundation
+import Subscription
 
 public protocol NetworkProtectionFeatureVisibility {
     func isWaitlistBetaActive() -> Bool
@@ -49,8 +50,16 @@ public extension NetworkProtectionFeatureVisibility {
         !isPrivacyProLaunched() && isWaitlistBetaActive() && isWaitlistUser()
     }
 
-    // todo - https://app.asana.com/0/0/1206827703748771/f
     func shouldShowVPNShortcut() -> Bool {
-        isPrivacyProLaunched() || shouldKeepVPNAccessViaWaitlist()
+        if isPrivacyProLaunched() {
+#if SUBSCRIPTION
+            let accountManager = AccountManager()
+            return accountManager.isUserAuthenticated
+#else
+            return false
+#endif
+        } else {
+            return shouldKeepVPNAccessViaWaitlist()
+        }
     }
 }
diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift
@@ -85,6 +85,13 @@ final class NetworkProtectionTunnelController: TunnelController {
 
     // MARK: - Connection Status Querying
 
+    var isInstalled: Bool {
+        get async {
+            let tunnelManager = await loadTunnelManager()
+            return tunnelManager != nil
+        }
+    }
+
     /// Queries Network Protection to know if its VPN is connected.
     ///
     /// - Returns: `true` if the VPN is connected, connecting or reasserting, and `false` otherwise.

diff --git a/DuckDuckGo/SettingsView.swift b/DuckDuckGo/SettingsView.swift
@@ -97,10 +97,13 @@ struct SettingsView: View {
                     }
                 })
 
-        .onReceive(viewModel.$deepLinkTarget, perform: { link in
-            guard let link else { return }
+        .onReceive(viewModel.$deepLinkTarget.removeDuplicates(), perform: { link in
+            guard let link, link != self.deepLinkTarget else {
+                return
+            }
+
             self.deepLinkTarget = link
-            
+
             switch link.type {
             case .sheet:
                 DispatchQueue.main.async {

diff --git a/DuckDuckGoTests/NetworkProtectionFeatureVisibilityTests.swift b/DuckDuckGoTests/NetworkProtectionFeatureVisibilityTests.swift
@@ -60,28 +60,28 @@ final class NetworkProtectionFeatureVisibilityTests: XCTestCase {
         XCTAssertTrue(mockWithVPNAccess.shouldMonitorEntitlement())
         XCTAssertFalse(mockWithVPNAccess.shouldKeepVPNAccessViaWaitlist())
         XCTAssertTrue(mockWithVPNAccess.shouldShowThankYouMessaging())
-        XCTAssertTrue(mockWithVPNAccess.shouldShowVPNShortcut())
+        XCTAssertFalse(mockWithVPNAccess.shouldShowVPNShortcut())
 
         // Not current waitlist user -> Enforce entitlement check, no more VPN use, no thank-you
         let mockWithBetaActive = baseMock.adding([.isWaitlistBetaActive])
         XCTAssertTrue(mockWithBetaActive.shouldMonitorEntitlement())
         XCTAssertFalse(mockWithBetaActive.shouldKeepVPNAccessViaWaitlist())
         XCTAssertFalse(mockWithBetaActive.shouldShowThankYouMessaging())
-        XCTAssertTrue(mockWithBetaActive.shouldShowVPNShortcut())
+        XCTAssertFalse(mockWithBetaActive.shouldShowVPNShortcut())
 
         // Waitlist beta OFF, current waitlist user -> Show thank-you, enforce entitlement check, no more VPN use
         let mockWithBetaInactive = baseMock.adding([.isWaitlistUser])
         XCTAssertTrue(mockWithBetaInactive.shouldMonitorEntitlement())
         XCTAssertFalse(mockWithBetaInactive.shouldKeepVPNAccessViaWaitlist())
         XCTAssertTrue(mockWithBetaInactive.shouldShowThankYouMessaging())
-        XCTAssertTrue(mockWithBetaInactive.shouldShowVPNShortcut())
+        XCTAssertFalse(mockWithBetaInactive.shouldShowVPNShortcut())
 
-        // Waitlist beta OFF, not current wailist user -> Enforce entitlement check, nothing else
+        // Waitlist beta OFF, not current waitlist user -> Enforce entitlement check, nothing else
         let mockWithNothingElse = baseMock
         XCTAssertTrue(mockWithNothingElse.shouldMonitorEntitlement())
         XCTAssertFalse(mockWithNothingElse.shouldKeepVPNAccessViaWaitlist())
         XCTAssertFalse(mockWithNothingElse.shouldShowThankYouMessaging())
-        XCTAssertTrue(mockWithNothingElse.shouldShowVPNShortcut())
+        XCTAssertFalse(mockWithNothingElse.shouldShowVPNShortcut())
     }
 }
-Original file line number
+Diff line change
@@ Expand Up @@
         }
         func revokeNetworkProtectionAccess() {
-            networkProtectionWaitlistStorage.deleteWaitlistState()
             try? NetworkProtectionKeychainTokenStore().deleteToken()
             Task {
@@ Expand Down @@