From b2a7f389ac101253ab686fa7d9aeadcad0d0a284 Mon Sep 17 00:00:00 2001 From: Pete Smith <5278441+aataraxiaa@users.noreply.github.com> Date: Wed, 4 Sep 2024 10:54:23 +0200 Subject: [PATCH] Freemium PIR: Refactor Freemium PIR State to Remove Dependency on AccountManager (#3197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/1201621853593513/1208197280287450/f CC: @miasma13 **Description**: As an outcome of [this](https://github.com/duckduckgo/macos-browser/pull/3178#discussion_r1740054714) discussion, I have refactored `FreemiumPIRUserStateManager` to remove it’s dependency on `AccountManager`. This includes the removal of the `isActiveUser` computed property. --- DuckDuckGo/Application/AppDelegate.swift | 4 +- ...ataBrokerProtectionFeatureGatekeeper.swift | 7 ++- DuckDuckGo/Freemium/FreemiumDebugMenu.swift | 4 +- .../View/NavigationBarViewController.swift | 2 +- ...RemoteMessagingConfigMatcherProvider.swift | 5 +- .../Tab/View/BrowserTabViewController.swift | 2 +- .../DataBrokerProtectionAgentManager.swift | 14 +++-- .../DataBrokerProtectionAgentStopper.swift | 16 ++++-- ...ataBrokerProtectionAgentManagerTests.swift | 38 +++++++++---- ...ataBrokerProtectionAgentStopperTests.swift | 36 ++++++------- .../DataBrokerProtectionTests/Mocks.swift | 1 - LocalPackages/Freemium/Package.swift | 15 +----- .../FreemiumPIRUserStateManager.swift | 19 ++----- .../FreemiumPIRUserStateManagerTests.swift | 53 +------------------ ...okerProtectionFeatureGatekeeperTests.swift | 14 ++--- .../RemoteMessagingClientTests.swift | 1 - 16 files changed, 94 insertions(+), 137 deletions(-) diff --git a/DuckDuckGo/Application/AppDelegate.swift b/DuckDuckGo/Application/AppDelegate.swift index a0a50b37ac..65d7875891 100644 --- a/DuckDuckGo/Application/AppDelegate.swift +++ b/DuckDuckGo/Application/AppDelegate.swift @@ -380,7 +380,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { dataBrokerProtectionSubscriptionEventHandler.registerForSubscriptionAccountManagerEvents() - let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp, accountManager: subscriptionManager.accountManager) + let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp) let pirGatekeeper = DefaultDataBrokerProtectionFeatureGatekeeper(accountManager: subscriptionManager.accountManager, freemiumPIRUserStateManager: freemiumPIRUserStateManager) @@ -432,7 +432,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { NetworkProtectionAppEvents(featureGatekeeper: DefaultVPNFeatureGatekeeper(subscriptionManager: subscriptionManager)).applicationDidBecomeActive() - let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp, accountManager: subscriptionManager.accountManager) + let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp) let pirGatekeeper = DefaultDataBrokerProtectionFeatureGatekeeper(accountManager: subscriptionManager.accountManager, freemiumPIRUserStateManager: freemiumPIRUserStateManager) diff --git a/DuckDuckGo/DBP/DataBrokerProtectionFeatureGatekeeper.swift b/DuckDuckGo/DBP/DataBrokerProtectionFeatureGatekeeper.swift index d0705f32b5..756e0b08b1 100644 --- a/DuckDuckGo/DBP/DataBrokerProtectionFeatureGatekeeper.swift +++ b/DuckDuckGo/DBP/DataBrokerProtectionFeatureGatekeeper.swift @@ -81,13 +81,14 @@ struct DefaultDataBrokerProtectionFeatureGatekeeper: DataBrokerProtectionFeature /// Checks PIR prerequisites /// /// Prerequisites are satisified if either: - /// 1. The user is an active freemium user + /// 1. The user is an active freemium user (e.g has onboarded to freemium and is not authenticated) /// 2. The user has a subscription with valid entitlements /// /// - Returns: Bool indicating prerequisites are satisfied func arePrerequisitesSatisfied() async -> Bool { - if freemiumPIRUserStateManager.isActiveUser { return true } + let isAuthenticated = accountManager.isUserAuthenticated + if !isAuthenticated && freemiumPIRUserStateManager.didOnboard { return true } let entitlements = await accountManager.hasEntitlement(forProductName: .dataBrokerProtection, cachePolicy: .reloadIgnoringLocalCacheData) @@ -99,8 +100,6 @@ struct DefaultDataBrokerProtectionFeatureGatekeeper: DataBrokerProtectionFeature hasEntitlements = false } - let isAuthenticated = accountManager.accessToken != nil - firePrerequisitePixelsAndLogIfNecessary(hasEntitlements: hasEntitlements, isAuthenticatedResult: isAuthenticated) return hasEntitlements && isAuthenticated diff --git a/DuckDuckGo/Freemium/FreemiumDebugMenu.swift b/DuckDuckGo/Freemium/FreemiumDebugMenu.swift index 7e46510a4f..c8604f104c 100644 --- a/DuckDuckGo/Freemium/FreemiumDebugMenu.swift +++ b/DuckDuckGo/Freemium/FreemiumDebugMenu.swift @@ -42,11 +42,11 @@ final class FreemiumDebugMenu: NSMenuItem { @objc func setFreemiumPIROnboardStateEnabled() { - DefaultFreemiumPIRUserStateManager(userDefaults: .dbp, accountManager: Application.appDelegate.subscriptionManager.accountManager).didOnboard = true + DefaultFreemiumPIRUserStateManager(userDefaults: .dbp).didOnboard = true } @objc func setFreemiumPIROnboardStateDisabled() { - DefaultFreemiumPIRUserStateManager(userDefaults: .dbp, accountManager: Application.appDelegate.subscriptionManager.accountManager).didOnboard = false + DefaultFreemiumPIRUserStateManager(userDefaults: .dbp).didOnboard = false } } diff --git a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift index ea0df377e4..8e04e6d0d9 100644 --- a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift +++ b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift @@ -286,7 +286,7 @@ final class NavigationBarViewController: NSViewController { @IBAction func optionsButtonAction(_ sender: NSButton) { let internalUserDecider = NSApp.delegateTyped.internalUserDecider - let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp, accountManager: subscriptionManager.accountManager) + let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp) let freemiumPIRFeature = DefaultFreemiumPIRFeature(subscriptionManager: subscriptionManager, accountManager: subscriptionManager.accountManager) let menu = MoreOptionsMenu(tabCollectionViewModel: tabCollectionViewModel, passwordManagerCoordinator: PasswordManagerCoordinator.shared, diff --git a/DuckDuckGo/RemoteMessaging/RemoteMessagingConfigMatcherProvider.swift b/DuckDuckGo/RemoteMessaging/RemoteMessagingConfigMatcherProvider.swift index 21298d2df7..fbfbdd828f 100644 --- a/DuckDuckGo/RemoteMessaging/RemoteMessagingConfigMatcherProvider.swift +++ b/DuckDuckGo/RemoteMessaging/RemoteMessagingConfigMatcherProvider.swift @@ -137,7 +137,8 @@ final class RemoteMessagingConfigMatcherProvider: RemoteMessagingConfigMatcherPr let deprecatedRemoteMessageStorage = DefaultSurveyRemoteMessagingStorage.surveys() - let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp, accountManager: subscriptionManager.accountManager) + let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp) + let isCurrentFreemiumPIRUser = !subscriptionManager.accountManager.isUserAuthenticated && freemiumPIRUserStateManager.didOnboard return RemoteMessagingConfigMatcher( appAttributeMatcher: AppAttributeMatcher(statisticsStore: statisticsStore, @@ -164,7 +165,7 @@ final class RemoteMessagingConfigMatcherProvider: RemoteMessagingConfigMatcherPr hasCustomHomePage: startupPreferencesPersistor().launchToCustomHomePage, isDuckPlayerOnboarded: duckPlayerPreferencesPersistor.youtubeOverlayAnyButtonPressed, isDuckPlayerEnabled: duckPlayerPreferencesPersistor.duckPlayerModeBool != false, - isCurrentFreemiumPIRUser: freemiumPIRUserStateManager.isActiveUser, + isCurrentFreemiumPIRUser: isCurrentFreemiumPIRUser, dismissedDeprecatedMacRemoteMessageIds: deprecatedRemoteMessageStorage.dismissedMessageIDs() ), percentileStore: RemoteMessagingPercentileUserDefaultsStore(keyValueStore: UserDefaults.standard), diff --git a/DuckDuckGo/Tab/View/BrowserTabViewController.swift b/DuckDuckGo/Tab/View/BrowserTabViewController.swift index d4b059e094..b3dcd6082a 100644 --- a/DuckDuckGo/Tab/View/BrowserTabViewController.swift +++ b/DuckDuckGo/Tab/View/BrowserTabViewController.swift @@ -791,7 +791,7 @@ final class BrowserTabViewController: NSViewController { return homePageViewController ?? { let subscriptionManager = Application.appDelegate.subscriptionManager let freemiumPIRFeature = DefaultFreemiumPIRFeature(subscriptionManager: subscriptionManager, accountManager: subscriptionManager.accountManager) - let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp, accountManager: subscriptionManager.accountManager) + let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp) let homePageViewController = HomePageViewController(tabCollectionViewModel: tabCollectionViewModel, bookmarkManager: bookmarkManager, freemiumPIRFeature: freemiumPIRFeature, freemiumPIRUserStateManager: freemiumPIRUserStateManager) diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift index 74a78e0de0..884b577a0e 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Scheduler/DataBrokerProtectionAgentManager.swift @@ -77,7 +77,7 @@ public class DataBrokerProtectionAgentManagerProvider { emailService: emailService, captchaService: captchaService) - let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp, accountManager: accountManager) + let freemiumPIRUserStateManager = DefaultFreemiumPIRUserStateManager(userDefaults: .dbp) let agentstopper = DefaultDataBrokerProtectionAgentStopper(dataManager: dataManager, entitlementMonitor: DataBrokerProtectionEntitlementMonitor(), @@ -102,6 +102,7 @@ public class DataBrokerProtectionAgentManagerProvider { operationDependencies: operationDependencies, pixelHandler: pixelHandler, agentStopper: agentstopper, + authenticationManager: authenticationManager, freemiumPIRUserStateManager: freemiumPIRUserStateManager) } } @@ -116,6 +117,7 @@ public final class DataBrokerProtectionAgentManager { private let operationDependencies: DataBrokerOperationDependencies private let pixelHandler: EventMapping private let agentStopper: DataBrokerProtectionAgentStopper + private let authenticationManager: DataBrokerProtectionAuthenticationManaging private let freemiumPIRUserStateManager: FreemiumPIRUserStateManager // Used for debug functions only, so not injected @@ -131,6 +133,7 @@ public final class DataBrokerProtectionAgentManager { operationDependencies: DataBrokerOperationDependencies, pixelHandler: EventMapping, agentStopper: DataBrokerProtectionAgentStopper, + authenticationManager: DataBrokerProtectionAuthenticationManaging, freemiumPIRUserStateManager: FreemiumPIRUserStateManager ) { self.userNotificationService = userNotificationService @@ -141,6 +144,7 @@ public final class DataBrokerProtectionAgentManager { self.operationDependencies = operationDependencies self.pixelHandler = pixelHandler self.agentStopper = agentStopper + self.authenticationManager = authenticationManager self.freemiumPIRUserStateManager = freemiumPIRUserStateManager self.activityScheduler.delegate = self @@ -191,7 +195,7 @@ extension DataBrokerProtectionAgentManager { private extension DataBrokerProtectionAgentManager { - /// Starts either Freemium (scan-only) or Subscription (scan and opt-out) scheduled operations + /// Starts either Subscription (scan and opt-out) or Freemium (scan-only) scheduled operations /// - Parameters: /// - showWebView: Whether to show the web view or not /// - operationDependencies: Operation dependencies @@ -199,10 +203,10 @@ private extension DataBrokerProtectionAgentManager { func startFreemiumOrSubscriptionScheduledOperations(showWebView: Bool, operationDependencies: DataBrokerOperationDependencies, completion: ((DataBrokerProtectionAgentErrorCollection?) -> Void)?) { - if freemiumPIRUserStateManager.isActiveUser { - queueManager.startScheduledScanOperationsIfPermitted(showWebView: showWebView, operationDependencies: operationDependencies, completion: completion) - } else { + if authenticationManager.isUserAuthenticated { queueManager.startScheduledAllOperationsIfPermitted(showWebView: showWebView, operationDependencies: operationDependencies, completion: completion) + } else { + queueManager.startScheduledScanOperationsIfPermitted(showWebView: showWebView, operationDependencies: operationDependencies, completion: completion) } } } diff --git a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift index a7ae9be234..42edf8f3a5 100644 --- a/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift +++ b/LocalPackages/DataBrokerProtection/Sources/DataBrokerProtection/Utils/DataBrokerProtectionAgentStopper.swift @@ -62,16 +62,15 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper do { let hasProfile = try dataManager.fetchProfile() != nil let isAuthenticated = authenticationManager.isUserAuthenticated - let isFreemium = freemiumPIRUserStateManager.isActiveUser + let didOnboardToFreemium = freemiumPIRUserStateManager.didOnboard - if !hasProfile || (!isAuthenticated && !isFreemium) { + if !hasProfile || (!isAuthenticated && !didOnboardToFreemium) { Logger.dataBrokerProtection.debug("Prerequisites are invalid") stopAgent() return } - - if !isAuthenticated && isFreemium { + if satisfiesFreemiumPrerequisites() { Logger.dataBrokerProtection.debug("User is Freemium") return } @@ -88,11 +87,18 @@ struct DefaultDataBrokerProtectionAgentStopper: DataBrokerProtectionAgentStopper public func monitorEntitlementAndStopAgentIfEntitlementIsInvalidAndUserIsNotFreemium(interval: TimeInterval) { entitlementMonitor.start(checkEntitlementFunction: authenticationManager.hasValidEntitlement, interval: interval) { result in - guard !self.freemiumPIRUserStateManager.isActiveUser else { return } + + if satisfiesFreemiumPrerequisites() { return } stopAgentBasedOnEntitlementCheckResult(result) } } + private func satisfiesFreemiumPrerequisites() -> Bool { + let isAuthenticated = authenticationManager.isUserAuthenticated + let didOnboardToFreemium = freemiumPIRUserStateManager.didOnboard + return !isAuthenticated && didOnboardToFreemium + } + private func stopAgent() { stopAction.stopAgent() } diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift index 8f79b47a55..e488134321 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentManagerTests.swift @@ -32,6 +32,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { private var mockDependencies: DefaultDataBrokerOperationDependencies! private var mockProfile: DataBrokerProtectionProfile! private var mockAgentStopper: MockAgentStopper! + private var mockAuthenticationManager: MockAuthenticationManager! private var mockFreemiumPIRUserState: MockFreemiumPIRUserState! override func setUpWithError() throws { @@ -39,6 +40,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { mockPixelHandler = MockPixelHandler() mockActivityScheduler = MockDataBrokerProtectionBackgroundActivityScheduler() mockNotificationService = MockUserNotificationService() + mockAuthenticationManager = MockAuthenticationManager() mockAgentStopper = MockAgentStopper() let mockDatabase = MockDatabase() @@ -82,10 +84,12 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockDataManager.profileToReturn = mockProfile - mockFreemiumPIRUserState.isActiveUser = false + mockAuthenticationManager.isUserAuthenticatedValue = true + mockFreemiumPIRUserState.didOnboard = false let schedulerStartedExpectation = XCTestExpectation(description: "Scheduler started") var schedulerStarted = false @@ -121,10 +125,11 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockDataManager.profileToReturn = mockProfile - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let schedulerStartedExpectation = XCTestExpectation(description: "Scheduler started") var schedulerStarted = false @@ -166,10 +171,11 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: agentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockDataManager.profileToReturn = nil - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let stopAgentExpectation = XCTestExpectation(description: "Stop agent expectation") @@ -200,6 +206,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockDataManager.profileToReturn = nil @@ -238,10 +245,12 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockDataManager.profileToReturn = mockProfile - mockFreemiumPIRUserState.isActiveUser = false + mockAuthenticationManager.isUserAuthenticatedValue = true + mockFreemiumPIRUserState.didOnboard = false var startScheduledScansCalled = false mockQueueManager.startScheduledAllOperationsIfPermittedCalledCompletion = { _ in @@ -266,10 +275,11 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockDataManager.profileToReturn = mockProfile - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true var startScheduledScansCalled = false mockQueueManager.startScheduledScanOperationsIfPermittedCalledCompletion = { _ in @@ -294,10 +304,11 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockDataManager.profileToReturn = mockProfile - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false var startImmediateScansCalled = false mockQueueManager.startImmediateScanOperationsIfPermittedCalledCompletion = { _ in @@ -322,10 +333,11 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockDataManager.profileToReturn = mockProfile - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true var startImmediateScansCalled = false mockQueueManager.startImmediateScanOperationsIfPermittedCalledCompletion = { _ in @@ -350,6 +362,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockNotificationService.reset() @@ -372,6 +385,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockNotificationService.reset() @@ -394,6 +408,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockNotificationService.reset() @@ -417,6 +432,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockNotificationService.reset() @@ -440,6 +456,7 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) mockNotificationService.reset() @@ -463,9 +480,11 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) - mockFreemiumPIRUserState.isActiveUser = false + mockAuthenticationManager.isUserAuthenticatedValue = true + mockFreemiumPIRUserState.didOnboard = false var startScheduledScansCalled = false mockQueueManager.startScheduledAllOperationsIfPermittedCalledCompletion = { _ in @@ -490,9 +509,10 @@ final class DataBrokerProtectionAgentManagerTests: XCTestCase { operationDependencies: mockDependencies, pixelHandler: mockPixelHandler, agentStopper: mockAgentStopper, + authenticationManager: mockAuthenticationManager, freemiumPIRUserStateManager: mockFreemiumPIRUserState) - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true var startScheduledScansCalled = false mockQueueManager.startScheduledScanOperationsIfPermittedCalledCompletion = { _ in diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift index a605d74d6d..ff91b82ff5 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/DataBrokerProtectionAgentStopperTests.swift @@ -47,7 +47,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { fakeBrokerFlag: DataBrokerDebugFlagFakeBroker()) mockStopAction = MockDataProtectionStopAction() mockFreemiumPIRUserState = MockFreemiumPIRUserState() - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false } override func tearDown() { @@ -63,7 +63,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = false mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = nil - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -80,7 +80,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = false mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = nil - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -97,7 +97,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = nil - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -114,7 +114,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = nil - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -131,7 +131,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = false mockAuthenticationManager.hasValidEntitlementValue = false mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -148,7 +148,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = false mockAuthenticationManager.hasValidEntitlementValue = false mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -165,7 +165,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = false mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -182,7 +182,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = false mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -199,7 +199,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = false mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -216,7 +216,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = false mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -249,7 +249,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -266,7 +266,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -283,7 +283,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -307,7 +307,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = true mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -331,7 +331,7 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { mockAuthenticationManager.isUserAuthenticatedValue = true mockAuthenticationManager.hasValidEntitlementValue = false mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, @@ -352,10 +352,10 @@ final class DataBrokerProtectionAgentStopperTests: XCTestCase { } func testEntitlementMonitorWithInValidResult_andUserIsFreemium_thenStopAgentIsNotCalled() { - mockAuthenticationManager.isUserAuthenticatedValue = true + mockAuthenticationManager.isUserAuthenticatedValue = false mockAuthenticationManager.hasValidEntitlementValue = false mockDataManager.profileToReturn = fakeProfile - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true let stopper = DefaultDataBrokerProtectionAgentStopper(dataManager: mockDataManager, entitlementMonitor: mockEntitlementMonitor, diff --git a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift index a8dbae850b..f74d93f5c9 100644 --- a/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift +++ b/LocalPackages/DataBrokerProtection/Tests/DataBrokerProtectionTests/Mocks.swift @@ -1941,5 +1941,4 @@ struct MockMigrationsProvider: DataBrokerProtectionDatabaseMigrationsProvider { final class MockFreemiumPIRUserState: FreemiumPIRUserStateManager { var didOnboard = false - var isActiveUser = false } diff --git a/LocalPackages/Freemium/Package.swift b/LocalPackages/Freemium/Package.swift index 82d0e28d95..8f8f453dc1 100644 --- a/LocalPackages/Freemium/Package.swift +++ b/LocalPackages/Freemium/Package.swift @@ -28,22 +28,11 @@ let package = Package( name: "Freemium", targets: ["Freemium"]), ], - dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "186.0.0"), - ], targets: [ .target( - name: "Freemium", - dependencies: [ - .product(name: "Subscription", package: "BrowserServicesKit"), - ] - ), + name: "Freemium"), .testTarget( name: "FreemiumTests", - dependencies: [ - "Freemium", - .product(name: "SubscriptionTestingUtilities", package: "BrowserServicesKit") - ] - ), + dependencies: ["Freemium"]), ] ) diff --git a/LocalPackages/Freemium/Sources/Freemium/FreemiumPIRUserStateManager.swift b/LocalPackages/Freemium/Sources/Freemium/FreemiumPIRUserStateManager.swift index 315eab91b2..13edfc2377 100644 --- a/LocalPackages/Freemium/Sources/Freemium/FreemiumPIRUserStateManager.swift +++ b/LocalPackages/Freemium/Sources/Freemium/FreemiumPIRUserStateManager.swift @@ -17,14 +17,11 @@ // import Foundation -import Subscription /// `FreemiumPIRUserStateManager` types provide access to Freemium PIR-related state public protocol FreemiumPIRUserStateManager { var didOnboard: Bool { get set } - /// `isActiveUser` implementations`should only return `true` if the current user DOES NOT have a subscription - var isActiveUser: Bool { get } } /// Default implementation of `FreemiumPIRUserStateManager`. `UserDefaults` is used as underlying storage. @@ -35,7 +32,6 @@ public final class DefaultFreemiumPIRUserStateManager: FreemiumPIRUserStateManag } private let userDefaults: UserDefaults - private let accountManager: AccountManager private let key = "macos.browser.freemium.pir.did.onboard" public var didOnboard: Bool { @@ -46,21 +42,14 @@ public final class DefaultFreemiumPIRUserStateManager: FreemiumPIRUserStateManag } } - /// Logic is based on `didOnboard` && `accountManager.isUserAuthenticated` - /// A user can only be a current freemium user is they onboarded and DON'T have a subscription - public var isActiveUser: Bool { - didOnboard && !accountManager.isUserAuthenticated - } - /// Initializes a `DefaultFreemiumPIRState` instance - /// Note: The `UserDefaults` parameter will be used to get and set state values. If creating and accessing this type from + /// + /// Note 1: The `UserDefaults` parameter will be used to get and set state values. If creating and accessing this type from /// multiple places, you must ensure you always pass the same `UserDefaults` instance to get consistent results. + /// . /// - Parameters: /// - userDefaults: The `UserDefaults` parameter will be used to get and set state values. - /// - accountManager: the `AccountManager` parameter is used to check if a user has a privacy pro subscription - public init(userDefaults: UserDefaults, - accountManager: AccountManager) { + public init(userDefaults: UserDefaults) { self.userDefaults = userDefaults - self.accountManager = accountManager } } diff --git a/LocalPackages/Freemium/Tests/FreemiumTests/FreemiumPIRUserStateManagerTests.swift b/LocalPackages/Freemium/Tests/FreemiumTests/FreemiumPIRUserStateManagerTests.swift index d17f2e0676..29239a6803 100644 --- a/LocalPackages/Freemium/Tests/FreemiumTests/FreemiumPIRUserStateManagerTests.swift +++ b/LocalPackages/Freemium/Tests/FreemiumTests/FreemiumPIRUserStateManagerTests.swift @@ -18,24 +18,20 @@ import XCTest @testable import Freemium -import Subscription -import SubscriptionTestingUtilities final class FreemiumPIRUserStateManagerTests: XCTestCase { private static let testSuiteName = "test.defaults.freemium.user.state.tests" private let pir = "macos.browser.freemium.pir.did.onboard" private let testUserDefaults = UserDefaults(suiteName: FreemiumPIRUserStateManagerTests.testSuiteName)! - private var mockAccountManager: AccountManagerMock! override func setUpWithError() throws { - mockAccountManager = AccountManagerMock() testUserDefaults.removePersistentDomain(forName: FreemiumPIRUserStateManagerTests.testSuiteName) } func testSetsHasFreemiumPIR() throws { // Given - let sut = DefaultFreemiumPIRUserStateManager(userDefaults: testUserDefaults, accountManager: mockAccountManager) + let sut = DefaultFreemiumPIRUserStateManager(userDefaults: testUserDefaults) XCTAssertFalse(testUserDefaults.bool(forKey: pir)) // When @@ -47,7 +43,7 @@ final class FreemiumPIRUserStateManagerTests: XCTestCase { func testGetsHasFreemiumPIR() throws { // Given - let sut = DefaultFreemiumPIRUserStateManager(userDefaults: testUserDefaults, accountManager: mockAccountManager) + let sut = DefaultFreemiumPIRUserStateManager(userDefaults: testUserDefaults) XCTAssertFalse(sut.didOnboard) testUserDefaults.setValue(true, forKey: pir) XCTAssertTrue(testUserDefaults.bool(forKey: pir)) @@ -58,49 +54,4 @@ final class FreemiumPIRUserStateManagerTests: XCTestCase { // Then XCTAssertTrue(result) } - - func testIsCurrentFreemiumPIRUser_WhenDidOnboardIsTrueAndUserIsNotAuthenticated_ShouldReturnTrue() { - // Given - let sut = DefaultFreemiumPIRUserStateManager(userDefaults: testUserDefaults, accountManager: mockAccountManager) - XCTAssertFalse(sut.didOnboard) - testUserDefaults.setValue(true, forKey: pir) - mockAccountManager.accessToken = nil - XCTAssertTrue(testUserDefaults.bool(forKey: pir)) - - // When - let result = sut.isActiveUser - - // Then - XCTAssertTrue(result) - } - - func testIsCurrentFreemiumPIRUser_WhenDidOnboardIsTrueAndUserIsAuthenticated_ShouldReturnFalse() { - // Given - let sut = DefaultFreemiumPIRUserStateManager(userDefaults: testUserDefaults, accountManager: mockAccountManager) - XCTAssertFalse(sut.didOnboard) - testUserDefaults.setValue(true, forKey: pir) - mockAccountManager.accessToken = "some_token" - XCTAssertTrue(testUserDefaults.bool(forKey: pir)) - - // When - let result = sut.isActiveUser - - // Then - XCTAssertFalse(result) - } - - func testIsCurrentFreemiumPIRUser_WhenDidOnboardIsFalse_ShouldReturnFalse() { - // Given - let sut = DefaultFreemiumPIRUserStateManager(userDefaults: testUserDefaults, accountManager: mockAccountManager) - XCTAssertFalse(sut.didOnboard) - testUserDefaults.setValue(false, forKey: pir) - mockAccountManager.accessToken = "some_token" - XCTAssertFalse(testUserDefaults.bool(forKey: pir)) - - // When - let result = sut.isActiveUser - - // Then - XCTAssertFalse(result) - } } diff --git a/UnitTests/DBP/Tests/DataBrokerProtectionFeatureGatekeeperTests.swift b/UnitTests/DBP/Tests/DataBrokerProtectionFeatureGatekeeperTests.swift index 2d43206eca..39549e751c 100644 --- a/UnitTests/DBP/Tests/DataBrokerProtectionFeatureGatekeeperTests.swift +++ b/UnitTests/DBP/Tests/DataBrokerProtectionFeatureGatekeeperTests.swift @@ -39,7 +39,7 @@ final class DataBrokerProtectionFeatureGatekeeperTests: XCTestCase { mockFeatureAvailability = MockFeatureAvailability() mockAccountManager = MockAccountManager() mockFreemiumPIRUserState = MockFreemiumPIRUserState() - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false } func testWhenNoAccessTokenIsFound_butEntitlementIs_andIsNotActiveFreemiumUser_thenFeatureIsDisabled() async { @@ -63,7 +63,7 @@ final class DataBrokerProtectionFeatureGatekeeperTests: XCTestCase { // Given mockAccountManager.accessToken = "token" mockAccountManager.hasEntitlementResult = .failure(MockError.someError) - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false sut = DefaultDataBrokerProtectionFeatureGatekeeper(featureDisabler: mockFeatureDisabler, userDefaults: userDefaults(), subscriptionAvailability: mockFeatureAvailability, @@ -81,7 +81,7 @@ final class DataBrokerProtectionFeatureGatekeeperTests: XCTestCase { // Given mockAccountManager.accessToken = "token" mockAccountManager.hasEntitlementResult = .failure(MockError.someError) - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true sut = DefaultDataBrokerProtectionFeatureGatekeeper(featureDisabler: mockFeatureDisabler, userDefaults: userDefaults(), subscriptionAvailability: mockFeatureAvailability, @@ -92,14 +92,14 @@ final class DataBrokerProtectionFeatureGatekeeperTests: XCTestCase { let result = await sut.arePrerequisitesSatisfied() // Then - XCTAssertTrue(result) + XCTAssertFalse(result) } func testWhenAccessTokenAndEntitlementAreNotFound_andIsNotActiveFreemiumUser_thenFeatureIsDisabled() async { // Given mockAccountManager.accessToken = nil mockAccountManager.hasEntitlementResult = .failure(MockError.someError) - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false sut = DefaultDataBrokerProtectionFeatureGatekeeper(featureDisabler: mockFeatureDisabler, userDefaults: userDefaults(), subscriptionAvailability: mockFeatureAvailability, @@ -117,7 +117,7 @@ final class DataBrokerProtectionFeatureGatekeeperTests: XCTestCase { // Given mockAccountManager.accessToken = "token" mockAccountManager.hasEntitlementResult = .success(true) - mockFreemiumPIRUserState.isActiveUser = false + mockFreemiumPIRUserState.didOnboard = false sut = DefaultDataBrokerProtectionFeatureGatekeeper(featureDisabler: mockFeatureDisabler, userDefaults: userDefaults(), subscriptionAvailability: mockFeatureAvailability, @@ -135,7 +135,7 @@ final class DataBrokerProtectionFeatureGatekeeperTests: XCTestCase { // Given mockAccountManager.accessToken = nil mockAccountManager.hasEntitlementResult = .failure(MockError.someError) - mockFreemiumPIRUserState.isActiveUser = true + mockFreemiumPIRUserState.didOnboard = true sut = DefaultDataBrokerProtectionFeatureGatekeeper(featureDisabler: mockFeatureDisabler, userDefaults: userDefaults(), subscriptionAvailability: mockFeatureAvailability, diff --git a/UnitTests/RemoteMessaging/RemoteMessagingClientTests.swift b/UnitTests/RemoteMessaging/RemoteMessagingClientTests.swift index 2a8ed992bf..cf2dcd7e28 100644 --- a/UnitTests/RemoteMessaging/RemoteMessagingClientTests.swift +++ b/UnitTests/RemoteMessaging/RemoteMessagingClientTests.swift @@ -32,7 +32,6 @@ struct MockRemoteMessagingStoreProvider: RemoteMessagingStoreProviding { struct MockFreemiumPIRUserState: FreemiumPIRUserStateManager { var didOnboard = false - var isActiveUser = false } final class RemoteMessagingClientTests: XCTestCase {