Skip to content

Conversation

@nockhigan
Copy link
Contributor

The date comparison validators (before, beforeOrEqual, after, afterOrEqual, DateEqual) throw an error when comparing the target field to another field on the model if the comparison field is empty

When using this validator to compare against another field in the model, if that field is null the validator will fail
When using this validator to compare against another field in the model, if that field is null the validator will fail
This validator fails when comparing to another field that is null
This validator fails when comparing to another field that is null
this validator fails when comparing to another field that is null
@lmajano
Copy link
Contributor

lmajano commented Nov 12, 2021

I like the pr, however, I think the resolution is not correct. What you are assuming is that if no value is found in the other field to compare that the validation passes, but in reality, it shouldn't because the requirement has NOT been met. So I think that the validators should basically fail because you are expecting a date comparison but you don't get a date.

@nockhigan
Copy link
Contributor Author

I thought about that, but I'm not sure if having it fail validation if the compare date is null is the best way to handle this since that would make these validators unusable if one of the fields isn't required.

Let's say you have a start date and an end date. You set up a constraint on the start date that it must be before the end date. Now if the user doesn't enter an end date the validation will fail for start date. But maybe the end date isn't required to be entered at this time. If the date comparison validators all fail if the compare date passed to them is null it is really two constraints: the original date comparison and that the compare date value is required.

I think the better solution if that is the desired behavior in this example would be two separate constraints: the before constraint on the start date and a required constraint on the end date.

@lmajano
Copy link
Contributor

lmajano commented Nov 12, 2021

You know what, this follows the same pattern as the other validators, so I was incorrect.

@lmajano lmajano merged commit 5cc0484 into coldbox-modules:development Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants