Corrected the Wrong edit user details error #1075
Conversation
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
|
@umangachapagain what is this PR about? why does it has 9 commits? can you please go through this book Pro Git(https://git-scm.com/book/en/v2). That will be good. |
|
@umangachapagain There needs to be a bugzilla created for this issue and once the bugzilla gets acked, after that only this PR can be merged. Please rebase and attach bugzilla id to the PR as well. |
|
@gnehapk @cloudbehl I have rebased and attached the bugzilla id. Please verify. |
| @@ -65,7 +65,10 @@ | |||
| vm.errorMsg = "Please enter a valid Email Id"; | |||
| } | |||
| } else if (keys.indexOf("name") !== -1) { | |||
| vm.errorMsg = "Name is too short (minimum is 4 characters)."; | |||
| if(messages.indexOf("is too short (minimum is 4 characters)") !== -1) | |||
There was a problem hiding this comment.
not in favor of doing character/string machine to show an error. Easy way to handle this is with below example.
else if (keys.indexOf("name") !== -1) {
vm.errorMsg = messages;
}
Also, I am not in favor of checking this kind of error messages from the API. Before doing an API call we should test this on client side and validate. because I think these are simple validations and can be handled on UI side Easily. @gnehapk thoughts?
There was a problem hiding this comment.
@umangachapagain Can you please handle this validation from the client side so that API errors do not need to be checked.
| if(form.name.$dirty && form.name.$error.maxlength){ | ||
| vm.errorMsg = "Name is too long (maximum 20 characters)."; | ||
| isFormValid = false; | ||
| } else if (form.name.$invalid) { |
There was a problem hiding this comment.
Is there a case where "form.name.$invalid" is reached?
There was a problem hiding this comment.
@umangachapagain why you think it will not?
I think when user will try to submit form without name it will show this error? what you think
There was a problem hiding this comment.
can user submit form without name?
Because we have minlength of 4 and maxlength of 20, whenever user types anything smaller than 4 characters, i get an inbuilt error prompt that says "Extend name to 4 characters" and when its longer than 20 characters, we throw a "Name is too long" error.
any thing other than minlength and maxlength could be caught here, but as I see it, there are no other restrictions on name. I can even write a whole script as my name and it will accept it as long as length requirement is satisfied.
|
@gnehapk Reverted to previous fix. Please review. |
| if(messages.indexOf("is too short (minimum is 4 characters)") !== -1) | ||
| vm.errorMsg = "Name is too short (minimum is 4 characters)."; | ||
| else if(messages.indexOf("is too long (maximum is 20 characters)") !== -1) | ||
| vm.errorMsg = "Name is too long (maximum 20 characters)."; |
There was a problem hiding this comment.
@umangachapagain you have missed "is" in the error message. It should be - Name is too long (maximum is 20 characters).
umangachapagain
dismissed stale reviews from cloudbehl and gnehapk
via
dd738f3
|
@TimothyAsirJeyasing the codacy CI build for this PR is on waiting state. Can you please check? |
|
run build |
tendrl-bug-id: /issues/1070 bugzilla-id: 1633542 Signed-off-by: Umanga Chapagain <[email protected]>
tendrl-bug-id: /issues/1070
bugzilla-id: 1633542
Signed-off-by: Umanga Chapagain [email protected]