Skip to content

feat: add profile id metametrics trait #14189

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mathieuartu
Copy link
Contributor

Description

This PR adds profile_id as a MetaMetrics trait.

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/IDENTITY-58

Manual testing steps

  1. No manual testing steps

Screenshots/Recordings

Before

After

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.

@mathieuartu mathieuartu added No QA Needed Apply this label when your PR does not need any QA effort. No E2E Smoke Needed If the PR does not need E2E smoke test run team-identity labels Mar 21, 2025
@mathieuartu mathieuartu self-assigned this Mar 21, 2025
Copy link
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.

@mathieuartu mathieuartu marked this pull request as ready for review March 21, 2025 14:17
@mathieuartu mathieuartu requested a review from a team as a code owner March 21, 2025 14:17
Copy link
Contributor

@NicolasMassart NicolasMassart left a comment

Choose a reason for hiding this comment

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

Where is this profile_id generated and set for the first time?
We should have a call to addUserTrait when it's set or updated.
Eventually if you don't have this updated in this PR, it will have to be added in #14088

@mathieuartu
Copy link
Contributor Author

Where is this profile_id generated and set for the first time? We should have a call to addUserTrait when it's set or updated. Eventually if you don't have this updated in this PR, it will have to be added in #14088

@NicolasMassart I was under the impression that it would eventually happen at this point

Additionally, searching the project for addUserTrait does not return any single occurrence.
I'm not sure what you're asking here, should I wait for your mentioned PR to be merged before doing anything?

@NicolasMassart
Copy link
Contributor

Where is this profile_id generated and set for the first time? We should have a call to addUserTrait when it's set or updated. Eventually if you don't have this updated in this PR, it will have to be added in #14088

@NicolasMassart I was under the impression that it would eventually happen at this point

It will indeed happen here but only if user comes back to the app or opens it. So ideally the user trait is updated when you actually update the value. The app event listener entrypoint is a workaround that makes sure the value is updated if user crashes the app or never returns for values that are not updated at the moment they are changed. But with #14088 this should not be required, so ideall all new values should update when they are set.

Additionally, searching the project for addUserTrait does not return any single occurrence. I'm not sure what you're asking here, should I wait for your mentioned PR to be merged before doing anything?

Yes, sorry, I typed it by memory, it's addTraitsToUser

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No E2E Smoke Needed If the PR does not need E2E smoke test run No QA Needed Apply this label when your PR does not need any QA effort. team-identity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants