-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add Validation to Create User Flow #1581
Add Validation to Create User Flow #1581
Conversation
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
…rds-plugin into create-user-validation
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1581 +/- ##
=======================================
Coverage 66.18% 66.18%
=======================================
Files 93 93
Lines 2339 2354 +15
Branches 312 318 +6
=======================================
+ Hits 1548 1558 +10
- Misses 722 725 +3
- Partials 69 71 +2
|
Signed-off-by: Derek Ho <[email protected]>
…-dashboards-plugin into create-user-validation
Signed-off-by: Derek Ho <[email protected]>
@@ -144,4 +147,60 @@ describe('Internal user edit', () => { | |||
expect(createErrorToast).toBeCalled(); | |||
expect(updateUser).toBeCalledTimes(0); | |||
}); | |||
|
|||
it('Create button should be disabled if no password or username on create', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should split this test into there to ensure both cases work. One where username is provided but password is not, other where password is provided and username is not, and last where both are provided
|
||
const validatePassword = () => { | ||
if (!password.match(passwordValidationRegex)) { | ||
const errorMessages = ['Password does not match minimum criteria']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about adding a dialog box that suggests the criteria. See this for reference: https://ux.stackexchange.com/questions/108307/a-clear-error-message-for-wrong-password-format
This should be better than a statement with does not match minimum criteria
. Thoughts?
/> | ||
</FormRow> | ||
|
||
<FormRow | ||
headerText="Re-enter password" | ||
helpText="The password must be identical to what you entered above." | ||
isInvalid={isRepeatPasswordInvalid} | ||
error={ | ||
isRepeatPasswordInvalid ? ['The password is not identical to the one entered above.'] : [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Both password do not match.
OR The password is not same as above.
@derek-ho / @DarshitChanpura this PR has been open for over a month. Anything we can do to move forward? (or if it's no longer something we want to do, close?) |
Closing this one, @derek-ho please re-open when you pick this back-up |
Description
Adds validation that disables the create/save changes button in the create/edit/duplicate internal user flow. Part 1 to clean-up and make the UI consistent with the checks that the backend is performing. Part 2 will be to create a password strength UI, so that users can know ahead of time whether their internal user creation will go through or not.
Category
[Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation]
Why these changes are required?
What is the old behavior before changes and new behavior after changes?
Issues Resolved
fix: #1234
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.