fix(settings): skip redundant set-password account-status check#20719
Open
vpomerleau wants to merge 1 commit into
Open
fix(settings): skip redundant set-password account-status check#20719vpomerleau wants to merge 1 commit into
vpomerleau wants to merge 1 commit into
Conversation
…rward flows Because: - The post-verify set-password page blocked first paint on an accountStatus round-trip, intermittently exceeding the stage smoke test's 10s assertion and flaking passkeySetPassword.spec.ts. - Every forward flow (passkey/OTP/third-party) only routes here for passwordless accounts, so re-deriving the password state is redundant. This commit: - Skips the accountStatus check when routed here with router state, running it only for stateless arrivals (refresh/direct URL/back) where a password may exist and a redirect to /signin is still wanted. - Simplifies the check effect to a flat then/catch and resolves the stale retry TODO; corrects the inaccurate "third-party may have a password" note. - Adds container tests covering the gating. Closes #FXA-13937
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces first-paint latency (and stage smoke-test flakes) on the PostVerify Set Password page by skipping a redundant accountStatus round-trip when the page is reached via known forward flows that already imply a passwordless account.
Changes:
- Gate the blocking
accountStatuscheck based on presence of router state to allow immediate render for forward flows. - Simplify the
accountStatuseffect logic (flatthen/catch) and update the related inline comment. - Add tests to validate when the
accountStatuscheck is skipped vs executed, including fail-open behavior on errors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx | Skips blocking accountStatus check for state-routed forward flows to improve first paint; keeps redirect behavior for password-present cases. |
| packages/fxa-settings/src/pages/PostVerify/SetPassword/container.test.tsx | Adds unit tests covering the new gating behavior for accountStatus, including skip/run/error paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+74
to
78
| const stateRouted = location.state?.passwordCreationReason !== undefined; | ||
|
|
||
| const [passwordStatus, setPasswordStatus] = useState<{ | ||
| isLoading: boolean; | ||
| hasPassword: boolean; |
| }); | ||
| expect(mockNavigate).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
accountStatusround-trip, which intermittently exceeded the stage smoke test's 10s assertion and flakedpasskeySetPassword.spec.ts.This pull request
accountStatuscheck on router state inpackages/fxa-settings/src/pages/PostVerify/SetPassword/container.tsx: when the page is reached via a forward flow (location.state.passwordCreationReasonis set), it paints immediately without the round-trip./signinis still wanted.then/catch, resolves the stale retry TODO, and corrects the inaccurate "third-party may have a password" comment.Issue that this pull request solves
Closes: FXA-13937
Checklist
Put an
xin the boxes that applyHow to review (Optional)
Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Known pre-existing limitation (out of scope): on a stateless arrival where the
accountStatuscheck itself fails and the account already has a password, the page falls through to the form and/password/createrejects server-side with a "password already set" banner instead of redirecting to/signin. This requires a rare triple-coincidence and is a candidate for a follow-up.