From 94f99dec097cb99ffadbccfccc9712eb3a4a7237 Mon Sep 17 00:00:00 2001 From: Fernando Bunn Date: Fri, 13 Sep 2024 18:46:46 +0100 Subject: [PATCH] Fix issue with returning user post launch --- Core/MarketplaceAdPostbackManager.swift | 29 +++++++-- Core/MarketplaceAdPostbackStorage.swift | 62 +++++++++++++++++++ DuckDuckGo.xcodeproj/project.pbxproj | 4 ++ DuckDuckGo/AppDelegate.swift | 6 +- .../MarketplaceAdPostbackManagerTests.swift | 33 +++++++++- 5 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 Core/MarketplaceAdPostbackStorage.swift diff --git a/Core/MarketplaceAdPostbackManager.swift b/Core/MarketplaceAdPostbackManager.swift index 8dfa7bebf1..32cfbdea7f 100644 --- a/Core/MarketplaceAdPostbackManager.swift +++ b/Core/MarketplaceAdPostbackManager.swift @@ -30,28 +30,45 @@ public protocol MarketplaceAdPostbackManaging { /// > For the time being, we're also sending `lockPostback` to `true`. /// > More information can be found [here](https://app.asana.com/0/0/1208126219488943/1208289369964239/f). func sendAppLaunchPostback() + + /// Updates the stored value for the returning user state. + /// + /// This method updates the storage with the current state of the user (returning or new). + /// Since `ReturnUserMeasurement` will always return `isReturningUser` as `false` after the first run, + /// `MarketplaceAdPostbackManaging` maintains its own storage of the user's state across app launches. + func updateReturningUserValue() } public struct MarketplaceAdPostbackManager: MarketplaceAdPostbackManaging { - private let returningUserMeasurement: ReturnUserMeasurement + private let storage: MarketplaceAdPostbackStorage private let updater: MarketplaceAdPostbackUpdating + private let returningUserMeasurement: ReturnUserMeasurement - internal init(returningUserMeasurement: ReturnUserMeasurement = KeychainReturnUserMeasurement(), - updater: MarketplaceAdPostbackUpdating = MarketplaceAdPostbackUpdater()) { - self.returningUserMeasurement = returningUserMeasurement + internal init(storage: MarketplaceAdPostbackStorage = UserDefaultsMarketplaceAdPostbackStorage(), + updater: MarketplaceAdPostbackUpdating = MarketplaceAdPostbackUpdater(), + returningUserMeasurement: ReturnUserMeasurement = KeychainReturnUserMeasurement()) { + self.storage = storage self.updater = updater + self.returningUserMeasurement = returningUserMeasurement } public init() { - self.returningUserMeasurement = KeychainReturnUserMeasurement() + self.storage = UserDefaultsMarketplaceAdPostbackStorage() self.updater = MarketplaceAdPostbackUpdater() + self.returningUserMeasurement = KeychainReturnUserMeasurement() } public func sendAppLaunchPostback() { - if returningUserMeasurement.isReturningUser { + guard let isReturningUser = storage.isReturningUser else { return } + + if isReturningUser { updater.updatePostback(.installReturningUser, lockPostback: true) } else { updater.updatePostback(.installNewUser, lockPostback: true) } } + + public func updateReturningUserValue() { + storage.updateReturningUserValue(returningUserMeasurement.isReturningUser) + } } diff --git a/Core/MarketplaceAdPostbackStorage.swift b/Core/MarketplaceAdPostbackStorage.swift new file mode 100644 index 0000000000..66734f1818 --- /dev/null +++ b/Core/MarketplaceAdPostbackStorage.swift @@ -0,0 +1,62 @@ +// +// MarketplaceAdPostbackStorage.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +/// A protocol defining the storage for marketplace ad postback data. +protocol MarketplaceAdPostbackStorage { + + /// A Boolean value indicating whether the user is a returning user. + /// + /// If the value is `nil`, it means the storage was never set. + var isReturningUser: Bool? { get } + + /// Updates the stored value indicating whether the user is a returning user. + /// + /// - Parameter value: A Boolean value indicating whether the user is a returning user. + func updateReturningUserValue(_ value: Bool) +} + +/// A concrete implementation of `MarketplaceAdPostbackStorage` that uses `UserDefaults` for storage. +struct UserDefaultsMarketplaceAdPostbackStorage: MarketplaceAdPostbackStorage { + private let userDefaults: UserDefaults + + init(userDefaults: UserDefaults = .standard) { + self.userDefaults = userDefaults + } + + var isReturningUser: Bool? { + userDefaults.isReturningUser + } + + func updateReturningUserValue(_ value: Bool) { + userDefaults.isReturningUser = value + } +} + +private extension UserDefaults { + enum Keys { + static let isReturningUser = "marketplaceAdPostback.isReturningUser" + } + + var isReturningUser: Bool? { + get { object(forKey: Keys.isReturningUser) as? Bool } + set { set(newValue, forKey: Keys.isReturningUser) } + } +} diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 8e32072f6e..d751612bbe 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -159,6 +159,7 @@ 316931D927BD22A80095F5ED /* DownloadActionMessageViewHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 316931D827BD22A80095F5ED /* DownloadActionMessageViewHelper.swift */; }; 3170048227A9504F00C03F35 /* DownloadMocks.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3170048127A9504F00C03F35 /* DownloadMocks.swift */; }; 317045C02858C6B90016ED1F /* AutofillInterfaceEmailTruncatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 317045BF2858C6B90016ED1F /* AutofillInterfaceEmailTruncatorTests.swift */; }; + 317F5F982C94A9EB0081666F /* MarketplaceAdPostbackStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 317F5F972C94A9EB0081666F /* MarketplaceAdPostbackStorage.swift */; }; 31860A5B2C57ED2D005561F5 /* DuckPlayerStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31860A5A2C57ED2D005561F5 /* DuckPlayerStorage.swift */; }; 31951E8E2823003200CAF535 /* AutofillLoginDetailsHeaderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 31951E8D2823003200CAF535 /* AutofillLoginDetailsHeaderView.swift */; }; 319A371028299A850079FBCE /* PasswordHider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 319A370F28299A850079FBCE /* PasswordHider.swift */; }; @@ -1442,6 +1443,7 @@ 3170048127A9504F00C03F35 /* DownloadMocks.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloadMocks.swift; sourceTree = ""; }; 317045BF2858C6B90016ED1F /* AutofillInterfaceEmailTruncatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutofillInterfaceEmailTruncatorTests.swift; sourceTree = ""; }; 31794BFF2821DFB600F18633 /* DuckUI */ = {isa = PBXFileReference; lastKnownFileType = wrapper; path = DuckUI; sourceTree = ""; }; + 317F5F972C94A9EB0081666F /* MarketplaceAdPostbackStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MarketplaceAdPostbackStorage.swift; sourceTree = ""; }; 31860A5A2C57ED2D005561F5 /* DuckPlayerStorage.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DuckPlayerStorage.swift; sourceTree = ""; }; 31951E8D2823003200CAF535 /* AutofillLoginDetailsHeaderView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutofillLoginDetailsHeaderView.swift; sourceTree = ""; }; 319A370F28299A850079FBCE /* PasswordHider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PasswordHider.swift; sourceTree = ""; }; @@ -3584,6 +3586,7 @@ 31B2F10E2C92FECC00CD30E3 /* MarketplaceAdPostbackManager.swift */, 31B2F1122C92FEF500CD30E3 /* MarketplaceAdPostbackUpdater.swift */, 31B2F1102C92FEE000CD30E3 /* MarketplaceAdPostback.swift */, + 317F5F972C94A9EB0081666F /* MarketplaceAdPostbackStorage.swift */, ); name = MarketplaceAdPostback; sourceTree = ""; @@ -8212,6 +8215,7 @@ 85200FA11FBC5BB5001AF290 /* DDGPersistenceContainer.swift in Sources */, 9FEA22322C3270BD006B03BF /* TimerInterface.swift in Sources */, 1E4DCF4C27B6A4CB00961E25 /* URLFileExtension.swift in Sources */, + 317F5F982C94A9EB0081666F /* MarketplaceAdPostbackStorage.swift in Sources */, EE50053029C3BA0800AE0773 /* InternalUserStore.swift in Sources */, F1D477CB1F2149C40031ED49 /* Type.swift in Sources */, 983C52E42C2C050B007B5747 /* BookmarksStateRepair.swift in Sources */, diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index 7a7988fd57..50b89c4358 100644 --- a/DuckDuckGo/AppDelegate.swift +++ b/DuckDuckGo/AppDelegate.swift @@ -384,8 +384,6 @@ import os.log didCrashDuringCrashHandlersSetUp = false } - sendAppLaunchPostback() - return true } @@ -523,6 +521,7 @@ import os.log } AppConfigurationFetch().start { result in + self.sendAppLaunchPostback() if case .assetsUpdated(let protectionsUpdated) = result, protectionsUpdated { ContentBlocking.shared.contentBlockingManager.scheduleCompilation() } @@ -780,6 +779,9 @@ import os.log // New users don't see the message historyMessageManager.dismiss() + + // Setup storage for marketplace postback + marketplaceAdPostbackManager.updateReturningUserValue() } } diff --git a/DuckDuckGoTests/MarketplaceAdPostbackManagerTests.swift b/DuckDuckGoTests/MarketplaceAdPostbackManagerTests.swift index 7f73f096d4..332aa04d7b 100644 --- a/DuckDuckGoTests/MarketplaceAdPostbackManagerTests.swift +++ b/DuckDuckGoTests/MarketplaceAdPostbackManagerTests.swift @@ -25,7 +25,8 @@ class MarketplaceAdPostbackManagerTests: XCTestCase { func testSendAppLaunchPostback_NewUser() { let mockReturnUserMeasurement = MockReturnUserMeasurement(isReturningUser: false) let mockUpdater = MockMarketplaceAdPostbackUpdater() - let manager = MarketplaceAdPostbackManager(returningUserMeasurement: mockReturnUserMeasurement, updater: mockUpdater) + let mockStorage = MockMarketplaceAdPostbackStorage(isReturningUser: false) + let manager = MarketplaceAdPostbackManager(storage: mockStorage, updater: mockUpdater, returningUserMeasurement: mockReturnUserMeasurement) manager.sendAppLaunchPostback() @@ -37,7 +38,8 @@ class MarketplaceAdPostbackManagerTests: XCTestCase { func testSendAppLaunchPostback_ReturningUser() { let mockReturnUserMeasurement = MockReturnUserMeasurement(isReturningUser: true) let mockUpdater = MockMarketplaceAdPostbackUpdater() - let manager = MarketplaceAdPostbackManager(returningUserMeasurement: mockReturnUserMeasurement, updater: mockUpdater) + let mockStorage = MockMarketplaceAdPostbackStorage(isReturningUser: true) + let manager = MarketplaceAdPostbackManager(storage: mockStorage, updater: mockUpdater, returningUserMeasurement: mockReturnUserMeasurement) manager.sendAppLaunchPostback() @@ -45,6 +47,21 @@ class MarketplaceAdPostbackManagerTests: XCTestCase { XCTAssertEqual(mockUpdater.postbackSent?.coarseValue, .low) XCTAssertEqual(mockUpdater.lockPostbackSent, true) } + + func testSendAppLaunchPostback_AfterMeasurementChangesState() { + /// Sets return user to true to mock the situation where the user is opening the app again + /// If the storage is set to false, it should still be set as new user + let mockReturnUserMeasurement = MockReturnUserMeasurement(isReturningUser: true) + let mockUpdater = MockMarketplaceAdPostbackUpdater() + let mockStorage = MockMarketplaceAdPostbackStorage(isReturningUser: false) + let manager = MarketplaceAdPostbackManager(storage: mockStorage, updater: mockUpdater, returningUserMeasurement: mockReturnUserMeasurement) + + manager.sendAppLaunchPostback() + + XCTAssertEqual(mockUpdater.postbackSent, .installNewUser) + XCTAssertEqual(mockUpdater.postbackSent?.coarseValue, .high) + XCTAssertEqual(mockUpdater.lockPostbackSent, true) + } } private final class MockReturnUserMeasurement: ReturnUserMeasurement { @@ -68,3 +85,15 @@ private final class MockMarketplaceAdPostbackUpdater: MarketplaceAdPostbackUpdat lockPostbackSent = lockPostback } } + +private final class MockMarketplaceAdPostbackStorage: MarketplaceAdPostbackStorage { + var isReturningUser: Bool? + + init(isReturningUser: Bool?) { + self.isReturningUser = isReturningUser + } + + func updateReturningUserValue(_ value: Bool) { + isReturningUser = value + } +}