From fb1e132c664f973e1c0acd88e6c609c3c6b00b14 Mon Sep 17 00:00:00 2001 From: Diego Rey Mendez Date: Tue, 9 Apr 2024 20:43:37 +0200 Subject: [PATCH] Hotfixing an issue with VPN visibility for certain waitlist users (#2591) Task/Issue URL: https://app.asana.com/0/0/1207040067636855/f Description Fixes an issue where some users were seeing waitlist UI when the subscription was avilable. --- .../View/NavigationBarViewController.swift | 30 ++++++++++++------- .../NetworkProtectionFeatureDisabler.swift | 4 ++- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift index 2148a66473..fbf3cc1ae8 100644 --- a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift +++ b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift @@ -338,18 +338,26 @@ final class NavigationBarViewController: NSViewController { } #endif - // 1. If the user is on the waitlist but hasn't been invited or accepted terms and conditions, show the waitlist screen. - // 2. If the user has no waitlist state but has an auth token, show the NetP popover. - // 3. If the user has no state of any kind, show the waitlist screen. - - if NetworkProtectionWaitlist().shouldShowWaitlistViewController { - NetworkProtectionWaitlistViewControllerPresenter.show() - DailyPixel.fire(pixel: .networkProtectionWaitlistIntroDisplayed, frequency: .dailyAndCount) - } else if NetworkProtectionKeychainTokenStore().isFeatureActivated { - popovers.toggleNetworkProtectionPopover(usingView: networkProtectionButton, withDelegate: networkProtectionButtonModel) + // Note: the following code is quite contrived but we're aiming to hotfix issues without mixing subscription and + // waitlist logic. This should be cleaned up once waitlist can safely be removed. + + if DefaultSubscriptionFeatureAvailability().isFeatureAvailable { + if NetworkProtectionKeychainTokenStore().isFeatureActivated { + popovers.toggleNetworkProtectionPopover(usingView: networkProtectionButton, withDelegate: networkProtectionButtonModel) + } } else { - NetworkProtectionWaitlistViewControllerPresenter.show() - DailyPixel.fire(pixel: .networkProtectionWaitlistIntroDisplayed, frequency: .dailyAndCount) + // 1. If the user is on the waitlist but hasn't been invited or accepted terms and conditions, show the waitlist screen. + // 2. If the user has no waitlist state but has an auth token, show the NetP popover. + // 3. If the user has no state of any kind, show the waitlist screen. + + if NetworkProtectionWaitlist().shouldShowWaitlistViewController { + NetworkProtectionWaitlistViewControllerPresenter.show() + DailyPixel.fire(pixel: .networkProtectionWaitlistIntroDisplayed, frequency: .dailyAndCount) + } else if NetworkProtectionKeychainTokenStore().isFeatureActivated { + popovers.toggleNetworkProtectionPopover(usingView: networkProtectionButton, withDelegate: networkProtectionButtonModel) + } else { + NetworkProtectionWaitlistViewControllerPresenter.show() + } } } #endif diff --git a/DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift b/DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift index ba12c9cdd2..d7cc337991 100644 --- a/DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift +++ b/DuckDuckGo/Waitlist/NetworkProtectionFeatureDisabler.swift @@ -76,6 +76,9 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling @MainActor @discardableResult func disable(keepAuthToken: Bool, uninstallSystemExtension: Bool) async -> Bool { + // We can do this optimistically as it has little if any impact. + unpinNetworkProtection() + // To disable NetP we need the login item to be running // This should be fine though as we'll disable them further down below guard canUninstall(includingSystemExtension: uninstallSystemExtension) else { @@ -85,7 +88,6 @@ final class NetworkProtectionFeatureDisabler: NetworkProtectionFeatureDisabling isDisabling = true defer { - unpinNetworkProtection() resetUserDefaults(uninstallSystemExtension: uninstallSystemExtension) }