Skip to content

#171784033 Feature resetpassword #10

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

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

benshidanny11
Copy link
Collaborator

What does this PR do?

  • It adds reset password feature to the system

Description of Task to be completed?

  • Reset password UI
  • Send reset password email link
  • Integrate UIs with backend API

How should this be manually tested?

What are the relevant pivotal tracker stories?

Screenshots

Send reset email screen shoot
war5555

Reset password screen shoot
war23

@benshidanny11 benshidanny11 added the Ready For Review Requires code review from Dev team label Jun 12, 2020
@benshidanny11 benshidanny11 force-pushed the ft-resetpassword-171784033 branch 5 times, most recently from 7c4ad8f to 56d8743 Compare June 12, 2020 21:45
@Francois-MUGOROZI
Copy link
Collaborator

Daniel, could you use the same input fields used on Signup and login.
And also make those buttons small in width.

@benshidanny11 benshidanny11 force-pushed the ft-resetpassword-171784033 branch from 56d8743 to d94f4a9 Compare June 16, 2020 09:25
Copy link
Collaborator

@irfiacre irfiacre left a comment

Choose a reason for hiding this comment

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

Nice work @benshidanny11

@@ -0,0 +1,22 @@
import {RESET_ACTION,SEND_RESET_EMAIL_ACTION,SEND_RESET_EMAIL_IN_PROGRESS,RESET_IN_PROGRESS} from './actionTypes';
import axios from 'axios';
const sendResetEmailAPILink='https://warriorz-staging.herokuapp.com/api/v1/password/forgot';
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the sake of security, I think these two routes may be better if they were obtained from the environmental variables in the .env

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, try to resolve the conflicts and make sure everything is good.

/>
<div className="div-contaier">
<Link
className={cx("btn","btn-cancel")}
Copy link
Collaborator

@irfiacre irfiacre Jun 17, 2020

Choose a reason for hiding this comment

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

When I try to test LINK, Jest gives me an error.
If you are facing a similar issue I think you could alternatively use window.location.assign('pathname') or use useHistory
This might be easier to test and I guess more efficient.

@benshidanny11 benshidanny11 force-pushed the ft-resetpassword-171784033 branch 2 times, most recently from 0af8cd4 to c91bff8 Compare June 21, 2020 21:41
@benshidanny11 benshidanny11 force-pushed the ft-resetpassword-171784033 branch from c91bff8 to ef4eba2 Compare June 21, 2020 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review Requires code review from Dev team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants