From d4d4b997e35291a42e87fdf604ec7e28e92d3da7 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 26 Jul 2024 22:02:58 +1000 Subject: [PATCH 01/16] Pixel WIP --- Core/Pixel.swift | 3 + Core/PixelEvent.swift | 22 +++ DuckDuckGo/AppDelegate.swift | 3 +- DuckDuckGo/DaxDialogs.swift | 24 ++- DuckDuckGo/MainViewController.swift | 12 ++ .../OnboardingSuggestionsViewModel.swift | 48 +++++- .../Pixels/OnboardingPixelReporter.swift | 125 +++++++++++++- DuckDuckGo/TabViewController.swift | 20 ++- .../OnboardingPixelReporterTests.swift | 153 ++++++++++++++++++ ...OnboardingSuggestionsViewModelsTests.swift | 107 +++++++++++- 10 files changed, 506 insertions(+), 11 deletions(-) diff --git a/Core/Pixel.swift b/Core/Pixel.swift index 9a31ed5f56..a50f8911a0 100644 --- a/Core/Pixel.swift +++ b/Core/Pixel.swift @@ -143,6 +143,9 @@ public struct PixelParameters { // Autofill public static let countBucket = "count_bucket" + + // Privacy Dashboard + public static let daysSinceInstall = "daysSinceInstall" } public struct PixelValues { diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index e6c6e2014b..2cc7266c2e 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -142,6 +142,17 @@ extension Pixel { case onboardingIntroShownUnique case onboardingIntroComparisonChartShownUnique case onboardingIntroChooseBrowserCTAPressed + case onboardingContextualSearchSayDuckUnique + case onboardingContextualSearchMightyDuckUnique + case onboardingContextualSearchWeatherUnique + case onboardingContextualSearchSurpriseMeUnique + case onboardingContextualSearchCustomUnique + case onboardingContextualSiteESPNUnique + case onboardingContextualSiteYahooUnique + case onboardingContextualSiteEbayUnique + case onboardingContextualSiteSurpriseMeUnique + case onboardingContextualSiteCustomUnique + case onboardingContextualSecondSiteVisitUnique case daxDialogsSerp case daxDialogsWithoutTrackers @@ -865,6 +876,17 @@ extension Pixel.Event { case .onboardingIntroShownUnique: return "m_preonboarding_intro_shown_unique" case .onboardingIntroComparisonChartShownUnique: return "m_preonboarding_comparison_chart_shown_unique" case .onboardingIntroChooseBrowserCTAPressed: return "m_preonboarding_choose_browser_pressed" + case .onboardingContextualSearchSayDuckUnique: return "m_onboarding_search_say_duck_unique" + case .onboardingContextualSearchMightyDuckUnique: return "m_onboarding_search_mighty_duck_unique" + case .onboardingContextualSearchWeatherUnique: return "m_onboarding_search_weather_unique" + case .onboardingContextualSearchSurpriseMeUnique: return "m_onboarding_search_surprise_me_unique" + case .onboardingContextualSearchCustomUnique: return "m_onboarding_search_custom_unique" + case .onboardingContextualSiteESPNUnique: return "m_onboarding_visit_site_espn_unique" + case .onboardingContextualSiteYahooUnique: return "m_onboarding_visit_site_yahoo_unique" + case .onboardingContextualSiteEbayUnique: return "m_onboarding_visit_site_ebay_unique" + case .onboardingContextualSiteSurpriseMeUnique: return "m_onboarding_visit_site_surprise_me_unique" + case .onboardingContextualSiteCustomUnique: return "m_onboarding_visit_site_custom_unique" + case .onboardingContextualSecondSiteVisitUnique: return "m_second_sitevisit_unique" case .daxDialogsSerp: return "m_dx_s" case .daxDialogsWithoutTrackers: return "m_dx_wo" diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index b7337df33d..7c5a123e04 100644 --- a/DuckDuckGo/AppDelegate.swift +++ b/DuckDuckGo/AppDelegate.swift @@ -317,7 +317,8 @@ import WebKit privacyProDataReporter: privacyProDataReporter, variantManager: variantManager, contextualOnboardingPresenter: ContextualOnboardingPresenter(variantManager: variantManager), - contextualOnboardingLogic: daxDialogs) + contextualOnboardingLogic: daxDialogs, + contextualOnboardingCustomSearchPixelReporting: OnboardingPixelReporter()) main.loadViewIfNeeded() syncErrorHandler.alertPresenter = main diff --git a/DuckDuckGo/DaxDialogs.swift b/DuckDuckGo/DaxDialogs.swift index 3f584baa50..68a0b3355d 100644 --- a/DuckDuckGo/DaxDialogs.swift +++ b/DuckDuckGo/DaxDialogs.swift @@ -39,6 +39,9 @@ protocol NewTabDialogSpecProvider { protocol ContextualOnboardingLogic { var isShowingFireDialog: Bool { get } var shouldShowPrivacyButtonPulse: Bool { get } + var isShowingSearchSuggestions: Bool { get } + var isShowingSitesSuggestions: Bool { get } + func setSearchMessageSeen() func setFireEducationMessageSeen() func setFinalOnboardingDialogSeen() @@ -206,6 +209,8 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { // So we can avoid showing two dialogs for the same page private var lastURLDaxDialogReturnedFor: URL? + private var currentHomeSpec: HomeScreenSpec? + /// Use singleton accessor, this is only accessible for tests init(settings: DaxDialogsSettings = DefaultDaxDialogsSettings(), entityProviding: EntityProviding, @@ -253,6 +258,16 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { visitedSiteAndFireButtonSeen } + var isShowingSearchSuggestions: Bool { + guard isNewOnboarding else { return false } + return currentHomeSpec == .initial + } + + var isShowingSitesSuggestions: Bool { + guard isNewOnboarding else { return false } + return lastShownDaxDialogType.flatMap(BrowsingSpec.SpecType.init(rawValue:)) == .visitWebsite || currentHomeSpec == .subsequent + } + var isEnabled: Bool { // skip dax dialogs in integration tests guard ProcessInfo.processInfo.environment["DAXDIALOGS"] != "false" else { return false } @@ -482,6 +497,9 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { isFacebookOrGoogle(privacyInfo.url) || isOwnedByFacebookOrGoogle(host) != nil || blockedEntityNames(privacyInfo.trackerInfo) != nil } + // Reset current home spec when navigating + currentHomeSpec = nil + guard isEnabled, nextHomeScreenMessageOverride == nil else { return nil } guard let host = privacyInfo.domain else { return nil } @@ -528,7 +546,11 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { } func nextHomeScreenMessageNew() -> HomeScreenSpec? { - guard let homeScreenSpec = peekNextHomeScreenMessageExperiment() else { return nil } + guard let homeScreenSpec = peekNextHomeScreenMessageExperiment() else { + currentHomeSpec = nil + return nil + } + currentHomeSpec = homeScreenSpec return homeScreenSpec } diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 1c22507d7b..0433304b31 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -103,6 +103,7 @@ class MainViewController: UIViewController { private let variantManager: VariantManager private let tutorialSettings: TutorialSettings private let contextualOnboardingLogic: ContextualOnboardingLogic + private let contextualOnboardingPixelReporting: OnboardingCustomSearchPixelReporting @UserDefaultsWrapper(key: .syncDidShowSyncPausedByFeatureFlagAlert, defaultValue: false) private var syncDidShowSyncPausedByFeatureFlagAlert: Bool @@ -184,6 +185,7 @@ class MainViewController: UIViewController { variantManager: VariantManager, contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, + contextualOnboardingCustomSearchPixelReporting: OnboardingCustomSearchPixelReporting tutorialSettings: TutorialSettings = DefaultTutorialSettings() ) { self.bookmarksDatabase = bookmarksDatabase @@ -212,6 +214,7 @@ class MainViewController: UIViewController { self.variantManager = variantManager self.tutorialSettings = tutorialSettings self.contextualOnboardingLogic = contextualOnboardingLogic + self.contextualOnboardingPixelReporting = contextualOnboardingCustomSearchPixelReporting super.init(nibName: nil, bundle: nil) @@ -1288,6 +1291,14 @@ class MainViewController: UIViewController { } } + func fireOnboardingCustomSearchPixelIfNeeded(query: String) { + if contextualOnboardingLogic.isShowingSearchSuggestions { + contextualOnboardingPixelReporting.trackCustomSearch() + } else if contextualOnboardingLogic.isShowingSitesSuggestions { + contextualOnboardingPixelReporting.trackCustomSite() + } + } + func animateBackgroundTab() { showBars() tabSwitcherButton.incrementAnimated() @@ -1709,6 +1720,7 @@ extension MainViewController: OmniBarDelegate { loadQuery(query) hideSuggestionTray() showHomeRowReminder() + fireOnboardingCustomSearchPixelIfNeeded(query: query) } func onPrivacyIconPressed() { diff --git a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/OnboardingSuggestionsViewModel.swift b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/OnboardingSuggestionsViewModel.swift index 11a8ee994d..ff79614f62 100644 --- a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/OnboardingSuggestionsViewModel.swift +++ b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/OnboardingSuggestionsViewModel.swift @@ -27,12 +27,16 @@ protocol OnboardingNavigationDelegate: AnyObject { struct OnboardingSearchSuggestionsViewModel { let suggestedSearchesProvider: OnboardingSuggestionsItemsProviding weak var delegate: OnboardingNavigationDelegate? + private let pixelReporter: OnboardingSearchSuggestionsPixelReporting init( suggestedSearchesProvider: OnboardingSuggestionsItemsProviding = OnboardingSuggestedSearchesProvider(), - delegate: OnboardingNavigationDelegate? = nil) { + delegate: OnboardingNavigationDelegate? = nil, + pixelReporter: OnboardingSearchSuggestionsPixelReporting = OnboardingPixelReporter() + ) { self.suggestedSearchesProvider = suggestedSearchesProvider self.delegate = delegate + self.pixelReporter = pixelReporter } var itemsList: [ContextualOnboardingListItem] { @@ -40,22 +44,44 @@ struct OnboardingSearchSuggestionsViewModel { } func listItemPressed(_ item: ContextualOnboardingListItem) { + firePixel(for: item) delegate?.searchFor(item.title) } + + // Temporary pixels, they will be removed. + // This avoid refactoring `OnboardingSuggestedSearchesProvider` and `ContextualOnboardingListItem` when removing the pixels + // This is covered by tests + private func firePixel(for item: ContextualOnboardingListItem) { + guard let index = itemsList.firstIndex(of: item) else { return } + switch index { + case 0: + pixelReporter.trackSearchSuggestionSayDuck() + case 1: + pixelReporter.trackSearchSuggestionMightyDuck() + case 2: + pixelReporter.trackSearchSuggestionWeather() + case 3: + pixelReporter.trackSearchSuggestionSurpriseMe() + default: break + } + } } struct OnboardingSiteSuggestionsViewModel { let suggestedSitesProvider: OnboardingSuggestionsItemsProviding weak var delegate: OnboardingNavigationDelegate? + private let pixelReporter: OnboardingSiteSuggestionsPixelReporting init( title: String, suggestedSitesProvider: OnboardingSuggestionsItemsProviding = OnboardingSuggestedSitesProvider(), delegate: OnboardingNavigationDelegate? = nil + pixelReporter: OnboardingSiteSuggestionsPixelReporting = OnboardingPixelReporter() ) { self.title = title self.suggestedSitesProvider = suggestedSitesProvider self.delegate = delegate + self.pixelReporter = pixelReporter } let title: String @@ -66,6 +92,26 @@ struct OnboardingSiteSuggestionsViewModel { func listItemPressed(_ item: ContextualOnboardingListItem) { guard let url = URL(string: item.title) else { return } + firePixel(for: item) delegate?.navigateTo(url: url) } + + // Temporary pixels, they will be removed. + // This avoid refactoring `OnboardingSuggestedSitesProvider` and `ContextualOnboardingListItem` when removing the pixels + // This is covered by tests + private func firePixel(for item: ContextualOnboardingListItem) { + guard let index = itemsList.firstIndex(of: item) else { return } + switch index { + case 0: + pixelReporter.trackSiteSuggestionESPN() + case 1: + pixelReporter.trackSiteSuggestionYahoo() + case 2: + pixelReporter.trackSiteSuggestionEbay() + case 3: + pixelReporter.trackSiteSuggestionSurpriseMe() + default: break + } + } + } diff --git a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift index d1310a5627..938004ec3e 100644 --- a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift +++ b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift @@ -19,6 +19,7 @@ import Foundation import Core +import BrowserServicesKit // MARK: - Pixel Fire Interface @@ -49,15 +50,45 @@ protocol OnboardingIntroPixelReporting: OnboardingIntroImpressionReporting { func trackChooseBrowserCTAAction() } +protocol OnboardingSearchSuggestionsPixelReporting { + func trackSearchSuggestionSayDuck() + func trackSearchSuggestionMightyDuck() + func trackSearchSuggestionWeather() + func trackSearchSuggestionSurpriseMe() +} + +protocol OnboardingSiteSuggestionsPixelReporting { + func trackSiteSuggestionESPN() + func trackSiteSuggestionYahoo() + func trackSiteSuggestionEbay() + func trackSiteSuggestionSurpriseMe() +} + +protocol OnboardingCustomSearchPixelReporting { + func trackCustomSearch() + func trackCustomSite() + func trackSecondSiteVisit() +} + +protocol OnboardingPrivacyDashboardPixelReporting { + func trackPrivacyDashboardOpen() +} + // MARK: - Implementation final class OnboardingPixelReporter { private let pixel: OnboardingPixelFiring.Type private let uniquePixel: OnboardingPixelFiring.Type + private let daysSinceInstallProvider: DaysSinceInstallProviding - init(pixel: OnboardingPixelFiring.Type = Pixel.self, uniquePixel: OnboardingPixelFiring.Type = UniquePixel.self) { + init( + pixel: OnboardingPixelFiring.Type = Pixel.self, + uniquePixel: OnboardingPixelFiring.Type = UniquePixel.self, + daysSinceInstallProvider: DaysSinceInstallProviding = DaysSinceInstallProvider() + ) { self.pixel = pixel self.uniquePixel = uniquePixel + self.daysSinceInstallProvider = daysSinceInstallProvider } private func fire(event: Pixel.Event, unique: Bool, additionalParameters: [String: String] = [:]) { @@ -71,7 +102,7 @@ final class OnboardingPixelReporter { } -// MARK: - OnboardingAnalytics + Intro +// MARK: - OnboardingPixelReporter + Intro extension OnboardingPixelReporter: OnboardingIntroPixelReporting { @@ -88,3 +119,93 @@ extension OnboardingPixelReporter: OnboardingIntroPixelReporting { } } + +// MARK: - OnboardingPixelReporter + List + +extension OnboardingPixelReporter: OnboardingSearchSuggestionsPixelReporting { + + func trackSearchSuggestionSayDuck() { + fire(event: .onboardingContextualSearchSayDuckUnique, unique: true) + } + + func trackSearchSuggestionMightyDuck() { + fire(event: .onboardingContextualSearchMightyDuckUnique, unique: true) + } + + func trackSearchSuggestionWeather() { + fire(event: .onboardingContextualSearchWeatherUnique, unique: true) + } + + func trackSearchSuggestionSurpriseMe() { + fire(event: .onboardingContextualSearchSurpriseMeUnique, unique: true) + } + +} + +extension OnboardingPixelReporter: OnboardingSiteSuggestionsPixelReporting { + + func trackSiteSuggestionESPN() { + fire(event: .onboardingContextualSiteESPNUnique, unique: true) + } + + func trackSiteSuggestionYahoo() { + fire(event: .onboardingContextualSiteYahooUnique, unique: true) + } + + func trackSiteSuggestionEbay() { + fire(event: .onboardingContextualSiteEbayUnique, unique: true) + } + + func trackSiteSuggestionSurpriseMe() { + fire(event: .onboardingContextualSiteSurpriseMeUnique, unique: true) + } + +} + +// MARK: - OnboardingPixelReporter + Custom Search + +extension OnboardingPixelReporter: OnboardingCustomSearchPixelReporting { + + func trackCustomSearch() { + fire(event: .onboardingContextualSearchCustomUnique, unique: true) + } + + func trackCustomSite() { + fire(event: .onboardingContextualSiteCustomUnique, unique: true) + } + + func trackSecondSiteVisit() { + fire(event: .onboardingContextualSecondSiteVisitUnique, unique: true) + } + +} + +// MARK: - OnboardingPixelReporter + Privacy Dashboard + +extension OnboardingPixelReporter: OnboardingPrivacyDashboardPixelReporting { + + func trackPrivacyDashboardOpen() { + guard let daysSinceInstall = daysSinceInstallProvider.daysSinceInstall else { return } + fire(event: .privacyDashboardOpened, unique: true, additionalParameters: ["daysSinceInstall": String(daysSinceInstall)]) + } + +} + +protocol DaysSinceInstallProviding { + var daysSinceInstall: Int? { get } +} + +final class DaysSinceInstallProvider: DaysSinceInstallProviding { + private let store: StatisticsStore + private let dateProvider: () -> Date + + init(store: StatisticsStore = StatisticsUserDefaults(), dateProvider: @escaping () -> Date = Date.init) { + self.store = store + self.dateProvider = dateProvider + } + + var daysSinceInstall: Int? { + guard let installDate = store.installDate else { return nil } + return Calendar.current.numberOfDaysBetween(installDate, and: dateProvider()) + } +} diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index cd5ee3095c..c0a70725da 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -297,7 +297,8 @@ class TabViewController: UIViewController { duckPlayer: DuckPlayerProtocol, privacyProDataReporter: PrivacyProDataReporting, contextualOnboardingPresenter: ContextualOnboardingPresenting, - contextualOnboardingLogic: ContextualOnboardingLogic) -> TabViewController { + contextualOnboardingLogic: ContextualOnboardingLogic, + daysSinceInstallProvider: DaysSinceInstallProviding = DaysSinceInstallProvider()) -> TabViewController { let storyboard = UIStoryboard(name: "Tab", bundle: nil) let controller = storyboard.instantiateViewController(identifier: "TabViewController", creator: { coder in TabViewController(coder: coder, @@ -309,7 +310,9 @@ class TabViewController: UIViewController { duckPlayer: duckPlayer, privacyProDataReporter: privacyProDataReporter, contextualOnboardingPresenter: contextualOnboardingPresenter, - contextualOnboardingLogic: contextualOnboardingLogic) + contextualOnboardingLogic: contextualOnboardingLogic, + daysSinceInstallProvider: daysSinceInstallProvider + ) }) return controller } @@ -325,6 +328,7 @@ class TabViewController: UIViewController { let contextualOnboardingPresenter: ContextualOnboardingPresenting let contextualOnboardingLogic: ContextualOnboardingLogic + let daysSinceInstallProvider: DaysSinceInstallProviding required init?(coder aDecoder: NSCoder, tabModel: Tab, @@ -335,7 +339,8 @@ class TabViewController: UIViewController { duckPlayer: DuckPlayerProtocol, privacyProDataReporter: PrivacyProDataReporting, contextualOnboardingPresenter: ContextualOnboardingPresenting, - contextualOnboardingLogic: ContextualOnboardingLogic) { + contextualOnboardingLogic: ContextualOnboardingLogic, + daysSinceInstallProvider: DaysSinceInstallProviding) { self.tabModel = tabModel self.appSettings = appSettings self.bookmarksDatabase = bookmarksDatabase @@ -347,6 +352,7 @@ class TabViewController: UIViewController { self.privacyProDataReporter = privacyProDataReporter self.contextualOnboardingPresenter = contextualOnboardingPresenter self.contextualOnboardingLogic = contextualOnboardingLogic + self.daysSinceInstallProvider = daysSinceInstallProvider super.init(coder: aDecoder) } @@ -922,7 +928,13 @@ class TabViewController: UIViewController { } func showPrivacyDashboard() { - Pixel.fire(pixel: .privacyDashboardOpened) + func firePrivacyDashboardOpenPixel() { + let additionalParameters = daysSinceInstallProvider.daysSinceInstall + .flatMap { [PixelParameters.daysSinceInstall: String($0)] } ?? [:] + UniquePixel.fire(pixel: .privacyDashboardOpened, withAdditionalParameters: additionalParameters, includedParameters: [.appVersion, .atb]) + } + + firePrivacyDashboardOpenPixel() performSegue(withIdentifier: "PrivacyDashboard", sender: self) } diff --git a/DuckDuckGoTests/OnboardingPixelReporterTests.swift b/DuckDuckGoTests/OnboardingPixelReporterTests.swift index b89a4f18a1..12beccd580 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterTests.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterTests.swift @@ -93,4 +93,157 @@ final class OnboardingPixelReporterTests: XCTestCase { XCTAssertEqual(OnboardingPixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) } + // MARK: - List + + func testWhenTrackSearchSuggestionSayDuckThenExpectedEventIsCalled() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSearchSayDuckUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSearchSuggestionSayDuck() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_search_say_duck_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackSearchSuggestionMightyDuckThenExpectedEventIsCalled() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSearchMightyDuckUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSearchSuggestionMightyDuck() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_search_mighty_duck_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackSearchSuggestionWeatherThenExpectedEventIsCalled() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSearchWeatherUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSearchSuggestionWeather() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_search_weather_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackSearchSuggestionSurpriseMeThenExpectedEventIsCalled() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSearchSurpriseMeUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSearchSuggestionSurpriseMe() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_search_surprise_me_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackSiteSuggestionESPNThenExpectedEventIsCalled() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSiteESPNUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSiteSuggestionESPN() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_espn_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackSiteSuggestionYahooThenExpectedEventIsCalled() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSiteYahooUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSiteSuggestionYahoo() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_yahoo_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackSiteSuggestionEbayThenExpectedEventIsCalled() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSiteEbayUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSiteSuggestionEbay() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_ebay_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackSiteSuggestionSurpriseMeThenExpectedEventIsCalled() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSiteSurpriseMeUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSiteSuggestionSurpriseMe() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_surprise_me_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } } diff --git a/DuckDuckGoTests/OnboardingSuggestionsViewModelsTests.swift b/DuckDuckGoTests/OnboardingSuggestionsViewModelsTests.swift index b55a552d82..80bdcf9bc3 100644 --- a/DuckDuckGoTests/OnboardingSuggestionsViewModelsTests.swift +++ b/DuckDuckGoTests/OnboardingSuggestionsViewModelsTests.swift @@ -18,6 +18,7 @@ // import XCTest +import Core @testable import DuckDuckGo final class OnboardingSuggestionsViewModelsTests: XCTestCase { @@ -25,17 +26,20 @@ final class OnboardingSuggestionsViewModelsTests: XCTestCase { var navigationDelegate: CapturingOnboardingNavigationDelegate! var searchSuggestionsVM: OnboardingSearchSuggestionsViewModel! var siteSuggestionsVM: OnboardingSiteSuggestionsViewModel! + var pixelReporterMock: OnboardingSuggestionsPixelReporterMock! override func setUp() { suggestionsProvider = MockOnboardingSuggestionsProvider() navigationDelegate = CapturingOnboardingNavigationDelegate() - searchSuggestionsVM = OnboardingSearchSuggestionsViewModel(suggestedSearchesProvider: suggestionsProvider, delegate: navigationDelegate) - siteSuggestionsVM = OnboardingSiteSuggestionsViewModel(title: "", suggestedSitesProvider: suggestionsProvider, delegate: navigationDelegate) + pixelReporterMock = OnboardingSuggestionsPixelReporterMock() + searchSuggestionsVM = OnboardingSearchSuggestionsViewModel(suggestedSearchesProvider: suggestionsProvider, delegate: navigationDelegate, pixelReporter: pixelReporterMock) + siteSuggestionsVM = OnboardingSiteSuggestionsViewModel(title: "", suggestedSitesProvider: suggestionsProvider, delegate: navigationDelegate, pixelReporter: pixelReporterMock) } override func tearDown() { suggestionsProvider = nil navigationDelegate = nil + pixelReporterMock = nil searchSuggestionsVM = nil siteSuggestionsVM = nil } @@ -93,6 +97,60 @@ final class OnboardingSuggestionsViewModelsTests: XCTestCase { XCTAssertEqual(navigationDelegate.urlToNavigateTo, URL(string: randomItem.title)) } + // MARK: - Pixels + + func testWhenSearchSuggestionsTapped_ThenPixelReporterIsCalled() { + // GIVEN + let searches: [ContextualOnboardingListItem] = [ + ContextualOnboardingListItem.search(title: "First"), + ContextualOnboardingListItem.search(title: "Second"), + ContextualOnboardingListItem.search(title: "Third"), + ContextualOnboardingListItem.surprise(title: "Surprise"), + ] + suggestionsProvider.list = searches + XCTAssertFalse(pixelReporterMock.didCallTrackSearchSayDuck) + XCTAssertFalse(pixelReporterMock.didCallTrackSearchMightyDuck) + XCTAssertFalse(pixelReporterMock.didCallTrackSearchWeather) + XCTAssertFalse(pixelReporterMock.didCallTrackSearchSurpriseMe) + + // WHEN + searches.forEach { searchItem in + searchSuggestionsVM.listItemPressed(searchItem) + } + + // THEN + XCTAssertTrue(pixelReporterMock.didCallTrackSearchSayDuck) + XCTAssertTrue(pixelReporterMock.didCallTrackSearchMightyDuck) + XCTAssertTrue(pixelReporterMock.didCallTrackSearchWeather) + XCTAssertTrue(pixelReporterMock.didCallTrackSearchSurpriseMe) + } + + func testWhenSiteSuggestionsTapped_ThenPixelReporterIsCalled() { + // GIVEN + let searches: [ContextualOnboardingListItem] = [ + ContextualOnboardingListItem.site(title: "First"), + ContextualOnboardingListItem.site(title: "Second"), + ContextualOnboardingListItem.site(title: "Third"), + ContextualOnboardingListItem.surprise(title: "Surprise"), + ] + suggestionsProvider.list = searches + XCTAssertFalse(pixelReporterMock.didCallTrackSiteESPN) + XCTAssertFalse(pixelReporterMock.didCallTrackSiteYahoo) + XCTAssertFalse(pixelReporterMock.didCallTrackSiteEbay) + XCTAssertFalse(pixelReporterMock.didCallTrackSiteSurpriseMe) + + // WHEN + searches.forEach { searchItem in + siteSuggestionsVM.listItemPressed(searchItem) + } + + // THEN + XCTAssertTrue(pixelReporterMock.didCallTrackSiteESPN) + XCTAssertTrue(pixelReporterMock.didCallTrackSiteYahoo) + XCTAssertTrue(pixelReporterMock.didCallTrackSiteEbay) + XCTAssertTrue(pixelReporterMock.didCallTrackSiteSurpriseMe) + } + } class MockOnboardingSuggestionsProvider: OnboardingSuggestionsItemsProviding { @@ -111,3 +169,48 @@ class CapturingOnboardingNavigationDelegate: OnboardingNavigationDelegate { urlToNavigateTo = url } } + +final class OnboardingSuggestionsPixelReporterMock: OnboardingSiteSuggestionsPixelReporting, OnboardingSearchSuggestionsPixelReporting { + private(set) var didCallTrackSearchSayDuck = false + private(set) var didCallTrackSearchMightyDuck = false + private(set) var didCallTrackSearchWeather = false + private(set) var didCallTrackSearchSurpriseMe = false + + private(set) var didCallTrackSiteESPN = false + private(set) var didCallTrackSiteYahoo = false + private(set) var didCallTrackSiteEbay = false + private(set) var didCallTrackSiteSurpriseMe = false + + func trackSearchSuggestionSayDuck() { + didCallTrackSearchSayDuck = true + } + + func trackSearchSuggestionMightyDuck() { + didCallTrackSearchMightyDuck = true + } + + func trackSearchSuggestionWeather() { + didCallTrackSearchWeather = true + } + + func trackSearchSuggestionSurpriseMe() { + didCallTrackSearchSurpriseMe = true + } + + func trackSiteSuggestionESPN() { + didCallTrackSiteESPN = true + } + + func trackSiteSuggestionYahoo() { + didCallTrackSiteYahoo = true + } + + func trackSiteSuggestionEbay() { + didCallTrackSiteEbay = true + } + + func trackSiteSuggestionSurpriseMe() { + didCallTrackSiteSurpriseMe = true + } + +} From 7f261a5b1f126aec476e57b34e5a59ba9715926d Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 26 Jul 2024 15:49:04 +0200 Subject: [PATCH 02/16] add visit second site pixel --- .../Pixels/OnboardingPixelReporter.swift | 12 ++++++++++-- DuckDuckGo/TabViewController.swift | 8 +++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift index 938004ec3e..faf5f6e959 100644 --- a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift +++ b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift @@ -80,15 +80,19 @@ final class OnboardingPixelReporter { private let pixel: OnboardingPixelFiring.Type private let uniquePixel: OnboardingPixelFiring.Type private let daysSinceInstallProvider: DaysSinceInstallProviding + private let userDefaults: UserDefaults + private let siteVisitedUserDefaultsKey = "com.duckduckgo.ios.site-visited" init( pixel: OnboardingPixelFiring.Type = Pixel.self, uniquePixel: OnboardingPixelFiring.Type = UniquePixel.self, - daysSinceInstallProvider: DaysSinceInstallProviding = DaysSinceInstallProvider() + daysSinceInstallProvider: DaysSinceInstallProviding = DaysSinceInstallProvider(), + userDefaults: UserDefaults = UserDefaults.standard ) { self.pixel = pixel self.uniquePixel = uniquePixel self.daysSinceInstallProvider = daysSinceInstallProvider + self.userDefaults = userDefaults } private func fire(event: Pixel.Event, unique: Bool, additionalParameters: [String: String] = [:]) { @@ -175,7 +179,11 @@ extension OnboardingPixelReporter: OnboardingCustomSearchPixelReporting { } func trackSecondSiteVisit() { - fire(event: .onboardingContextualSecondSiteVisitUnique, unique: true) + if userDefaults.bool(forKey: siteVisitedUserDefaultsKey) { + fire(event: .onboardingContextualSecondSiteVisitUnique, unique: true) + } else { + userDefaults.set(true, forKey: siteVisitedUserDefaultsKey) + } } } diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index c0a70725da..bf62850769 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -298,6 +298,7 @@ class TabViewController: UIViewController { privacyProDataReporter: PrivacyProDataReporting, contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, + onboardingPixelReporter: OnboardingPixelReporter = OnboardingPixelReporter(), daysSinceInstallProvider: DaysSinceInstallProviding = DaysSinceInstallProvider()) -> TabViewController { let storyboard = UIStoryboard(name: "Tab", bundle: nil) let controller = storyboard.instantiateViewController(identifier: "TabViewController", creator: { coder in @@ -311,6 +312,7 @@ class TabViewController: UIViewController { privacyProDataReporter: privacyProDataReporter, contextualOnboardingPresenter: contextualOnboardingPresenter, contextualOnboardingLogic: contextualOnboardingLogic, + onboardingPixelReporter: onboardingPixelReporter, daysSinceInstallProvider: daysSinceInstallProvider ) }) @@ -329,6 +331,7 @@ class TabViewController: UIViewController { let contextualOnboardingPresenter: ContextualOnboardingPresenting let contextualOnboardingLogic: ContextualOnboardingLogic let daysSinceInstallProvider: DaysSinceInstallProviding + let onboardingPixelReporter: OnboardingPixelReporter required init?(coder aDecoder: NSCoder, tabModel: Tab, @@ -340,6 +343,7 @@ class TabViewController: UIViewController { privacyProDataReporter: PrivacyProDataReporting, contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, + onboardingPixelReporter: OnboardingPixelReporter, daysSinceInstallProvider: DaysSinceInstallProviding) { self.tabModel = tabModel self.appSettings = appSettings @@ -352,6 +356,7 @@ class TabViewController: UIViewController { self.privacyProDataReporter = privacyProDataReporter self.contextualOnboardingPresenter = contextualOnboardingPresenter self.contextualOnboardingLogic = contextualOnboardingLogic + self.onboardingPixelReporter = onboardingPixelReporter self.daysSinceInstallProvider = daysSinceInstallProvider super.init(coder: aDecoder) } @@ -1321,7 +1326,8 @@ extension TabViewController: WKNavigationDelegate { onWebpageDidFinishLoading() instrumentation.didLoadURL() checkLoginDetectionAfterNavigation() - + onboardingPixelReporter.trackSecondSiteVisit() + // definitely finished with any potential login cycle by this point, so don't try and handle it any more detectedLoginURL = nil updatePreview() From ec057322b7d53b490c06cba3e524d3bb022e5f1f Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Wed, 31 Jul 2024 10:25:55 +1000 Subject: [PATCH 03/16] Refactor Pixel logic to use generic pixels for tracking suggestion options --- Core/PixelEvent.swift | 22 +-- DuckDuckGo.xcodeproj/project.pbxproj | 4 + .../OnboardingSuggestionsViewModel.swift | 43 +----- .../Pixels/OnboardingPixelReporter.swift | 42 +----- .../OnboardingNavigationDelegateTests.swift | 3 +- .../OnboardingPixelReporterMock.swift | 56 ++++++++ .../OnboardingPixelReporterTests.swift | 129 ++---------------- ...OnboardingSuggestionsViewModelsTests.swift | 69 +--------- .../TabViewControllerDaxDialogTests.swift | 3 +- 9 files changed, 92 insertions(+), 279 deletions(-) create mode 100644 DuckDuckGoTests/OnboardingPixelReporterMock.swift diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index 2cc7266c2e..2e0e82fd41 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -142,15 +142,9 @@ extension Pixel { case onboardingIntroShownUnique case onboardingIntroComparisonChartShownUnique case onboardingIntroChooseBrowserCTAPressed - case onboardingContextualSearchSayDuckUnique - case onboardingContextualSearchMightyDuckUnique - case onboardingContextualSearchWeatherUnique - case onboardingContextualSearchSurpriseMeUnique + case onboardingContextualSearchOptionTappedUnique case onboardingContextualSearchCustomUnique - case onboardingContextualSiteESPNUnique - case onboardingContextualSiteYahooUnique - case onboardingContextualSiteEbayUnique - case onboardingContextualSiteSurpriseMeUnique + case onboardingContextualSiteOptionTappedUnique case onboardingContextualSiteCustomUnique case onboardingContextualSecondSiteVisitUnique @@ -876,17 +870,11 @@ extension Pixel.Event { case .onboardingIntroShownUnique: return "m_preonboarding_intro_shown_unique" case .onboardingIntroComparisonChartShownUnique: return "m_preonboarding_comparison_chart_shown_unique" case .onboardingIntroChooseBrowserCTAPressed: return "m_preonboarding_choose_browser_pressed" - case .onboardingContextualSearchSayDuckUnique: return "m_onboarding_search_say_duck_unique" - case .onboardingContextualSearchMightyDuckUnique: return "m_onboarding_search_mighty_duck_unique" - case .onboardingContextualSearchWeatherUnique: return "m_onboarding_search_weather_unique" - case .onboardingContextualSearchSurpriseMeUnique: return "m_onboarding_search_surprise_me_unique" + case .onboardingContextualSearchOptionTappedUnique: return "m_onboarding_search_option_tapped_unique" + case .onboardingContextualSiteOptionTappedUnique: return "m_onboarding_visit_site_option_tapped_unique" + case .onboardingContextualSecondSiteVisitUnique: return "m_second_sitevisit_unique" case .onboardingContextualSearchCustomUnique: return "m_onboarding_search_custom_unique" - case .onboardingContextualSiteESPNUnique: return "m_onboarding_visit_site_espn_unique" - case .onboardingContextualSiteYahooUnique: return "m_onboarding_visit_site_yahoo_unique" - case .onboardingContextualSiteEbayUnique: return "m_onboarding_visit_site_ebay_unique" - case .onboardingContextualSiteSurpriseMeUnique: return "m_onboarding_visit_site_surprise_me_unique" case .onboardingContextualSiteCustomUnique: return "m_onboarding_visit_site_custom_unique" - case .onboardingContextualSecondSiteVisitUnique: return "m_second_sitevisit_unique" case .daxDialogsSerp: return "m_dx_s" case .daxDialogsWithoutTrackers: return "m_dx_wo" diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 8a5ff2055e..306dd94459 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -655,6 +655,7 @@ 9F5E5AB22C3E606D00165F54 /* ContextualOnboardingPresenterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F5E5AB12C3E606D00165F54 /* ContextualOnboardingPresenterTests.swift */; }; 9F69331B2C5A16E200CD6A5D /* OnboardingDaxFavouritesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F69331A2C5A16E200CD6A5D /* OnboardingDaxFavouritesTests.swift */; }; 9F69331D2C5A191400CD6A5D /* MockTutorialSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F69331C2C5A191400CD6A5D /* MockTutorialSettings.swift */; }; + 9F6933192C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F6933182C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift */; }; 9F8007262C5261AF003EDAF4 /* MockPrivacyDataReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F8007252C5261AF003EDAF4 /* MockPrivacyDataReporter.swift */; }; 9F8FE9492BAE50E50071E372 /* Lottie in Frameworks */ = {isa = PBXBuildFile; productRef = 9F8FE9482BAE50E50071E372 /* Lottie */; }; 9F9EE4CE2C377D4900D4118E /* OnboardingFirePixelMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F9EE4CC2C377D3F00D4118E /* OnboardingFirePixelMock.swift */; }; @@ -2367,6 +2368,7 @@ 9F5E5AB12C3E606D00165F54 /* ContextualOnboardingPresenterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContextualOnboardingPresenterTests.swift; sourceTree = ""; }; 9F69331A2C5A16E200CD6A5D /* OnboardingDaxFavouritesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingDaxFavouritesTests.swift; sourceTree = ""; }; 9F69331C2C5A191400CD6A5D /* MockTutorialSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockTutorialSettings.swift; sourceTree = ""; }; + 9F6933182C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingPixelReporterMock.swift; sourceTree = ""; }; 9F8007252C5261AF003EDAF4 /* MockPrivacyDataReporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockPrivacyDataReporter.swift; sourceTree = ""; }; 9F9EE4CC2C377D3F00D4118E /* OnboardingFirePixelMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingFirePixelMock.swift; sourceTree = ""; }; 9F9EE4D32C37BB1300D4118E /* OnboardingView+Landing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OnboardingView+Landing.swift"; sourceTree = ""; }; @@ -4466,6 +4468,7 @@ children = ( 9F9EE4CC2C377D3F00D4118E /* OnboardingFirePixelMock.swift */, 9F4CC5142C47AD08006A96EB /* ContextualOnboardingPresenterMock.swift */, + 9F6933182C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift */, ); name = Mocks; sourceTree = ""; @@ -7486,6 +7489,7 @@ 9F23B8062C2BE22700950875 /* OnboardingIntroViewModelTests.swift in Sources */, 9F69331D2C5A191400CD6A5D /* MockTutorialSettings.swift in Sources */, 85D2187424BF25CD004373D2 /* FaviconsTests.swift in Sources */, + 9F6933192C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift in Sources */, 56D0602D2C383FD2003BAEB5 /* OnboardingSuggestedSearchesProviderTests.swift in Sources */, 85AD49EE2B6149110085D2D1 /* CookieStorageTests.swift in Sources */, 569437242BDD405400C0881B /* SyncBookmarksAdapterTests.swift in Sources */, diff --git a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/OnboardingSuggestionsViewModel.swift b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/OnboardingSuggestionsViewModel.swift index ff79614f62..2a4e631a65 100644 --- a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/OnboardingSuggestionsViewModel.swift +++ b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/OnboardingSuggestionsViewModel.swift @@ -44,27 +44,9 @@ struct OnboardingSearchSuggestionsViewModel { } func listItemPressed(_ item: ContextualOnboardingListItem) { - firePixel(for: item) + pixelReporter.trackSearchSuggetionOptionTapped() delegate?.searchFor(item.title) } - - // Temporary pixels, they will be removed. - // This avoid refactoring `OnboardingSuggestedSearchesProvider` and `ContextualOnboardingListItem` when removing the pixels - // This is covered by tests - private func firePixel(for item: ContextualOnboardingListItem) { - guard let index = itemsList.firstIndex(of: item) else { return } - switch index { - case 0: - pixelReporter.trackSearchSuggestionSayDuck() - case 1: - pixelReporter.trackSearchSuggestionMightyDuck() - case 2: - pixelReporter.trackSearchSuggestionWeather() - case 3: - pixelReporter.trackSearchSuggestionSurpriseMe() - default: break - } - } } struct OnboardingSiteSuggestionsViewModel { @@ -75,7 +57,7 @@ struct OnboardingSiteSuggestionsViewModel { init( title: String, suggestedSitesProvider: OnboardingSuggestionsItemsProviding = OnboardingSuggestedSitesProvider(), - delegate: OnboardingNavigationDelegate? = nil + delegate: OnboardingNavigationDelegate? = nil, pixelReporter: OnboardingSiteSuggestionsPixelReporting = OnboardingPixelReporter() ) { self.title = title @@ -92,26 +74,7 @@ struct OnboardingSiteSuggestionsViewModel { func listItemPressed(_ item: ContextualOnboardingListItem) { guard let url = URL(string: item.title) else { return } - firePixel(for: item) + pixelReporter.trackSiteSuggetionOptionTapped() delegate?.navigateTo(url: url) } - - // Temporary pixels, they will be removed. - // This avoid refactoring `OnboardingSuggestedSitesProvider` and `ContextualOnboardingListItem` when removing the pixels - // This is covered by tests - private func firePixel(for item: ContextualOnboardingListItem) { - guard let index = itemsList.firstIndex(of: item) else { return } - switch index { - case 0: - pixelReporter.trackSiteSuggestionESPN() - case 1: - pixelReporter.trackSiteSuggestionYahoo() - case 2: - pixelReporter.trackSiteSuggestionEbay() - case 3: - pixelReporter.trackSiteSuggestionSurpriseMe() - default: break - } - } - } diff --git a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift index faf5f6e959..2e123fab60 100644 --- a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift +++ b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift @@ -51,17 +51,11 @@ protocol OnboardingIntroPixelReporting: OnboardingIntroImpressionReporting { } protocol OnboardingSearchSuggestionsPixelReporting { - func trackSearchSuggestionSayDuck() - func trackSearchSuggestionMightyDuck() - func trackSearchSuggestionWeather() - func trackSearchSuggestionSurpriseMe() + func trackSearchSuggetionOptionTapped() } protocol OnboardingSiteSuggestionsPixelReporting { - func trackSiteSuggestionESPN() - func trackSiteSuggestionYahoo() - func trackSiteSuggestionEbay() - func trackSiteSuggestionSurpriseMe() + func trackSiteSuggetionOptionTapped() } protocol OnboardingCustomSearchPixelReporting { @@ -127,41 +121,17 @@ extension OnboardingPixelReporter: OnboardingIntroPixelReporting { // MARK: - OnboardingPixelReporter + List extension OnboardingPixelReporter: OnboardingSearchSuggestionsPixelReporting { - - func trackSearchSuggestionSayDuck() { - fire(event: .onboardingContextualSearchSayDuckUnique, unique: true) - } - - func trackSearchSuggestionMightyDuck() { - fire(event: .onboardingContextualSearchMightyDuckUnique, unique: true) - } - func trackSearchSuggestionWeather() { - fire(event: .onboardingContextualSearchWeatherUnique, unique: true) - } - - func trackSearchSuggestionSurpriseMe() { - fire(event: .onboardingContextualSearchSurpriseMeUnique, unique: true) + func trackSearchSuggetionOptionTapped() { + fire(event: .onboardingContextualSearchOptionTappedUnique, unique: true) } } extension OnboardingPixelReporter: OnboardingSiteSuggestionsPixelReporting { - func trackSiteSuggestionESPN() { - fire(event: .onboardingContextualSiteESPNUnique, unique: true) - } - - func trackSiteSuggestionYahoo() { - fire(event: .onboardingContextualSiteYahooUnique, unique: true) - } - - func trackSiteSuggestionEbay() { - fire(event: .onboardingContextualSiteEbayUnique, unique: true) - } - - func trackSiteSuggestionSurpriseMe() { - fire(event: .onboardingContextualSiteSurpriseMeUnique, unique: true) + func trackSiteSuggetionOptionTapped() { + fire(event: .onboardingContextualSiteOptionTappedUnique, unique: true) } } diff --git a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift index 1cdac96bae..e07d34761c 100644 --- a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift +++ b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift @@ -71,7 +71,8 @@ final class OnboardingNavigationDelegateTests: XCTestCase { privacyProDataReporter: MockPrivacyProDataReporter(), variantManager: MockVariantManager(), contextualOnboardingPresenter: ContextualOnboardingPresenterMock(), - contextualOnboardingLogic: ContextualOnboardingLogicMock()) + contextualOnboardingLogic: ContextualOnboardingLogicMock(), + contextualOnboardingCustomSearchPixelReporting: OnboardingPixelReporterMock()) let window = UIWindow(frame: UIScreen.main.bounds) window.rootViewController = UIViewController() window.makeKeyAndVisible() diff --git a/DuckDuckGoTests/OnboardingPixelReporterMock.swift b/DuckDuckGoTests/OnboardingPixelReporterMock.swift new file mode 100644 index 0000000000..2de06cc485 --- /dev/null +++ b/DuckDuckGoTests/OnboardingPixelReporterMock.swift @@ -0,0 +1,56 @@ +// +// OnboardingPixelReporterMock.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 +@testable import DuckDuckGo + +final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting, OnboardingSearchSuggestionsPixelReporting, OnboardingCustomSearchPixelReporting { + + private(set) var didCallTrackSearchOptionTapped = false + private(set) var didCallTrackSiteOptionTapped = false + private(set) var didCallTrackCustomSearch = false + private(set) var didCallTrackCustomSite = false + private(set) var didCallSecondSiteVisit = false { + didSet { + secondSiteVisitCounter += 1 + } + } + private(set) var secondSiteVisitCounter = 0 + + func trackSiteSuggetionOptionTapped() { + didCallTrackSiteOptionTapped = true + } + + func trackSearchSuggetionOptionTapped() { + didCallTrackSearchOptionTapped = true + } + + func trackCustomSearch() { + didCallTrackCustomSearch = true + } + + func trackCustomSite() { + didCallTrackCustomSite = true + } + + func trackSecondSiteVisit() { + didCallSecondSiteVisit = true + } + +} diff --git a/DuckDuckGoTests/OnboardingPixelReporterTests.swift b/DuckDuckGoTests/OnboardingPixelReporterTests.swift index 12beccd580..7764d8ad12 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterTests.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterTests.swift @@ -95,155 +95,42 @@ final class OnboardingPixelReporterTests: XCTestCase { // MARK: - List - func testWhenTrackSearchSuggestionSayDuckThenExpectedEventIsCalled() { + func testWhenTrackSearchSuggestionOptionTappedThenExpectedEventIsCalled() { // GIVEN - let expectedPixel = Pixel.Event.onboardingContextualSearchSayDuckUnique + let expectedPixel = Pixel.Event.onboardingContextualSearchOptionTappedUnique XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) // WHEN - sut.trackSearchSuggestionSayDuck() + sut.trackSearchSuggetionOptionTapped() // THEN XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) - XCTAssertEqual(expectedPixel.name, "m_onboarding_search_say_duck_unique") - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) - } - - func testWhenTrackSearchSuggestionMightyDuckThenExpectedEventIsCalled() { - // GIVEN - let expectedPixel = Pixel.Event.onboardingContextualSearchMightyDuckUnique - XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) - - // WHEN - sut.trackSearchSuggestionMightyDuck() - - // THEN - XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) - XCTAssertEqual(expectedPixel.name, "m_onboarding_search_mighty_duck_unique") - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) - } - - func testWhenTrackSearchSuggestionWeatherThenExpectedEventIsCalled() { - // GIVEN - let expectedPixel = Pixel.Event.onboardingContextualSearchWeatherUnique - XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) - - // WHEN - sut.trackSearchSuggestionWeather() - - // THEN - XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) - XCTAssertEqual(expectedPixel.name, "m_onboarding_search_weather_unique") - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) - } - - func testWhenTrackSearchSuggestionSurpriseMeThenExpectedEventIsCalled() { - // GIVEN - let expectedPixel = Pixel.Event.onboardingContextualSearchSurpriseMeUnique - XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) - - // WHEN - sut.trackSearchSuggestionSurpriseMe() - - // THEN - XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) - XCTAssertEqual(expectedPixel.name, "m_onboarding_search_surprise_me_unique") + XCTAssertEqual(expectedPixel.name, "m_onboarding_search_option_tapped_unique") XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) } - func testWhenTrackSiteSuggestionESPNThenExpectedEventIsCalled() { + func testWhenTrackSiteSuggestionThenExpectedEventIsCalled() { // GIVEN - let expectedPixel = Pixel.Event.onboardingContextualSiteESPNUnique + let expectedPixel = Pixel.Event.onboardingContextualSiteOptionTappedUnique XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) // WHEN - sut.trackSiteSuggestionESPN() + sut.trackSiteSuggetionOptionTapped() // THEN XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) - XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_espn_unique") + XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_option_tapped_unique") XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) } - func testWhenTrackSiteSuggestionYahooThenExpectedEventIsCalled() { - // GIVEN - let expectedPixel = Pixel.Event.onboardingContextualSiteYahooUnique - XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) - - // WHEN - sut.trackSiteSuggestionYahoo() - - // THEN - XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) - XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_yahoo_unique") - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) - } - - func testWhenTrackSiteSuggestionEbayThenExpectedEventIsCalled() { - // GIVEN - let expectedPixel = Pixel.Event.onboardingContextualSiteEbayUnique - XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) - - // WHEN - sut.trackSiteSuggestionEbay() - - // THEN - XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) - XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_ebay_unique") - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) - } - - func testWhenTrackSiteSuggestionSurpriseMeThenExpectedEventIsCalled() { - // GIVEN - let expectedPixel = Pixel.Event.onboardingContextualSiteSurpriseMeUnique - XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) - - // WHEN - sut.trackSiteSuggestionSurpriseMe() - - // THEN - XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) - XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_surprise_me_unique") - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) - } } diff --git a/DuckDuckGoTests/OnboardingSuggestionsViewModelsTests.swift b/DuckDuckGoTests/OnboardingSuggestionsViewModelsTests.swift index 80bdcf9bc3..2ce7ad24cc 100644 --- a/DuckDuckGoTests/OnboardingSuggestionsViewModelsTests.swift +++ b/DuckDuckGoTests/OnboardingSuggestionsViewModelsTests.swift @@ -26,12 +26,12 @@ final class OnboardingSuggestionsViewModelsTests: XCTestCase { var navigationDelegate: CapturingOnboardingNavigationDelegate! var searchSuggestionsVM: OnboardingSearchSuggestionsViewModel! var siteSuggestionsVM: OnboardingSiteSuggestionsViewModel! - var pixelReporterMock: OnboardingSuggestionsPixelReporterMock! + var pixelReporterMock: OnboardingPixelReporterMock! override func setUp() { suggestionsProvider = MockOnboardingSuggestionsProvider() navigationDelegate = CapturingOnboardingNavigationDelegate() - pixelReporterMock = OnboardingSuggestionsPixelReporterMock() + pixelReporterMock = OnboardingPixelReporterMock() searchSuggestionsVM = OnboardingSearchSuggestionsViewModel(suggestedSearchesProvider: suggestionsProvider, delegate: navigationDelegate, pixelReporter: pixelReporterMock) siteSuggestionsVM = OnboardingSiteSuggestionsViewModel(title: "", suggestedSitesProvider: suggestionsProvider, delegate: navigationDelegate, pixelReporter: pixelReporterMock) } @@ -108,10 +108,7 @@ final class OnboardingSuggestionsViewModelsTests: XCTestCase { ContextualOnboardingListItem.surprise(title: "Surprise"), ] suggestionsProvider.list = searches - XCTAssertFalse(pixelReporterMock.didCallTrackSearchSayDuck) - XCTAssertFalse(pixelReporterMock.didCallTrackSearchMightyDuck) - XCTAssertFalse(pixelReporterMock.didCallTrackSearchWeather) - XCTAssertFalse(pixelReporterMock.didCallTrackSearchSurpriseMe) + XCTAssertFalse(pixelReporterMock.didCallTrackSearchOptionTapped) // WHEN searches.forEach { searchItem in @@ -119,10 +116,7 @@ final class OnboardingSuggestionsViewModelsTests: XCTestCase { } // THEN - XCTAssertTrue(pixelReporterMock.didCallTrackSearchSayDuck) - XCTAssertTrue(pixelReporterMock.didCallTrackSearchMightyDuck) - XCTAssertTrue(pixelReporterMock.didCallTrackSearchWeather) - XCTAssertTrue(pixelReporterMock.didCallTrackSearchSurpriseMe) + XCTAssertTrue(pixelReporterMock.didCallTrackSearchOptionTapped) } func testWhenSiteSuggestionsTapped_ThenPixelReporterIsCalled() { @@ -134,10 +128,7 @@ final class OnboardingSuggestionsViewModelsTests: XCTestCase { ContextualOnboardingListItem.surprise(title: "Surprise"), ] suggestionsProvider.list = searches - XCTAssertFalse(pixelReporterMock.didCallTrackSiteESPN) - XCTAssertFalse(pixelReporterMock.didCallTrackSiteYahoo) - XCTAssertFalse(pixelReporterMock.didCallTrackSiteEbay) - XCTAssertFalse(pixelReporterMock.didCallTrackSiteSurpriseMe) + XCTAssertFalse(pixelReporterMock.didCallTrackSiteOptionTapped) // WHEN searches.forEach { searchItem in @@ -145,10 +136,7 @@ final class OnboardingSuggestionsViewModelsTests: XCTestCase { } // THEN - XCTAssertTrue(pixelReporterMock.didCallTrackSiteESPN) - XCTAssertTrue(pixelReporterMock.didCallTrackSiteYahoo) - XCTAssertTrue(pixelReporterMock.didCallTrackSiteEbay) - XCTAssertTrue(pixelReporterMock.didCallTrackSiteSurpriseMe) + XCTAssertTrue(pixelReporterMock.didCallTrackSiteOptionTapped) } } @@ -169,48 +157,3 @@ class CapturingOnboardingNavigationDelegate: OnboardingNavigationDelegate { urlToNavigateTo = url } } - -final class OnboardingSuggestionsPixelReporterMock: OnboardingSiteSuggestionsPixelReporting, OnboardingSearchSuggestionsPixelReporting { - private(set) var didCallTrackSearchSayDuck = false - private(set) var didCallTrackSearchMightyDuck = false - private(set) var didCallTrackSearchWeather = false - private(set) var didCallTrackSearchSurpriseMe = false - - private(set) var didCallTrackSiteESPN = false - private(set) var didCallTrackSiteYahoo = false - private(set) var didCallTrackSiteEbay = false - private(set) var didCallTrackSiteSurpriseMe = false - - func trackSearchSuggestionSayDuck() { - didCallTrackSearchSayDuck = true - } - - func trackSearchSuggestionMightyDuck() { - didCallTrackSearchMightyDuck = true - } - - func trackSearchSuggestionWeather() { - didCallTrackSearchWeather = true - } - - func trackSearchSuggestionSurpriseMe() { - didCallTrackSearchSurpriseMe = true - } - - func trackSiteSuggestionESPN() { - didCallTrackSiteESPN = true - } - - func trackSiteSuggestionYahoo() { - didCallTrackSiteYahoo = true - } - - func trackSiteSuggestionEbay() { - didCallTrackSiteEbay = true - } - - func trackSiteSuggestionSurpriseMe() { - didCallTrackSiteSurpriseMe = true - } - -} diff --git a/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift b/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift index 3a6fc53816..0babd5ef3f 100644 --- a/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift +++ b/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift @@ -179,7 +179,6 @@ final class TabViewControllerDaxDialogTests: XCTestCase { } final class ContextualOnboardingLogicMock: ContextualOnboardingLogic { - var expectation: XCTestExpectation? private(set) var didCallSetFireEducationMessageSeen = false private(set) var didCallsetFinalOnboardingDialogSeen = false @@ -191,6 +190,8 @@ final class ContextualOnboardingLogicMock: ContextualOnboardingLogic { var isShowingFireDialog: Bool = false var shouldShowPrivacyButtonPulse: Bool = false + var isShowingSearchSuggestions: Bool = false + var isShowingSitesSuggestions: Bool = false func setFireEducationMessageSeen() { didCallSetFireEducationMessageSeen = true From a5ca7e339c4af0fd11a7285ee0e63cf5dc3f5729 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Thu, 1 Aug 2024 16:52:40 +1000 Subject: [PATCH 04/16] Move OnFirstAppearViewModifier to common SwiftUI extensions --- DuckDuckGo.xcodeproj/project.pbxproj | 8 +++--- ....swift => OnFirstAppearViewModifier.swift} | 28 +++++++++++++++---- 2 files changed, 27 insertions(+), 9 deletions(-) rename DuckDuckGo/{Subscription/Extensions/View+AppearModifiers.swift => OnFirstAppearViewModifier.swift} (61%) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 306dd94459..0fd0e00ce9 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -656,6 +656,7 @@ 9F69331B2C5A16E200CD6A5D /* OnboardingDaxFavouritesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F69331A2C5A16E200CD6A5D /* OnboardingDaxFavouritesTests.swift */; }; 9F69331D2C5A191400CD6A5D /* MockTutorialSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F69331C2C5A191400CD6A5D /* MockTutorialSettings.swift */; }; 9F6933192C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F6933182C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift */; }; + 9F69331F2C5B1D0C00CD6A5D /* OnFirstAppearViewModifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F69331E2C5B1D0C00CD6A5D /* OnFirstAppearViewModifier.swift */; }; 9F8007262C5261AF003EDAF4 /* MockPrivacyDataReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F8007252C5261AF003EDAF4 /* MockPrivacyDataReporter.swift */; }; 9F8FE9492BAE50E50071E372 /* Lottie in Frameworks */ = {isa = PBXBuildFile; productRef = 9F8FE9482BAE50E50071E372 /* Lottie */; }; 9F9EE4CE2C377D4900D4118E /* OnboardingFirePixelMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F9EE4CC2C377D3F00D4118E /* OnboardingFirePixelMock.swift */; }; @@ -885,7 +886,6 @@ D668D92B2B696840008E2FF2 /* IdentityTheftRestorationPagesFeature.swift in Sources */ = {isa = PBXBuildFile; fileRef = D668D92A2B696840008E2FF2 /* IdentityTheftRestorationPagesFeature.swift */; }; D66F683D2BB333C100AE93E2 /* SubscriptionContainerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D66F683C2BB333C100AE93E2 /* SubscriptionContainerView.swift */; }; D670E5BB2BB6A75300941A42 /* SubscriptionNavigationCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = D670E5BA2BB6A75200941A42 /* SubscriptionNavigationCoordinator.swift */; }; - D670E5BD2BB6AA0000941A42 /* View+AppearModifiers.swift in Sources */ = {isa = PBXBuildFile; fileRef = D670E5BC2BB6AA0000941A42 /* View+AppearModifiers.swift */; }; D67969112BC84CE700BA8B34 /* SubscriptionContainerViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = D67969102BC84CE700BA8B34 /* SubscriptionContainerViewModel.swift */; }; D68A21442B7EC08500BB372E /* SubscriptionExternalLinkView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D68A21432B7EC08500BB372E /* SubscriptionExternalLinkView.swift */; }; D68A21462B7EC16200BB372E /* SubscriptionExternalLinkViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = D68A21452B7EC16200BB372E /* SubscriptionExternalLinkViewModel.swift */; }; @@ -2369,6 +2369,7 @@ 9F69331A2C5A16E200CD6A5D /* OnboardingDaxFavouritesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingDaxFavouritesTests.swift; sourceTree = ""; }; 9F69331C2C5A191400CD6A5D /* MockTutorialSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockTutorialSettings.swift; sourceTree = ""; }; 9F6933182C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingPixelReporterMock.swift; sourceTree = ""; }; + 9F69331E2C5B1D0C00CD6A5D /* OnFirstAppearViewModifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnFirstAppearViewModifier.swift; sourceTree = ""; }; 9F8007252C5261AF003EDAF4 /* MockPrivacyDataReporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockPrivacyDataReporter.swift; sourceTree = ""; }; 9F9EE4CC2C377D3F00D4118E /* OnboardingFirePixelMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingFirePixelMock.swift; sourceTree = ""; }; 9F9EE4D32C37BB1300D4118E /* OnboardingView+Landing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OnboardingView+Landing.swift"; sourceTree = ""; }; @@ -2608,7 +2609,6 @@ D668D92A2B696840008E2FF2 /* IdentityTheftRestorationPagesFeature.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = IdentityTheftRestorationPagesFeature.swift; sourceTree = ""; }; D66F683C2BB333C100AE93E2 /* SubscriptionContainerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionContainerView.swift; sourceTree = ""; }; D670E5BA2BB6A75200941A42 /* SubscriptionNavigationCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionNavigationCoordinator.swift; sourceTree = ""; }; - D670E5BC2BB6AA0000941A42 /* View+AppearModifiers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "View+AppearModifiers.swift"; sourceTree = ""; }; D67969102BC84CE700BA8B34 /* SubscriptionContainerViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SubscriptionContainerViewModel.swift; sourceTree = ""; }; D68A21432B7EC08500BB372E /* SubscriptionExternalLinkView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SubscriptionExternalLinkView.swift; sourceTree = ""; }; D68A21452B7EC16200BB372E /* SubscriptionExternalLinkViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SubscriptionExternalLinkViewModel.swift; sourceTree = ""; }; @@ -3066,6 +3066,7 @@ 6FD3F80E2C3EF4F000DA5797 /* DeviceOrientationEnvironmentValue.swift */, 9FEA222D2C324ECD006B03BF /* ViewVisibility.swift */, 9FEA22282C2E38FA006B03BF /* AnimatableTypingText.swift */, + 9F69331E2C5B1D0C00CD6A5D /* OnFirstAppearViewModifier.swift */, ); name = SwiftUI; sourceTree = ""; @@ -4910,7 +4911,6 @@ children = ( F1FDC9342BF51E41006B1435 /* VPNSettings+Environment.swift */, D664C7982B289AA000CBFA76 /* WKUserContentController+Handler.swift */, - D670E5BC2BB6AA0000941A42 /* View+AppearModifiers.swift */, ); path = Extensions; sourceTree = ""; @@ -7240,7 +7240,6 @@ 984D035C24AE15CD0066CFB8 /* TabSwitcherSettings.swift in Sources */, D6E83C562B21ECC1006C8AFB /* SettingsLegacyViewProvider.swift in Sources */, 98B31292218CCB8C00E54DE1 /* AppDependencyProvider.swift in Sources */, - D670E5BD2BB6AA0000941A42 /* View+AppearModifiers.swift in Sources */, D6ACEA322BBD55BF008FADDF /* TabURLInterceptor.swift in Sources */, C13F3F6A2B7F883A0083BE40 /* AuthConfirmationPromptViewController.swift in Sources */, 851624C72B96389D002D5CD7 /* HistoryDebugViewController.swift in Sources */, @@ -7304,6 +7303,7 @@ CB84C7BD29A3EF530088A5B8 /* AppConfigurationURLProvider.swift in Sources */, AA3D854723D9E88E00788410 /* AppIconSettingsCell.swift in Sources */, 316931D927BD22A80095F5ED /* DownloadActionMessageViewHelper.swift in Sources */, + 9F69331F2C5B1D0C00CD6A5D /* OnFirstAppearViewModifier.swift in Sources */, BDF8D0022C1B87F4003E3B27 /* NetworkProtectionDNSSettingsViewModel.swift in Sources */, 9838059F2228208E00385F1A /* PositiveFeedbackViewController.swift in Sources */, 8590CB67268A2E520089F6BF /* RootDebugViewController.swift in Sources */, diff --git a/DuckDuckGo/Subscription/Extensions/View+AppearModifiers.swift b/DuckDuckGo/OnFirstAppearViewModifier.swift similarity index 61% rename from DuckDuckGo/Subscription/Extensions/View+AppearModifiers.swift rename to DuckDuckGo/OnFirstAppearViewModifier.swift index bd7c78138d..6f424bd945 100644 --- a/DuckDuckGo/Subscription/Extensions/View+AppearModifiers.swift +++ b/DuckDuckGo/OnFirstAppearViewModifier.swift @@ -1,5 +1,5 @@ // -// View+AppearModifiers.swift +// OnFirstAppear.swift // DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. @@ -19,15 +19,29 @@ import SwiftUI +/// A view modifier that executes a specified action only once when the view first appears. +/// +/// Use this modifier to perform an action the first time the view becomes visible on the screen. +/// The action will not be executed again if the view reappears or is recreated. +/// +/// Example: +/// ```swift +/// Text("Hello, World!") +/// .modifier(OnFirstAppearModifier { +/// print("The view has appeared for the first time.") +/// }) +/// ``` +/// +/// - Parameter onFirstAppearAction: A closure to be executed the first time the view appears. public struct OnFirstAppearModifier: ViewModifier { private let onFirstAppearAction: () -> Void @State private var hasAppeared = false - + public init(_ onFirstAppearAction: @escaping () -> Void) { self.onFirstAppearAction = onFirstAppearAction } - + public func body(content: Content) -> some View { content .onAppear { @@ -39,9 +53,13 @@ public struct OnFirstAppearModifier: ViewModifier { } extension View { - + + /// Adds an action to perform that is executed only the first time before this view appears. + /// + /// - Parameter action: A closure to be executed the first time the view appears. + /// - Returns: A view that will execute `action` only once when it appears. func onFirstAppear(_ onFirstAppearAction: @escaping () -> Void ) -> some View { return modifier(OnFirstAppearModifier(onFirstAppearAction)) } - + } From 1d34c29ff7c10748609da87139e9c24421ceb2ce Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Thu, 1 Aug 2024 19:33:14 +1000 Subject: [PATCH 05/16] Make onboarding events unique --- Core/Pixel.swift | 1 + Core/PixelEvent.swift | 54 +++++++++++-------- DuckDuckGo/DaxDialogs.swift | 34 ++++++------ .../FullscreenDaxDialogViewController.swift | 2 +- 4 files changed, 50 insertions(+), 41 deletions(-) diff --git a/Core/Pixel.swift b/Core/Pixel.swift index a50f8911a0..b569bea727 100644 --- a/Core/Pixel.swift +++ b/Core/Pixel.swift @@ -146,6 +146,7 @@ public struct PixelParameters { // Privacy Dashboard public static let daysSinceInstall = "daysSinceInstall" + public static let fromOnboarding = "from_onboarding" } public struct PixelValues { diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index 2e0e82fd41..ab0e040b79 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -44,7 +44,8 @@ extension Pixel { case forgetAllDataCleared case privacyDashboardOpened - + case privacyDashboardFirstTimeOpenedUnique + case dashboardProtectionAllowlistAdd case dashboardProtectionAllowlistRemove @@ -147,18 +148,21 @@ extension Pixel { case onboardingContextualSiteOptionTappedUnique case onboardingContextualSiteCustomUnique case onboardingContextualSecondSiteVisitUnique + case onboardingContextualTrySearchUnique + case onboardingContextualTryVisitSiteUnique - case daxDialogsSerp - case daxDialogsWithoutTrackers + case daxDialogsSerpUnique + case daxDialogsWithoutTrackersUnique case daxDialogsWithoutTrackersFollowUp - case daxDialogsWithTrackers - case daxDialogsSiteIsMajor - case daxDialogsSiteOwnedByMajor - case daxDialogsHidden - case daxDialogsFireEducationShown - case daxDialogsFireEducationConfirmed - case daxDialogsFireEducationCancelled - + case daxDialogsWithTrackersUnique + case daxDialogsSiteIsMajorUnique + case daxDialogsSiteOwnedByMajorUnique + case daxDialogsHiddenUnique + case daxDialogsFireEducationShownUnique + case daxDialogsFireEducationConfirmedUnique + case daxDialogsFireEducationCancelledUnique + case daxDialogsEndOfJourneyUnique + case widgetsOnboardingCTAPressed case widgetsOnboardingDeclineOptionPressed case widgetsOnboardingMovedToBackground @@ -764,7 +768,8 @@ extension Pixel.Event { case .forgetAllDataCleared: return "mf_dc" case .privacyDashboardOpened: return "mp" - + case .privacyDashboardFirstTimeOpenedUnique: return "m_privacy_dashboard_first_time_used_unique" + case .dashboardProtectionAllowlistAdd: return "mp_wla" case .dashboardProtectionAllowlistRemove: return "mp_wlr" @@ -875,18 +880,21 @@ extension Pixel.Event { case .onboardingContextualSecondSiteVisitUnique: return "m_second_sitevisit_unique" case .onboardingContextualSearchCustomUnique: return "m_onboarding_search_custom_unique" case .onboardingContextualSiteCustomUnique: return "m_onboarding_visit_site_custom_unique" - - case .daxDialogsSerp: return "m_dx_s" - case .daxDialogsWithoutTrackers: return "m_dx_wo" + case .onboardingContextualTrySearchUnique: return "m_dx_try_a_search_unique" + case .onboardingContextualTryVisitSiteUnique: return "m_dx_try_visit_site_unique" + + case .daxDialogsSerpUnique: return "m_dx_s_unique" + case .daxDialogsWithoutTrackersUnique: return "m_dx_wo_unique" case .daxDialogsWithoutTrackersFollowUp: return "m_dx_wof" - case .daxDialogsWithTrackers: return "m_dx_wt" - case .daxDialogsSiteIsMajor: return "m_dx_sm" - case .daxDialogsSiteOwnedByMajor: return "m_dx_so" - case .daxDialogsHidden: return "m_dx_h" - case .daxDialogsFireEducationShown: return "m_dx_fe_s" - case .daxDialogsFireEducationConfirmed: return "m_dx_fe_co" - case .daxDialogsFireEducationCancelled: return "m_dx_fe_ca" - + case .daxDialogsWithTrackersUnique: return "m_dx_wt_unique" + case .daxDialogsSiteIsMajorUnique: return "m_dx_sm_unique" + case .daxDialogsSiteOwnedByMajorUnique: return "m_dx_so_unique" + case .daxDialogsHiddenUnique: return "m_dx_h_unique" + case .daxDialogsFireEducationShownUnique: return "m_dx_fe_s_unique" + case .daxDialogsFireEducationConfirmedUnique: return "m_dx_fe_co_unique" + case .daxDialogsFireEducationCancelledUnique: return "m_dx_fe_ca_unique" + case .daxDialogsEndOfJourneyUnique: return "m_dx_end_unique_unique" + case .widgetsOnboardingCTAPressed: return "m_o_w_a" case .widgetsOnboardingDeclineOptionPressed: return "m_o_w_d" case .widgetsOnboardingMovedToBackground: return "m_o_w_b" diff --git a/DuckDuckGo/DaxDialogs.swift b/DuckDuckGo/DaxDialogs.swift index 68a0b3355d..9d9f42b1c3 100644 --- a/DuckDuckGo/DaxDialogs.swift +++ b/DuckDuckGo/DaxDialogs.swift @@ -120,35 +120,35 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { static let afterSearch = BrowsingSpec(message: UserText.daxDialogBrowsingAfterSearch, cta: UserText.daxDialogBrowsingAfterSearchCTA, highlightAddressBar: false, - pixelName: .daxDialogsSerp, + pixelName: .daxDialogsSerpUnique, type: .afterSearch) static let withoutTrackers = BrowsingSpec(message: UserText.daxDialogBrowsingWithoutTrackers, cta: UserText.daxDialogBrowsingWithoutTrackersCTA, highlightAddressBar: false, - pixelName: .daxDialogsWithoutTrackers, type: .withoutTrackers) - + pixelName: .daxDialogsWithoutTrackersUnique, type: .withoutTrackers) + static let siteIsMajorTracker = BrowsingSpec(message: UserText.daxDialogBrowsingSiteIsMajorTracker, cta: UserText.daxDialogBrowsingSiteIsMajorTrackerCTA, highlightAddressBar: false, - pixelName: .daxDialogsSiteIsMajor, type: .siteIsMajorTracker) - + pixelName: .daxDialogsSiteIsMajorUnique, type: .siteIsMajorTracker) + static let siteOwnedByMajorTracker = BrowsingSpec(message: UserText.daxDialogBrowsingSiteOwnedByMajorTracker, cta: UserText.daxDialogBrowsingSiteOwnedByMajorTrackerCTA, highlightAddressBar: false, - pixelName: .daxDialogsSiteOwnedByMajor, type: .siteOwnedByMajorTracker) - + pixelName: .daxDialogsSiteOwnedByMajorUnique, type: .siteOwnedByMajorTracker) + static let withOneTracker = BrowsingSpec(message: UserText.daxDialogBrowsingWithOneTracker, cta: UserText.daxDialogBrowsingWithOneTrackerCTA, highlightAddressBar: true, - pixelName: .daxDialogsWithTrackers, type: .withOneTracker) - + pixelName: .daxDialogsWithTrackersUnique, type: .withOneTracker) + static let withMultipleTrackers = BrowsingSpec(message: UserText.daxDialogBrowsingWithMultipleTrackers, cta: UserText.daxDialogBrowsingWithMultipleTrackersCTA, highlightAddressBar: true, - pixelName: .daxDialogsWithTrackers, type: .withMultipleTrackers) + pixelName: .daxDialogsWithTrackersUnique, type: .withMultipleTrackers) - static let final = BrowsingSpec(message: UserText.daxDialogHomeSubsequent, cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationShown, type: .final) + static let final = BrowsingSpec(message: UserText.daxDialogHomeSubsequent, cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationShownUnique, type: .final) let message: String let cta: String @@ -180,10 +180,10 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { confirmAction: UserText.daxDialogFireButtonEducationConfirmAction, cancelAction: UserText.daxDialogFireButtonEducationCancelAction, isConfirmActionDestructive: true, - displayedPixelName: .daxDialogsFireEducationShown, - confirmActionPixelName: .daxDialogsFireEducationConfirmed, - cancelActionPixelName: .daxDialogsFireEducationCancelled) - + displayedPixelName: .daxDialogsFireEducationShownUnique, + confirmActionPixelName: .daxDialogsFireEducationConfirmedUnique, + cancelActionPixelName: .daxDialogsFireEducationCancelledUnique) + let message: String let confirmAction: String let cancelAction: String @@ -373,7 +373,7 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { case BrowsingSpec.SpecType.afterSearch.rawValue: return BrowsingSpec.afterSearch case BrowsingSpec.SpecType.visitWebsite.rawValue: - return BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationConfirmed, type: .visitWebsite) + return BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationConfirmedUnique, type: .visitWebsite) case BrowsingSpec.SpecType.withoutTrackers.rawValue: return BrowsingSpec.withoutTrackers case BrowsingSpec.SpecType.siteIsMajorTracker.rawValue: @@ -386,7 +386,7 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { guard let entityNames = blockedEntityNames(privacyInfo.trackerInfo) else { return nil } return trackersBlockedMessage(entityNames) case BrowsingSpec.SpecType.fire.rawValue: - return BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationConfirmed, type: .fire) + return BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationConfirmedUnique, type: .fire) case BrowsingSpec.SpecType.final.rawValue: return nil default: return nil diff --git a/DuckDuckGo/FullscreenDaxDialogViewController.swift b/DuckDuckGo/FullscreenDaxDialogViewController.swift index 1557ae3c70..3a49583756 100644 --- a/DuckDuckGo/FullscreenDaxDialogViewController.swift +++ b/DuckDuckGo/FullscreenDaxDialogViewController.swift @@ -137,7 +137,7 @@ extension TabViewController: FullscreenDaxDialogDelegate { preferredStyle: isPad ? .alert : .actionSheet) alertController.addAction(title: UserText.daxDialogHideButton, style: .default) { - Pixel.fire(pixel: .daxDialogsHidden, withAdditionalParameters: [ "c": DefaultDaxDialogsSettings().browsingDialogsSeenCount ]) + Pixel.fire(pixel: .daxDialogsHiddenUnique, withAdditionalParameters: [ "c": DefaultDaxDialogsSettings().browsingDialogsSeenCount ]) DaxDialogs.shared.dismiss() } alertController.addAction(title: UserText.daxDialogHideCancel, style: .cancel) { From 1f256f33f90b0e9094e090eaa7043c57934af12d Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Thu, 1 Aug 2024 20:22:39 +1000 Subject: [PATCH 06/16] Add tests for pixel thrown when new tab page dialogs appear --- .../NewTabDaxDialogFactory.swift | 23 +++++-- .../Pixels/OnboardingPixelReporter.swift | 14 ++++ ...alOnboardingNewTabDialogFactoryTests.swift | 65 ++++++++++++++++++- .../OnboardingPixelReporterMock.swift | 10 ++- 4 files changed, 104 insertions(+), 8 deletions(-) diff --git a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift index b419180374..e7cac856a2 100644 --- a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift +++ b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift @@ -25,13 +25,19 @@ protocol NewTabDaxDialogProvider { func createDaxDialog(for homeDialog: DaxDialogs.HomeScreenSpec, onDismiss: @escaping () -> Void) -> DaxDialog } -class NewTabDaxDialogFactory: NewTabDaxDialogProvider { - var delegate: OnboardingNavigationDelegate? - var contextualOnboardingLogic: ContextualOnboardingLogic +final class NewTabDaxDialogFactory: NewTabDaxDialogProvider { + private var delegate: OnboardingNavigationDelegate? + private let contextualOnboardingLogic: ContextualOnboardingLogic + private let onboardingPixelReporter: OnboardingScreenImpressionReporting - init(delegate: OnboardingNavigationDelegate?, contextualOnboardingLogic: ContextualOnboardingLogic) { + init( + delegate: OnboardingNavigationDelegate?, + contextualOnboardingLogic: ContextualOnboardingLogic, + onboardingPixelReporter: OnboardingScreenImpressionReporting = OnboardingPixelReporter() + ) { self.delegate = delegate self.contextualOnboardingLogic = contextualOnboardingLogic + self.onboardingPixelReporter = onboardingPixelReporter } @ViewBuilder @@ -58,6 +64,9 @@ class NewTabDaxDialogFactory: NewTabDaxDialogProvider { .onboardingDaxDialogStyle() } .onboardingContextualBackgroundStyle() + .onFirstAppear { [weak self] in + self?.onboardingPixelReporter.trackScreenImpression(event: .onboardingContextualTrySearchUnique) + } } private func createSubsequentDialog() -> some View { @@ -67,6 +76,9 @@ class NewTabDaxDialogFactory: NewTabDaxDialogProvider { .onboardingDaxDialogStyle() } .onboardingContextualBackgroundStyle() + .onFirstAppear { [weak self] in + self?.onboardingPixelReporter.trackScreenImpression(event: .onboardingContextualTryVisitSiteUnique) + } } private func createAddFavoriteDialog(message: String) -> some View { @@ -86,8 +98,9 @@ class NewTabDaxDialogFactory: NewTabDaxDialogProvider { .onboardingDaxDialogStyle() } .onboardingContextualBackgroundStyle() - .onAppear { [weak self] in + .onFirstAppear { [weak self] in self?.contextualOnboardingLogic.setFinalOnboardingDialogSeen() + self?.onboardingPixelReporter.trackScreenImpression(event: .daxDialogsEndOfJourneyUnique) } } } diff --git a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift index 2e123fab60..71a3c6c225 100644 --- a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift +++ b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift @@ -68,6 +68,10 @@ protocol OnboardingPrivacyDashboardPixelReporting { func trackPrivacyDashboardOpen() } +protocol OnboardingScreenImpressionReporting { + func trackScreenImpression(event: Pixel.Event) +} + // MARK: - Implementation final class OnboardingPixelReporter { @@ -169,6 +173,16 @@ extension OnboardingPixelReporter: OnboardingPrivacyDashboardPixelReporting { } +// MARK: - OnboardingPixelReporter + Screen Impression + +extension OnboardingPixelReporter: OnboardingScreenImpressionReporting { + + func trackScreenImpression(event: Pixel.Event) { + fire(event: event, unique: true) + } + +} + protocol DaysSinceInstallProviding { var daysSinceInstall: Int? { get } } diff --git a/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift b/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift index dd72cbd7eb..8e477826e3 100644 --- a/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift +++ b/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift @@ -19,6 +19,7 @@ import XCTest import SwiftUI +import Core @testable import DuckDuckGo class ContextualOnboardingNewTabDialogFactoryTests: XCTestCase { @@ -26,6 +27,7 @@ class ContextualOnboardingNewTabDialogFactoryTests: XCTestCase { var factory: NewTabDaxDialogFactory! var mockDelegate: CapturingOnboardingNavigationDelegate! var contextualOnboardingLogicMock: ContextualOnboardingLogicMock! + var pixelReporterMock: OnboardingPixelReporterMock! var onDismissCalled: Bool! var window: UIWindow! @@ -34,7 +36,8 @@ class ContextualOnboardingNewTabDialogFactoryTests: XCTestCase { mockDelegate = CapturingOnboardingNavigationDelegate() contextualOnboardingLogicMock = ContextualOnboardingLogicMock() onDismissCalled = false - factory = NewTabDaxDialogFactory(delegate: mockDelegate, contextualOnboardingLogic: contextualOnboardingLogicMock) + pixelReporterMock = OnboardingPixelReporterMock() + factory = NewTabDaxDialogFactory(delegate: mockDelegate, contextualOnboardingLogic: contextualOnboardingLogicMock, onboardingPixelReporter: pixelReporterMock) window = UIWindow(frame: UIScreen.main.bounds) window.makeKeyAndVisible() } @@ -117,4 +120,64 @@ class ContextualOnboardingNewTabDialogFactoryTests: XCTestCase { XCTAssertEqual(addFavoriteDialog?.message.string, homeDialog.message) } + // MARK: - Pixels + + func testWhenOnboardingTrySearchDialogAppearForTheFirstTime_ThenSendFireExpectedPixel() { + // GIVEN + let spec = DaxDialogs.HomeScreenSpec.initial + let pixelEvent = Pixel.Event.onboardingContextualTrySearchUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: pixelEvent) + } + + func testWhenOnboardingTryVisitSiteDialogAppearForTheFirstTime_ThenSendFireExpectedPixel() { + // GIVEN + let spec = DaxDialogs.HomeScreenSpec.subsequent + let pixelEvent = Pixel.Event.onboardingContextualTryVisitSiteUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: pixelEvent) + } + + func testWhenOnboardingFinalDialogAppearForTheFirstTime_ThenSendFireExpectedPixel() { + // GIVEN + let spec = DaxDialogs.HomeScreenSpec.final + let pixelEvent = Pixel.Event.daxDialogsEndOfJourneyUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: pixelEvent) + } + +} + +private extension ContextualOnboardingNewTabDialogFactoryTests { + + func testDialogDefinedBy(spec: DaxDialogs.HomeScreenSpec, firesEvent event: Pixel.Event) { + // GIVEN + let expectation = self.expectation(description: #function) + XCTAssertFalse(pixelReporterMock.didCallTrackScreenImpressionCalled) + XCTAssertNil(pixelReporterMock.capturedScreenImpression) + + // WHEN + let view = factory.createDaxDialog(for: spec, onDismiss: {}) + let host = UIHostingControllerMock(rootView: AnyView(view)) + host.onAppearExpectation = expectation + window.rootViewController = host + XCTAssertNotNil(host.view) + + // THEN + waitForExpectations(timeout: 2.0) + XCTAssertTrue(pixelReporterMock.didCallTrackScreenImpressionCalled) + XCTAssertEqual(pixelReporterMock.capturedScreenImpression, event) + } + +} + +private final class UIHostingControllerMock: UIHostingController { + + var onAppearExpectation: XCTestExpectation? + + override func viewDidAppear(_ animated: Bool) { + super.viewDidAppear(animated) + onAppearExpectation?.fulfill() + } + } diff --git a/DuckDuckGoTests/OnboardingPixelReporterMock.swift b/DuckDuckGoTests/OnboardingPixelReporterMock.swift index 2de06cc485..d06a48153c 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterMock.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterMock.swift @@ -18,10 +18,10 @@ // import Foundation +import Core @testable import DuckDuckGo -final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting, OnboardingSearchSuggestionsPixelReporting, OnboardingCustomSearchPixelReporting { - +final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting, OnboardingSearchSuggestionsPixelReporting, OnboardingCustomSearchPixelReporting, OnboardingScreenImpressionReporting { private(set) var didCallTrackSearchOptionTapped = false private(set) var didCallTrackSiteOptionTapped = false private(set) var didCallTrackCustomSearch = false @@ -32,6 +32,8 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting } } private(set) var secondSiteVisitCounter = 0 + private(set) var didCallTrackScreenImpressionCalled = false + private(set) var capturedScreenImpression: Pixel.Event? func trackSiteSuggetionOptionTapped() { didCallTrackSiteOptionTapped = true @@ -53,4 +55,8 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting didCallSecondSiteVisit = true } + func trackScreenImpression(event: Pixel.Event) { + didCallTrackScreenImpressionCalled = true + capturedScreenImpression = event + } } From 039c8e677037bd862a1fe8c23ef34d767597aade Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Thu, 1 Aug 2024 21:53:54 +1000 Subject: [PATCH 07/16] Add pixel tests for contextual dialogs --- DuckDuckGo.xcodeproj/project.pbxproj | 4 + DuckDuckGo/DaxDialogs.swift | 12 +- .../ContextualDaxDialogsFactory.swift | 56 +++++-- .../ContextualDaxDialogsFactoryTests.swift | 154 +++++++++++++++++- ...alOnboardingNewTabDialogFactoryTests.swift | 14 +- .../OnboardingHostingControllerMock.swift | 32 ++++ 6 files changed, 242 insertions(+), 30 deletions(-) create mode 100644 DuckDuckGoTests/OnboardingHostingControllerMock.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 0fd0e00ce9..80272d19d1 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -657,6 +657,7 @@ 9F69331D2C5A191400CD6A5D /* MockTutorialSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F69331C2C5A191400CD6A5D /* MockTutorialSettings.swift */; }; 9F6933192C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F6933182C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift */; }; 9F69331F2C5B1D0C00CD6A5D /* OnFirstAppearViewModifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F69331E2C5B1D0C00CD6A5D /* OnFirstAppearViewModifier.swift */; }; + 9F6933212C5B9A5B00CD6A5D /* OnboardingHostingControllerMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F6933202C5B9A5B00CD6A5D /* OnboardingHostingControllerMock.swift */; }; 9F8007262C5261AF003EDAF4 /* MockPrivacyDataReporter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F8007252C5261AF003EDAF4 /* MockPrivacyDataReporter.swift */; }; 9F8FE9492BAE50E50071E372 /* Lottie in Frameworks */ = {isa = PBXBuildFile; productRef = 9F8FE9482BAE50E50071E372 /* Lottie */; }; 9F9EE4CE2C377D4900D4118E /* OnboardingFirePixelMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F9EE4CC2C377D3F00D4118E /* OnboardingFirePixelMock.swift */; }; @@ -2370,6 +2371,7 @@ 9F69331C2C5A191400CD6A5D /* MockTutorialSettings.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockTutorialSettings.swift; sourceTree = ""; }; 9F6933182C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingPixelReporterMock.swift; sourceTree = ""; }; 9F69331E2C5B1D0C00CD6A5D /* OnFirstAppearViewModifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnFirstAppearViewModifier.swift; sourceTree = ""; }; + 9F6933202C5B9A5B00CD6A5D /* OnboardingHostingControllerMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingHostingControllerMock.swift; sourceTree = ""; }; 9F8007252C5261AF003EDAF4 /* MockPrivacyDataReporter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockPrivacyDataReporter.swift; sourceTree = ""; }; 9F9EE4CC2C377D3F00D4118E /* OnboardingFirePixelMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingFirePixelMock.swift; sourceTree = ""; }; 9F9EE4D32C37BB1300D4118E /* OnboardingView+Landing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OnboardingView+Landing.swift"; sourceTree = ""; }; @@ -4470,6 +4472,7 @@ 9F9EE4CC2C377D3F00D4118E /* OnboardingFirePixelMock.swift */, 9F4CC5142C47AD08006A96EB /* ContextualOnboardingPresenterMock.swift */, 9F6933182C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift */, + 9F6933202C5B9A5B00CD6A5D /* OnboardingHostingControllerMock.swift */, ); name = Mocks; sourceTree = ""; @@ -7489,6 +7492,7 @@ 9F23B8062C2BE22700950875 /* OnboardingIntroViewModelTests.swift in Sources */, 9F69331D2C5A191400CD6A5D /* MockTutorialSettings.swift in Sources */, 85D2187424BF25CD004373D2 /* FaviconsTests.swift in Sources */, + 9F6933212C5B9A5B00CD6A5D /* OnboardingHostingControllerMock.swift in Sources */, 9F6933192C59BB0300CD6A5D /* OnboardingPixelReporterMock.swift in Sources */, 56D0602D2C383FD2003BAEB5 /* OnboardingSuggestedSearchesProviderTests.swift in Sources */, 85AD49EE2B6149110085D2D1 /* CookieStorageTests.swift in Sources */, diff --git a/DuckDuckGo/DaxDialogs.swift b/DuckDuckGo/DaxDialogs.swift index 9d9f42b1c3..d72fa5cbe4 100644 --- a/DuckDuckGo/DaxDialogs.swift +++ b/DuckDuckGo/DaxDialogs.swift @@ -123,6 +123,9 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { pixelName: .daxDialogsSerpUnique, type: .afterSearch) + // Message and CTA empty on purpose as for this case we use only pixelName and type + static let visitWebsite = BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .onboardingContextualTryVisitSiteUnique, type: .visitWebsite) + static let withoutTrackers = BrowsingSpec(message: UserText.daxDialogBrowsingWithoutTrackers, cta: UserText.daxDialogBrowsingWithoutTrackersCTA, highlightAddressBar: false, @@ -148,7 +151,10 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { highlightAddressBar: true, pixelName: .daxDialogsWithTrackersUnique, type: .withMultipleTrackers) - static let final = BrowsingSpec(message: UserText.daxDialogHomeSubsequent, cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationShownUnique, type: .final) + // Message and CTA empty on purpose as for this case we use only pixelName and type + static let fire = BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationShownUnique, type: .fire) + + static let final = BrowsingSpec(message: UserText.daxDialogHomeSubsequent, cta: "", highlightAddressBar: false, pixelName: .daxDialogsEndOfJourneyUnique, type: .final) let message: String let cta: String @@ -373,7 +379,7 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { case BrowsingSpec.SpecType.afterSearch.rawValue: return BrowsingSpec.afterSearch case BrowsingSpec.SpecType.visitWebsite.rawValue: - return BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationConfirmedUnique, type: .visitWebsite) + return .visitWebsite case BrowsingSpec.SpecType.withoutTrackers.rawValue: return BrowsingSpec.withoutTrackers case BrowsingSpec.SpecType.siteIsMajorTracker.rawValue: @@ -386,7 +392,7 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { guard let entityNames = blockedEntityNames(privacyInfo.trackerInfo) else { return nil } return trackersBlockedMessage(entityNames) case BrowsingSpec.SpecType.fire.rawValue: - return BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationConfirmedUnique, type: .fire) + return .fire case BrowsingSpec.SpecType.final.rawValue: return nil default: return nil diff --git a/DuckDuckGo/OnboardingExperiment/ContextualOnboarding/ContextualDaxDialogsFactory.swift b/DuckDuckGo/OnboardingExperiment/ContextualOnboarding/ContextualDaxDialogsFactory.swift index 772c4f964c..1f71c8b4d1 100644 --- a/DuckDuckGo/OnboardingExperiment/ContextualOnboarding/ContextualDaxDialogsFactory.swift +++ b/DuckDuckGo/OnboardingExperiment/ContextualOnboarding/ContextualDaxDialogsFactory.swift @@ -18,6 +18,7 @@ // import SwiftUI +import Core // MARK: - ContextualOnboardingEventDelegate @@ -42,12 +43,18 @@ protocol ContextualDaxDialogsFactory { } final class ExperimentContextualDaxDialogsFactory: ContextualDaxDialogsFactory { - private let contextualOnboardingSettings: ContextualOnboardingSettings private let contextualOnboardingLogic: ContextualOnboardingLogic + private let contextualOnboardingSettings: ContextualOnboardingSettings + private let contextualOnboardingPixelReporter: OnboardingScreenImpressionReporting - init(contextualOnboardingSettings: ContextualOnboardingSettings = DefaultDaxDialogsSettings(), contextualOnboardingLogic: ContextualOnboardingLogic) { + init( + contextualOnboardingLogic: ContextualOnboardingLogic, + contextualOnboardingSettings: ContextualOnboardingSettings = DefaultDaxDialogsSettings(), + contextualOnboardingPixelReporter: OnboardingScreenImpressionReporting = OnboardingPixelReporter() + ) { self.contextualOnboardingSettings = contextualOnboardingSettings self.contextualOnboardingLogic = contextualOnboardingLogic + self.contextualOnboardingPixelReporter = contextualOnboardingPixelReporter } func makeView(for spec: DaxDialogs.BrowsingSpec, delegate: ContextualOnboardingDelegate, onSizeUpdate: @escaping () -> Void) -> UIHostingController { @@ -56,9 +63,10 @@ final class ExperimentContextualDaxDialogsFactory: ContextualDaxDialogsFactory { case .afterSearch: rootView = AnyView( afterSearchDialog( - shouldFollowUpToWebsiteSearch: !contextualOnboardingSettings.userHasSeenTrackersDialog, - delegate: delegate, - onSizeUpdate: onSizeUpdate + shouldFollowUpToWebsiteSearch: !contextualOnboardingSettings.userHasSeenTrackersDialog, + delegate: delegate, + afterSearchPixelEvent: spec.pixelName, + onSizeUpdate: onSizeUpdate ) ) case .visitWebsite: @@ -73,9 +81,9 @@ final class ExperimentContextualDaxDialogsFactory: ContextualDaxDialogsFactory { ) ) case .fire: - rootView = AnyView(OnboardingFireDialog()) + rootView = AnyView(fireDialog(pixelName: spec.pixelName)) case .final: - rootView = AnyView(endOfJourneyDialog(delegate: delegate)) + rootView = AnyView(endOfJourneyDialog(delegate: delegate, pixelName: spec.pixelName)) } let viewWithBackground = rootView @@ -89,14 +97,20 @@ final class ExperimentContextualDaxDialogsFactory: ContextualDaxDialogsFactory { return hostingController } - private func afterSearchDialog(shouldFollowUpToWebsiteSearch: Bool, delegate: ContextualOnboardingDelegate, onSizeUpdate: @escaping () -> Void) -> some View { + private func afterSearchDialog( + shouldFollowUpToWebsiteSearch: Bool, + delegate: ContextualOnboardingDelegate, + afterSearchPixelEvent: Pixel.Event, + onSizeUpdate: @escaping () -> Void + ) -> some View { let viewModel = OnboardingSiteSuggestionsViewModel(title: UserText.DaxOnboardingExperiment.ContextualOnboarding.onboardingTryASiteTitle, delegate: delegate) + // If should not show websites search after searching inform the delegate that the user dimissed the dialog, otherwise let the dialog handle it. - let gotItAction: () -> Void = if shouldFollowUpToWebsiteSearch { - { [weak delegate] in + { [weak delegate, weak self] in onSizeUpdate() delegate?.didAcknowledgeContextualOnboardingSearch() + self?.contextualOnboardingPixelReporter.trackScreenImpression(event: .onboardingContextualTryVisitSiteUnique) } } else { { [weak delegate] in @@ -105,11 +119,17 @@ final class ExperimentContextualDaxDialogsFactory: ContextualDaxDialogsFactory { } return OnboardingFirstSearchDoneDialog(shouldFollowUp: shouldFollowUpToWebsiteSearch, viewModel: viewModel, gotItAction: gotItAction) + .onFirstAppear { [weak self] in + self?.contextualOnboardingPixelReporter.trackScreenImpression(event: afterSearchPixelEvent) + } } private func tryVisitingSiteDialog(delegate: ContextualOnboardingDelegate) -> some View { let viewModel = OnboardingSiteSuggestionsViewModel(title: UserText.DaxOnboardingExperiment.ContextualOnboarding.onboardingTryASiteTitle, delegate: delegate) return OnboardingTryVisitingSiteDialog(logoPosition: .left, viewModel: viewModel) + .onFirstAppear { [weak self] in + self?.contextualOnboardingPixelReporter.trackScreenImpression(event: .onboardingContextualTryVisitSiteUnique) + } } private func withTrackersDialog(for spec: DaxDialogs.BrowsingSpec, shouldFollowUpToFireDialog: Bool, delegate: ContextualOnboardingDelegate, onSizeUpdate: @escaping () -> Void) -> some View { @@ -121,19 +141,31 @@ final class ExperimentContextualDaxDialogsFactory: ContextualDaxDialogsFactory { } else { onSizeUpdate() delegate?.didAcknowledgeContextualOnboardingTrackersDialog() + self?.contextualOnboardingPixelReporter.trackScreenImpression(event: .daxDialogsFireEducationShownUnique) } }) .onAppear { [weak delegate] in delegate?.didShowContextualOnboardingTrackersDialog() } + .onFirstAppear { [weak self] in + self?.contextualOnboardingPixelReporter.trackScreenImpression(event: spec.pixelName) + } + } + + private func fireDialog(pixelName: Pixel.Event) -> some View { + OnboardingFireDialog() + .onFirstAppear { [weak self] in + self?.contextualOnboardingPixelReporter.trackScreenImpression(event: pixelName) + } } - private func endOfJourneyDialog(delegate: ContextualOnboardingDelegate) -> some View { + private func endOfJourneyDialog(delegate: ContextualOnboardingDelegate, pixelName: Pixel.Event) -> some View { OnboardingFinalDialog(highFiveAction: { [weak delegate] in delegate?.didTapDismissContextualOnboardingAction() }) - .onAppear { [weak self] in + .onFirstAppear { [weak self] in self?.contextualOnboardingLogic.setFinalOnboardingDialogSeen() + self?.contextualOnboardingPixelReporter.trackScreenImpression(event: pixelName) } } diff --git a/DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift b/DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift index 017e9baff7..e813b0080e 100644 --- a/DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift +++ b/DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift @@ -19,24 +19,36 @@ import XCTest import SwiftUI +import Core @testable import DuckDuckGo final class ContextualDaxDialogsFactoryTests: XCTestCase { private var sut: ExperimentContextualDaxDialogsFactory! private var delegate: ContextualOnboardingDelegateMock! private var settingsMock: ContextualOnboardingSettingsMock! - + private var pixelReporterMock: OnboardingPixelReporterMock! + private var window: UIWindow! override func setUpWithError() throws { try super.setUpWithError() delegate = ContextualOnboardingDelegateMock() settingsMock = ContextualOnboardingSettingsMock() - sut = ExperimentContextualDaxDialogsFactory(contextualOnboardingSettings: settingsMock, contextualOnboardingLogic: ContextualOnboardingLogicMock()) + pixelReporterMock = OnboardingPixelReporterMock() + sut = ExperimentContextualDaxDialogsFactory( + contextualOnboardingLogic: ContextualOnboardingLogicMock(), + contextualOnboardingSettings: settingsMock, + contextualOnboardingPixelReporter: pixelReporterMock + ) + window = UIWindow(frame: UIScreen.main.bounds) + window.makeKeyAndVisible() } override func tearDownWithError() throws { + window.isHidden = true + window = nil delegate = nil settingsMock = nil + pixelReporterMock = nil sut = nil try super.tearDownWithError() } @@ -167,7 +179,6 @@ final class ContextualDaxDialogsFactoryTests: XCTestCase { XCTAssertNotNil(view) } - // MARK: - Final func test_WhenMakeViewForFinalSpec_ThenReturnViewOnboardingFinalDialog() throws { @@ -195,6 +206,143 @@ final class ContextualDaxDialogsFactoryTests: XCTestCase { // THEN XCTAssertTrue(delegate.didCallDidTapDismissContextualOnboardingAction) } + + // MARK: - Pixels + + func testWhenViewForAfterSearchSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.afterSearch + let expectedPixel = Pixel.Event.daxDialogsSerpUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenViewForVisitSiteSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.visitWebsite + let expectedPixel = Pixel.Event.onboardingContextualTryVisitSiteUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenViewForWithoutTrackersSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.withoutTrackers + let expectedPixel = Pixel.Event.daxDialogsWithoutTrackersUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenViewForWithOneTrackerSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.withOneTracker + let expectedPixel = Pixel.Event.daxDialogsWithTrackersUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenViewForWithTrackersSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.withMultipleTrackers + let expectedPixel = Pixel.Event.daxDialogsWithTrackersUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenViewForSiteIsMajorTrackerSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.siteIsMajorTracker + let expectedPixel = Pixel.Event.daxDialogsSiteIsMajorUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenViewForSiteIsOwnedByMajorTrackerSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.siteOwnedByMajorTracker + let expectedPixel = Pixel.Event.daxDialogsSiteOwnedByMajorUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenViewForFireSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.fire + let expectedPixel = Pixel.Event.daxDialogsFireEducationShownUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenViewForFinalSpecAppearsThenExpectedPixelFires() { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.final + let expectedPixel = Pixel.Event.daxDialogsEndOfJourneyUnique + // TEST + testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) + } + + func testWhenAfterSearchCTAIsTappedAndTryVisitWebsiteDialogThenExpectedPixelFires() throws { + try [DaxDialogs.BrowsingSpec.siteIsMajorTracker, .siteOwnedByMajorTracker, .withMultipleTrackers, .withoutTrackers, .withoutTrackers].forEach { spec in + // GIVEN + settingsMock.userHasSeenFireDialog = false + pixelReporterMock = OnboardingPixelReporterMock() + sut = ExperimentContextualDaxDialogsFactory( + contextualOnboardingLogic: ContextualOnboardingLogicMock(), + contextualOnboardingSettings: settingsMock, + contextualOnboardingPixelReporter: pixelReporterMock + ) + let result = sut.makeView(for: spec, delegate: delegate, onSizeUpdate: {}) + let view = try XCTUnwrap(find(OnboardingTrackersDoneDialog.self, in: result)) + XCTAssertFalse(pixelReporterMock.didCallTrackScreenImpressionCalled) + XCTAssertNil(pixelReporterMock.capturedScreenImpression) + + // WHEN + view.blockedTrackersCTAAction() + + // THEN + XCTAssertTrue(pixelReporterMock.didCallTrackScreenImpressionCalled) + XCTAssertEqual(pixelReporterMock.capturedScreenImpression, .daxDialogsFireEducationShownUnique) + } + } + + func testWhenTrackersDialogCTAIsTappedAndFireDialogThenExpectedPixelFires() throws { + // GIVEN + let spec = DaxDialogs.BrowsingSpec.afterSearch + let result = sut.makeView(for: spec, delegate: delegate, onSizeUpdate: {}) + let view = try XCTUnwrap(find(OnboardingFirstSearchDoneDialog.self, in: result)) + XCTAssertFalse(pixelReporterMock.didCallTrackScreenImpressionCalled) + XCTAssertNil(pixelReporterMock.capturedScreenImpression) + + // WHEN + view.gotItAction() + + // THEN + XCTAssertTrue(pixelReporterMock.didCallTrackScreenImpressionCalled) + XCTAssertEqual(pixelReporterMock.capturedScreenImpression, .onboardingContextualTryVisitSiteUnique) + } +} + +extension ContextualDaxDialogsFactoryTests { + + func testDialogDefinedBy(spec: DaxDialogs.BrowsingSpec, firesEvent event: Pixel.Event) { + // GIVEN + let expectation = self.expectation(description: #function) + XCTAssertFalse(pixelReporterMock.didCallTrackScreenImpressionCalled) + XCTAssertNil(pixelReporterMock.capturedScreenImpression) + + // WHEN + let view = sut.makeView(for: spec, delegate: ContextualOnboardingDelegateMock(), onSizeUpdate: {}).rootView + let host = OnboardingHostingControllerMock(rootView: AnyView(view)) + host.onAppearExpectation = expectation + window.rootViewController = host + XCTAssertNotNil(host.view) + + // THEN + waitForExpectations(timeout: 2.0) + XCTAssertTrue(pixelReporterMock.didCallTrackScreenImpressionCalled) + XCTAssertEqual(pixelReporterMock.capturedScreenImpression, event) + } + } final class ContextualOnboardingSettingsMock: ContextualOnboardingSettings { diff --git a/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift b/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift index 8e477826e3..c12372ea53 100644 --- a/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift +++ b/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift @@ -49,6 +49,7 @@ class ContextualOnboardingNewTabDialogFactoryTests: XCTestCase { mockDelegate = nil onDismissCalled = nil contextualOnboardingLogicMock = nil + pixelReporterMock = nil super.tearDown() } @@ -158,7 +159,7 @@ private extension ContextualOnboardingNewTabDialogFactoryTests { // WHEN let view = factory.createDaxDialog(for: spec, onDismiss: {}) - let host = UIHostingControllerMock(rootView: AnyView(view)) + let host = OnboardingHostingControllerMock(rootView: AnyView(view)) host.onAppearExpectation = expectation window.rootViewController = host XCTAssertNotNil(host.view) @@ -170,14 +171,3 @@ private extension ContextualOnboardingNewTabDialogFactoryTests { } } - -private final class UIHostingControllerMock: UIHostingController { - - var onAppearExpectation: XCTestExpectation? - - override func viewDidAppear(_ animated: Bool) { - super.viewDidAppear(animated) - onAppearExpectation?.fulfill() - } - -} diff --git a/DuckDuckGoTests/OnboardingHostingControllerMock.swift b/DuckDuckGoTests/OnboardingHostingControllerMock.swift new file mode 100644 index 0000000000..29a7864a2d --- /dev/null +++ b/DuckDuckGoTests/OnboardingHostingControllerMock.swift @@ -0,0 +1,32 @@ +// +// OnboardingHostingControllerMock.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 SwiftUI +import XCTest + +final class OnboardingHostingControllerMock: UIHostingController { + + var onAppearExpectation: XCTestExpectation? + + override func viewDidAppear(_ animated: Bool) { + super.viewDidAppear(animated) + onAppearExpectation?.fulfill() + } + +} From 6ea5ae47d99d53e805d2c4da13050c8b04a52bfe Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 13:03:14 +1000 Subject: [PATCH 08/16] Refactor onboarding pixel reporter and add tests --- DuckDuckGo/AppDelegate.swift | 2 +- DuckDuckGo/MainViewController.swift | 13 +- .../Pixels/OnboardingPixelReporter.swift | 62 +++---- DuckDuckGo/TabViewController.swift | 11 +- .../OnboardingNavigationDelegateTests.swift | 2 +- .../OnboardingPixelReporterMock.swift | 9 +- .../OnboardingPixelReporterTests.swift | 170 +++++++++++++++++- 7 files changed, 210 insertions(+), 59 deletions(-) diff --git a/DuckDuckGo/AppDelegate.swift b/DuckDuckGo/AppDelegate.swift index 7c5a123e04..1f08bcd1c5 100644 --- a/DuckDuckGo/AppDelegate.swift +++ b/DuckDuckGo/AppDelegate.swift @@ -318,7 +318,7 @@ import WebKit variantManager: variantManager, contextualOnboardingPresenter: ContextualOnboardingPresenter(variantManager: variantManager), contextualOnboardingLogic: daxDialogs, - contextualOnboardingCustomSearchPixelReporting: OnboardingPixelReporter()) + contextualOnboardingPixelReporter: OnboardingPixelReporter()) main.loadViewIfNeeded() syncErrorHandler.alertPresenter = main diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 0433304b31..17a70f6072 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -103,7 +103,8 @@ class MainViewController: UIViewController { private let variantManager: VariantManager private let tutorialSettings: TutorialSettings private let contextualOnboardingLogic: ContextualOnboardingLogic - private let contextualOnboardingPixelReporting: OnboardingCustomSearchPixelReporting + private let contextualOnboardingPixelReporter: OnboardingCustomInteractionPixelReporting + private let statisticsStore: StatisticsStore @UserDefaultsWrapper(key: .syncDidShowSyncPausedByFeatureFlagAlert, defaultValue: false) private var syncDidShowSyncPausedByFeatureFlagAlert: Bool @@ -185,8 +186,9 @@ class MainViewController: UIViewController { variantManager: VariantManager, contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, - contextualOnboardingCustomSearchPixelReporting: OnboardingCustomSearchPixelReporting + contextualOnboardingPixelReporter: OnboardingCustomInteractionPixelReporting, tutorialSettings: TutorialSettings = DefaultTutorialSettings() + statisticsStore: StatisticsStore = StatisticsUserDefaults() ) { self.bookmarksDatabase = bookmarksDatabase self.bookmarksDatabaseCleaner = bookmarksDatabaseCleaner @@ -214,7 +216,8 @@ class MainViewController: UIViewController { self.variantManager = variantManager self.tutorialSettings = tutorialSettings self.contextualOnboardingLogic = contextualOnboardingLogic - self.contextualOnboardingPixelReporting = contextualOnboardingCustomSearchPixelReporting + self.contextualOnboardingPixelReporter = contextualOnboardingPixelReporter + self.statisticsStore = statisticsStore super.init(nibName: nil, bundle: nil) @@ -1293,9 +1296,9 @@ class MainViewController: UIViewController { func fireOnboardingCustomSearchPixelIfNeeded(query: String) { if contextualOnboardingLogic.isShowingSearchSuggestions { - contextualOnboardingPixelReporting.trackCustomSearch() + contextualOnboardingPixelReporter.trackCustomSearch() } else if contextualOnboardingLogic.isShowingSitesSuggestions { - contextualOnboardingPixelReporting.trackCustomSite() + contextualOnboardingPixelReporter.trackCustomSite() } } diff --git a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift index 71a3c6c225..827625299f 100644 --- a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift +++ b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift @@ -58,14 +58,11 @@ protocol OnboardingSiteSuggestionsPixelReporting { func trackSiteSuggetionOptionTapped() } -protocol OnboardingCustomSearchPixelReporting { +protocol OnboardingCustomInteractionPixelReporting { func trackCustomSearch() func trackCustomSite() func trackSecondSiteVisit() -} - -protocol OnboardingPrivacyDashboardPixelReporting { - func trackPrivacyDashboardOpen() + func trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: Bool) } protocol OnboardingScreenImpressionReporting { @@ -77,19 +74,25 @@ protocol OnboardingScreenImpressionReporting { final class OnboardingPixelReporter { private let pixel: OnboardingPixelFiring.Type private let uniquePixel: OnboardingPixelFiring.Type - private let daysSinceInstallProvider: DaysSinceInstallProviding + private let statisticsStore: StatisticsStore + private let calendar: Calendar + private let dateProvider: () -> Date private let userDefaults: UserDefaults private let siteVisitedUserDefaultsKey = "com.duckduckgo.ios.site-visited" init( pixel: OnboardingPixelFiring.Type = Pixel.self, uniquePixel: OnboardingPixelFiring.Type = UniquePixel.self, - daysSinceInstallProvider: DaysSinceInstallProviding = DaysSinceInstallProvider(), - userDefaults: UserDefaults = UserDefaults.standard + statisticsStore: StatisticsStore = StatisticsUserDefaults(), + calendar: Calendar = .current, + dateProvider: @escaping () -> Date = Date.init, + userDefaults: UserDefaults = UserDefaults.app ) { self.pixel = pixel self.uniquePixel = uniquePixel - self.daysSinceInstallProvider = daysSinceInstallProvider + self.statisticsStore = statisticsStore + self.calendar = calendar + self.dateProvider = dateProvider self.userDefaults = userDefaults } @@ -140,10 +143,10 @@ extension OnboardingPixelReporter: OnboardingSiteSuggestionsPixelReporting { } -// MARK: - OnboardingPixelReporter + Custom Search +// MARK: - OnboardingPixelReporter + Custom Interaction + +extension OnboardingPixelReporter: OnboardingCustomInteractionPixelReporting { -extension OnboardingPixelReporter: OnboardingCustomSearchPixelReporting { - func trackCustomSearch() { fire(event: .onboardingContextualSearchCustomUnique, unique: true) } @@ -160,15 +163,13 @@ extension OnboardingPixelReporter: OnboardingCustomSearchPixelReporting { } } -} - -// MARK: - OnboardingPixelReporter + Privacy Dashboard - -extension OnboardingPixelReporter: OnboardingPrivacyDashboardPixelReporting { - - func trackPrivacyDashboardOpen() { - guard let daysSinceInstall = daysSinceInstallProvider.daysSinceInstall else { return } - fire(event: .privacyDashboardOpened, unique: true, additionalParameters: ["daysSinceInstall": String(daysSinceInstall)]) + func trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: Bool) { + let daysSinceInstall = statisticsStore.installDate.flatMap { calendar.numberOfDaysBetween($0, and: dateProvider()) } + let additionalParameters = [ + PixelParameters.fromOnboarding: String(fromOnboarding), + PixelParameters.daysSinceInstall: String(daysSinceInstall ?? 0) + ] + fire(event: .privacyDashboardFirstTimeOpenedUnique, unique: true, additionalParameters: additionalParameters) } } @@ -182,22 +183,3 @@ extension OnboardingPixelReporter: OnboardingScreenImpressionReporting { } } - -protocol DaysSinceInstallProviding { - var daysSinceInstall: Int? { get } -} - -final class DaysSinceInstallProvider: DaysSinceInstallProviding { - private let store: StatisticsStore - private let dateProvider: () -> Date - - init(store: StatisticsStore = StatisticsUserDefaults(), dateProvider: @escaping () -> Date = Date.init) { - self.store = store - self.dateProvider = dateProvider - } - - var daysSinceInstall: Int? { - guard let installDate = store.installDate else { return nil } - return Calendar.current.numberOfDaysBetween(installDate, and: dateProvider()) - } -} diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index bf62850769..b87c81fbaf 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -298,8 +298,7 @@ class TabViewController: UIViewController { privacyProDataReporter: PrivacyProDataReporting, contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, - onboardingPixelReporter: OnboardingPixelReporter = OnboardingPixelReporter(), - daysSinceInstallProvider: DaysSinceInstallProviding = DaysSinceInstallProvider()) -> TabViewController { + onboardingPixelReporter: OnboardingPixelReporter = OnboardingPixelReporter()) -> TabViewController { let storyboard = UIStoryboard(name: "Tab", bundle: nil) let controller = storyboard.instantiateViewController(identifier: "TabViewController", creator: { coder in TabViewController(coder: coder, @@ -312,8 +311,7 @@ class TabViewController: UIViewController { privacyProDataReporter: privacyProDataReporter, contextualOnboardingPresenter: contextualOnboardingPresenter, contextualOnboardingLogic: contextualOnboardingLogic, - onboardingPixelReporter: onboardingPixelReporter, - daysSinceInstallProvider: daysSinceInstallProvider + onboardingPixelReporter: onboardingPixelReporter ) }) return controller @@ -330,7 +328,6 @@ class TabViewController: UIViewController { let contextualOnboardingPresenter: ContextualOnboardingPresenting let contextualOnboardingLogic: ContextualOnboardingLogic - let daysSinceInstallProvider: DaysSinceInstallProviding let onboardingPixelReporter: OnboardingPixelReporter required init?(coder aDecoder: NSCoder, @@ -343,8 +340,7 @@ class TabViewController: UIViewController { privacyProDataReporter: PrivacyProDataReporting, contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, - onboardingPixelReporter: OnboardingPixelReporter, - daysSinceInstallProvider: DaysSinceInstallProviding) { + onboardingPixelReporter: OnboardingPixelReporter) { self.tabModel = tabModel self.appSettings = appSettings self.bookmarksDatabase = bookmarksDatabase @@ -357,7 +353,6 @@ class TabViewController: UIViewController { self.contextualOnboardingPresenter = contextualOnboardingPresenter self.contextualOnboardingLogic = contextualOnboardingLogic self.onboardingPixelReporter = onboardingPixelReporter - self.daysSinceInstallProvider = daysSinceInstallProvider super.init(coder: aDecoder) } diff --git a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift index e07d34761c..9638da3751 100644 --- a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift +++ b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift @@ -72,7 +72,7 @@ final class OnboardingNavigationDelegateTests: XCTestCase { variantManager: MockVariantManager(), contextualOnboardingPresenter: ContextualOnboardingPresenterMock(), contextualOnboardingLogic: ContextualOnboardingLogicMock(), - contextualOnboardingCustomSearchPixelReporting: OnboardingPixelReporterMock()) + contextualOnboardingPixelReporter: OnboardingPixelReporterMock()) let window = UIWindow(frame: UIScreen.main.bounds) window.rootViewController = UIViewController() window.makeKeyAndVisible() diff --git a/DuckDuckGoTests/OnboardingPixelReporterMock.swift b/DuckDuckGoTests/OnboardingPixelReporterMock.swift index d06a48153c..cbabe2d50d 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterMock.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterMock.swift @@ -21,7 +21,7 @@ import Foundation import Core @testable import DuckDuckGo -final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting, OnboardingSearchSuggestionsPixelReporting, OnboardingCustomSearchPixelReporting, OnboardingScreenImpressionReporting { +final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting, OnboardingSearchSuggestionsPixelReporting, OnboardingCustomInteractionPixelReporting, OnboardingScreenImpressionReporting { private(set) var didCallTrackSearchOptionTapped = false private(set) var didCallTrackSiteOptionTapped = false private(set) var didCallTrackCustomSearch = false @@ -34,6 +34,8 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting private(set) var secondSiteVisitCounter = 0 private(set) var didCallTrackScreenImpressionCalled = false private(set) var capturedScreenImpression: Pixel.Event? + private(set) var didCallPrivacyDashboardOpenedForFirstTime = false + private(set) var capturedFromOnboarding: Bool? func trackSiteSuggetionOptionTapped() { didCallTrackSiteOptionTapped = true @@ -59,4 +61,9 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting didCallTrackScreenImpressionCalled = true capturedScreenImpression = event } + + func trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: Bool) { + didCallPrivacyDashboardOpenedForFirstTime = true + capturedFromOnboarding = fromOnboarding + } } diff --git a/DuckDuckGoTests/OnboardingPixelReporterTests.swift b/DuckDuckGoTests/OnboardingPixelReporterTests.swift index 7764d8ad12..cb49a7d4e5 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterTests.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterTests.swift @@ -22,16 +22,29 @@ import Core @testable import DuckDuckGo final class OnboardingPixelReporterTests: XCTestCase { + private static let suiteName = "testing_onboarding_pixel_store" private var sut: OnboardingPixelReporter! + private var statisticsStoreMock: MockStatisticsStore! + private var now: Date! + private var userDefaultsMock: UserDefaults! override func setUpWithError() throws { - sut = OnboardingPixelReporter(pixel: OnboardingPixelFireMock.self, uniquePixel: OnboardingUniquePixelFireMock.self) + statisticsStoreMock = MockStatisticsStore() + now = Date() + var calendar = Calendar(identifier: .gregorian) + calendar.timeZone = try XCTUnwrap(TimeZone(secondsFromGMT: 0)) + userDefaultsMock = UserDefaults(suiteName: Self.suiteName) + sut = OnboardingPixelReporter(pixel: OnboardingPixelFireMock.self, uniquePixel: OnboardingUniquePixelFireMock.self, statisticsStore: statisticsStoreMock, calendar: calendar, dateProvider: { self.now }, userDefaults: userDefaultsMock) try super.setUpWithError() } override func tearDownWithError() throws { OnboardingPixelFireMock.tearDown() OnboardingUniquePixelFireMock.tearDown() + statisticsStoreMock = nil + now = nil + userDefaultsMock.removePersistentDomain(forName: Self.suiteName) + userDefaultsMock = nil sut = nil try super.tearDownWithError() } @@ -95,7 +108,7 @@ final class OnboardingPixelReporterTests: XCTestCase { // MARK: - List - func testWhenTrackSearchSuggestionOptionTappedThenExpectedEventIsCalled() { + func testWhenTrackSearchSuggestionOptionTappedThenSearchOptionTappedFires() { // GIVEN let expectedPixel = Pixel.Event.onboardingContextualSearchOptionTappedUnique XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) @@ -114,7 +127,7 @@ final class OnboardingPixelReporterTests: XCTestCase { XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) } - func testWhenTrackSiteSuggestionThenExpectedEventIsCalled() { + func testWhenTrackSiteSuggestionThenSiteOptionsTappedFires() { // GIVEN let expectedPixel = Pixel.Event.onboardingContextualSiteOptionTappedUnique XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) @@ -133,4 +146,155 @@ final class OnboardingPixelReporterTests: XCTestCase { XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) } + // MARK: - Custom Interactions + + func testWhenTrackCustomSearchIsCalledThenSearchCustomFires() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSearchCustomUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackCustomSearch() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_search_custom_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackCustomSiteIsCalledThenSiteCustomFires() { + // GIVEN + let expectedPixel = Pixel.Event.onboardingContextualSiteCustomUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackCustomSite() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_onboarding_visit_site_custom_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackSecondVisitIsCalledAndStoreDoesNotContainPixelThenPixelIsNotFired() { + // GIVEN + XCTAssertNil(userDefaultsMock.value(forKey: "com.duckduckgo.ios.site-visited")) + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSecondSiteVisit() + + // THEN + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + } + + func testWhenTrackSecondVisitIsCalledThenFiresOnlyOnSecondTime() { + // GIVEN + let key = "com.duckduckgo.ios.site-visited" + userDefaultsMock.set(true, forKey: key) + XCTAssertTrue(userDefaultsMock.bool(forKey: key)) + let expectedPixel = Pixel.Event.onboardingContextualSecondSiteVisitUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackSecondSiteVisit() + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_second_sitevisit_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackPrivacyDashboardOpenedForFirstTimeThenPrivacyDashboardFirstTimeOpenedPixelFires() { + // GIVEN + let expectedPixel = Pixel.Event.privacyDashboardFirstTimeOpenedUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: true) + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, "m_privacy_dashboard_first_time_used_unique") + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } + + func testWhenTrackPrivacyDashboardOpenedForFirstTimeAndFromOnboardingFalseThenParameterIsSetToFalse() { + // GIVEN + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + + // WHEN + sut.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: true) + + // THEN + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams["from_onboarding"], "true") + } + + func testWhenTrackPrivacyDashboardOpenedForFirstTimeAndFromOnboardingTrueThenParameterIsSetToTrue() { + // GIVEN + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + + // WHEN + sut.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: false) + + // THEN + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams["from_onboarding"], "false") + } + + func testWhenTrackPrivacyDashboardOpenedForFirstTimeThenDaysSinceInstallParameterIsSet() { + // GIVEN + let installDate = Date(timeIntervalSince1970: 1722348000) // 30th July 2024 GMT + now = Date(timeIntervalSince1970: 1722607200) // 1st August 2024 GMT + statisticsStoreMock.installDate = installDate + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) + + // WHEN + sut.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: false) + + // THEN + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams["daysSinceInstall"], "3") + } + + // MARK: - Screen Impressions + + func testWhenTrackScreenImpressionIsCalledThenPixelFires() { + // GIVEN + let expectedPixel = Pixel.Event.daxDialogsSerpUnique + XCTAssertFalse(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertNil(OnboardingUniquePixelFireMock.capturedPixelEvent) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) + + // WHEN + sut.trackScreenImpression(event: expectedPixel) + + // THEN + XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) + XCTAssertEqual(expectedPixel.name, expectedPixel.name) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + } } From 924594567f58a7d358b36c77c4f6248e3d821426 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 13:06:07 +1000 Subject: [PATCH 09/16] Revert changes to existing privacy dashboard pixel --- DuckDuckGo/TabViewController.swift | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index b87c81fbaf..cc8cddd2de 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -928,13 +928,7 @@ class TabViewController: UIViewController { } func showPrivacyDashboard() { - func firePrivacyDashboardOpenPixel() { - let additionalParameters = daysSinceInstallProvider.daysSinceInstall - .flatMap { [PixelParameters.daysSinceInstall: String($0)] } ?? [:] - UniquePixel.fire(pixel: .privacyDashboardOpened, withAdditionalParameters: additionalParameters, includedParameters: [.appVersion, .atb]) - } - - firePrivacyDashboardOpenPixel() + Pixel.fire(pixel: .privacyDashboardOpened) performSegue(withIdentifier: "PrivacyDashboard", sender: self) } From 1872b7a9b918d0d58b489a949faa40987b58d860 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 13:46:13 +1000 Subject: [PATCH 10/16] Add unit tests for tracking second site visit --- DuckDuckGo/TabViewController.swift | 14 +++-- DuckDuckGoTests/MockTabDelegate.swift | 6 +- .../OnboardingPixelReporterMock.swift | 4 +- .../TabViewControllerDaxDialogTests.swift | 60 ++++++++++++++++++- 4 files changed, 75 insertions(+), 9 deletions(-) diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index cc8cddd2de..6a326deb5e 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -298,7 +298,7 @@ class TabViewController: UIViewController { privacyProDataReporter: PrivacyProDataReporting, contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, - onboardingPixelReporter: OnboardingPixelReporter = OnboardingPixelReporter()) -> TabViewController { + onboardingPixelReporter: OnboardingCustomInteractionPixelReporting = OnboardingPixelReporter()) -> TabViewController { let storyboard = UIStoryboard(name: "Tab", bundle: nil) let controller = storyboard.instantiateViewController(identifier: "TabViewController", creator: { coder in TabViewController(coder: coder, @@ -328,7 +328,7 @@ class TabViewController: UIViewController { let contextualOnboardingPresenter: ContextualOnboardingPresenting let contextualOnboardingLogic: ContextualOnboardingLogic - let onboardingPixelReporter: OnboardingPixelReporter + let onboardingPixelReporter: OnboardingCustomInteractionPixelReporting required init?(coder aDecoder: NSCoder, tabModel: Tab, @@ -340,7 +340,7 @@ class TabViewController: UIViewController { privacyProDataReporter: PrivacyProDataReporting, contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, - onboardingPixelReporter: OnboardingPixelReporter) { + onboardingPixelReporter: OnboardingCustomInteractionPixelReporting) { self.tabModel = tabModel self.appSettings = appSettings self.bookmarksDatabase = bookmarksDatabase @@ -1315,7 +1315,7 @@ extension TabViewController: WKNavigationDelegate { onWebpageDidFinishLoading() instrumentation.didLoadURL() checkLoginDetectionAfterNavigation() - onboardingPixelReporter.trackSecondSiteVisit() + trackSecondSiteVisitIfNeeded(url: webView.url) // definitely finished with any potential login cycle by this point, so don't try and handle it any more detectedLoginURL = nil @@ -1373,6 +1373,12 @@ extension TabViewController: WKNavigationDelegate { } } + func trackSecondSiteVisitIfNeeded(url: URL?) { + // Track second non-SERP webpage visit + guard url?.isDuckDuckGoSearch == false else { return } + onboardingPixelReporter.trackSecondSiteVisit() + } + func showDaxDialogOrStartTrackerNetworksAnimationIfNeeded() { guard !isLinkPreview else { return } diff --git a/DuckDuckGoTests/MockTabDelegate.swift b/DuckDuckGoTests/MockTabDelegate.swift index b85cc347d0..e271e41a6c 100644 --- a/DuckDuckGoTests/MockTabDelegate.swift +++ b/DuckDuckGoTests/MockTabDelegate.swift @@ -115,7 +115,8 @@ extension TabViewController { static func fake( contextualOnboardingPresenter: ContextualOnboardingPresenting = ContextualOnboardingPresenterMock(), - contextualOnboardingLogic: ContextualOnboardingLogic = ContextualOnboardingLogicMock() + contextualOnboardingLogic: ContextualOnboardingLogic = ContextualOnboardingLogicMock(), + contextualOnboardingPixelReporter: OnboardingCustomInteractionPixelReporting = OnboardingPixelReporterMock() ) -> TabViewController { let tab = TabViewController.loadFromStoryboard( model: .init(link: Link(title: nil, url: .ddg)), @@ -126,7 +127,8 @@ extension TabViewController { duckPlayer: MockDuckPlayer(settings: MockDuckPlayerSettings(privacyConfigManager: PrivacyConfigurationManagerMock())), privacyProDataReporter: MockPrivacyProDataReporter(), contextualOnboardingPresenter: contextualOnboardingPresenter, - contextualOnboardingLogic: contextualOnboardingLogic + contextualOnboardingLogic: contextualOnboardingLogic, + onboardingPixelReporter: contextualOnboardingPixelReporter ) tab.attachWebView(configuration: .nonPersistent(), andLoadRequest: nil, consumeCookies: false) return tab diff --git a/DuckDuckGoTests/OnboardingPixelReporterMock.swift b/DuckDuckGoTests/OnboardingPixelReporterMock.swift index cbabe2d50d..0ea6394a0c 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterMock.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterMock.swift @@ -26,7 +26,7 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting private(set) var didCallTrackSiteOptionTapped = false private(set) var didCallTrackCustomSearch = false private(set) var didCallTrackCustomSite = false - private(set) var didCallSecondSiteVisit = false { + private(set) var didCallTrackSecondSiteVisit = false { didSet { secondSiteVisitCounter += 1 } @@ -54,7 +54,7 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting } func trackSecondSiteVisit() { - didCallSecondSiteVisit = true + didCallTrackSecondSiteVisit = true } func trackScreenImpression(event: Pixel.Event) { diff --git a/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift b/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift index 0babd5ef3f..30d9b2662f 100644 --- a/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift +++ b/DuckDuckGoTests/TabViewControllerDaxDialogTests.swift @@ -20,6 +20,7 @@ import XCTest import Persistence import Core +import WebKit @testable import DuckDuckGo final class TabViewControllerDaxDialogTests: XCTestCase { @@ -27,13 +28,15 @@ final class TabViewControllerDaxDialogTests: XCTestCase { private var delegateMock: MockTabDelegate! private var onboardingPresenterMock: ContextualOnboardingPresenterMock! private var onboardingLogicMock: ContextualOnboardingLogicMock! + private var onboardingPixelReporterMock: OnboardingPixelReporterMock! override func setUpWithError() throws { try super.setUpWithError() delegateMock = MockTabDelegate() onboardingPresenterMock = ContextualOnboardingPresenterMock() onboardingLogicMock = ContextualOnboardingLogicMock() - sut = .fake(contextualOnboardingPresenter: onboardingPresenterMock, contextualOnboardingLogic: onboardingLogicMock) + onboardingPixelReporterMock = OnboardingPixelReporterMock() + sut = .fake(contextualOnboardingPresenter: onboardingPresenterMock, contextualOnboardingLogic: onboardingLogicMock, contextualOnboardingPixelReporter: onboardingPixelReporterMock) sut.delegate = delegateMock } @@ -176,6 +179,40 @@ final class TabViewControllerDaxDialogTests: XCTestCase { XCTAssertFalse(onboardingPresenterMock.didCallDismissContextualOnboardingIfNeeded) } + // MARK: - SecondSite Visit Pixel + + func testWhenWebsiteFinishLoading_andIsNotSERP_ThenFireSecondSiteVisitPixel() { + // GIVEN + WKNavigation.swizzleDealloc() + let url = URL.ddg + let webView = MockWebView() + webView.setCurrentURL(url) + XCTAssertFalse(onboardingPixelReporterMock.didCallTrackSecondSiteVisit) + + // WHEN + sut.webView(webView, didFinish: WKNavigation()) + + // THEN + XCTAssertTrue(onboardingPixelReporterMock.didCallTrackSecondSiteVisit) + WKNavigation.restoreDealloc() + } + + func testWhenWebsiteFinishLoading_andIsSERP_ThenDoNotFireSecondSiteVisitPixel() throws { + // GIVEN + WKNavigation.swizzleDealloc() + let url = try XCTUnwrap(URL.makeSearchURL(text: "test")) + let webView = MockWebView() + webView.setCurrentURL(url) + XCTAssertFalse(onboardingPixelReporterMock.didCallTrackSecondSiteVisit) + + // WHEN + sut.webView(webView, didFinish: WKNavigation()) + + // THEN + XCTAssertFalse(onboardingPixelReporterMock.didCallTrackSecondSiteVisit) + WKNavigation.restoreDealloc() + } + } final class ContextualOnboardingLogicMock: ContextualOnboardingLogic { @@ -220,3 +257,24 @@ final class ContextualOnboardingLogicMock: ContextualOnboardingLogic { } } + +private extension WKNavigation { + private static var isSwizzled = false + private static let originalDealloc = { class_getInstanceMethod(WKNavigation.self, NSSelectorFromString("dealloc"))! }() + private static let swizzledDealloc = { class_getInstanceMethod(WKNavigation.self, #selector(swizzled_dealloc))! }() + + static func swizzleDealloc() { + guard !self.isSwizzled else { return } + self.isSwizzled = true + method_exchangeImplementations(originalDealloc, swizzledDealloc) + } + + static func restoreDealloc() { + guard self.isSwizzled else { return } + self.isSwizzled = false + method_exchangeImplementations(originalDealloc, swizzledDealloc) + } + + @objc + func swizzled_dealloc() { } +} From 094580eb2130714dbb15c7c69f4178257a4ea32b Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 14:38:00 +1000 Subject: [PATCH 11/16] Add pixel tests for opening privacy dashboard first time --- DuckDuckGo/MainViewController.swift | 4 ++- DuckDuckGo/OmniBar.swift | 3 +- DuckDuckGo/OmniBarDelegate.swift | 6 ++-- ...vacyIconContextualOnboardingAnimator.swift | 5 ++- .../OnboardingNavigationDelegateTests.swift | 34 ++++++++++++++++++- .../OnboardingPixelReporterMock.swift | 4 +-- 6 files changed, 47 insertions(+), 9 deletions(-) diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 17a70f6072..6f99f0be91 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -1726,9 +1726,11 @@ extension MainViewController: OmniBarDelegate { fireOnboardingCustomSearchPixelIfNeeded(query: query) } - func onPrivacyIconPressed() { + func onPrivacyIconPressed(isHighlighted: Bool) { guard !isSERPPresented else { return } + // Track first tap of privacy icon button + contextualOnboardingPixelReporter.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: isHighlighted) // Dismiss privacy icon animation when showing privacy dashboard dismissPrivacyDashboardButtonPulse() diff --git a/DuckDuckGo/OmniBar.swift b/DuckDuckGo/OmniBar.swift index 5e8e4ca566..e89b1508e3 100644 --- a/DuckDuckGo/OmniBar.swift +++ b/DuckDuckGo/OmniBar.swift @@ -466,7 +466,8 @@ class OmniBar: UIView { } @IBAction func onPrivacyIconPressed(_ sender: Any) { - omniDelegate?.onPrivacyIconPressed() + let isPrivacyIconHighlighted = privacyIconContextualOnboardingAnimator.isPrivacyIconHighlighted(privacyInfoContainer.privacyIcon) + omniDelegate?.onPrivacyIconPressed(isHighlighted: isPrivacyIconHighlighted) } @IBAction func onMenuButtonPressed(_ sender: UIButton) { diff --git a/DuckDuckGo/OmniBarDelegate.swift b/DuckDuckGo/OmniBarDelegate.swift index c861af47d0..e1153fd7a3 100644 --- a/DuckDuckGo/OmniBarDelegate.swift +++ b/DuckDuckGo/OmniBarDelegate.swift @@ -35,7 +35,7 @@ protocol OmniBarDelegate: AnyObject { func onEditingEnd() -> OmniBarEditingEndResult - func onPrivacyIconPressed() + func onPrivacyIconPressed(isHighlighted: Bool) func onMenuPressed() @@ -82,8 +82,8 @@ extension OmniBarDelegate { } - func onPrivacyIconPressed() { - + func onPrivacyIconPressed(isHighlighted: Bool) { + } func onMenuPressed() { diff --git a/DuckDuckGo/PrivacyIconContextualOnboardingAnimator.swift b/DuckDuckGo/PrivacyIconContextualOnboardingAnimator.swift index 7b44f22dbd..10b762b238 100644 --- a/DuckDuckGo/PrivacyIconContextualOnboardingAnimator.swift +++ b/DuckDuckGo/PrivacyIconContextualOnboardingAnimator.swift @@ -27,9 +27,12 @@ final class PrivacyIconContextualOnboardingAnimator { } func dismissPrivacyIconAnimation(_ view: PrivacyIconView) { - if ViewHighlighter.highlightedViews.contains(where: { $0.view == view }) { + if isPrivacyIconHighlighted(view) { ViewHighlighter.hideAll() } } + func isPrivacyIconHighlighted(_ view: PrivacyIconView) -> Bool { + ViewHighlighter.highlightedViews.contains(where: { $0.view == view }) + } } diff --git a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift index 9638da3751..6cd4142b1b 100644 --- a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift +++ b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift @@ -32,6 +32,7 @@ import Combine final class OnboardingNavigationDelegateTests: XCTestCase { var mainVC: MainViewController! + var onboardingPixelReporter: OnboardingPixelReporterMock! override func setUp() { let db = CoreDataDatabase.bookmarksMock @@ -57,6 +58,7 @@ final class OnboardingNavigationDelegateTests: XCTestCase { ) let homePageConfiguration = HomePageConfiguration(remoteMessagingClient: remoteMessagingClient, privacyProDataReporter: MockPrivacyProDataReporter()) let tabsModel = TabsModel(desktop: true) + onboardingPixelReporter = OnboardingPixelReporterMock() mainVC = MainViewController( bookmarksDatabase: db, bookmarksDatabaseCleaner: bookmarkDatabaseCleaner, @@ -72,7 +74,7 @@ final class OnboardingNavigationDelegateTests: XCTestCase { variantManager: MockVariantManager(), contextualOnboardingPresenter: ContextualOnboardingPresenterMock(), contextualOnboardingLogic: ContextualOnboardingLogicMock(), - contextualOnboardingPixelReporter: OnboardingPixelReporterMock()) + contextualOnboardingPixelReporter: onboardingPixelReporter) let window = UIWindow(frame: UIScreen.main.bounds) window.rootViewController = UIViewController() window.makeKeyAndVisible() @@ -151,6 +153,36 @@ final class OnboardingNavigationDelegateTests: XCTestCase { XCTAssertEqual(mainVC.currentTab?.url, url) } + // MARK: Pixel + + func testWhenPrivacyBarIconIsPressed_AndPrivacyIconIsHighlighted_ThenFireFirstTimePrivacyDashboardUsedPixel_AndFromOnboardingIsTrue() { + // GIVEN + let isHighlighted = true + XCTAssertFalse(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) + XCTAssertNil(onboardingPixelReporter.capturedFromOnboarding) + + // WHEN + mainVC.onPrivacyIconPressed(isHighlighted: isHighlighted) + + // THEN + XCTAssertTrue(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) + XCTAssertEqual(onboardingPixelReporter.capturedFromOnboarding, true) + } + + func testWhenPrivacyBarIconIsPressed_AndPrivacyIconIsNotHighlighted_ThenFireFirstTimePrivacyDashboardUsedPixel_AndFromOnboardingIsFalse() { + // GIVEN + let isHighlighted = false + XCTAssertFalse(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) + XCTAssertNil(onboardingPixelReporter.capturedFromOnboarding) + + // WHEN + mainVC.onPrivacyIconPressed(isHighlighted: isHighlighted) + + // THEN + XCTAssertTrue(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) + XCTAssertEqual(onboardingPixelReporter.capturedFromOnboarding, false) + } + } class MockConfigurationStoring: ConfigurationStoring { diff --git a/DuckDuckGoTests/OnboardingPixelReporterMock.swift b/DuckDuckGoTests/OnboardingPixelReporterMock.swift index 0ea6394a0c..3c3386d8de 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterMock.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterMock.swift @@ -34,7 +34,7 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting private(set) var secondSiteVisitCounter = 0 private(set) var didCallTrackScreenImpressionCalled = false private(set) var capturedScreenImpression: Pixel.Event? - private(set) var didCallPrivacyDashboardOpenedForFirstTime = false + private(set) var didCallTrackPrivacyDashboardOpenedForFirstTime = false private(set) var capturedFromOnboarding: Bool? func trackSiteSuggetionOptionTapped() { @@ -63,7 +63,7 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting } func trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: Bool) { - didCallPrivacyDashboardOpenedForFirstTime = true + didCallTrackPrivacyDashboardOpenedForFirstTime = true capturedFromOnboarding = fromOnboarding } } From f330fb01e0e42e96b3f54731de567116edb75e09 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 15:38:47 +1000 Subject: [PATCH 12/16] Send try visit site pixel from old onboarding --- DuckDuckGo/HomeViewController+DaxDialogs.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/DuckDuckGo/HomeViewController+DaxDialogs.swift b/DuckDuckGo/HomeViewController+DaxDialogs.swift index 2a8f0bf5ef..c9dd772944 100644 --- a/DuckDuckGo/HomeViewController+DaxDialogs.swift +++ b/DuckDuckGo/HomeViewController+DaxDialogs.swift @@ -19,6 +19,7 @@ import Foundation import UIKit +import Core import SwiftUI extension HomeViewController { @@ -35,6 +36,10 @@ extension HomeViewController { daxDialogViewController.message = spec.message daxDialogViewController.accessibleMessage = spec.accessibilityLabel + if spec == .initial { + UniquePixel.fire(pixel: .onboardingContextualTryVisitSiteUnique, includedParameters: [.appVersion, .atb]) + } + view.addGestureRecognizer(daxDialogViewController.tapToCompleteGestureRecognizer) daxDialogContainerHeight.constant = daxDialogViewController.calculateHeight() From daa9e78eb0d9d8f75fe3d68dfd649eb4fb8675b4 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 15:52:05 +1000 Subject: [PATCH 13/16] Differentiate end of journey dialog from ntp and tab --- Core/PixelEvent.swift | 6 ++++-- DuckDuckGo/DaxDialogs.swift | 2 +- .../ContextualDaxDialogs/NewTabDaxDialogFactory.swift | 2 +- DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift | 2 +- .../ContextualOnboardingNewTabDialogFactoryTests.swift | 2 +- 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index ab0e040b79..b0eb84228d 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -161,7 +161,8 @@ extension Pixel { case daxDialogsFireEducationShownUnique case daxDialogsFireEducationConfirmedUnique case daxDialogsFireEducationCancelledUnique - case daxDialogsEndOfJourneyUnique + case daxDialogsEndOfJourneyTabUnique + case daxDialogsEndOfJourneyNewTabUnique case widgetsOnboardingCTAPressed case widgetsOnboardingDeclineOptionPressed @@ -893,7 +894,8 @@ extension Pixel.Event { case .daxDialogsFireEducationShownUnique: return "m_dx_fe_s_unique" case .daxDialogsFireEducationConfirmedUnique: return "m_dx_fe_co_unique" case .daxDialogsFireEducationCancelledUnique: return "m_dx_fe_ca_unique" - case .daxDialogsEndOfJourneyUnique: return "m_dx_end_unique_unique" + case .daxDialogsEndOfJourneyTabUnique: return "m_dx_end_tab_unique" + case .daxDialogsEndOfJourneyNewTabUnique: return "m_dx_end_new_tab_unique" case .widgetsOnboardingCTAPressed: return "m_o_w_a" case .widgetsOnboardingDeclineOptionPressed: return "m_o_w_d" diff --git a/DuckDuckGo/DaxDialogs.swift b/DuckDuckGo/DaxDialogs.swift index d72fa5cbe4..f3ae16b179 100644 --- a/DuckDuckGo/DaxDialogs.swift +++ b/DuckDuckGo/DaxDialogs.swift @@ -154,7 +154,7 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { // Message and CTA empty on purpose as for this case we use only pixelName and type static let fire = BrowsingSpec(message: "", cta: "", highlightAddressBar: false, pixelName: .daxDialogsFireEducationShownUnique, type: .fire) - static let final = BrowsingSpec(message: UserText.daxDialogHomeSubsequent, cta: "", highlightAddressBar: false, pixelName: .daxDialogsEndOfJourneyUnique, type: .final) + static let final = BrowsingSpec(message: UserText.daxDialogHomeSubsequent, cta: "", highlightAddressBar: false, pixelName: .daxDialogsEndOfJourneyTabUnique, type: .final) let message: String let cta: String diff --git a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift index e7cac856a2..0beeabbdd6 100644 --- a/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift +++ b/DuckDuckGo/OnboardingExperiment/ContextualDaxDialogs/NewTabDaxDialogFactory.swift @@ -100,7 +100,7 @@ final class NewTabDaxDialogFactory: NewTabDaxDialogProvider { .onboardingContextualBackgroundStyle() .onFirstAppear { [weak self] in self?.contextualOnboardingLogic.setFinalOnboardingDialogSeen() - self?.onboardingPixelReporter.trackScreenImpression(event: .daxDialogsEndOfJourneyUnique) + self?.onboardingPixelReporter.trackScreenImpression(event: .daxDialogsEndOfJourneyNewTabUnique) } } } diff --git a/DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift b/DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift index e813b0080e..5ca4961dea 100644 --- a/DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift +++ b/DuckDuckGoTests/ContextualDaxDialogsFactoryTests.swift @@ -276,7 +276,7 @@ final class ContextualDaxDialogsFactoryTests: XCTestCase { func testWhenViewForFinalSpecAppearsThenExpectedPixelFires() { // GIVEN let spec = DaxDialogs.BrowsingSpec.final - let expectedPixel = Pixel.Event.daxDialogsEndOfJourneyUnique + let expectedPixel = Pixel.Event.daxDialogsEndOfJourneyTabUnique // TEST testDialogDefinedBy(spec: spec, firesEvent: expectedPixel) } diff --git a/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift b/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift index c12372ea53..de22783748 100644 --- a/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift +++ b/DuckDuckGoTests/ContextualOnboardingNewTabDialogFactoryTests.swift @@ -142,7 +142,7 @@ class ContextualOnboardingNewTabDialogFactoryTests: XCTestCase { func testWhenOnboardingFinalDialogAppearForTheFirstTime_ThenSendFireExpectedPixel() { // GIVEN let spec = DaxDialogs.HomeScreenSpec.final - let pixelEvent = Pixel.Event.daxDialogsEndOfJourneyUnique + let pixelEvent = Pixel.Event.daxDialogsEndOfJourneyNewTabUnique // TEST testDialogDefinedBy(spec: spec, firesEvent: pixelEvent) } From 7f146c85010876f420ab1cf5738db22b89c5ffd2 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 18:29:30 +1000 Subject: [PATCH 14/16] fix rebase conflicts --- DuckDuckGo/MainViewController.swift | 2 +- DuckDuckGoTests/OnboardingDaxFavouritesTests.swift | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 6f99f0be91..899293ccef 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -187,7 +187,7 @@ class MainViewController: UIViewController { contextualOnboardingPresenter: ContextualOnboardingPresenting, contextualOnboardingLogic: ContextualOnboardingLogic, contextualOnboardingPixelReporter: OnboardingCustomInteractionPixelReporting, - tutorialSettings: TutorialSettings = DefaultTutorialSettings() + tutorialSettings: TutorialSettings = DefaultTutorialSettings(), statisticsStore: StatisticsStore = StatisticsUserDefaults() ) { self.bookmarksDatabase = bookmarksDatabase diff --git a/DuckDuckGoTests/OnboardingDaxFavouritesTests.swift b/DuckDuckGoTests/OnboardingDaxFavouritesTests.swift index e85d1416e3..2d72febe57 100644 --- a/DuckDuckGoTests/OnboardingDaxFavouritesTests.swift +++ b/DuckDuckGoTests/OnboardingDaxFavouritesTests.swift @@ -75,6 +75,7 @@ final class OnboardingDaxFavouritesTests: XCTestCase { variantManager: MockVariantManager(), contextualOnboardingPresenter: ContextualOnboardingPresenterMock(), contextualOnboardingLogic: contextualOnboardingLogicMock, + contextualOnboardingPixelReporter: OnboardingPixelReporterMock(), tutorialSettings: tutorialSettingsMock ) let window = UIWindow(frame: UIScreen.main.bounds) From 6894299c14a7c6eed7c805388bbdb35512768c42 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 18:41:10 +1000 Subject: [PATCH 15/16] Aligned Privacy dashboard first time pixel with Android --- DuckDuckGo/MainViewController.swift | 4 +++- .../Pixels/OnboardingPixelReporter.swift | 15 +++++++------ .../OnboardingNavigationDelegateTests.swift | 10 +++------ .../OnboardingPixelReporterMock.swift | 4 +--- .../OnboardingPixelReporterTests.swift | 21 +++++-------------- 5 files changed, 19 insertions(+), 35 deletions(-) diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 899293ccef..9a3ec77028 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -1730,7 +1730,9 @@ extension MainViewController: OmniBarDelegate { guard !isSERPPresented else { return } // Track first tap of privacy icon button - contextualOnboardingPixelReporter.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: isHighlighted) + if isHighlighted { + contextualOnboardingPixelReporter.trackPrivacyDashboardOpenedForFirstTime() + } // Dismiss privacy icon animation when showing privacy dashboard dismissPrivacyDashboardButtonPulse() diff --git a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift index 827625299f..ab9f2b6144 100644 --- a/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift +++ b/DuckDuckGo/OnboardingExperiment/Pixels/OnboardingPixelReporter.swift @@ -62,7 +62,7 @@ protocol OnboardingCustomInteractionPixelReporting { func trackCustomSearch() func trackCustomSite() func trackSecondSiteVisit() - func trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: Bool) + func trackPrivacyDashboardOpenedForFirstTime() } protocol OnboardingScreenImpressionReporting { @@ -96,12 +96,11 @@ final class OnboardingPixelReporter { self.userDefaults = userDefaults } - private func fire(event: Pixel.Event, unique: Bool, additionalParameters: [String: String] = [:]) { - let parameters: [Pixel.QueryParameters] = [.appVersion, .atb] + private func fire(event: Pixel.Event, unique: Bool, additionalParameters: [String: String] = [:], includedParameters: [Pixel.QueryParameters] = [.appVersion, .atb]) { if unique { - uniquePixel.fire(pixel: event, withAdditionalParameters: additionalParameters, includedParameters: parameters) + uniquePixel.fire(pixel: event, withAdditionalParameters: additionalParameters, includedParameters: includedParameters) } else { - pixel.fire(pixel: event, withAdditionalParameters: additionalParameters, includedParameters: parameters) + pixel.fire(pixel: event, withAdditionalParameters: additionalParameters, includedParameters: includedParameters) } } @@ -163,13 +162,13 @@ extension OnboardingPixelReporter: OnboardingCustomInteractionPixelReporting { } } - func trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: Bool) { + func trackPrivacyDashboardOpenedForFirstTime() { let daysSinceInstall = statisticsStore.installDate.flatMap { calendar.numberOfDaysBetween($0, and: dateProvider()) } let additionalParameters = [ - PixelParameters.fromOnboarding: String(fromOnboarding), + PixelParameters.fromOnboarding: "true", PixelParameters.daysSinceInstall: String(daysSinceInstall ?? 0) ] - fire(event: .privacyDashboardFirstTimeOpenedUnique, unique: true, additionalParameters: additionalParameters) + fire(event: .privacyDashboardFirstTimeOpenedUnique, unique: true, additionalParameters: additionalParameters, includedParameters: [.appVersion]) } } diff --git a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift index 6cd4142b1b..ab48dedbbc 100644 --- a/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift +++ b/DuckDuckGoTests/OnboardingNavigationDelegateTests.swift @@ -155,32 +155,28 @@ final class OnboardingNavigationDelegateTests: XCTestCase { // MARK: Pixel - func testWhenPrivacyBarIconIsPressed_AndPrivacyIconIsHighlighted_ThenFireFirstTimePrivacyDashboardUsedPixel_AndFromOnboardingIsTrue() { + func testWhenPrivacyBarIconIsPressed_AndPrivacyIconIsHighlighted_ThenFireFirstTimePrivacyDashboardUsedPixel() { // GIVEN let isHighlighted = true XCTAssertFalse(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) - XCTAssertNil(onboardingPixelReporter.capturedFromOnboarding) // WHEN mainVC.onPrivacyIconPressed(isHighlighted: isHighlighted) // THEN XCTAssertTrue(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) - XCTAssertEqual(onboardingPixelReporter.capturedFromOnboarding, true) } - func testWhenPrivacyBarIconIsPressed_AndPrivacyIconIsNotHighlighted_ThenFireFirstTimePrivacyDashboardUsedPixel_AndFromOnboardingIsFalse() { + func testWhenPrivacyBarIconIsPressed_AndPrivacyIconIsNotHighlighted_ThenDoNotFireFirstTimePrivacyDashboardUsedPixel() { // GIVEN let isHighlighted = false XCTAssertFalse(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) - XCTAssertNil(onboardingPixelReporter.capturedFromOnboarding) // WHEN mainVC.onPrivacyIconPressed(isHighlighted: isHighlighted) // THEN - XCTAssertTrue(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) - XCTAssertEqual(onboardingPixelReporter.capturedFromOnboarding, false) + XCTAssertFalse(onboardingPixelReporter.didCallTrackPrivacyDashboardOpenedForFirstTime) } } diff --git a/DuckDuckGoTests/OnboardingPixelReporterMock.swift b/DuckDuckGoTests/OnboardingPixelReporterMock.swift index 3c3386d8de..fcfe097b50 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterMock.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterMock.swift @@ -35,7 +35,6 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting private(set) var didCallTrackScreenImpressionCalled = false private(set) var capturedScreenImpression: Pixel.Event? private(set) var didCallTrackPrivacyDashboardOpenedForFirstTime = false - private(set) var capturedFromOnboarding: Bool? func trackSiteSuggetionOptionTapped() { didCallTrackSiteOptionTapped = true @@ -62,8 +61,7 @@ final class OnboardingPixelReporterMock: OnboardingSiteSuggestionsPixelReporting capturedScreenImpression = event } - func trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: Bool) { + func trackPrivacyDashboardOpenedForFirstTime() { didCallTrackPrivacyDashboardOpenedForFirstTime = true - capturedFromOnboarding = fromOnboarding } } diff --git a/DuckDuckGoTests/OnboardingPixelReporterTests.swift b/DuckDuckGoTests/OnboardingPixelReporterTests.swift index cb49a7d4e5..6eaa415171 100644 --- a/DuckDuckGoTests/OnboardingPixelReporterTests.swift +++ b/DuckDuckGoTests/OnboardingPixelReporterTests.swift @@ -234,37 +234,26 @@ final class OnboardingPixelReporterTests: XCTestCase { XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, []) // WHEN - sut.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: true) + sut.trackPrivacyDashboardOpenedForFirstTime() // THEN XCTAssertTrue(OnboardingUniquePixelFireMock.didCallFire) XCTAssertEqual(OnboardingUniquePixelFireMock.capturedPixelEvent, expectedPixel) XCTAssertEqual(expectedPixel.name, "m_privacy_dashboard_first_time_used_unique") - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion, .atb]) + XCTAssertEqual(OnboardingUniquePixelFireMock.capturedIncludeParameters, [.appVersion]) } - func testWhenTrackPrivacyDashboardOpenedForFirstTimeAndFromOnboardingFalseThenParameterIsSetToFalse() { + func testWhenTrackPrivacyDashboardOpenedForFirstTimeThenFromOnboardingParameterIsSetToTrue() { // GIVEN XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) // WHEN - sut.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: true) + sut.trackPrivacyDashboardOpenedForFirstTime() // THEN XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams["from_onboarding"], "true") } - func testWhenTrackPrivacyDashboardOpenedForFirstTimeAndFromOnboardingTrueThenParameterIsSetToTrue() { - // GIVEN - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) - - // WHEN - sut.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: false) - - // THEN - XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams["from_onboarding"], "false") - } - func testWhenTrackPrivacyDashboardOpenedForFirstTimeThenDaysSinceInstallParameterIsSet() { // GIVEN let installDate = Date(timeIntervalSince1970: 1722348000) // 30th July 2024 GMT @@ -273,7 +262,7 @@ final class OnboardingPixelReporterTests: XCTestCase { XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams, [:]) // WHEN - sut.trackPrivacyDashboardOpenedForFirstTime(fromOnboarding: false) + sut.trackPrivacyDashboardOpenedForFirstTime() // THEN XCTAssertEqual(OnboardingUniquePixelFireMock.capturedParams["daysSinceInstall"], "3") From 4acae5752a77917f52574c6cec7d3c83f8d3007c Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Fri, 2 Aug 2024 18:51:54 +1000 Subject: [PATCH 16/16] Fix Swiftlint comment --- DuckDuckGo/OnFirstAppearViewModifier.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/DuckDuckGo/OnFirstAppearViewModifier.swift b/DuckDuckGo/OnFirstAppearViewModifier.swift index 6f424bd945..784ab8999b 100644 --- a/DuckDuckGo/OnFirstAppearViewModifier.swift +++ b/DuckDuckGo/OnFirstAppearViewModifier.swift @@ -1,5 +1,5 @@ // -// OnFirstAppear.swift +// OnFirstAppearViewModifier.swift // DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. @@ -33,7 +33,7 @@ import SwiftUI /// ``` /// /// - Parameter onFirstAppearAction: A closure to be executed the first time the view appears. -public struct OnFirstAppearModifier: ViewModifier { +public struct OnFirstAppearViewModifier: ViewModifier { private let onFirstAppearAction: () -> Void @State private var hasAppeared = false @@ -59,7 +59,7 @@ extension View { /// - Parameter action: A closure to be executed the first time the view appears. /// - Returns: A view that will execute `action` only once when it appears. func onFirstAppear(_ onFirstAppearAction: @escaping () -> Void ) -> some View { - return modifier(OnFirstAppearModifier(onFirstAppearAction)) + return modifier(OnFirstAppearViewModifier(onFirstAppearAction)) } }