Skip to content
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

fix: add client-side validation for @PastOrPresent and @FutureOrPresent #8684

Merged
merged 2 commits into from Jul 6, 2020

Conversation

vlukashov
Copy link

Client-side validators interpret 'present' as 'within one second of now'.

NOTE: JSR-380 server-side validators interpret 'present' depending on the Java property type (could be the whole current year for java.time.Year (see https://beanvalidation.org/2.0-jsr380/spec/#builtinconstraints-pastorpresent)

@vlukashov vlukashov added the hilla Issues related to Hilla label Jul 3, 2020
@Haprog
Copy link
Contributor

Haprog commented Jul 3, 2020

I think this implementation is not really useful for real use cases. Proper way imo would be to check the resolution of the date value (in this case see does the date string contain hours, minutes, seconds etc.) and then create the comparison date and convert that to the same resolution so e.g. by dropping time of day if the original value only has a date but no time. And then the comparison needs to be done with >= or <= so I think it would make sense to make our own implementation of the comparison methods similar to https://github.com/validatorjs/validator.js/blob/master/src/lib/isBefore.js but using different operator.

Then it would be possible to have e.g. date picker where the value doesn't have a time and "present" would match the date of today in all cases.

Viktor Lukashov added 2 commits July 6, 2020 11:20
Client-side validators interpret 'present' as 'within one second of _now_'.

NOTE: JSR-380 server-side validators interpret 'present' depending on the Java property type (could be the whole current year for `java.time.Year` (see https://beanvalidation.org/2.0-jsr380/spec/#builtinconstraints-pastorpresent)
…OrPresent

It's not trivial to ensure the same granularity of _present_ as on the server-side: year / month / day / minute.
Until there is a sensible client-side implementation that works close enough to the server-side, it's better not to have any client-side validation for these constraints than to have a broken implementaiton.
@vlukashov vlukashov force-pushed the vl/fix/add-missing-validators-impl branch from b3e559d to 0793bfc Compare July 6, 2020 08:33
@vaadin-bot
Copy link
Collaborator

SonarQube analysis reported 1 issue

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MAJOR OpenApiObjectGenerator.java#: This file has 753 lines, which is greater than 750 authorized. Split it into smaller files. rule

@vaadin-bot vaadin-bot added +1.0.0 and removed +0.0.1 labels Jul 6, 2020
@vlukashov vlukashov merged commit f987bb9 into master Jul 6, 2020
@vlukashov vlukashov deleted the vl/fix/add-missing-validators-impl branch July 6, 2020 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hilla Issues related to Hilla +1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants