fix(glean): Add missing 2FA Glean events#18345
Conversation
vpomerleau
changed the title
vpomerleau
force-pushed
the
FXA-11087
branch
from
37a3778 to
602ed42
Compare
Because: * We want full metrics coverage for the 2FA flows before rolling out recovery phone This commit: * Add missing 2FA setup custom events, automated click events, and reason codes (ensure events have reason codes for setup from settings, inline or to replace codes) Closes #FXA-11087
vpomerleau
force-pushed
the
FXA-11087
branch
from
602ed42 to
830e583
Compare
vpomerleau
changed the title
vbudhram
left a comment
vbudhram
left a comment
There was a problem hiding this comment.
@vpomerleau Thanks for doing the metrics audit and adding missing metrics!
| * | ||
| * Generated from `account_banner.reactivation_success_view`. | ||
| */ | ||
| export const reactivationSuccessView = new EventMetricType( |
There was a problem hiding this comment.
I thought these got generated automatically, this is unrelated to the PR.
There was a problem hiding this comment.
We need to manually run glean-generate for a few packages, and maybe it hadn't been done for content-server in a while?
| }); | ||
|
|
||
| expect(alertBar.success).toHaveBeenCalledWith('Recovery phone removed'); | ||
| expect(alertBar.success).toHaveBeenCalledWith( |
There was a problem hiding this comment.
Thank you! I missed these when implementing the remove phone part.
| 'Recovery phone removed' | ||
| ) | ||
| ), | ||
| gleanSuccessView |
There was a problem hiding this comment.
Is this correct? Does not look like alertBar takes a glean metric.
| } | ||
|
|
||
| success(message: string) { | ||
| success(message: string, gleanEvent?: () => void) { |
There was a problem hiding this comment.
I think this is fine, but I think might be nice to have a setMetric property that gets called on this.show. Maybe a follow up.
There was a problem hiding this comment.
Sounds good, will file a follow up for this!
2 participants
Because
This pull request
Issue that this pull request solves
Closes: #FXA-11087
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
See original ticket for link to glean event audit. New and corrected events are marked as "in progress"