From 74aec07b6236d6a0a2bc1225f62af6c5de4aae3f Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Fri, 17 Jan 2025 15:36:59 -0500 Subject: [PATCH 1/3] - Moved default browser user default to default browser util - Change logic of rating prompt, remove cumulative days of use counter - Call rating prompt on init of BVC - Remove rating prompt from dependency container --- firefox-ios/Client.xcodeproj/project.pbxproj | 8 - .../Client/Application/AppDelegate.swift | 2 - .../Application/DefaultBrowserUtil.swift | 9 + .../Client/Application/DependencyHelper.swift | 3 - .../Coordinators/Router/RouteBuilder.swift | 2 +- .../Messaging/GleanPlumbContextProvider.swift | 2 +- .../Views/BrowserViewController.swift | 6 +- firefox-ios/Client/RatingPromptManager.swift | 176 +++++------ .../Utils/CumulativeDaysOfUseCounter.swift | 93 ------ .../DependencyHelperMock.swift | 3 - .../Helpers/RatingPromptManagerTests.swift | 282 +++--------------- .../CumulativeDaysOfUseCounterTests.swift | 192 ------------ 12 files changed, 143 insertions(+), 635 deletions(-) delete mode 100644 firefox-ios/Client/Utils/CumulativeDaysOfUseCounter.swift delete mode 100644 firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/CumulativeDaysOfUseCounterTests.swift diff --git a/firefox-ios/Client.xcodeproj/project.pbxproj b/firefox-ios/Client.xcodeproj/project.pbxproj index 4c401becf33f..77f1ea9ab04e 100644 --- a/firefox-ios/Client.xcodeproj/project.pbxproj +++ b/firefox-ios/Client.xcodeproj/project.pbxproj @@ -1053,8 +1053,6 @@ 8ADC2A212A3399DC00543DAA /* YourRightsSetting.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ADC2A202A3399DC00543DAA /* YourRightsSetting.swift */; }; 8ADEC6832A40F208002D2ED8 /* AppSettingsTableViewControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ADEC6822A40F208002D2ED8 /* AppSettingsTableViewControllerTests.swift */; }; 8ADED7EC27691351009C19E6 /* CalendarExtensionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ADED7EB27691351009C19E6 /* CalendarExtensionsTests.swift */; }; - 8ADED7EE276A7750009C19E6 /* CumulativeDaysOfUseCounter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ADED7ED276A7750009C19E6 /* CumulativeDaysOfUseCounter.swift */; }; - 8ADED7F0276A7788009C19E6 /* CumulativeDaysOfUseCounterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8ADED7EF276A7788009C19E6 /* CumulativeDaysOfUseCounterTests.swift */; }; 8AE0BF4F2819B10E00F33EC4 /* TopSitesSettingsViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AE0BF4E2819B10E00F33EC4 /* TopSitesSettingsViewController.swift */; }; 8AE1E1CB27B18F560024C45E /* SearchBarSettingsViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AE1E1CA27B18F560024C45E /* SearchBarSettingsViewController.swift */; }; 8AE1E1CD27B191110024C45E /* SearchBarSettingsViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AE1E1CC27B191110024C45E /* SearchBarSettingsViewModel.swift */; }; @@ -7876,8 +7874,6 @@ 8ADC2A202A3399DC00543DAA /* YourRightsSetting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = YourRightsSetting.swift; sourceTree = ""; }; 8ADEC6822A40F208002D2ED8 /* AppSettingsTableViewControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppSettingsTableViewControllerTests.swift; sourceTree = ""; }; 8ADED7EB27691351009C19E6 /* CalendarExtensionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CalendarExtensionsTests.swift; sourceTree = ""; }; - 8ADED7ED276A7750009C19E6 /* CumulativeDaysOfUseCounter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CumulativeDaysOfUseCounter.swift; sourceTree = ""; }; - 8ADED7EF276A7788009C19E6 /* CumulativeDaysOfUseCounterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CumulativeDaysOfUseCounterTests.swift; sourceTree = ""; }; 8AE0BF4E2819B10E00F33EC4 /* TopSitesSettingsViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TopSitesSettingsViewController.swift; sourceTree = ""; }; 8AE1E1CA27B18F560024C45E /* SearchBarSettingsViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchBarSettingsViewController.swift; sourceTree = ""; }; 8AE1E1CC27B191110024C45E /* SearchBarSettingsViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchBarSettingsViewModel.swift; sourceTree = ""; }; @@ -11533,7 +11529,6 @@ 8A1E3BE028CBAC1F003388C4 /* Utils */ = { isa = PBXGroup; children = ( - 8ADED7EF276A7788009C19E6 /* CumulativeDaysOfUseCounterTests.swift */, 8A13FA8E2AD83F00007527AB /* DefaultBackgroundTabLoaderTests.swift */, 961D6B822995AF84001B9CF1 /* GeneralizedImageFetcherTests.swift */, C8E78BDC27F4A1E700C48BAA /* HistoryDeletionUtilityTests.swift */, @@ -14064,7 +14059,6 @@ 5AD3B67D2CF665AE00AFA1FE /* UIApplicationInterface.swift */, 5AD3B67B2CF65DE300AFA1FE /* LocaleInterface.swift */, 8A13FA8C2AD834FA007527AB /* BackgroundTabLoader.swift */, - 8ADED7ED276A7750009C19E6 /* CumulativeDaysOfUseCounter.swift */, C8BA0E7527F20B8E00DD8214 /* HistoryDeletionUtility.swift */, C2296FCB2A601C190046ECA6 /* IntensityVisualEffectView.swift */, F85C7F112721048E004BDBA4 /* Layout.swift */, @@ -16390,7 +16384,6 @@ 8AAAB0592C1B7240008830B3 /* MockRustFirefoxSuggest.swift in Sources */, 0BDDB33F2CA6B1F000D501DF /* EditFolderViewModel.swift in Sources */, 602B3D6729B0E1DB0066DEF8 /* ConversionValueUtil.swift in Sources */, - 8ADED7EE276A7750009C19E6 /* CumulativeDaysOfUseCounter.swift in Sources */, 7AC7E0502C160FF800051D4D /* ReaderPanelEmptyStateView.swift in Sources */, 4346FF08295BA6A300F4D220 /* CreditCardSettingsViewController.swift in Sources */, E19B38B528A42EBC00D8C541 /* WallpaperCellViewModel.swift in Sources */, @@ -17435,7 +17428,6 @@ 8A5189C92C1B614E00CDB668 /* SearchViewModelTests.swift in Sources */, E1463D0629830E4F0074E16E /* MockUserNotificationCenter.swift in Sources */, 439B78182A09721600CAAE37 /* FormAutofillHelperTests.swift in Sources */, - 8ADED7F0276A7788009C19E6 /* CumulativeDaysOfUseCounterTests.swift in Sources */, 8A7653C528A2E69100924ABF /* MockPocketAPI.swift in Sources */, 8A83B74A2A265044002FF9AC /* SettingsCoordinatorTests.swift in Sources */, E1B9A2C42CADA78300F6A0E9 /* ToolbarTelemetryTests.swift in Sources */, diff --git a/firefox-ios/Client/Application/AppDelegate.swift b/firefox-ios/Client/Application/AppDelegate.swift index 45fca12b2b11..731473aa43bc 100644 --- a/firefox-ios/Client/Application/AppDelegate.swift +++ b/firefox-ios/Client/Application/AppDelegate.swift @@ -29,7 +29,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate, FeatureFlaggable { ) lazy var themeManager: ThemeManager = DefaultThemeManager(sharedContainerIdentifier: AppInfo.sharedContainerIdentifier) - lazy var ratingPromptManager = RatingPromptManager(profile: profile) lazy var appSessionManager: AppSessionProvider = AppSessionManager() lazy var notificationSurfaceManager = NotificationSurfaceManager() lazy var tabDataStore = DefaultTabDataStore() @@ -168,7 +167,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate, FeatureFlaggable { if self?.featureFlags.isFeatureEnabled(.cleanupHistoryReenabled, checking: .buildOnly) ?? false { self?.profile.cleanupHistoryIfNeeded() } - self?.ratingPromptManager.updateData() } DispatchQueue.global().async { [weak self] in diff --git a/firefox-ios/Client/Application/DefaultBrowserUtil.swift b/firefox-ios/Client/Application/DefaultBrowserUtil.swift index 26c9666b77a8..4c3f1f42107c 100644 --- a/firefox-ios/Client/Application/DefaultBrowserUtil.swift +++ b/firefox-ios/Client/Application/DefaultBrowserUtil.swift @@ -26,6 +26,15 @@ struct DefaultBrowserUtil { self.logger = logger } + enum UserDefaultsKey: String { + case keyIsBrowserDefault = "com.moz.isBrowserDefault.key" + } + + static var isBrowserDefault: Bool { + get { UserDefaults.standard.object(forKey: UserDefaultsKey.keyIsBrowserDefault.rawValue) as? Bool ?? false } + set { UserDefaults.standard.set(newValue, forKey: UserDefaultsKey.keyIsBrowserDefault.rawValue) } + } + func processUserDefaultState(isFirstRun: Bool) { guard #available(iOS 18.2, *) else { return } diff --git a/firefox-ios/Client/Application/DependencyHelper.swift b/firefox-ios/Client/Application/DependencyHelper.swift index 01c400073a61..2732468923d6 100644 --- a/firefox-ios/Client/Application/DependencyHelper.swift +++ b/firefox-ios/Client/Application/DependencyHelper.swift @@ -26,9 +26,6 @@ class DependencyHelper { let appSessionProvider: AppSessionProvider = appDelegate.appSessionManager AppContainer.shared.register(service: appSessionProvider) - let ratingPromptManager: RatingPromptManager = appDelegate.ratingPromptManager - AppContainer.shared.register(service: ratingPromptManager) - let downloadQueue: DownloadQueue = appDelegate.appSessionManager.downloadQueue AppContainer.shared.register(service: downloadQueue) diff --git a/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift b/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift index aeabed693659..1152bc702218 100644 --- a/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift +++ b/firefox-ios/Client/Coordinators/Router/RouteBuilder.swift @@ -142,7 +142,7 @@ final class RouteBuilder: FeatureFlaggable { } } else if urlScanner.isHTTPScheme { TelemetryWrapper.gleanRecordEvent(category: .action, method: .open, object: .asDefaultBrowser) - RatingPromptManager.isBrowserDefault = true + DefaultBrowserUtil.isBrowserDefault = true // Use the last browsing mode the user was in return .search(url: url, isPrivate: isPrivate, options: [.focusLocationField]) } else { diff --git a/firefox-ios/Client/Experiments/Messaging/GleanPlumbContextProvider.swift b/firefox-ios/Client/Experiments/Messaging/GleanPlumbContextProvider.swift index 64f0615fed93..64a708a92d78 100644 --- a/firefox-ios/Client/Experiments/Messaging/GleanPlumbContextProvider.swift +++ b/firefox-ios/Client/Experiments/Messaging/GleanPlumbContextProvider.swift @@ -36,7 +36,7 @@ class GleanPlumbContextProvider { } private var isDefaultBrowser: Bool { - return userDefaults.bool(forKey: RatingPromptManager.UserDefaultsKey.keyIsBrowserDefault.rawValue) + return userDefaults.bool(forKey: DefaultBrowserUtil.UserDefaultsKey.keyIsBrowserDefault.rawValue) } private var numberOfAppLaunches: Int32 { diff --git a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift index 39c295f9ed81..1d222b560836 100644 --- a/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift +++ b/firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift @@ -272,7 +272,6 @@ class BrowserViewController: UIViewController, tabManager: TabManager, themeManager: ThemeManager = AppContainer.shared.resolve(), notificationCenter: NotificationProtocol = NotificationCenter.default, - ratingPromptManager: RatingPromptManager = AppContainer.shared.resolve(), downloadQueue: DownloadQueue = AppContainer.shared.resolve(), logger: Logger = DefaultLogger.shared, appAuthenticator: AppAuthenticationProtocol = AppAuthenticator() @@ -281,7 +280,7 @@ class BrowserViewController: UIViewController, self.tabManager = tabManager self.themeManager = themeManager self.notificationCenter = notificationCenter - self.ratingPromptManager = ratingPromptManager + self.ratingPromptManager = RatingPromptManager(prefs: profile.prefs) self.readerModeCache = DiskReaderModeCache.sharedInstance self.downloadQueue = downloadQueue self.logger = logger @@ -345,6 +344,9 @@ class BrowserViewController: UIViewController, guard !AppEventQueue.activityIsCompleted(.browserUpdatedForAppActivation(tabWindowUUID)) else { return } self?.browserDidBecomeActive() } + + ratingPromptManager.updateData() + ratingPromptManager.showRatingPromptIfNeeded() } @objc diff --git a/firefox-ios/Client/RatingPromptManager.swift b/firefox-ios/Client/RatingPromptManager.swift index a89fe4a04c85..366147ad4839 100644 --- a/firefox-ios/Client/RatingPromptManager.swift +++ b/firefox-ios/Client/RatingPromptManager.swift @@ -2,48 +2,43 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/ +import Common import Foundation import StoreKit import Shared -import Storage -import Common - -import class MozillaAppServices.BookmarkFolderData -import enum MozillaAppServices.BookmarkRoots /// The `RatingPromptManager` handles app store review requests and the internal logic of when /// they can be presented to a user. final class RatingPromptManager { - private let profile: Profile - private let daysOfUseCounter: CumulativeDaysOfUseCounter - - private var hasMinimumMobileBookmarksCount = false - private let minimumMobileBookmarksCount = 5 + private let prefs: Prefs private let logger: Logger - private let group: DispatchGroupInterface + private let userDefaults: UserDefaultsInterface - private let dataQueue = DispatchQueue(label: "com.moz.ratingPromptManager.queue") + struct Constants { + static let minDaysBetweenReviewRequest = 60 + static let firstThreshold = 30 + static let secondThreshold = 90 + static let thirdThreshold = 120 + } enum UserDefaultsKey: String { - case keyIsBrowserDefault = "com.moz.isBrowserDefault.key" case keyRatingPromptLastRequestDate = "com.moz.ratingPromptLastRequestDate.key" case keyRatingPromptRequestCount = "com.moz.ratingPromptRequestCount.key" + case keyRatingPromptThreshold = "com.moz.keyRatingPromptThreshold.key" + case keyLastCrashDateKey = "com.moz.lastCrashDateKey.key" } /// Initializes the `RatingPromptManager` using the provided profile and the user's current days of use of Firefox /// /// - Parameters: - /// - profile: User's profile data - /// - daysOfUseCounter: Counter for the cumulative days of use of the application by the user + /// - prefs: User's profile data /// - logger: Logger protocol to override in Unit test - init(profile: Profile, - daysOfUseCounter: CumulativeDaysOfUseCounter = CumulativeDaysOfUseCounter(), + init(prefs: Prefs, logger: Logger = DefaultLogger.shared, - group: DispatchGroupInterface = DispatchGroup()) { - self.profile = profile - self.daysOfUseCounter = daysOfUseCounter + userDefaults: UserDefaultsInterface = UserDefaults.standard) { + self.prefs = prefs self.logger = logger - self.group = group + self.userDefaults = userDefaults } /// Show the in-app rating prompt if needed @@ -51,20 +46,15 @@ final class RatingPromptManager { func showRatingPromptIfNeeded(at date: Date = Date()) { if shouldShowPrompt { requestRatingPrompt(at: date) - UserDefaults.standard.set(false, forKey: PrefsKeys.ForceShowAppReviewPromptOverride) + userDefaults.set(false, forKey: PrefsKeys.ForceShowAppReviewPromptOverride) + userDefaults.set(false, forKey: PrefsKeys.ForceShowAppReviewPromptOverride) } } - /// Update rating prompt data. Bookmarks and pinned sites data is loaded asynchronously. - /// - Parameter dataLoadingCompletion: Complete when the loading of data from the profile is done - /// Used in unit tests - func updateData(dataLoadingCompletion: (() -> Void)? = nil) { - daysOfUseCounter.updateCounter() - - updateBookmarksCount(group: group) - - group.notify(queue: dataQueue) { - dataLoadingCompletion?() + /// Update rating prompt data + func updateData() { + if logger.crashedLastLaunch { + userDefaults.set(Date(), forKey: UserDefaultsKey.keyLastCrashDateKey.rawValue) } } @@ -79,19 +69,14 @@ final class RatingPromptManager { // MARK: UserDefaults - static var isBrowserDefault: Bool { - get { UserDefaults.standard.object(forKey: UserDefaultsKey.keyIsBrowserDefault.rawValue) as? Bool ?? false } - set { UserDefaults.standard.set(newValue, forKey: UserDefaultsKey.keyIsBrowserDefault.rawValue) } - } - private var lastRequestDate: Date? { get { - return UserDefaults.standard.object( + return userDefaults.object( forKey: UserDefaultsKey.keyRatingPromptLastRequestDate.rawValue ) as? Date } set { - UserDefaults.standard.set( + userDefaults.set( newValue, forKey: UserDefaultsKey.keyRatingPromptLastRequestDate.rawValue ) @@ -100,15 +85,23 @@ final class RatingPromptManager { private var requestCount: Int { get { - UserDefaults.standard.object( + userDefaults.object( forKey: UserDefaultsKey.keyRatingPromptRequestCount.rawValue ) as? Int ?? 0 } - set { UserDefaults.standard.set(newValue, forKey: UserDefaultsKey.keyRatingPromptRequestCount.rawValue) } + set { userDefaults.set(newValue, forKey: UserDefaultsKey.keyRatingPromptRequestCount.rawValue) } + } + + private var threshold: Int { + get { + userDefaults.object( + forKey: UserDefaultsKey.keyRatingPromptThreshold.rawValue + ) as? Int ?? 0 + } + set { userDefaults.set(newValue, forKey: UserDefaultsKey.keyRatingPromptThreshold.rawValue) } } func reset() { - RatingPromptManager.isBrowserDefault = false lastRequestDate = nil requestCount = 0 } @@ -116,44 +109,53 @@ final class RatingPromptManager { // MARK: Private private var shouldShowPrompt: Bool { - if UserDefaults.standard.bool(forKey: PrefsKeys.ForceShowAppReviewPromptOverride) { + if userDefaults.bool(forKey: PrefsKeys.ForceShowAppReviewPromptOverride) { return true } - // Required: 5th launch or more - let currentSessionCount = profile.prefs.intForKey(PrefsKeys.Session.Count) ?? 0 - guard currentSessionCount >= 5 else { return false } - - // Required: 5 consecutive days of use in the last 7 days - guard daysOfUseCounter.hasRequiredCumulativeDaysOfUse else { return false } - - // Required: has not crashed in the last session - guard !logger.crashedLastLaunch else { return false } - - // One of the following - let isBrowserDefault = RatingPromptManager.isBrowserDefault - let hasTPStrict = profile.prefs.stringForKey( - ContentBlockingConfig.Prefs.StrengthKey - ).flatMap({ BlockingStrength(rawValue: $0) }) == .strict - guard isBrowserDefault - || hasMinimumMobileBookmarksCount - || hasTPStrict - else { return false } - - // Ensure we ask again only if 2 weeks has passed - guard !hasRequestedInTheLastTwoWeeks else { return false } - - // As per Apple's framework, an app can only present the prompt three times per period of 365 days. - // Because of this, Firefox will currently limit its request to show the ratings prompt to one time, given - // that the triggers are fulfilled. As such, requirements and attempts to further show the ratings prompt - // will be implemented later in the future. - return requestCount < 1 + // Required: has not crashed in the last 3 days + guard !hasCrashedInLast3Days() else { return false } + + let launchCount = prefs.intForKey(PrefsKeys.Session.Count) ?? 0 + + var daysSinceLastRequest = 0 + if let previousRequest = lastRequestDate { + daysSinceLastRequest = Calendar.current.numberOfDaysBetween(previousRequest, and: Date()) + } else { + daysSinceLastRequest = Constants.minDaysBetweenReviewRequest + } + + // Required: More than `minDaysBetweenReviewRequest` since last request + if daysSinceLastRequest < Constants.minDaysBetweenReviewRequest { + return false + } + + // Required: Launch count is greater than or equal to threshold + if launchCount <= threshold { + return false + } + + // Change threshold for next iteration of the prompt request + switch threshold { + case Constants.firstThreshold: + threshold = Constants.secondThreshold + case Constants.secondThreshold: + threshold = Constants.thirdThreshold + default: + break + } + + return true } private func requestRatingPrompt(at date: Date) { lastRequestDate = date requestCount += 1 + logger.log("Rating prompt is being requested, this is the \(requestCount) number of time the request is made", + level: .info, + category: .setup) + guard let scene = UIApplication.shared.connectedScenes.first(where: { $0.activationState == .foregroundActive }) as? UIWindowScene else { return } @@ -163,33 +165,13 @@ final class RatingPromptManager { } } - private func updateBookmarksCount(group: DispatchGroupInterface) { - group.enter() - profile.places.getBookmarksTree( - rootGUID: BookmarkRoots.MobileFolderGUID, - recursive: false - ).uponQueue(.main) { [weak self] result in - guard let strongSelf = self, - let mobileFolder = result.successValue as? BookmarkFolderData, - let children = mobileFolder.children - else { - group.leave() - return - } - - let bookmarksCounts = children.filter { $0.type == .bookmark }.count - strongSelf.hasMinimumMobileBookmarksCount = bookmarksCounts >= strongSelf.minimumMobileBookmarksCount - group.leave() - } - } - - private var hasRequestedInTheLastTwoWeeks: Bool { - guard let lastRequestDate = lastRequestDate else { return false } - - let currentDate = Date() - let numberOfDays = Calendar.current.numberOfDaysBetween(lastRequestDate, and: currentDate) + private func hasCrashedInLast3Days() -> Bool { + guard let lastCrashDate = userDefaults.object( + forKey: UserDefaultsKey.keyLastCrashDateKey.rawValue + ) as? Date else { return false } - return numberOfDays <= 14 + let threeDaysAgo = Calendar.current.date(byAdding: .day, value: -3, to: Date())! + return lastCrashDate >= threeDaysAgo } } diff --git a/firefox-ios/Client/Utils/CumulativeDaysOfUseCounter.swift b/firefox-ios/Client/Utils/CumulativeDaysOfUseCounter.swift deleted file mode 100644 index 9dae7493e33c..000000000000 --- a/firefox-ios/Client/Utils/CumulativeDaysOfUseCounter.swift +++ /dev/null @@ -1,93 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/ - -import Foundation - -// Counter to know if a user has used the app a certain number of days in a row, -// used for `RatingPromptManager` requirements. -class CumulativeDaysOfUseCounter { - private let calendar = Calendar.current - private let maximumNumberOfDaysToCollect = 7 - private let requiredCumulativeDaysOfUseCount = 5 - - private enum UserDefaultsKey: String { - case keyArrayDaysOfUse = "com.moz.arrayDaysOfUse.key" - case keyRequiredCumulativeDaysOfUseCount = "com.moz.hasRequiredCumulativeDaysOfUseCount.key" - } - - private(set) var hasRequiredCumulativeDaysOfUse: Bool { - get { - UserDefaults.standard.object( - forKey: UserDefaultsKey.keyRequiredCumulativeDaysOfUseCount.rawValue - ) as? Bool ?? false - } - set { - UserDefaults.standard.set( - newValue, - forKey: UserDefaultsKey.keyRequiredCumulativeDaysOfUseCount.rawValue - ) - } - } - - var daysOfUse: [Date]? { - get { UserDefaults.standard.array(forKey: UserDefaultsKey.keyArrayDaysOfUse.rawValue) as? [Date] } - set { UserDefaults.standard.set(newValue, forKey: UserDefaultsKey.keyArrayDaysOfUse.rawValue) } - } - - func updateCounter(currentDate: Date = Date()) { - // If there's no data, add current day of usage - guard var daysOfUse = daysOfUse, let lastDayOfUse = daysOfUse.last else { - daysOfUse = [currentDate] - return - } - - // Append usage days that are not already saved - let numberOfDaysSinceLastUse = calendar.numberOfDaysBetween(lastDayOfUse, and: currentDate) - if numberOfDaysSinceLastUse >= 1 { - daysOfUse.append(currentDate) - self.daysOfUse = daysOfUse - } - - // Check if we have 5 consecutive days in the last 7 days - hasRequiredCumulativeDaysOfUse = hasRequiredCumulativeDaysOfUse(daysOfUse: daysOfUse) - - // Clean data older than 7 days - cleanDaysOfUseData(daysOfUse: daysOfUse, currentDate: currentDate) - } - - private func hasRequiredCumulativeDaysOfUse(daysOfUse: [Date]) -> Bool { - var cumulativeDaysCount = 0 - var previousDay: Date? - var maxNumberOfConsecutiveDays = 0 - - daysOfUse.forEach { dayOfUse in - if let previousDay = previousDay { - let numberOfDaysBetween = calendar.numberOfDaysBetween(previousDay, and: dayOfUse) - cumulativeDaysCount = numberOfDaysBetween == 1 ? cumulativeDaysCount + 1 : 0 - } else { - cumulativeDaysCount += 1 - } - - maxNumberOfConsecutiveDays = max(cumulativeDaysCount, maxNumberOfConsecutiveDays) - previousDay = dayOfUse - } - - return maxNumberOfConsecutiveDays >= requiredCumulativeDaysOfUseCount - } - - private func cleanDaysOfUseData(daysOfUse: [Date], currentDate: Date) { - var cleanedDaysOfUse = daysOfUse - cleanedDaysOfUse.removeAll(where: { - let numberOfDays = calendar.numberOfDaysBetween($0, and: currentDate) - return numberOfDays >= maximumNumberOfDaysToCollect - }) - - self.daysOfUse = cleanedDaysOfUse - } - - func reset() { - hasRequiredCumulativeDaysOfUse = false - daysOfUse = nil - } -} diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/DependencyInjection/DependencyHelperMock.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/DependencyInjection/DependencyHelperMock.swift index 1ea67cd1782a..08c609d9396b 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/DependencyInjection/DependencyHelperMock.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/DependencyInjection/DependencyHelperMock.swift @@ -40,9 +40,6 @@ class DependencyHelperMock { let themeManager: ThemeManager = MockThemeManager() AppContainer.shared.register(service: themeManager) - let ratingPromptManager = RatingPromptManager(profile: profile) - AppContainer.shared.register(service: ratingPromptManager) - let downloadQueue = DownloadQueue() AppContainer.shared.register(service: downloadQueue) diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift index 5cc205bfaae9..f892f9ee2e24 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift @@ -13,145 +13,76 @@ import XCTest class RatingPromptManagerTests: XCTestCase { var urlOpenerSpy: URLOpenerSpy! - var promptManager: RatingPromptManager! - var mockProfile: MockProfile! - var createdGuids: [String] = [] + var prefs: MockProfilePrefs! var logger: CrashingMockLogger! - var mockDispatchGroup: MockDispatchGroup! + var userDefaults: MockUserDefaults! override func setUp() { super.setUp() - if let bundleID = Bundle.main.bundleIdentifier { - UserDefaults.standard.removePersistentDomain(forName: bundleID) - } + prefs = MockProfilePrefs() + logger = CrashingMockLogger() urlOpenerSpy = URLOpenerSpy() + userDefaults = MockUserDefaults() } override func tearDown() { - super.tearDown() - - createdGuids = [] - promptManager?.reset() - promptManager = nil - mockProfile?.shutdown() - mockProfile = nil + prefs.clearAll() + prefs = nil logger = nil urlOpenerSpy = nil + userDefaults = nil + + super.tearDown() } - func testShouldShowPrompt_requiredAreFalse_returnsFalse() { - setupEnvironment(numberOfSession: 0, - hasCumulativeDaysOfUse: false) - promptManager.showRatingPromptIfNeeded() - XCTAssertEqual(ratingPromptOpenCount, 0) + func testShouldShowPrompt_forceShow() { + let subject = createSubject() + userDefaults.set(true, forKey: PrefsKeys.ForceShowAppReviewPromptOverride) + subject.showRatingPromptIfNeeded() + XCTAssertEqual(ratingPromptOpenCount, 1) } - func testShouldShowPrompt_requiredTrueWithoutOptional_returnsFalse() { - setupEnvironment() - promptManager.showRatingPromptIfNeeded() + func testShouldShowPrompt_requiredAreFalse_returnsFalse() { + let subject = createSubject() + prefs.setInt(0, forKey: PrefsKeys.Session.Count) + subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 0) } func testShouldShowPrompt_withRequiredRequirementsAndOneOptional_returnsTrue() { - setupEnvironment(isBrowserDefault: true) - promptManager.showRatingPromptIfNeeded() + let subject = createSubject() + subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 1) } func testShouldShowPrompt_lessThanSession5_returnsFalse() { - setupEnvironment(numberOfSession: 4, - hasCumulativeDaysOfUse: true, - isBrowserDefault: true) - promptManager.showRatingPromptIfNeeded() - XCTAssertEqual(ratingPromptOpenCount, 0) - } - - func testShouldShowPrompt_cumulativeDaysOfUseFalse_returnsFalse() { - setupEnvironment(numberOfSession: 5, - hasCumulativeDaysOfUse: false, - isBrowserDefault: true) - promptManager.showRatingPromptIfNeeded() + let subject = createSubject() + prefs.setInt(0, forKey: PrefsKeys.Session.Count) + subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 0) } func testShouldShowPrompt_loggerHasCrashedInLastSession_returnsFalse() { - setupEnvironment(isBrowserDefault: true) + let subject = createSubject() logger?.enableCrashOnLastLaunch = true - promptManager.showRatingPromptIfNeeded() + subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 0) } - func testShouldShowPrompt_isBrowserDefaultTrue_returnsTrue() { - setupEnvironment(isBrowserDefault: true) - promptManager.showRatingPromptIfNeeded() - XCTAssertEqual(ratingPromptOpenCount, 1) - } - - func testShouldShowPrompt_hasTPStrict_returnsTrue() { - setupEnvironment() - mockProfile.prefs.setString( - BlockingStrength.strict.rawValue, - forKey: ContentBlockingConfig.Prefs.StrengthKey - ) - - promptManager.showRatingPromptIfNeeded() - XCTAssertEqual(ratingPromptOpenCount, 1) - } - - // MARK: Bookmarks - - func testShouldShowPrompt_hasNotMinimumMobileBookmarksCount_returnsFalse() { - setupEnvironment() - createBookmarks(bookmarkCount: 2, withRoot: BookmarkRoots.MobileFolderGUID) - updateData(expectedRatingPromptOpenCount: 0) - } - - func testShouldShowPrompt_hasMinimumMobileBookmarksCount_returnsTrue() { - _ = XCTSkip("flakey test") -// setupEnvironment() -// createBookmarks(bookmarkCount: 5, withRoot: BookmarkRoots.MobileFolderGUID) -// updateData(expectedRatingPromptOpenCount: 1) - } - - func testShouldShowPrompt_hasOtherBookmarksCount_returnsFalse() { - setupEnvironment() - createBookmarks(bookmarkCount: 5, withRoot: BookmarkRoots.ToolbarFolderGUID) - updateData(expectedRatingPromptOpenCount: 0) - } - - func testShouldShowPrompt_has5FoldersInMobileBookmarks_returnsFalse() { - setupEnvironment() - createFolders(folderCount: 5, withRoot: BookmarkRoots.MobileFolderGUID) - updateData(expectedRatingPromptOpenCount: 0) - } - - func testShouldShowPrompt_has5SeparatorsInMobileBookmarks_returnsFalse() { - setupEnvironment() - createSeparators(separatorCount: 5, withRoot: BookmarkRoots.MobileFolderGUID) - updateData(expectedRatingPromptOpenCount: 0) - } - - func testShouldShowPrompt_hasRequestedTwoWeeksAgo_returnsTrue() { - setupEnvironment(isBrowserDefault: true) - promptManager.showRatingPromptIfNeeded(at: Date().lastTwoWeek) - XCTAssertEqual(ratingPromptOpenCount, 1) - } - // MARK: Number of times asked func testShouldShowPrompt_hasRequestedInTheLastTwoWeeks_returnsFalse() { - setupEnvironment() - - promptManager.showRatingPromptIfNeeded(at: Date().lastTwoWeek) + let subject = createSubject() + subject.showRatingPromptIfNeeded(at: Date().lastTwoWeek) XCTAssertEqual(ratingPromptOpenCount, 0) } func testShouldShowPrompt_requestCountTwiceCountIsAtOne() { - setupEnvironment(isBrowserDefault: true) - promptManager.showRatingPromptIfNeeded() - promptManager.showRatingPromptIfNeeded() + let subject = createSubject() + subject.showRatingPromptIfNeeded() + subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 1) } @@ -166,131 +97,13 @@ class RatingPromptManagerTests: XCTestCase { "https://itunes.apple.com/app/id\(AppInfo.appStoreId)?action=write-review" ) } -} - -// MARK: - Places helpers - -private extension RatingPromptManagerTests { - func createFolders(folderCount: Int, withRoot root: String, file: StaticString = #filePath, line: UInt = #line) { - (1...folderCount).forEach { index in - mockProfile.places.createFolder( - parentGUID: root, - title: "Folder \(index)", - position: nil - ).uponQueue(.main) { guid in - guard let guid = guid.successValue else { - XCTFail("CreateFolder method did not return GUID", file: file, line: line) - return - } - self.createdGuids.append(guid) - } - } - - // Make sure the folders we create are deleted at the end of the test - addTeardownBlock { [weak self] in - self?.createdGuids.forEach { guid in - _ = self?.mockProfile.places.deleteBookmarkNode(guid: guid) - } - } - } - - func createSeparators( - separatorCount: Int, - withRoot root: String, - file: StaticString = #filePath, - line: UInt = #line - ) { - (1...separatorCount).forEach { index in - mockProfile.places.createSeparator(parentGUID: root, position: nil).uponQueue(.main) { guid in - guard let guid = guid.successValue else { - XCTFail("CreateFolder method did not return GUID", file: file, line: line) - return - } - self.createdGuids.append(guid) - } - } - - // Make sure the separators we create are deleted at the end of the test - addTeardownBlock { [weak self] in - self?.createdGuids.forEach { guid in - _ = self?.mockProfile.places.deleteBookmarkNode(guid: guid) - } - } - } - - func createBookmarks(bookmarkCount: Int, withRoot root: String) { - (1...bookmarkCount).forEach { index in - let bookmark = ShareItem(url: "http://www.example.com/\(index)", title: "Example \(index)") - _ = mockProfile.places.createBookmark(parentGUID: root, - url: bookmark.url, - title: bookmark.title, - position: nil).value - } - - // Make sure the bookmarks we create are deleted at the end of the test - addTeardownBlock { [weak self] in - self?.deleteBookmarks(bookmarkCount: bookmarkCount) - } - } - - func deleteBookmarks(bookmarkCount: Int) { - (1...bookmarkCount).forEach { index in - _ = mockProfile.places.deleteBookmarksWithURL(url: "http://www.example.com/\(index)") - } - } - func updateData(expectedRatingPromptOpenCount: Int, file: StaticString = #filePath, line: UInt = #line) { - let expectation = self.expectation(description: "Rating prompt manager data is loaded") - promptManager.updateData(dataLoadingCompletion: { [weak self] in - guard let promptManager = self?.promptManager else { - XCTFail("Should have reference to promptManager", file: file, line: line) - return - } + // MARK: - Setup helpers - promptManager.showRatingPromptIfNeeded() - XCTAssertEqual( - self?.ratingPromptOpenCount, - expectedRatingPromptOpenCount, - file: file, - line: line - ) - expectation.fulfill() - }) - waitForExpectations(timeout: 5, handler: nil) - } -} - -// MARK: - Setup helpers - -private extension RatingPromptManagerTests { - func setupEnvironment(numberOfSession: Int32 = 5, - hasCumulativeDaysOfUse: Bool = true, - isBrowserDefault: Bool = false, - functionName: String = #function) { - mockProfile = MockProfile(databasePrefix: functionName) - mockProfile.reopen() - - mockProfile.prefs.setInt(numberOfSession, forKey: PrefsKeys.Session.Count) - setupPromptManager(hasCumulativeDaysOfUse: hasCumulativeDaysOfUse) - RatingPromptManager.isBrowserDefault = isBrowserDefault - } - - func setupPromptManager(hasCumulativeDaysOfUse: Bool) { - let mockCounter = CumulativeDaysOfUseCounterMock(hasCumulativeDaysOfUse) - logger = CrashingMockLogger() - mockDispatchGroup = MockDispatchGroup() - promptManager = RatingPromptManager(profile: mockProfile, - daysOfUseCounter: mockCounter, - logger: logger, - group: mockDispatchGroup) - } - - func createSite(number: Int) -> Site { - let site = Site(url: "http://s\(number)ite\(number).com/foo", title: "A \(number)") - site.id = number - site.guid = "abc\(number)def" - - return site + func createSubject(file: StaticString = #file, line: UInt = #line) -> RatingPromptManager { + let subject = RatingPromptManager(prefs: prefs, logger: logger, userDefaults: userDefaults) + trackForMemoryLeaks(subject, file: file, line: line) + return subject } var ratingPromptOpenCount: Int { @@ -298,17 +111,11 @@ private extension RatingPromptManagerTests { forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptRequestCount.rawValue ) as? Int ?? 0 } -} - -// MARK: - CumulativeDaysOfUseCounterMock -class CumulativeDaysOfUseCounterMock: CumulativeDaysOfUseCounter { - private let hasMockRequiredDaysOfUse: Bool - init(_ hasRequiredCumulativeDaysOfUse: Bool) { - self.hasMockRequiredDaysOfUse = hasRequiredCumulativeDaysOfUse - } - override var hasRequiredCumulativeDaysOfUse: Bool { - return hasMockRequiredDaysOfUse + var lastCrashDateKey: Date? { + UserDefaults.standard.object( + forKey: RatingPromptManager.UserDefaultsKey.keyLastCrashDateKey.rawValue + ) as? Date } } @@ -324,6 +131,15 @@ class CrashingMockLogger: Logger { var crashedLastLaunch: Bool { return enableCrashOnLastLaunch } + + func log(_ message: String, + level: LoggerLevel, + category: LoggerCategory, + extra: [String: String]? = nil, + description: String? = nil, + file: String = #file, + function: String = #function, + line: Int = #line) {} } // MARK: - URLOpenerSpy diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/CumulativeDaysOfUseCounterTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/CumulativeDaysOfUseCounterTests.swift deleted file mode 100644 index 3fee1db96bf3..000000000000 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/CumulativeDaysOfUseCounterTests.swift +++ /dev/null @@ -1,192 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/ - -import XCTest - -@testable import Client - -class CumulativeDaysOfUseCounterTests: XCTestCase { - private var calendar: Calendar! - private var counter: CumulativeDaysOfUseCounter! - - override func setUp() { - super.setUp() - calendar = Calendar.current - counter = CumulativeDaysOfUseCounter() - counter.reset() - } - - override func tearDown() { - super.tearDown() - counter = nil - calendar = nil - } - - func testByDefaultCounter_isFalse() { - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertNil(counter.daysOfUse) - } - - func testUpdateCounterOnce_isFalse() { - counter.updateCounter() - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 1) - } - - func testUpdateCounter5TimesSameDay_isFalse() { - for _ in 0...5 { - counter.updateCounter() - } - - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 1) - } - - func testUpdateCounterMoreThan5TimesSameDay_isFalse() { - for _ in 0...10 { - counter.updateCounter() - } - - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 1) - } - - func testUpdateCounterFiveTimeDifferentDaysWithOneDayBetween_isFalse() { - let currentDate = Date() - addUsageDays(from: 0, to: 2, currentDate: currentDate) - addUsageDays(from: 4, to: 5, currentDate: currentDate) - - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 5) - } - - func testUpdateCounterFiveTimeDifferentDaysWithDaysBetween_isFalse() { - let currentDate = Date() - addUsageDays(from: 1, to: 2, currentDate: currentDate) - addUsageDays(from: 6, to: 8, currentDate: currentDate) - - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 4) - } - - func testUpdateCounterFiveTimeDifferentDaysInARow_isTrue() { - let currentDate = Date() - addUsageDays(from: 0, to: 4, currentDate: currentDate) - - XCTAssertTrue(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 5) - } - - func testUpdateCounterFiveTimeValidWithinSevenDays_isTrue() { - let currentDate = Date() - addUsageDays(from: 0, to: 4, currentDate: currentDate) - addUsageDays(from: 6, to: 6, currentDate: currentDate) - - XCTAssertTrue(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 6) - } - - func testUpdateCounterFiveTimeInARowThenNoUsageForTwoDays_isFalse() { - // i.e data shouldn't be kept longer than 7 days - let currentDate = Date() - addUsageDays(from: 0, to: 4, currentDate: currentDate) // 5 days cumulative - addUsageDays(from: 7, to: 8, currentDate: currentDate) // 2 days no usage + 2 cumulative days - - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 5) - } - - func testUpdateCounterMultipleTimesDailyForMultipleDaysExpectDay5_isFalse() { - // Day 1: Opens the app 3 times - let currentDate = Date() - counter.updateCounter(currentDate: currentDate) - counter.updateCounter(currentDate: currentDate) - counter.updateCounter(currentDate: currentDate) - - // Day 2: Opens the app 2 times - updateCounter(numberOfDays: 1, currentDate: currentDate) - updateCounter(numberOfDays: 1, currentDate: currentDate) - - // Day 3: Opens the app 1 time - updateCounter(numberOfDays: 2, currentDate: currentDate) - - // Day 4: Opens the app 3 times - updateCounter(numberOfDays: 3, currentDate: currentDate) - updateCounter(numberOfDays: 3, currentDate: currentDate) - updateCounter(numberOfDays: 3, currentDate: currentDate) - - // Day 5: Nothing - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 4) - - // Day 6: Opens the app 2 times - updateCounter(numberOfDays: 5, currentDate: currentDate) - updateCounter(numberOfDays: 5, currentDate: currentDate) - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 5) - } - - func testUpdateCounterMultipleTimesDailyForMultipleDays_isTrue() { - // Day 1: Opens the app 3 times - let currentDate = Date() - counter.updateCounter(currentDate: currentDate) - counter.updateCounter(currentDate: currentDate) - counter.updateCounter(currentDate: currentDate) - - // Day 2: Opens the app 2 times - updateCounter(numberOfDays: 1, currentDate: currentDate) - updateCounter(numberOfDays: 1, currentDate: currentDate) - - // Day 3: Opens the app 1 time - updateCounter(numberOfDays: 2, currentDate: currentDate) - - // Day 4: Opens the app 3 times - updateCounter(numberOfDays: 3, currentDate: currentDate) - updateCounter(numberOfDays: 3, currentDate: currentDate) - updateCounter(numberOfDays: 3, currentDate: currentDate) - - // Day 5: Opens the app 1 time - updateCounter(numberOfDays: 4, currentDate: currentDate) - XCTAssertTrue(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 5) - - // Day 6: Opens the app 2 times - updateCounter(numberOfDays: 5, currentDate: currentDate) - updateCounter(numberOfDays: 5, currentDate: currentDate) - XCTAssertTrue(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 6) - - // Day 9: Opens the app 2 times - updateCounter(numberOfDays: 8, currentDate: currentDate) - updateCounter(numberOfDays: 8, currentDate: currentDate) - XCTAssertFalse(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 5) - } - - func testHadFiveCumulativeDaysInPastCanBeTrueAgain() { - // Day 1 to 5: daily usage - let currentDate = Date() - addUsageDays(from: 0, to: 4, currentDate: currentDate) - XCTAssertEqual(counter.daysOfUse?.count, 5) - - // 4 days break then day 9 to 13: daily usage - addUsageDays(from: 9, to: 13, currentDate: currentDate) - XCTAssertTrue(counter.hasRequiredCumulativeDaysOfUse) - XCTAssertEqual(counter.daysOfUse?.count, 5) - } -} - -// MARK: Helpers -private extension CumulativeDaysOfUseCounterTests { - func addUsageDays(from: Int, to: Int, currentDate: Date) { - for numberOfDay in from...to { - updateCounter(numberOfDays: numberOfDay, currentDate: currentDate) - } - } - - func updateCounter(numberOfDays: Int, currentDate: Date) { - let date = calendar.add(numberOfDays: numberOfDays, to: currentDate)! - counter.updateCounter(currentDate: date) - } -} From 955ba88dd68d3a40b41c34afc41167a382aad36d Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Mon, 20 Jan 2025 15:41:56 -0500 Subject: [PATCH 2/3] Fix tests --- firefox-ios/Client/RatingPromptManager.swift | 24 ++-- .../Helpers/RatingPromptManagerTests.swift | 105 ++++++++++++++---- 2 files changed, 95 insertions(+), 34 deletions(-) diff --git a/firefox-ios/Client/RatingPromptManager.swift b/firefox-ios/Client/RatingPromptManager.swift index 366147ad4839..0f2e53fabfbc 100644 --- a/firefox-ios/Client/RatingPromptManager.swift +++ b/firefox-ios/Client/RatingPromptManager.swift @@ -42,19 +42,17 @@ final class RatingPromptManager { } /// Show the in-app rating prompt if needed - /// - Parameter date: Request at a certain date - Useful for unit tests - func showRatingPromptIfNeeded(at date: Date = Date()) { + func showRatingPromptIfNeeded() { if shouldShowPrompt { - requestRatingPrompt(at: date) - userDefaults.set(false, forKey: PrefsKeys.ForceShowAppReviewPromptOverride) + requestRatingPrompt() userDefaults.set(false, forKey: PrefsKeys.ForceShowAppReviewPromptOverride) } } /// Update rating prompt data - func updateData() { + func updateData(currentDate: Date = Date()) { if logger.crashedLastLaunch { - userDefaults.set(Date(), forKey: UserDefaultsKey.keyLastCrashDateKey.rawValue) + userDefaults.set(currentDate, forKey: UserDefaultsKey.keyLastCrashDateKey.rawValue) } } @@ -96,7 +94,7 @@ final class RatingPromptManager { get { userDefaults.object( forKey: UserDefaultsKey.keyRatingPromptThreshold.rawValue - ) as? Int ?? 0 + ) as? Int ?? Constants.firstThreshold } set { userDefaults.set(newValue, forKey: UserDefaultsKey.keyRatingPromptThreshold.rawValue) } } @@ -104,6 +102,7 @@ final class RatingPromptManager { func reset() { lastRequestDate = nil requestCount = 0 + threshold = 0 } // MARK: Private @@ -116,8 +115,6 @@ final class RatingPromptManager { // Required: has not crashed in the last 3 days guard !hasCrashedInLast3Days() else { return false } - let launchCount = prefs.intForKey(PrefsKeys.Session.Count) ?? 0 - var daysSinceLastRequest = 0 if let previousRequest = lastRequestDate { daysSinceLastRequest = Calendar.current.numberOfDaysBetween(previousRequest, and: Date()) @@ -126,12 +123,13 @@ final class RatingPromptManager { } // Required: More than `minDaysBetweenReviewRequest` since last request - if daysSinceLastRequest < Constants.minDaysBetweenReviewRequest { + guard daysSinceLastRequest >= Constants.minDaysBetweenReviewRequest else { return false } // Required: Launch count is greater than or equal to threshold - if launchCount <= threshold { + let launchCount = prefs.intForKey(PrefsKeys.Session.Count) ?? 0 + guard launchCount >= threshold else { return false } @@ -148,8 +146,8 @@ final class RatingPromptManager { return true } - private func requestRatingPrompt(at date: Date) { - lastRequestDate = date + private func requestRatingPrompt() { + lastRequestDate = Date() requestCount += 1 logger.log("Rating prompt is being requested, this is the \(requestCount) number of time the request is made", diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift index f892f9ee2e24..7e981265dd2a 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Helpers/RatingPromptManagerTests.swift @@ -16,6 +16,7 @@ class RatingPromptManagerTests: XCTestCase { var prefs: MockProfilePrefs! var logger: CrashingMockLogger! var userDefaults: MockUserDefaults! + var subject: RatingPromptManager! override func setUp() { super.setUp() @@ -24,63 +25,131 @@ class RatingPromptManagerTests: XCTestCase { logger = CrashingMockLogger() urlOpenerSpy = URLOpenerSpy() userDefaults = MockUserDefaults() + subject = RatingPromptManager(prefs: prefs, logger: logger, userDefaults: userDefaults) } override func tearDown() { prefs.clearAll() + subject.reset() prefs = nil logger = nil urlOpenerSpy = nil userDefaults = nil + subject = nil super.tearDown() } func testShouldShowPrompt_forceShow() { - let subject = createSubject() userDefaults.set(true, forKey: PrefsKeys.ForceShowAppReviewPromptOverride) subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 1) } func testShouldShowPrompt_requiredAreFalse_returnsFalse() { - let subject = createSubject() prefs.setInt(0, forKey: PrefsKeys.Session.Count) subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 0) } - func testShouldShowPrompt_withRequiredRequirementsAndOneOptional_returnsTrue() { - let subject = createSubject() + func testShouldShowPrompt_withRequiredRequirements_returnsTrue() { + prefs.setInt(30, forKey: PrefsKeys.Session.Count) subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 1) } - func testShouldShowPrompt_lessThanSession5_returnsFalse() { - let subject = createSubject() - prefs.setInt(0, forKey: PrefsKeys.Session.Count) + func testShouldShowPrompt_loggerHasCrashedInLastSession_returnsFalse() { + logger?.enableCrashOnLastLaunch = true + subject.updateData() + prefs.setInt(30, forKey: PrefsKeys.Session.Count) + subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 0) } - func testShouldShowPrompt_loggerHasCrashedInLastSession_returnsFalse() { - let subject = createSubject() + func testShouldShowPrompt_loggerHasCrashedYesterday_returnsFalse() { + logger?.enableCrashOnLastLaunch = true + subject.updateData(currentDate: Date().dayBefore) + prefs.setInt(30, forKey: PrefsKeys.Session.Count) + + subject.showRatingPromptIfNeeded() + XCTAssertEqual(ratingPromptOpenCount, 0) + } + + func testShouldShowPrompt_loggerHasCrashedLastWeek_returnsTrue() { logger?.enableCrashOnLastLaunch = true + subject.updateData(currentDate: Date().lastWeek) + prefs.setInt(30, forKey: PrefsKeys.Session.Count) + + subject.showRatingPromptIfNeeded() + XCTAssertEqual(ratingPromptOpenCount, 1) + } + + func testShouldShowPrompt_currentLastRequestDate_returnsFalse() { + prefs.setInt(30, forKey: PrefsKeys.Session.Count) + userDefaults.set(Date(), forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptLastRequestDate.rawValue) subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 0) } - // MARK: Number of times asked + func testShouldShowPrompt_pastDateLastRequestDate_returnsTrue() { + prefs.setInt(30, forKey: PrefsKeys.Session.Count) + let pastDate = Calendar.current.date(byAdding: .day, value: -61, to: Date()) ?? Date() + userDefaults.set(pastDate, forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptLastRequestDate.rawValue) + + subject.showRatingPromptIfNeeded() + XCTAssertEqual(ratingPromptOpenCount, 1) + } func testShouldShowPrompt_hasRequestedInTheLastTwoWeeks_returnsFalse() { - let subject = createSubject() - subject.showRatingPromptIfNeeded(at: Date().lastTwoWeek) + prefs.setInt(30, forKey: PrefsKeys.Session.Count) + let twoWeeksAgo = Calendar.current.date(byAdding: .day, value: -14, to: Date()) ?? Date() + userDefaults.set(twoWeeksAgo, + forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptLastRequestDate.rawValue) + + subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 0) } + func testShouldShowPrompt_hasNotReachedSecondThreshold_returnsFalse() { + userDefaults.set(RatingPromptManager.Constants.secondThreshold, + forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptThreshold.rawValue) + prefs.setInt(31, forKey: PrefsKeys.Session.Count) + + subject.showRatingPromptIfNeeded() + XCTAssertEqual(ratingPromptOpenCount, 0) + } + + func testShouldShowPrompt_reachedSecondThreshold_returnsTrue() { + userDefaults.set(RatingPromptManager.Constants.secondThreshold, + forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptThreshold.rawValue) + prefs.setInt(91, forKey: PrefsKeys.Session.Count) + + subject.showRatingPromptIfNeeded() + XCTAssertEqual(ratingPromptOpenCount, 1) + } + + func testShouldShowPrompt_hasNotReachedThirdThreshold_returnsFalse() { + userDefaults.set(RatingPromptManager.Constants.thirdThreshold, + forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptThreshold.rawValue) + prefs.setInt(91, forKey: PrefsKeys.Session.Count) + + subject.showRatingPromptIfNeeded() + XCTAssertEqual(ratingPromptOpenCount, 0) + } + + func testShouldShowPrompt_reachedThirdThreshold_returnsTrue() { + userDefaults.set(RatingPromptManager.Constants.thirdThreshold, + forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptThreshold.rawValue) + prefs.setInt(121, forKey: PrefsKeys.Session.Count) + + subject.showRatingPromptIfNeeded() + XCTAssertEqual(ratingPromptOpenCount, 1) + } + func testShouldShowPrompt_requestCountTwiceCountIsAtOne() { - let subject = createSubject() + prefs.setInt(30, forKey: PrefsKeys.Session.Count) subject.showRatingPromptIfNeeded() subject.showRatingPromptIfNeeded() XCTAssertEqual(ratingPromptOpenCount, 1) @@ -100,20 +169,14 @@ class RatingPromptManagerTests: XCTestCase { // MARK: - Setup helpers - func createSubject(file: StaticString = #file, line: UInt = #line) -> RatingPromptManager { - let subject = RatingPromptManager(prefs: prefs, logger: logger, userDefaults: userDefaults) - trackForMemoryLeaks(subject, file: file, line: line) - return subject - } - var ratingPromptOpenCount: Int { - UserDefaults.standard.object( + userDefaults.object( forKey: RatingPromptManager.UserDefaultsKey.keyRatingPromptRequestCount.rawValue ) as? Int ?? 0 } var lastCrashDateKey: Date? { - UserDefaults.standard.object( + userDefaults.object( forKey: RatingPromptManager.UserDefaultsKey.keyLastCrashDateKey.rawValue ) as? Date } From b4c6f8441bdf568cce3ccaff0ce2de628ce4c314 Mon Sep 17 00:00:00 2001 From: Laurie Marceau Date: Tue, 21 Jan 2025 13:31:00 -0500 Subject: [PATCH 3/3] Use time interval instead of calendar --- firefox-ios/Client/RatingPromptManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firefox-ios/Client/RatingPromptManager.swift b/firefox-ios/Client/RatingPromptManager.swift index 0f2e53fabfbc..56fc64cf9157 100644 --- a/firefox-ios/Client/RatingPromptManager.swift +++ b/firefox-ios/Client/RatingPromptManager.swift @@ -168,7 +168,7 @@ final class RatingPromptManager { forKey: UserDefaultsKey.keyLastCrashDateKey.rawValue ) as? Date else { return false } - let threeDaysAgo = Calendar.current.date(byAdding: .day, value: -3, to: Date())! + let threeDaysAgo = Date(timeIntervalSinceNow: -(3 * 24 * 60 * 60)) return lastCrashDate >= threeDaysAgo } }