mozilla / fxa

Monorepo for Mozilla Accounts (formerly Firefox Accounts)
https://mozilla.github.io/ecosystem-platform/
Mozilla Public License 2.0
580 stars 210 forks source link

bug(settings): Fix unnecessary bounce to signin page #17136

Closed dschom closed 1 month ago

dschom commented 1 month ago

Because

This pull request

Issue that this pull request solves

Closes: FXA-9848

Checklist

Put an x in the boxes that apply

Other information (Optional)

The issue here was that the code did allow the call that checked for totpStatus to settle. As a result, the code would createTotp when it did not need to. An error would ensue, and the catch block surrounding createTotp would redirect the user back to the signin page needlessly.

In addition to test updates, I've manually validated with the following steps:

LZoog commented 1 month ago

@dschom Looks like you are missing a few dependencies in that useEffect. Code changes look fine though, I'll circle back and test locally in the morning and r+. Thanks!

dschom commented 1 month ago

@LZoog Thanks for testing. I've added those dependencies.

vpomerleau commented 1 month ago

I ran through the flow locally, can confirm I don't see a bounce back to cached signin after signing in with password 🎉

I was initially confused by the mention of signup in the PR description, so tried out that flow too. Seeing a similar bounce back there where after signup: there's a redirect to signin before proceeding to inline_totp_setup. Not the end of the world, but maybe we can file a follow up?

dschom commented 1 month ago

I ran through the flow locally, can confirm I don't see a bounce back to cached signin after signing in with password 🎉

I was initially confused by the mention of signup in the PR description, so tried out that flow too.

Ah, the title should say sign in.. Sorry about that. I'll file a follow up for the sign up case as well.