Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent sending duplicate home tab pixel #3580

Merged
merged 3 commits into from
Nov 15, 2024
Merged

Prevent sending duplicate home tab pixel #3580

merged 3 commits into from
Nov 15, 2024

Conversation

dus7
Copy link
Contributor

@dus7 dus7 commented Nov 15, 2024 •
edited
Loading

Task/Issue URL: https://app.asana.com/0/414235014887631/1208773981053518/f
Tech Design URL:
CC:

Description:

Re-applies a fix from #2503 on the NewTabPageViewController fixing a regression of firing duplicate mh pixels.

Steps to test this PR:

Cold start scenario

  1. Turn off automatic data clearing (see https://app.asana.com/0/0/1208749384570653/f for context).
  2. Clear all tabs or make sure New Tab page is the active tab.
  3. Cold launch the app, verify single instance of mh pixel is sent.

Tab switcher scenario

  1. Open multiple tabs, make sure there's at least one New Tab.
  2. In the tab switcher close some of tabs except New Tab.
  3. mh pixel should not be fired.

Definition of Done (Internal Only):

Internal references:

Software Engineering Expectations
Technical Design Template

@dus7 dus7 force-pushed the mariusz/duplicate-home-pixel branch from fdfacfa to 782d9fe Compare November 15, 2024 12:33
@dus7 dus7 changed the base branch from main to release/7.145.0 November 15, 2024 12:34
@dus7 dus7 force-pushed the mariusz/duplicate-home-pixel branch from 782d9fe to f783437 Compare November 15, 2024 12:35
@duckduckgo duckduckgo deleted a comment from github-actions bot Nov 15, 2024
federicocappelli
federicocappelli requested changes Nov 15, 2024
View reviewed changes
Copy link
Member

@federicocappelli federicocappelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible build issue?

DuckDuckGoTests/NewTabPageControllerPixelTests.swift Outdated
import XCTest
import Core
@testable import DuckDuckGo
import SwiftUICore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f1c1427

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All green now ✅

@dus7 dus7 changed the base branch from release/7.145.0 to main November 15, 2024 12:51
federicocappelli
federicocappelli approved these changes Nov 15, 2024
View reviewed changes
Copy link
Member

@federicocappelli federicocappelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dus7 dus7 merged commit e9bec32 into main Nov 15, 2024
16 checks passed
@dus7 dus7 deleted the mariusz/duplicate-home-pixel branch November 15, 2024 16:26
samsymons added a commit that referenced this pull request Nov 21, 2024
@samsymons
Merge branch 'main' into sam/delay-shutting-down-vpn-when-encounterin…
0af1673 
…g-error

* main:
  Wrap subscription-related navigation links in `LazyView` (#3576)
  Set `dryRun` for VPN and alpha pixels (#3578)
  apply translations for zoom text (#3583)
  Prevent sending duplicate home tab pixel (#3580)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@federicocappelli federicocappelli federicocappelli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dus7 @federicocappelli