Skip to content

fix: reset password errors now work again#6263

Open
YazeedLoonat wants to merge 2 commits into
mainfrom
6258/password-reset-errors-fix
Open

fix: reset password errors now work again#6263
YazeedLoonat wants to merge 2 commits into
mainfrom
6258/password-reset-errors-fix

Conversation

@YazeedLoonat
Copy link
Copy Markdown
Collaborator

This PR addresses #6258

  • Addresses the issue in full

Description

Our error messaging for forgot password flows meant that the UI never pointed users to trying re-requests for password reset tokens that have been used or expired

This pr updates our error logging to fix that

How Can This Be Tested/Reviewed?

Create a public account -> go through the forgot password flow -> you should be able to successfully reset your password -> use the same link in the email you received to try and reset your password again -> you should get an error telling you to re-request a forgot password

Create a public account -> request a reset password email -> do not open it until the token expires (1 hr after getting the email) -> try and reset password with that email -> you should get an error telling you to re-request a forgot password

^ repeat these steps as a partner user

Author Checklist:

  • Added QA notes to the issue with applicable URLs
  • Reviewed in a desktop view
  • Reviewed in a mobile view
  • Reviewed considering accessibility
  • Added tests covering the changes
  • Made corresponding changes to the documentation
  • Ran yarn generate:client and/or created a migration when required

Review Process:

  • Read and understand the issue
  • Ensure the author has added QA notes
  • Review the code itself from a style point of view
  • Pull the changes down locally and test that the acceptance criteria is met
  • Either (1) explicitly ask a clarifying question, (2) request changes, or (3) approve the PR, even if there are very small remaining changes, if you don't need to re-review after the updates

@YazeedLoonat YazeedLoonat added the 1 review needed Requires 1 more review before ready to merge label May 5, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

Deploy Preview for partners-bloom-dev ready!

Name Link
🔨 Latest commit b53ad20
🔍 Latest deploy log https://app.netlify.com/projects/partners-bloom-dev/deploys/69fa201bdd15c20007926587
😎 Deploy Preview https://deploy-preview-6263--partners-bloom-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

Deploy Preview for bloom-angelopolis ready!

Name Link
🔨 Latest commit b53ad20
🔍 Latest deploy log https://app.netlify.com/projects/bloom-angelopolis/deploys/69fa201b77b71e0008e99c21
😎 Deploy Preview https://deploy-preview-6263--bloom-angelopolis.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

Deploy Preview for partners-bloom-msq2 canceled.

Name Link
🔨 Latest commit b53ad20
🔍 Latest deploy log https://app.netlify.com/projects/partners-bloom-msq2/deploys/69fa201b4b374d00082dfafa

@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

Deploy Preview for bloom-public-seeds ready!

Name Link
🔨 Latest commit b53ad20
🔍 Latest deploy log https://app.netlify.com/projects/bloom-public-seeds/deploys/69fa201bcf6c3000086f6eb4
😎 Deploy Preview https://deploy-preview-6263--bloom-public-seeds.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 5, 2026

Deploy Preview for bloom-exygy-dev ready!

Name Link
🔨 Latest commit b53ad20
🔍 Latest deploy log https://app.netlify.com/projects/bloom-exygy-dev/deploys/69fa201bffa33200084dd22a
😎 Deploy Preview https://deploy-preview-6263--bloom-exygy-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Collaborator

@emilyjablonski emilyjablonski left a comment

Choose a reason for hiding this comment

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

Would love to see some tests addded here

@YazeedLoonat
Copy link
Copy Markdown
Collaborator Author

Hey @emilyjablonski we actually have had test suites for this (reset-password.test.tsx on the public side as a unit test for example)

when we re-wrote the auth service for the prisma rework we changed how the errors were thrown and that change meant that the UI was no longer getting the kinds of errors it was expecting

or do you mean an e2e test suite on this? that might be tough with how the token is a generated jwt

@emilyjablonski
Copy link
Copy Markdown
Collaborator

The last commit you made covers em!

Copy link
Copy Markdown
Collaborator

@emilyjablonski emilyjablonski left a comment

Choose a reason for hiding this comment

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

Image

Given we can already know the token is missing, we should not require the user to enter in both passwords just to see the error, and then not be given direction about what to do next. Not all users are going to know what "Token not found. Please request for a new one." means. We should do what we do in other cases, which is to show the request modal and redirect. I worry as-is this is going to cause more confusion.

@emilyjablonski emilyjablonski added needs changes The author must make changes and then re-request review before merging and removed 1 review needed Requires 1 more review before ready to merge labels May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changes The author must make changes and then re-request review before merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants