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

chore: update the remaining trackEvent calls to use new builder pattern #12262

Open
1 of 9 tasks
NicolasMassart opened this issue Nov 12, 2024 · 1 comment · May be fixed by #12267
Open
1 of 9 tasks

chore: update the remaining trackEvent calls to use new builder pattern #12262

NicolasMassart opened this issue Nov 12, 2024 · 1 comment · May be fixed by #12267
Assignees
Labels
Code Impact - High Major code change that can have an high impact on the codebase functionality Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing team-mobile-platform type-bug Something isn't working

Comments

@NicolasMassart
Copy link
Contributor

NicolasMassart commented Nov 12, 2024

What is this about?

Followup on #12180 that updated the metrics trackEvent and some files calling it, but not all.
Finish the update of the files that are using trackEvent in the old way to use the new one with builder.

Scenario

No response

Design

No response

Technical Details

Example current code

const { trackEvent } = useMetrics();
...
trackEvent(MetaMetricsEvents.ACTIONS_BUTTON_CLICKED, {
  text: '',
  chain_id: getDecimalChainId(chainId),
});

Example target code

const { trackEvent, createEventBuilder } = useMetrics();
...
trackEvent(createEventBuilder(MetaMetricsEvents.ACTIONS_BUTTON_CLICKED)
      .addProperties({
      text: '',
      chain_id: getDecimalChainId(chainId),
    }).build()
  );

Threat Modeling Framework

No response

Acceptance Criteria

  • all files using trackevent updated
  • all unit test files updated and tests passing

Files list

The list of the files to update is the following (some may be already fixed after I write this list)

see #12180 (comment)

Stakeholder review needed before the work gets merged

  • Engineering (needed in most cases)
  • Design
  • Product
  • QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
  • Security
  • Legal
  • Marketing
  • Management (please specify)
  • Other (please specify)

References

fix: trackevent enabled is undefined #12180

@NicolasMassart NicolasMassart added team-mobile-platform Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing Code Impact - High Major code change that can have an high impact on the codebase functionality labels Nov 12, 2024
@gauthierpetetin gauthierpetetin added the type-bug Something isn't working label Nov 12, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Nov 12, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Nov 12, 2024
@NicolasMassart NicolasMassart self-assigned this Nov 12, 2024
@NicolasMassart
Copy link
Contributor Author

NicolasMassart commented Nov 12, 2024

Note, some of the files listed above may already be fixed in #12180
See #12180 (comment)

@NicolasMassart NicolasMassart linked a pull request Nov 12, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Impact - High Major code change that can have an high impact on the codebase functionality Sev1-high An issue that may have caused fund loss or access to wallet in the past & may still be ongoing team-mobile-platform type-bug Something isn't working
Projects
Status: To be fixed
Status: To be fixed
Development

Successfully merging a pull request may close this issue.

3 participants