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

feat(metrics): add view and submit events for password reset #15969

Merged
merged 1 commit into from
Oct 28, 2023

Conversation

chenba
Copy link
Contributor

@chenba chenba commented Oct 25, 2023

Because:

  • we should send metrics on page view and form submission on the password reset page

This commit:

  • adds the view and submit events to reset password

Closes FXA-8493, FXA-8494

Copy link
Contributor

@vpomerleau vpomerleau left a comment

Choose a reason for hiding this comment

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

LGTM! I went through the flow locally and see the pings for view and submit in the console and in the network tab, where payload is not human-readable.

Just one small question around tests, non-blocking.

@@ -139,6 +152,8 @@ describe('PageResetPassword', () => {
fireEvent.click(await screen.findByText('Begin reset'));
});

expect(GleanMetrics.resetPassword.submit).toHaveBeenCalledTimes(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also add a check for a submit ping when account is unknown or requests are throttled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 couldn't hurt.

Because:
 - we should send metrics on page view and form submission on the
   password reset page

This commit:
 - adds the view and submit events to reset password
@chenba chenba force-pushed the FXA-8493-password-reset-view-glean branch from 57026ff to f02ca03 Compare October 27, 2023 21:38
@chenba chenba merged commit 40cd252 into main Oct 28, 2023
18 checks passed
@chenba chenba deleted the FXA-8493-password-reset-view-glean branch October 28, 2023 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants