Extend AM validate_length_of matcher to support array attributes#1560
Merged
mcmire merged 11 commits intothoughtbot:mainfrom Jun 12, 2023
Conversation
…force validation as_array
mcmire
reviewed
Jun 5, 2023
lib/shoulda/matchers/active_model/validate_length_of_matcher.rb
Outdated
Show resolved
Hide resolved
spec/unit/shoulda/matchers/active_model/validate_length_of_matcher_spec.rb
Show resolved
Hide resolved
Contributor
Author
|
@mcmire changes applied. This one is ready for another iteration. 👍 |
mcmire
reviewed
Jun 9, 2023
Collaborator
mcmire
left a comment
There was a problem hiding this comment.
Looking better! I just had two more suggestions related to documentation.
lib/shoulda/matchers/active_model/validate_length_of_matcher.rb
Outdated
Show resolved
Hide resolved
lib/shoulda/matchers/active_model/validate_length_of_matcher.rb
Outdated
Show resolved
Hide resolved
mcmire
approved these changes
Jun 12, 2023
Collaborator
mcmire
left a comment
There was a problem hiding this comment.
Looks good! Thanks so much for the PR!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First step to close: #1559
Solution
Were added two mechanisms to treat
validate_length_of_matcherrelated attribute as array. One was througharray_column?helper(also used onvalidate_absence_of_matcher) based on column type ifarray?helper is defined on model'scolumns_hashand another one with an explicit chain methodas_arraythat will force to validate column with arrays values.Why this way? First one will cover pure
array: truecolumns for postgresql, second one could be dynamically used to check, for example, json columns storing arrays and being treated like that. This second approach will leave more freedom when using the validator.Main validate_length_of_matcher specs were duplicated for the two variants. After all review's iteration for this PR concluded and if it's finally approved and merged this approach, I'll continue extending the array support to other validators(as inclusion/exclusion).