Skip to content

Commit

Permalink
Fix an issue that causes the fire dialog to show multiple times for t…
Browse files Browse the repository at this point in the history
…he 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.
  • Loading branch information
alessandroboron authored Sep 3, 2024
1 parent 37e072f commit 4802df6
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 5 deletions.
14 changes: 9 additions & 5 deletions DuckDuckGo/DaxDialogs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -375,6 +378,7 @@ final class DaxDialogs: NewTabDialogSpecProvider, ContextualOnboardingLogic {
}

private func removeLastShownDaxDialog() {
guard isNewOnboarding else { return }
settings.lastShownContextualOnboardingDialogType = nil
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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?
Expand Down
34 changes: 34 additions & 0 deletions DuckDuckGoTests/DaxDialogTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 4802df6

Please sign in to comment.