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

Add comment about no support of validate_uniqueness_of(:item).scoped_to(array) #1355

Merged
merged 1 commit into from Sep 15, 2020

Conversation

gmmcal
Copy link
Contributor

@gmmcal gmmcal commented Sep 12, 2020

Closes #814

@gmmcal gmmcal changed the title Add comment about no support of validate_uniqueness_of(:item).scoped_to(array) [skip-ci] Add comment about no support of validate_uniqueness_of(:item).scoped_to(array) [skip ci] Sep 12, 2020
Comment on lines 160 to 165
# NOTE: This gem does not fully support testing uniqueness scoped to an
# array of associations. For a reference thread, please check this issue
# https://github.com/thoughtbot/shoulda-matchers/issues/814
#
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# NOTE: This gem does not fully support testing uniqueness scoped to an
# array of associations. For a reference thread, please check this issue
# https://github.com/thoughtbot/shoulda-matchers/issues/814
#
# Support for testing uniqueness validation scoped to an association is not available.
# For more information, please refer to https://github.com/thoughtbot/shoulda-matchers/issues/814

I think we can get rid of gem word as its contextual. Also i think array is not needed.
Does this change makes sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @KapilSachdev I completely agree with the first part about removing gem. But the array part is actually the point of this PR. We can validate scoped to an association (using attribute name, not object), but when an object is uniquely scoped to more than one object, we can't really validate it. It is possible to test with scope to a list of attributes, but it is unclear how this would behave in multiple scenarios.

The solutions proposed on #814 we not as simple as I thought it could be when I first created the issue, and manually testing it (without shoulda) would be easier to read. Because of this, it was decided to add this specific comment to make clear this limitation.

I'll change the text and update PR. Thanks for suggestion

@gmmcal gmmcal changed the title Add comment about no support of validate_uniqueness_of(:item).scoped_to(array) [skip ci] Add comment about no support of validate_uniqueness_of(:item).scoped_to(array) Sep 13, 2020
Copy link
Collaborator

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

undefined method `type' for nil:NilClass when using validate_uniqueness_of(...).scoped_to(AnArray)
3 participants