From 4802df64799d00d0023becfbc774525a1a215347 Mon Sep 17 00:00:00 2001 From: Alessandro Boron Date: Tue, 3 Sep 2024 23:36:07 +1000 Subject: [PATCH] Fix an issue that causes the fire dialog to show multiple times for the same website after dismissing it (#3305) Task/Issue URL: https://app.asana.com/0/1206329551987282/1208180024223015/f **Description**: This PR addresses an issue that causes the fire dialog to show multiple times for the same website. --- DuckDuckGo/DaxDialogs.swift | 14 ++++++++---- DuckDuckGoTests/DaxDialogTests.swift | 34 ++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/DaxDialogs.swift b/DuckDuckGo/DaxDialogs.swift index b90447f3c2..99e16d85cb 100644 --- a/DuckDuckGo/DaxDialogs.swift +++ b/DuckDuckGo/DaxDialogs.swift @@ -316,6 +316,9 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { func dismiss() { settings.isDismissed = true + // Reset last shown dialog as we don't have to show it anymore. + removeLastShownDaxDialog() + removeLastVisitedOnboardingWebsite() } func primeForUse() { @@ -375,6 +378,7 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { } private func removeLastShownDaxDialog() { + guard isNewOnboarding else { return } settings.lastShownContextualOnboardingDialogType = nil } @@ -499,11 +503,6 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { private func nextBrowsingMessageExperiment(privacyInfo: PrivacyInfo) -> BrowsingSpec? { - if let lastVisitedOnboardingWebsiteURLPath, - compareUrls(url1: URL(string: lastVisitedOnboardingWebsiteURLPath), url2: privacyInfo.url) { - return lastShownDaxDialog(privacyInfo: privacyInfo) - } - func hasTrackers(host: String) -> Bool { isFacebookOrGoogle(privacyInfo.url) || isOwnedByFacebookOrGoogle(host) != nil || blockedEntityNames(privacyInfo.trackerInfo) != nil } @@ -513,6 +512,11 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic { guard isEnabled, nextHomeScreenMessageOverride == nil else { return nil } + if let lastVisitedOnboardingWebsiteURLPath, + compareUrls(url1: URL(string: lastVisitedOnboardingWebsiteURLPath), url2: privacyInfo.url) { + return lastShownDaxDialog(privacyInfo: privacyInfo) + } + guard let host = privacyInfo.domain else { return nil } var spec: BrowsingSpec? diff --git a/DuckDuckGoTests/DaxDialogTests.swift b/DuckDuckGoTests/DaxDialogTests.swift index 5e1a3e2e73..7bf98a7423 100644 --- a/DuckDuckGoTests/DaxDialogTests.swift +++ b/DuckDuckGoTests/DaxDialogTests.swift @@ -1002,6 +1002,40 @@ final class DaxDialog: XCTestCase { XCTAssertEqual(result?.type, .siteOwnedByMajorTracker) } + func testWhenExperimentGroup_AndDismissIsCalled_ThenLastVisitedOnboardingWebsiteAndLastShownDaxDialogAreSetToNil() { + // GIVEN + let settings = InMemoryDaxDialogsSettings() + settings.lastShownContextualOnboardingDialogType = DaxDialogs.BrowsingSpec.fire.type.rawValue + settings.lastVisitedOnboardingWebsiteURLPath = "https://www.example.com" + let sut = makeExperimentSUT(settings: settings) + XCTAssertNotNil(settings.lastShownContextualOnboardingDialogType) + XCTAssertNotNil(settings.lastVisitedOnboardingWebsiteURLPath) + + // WHEN + sut.dismiss() + + // THEN + XCTAssertNil(settings.lastShownContextualOnboardingDialogType) + XCTAssertNil(settings.lastVisitedOnboardingWebsiteURLPath) + } + + func testWhenExperimentGroup_AndIsEnabledIsFalse_AndReloadWebsite_ThenReturnNilBrowsingSpec() throws { + // GIVEN + let lastVisitedWebsitePath = "https://www.example.com" + let lastVisitedWebsiteURL = try XCTUnwrap(URL(string: lastVisitedWebsitePath)) + let settings = InMemoryDaxDialogsSettings() + settings.lastShownContextualOnboardingDialogType = DaxDialogs.BrowsingSpec.fire.type.rawValue + settings.lastVisitedOnboardingWebsiteURLPath = lastVisitedWebsitePath + let sut = makeExperimentSUT(settings: settings) + sut.dismiss() + + // WHEN + let result = sut.nextBrowsingMessageIfShouldShow(for: makePrivacyInfo(url: lastVisitedWebsiteURL)) + + // THEN + XCTAssertNil(result) + } + private func detectedTrackerFrom(_ url: URL, pageUrl: String) -> DetectedRequest { let entity = entityProvider.entity(forHost: url.host!) return DetectedRequest(url: url.absoluteString,