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 FXIOS-10666 Implement iOS "onboarding-opt-out" ping #24101

Merged

Conversation

razvanlitianu
Copy link
Collaborator

@razvanlitianu razvanlitianu commented Jan 13, 2025

📜 Tickets

Jira ticket

💡 Description

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • If needed, I updated documentation / comments for complex code and public methods
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Jan 13, 2025

Messages
📖 Project coverage: 34.18%
📖 Edited 3 files
📖 Created 0 files

Client.app: Coverage: 32.4

File Coverage
PrivacyPreferencesViewController.swift 0.0% ⚠️

Generated by 🚫 Danger Swift against a6dc557

@razvanlitianu razvanlitianu changed the title Add FXIOS-10902 Firefox iOS: Send data deletion request when DAU ping is toggled off Add FXIOS-10666 Implement iOS "onboarding-opt-out" ping Jan 16, 2025
@razvanlitianu razvanlitianu marked this pull request as ready for review January 17, 2025 09:52
@razvanlitianu razvanlitianu requested a review from a team as a code owner January 17, 2025 09:52
Copy link
Contributor

mergify bot commented Jan 17, 2025

This pull request has conflicts when rebasing. Could you fix it @razvanlitianu? 🙏

@@ -91,3 +91,26 @@ usage-reporting:
include_info_sections: false
ping_schedule:
- baseline

onboarding-opt-out:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hi @travis79 I was wondering if you think the structure of this ping is correct.

Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

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

Aside from the inclusion of the client_id, which I've requested be changed, this looks just like what I was expecting.

initial onboarding flow.
If the global telemetry preference is disabled, this ping
includes specific settings to ensure it can still be submitted.
include_client_id: true
Copy link
Member

Choose a reason for hiding this comment

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

This should be false, we don't have an ID generated when this ping will be sent so it should not include it.

Copy link
Member

@travis79 travis79 left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good now!

@razvanlitianu razvanlitianu merged commit 5774a12 into main Jan 21, 2025
10 of 11 checks passed
@razvanlitianu razvanlitianu deleted the rlitianu/FXIOS-10666-Implement-iOS-onboarding-opt-out-ping branch January 21, 2025 13:33
clarmso pushed a commit that referenced this pull request Jan 22, 2025
* Add FXIOS-10902 Firefox iOS: Send data deletion request when DAU ping is toggled off

* Update data review link

* Fix PR comments
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.

4 participants