Skip to content

fix: cp-7.44.0 Update traits when tracked settings change 2/2#14088

Merged
NicolasMassart merged 29 commits into
mainfrom
fix/2150_update_traits
Apr 8, 2025
Merged

fix: cp-7.44.0 Update traits when tracked settings change 2/2#14088
NicolasMassart merged 29 commits into
mainfrom
fix/2150_update_traits

Conversation

@NicolasMassart

@NicolasMassart NicolasMassart commented Mar 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR allows to instantly update user traits used in app/util/metrics/UserSettingsAnalyticsMetaData/generateUserProfileAnalyticsMetaData.ts when a property used in the traits is changed.
This prevents having to wait for user to background/foreground the app to have the metrics profile updated.

Updated traits are:

  • - ENABLE_OPENSEA_API: preferencesController.displayNftMedia
  • - NFT_AUTODETECTION: preferencesController.useNftDetection
  • - THEME: from app style
  • - TOKEN_DETECTION: preferencesController.useTokenDetection
  • - MULTI_ACCOUNT_BALANCE: preferencesController.isMultiAccountBalancesEnabled
  • - SECURITY_PROVIDERS: if preferencesController.securityAlertsEnabled it return 'blockaid' otherwise empty string

Note

THEME can be updated only on app foregrounding as it requires to go on system settings to change, forcing app background then foreground. We already have this on app state listener. So no code change, but we still have manual test process in this PR.

Out of scope:

What was updated

  • addUserTraits calls with correct properties
  • some logic for associated track events
  • existing unit tests when track events values were fixed
  • some code cleanup/simplification when possible and directly related to the changes
  • some more new unit tests for coverage to pass

Related issues

Fixes https://github.com/MetaMask/mobile-planning/issues/2150

Manual testing steps

Start testing with a fully onboarded app with user optin for metrics.

Testing ENABLE_OPENSEA_API and NFT_AUTODETECTION

  • Go in security and privacy settings and change settings
  • Check that IDENTIFY events with these props are sent with proper ON/OFF values
Screenshot 2025-03-19 at 11 08 02

Display NFT Media

Note

There's no track event here, no event defined for this and this may be something product could be interested in adding in the future, but for now, no track.

Important

Initial state should be both switches to ON for the test (change if needed).
Both ON is the expected state after a regular onboarding

Switch Display NFT Media to OFF
  • Switch Display NFT Media to OFF
  • IMPORTANT: When it's your first time switching one of these two options OFF, you will have a modal appear:
  • image
  • Press "Allow"
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"Enable OpenSea API": "ON", "NFT Autodetection": "ON"}, "type": "identify", "userId": "122c6311-e5fb-4f23-bc6a-78a3518351a5"}
 INFO  TRACK event saved {"event": "Nft Autodetection Enabled from modal", "properties": {"chainId": "0x1"}, "type": "track"}
  • Switch Display NFT Media to OFF again (no modal this time)
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"Enable OpenSea API": "OFF", "NFT Autodetection": "OFF"}, "type": "identify", "userId": "122c6311-e5fb-4f23-bc6a-78a3518351a5"}
Switch Display NFT Media to ON
  • Switch Display NFT Media back to ON
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"Enable OpenSea API": "ON"}, "type": "identify", "userId": "122c6311-e5fb-4f23-bc6a-78a3518351a5"}

Autodetect NFT

Important

Initial state should be both switches to ON for the test (change if needed).
Both ON is the expected state after a regular onboarding

Switch Autodetect NFT to OFF
  • Switch Autodetect NFT to OFF
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"NFT Autodetection": "OFF"}, "type": "identify", "userId": "122c6311-e5fb-4f23-bc6a-78a3518351a5"}
 INFO  TRACK event saved {"event": "nft_autodetection_enabled", "properties": {"NFT Autodetection": false, "location": "app_settings"}, "type": "track"}
Switch Autodetect NFT to ON
  • Switch Autodetect NFT back to ON
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"Enable OpenSea API": "ON", "NFT Autodetection": "ON"}, "type": "identify", "userId": "122c6311-e5fb-4f23-bc6a-78a3518351a5"}
 INFO  TRACK event saved {"event": "nft_autodetection_enabled", "properties": {"Enable OpenSea API": true, "NFT Autodetection": true, "location": "app_settings"}, "type": "track"}
Switch Autodetect NFT to ON when Display NFT Media is OFF
  • Initial state should be both switches to OFF for the test (change if needed).
  • Switch Autodetect NFT back to ON when Display NFT Media is OFF
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"Enable OpenSea API": "ON", "NFT Autodetection": "ON"}, "type": "identify", "userId": "122c6311-e5fb-4f23-bc6a-78a3518351a5"}
 INFO  TRACK event saved {"event": "nft_autodetection_enabled", "properties": {"Enable OpenSea API": true, "NFT Autodetection": true, "location": "app_settings"}, "type": "track"}

