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

MPP-3644: Add Mozilla accounts opt-out property to Profile and API #4392

Merged
merged 9 commits into from
Feb 13, 2024
Merged

MPP-3644: Add Mozilla accounts opt-out property to Profile and API #4392

merged 9 commits into from
Feb 13, 2024

Conversation

jwhitlock
Copy link
Member

@jwhitlock jwhitlock commented Feb 5, 2024 •
edited by groovecoder
Loading

The Metrics for Relying Parties - Opt-Opt section says:

Users may opt-out of metrics from the FxA settings page. If they do, FxA will not collect or store metrics and relying parties must not either.

User profiles include the metricsEnabled boolean. When the value is false relying parties should not collect any metrics tied to the user. Relying parties should check this value every time the profile is requested.

For MPP-3644, this PR implements reading this value on the backend, and forwarding it via the API.

This also includes front-end changes to use this new signal to decide if we should load Google Analytics.

Testing

  1. Run npm -w frontend install to install the new react-singleton-hook dependency
  2. Run npm -w frontend run watch to build the frontend with debugging on
  3. Start up the server locally with ./manage.py runserver,
  4. Load http://127.0.0.1:8000/ , open developer tools, and reload
    • You should see Logs in the dev tools Console about Google Analytics loading, sending events and pageviews
  5. If not logged in, log in
    • You should see Logs in the dev tools Console about Google Analytics loading, sending events and pageviews
  6. Navigate to http://127.0.0.1:8000/api/v1/profiles/
    • You should see "metrics_enabled": true in the JSON response.
  7. Go to https://accounts.stage.mozaws.net, log in if needed, and deselect "Help improve ⁨Mozilla accounts⁩" under "Data Collection and Use".
  8. Go to http://127.0.0.1:8000/accounts/logout/ to log out, ensure the dev tools are open, clear the Console output, and then log in again
    • After log in, Google Analytics is not loaded.
  9. Go to http://127.0.0.1:8000/api/v1/profiles/.
  • You should see "metrics_enabled": false

groovecoder
groovecoder requested changes Feb 7, 2024
View reviewed changes
Copy link
Member

@groovecoder groovecoder left a comment

Choose a reason for hiding this comment

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

Agree we should add the front-end changes to this PR to land it all at once. If you want to time-box your own effort on it, I'd be happy to do it.

emails/models.py Show resolved Hide resolved
@jwhitlock jwhitlock force-pushed the add-fxa-opt-out-mpp-3644 branch from 2a04d0b to bc63d5f Compare February 9, 2024 15:34
@jwhitlock jwhitlock force-pushed the add-fxa-opt-out-mpp-3644 branch from bc63d5f to 12ef976 Compare February 9, 2024 15:44
@jwhitlock
Load GA only after checking for enabled metrics
2043ad3 
Use a singleton hook useGoogleAnalytics, default false, to control all
the code that emits Google Analytics (GA) events. MyApp watches for
metrics to load, and then, if enabled, runs initGoogleAnalytics to load
the GA script, then set useGoogleAnalytics to true to refresh with the
enabled event code.

This updates useIsLoggedIn to return a string, 'unknown' while loading,
and 'logged-in' / 'logged-out' once the profile API call is processed to
determine the logged-in status. In a similar way, useMetrics returns
'unknown', 'enabled', and 'disabled'. This protects hooks from being
called unnecessarily while the profile API is called and processed.
@jwhitlock jwhitlock force-pushed the add-fxa-opt-out-mpp-3644 branch from 12ef976 to 2043ad3 Compare February 9, 2024 15:50
@jwhitlock jwhitlock requested a review from groovecoder February 9, 2024 16:20
@jwhitlock
Copy link
Member Author

@groovecoder I've added the frontend changes to use metrics_enabled to avoid loading Google Analytics or sending event and pageview data.

bcolsson
bcolsson approved these changes Feb 9, 2024
View reviewed changes
Copy link
Collaborator

@bcolsson bcolsson left a comment

Choose a reason for hiding this comment

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

approving without comment as this doesn't affect l10n

groovecoder
groovecoder requested changes Feb 12, 2024
View reviewed changes
Copy link
Member

@groovecoder groovecoder left a comment

Choose a reason for hiding this comment

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

Some blocking questions/comments/suggestions.

Spot-check looks good!

frontend/src/hooks/googleAnalytics.ts Show resolved Hide resolved
frontend/src/apiMocks/mockData.ts Show resolved Hide resolved
frontend/src/components/layout/navigation/whatsnew/WhatsNewDashboard.test.tsx Show resolved Hide resolved
frontend/src/hooks/metrics.ts Outdated Show resolved Hide resolved
frontend/src/components/Banner.tsx Show resolved Hide resolved
frontend/src/pages/contains-tracker-warning.page.tsx Dismissed Show resolved Hide resolved
@jwhitlock
Copy link
Member Author

Thanks for the review!

  • Opened MPP-3750 to track signing tracker URL data, and marked the code scanning issues as Won't Fix
  • Answered question about OutboundLink vs Link
  • Moved the DNT check to useMetrics

@groovecoder, back you

@groovecoder groovecoder added this pull request to the merge queue Feb 13, 2024
Merged via the queue into main with commit d3d8069 Feb 13, 2024
27 checks passed
@groovecoder groovecoder deleted the add-fxa-opt-out-mpp-3644 branch February 13, 2024 18:54
@jwhitlock jwhitlock mentioned this pull request Feb 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bcolsson bcolsson bcolsson approved these changes

@groovecoder groovecoder groovecoder approved these changes

@stephaniehobson stephaniehobson Awaiting requested review from stephaniehobson

Assignees

@groovecoder groovecoder

@jwhitlock jwhitlock

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jwhitlock @groovecoder @bcolsson