-
Notifications
You must be signed in to change notification settings - Fork 77
JC-2379 Clear password and passwordConfirm fields when they're different during new user registration #261
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
base: master
Are you sure you want to change the base?
Conversation
…ent during new user registration
| if(result.hasErrors()) { | ||
| userDto.setPassword(notEncodedPassword); | ||
| if (wrongPasswordConfirmError(result)) { | ||
| removePasswords(registerUserDto); |
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.
removePasswordsFromForm?
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.
Don't think that it's correct to mix layers. TransactionalAuthenticator wouldn't know about form. Proposed method name just for programmer and can tangle.
| } | ||
|
|
||
|
|
||
| private void removePasswords(final RegisterUserDto registerUserDto) { |
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.
Please add JavaDocs explaining what this does and why it's needed.
Also, we don't have a habit to put final for local vars and parameters :)
| registerUserDto.getUserDto().setPassword(null); | ||
| } | ||
|
|
||
| private BindingResult removeWrongPasswordConfirmError(BindingResult srcErrors) { |
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.
JavaDocs for this too
| @Test | ||
| public void userMustSetToNullPasswordAndPasswordConfirmFieldsWhenTheyAreDifferent() throws Exception { | ||
|
|
||
| Validator customValidator = new Validator() { |
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.
Why can't you use the usual validator that's used in prod code?
| } | ||
|
|
||
| @Test | ||
| public void userMustSetToNullPasswordAndPasswordConfirmFieldsWhenTheyAreDifferent() throws Exception { |
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.
emptiesPassowordAndConfirmationFields_ifTheyDoNotMatch
Maybe let's add some javadocs here explaining why it behaves like this.
| } | ||
|
|
||
| @Test | ||
| public void userMustRestorePasswordAndPasswordConfirmFieldsWhenTheyAreEquals() throws Exception { |
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.
doesNotEmptyPasswordAndConfirmationFields_ifTheyMatch
No description provided.