Testing THEME

  • Change theme to light or dark in system settings (do not close MM app)
  • Go back to MM app
  • expected log for switch to dark theme: 
     INFO  IDENTIFY event saved {"traits": {"Batch account balance requests": "ON", "Enable OpenSea API": "OFF", "NFT Autodetection": "OFF", "Theme": "dark", "applicationVersion": "7.37.1", "currentBuildNumber": "1520", "deviceBrand": "Apple", "has_marketing_consent": "ON", "operatingSystemVersion": "18.2", "platform": "ios", "security_providers": "blockaid", "token_detection_enable": "ON"}, "type": "identify", "userId": "1855b4c7-bd95-4d3e-b7ae-a6fc4bac74b1"}
 INFO  TRACK event saved {"event": "App Opened", "properties": {}, "type": "track"}
  • expected log for switch to light theme: 
     INFO  IDENTIFY event saved {"traits": {"Batch account balance requests": "ON", "Enable OpenSea API": "OFF", "NFT Autodetection": "OFF", "Theme": "light", "applicationVersion": "7.37.1", "currentBuildNumber": "1520", "deviceBrand": "Apple", "has_marketing_consent": "ON", "operatingSystemVersion": "18.2", "platform": "ios", "security_providers": "blockaid", "token_detection_enable": "ON"}, "type": "identify", "userId": "1855b4c7-bd95-4d3e-b7ae-a6fc4bac74b1"}
 INFO  TRACK event saved {"event": "App Opened", "properties": {}, "type": "track"}

Testing TOKEN_DETECTION

  • Go in advanced settings and change settings
  • Check that IDENTIFY events with these props are sent with proper ON/OFF values
  • Screenshot 2025-03-19 at 12 21 57

Note

There's no track event here, no event defined for this and this may be something product could be interested in adding in the future, but for now, no track.

Important

Initial state should be ON for the test (change if needed).
ON is the expected state after a regular onboarding

Switch Autodetect tokens to OFF
  • Switch Autodetect tokens to OFF
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"token_detection_enable": "OFF"}, "type": "identify", "userId": "1855b4c7-bd95-4d3e-b7ae-a6fc4bac74b1"}
Switch Autodetect tokens to ON
  • Switch Autodetect tokens to ON
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"token_detection_enable": "ON"}, "type": "identify", "userId": "1855b4c7-bd95-4d3e-b7ae-a6fc4bac74b1"}

Testing MULTI_ACCOUNT_BALANCE

  • Go in security and privacy settings and change settings
  • Check that IDENTIFY events with these props are sent with proper ON/OFF values
  • Screenshot 2025-03-19 at 12 29 51

Note

There's no track event here, no event defined for this and this may be something product could be interested in adding in the future, but for now, no track.

Important

Initial state should be ON for the test (change if needed).
ON is the expected state after a regular onboarding

Switch Batch account balance requests to OFF
  • Switch Batch account balance requests to OFF
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"Batch account balance requests": "OFF"}, "type": "identify", "userId": "1855b4c7-bd95-4d3e-b7ae-a6fc4bac74b1"}
Switch Batch account balance requests to ON
  • Switch Batch account balance requests to ON
  • Expected event log: 
     INFO  IDENTIFY event saved {"traits": {"Batch account balance requests": "ON"}, "type": "identify", "userId": "1855b4c7-bd95-4d3e-b7ae-a6fc4bac74b1"}

Testing SECURITY_PROVIDERS

  • Go in security and privacy settings and change settings
  • Check that IDENTIFY events with these props are sent with proper ON/OFF values
  • Screenshot 2025-03-19 at 12 35 20

Important

Initial state should be ON for the test (change if needed).
ON is the expected state after a regular onboarding

Switch Security alerts to OFF
  • Switch Security alerts to OFF
  • Expected event log: 
     INFO  TRACK event saved {"event": "Settings Updated", "properties": {"security_alerts_enabled": false}, "type": "track"}
 INFO  IDENTIFY event saved {"traits": {"security_providers": ""}, "type": "identify", "userId": "1855b4c7-bd95-4d3e-b7ae-a6fc4bac74b1"}
Switch Security alerts to ON
  • Switch Security alerts to ON
  • Expected event log: 
     INFO  TRACK event saved {"event": "Settings Updated", "properties": {"security_alerts_enabled": true}, "type": "track"}
 INFO  IDENTIFY event saved {"traits": {"security_providers": "blockaid"}, "type": "identify", "userId": "1855b4c7-bd95-4d3e-b7ae-a6fc4bac74b1"}

Important

Track event has been added to this change to ON.
It was not fired and code was very non optimal.
So now we should have the Settings Updated to true in our metrics when it was not there before.

Screenshots/Recordings

Before

Unchanged, see screenshots in tests above

