Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

feat: show current email during password reset and auto-populate text-box after successful completion#59645

Merged
evict merged 7 commits into
sourcegraph:mainfrom
namit-chandwani:feat/populate-username-pwd-reset
Jul 12, 2024
Merged

feat: show current email during password reset and auto-populate text-box after successful completion#59645
evict merged 7 commits into
sourcegraph:mainfrom
namit-chandwani:feat/populate-username-pwd-reset

Conversation

@namit-chandwani

@namit-chandwani namit-chandwani commented Jan 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Linked Issues

Motivation and Context:

  • Improves the UX of the password reset flow

Changes Made:

  • Made changes to the following 2 flows:
    • On the new password entry screen:
      • Added a text which displays the email of the account for which the password change request has been raised
      • Added a back button to allow the users to go back to the previous email entry screen if the account they want to reset the password for is different
    • On the sign-in screen which comes after successful password reset request completion:
      • Made changes to auto-populate the email text-box with the email linked to the account on which the password reset request was completed recently

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (altering code without changing its external behaviour)
  • Documentation change
  • Other

Checklist:

  • Development completed
  • Comments added to code (wherever necessary)
  • Documentation updated (if applicable)
  • Tested changes locally

Follow-up tasks (if any):

  • None

Test Plan

Screen.Recording.2024-01-17.at.12.10.29.AM.mov
  • Screen recording of the sign-in screen after password reset is successfully done where the email ID is auto-populated in the text-box:
Screen.Recording.2024-01-17.at.12.33.23.AM.mov

Additional Comments

  • Please let me know if I need to make any further design changes from the frontend side or any API contract related changes from the backend side

@cla-bot cla-bot Bot added the cla-signed label Jan 16, 2024
@namit-chandwani namit-chandwani marked this pull request as draft January 16, 2024 18:46
@namit-chandwani namit-chandwani changed the title feat: show current email during password reset and auto-populate text-box after successful completion [WIP] feat: show current email during password reset and auto-populate text-box after successful completion Jan 16, 2024
@namit-chandwani namit-chandwani force-pushed the feat/populate-username-pwd-reset branch from 5df8962 to 72f6421 Compare January 16, 2024 20:39
@namit-chandwani namit-chandwani marked this pull request as ready for review January 16, 2024 20:49
@namit-chandwani namit-chandwani changed the title [WIP] feat: show current email during password reset and auto-populate text-box after successful completion feat: show current email during password reset and auto-populate text-box after successful completion Jan 16, 2024
@namit-chandwani namit-chandwani mentioned this pull request Jan 16, 2024
Closed
@namit-chandwani namit-chandwani force-pushed the feat/populate-username-pwd-reset branch from 996aaa0 to a76b5d3 Compare May 20, 2024 18:47
@bobheadxi bobheadxi requested review from a team May 20, 2024 20:21
@bobheadxi

bobheadxi commented May 20, 2024

Copy link
Copy Markdown
Member

Got a request for reviews for this contribution via https://github.com/sourcegraph/sourcegraph/issues/38348#issuecomment-2121019664, tagged the appropriate owners for review

evict
evict approved these changes May 21, 2024
View reviewed changes

@evict evict left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good from security PoV. 👍

eseliger
eseliger reviewed May 21, 2024
View reviewed changes

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Open question about forging the UI - otherwise LGTM

Comment thread client/web/src/auth/ResetPasswordPage.tsx
<>
{isErrorLike(this.state.submitOrError) && <ErrorAlert error={this.state.submitOrError} />}
<Container className="w-100">
<Link to='/password-reset'><Icon className="mr-1" aria-hidden={true} svgPath={mdiArrowLeftBoldBoxOutline} />Raise request for a different account</Link>

@eseliger eseliger May 21, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Curious about the choise of this icon 😬

@namit-chandwani namit-chandwani Jul 11, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I realized that this project uses icons from the @mdi/js library. After browsing the available options, I selected the one that seemed to best fit our use case 😅

Comment thread client/web/src/auth/ResetPasswordPage.tsx Outdated
Comment thread cmd/frontend/backend/users.go
eseliger
eseliger approved these changes May 21, 2024
View reviewed changes

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Talked to Vincent he didn't have concerns

@namit-chandwani

namit-chandwani commented Jul 11, 2024

Copy link
Copy Markdown
Contributor Author

Hey @eseliger @evict, thanks for the approval 🚀

Can we please merge the PR if it's good to go?

@evict evict force-pushed the feat/populate-username-pwd-reset branch from f407225 to 78cafd5 Compare July 12, 2024 14:00
@evict evict merged commit 528d98e into sourcegraph:main Jul 12, 2024
@evict

evict commented Jul 12, 2024

Copy link
Copy Markdown
Contributor

Hey @eseliger @evict, thanks for the approval 🚀

Can we please merge the PR if it's good to go?

Done! Thank you very much for working on this. 🫶 Apologies that it took a while from our side.

@namit-chandwani

namit-chandwani commented Jul 12, 2024

Copy link
Copy Markdown
Contributor Author

Hey @eseliger @evict, thanks for the approval 🚀
Can we please merge the PR if it's good to go?

Done! Thank you very much for working on this. 🫶 Apologies that it took a while from our side.

@evict No worries, I appreciate your time and feedback. Looking forward to contributing more to this project 🙌


Furthermore, I have another PR that requires review. Can you please direct me to someone who can assist with it?
Here's the link for the same: https://github.com/sourcegraph/sourcegraph/pull/60876

TIA

@namit-chandwani namit-chandwani deleted the feat/populate-username-pwd-reset branch July 12, 2024 18:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@evict evict evict approved these changes

@eseliger eseliger eseliger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Password reset link to auto populate username signin field

5 participants

@namit-chandwani @bobheadxi @evict @eseliger @shivasurya