-
Notifications
You must be signed in to change notification settings - Fork 503
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
Update input validation rules and messages in HTML templates to be aligned with the openAPI definition #142
Conversation
…igned with the openAPI definition
Thank you @Holger-Mayer for this precise comparison between the backend and the frontend. And the fix. This is a great job. |
<div class="form-group"> | ||
<label class="col-sm-1 control-label">Name</label> | ||
<div class="col-sm-6"> | ||
<input id="name" name="name" class="form-control" type="text" [(ngModel)]="speciality.name" #specialityName="ngModel" /> | ||
<input id="name" name="name" class="form-control" minlength="1" maxlength="80" pattern="^[A-Za-z0-9].{0,79}$" required type="text" [(ngModel)]="speciality.name" #specialityName="ngModel" /> |
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.
suggestion: in the openapi.yaml
specification, the specialty name
doesn't have any pattern:
https://github.com/spring-petclinic/spring-petclinic-rest/blob/master/src/main/resources/openapi.yml#L1909
Earch String
field should have a pattern.
src/app/specialties/specialty-edit/specialty-edit.component.html
Outdated
Show resolved
Hide resolved
<div class="form-group has-feedback" [class.has-success]="firstName.dirty && firstName.valid" [class.has-error]="firstName.dirty && !firstName.valid"> | ||
<label for="firstName" class="col-sm-2 control-label">First Name</label> | ||
<div class="col-sm-10"> | ||
<input type="text" class="form-control" id="firstName" [(ngModel)]="owner.firstName" minlength="1" maxlength="30" pattern="^[a-zA-Z]*$" required name="firstName" #firstName="ngModel"/> |
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.
For lastname
and firstname
I think we could authorize space
and dash
characters in the frontend and the backend. We could have compound name like Jean-Baptiste
.
What you think about @alexandre-touret ?
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.
This should happen with pet owner and veterinarien names. The definition of a pattern is not trivial.
D‘Artagnon - quotes
Peterson Jr. dot and inside single space
Jean- Baptiste inside dash
A possible solution is :
^[a-zA-Z]+([ -']{0,1}[a-zA-Z]+){0,2}[.]{0,1}$
taken from
https://a-tokyo.medium.com/first-and-last-name-validation-for-forms-and-databases-d3edf29ad29d
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.
Yes it's a good compromise. We don't support extended ASCII character for other fields.
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 could create another issue for this enhancement.
…ngth corrected (80) as well as pattern
Thanks again @Holger-Mayer for your PR. Don't hesitate to submit other enhancements. You are most welcome |
Align validation rules of add and edit forms to Rest OpenAPI validation rules
The pet clinic rest api defines validation rules for component properties (see here).
The pet clinic angular application defines validation rules for fields (bound to the previously cited component properties) within the angular components <entityname>-add.component.html and <entityname>-edit.component.html files.
Currently, there is a lack of alignment among these. Some definitions in the angular application are different, and some are not defined at all.
Overview
The following tables document the situation before this change
Field is the name of the openAPI component property
Rule is the kind of rule enforced
openAPI is the validation rule value openAPI enforces
Angular Add is the validation rule the Angluar front end enforces in add functionality
Angular Edit is the validation rule then Angular front end enforces in the edit functionality
Specialty
Owner
Pet
Vet
Visit
PetType
User
User is currently not implemented in the angular front end.
NA - not applicable
Role
Role is currently not implemented in the angular front end.
NA - not applicable
Note
An anomaly is defined within the openAPI validation.
We define some fields with a minimal length but we do not have a pattern validation rule defined. This enables the user of the api to define i.e. names with all blank characters. Here in the future, the openApi description should define pattern validation rules.
In the meantime, it is assumed, that any required name field with a length > 0 may not start with a character other than [A-za-z]. This is implemented in this pull request.
What did change?
The input field requirement rules are adjusted in the add-component.html or edit.component.html files so that they follow the openAPI rules with one exception.
Where names are defined the leading character must be a letter or digit so that all spaces names are not allowed. The rule used here is pattern="^[A-Za-z0-9].{0,N}$ with N the maxlength - 1;
Example
From
to
Then the help block for the fields are adjusted
Example
From
To
And finally, the validation state enables the add/save button.
Example
From
To
Extra
The specialty add function displayed error messages in the wrong color (grey insted of red).The reason was a missing definition.
[class.has-success]="specialityName.dirty && specialityName.valid" [class.has-error]="specialityName.dirty && !specialityName.valid"
This was corrected in the file specialty-add.component.html too.