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

Add FXIOS-10902 Firefox iOS send data deletion request when dau ping is toggled off #24263

Merged
merged 9 commits into from
Jan 27, 2025
Merged

Add FXIOS-10902 Firefox iOS send data deletion request when dau ping is toggled off #24263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0902758
Add FXIOS-10902 Firefox iOS: Send data deletion request when DAU ping…
razvanlitianu Jan 13, 2025
fe72b4c
Update data review link
razvanlitianu Jan 13, 2025
524e9d9
merge conflict
razvanlitianu Jan 17, 2025
f1128cb
Add FXIOS-10902 Firefox iOS: Send data deletion request when DAU ping…
razvanlitianu Jan 17, 2025
622a274
Merge conflict
razvanlitianu Jan 21, 2025
5912fe7
Add links for data review
razvanlitianu Jan 21, 2025
ad2987a
Fix conflict
razvanlitianu Jan 23, 2025
68d79ad
Enable follows_collection_enabled=false type pings
razvanlitianu Jan 23, 2025
a1ca3fa
Swiftlint fix
razvanlitianu Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions firefox-ios/Client/Frontend/Settings/Main/AppSettingsTableViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import Common
import UIKit
import Shared
import Glean

// MARK: - Settings Flow Delegate Protocol

Expand Down
3 changes: 3 additions & 0 deletions firefox-ios/Client/Telemetry/TelemetryWrapper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
GleanMetrics.LegacyIds.clientId.set(uuid)
}

GleanMetrics.Pings.shared.usageDeletionRequest.setEnabled(enabled: true)
GleanMetrics.Pings.shared.onboardingOptOut.setEnabled(enabled: true)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@badboy Do you think this is good spot, before init profile id? I assume we have to do the same for onboarding opt out since it has the same property.

I also wonder, do we ever need to disable them?

Copy link
Member

Choose a reason for hiding this comment

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

No disabling needed. It's only explicitly submitted anyway and we can't hit that codepath unless it's the actual disabling by a user through the UI.


// Set or generate profile id used for usage reporting
if sendUsageData {
if let uuidString = profile.prefs.stringForKey(PrefsKeys.Usage.profileId),
Expand Down
1 change: 1 addition & 0 deletions firefox-ios/Client/metrics.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,7 @@ usage:
expires: never
send_in_pings:
- usage-reporting
- usage-deletion-request

duration:
type: timing_distribution
Expand Down
24 changes: 24 additions & 0 deletions firefox-ios/Client/pings.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,27 @@ onboarding-opt-out:
metadata:
follows_collection_enabled: false
include_info_sections: false

usage-deletion-request:
description: |
This ping is submitted when a user opts out of sending usage
frequency of Firefox to Mozilla.
Sent in response to user action.
include_client_id: false
send_if_empty: true
bugs:
- https://mozilla-hub.atlassian.net/browse/FXIOS-10902
data_reviews:
- https://github.com/mozilla-mobile/firefox-ios/pull/24263
notification_emails:
- [email protected]
- [email protected]
- [email protected]
metadata:
follows_collection_enabled: false
include_info_sections: false
reasons:
set_upload_enabled: |
The ping was submitted between Glean init and Glean shutdown.
After init but before shutdown the upload of usage reporting data changed
from enabled to disabled.
Comment on lines +138 to +140
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The ping was submitted between Glean init and Glean shutdown.
After init but before shutdown the upload of usage reporting data changed
from enabled to disabled.
The ping was submitted due to the usage-reporting data preference changing from
enabled to disabled.

Copy link
Member

Choose a reason for hiding this comment

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

I said that before on, I think, the desktop implementation: due to this being a include_info_sections=false ping we won't see any reasons anyway. They are stripped from the final payload.

Copy link
Member

Choose a reason for hiding this comment

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

Note that desktop did land with the reason defined. So for documentation reasons we can probably keep it here too (it's not set in the above submit call anyway)