Add ComparisonValidator to validate comparison of any objects #40095
Conversation
activemodel/test/cases/validations/comparison_validation_test.rb
Outdated
Show resolved
Hide resolved
activemodel/test/cases/validations/comparison_validation_test.rb
Outdated
Show resolved
Hide resolved
| ### `comparison` | ||
|
|
||
| This check will validate a comparison between any two comparable values. | ||
| If an option is not passed, all validations pass. Each option accepts a value, proc, or symbol. |
There was a problem hiding this comment.
I was going to suggest "supplied" to avoid passed/pass... but maybe it should just be an immediate error to define a comparison validation without any constraints?
There was a problem hiding this comment.
Do we have any existing patterns for this? I think this is the first validator that needs checks to validate anything.
I leaned towards no error in case someone dynamically supplied options, and they were missing. I also thought if there were no constraints, then there should be no errors. I can also see the argument that an error could help nudge people who've misconfigured the comparison validator.
The way I see it, we have 3 options:
👀 Error if no options supplied
🚀 Pass if no options supplied
🎉 Validate if the attribute is comparable if no options supplied, making the checks optional
I don't have a strong preference, and they're all pretty easy to implement.
There was a problem hiding this comment.
Sorry it's taken me a silly amount of time to come back to this...
I believe this is what check_validity! is for. Numericality only does a typecheck [because it always implies the must-be-a-number constraint, even without any further options], but Length and Format have precedent for "you can supply any of these options, but must supply at least one (or for Format, exactly one).
There was a problem hiding this comment.
That makes sense. I'll update the tests, code, and guide.
There was a problem hiding this comment.
I have made the requested changes!
|
👋🏻! What do you think about having NumericalityValidator inherit from this, such that it ends up being "make_sure_its_a_number && super"? Worth considering as a future change, or probably not worth it? |
|
@matthewd I tried out the inheritance and it ended up being more trouble than expected. With a decent refactor to NumericalityValidator though, I can add a Comparability module similar to Clusivity. |
ChaelCodes
force-pushed
the
cc-comparablity-validator
branch
from
c342309
to
9bf9685
Compare
|
This PR is ready to review. Let me know if there are any other requested changes. I'd be happy to implement them. |
…omparable values. We allow for compare validations in NumericalityValidator, but these only work on numbers. There are various comparisons people may want to validate, from dates to strings, to custom comparisons. ``` validates_comparison_of :end_date, greater_than: :start_date ``` Refactor NumericalityValidator to share module Comparison with ComparabilityValidator * Move creating the option_value into a reusable module * Separate COMPARE_CHECKS which support compare functions and accept values * Move odd/even checks to NUMBER_CHECKS as they can only be run on numbers
ChaelCodes
force-pushed
the
cc-comparablity-validator
branch
from
b9c1557
to
9a08a2f
Compare
…sert_valid_values.
|
@rafaelfranca @matthewd I've just rebased to incorporate the new changes to NumericalityValidator, and renamed |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
| def check_validity! | ||
| unless (options.keys & COMPARE_CHECKS.keys).any? | ||
| raise ArgumentError, "Expected one of :greater_than, :greater_than_or_equal_to, "\ | ||
| ":equal_to, :less_than, :less_than_or_equal_to, nor :other_than supplied." |
There was a problem hiding this comment.
@ChaelCodes Typo: nor -> or
I would also add option in the exception message like: ... or :other_than option supplied.
It was accidentally changed in #40095.
Summary
We allow for compare validations in NumericalityValidator, but these
only work on numbers. There are various comparisons people may want
to validate, from dates to strings, to custom comparisons.
Other Information
My main reason for creating this validator is the number of workarounds for validating that a start_date begins before an end_date.
Instead of making something specific like a date validator, I thought it would be helpful to create a generic comparison validator, to support a variety of circumstances, including custom comparables.