O3-1409: Support for users changing passwords#902
O3-1409: Support for users changing passwords#902jnsereko wants to merge 3 commits intoopenmrs:mainfrom
Conversation
ibacher
left a comment
ibacher
left a comment
There was a problem hiding this comment.
@jnsereko This is great work in many ways! A few things:
- I think it would be preferable to have this as part of the (much neglected) openmrs-esm-admin-tools repo.
- Instead of a full page, I think the password change could be handled as a modal.
- As a modal, I think the width could be much smaller than it presently is.
|
|
||
| return ( | ||
| <> | ||
| <SwitcherDivider className={styles.divider} /> |
There was a problem hiding this comment.
I don't think that the SwitcherDivider should be part of this class.
| ); | ||
| }; | ||
|
|
||
| export default ChangePasswordLink; No newline at end of file |
There was a problem hiding this comment.
| export default ChangePasswordLink; | |
| export default ChangePasswordLink; | |
| padding-right: 0rem; | ||
| @include brand-02(background-color); | ||
| @extend .productiveHeading01; | ||
| width: 16rem; |
There was a problem hiding this comment.
Using the Carbon $spacing utils. I'm also not sure why you're importing ../root.scss here?
| const minLengthPassword = minLengthRegExp.test(passwordInputValue); | ||
| let errMsg = ''; | ||
| if (passwordLength === 0) { | ||
| errMsg = 'Password is empty'; |
There was a problem hiding this comment.
Any display string always needs to be translatable.
| {t('discard', 'Discard')} | ||
| </Button> | ||
| <Button | ||
| style={{ maxWidth: '50%' }} |
There was a problem hiding this comment.
I'd use SCSS rather than inline styles for this.
| import { openmrsFetch } from '@openmrs/esm-framework'; | ||
|
|
||
| export async function performPasswordChange(oldPassword: string, newPassword: string) { | ||
| const abortController = new AbortController(); |
There was a problem hiding this comment.
Not much use for an AbortController inside this method. This is what useAbortController() is for (automatically aborts requests when the UI unmounts a component).
| try { | ||
| setIsSavingPassword(true); | ||
| const response = await performPasswordChange(passwordInput.oldPassword, passwordInput.confirmPassword); | ||
| if (response.ok) { |
There was a problem hiding this comment.
if (repsonse.ok) is redundant. openmrsFetch throws an exception (so the Promise will throw an error) if the result isn't ok.
| const response = await performPasswordChange(passwordInput.oldPassword, passwordInput.confirmPassword); | ||
| if (response.ok) { | ||
| performLogout().then(() => { | ||
| const defaultLang = document.documentElement.getAttribute('data-default-lang'); |
There was a problem hiding this comment.
Uh... just redirect to the logout handler I guess... We've never forced people to re-login after changing their password though, so I'm not a huge fan of this.
|
thank you @ibacher for taking a look at this. |
2 participants
Requirements
feat,fix, orchore, among others). See existing PR titles for inspiration.For changes to apps
If applicable
Summary
O3 screen to allow users to change their password.
cc @michaelbontyes @vasharma05 @denniskigen @hadijahkyampeire
Screenshots
change.passwords.mov
Related Issue
https://openmrs.atlassian.net/browse/O3-1409
Other