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.

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

Add cohort assignment pixel for NTP Search Box Experiment #3539

Merged
merged 5 commits into from
Nov 13, 2024

Conversation

ayoy
Copy link
Collaborator

@ayoy ayoy commented Nov 12, 2024

Task/Issue URL: https://app.asana.com/0/1201621853593513/1208742047745355/f

Description:
This change adds a pixel that's fired when a user is enrolled into the NTP search box experiment.

Steps to test this PR:

  1. clear app data
  2. run the app from Xcode
  3. check console logs and verify that m_mac_initial-search-day-1 has been sent with count=0
  4. run the app again
  5. check console logs and verify that the pixel isn't sent.
  6. return nil in DefaultNewTabPageSearchBoxExperimentCohortDecider.cohort
  7. clear app data
  8. run the app again
  9. check console logs and verify that the pixel isn't sent.

Definition of Done:


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

Copy link
Collaborator

@SabrinaTardio SabrinaTardio left a comment

Choose a reason for hiding this comment

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

It looks good, however I noticed that the cohort assignment is triggered BEFORE the onboarding start (since we show NTP and then load the onboarding)

I think before you ask for a cohort you should check if the onboarding is finished

@ayoy
Copy link
Collaborator Author

ayoy commented Nov 13, 2024

Thanks @SabrinaTardio! I've made a change to only assign the cohort once onboarding is finished and to attempt cohort assignment on every viewWillAppear (instead of once upon HomePageVC initialization)

Copy link
Collaborator

@SabrinaTardio SabrinaTardio left a comment

Choose a reason for hiding this comment

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

Works great!

@ayoy ayoy assigned ayoy and unassigned SabrinaTardio Nov 13, 2024
@ayoy ayoy merged commit cf4389d into release/1.114.0 Nov 13, 2024
20 of 23 checks passed
@ayoy ayoy deleted the dominik/ntp-search-box-cohort-assignment-pixel branch November 13, 2024 12:01
samsymons added a commit that referenced this pull request Nov 14, 2024
# By Graeme Arthur (2) and others
# Via GitHub
* main:
  Disable reordering when sort by name is enabled (#3552)
  Move FE Overlay Pixel to the Native layer (#3550)
  Bump version to 1.114.0 (307)
  bump bsk for text zoom feature definition (#3496)
  Add support for local overrides for feature flags (#3545)
  Speculative password import prompt crash fix (#3542)
  Send error with fetch device failure pixel (#3546)
  DuckPlayer Overlay Pixels (#3538)
  Bump version to 1.114.0 (306)
  Enforce stricter hasPendingUpdate check (#3541)
  Implement Fire Window UI tests (#3544)
  Add cohort assignment pixel for NTP Search Box Experiment (#3539)
  Prevent Freemium PIR Pixel Firing for All Users (#3543)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants