diff --git a/Experiments/Experiments.xcodeproj/project.pbxproj b/Experiments/Experiments.xcodeproj/project.pbxproj index cfbaeaf7bb2..30ef763472c 100644 --- a/Experiments/Experiments.xcodeproj/project.pbxproj +++ b/Experiments/Experiments.xcodeproj/project.pbxproj @@ -16,6 +16,11 @@ BC10218D75FEA979BDA1E68C /* Pods_Experiments.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 33CEC0C5283FD4C9EF8C6A3C /* Pods_Experiments.framework */; }; C8E16F0EE6954B58A1C402F0 /* Pods_ExperimentsTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = AAC7C082DD376957B4676401 /* Pods_ExperimentsTests.framework */; }; CC53FB48275E426900C4CA4F /* ABTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = CC53FB47275E426900C4CA4F /* ABTest.swift */; }; + EE2EDFDF29879331004E702B /* ABTestVariationProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE2EDFDE29879331004E702B /* ABTestVariationProvider.swift */; }; + EEC8C0ED298A92F10047B4CB /* CachedABTestVariationProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEC8C0EC298A92F10047B4CB /* CachedABTestVariationProvider.swift */; }; + EEC8C0EF298A939C0047B4CB /* VariationCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEC8C0EE298A939C0047B4CB /* VariationCache.swift */; }; + EEC8C0F1298B5AFB0047B4CB /* VariationCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEC8C0F0298B5AFB0047B4CB /* VariationCacheTests.swift */; }; + EEC8C0F3298B5F950047B4CB /* CachedABTestVariationProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EEC8C0F2298B5F950047B4CB /* CachedABTestVariationProviderTests.swift */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -47,6 +52,11 @@ AAC7C082DD376957B4676401 /* Pods_ExperimentsTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_ExperimentsTests.framework; sourceTree = BUILT_PRODUCTS_DIR; }; AF72D9DB7771E7A5105C88B0 /* Pods-Experiments.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Experiments.release.xcconfig"; path = "Target Support Files/Pods-Experiments/Pods-Experiments.release.xcconfig"; sourceTree = ""; }; CC53FB47275E426900C4CA4F /* ABTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ABTest.swift; sourceTree = ""; }; + EE2EDFDE29879331004E702B /* ABTestVariationProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ABTestVariationProvider.swift; sourceTree = ""; }; + EEC8C0EC298A92F10047B4CB /* CachedABTestVariationProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CachedABTestVariationProvider.swift; sourceTree = ""; }; + EEC8C0EE298A939C0047B4CB /* VariationCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VariationCache.swift; sourceTree = ""; }; + EEC8C0F0298B5AFB0047B4CB /* VariationCacheTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VariationCacheTests.swift; sourceTree = ""; }; + EEC8C0F2298B5F950047B4CB /* CachedABTestVariationProviderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CachedABTestVariationProviderTests.swift; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -100,6 +110,9 @@ 0270C0A427069B8900FC799F /* DefaultFeatureFlagService.swift */, 0270C0A627069BA500FC799F /* BuildConfiguration.swift */, CC53FB47275E426900C4CA4F /* ABTest.swift */, + EE2EDFDE29879331004E702B /* ABTestVariationProvider.swift */, + EEC8C0EC298A92F10047B4CB /* CachedABTestVariationProvider.swift */, + EEC8C0EE298A939C0047B4CB /* VariationCache.swift */, ); path = Experiments; sourceTree = ""; @@ -108,6 +121,8 @@ isa = PBXGroup; children = ( 0270C09127069A8900FC799F /* Info.plist */, + EEC8C0F0298B5AFB0047B4CB /* VariationCacheTests.swift */, + EEC8C0F2298B5F950047B4CB /* CachedABTestVariationProviderTests.swift */, ); path = ExperimentsTests; sourceTree = ""; @@ -203,6 +218,7 @@ }; 0270C08927069A8900FC799F = { CreatedOnToolsVersion = 12.5.1; + LastSwiftMigration = 1420; }; }; }; @@ -311,10 +327,13 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + EEC8C0EF298A939C0047B4CB /* VariationCache.swift in Sources */, 0270C0A327069B7800FC799F /* FeatureFlagService.swift in Sources */, 0270C0A527069B8900FC799F /* DefaultFeatureFlagService.swift in Sources */, 0270C09C27069AE700FC799F /* FeatureFlag.swift in Sources */, + EE2EDFDF29879331004E702B /* ABTestVariationProvider.swift in Sources */, 0270C0A727069BA500FC799F /* BuildConfiguration.swift in Sources */, + EEC8C0ED298A92F10047B4CB /* CachedABTestVariationProvider.swift in Sources */, CC53FB48275E426900C4CA4F /* ABTest.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; @@ -323,6 +342,8 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + EEC8C0F1298B5AFB0047B4CB /* VariationCacheTests.swift in Sources */, + EEC8C0F3298B5F950047B4CB /* CachedABTestVariationProviderTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -517,6 +538,7 @@ baseConfigurationReference = 3022E2766134CE2735C73FC6 /* Pods-ExperimentsTests.debug.xcconfig */; buildSettings = { ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = "${inherited}"; + CLANG_ENABLE_MODULES = YES; CODE_SIGN_STYLE = Automatic; INFOPLIST_FILE = ExperimentsTests/Info.plist; LD_RUNPATH_SEARCH_PATHS = ( @@ -526,6 +548,7 @@ ); PRODUCT_BUNDLE_IDENTIFIER = com.automattic.ExperimentsTests; PRODUCT_NAME = "$(TARGET_NAME)"; + SWIFT_OPTIMIZATION_LEVEL = "-Onone"; SWIFT_VERSION = 5.0; TARGETED_DEVICE_FAMILY = "1,2"; }; @@ -536,6 +559,7 @@ baseConfigurationReference = 7C831644164B49828A485590 /* Pods-ExperimentsTests.release.xcconfig */; buildSettings = { ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = "${inherited}"; + CLANG_ENABLE_MODULES = YES; CODE_SIGN_STYLE = Automatic; INFOPLIST_FILE = ExperimentsTests/Info.plist; LD_RUNPATH_SEARCH_PATHS = ( @@ -640,6 +664,7 @@ baseConfigurationReference = 3F9DB5FBFF7A42EFBCB746F3 /* Pods-ExperimentsTests.release-alpha.xcconfig */; buildSettings = { ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = "${inherited}"; + CLANG_ENABLE_MODULES = YES; CODE_SIGN_STYLE = Automatic; INFOPLIST_FILE = ExperimentsTests/Info.plist; LD_RUNPATH_SEARCH_PATHS = ( diff --git a/Experiments/Experiments/ABTest.swift b/Experiments/Experiments/ABTest.swift index 9dc3d0f0c55..0cbefac8923 100644 --- a/Experiments/Experiments/ABTest.swift +++ b/Experiments/Experiments/ABTest.swift @@ -2,10 +2,9 @@ import AutomatticTracks /// ABTest adds A/B testing experiments and runs the tests based on their variations from the ExPlat service. /// -public enum ABTest: String, CaseIterable { - /// Throwaway case, to prevent a compiler error: - /// `An enum with no cases cannot declare a raw type` - case null +public enum ABTest: String, Codable, CaseIterable { + /// Mocks for unit testing + case mockLoggedIn, mockLoggedOut /// A/A test to make sure there is no bias in the logged out state. /// Experiment ref: pbxNRc-1QS-p2 @@ -20,8 +19,9 @@ public enum ABTest: String, CaseIterable { case applicationPasswordAuthentication = "woocommerceios_login_rest_api_project_202301_v2" /// Returns a variation for the given experiment - public var variation: Variation { - ExPlat.shared?.experiment(rawValue) ?? .control + /// + public var variation: Variation? { + ExPlat.shared?.experiment(rawValue) } /// Returns the context for the given experiment. @@ -33,10 +33,19 @@ public enum ABTest: String, CaseIterable { return .loggedIn case .aaTestLoggedOut, .applicationPasswordAuthentication: return .loggedOut - case .null: - return .none + // Mocks + case .mockLoggedIn: + return .loggedIn + case .mockLoggedOut: + return .loggedOut } } + + // Returns only the genuine ABTest cases. (After removing unit test mocks) + // + static var genuineCases: [ABTest] { + ABTest.allCases.filter { [.mockLoggedIn, .mockLoggedOut].contains($0) == false } + } } public extension ABTest { @@ -44,7 +53,7 @@ public extension ABTest { /// @MainActor static func start(for context: ExperimentContext) async { - let experiments = ABTest.allCases.filter { $0.context == context } + let experiments = ABTest.genuineCases.filter { $0.context == context } await withCheckedContinuation { continuation in guard !experiments.isEmpty else { diff --git a/Experiments/Experiments/ABTestVariationProvider.swift b/Experiments/Experiments/ABTestVariationProvider.swift new file mode 100644 index 00000000000..89eeccf9b7b --- /dev/null +++ b/Experiments/Experiments/ABTestVariationProvider.swift @@ -0,0 +1,16 @@ +import AutomatticTracks + +/// For getting the variation of a `ABTest` +public protocol ABTestVariationProvider { + /// Returns the `Variation` for the provided `ABTest` + func variation(for abTest: ABTest) -> Variation +} + +/// Default implementation of `ABTestVariationProvider` +public struct DefaultABTestVariationProvider: ABTestVariationProvider { + public init() { } + + public func variation(for abTest: ABTest) -> Variation { + abTest.variation ?? .control + } +} diff --git a/Experiments/Experiments/CachedABTestVariationProvider.swift b/Experiments/Experiments/CachedABTestVariationProvider.swift new file mode 100644 index 00000000000..6ec636382c1 --- /dev/null +++ b/Experiments/Experiments/CachedABTestVariationProvider.swift @@ -0,0 +1,30 @@ +import AutomatticTracks + +/// Cache based implementation of `ABTestVariationProvider` +/// +public struct CachedABTestVariationProvider: ABTestVariationProvider { + + private let cache: VariationCache + + public init(cache: VariationCache = VariationCache(userDefaults: .standard)) { + self.cache = cache + } + + public func variation(for abTest: ABTest) -> Variation { + // We cache only logged out ABTests as they are assigned based on `anonId` by `ExPlat`. + // There will be one value assigned to one device and it won't change. + // + guard abTest.context == .loggedOut else { + return abTest.variation ?? .control + } + + if let variation = abTest.variation { + try? cache.assign(variation: variation, for: abTest) + return variation + } else if let cachedVariation = cache.variation(for: abTest) { + return cachedVariation + } else { + return .control + } + } +} diff --git a/Experiments/Experiments/VariationCache.swift b/Experiments/Experiments/VariationCache.swift new file mode 100644 index 00000000000..d25db108877 --- /dev/null +++ b/Experiments/Experiments/VariationCache.swift @@ -0,0 +1,47 @@ +import AutomatticTracks + +enum VariationCacheError: Error { + case onlyLoggedOutExperimentsShouldBeCached +} + +public struct VariationCache { + private let variationKey = "VariationCacheKey" + + private let userDefaults: UserDefaults + + public init(userDefaults: UserDefaults) { + self.userDefaults = userDefaults + } + + func variation(for abTest: ABTest) -> Variation? { + guard abTest.context == .loggedOut else { + return nil + } + + guard let data = userDefaults.object(forKey: variationKey) as? Data, + let cachedVariations = try? JSONDecoder().decode([CachedVariation].self, from: data), + case let variation = cachedVariations.first(where: { $0.abTest == abTest })?.variation + else { + return nil + } + + return variation + } + + func assign(variation: Variation, for abTest: ABTest) throws { + guard abTest.context == .loggedOut else { + throw VariationCacheError.onlyLoggedOutExperimentsShouldBeCached + } + + var variations = userDefaults.object(forKey: variationKey) as? [CachedVariation] ?? [] + variations.append(CachedVariation(abTest: abTest, variation: variation)) + + let encodedVariation = try JSONEncoder().encode(variations) + userDefaults.set(encodedVariation, forKey: variationKey) + } +} + +public struct CachedVariation: Codable { + let abTest: ABTest + let variation: Variation +} diff --git a/Experiments/ExperimentsTests/CachedABTestVariationProviderTests.swift b/Experiments/ExperimentsTests/CachedABTestVariationProviderTests.swift new file mode 100644 index 00000000000..5b569c1cc8e --- /dev/null +++ b/Experiments/ExperimentsTests/CachedABTestVariationProviderTests.swift @@ -0,0 +1,29 @@ +import XCTest +@testable import Experiments + +final class CachedABTestVariationProviderTests: XCTestCase { + func test_variation_is_control_when_the_value_does_not_exist() throws { + // Given + let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString)) + + // When + let cache = VariationCache(userDefaults: userDefaults) + let provider = CachedABTestVariationProvider(cache: cache) + + // Then + XCTAssertEqual(provider.variation(for: .mockLoggedOut), .control) + } + + func test_correct_variation_is_returned_after_caching_it() throws { + // Given + let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString)) + let cache = VariationCache(userDefaults: userDefaults) + let provider = CachedABTestVariationProvider(cache: cache) + + // When + try cache.assign(variation: .treatment, for: .mockLoggedOut) + + // Then + XCTAssertEqual(provider.variation(for: .mockLoggedOut), .treatment) + } +} diff --git a/Experiments/ExperimentsTests/VariationCacheTests.swift b/Experiments/ExperimentsTests/VariationCacheTests.swift new file mode 100644 index 00000000000..5f039b0a47e --- /dev/null +++ b/Experiments/ExperimentsTests/VariationCacheTests.swift @@ -0,0 +1,36 @@ +import XCTest +@testable import Experiments + +final class VariationCacheTests: XCTestCase { + func test_variation_is_nil_when_the_value_does_not_exist() throws { + // Given + let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString)) + + // When + let cache = VariationCache(userDefaults: userDefaults) + + // Then + XCTAssertNil(cache.variation(for: .mockLoggedOut)) + } + + func test_correct_variation_is_returned_after_setting_it() throws { + // Given + let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString)) + let cache = VariationCache(userDefaults: userDefaults) + + // When + try cache.assign(variation: .treatment, for: .mockLoggedOut) + + // Then + XCTAssertEqual(cache.variation(for: .mockLoggedOut), .treatment) + } + + func test_it_throws_when_trying_to_cache_logged_in_experiment() throws { + // Given + let userDefaults = try XCTUnwrap(UserDefaults(suiteName: UUID().uuidString)) + let cache = VariationCache(userDefaults: userDefaults) + + // When + XCTAssertThrowsError(try cache.assign(variation: .treatment, for: .mockLoggedIn)) + } +} diff --git a/Podfile b/Podfile index c78588acfb7..4e50c0e928c 100644 --- a/Podfile +++ b/Podfile @@ -27,7 +27,7 @@ def aztec end def tracks - pod 'Automattic-Tracks-iOS', '~> 1.0.0' + pod 'Automattic-Tracks-iOS', '~> 2.0.0-beta.1' # pod 'Automattic-Tracks-iOS', :git => 'https://github.com/Automattic/Automattic-Tracks-iOS.git', :branch => 'trunk' # pod 'Automattic-Tracks-iOS', :git => 'https://github.com/Automattic/Automattic-Tracks-iOS.git', :commit => '' # pod 'Automattic-Tracks-iOS', :path => '../Automattic-Tracks-iOS' diff --git a/Podfile.lock b/Podfile.lock index cff0d5731fb..9b82e4960ca 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -6,7 +6,7 @@ PODS: - AppAuth/Core (1.6.0) - AppAuth/ExternalUserAgent (1.6.0): - AppAuth/Core - - Automattic-Tracks-iOS (1.0.0): + - Automattic-Tracks-iOS (2.0.0-beta.1): - Sentry (~> 7.25) - Sodium (>= 0.9.1) - UIDeviceIdentifier (~> 2.0) @@ -75,7 +75,7 @@ PODS: DEPENDENCIES: - Alamofire (~> 4.8) - - Automattic-Tracks-iOS (~> 1.0.0) + - Automattic-Tracks-iOS (~> 2.0.0-beta.1) - CocoaLumberjack (~> 3.7.4) - CocoaLumberjack/Swift (~> 3.7.4) - Gridicons (~> 1.2.0) @@ -135,7 +135,7 @@ SPEC REPOS: SPEC CHECKSUMS: Alamofire: 3ec537f71edc9804815215393ae2b1a8ea33a844 AppAuth: 8fca6b5563a5baef2c04bee27538025e4ceb2add - Automattic-Tracks-iOS: 93df154824af31eba947718110023acce1ce7905 + Automattic-Tracks-iOS: 20220d63a075787a890513ae56214a17a160ec43 CocoaLumberjack: 543c79c114dadc3b1aba95641d8738b06b05b646 GoogleSignIn: fd381840dbe7c1137aa6dc30849a5c3e070c034a Gridicons: 4455b9f366960121430e45997e32112ae49ffe1d @@ -169,6 +169,6 @@ SPEC CHECKSUMS: ZendeskSupportProvidersSDK: 2bdf8544f7cd0fd4c002546f5704b813845beb2a ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba -PODFILE CHECKSUM: 5611275f2e251a88706eac841eb27ae63383a7ac +PODFILE CHECKSUM: 60adb2c26ae523f8b998d67c2c81cddce40b9809 COCOAPODS: 1.11.3 diff --git a/WooCommerce/Classes/Authentication/AuthenticationManager.swift b/WooCommerce/Classes/Authentication/AuthenticationManager.swift index 9bf14754e3c..2902562d0b8 100644 --- a/WooCommerce/Classes/Authentication/AuthenticationManager.swift +++ b/WooCommerce/Classes/Authentication/AuthenticationManager.swift @@ -9,6 +9,8 @@ import struct Networking.Settings import protocol Experiments.FeatureFlagService import protocol Storage.StorageManagerType import class Networking.DefaultApplicationPasswordUseCase +import protocol Experiments.ABTestVariationProvider +import struct Experiments.CachedABTestVariationProvider /// Encapsulates all of the interactions with the WordPress Authenticator /// @@ -45,15 +47,19 @@ class AuthenticationManager: Authentication { private let analytics: Analytics + private let abTestVariationProvider: ABTestVariationProvider + /// Keeps a reference to the checker private var postSiteCredentialLoginChecker: PostSiteCredentialLoginChecker? init(storageManager: StorageManagerType = ServiceLocator.storageManager, featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService, - analytics: Analytics = ServiceLocator.analytics) { + analytics: Analytics = ServiceLocator.analytics, + abTestVariationProvider: ABTestVariationProvider = CachedABTestVariationProvider()) { self.storageManager = storageManager self.featureFlagService = featureFlagService self.analytics = analytics + self.abTestVariationProvider = abTestVariationProvider } /// Initializes the WordPress Authenticator. @@ -62,7 +68,7 @@ class AuthenticationManager: Authentication { let isWPComMagicLinkPreferredToPassword = featureFlagService.isFeatureFlagEnabled(.loginMagicLinkEmphasis) let isWPComMagicLinkShownAsSecondaryActionOnPasswordScreen = featureFlagService.isFeatureFlagEnabled(.loginMagicLinkEmphasisM2) let isStoreCreationMVPEnabled = featureFlagService.isFeatureFlagEnabled(.storeCreationMVP) - let enableSiteAddressLoginOnly = ABTest.applicationPasswordAuthentication.variation == .treatment + let enableSiteAddressLoginOnly = abTestVariationProvider.variation(for: .applicationPasswordAuthentication) == .treatment let configuration = WordPressAuthenticatorConfiguration(wpcomClientId: ApiCredentials.dotcomAppId, wpcomSecret: ApiCredentials.dotcomSecret, wpcomScheme: ApiCredentials.dotcomAuthScheme, @@ -341,7 +347,7 @@ extension AuthenticationManager: WordPressAuthenticatorDelegate { /// save the site to memory to check for jetpack requirement in epilogue currentSelfHostedSite = site - let enableWPComOnlyForWPComSites = ABTest.applicationPasswordAuthentication.variation == .treatment + let enableWPComOnlyForWPComSites = abTestVariationProvider.variation(for: .applicationPasswordAuthentication) == .treatment switch (enableWPComOnlyForWPComSites, site.isWPCom) { case (true, true), (false, _): @@ -387,7 +393,7 @@ extension AuthenticationManager: WordPressAuthenticatorDelegate { /// If the user logged in with site credentials and application password feature flag is enabled, /// check if they can use the app and navigates to the home screen. if let siteCredentials = credentials.wporg, - ABTest.applicationPasswordAuthentication.variation == .treatment { + abTestVariationProvider.variation(for: .applicationPasswordAuthentication) == .treatment { return didAuthenticateUser(to: siteURL, with: siteCredentials, in: navigationController) @@ -499,7 +505,7 @@ extension AuthenticationManager: WordPressAuthenticatorDelegate { /// func sync(credentials: AuthenticatorCredentials, onCompletion: @escaping () -> Void) { if let wporg = credentials.wporg, - ABTest.applicationPasswordAuthentication.variation == .treatment { + abTestVariationProvider.variation(for: .applicationPasswordAuthentication) == .treatment { ServiceLocator.stores.authenticate(credentials: .wporg(username: wporg.username, password: wporg.password, siteAddress: wporg.siteURL)) diff --git a/WooCommerce/Classes/ViewRelated/AppCoordinator.swift b/WooCommerce/Classes/ViewRelated/AppCoordinator.swift index fe6cf72f201..c1715db6f5a 100644 --- a/WooCommerce/Classes/ViewRelated/AppCoordinator.swift +++ b/WooCommerce/Classes/ViewRelated/AppCoordinator.swift @@ -5,6 +5,8 @@ import WordPressAuthenticator import Yosemite import class AutomatticTracks.CrashLogging import protocol Storage.StorageManagerType +import protocol Experiments.ABTestVariationProvider +import struct Experiments.CachedABTestVariationProvider /// Coordinates app navigation based on authentication state: tab bar UI is shown when the app is logged in, and authentication UI is shown /// when the app is logged out. @@ -26,6 +28,7 @@ final class AppCoordinator { private var authStatesSubscription: AnyCancellable? private var localNotificationResponsesSubscription: AnyCancellable? private var isLoggedIn: Bool = false + private let abTestVariationProvider: ABTestVariationProvider /// Checks on whether the Apple ID credential is valid when the app is logged in and becomes active. /// @@ -39,7 +42,8 @@ final class AppCoordinator { analytics: Analytics = ServiceLocator.analytics, loggedOutAppSettings: LoggedOutAppSettingsProtocol = LoggedOutAppSettings(userDefaults: .standard), pushNotesManager: PushNotesManager = ServiceLocator.pushNotesManager, - featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) { + featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService, + abTestVariationProvider: ABTestVariationProvider = CachedABTestVariationProvider()) { self.window = window self.tabBarController = { let storyboard = UIStoryboard(name: "Main", bundle: nil) // Main is the name of storyboard @@ -56,6 +60,7 @@ final class AppCoordinator { self.loggedOutAppSettings = loggedOutAppSettings self.pushNotesManager = pushNotesManager self.featureFlagService = featureFlagService + self.abTestVariationProvider = abTestVariationProvider authenticationManager.setLoggedOutAppSettings(loggedOutAppSettings) @@ -143,7 +148,8 @@ private extension AppCoordinator { configureAndDisplayAuthenticator() } - analytics.track(event: .ApplicationPassword.restAPILoginExperiment(variation: ABTest.applicationPasswordAuthentication.variation.analyticsValue)) + let applicationPasswordABTestVariation = abTestVariationProvider.variation(for: .applicationPasswordAuthentication) + analytics.track(event: .ApplicationPassword.restAPILoginExperiment(variation: applicationPasswordABTestVariation.analyticsValue)) } /// Configures the WPAuthenticator and sets the authenticator UI as the window's root view. diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index 6a3b2eab2e3..c4cb4b0e4a4 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -1968,6 +1968,7 @@ E1F52DC62668E03B00349D75 /* CardPresentModalBluetoothRequired.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1F52DC52668E03B00349D75 /* CardPresentModalBluetoothRequired.swift */; }; EE0EE7A628B7415200F6061E /* CustomHelpCenterContent.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE0EE7A528B7415200F6061E /* CustomHelpCenterContent.swift */; }; EE0EE7A828B74EF300F6061E /* CustomHelpCenterContentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE0EE7A728B74EF300F6061E /* CustomHelpCenterContentTests.swift */; }; + EE2EDFE12987A189004E702B /* MockABTestVariationProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE2EDFE02987A189004E702B /* MockABTestVariationProvider.swift */; }; EE57C11D297AC27300BC31E7 /* TrackEventRequestNotificationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE57C11C297AC27300BC31E7 /* TrackEventRequestNotificationHandler.swift */; }; EE57C11F297E742200BC31E7 /* WooAnalyticsEvent+ApplicationPassword.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE57C11E297E742200BC31E7 /* WooAnalyticsEvent+ApplicationPassword.swift */; }; EE57C121297E76E000BC31E7 /* TrackEventRequestNotificationHandlerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE57C120297E76E000BC31E7 /* TrackEventRequestNotificationHandlerTests.swift */; }; @@ -4066,6 +4067,7 @@ E1F52DC52668E03B00349D75 /* CardPresentModalBluetoothRequired.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CardPresentModalBluetoothRequired.swift; sourceTree = ""; }; EE0EE7A528B7415200F6061E /* CustomHelpCenterContent.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomHelpCenterContent.swift; sourceTree = ""; }; EE0EE7A728B74EF300F6061E /* CustomHelpCenterContentTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomHelpCenterContentTests.swift; sourceTree = ""; }; + EE2EDFE02987A189004E702B /* MockABTestVariationProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockABTestVariationProvider.swift; sourceTree = ""; }; EE57C11C297AC27300BC31E7 /* TrackEventRequestNotificationHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TrackEventRequestNotificationHandler.swift; sourceTree = ""; }; EE57C11E297E742200BC31E7 /* WooAnalyticsEvent+ApplicationPassword.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WooAnalyticsEvent+ApplicationPassword.swift"; sourceTree = ""; }; EE57C120297E76E000BC31E7 /* TrackEventRequestNotificationHandlerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TrackEventRequestNotificationHandlerTests.swift; sourceTree = ""; }; @@ -6798,6 +6800,7 @@ EE8DCA7F28BF964700F23B23 /* MockAuthentication.swift */, AEB4DB98290AE8F300AE4340 /* MockCookieJar.swift */, 02660503293D8D24004084EA /* PaymentCaptureCelebration.swift */, + EE2EDFE02987A189004E702B /* MockABTestVariationProvider.swift */, ); path = Mocks; sourceTree = ""; @@ -11412,6 +11415,7 @@ DE4B3B2C2692DC2200EEF2D8 /* ReviewOrderViewModelTests.swift in Sources */, D89C009425B4E9E2000E4683 /* ULAccountMismatchViewControllerTests.swift in Sources */, 573A960324F433DD0091F3A5 /* ProductsTopBannerFactoryTests.swift in Sources */, + EE2EDFE12987A189004E702B /* MockABTestVariationProvider.swift in Sources */, 0273707E24C0047800167204 /* SequenceHelpersTests.swift in Sources */, D802547326551D0F001B2CC1 /* CardPresentModalTapCardTests.swift in Sources */, F997174723DC070D00592D8E /* XLPagerStrip+AccessibilityIdentifierTests.swift in Sources */, diff --git a/WooCommerce/WooCommerceTests/AppCoordinatorTests.swift b/WooCommerce/WooCommerceTests/AppCoordinatorTests.swift index b60d300540c..41c8fcdce7c 100644 --- a/WooCommerce/WooCommerceTests/AppCoordinatorTests.swift +++ b/WooCommerce/WooCommerceTests/AppCoordinatorTests.swift @@ -325,10 +325,15 @@ final class AppCoordinatorTests: XCTestCase { // Given stores.deauthenticate() let analytics = MockAnalyticsProvider() + + let mockABTestVariationProvider = MockABTestVariationProvider() + mockABTestVariationProvider.mockVariationValue = .control + let appCoordinator = makeCoordinator(window: window, stores: stores, authenticationManager: authenticationManager, - analytics: WooAnalytics(analyticsProvider: analytics)) + analytics: WooAnalytics(analyticsProvider: analytics), + abTestVariationProvider: mockABTestVariationProvider) // When appCoordinator.start() @@ -336,7 +341,9 @@ final class AppCoordinatorTests: XCTestCase { // Then let indexOfEvent = try XCTUnwrap(analytics.receivedEvents.firstIndex(where: { $0 == "rest_api_login_experiment" })) let eventProperties = try XCTUnwrap(analytics.receivedProperties[indexOfEvent]) - XCTAssertNotNil(eventProperties["experiment_variant"] as? String) + + let variant = try XCTUnwrap(eventProperties["experiment_variant"] as? String) + XCTAssertEqual(variant, "control") } // MARK: - Login reminder analytics @@ -453,7 +460,8 @@ private extension AppCoordinatorTests { analytics: Analytics = ServiceLocator.analytics, loggedOutAppSettings: LoggedOutAppSettingsProtocol = MockLoggedOutAppSettings(), pushNotesManager: PushNotesManager = ServiceLocator.pushNotesManager, - featureFlagService: FeatureFlagService = MockFeatureFlagService()) -> AppCoordinator { + featureFlagService: FeatureFlagService = MockFeatureFlagService(), + abTestVariationProvider: ABTestVariationProvider = CachedABTestVariationProvider()) -> AppCoordinator { return AppCoordinator(window: window ?? self.window, stores: stores ?? self.stores, storageManager: storageManager ?? self.storageManager, @@ -462,6 +470,7 @@ private extension AppCoordinatorTests { analytics: analytics, loggedOutAppSettings: loggedOutAppSettings, pushNotesManager: pushNotesManager, - featureFlagService: featureFlagService) + featureFlagService: featureFlagService, + abTestVariationProvider: abTestVariationProvider) } } diff --git a/WooCommerce/WooCommerceTests/Authentication/AuthenticationManagerTests.swift b/WooCommerce/WooCommerceTests/Authentication/AuthenticationManagerTests.swift index c21a8a4a754..3fce915efab 100644 --- a/WooCommerce/WooCommerceTests/Authentication/AuthenticationManagerTests.swift +++ b/WooCommerce/WooCommerceTests/Authentication/AuthenticationManagerTests.swift @@ -137,9 +137,12 @@ final class AuthenticationManagerTests: XCTestCase { XCTAssertTrue(viewModel is NoSecureConnectionErrorViewModel) } - func test_it_presents_username_and_password_controller_for_non_jetpack_site() { + func test_it_presents_email_controller_for_non_jetpack_site_when_not_using_application_password_authentication() { // Given - let manager = AuthenticationManager() + let mockABTestVariationProvider = MockABTestVariationProvider() + mockABTestVariationProvider.mockVariationValue = .control + + let manager = AuthenticationManager(abTestVariationProvider: mockABTestVariationProvider) let siteInfo = WordPressComSiteInfo(remote: ["isWordPress": true, "hasJetpack": false]) var result: WordPressAuthenticatorResult? let completionHandler: (WordPressAuthenticatorResult) -> Void = { completionResult in @@ -155,6 +158,27 @@ final class AuthenticationManagerTests: XCTestCase { } } + func test_it_presents_username_and_password_controller_for_non_jetpack_site_when_using_application_password_authentication() { + // Given + let mockABTestVariationProvider = MockABTestVariationProvider() + mockABTestVariationProvider.mockVariationValue = .treatment + + let manager = AuthenticationManager(abTestVariationProvider: mockABTestVariationProvider) + let siteInfo = WordPressComSiteInfo(remote: ["isWordPress": true, "hasJetpack": false]) + var result: WordPressAuthenticatorResult? + let completionHandler: (WordPressAuthenticatorResult) -> Void = { completionResult in + result = completionResult + } + + // When + manager.shouldPresentUsernamePasswordController(for: siteInfo, onCompletion: completionHandler) + + // Then + guard case .presentPasswordController = result else { + return XCTFail("Unexpected result returned for non-Jetpack site") + } + } + func test_it_shows_error_upon_login_epilogue_if_the_self_hosted_site_does_not_have_jetpack() { // Given let manager = AuthenticationManager() @@ -206,9 +230,12 @@ final class AuthenticationManagerTests: XCTestCase { XCTAssertTrue(rootController is ULAccountMismatchViewController) } - func test_it_can_display_jetpack_error_for_org_site_credentials_sign_in() { + func test_it_can_display_jetpack_error_for_org_site_credentials_sign_in_when_not_using_application_password_authentication() { // Given - let manager = AuthenticationManager() + let mockABTestVariationProvider = MockABTestVariationProvider() + mockABTestVariationProvider.mockVariationValue = .control + + let manager = AuthenticationManager(abTestVariationProvider: mockABTestVariationProvider) let testSite = "http://test.com" let siteInfo = WordPressComSiteInfo(remote: ["isWordPress": true, "hasJetpack": false, "urlAfterRedirects": testSite]) let wporgCredentials = WordPressOrgCredentials(username: "cba", password: "password", xmlrpc: "http://test.com/xmlrpc.php", options: [:]) @@ -224,6 +251,27 @@ final class AuthenticationManagerTests: XCTestCase { XCTAssertTrue(rootController is ULErrorViewController) } + func test_it_does_not_display_jetpack_error_for_org_site_credentials_sign_in_when_using_application_password_authentication() { + // Given + let mockABTestVariationProvider = MockABTestVariationProvider() + mockABTestVariationProvider.mockVariationValue = .treatment + + let manager = AuthenticationManager(abTestVariationProvider: mockABTestVariationProvider) + let testSite = "http://test.com" + let siteInfo = WordPressComSiteInfo(remote: ["isWordPress": true, "hasJetpack": false, "urlAfterRedirects": testSite]) + let wporgCredentials = WordPressOrgCredentials(username: "cba", password: "password", xmlrpc: "http://test.com/xmlrpc.php", options: [:]) + let credentials = AuthenticatorCredentials(wpcom: nil, wporg: wporgCredentials) + let navigationController = UINavigationController() + + // When + manager.shouldPresentUsernamePasswordController(for: siteInfo, onCompletion: { _ in }) + manager.presentLoginEpilogue(in: navigationController, for: credentials, source: nil, onDismiss: {}) + + // Then + let rootController = navigationController.viewControllers.first + XCTAssertFalse(rootController is ULErrorViewController) + } + func test_errorViewController_display_account_mismatch_screen_if_no_site_matches_the_given_self_hosted_site() { // Given let manager = AuthenticationManager() @@ -459,7 +507,6 @@ final class AuthenticationManagerTests: XCTestCase { func test_shouldPresentUsernamePasswordController_tracks_fetched_site_info() throws { // Given - let navigationController = UINavigationController() let analyticsProvider = MockAnalyticsProvider() let analytics = WooAnalytics(analyticsProvider: analyticsProvider) diff --git a/WooCommerce/WooCommerceTests/Mocks/MockABTestVariationProvider.swift b/WooCommerce/WooCommerceTests/Mocks/MockABTestVariationProvider.swift new file mode 100644 index 00000000000..dd45cec5fbe --- /dev/null +++ b/WooCommerce/WooCommerceTests/Mocks/MockABTestVariationProvider.swift @@ -0,0 +1,11 @@ +import protocol Experiments.ABTestVariationProvider +import enum Experiments.ABTest +import enum AutomatticTracks.Variation + +final class MockABTestVariationProvider: ABTestVariationProvider { + var mockVariationValue: Variation! + + func variation(for abTest: ABTest) -> Variation { + mockVariationValue + } +} diff --git a/WooCommerce/WooCommerceTests/UnitTests.xctestplan b/WooCommerce/WooCommerceTests/UnitTests.xctestplan index e353c33e4b9..6697ca371ad 100644 --- a/WooCommerce/WooCommerceTests/UnitTests.xctestplan +++ b/WooCommerce/WooCommerceTests/UnitTests.xctestplan @@ -78,6 +78,13 @@ "identifier" : "B9C9C63D283E703C001B879F", "name" : "WooFoundationTests" } + }, + { + "target" : { + "containerPath" : "container:..\/Experiments\/Experiments.xcodeproj", + "identifier" : "0270C08927069A8900FC799F", + "name" : "ExperimentsTests" + } } ], "version" : 1