After

Unchanged, see screenshots in tests above

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@NicolasMassart NicolasMassart self-assigned this Mar 18, 2025
@github-actions

github-actions Bot commented Mar 18, 2025

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-mobile-platform Mobile Platform team label Mar 18, 2025
@NicolasMassart NicolasMassart requested a review from Copilot March 18, 2025 15:22
Copilot AI reviewed Mar 18, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request updates how NFT-related traits are tracked when display settings change by leveraging the useMetrics hook across various components.

  • Introduces additional metrics tracking by adding traits when users enable or disable NFT display and auto-detection.
  • Updates trait objects in multiple views to reflect the new behavior for enabling/disabling the OpenSea API and auto-detection.
  • Adjusts callback implementations to include these metric updates.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
app/components/Views/ShowDisplayMediaNFTSheet/ShowDisplayNFTMediaSheet.tsx Imports metrics types/hooks and updates the confirm callback to add NFT display trait.
app/components/Views/Settings/DisplayNFTMediaSettings/index.tsx Updates the setting toggle to conditionally include the OpenSea API trait along with NFT auto-detection.
app/components/Views/NFTAutoDetectionModal/NFTAutoDetectionModal.tsx Incorporates metrics addition in the auto-detection modal when enabling NFT auto-detection.
app/components/Views/Settings/AutoDetectNFTSettings/index.tsx Constructs trait objects reflecting state changes for both NFT auto-detection and OpenSea API.
app/components/UI/CollectibleDetectionModal/index.tsx Adds a metrics trait update to the collectible detection modal alongside NFT detection status.
Comments suppressed due to low confidence (2)

app/components/Views/ShowDisplayMediaNFTSheet/ShowDisplayNFTMediaSheet.tsx:22

  • [nitpick] The component name 'ShowDisplayNftMediaSheet' inconsistently capitalizes 'NFT' relative to the filename. Consider renaming it to 'ShowDisplayNFTMediaSheet' for consistency.
const ShowDisplayNftMediaSheet = () => {

app/components/UI/CollectibleDetectionModal/index.tsx:39

  • [nitpick] The function name 'showToastAndEnableNFtDetection' has inconsistent casing ('NFt' instead of 'NFT'). Consider renaming it to 'showToastAndEnableNFTDetection' for clarity.
const showToastAndEnableNFtDetection = useCallback(async () => {

@NicolasMassart NicolasMassart marked this pull request as ready for review March 19, 2025 13:02
@NicolasMassart NicolasMassart requested review from a team as code owners March 19, 2025 13:02
@github-actions

github-actions Bot commented Mar 19, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 0a13800
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/e2f0da7f-1001-4c77-8752-2b517a70900b

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@NicolasMassart NicolasMassart mentioned this pull request Apr 4, 2025
Closed
7 tasks
@github-actions

github-actions Bot commented Apr 7, 2025 •
edited by metamaskbot
Loading

Copy link
Copy Markdown
Contributor

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 4c3dbb2
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/36664944-2c02-4661-aef6-6a6ee0dadc6a

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@NicolasMassart NicolasMassart changed the title fix: Update traits when tracked settings change 2/2 fix: cp-7.44.0 Update traits when tracked settings change 2/2 Apr 8, 2025
@NicolasMassart NicolasMassart enabled auto-merge April 8, 2025 10:55
@sonarqubecloud

sonarqubecloud Bot commented Apr 8, 2025

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@NicolasMassart NicolasMassart added this pull request to the merge queue Apr 8, 2025
@github-project-automation github-project-automation Bot moved this from Needs dev review to Review finalised - Ready to be merged in PR review queue Apr 8, 2025
Merged via the queue into main with commit a09607a Apr 8, 2025
@NicolasMassart NicolasMassart deleted the fix/2150_update_traits branch April 8, 2025 11:34
@github-project-automation github-project-automation Bot moved this from Review finalised - Ready to be merged to Merged, Closed or Archived in PR review queue Apr 8, 2025
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 8, 2025
@metamaskbot metamaskbot added the release-7.45.0 Issue or pull request that will be included in release 7.45.0 label Apr 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@sahar-fehri sahar-fehri sahar-fehri approved these changes

@brianacnguyen brianacnguyen brianacnguyen approved these changes

@MarioAslau MarioAslau MarioAslau approved these changes

@NidhiKJha NidhiKJha NidhiKJha approved these changes

@joaoloureirop joaoloureirop joaoloureirop approved these changes

Assignees

@NicolasMassart NicolasMassart

Labels

area-metrics release-7.45.0 Issue or pull request that will be included in release 7.45.0 team-mobile-platform Mobile Platform team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@NicolasMassart @codecov-commenter @sahar-fehri @brianacnguyen @MarioAslau @NidhiKJha @joaoloureirop @gauthierpetetin @metamaskbot