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

Validates associated uniqueness for nested attributes #8308

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@jeyb
Contributor

jeyb commented Nov 25, 2012

As pointed out in #1572 and #4568, there is no uniqueness validation for nested attributes. It does check against the db for duplicate records but doesn't check the collection being passed in. This checks for that only when there is a uniqueness validation defined in the associated model.

It is a basic uniqueness validation by combining the attributes defined in the uniqueness validator. I've chosen to only pull the necessary attributes to generate the "key" instead of all attributes jeyb@1f618ff in case there are timestamps that or other user generated content that throws off the uniqueness check.

Let me know what you guys think.

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
Member

steveklabnik commented Nov 29, 2012

@acapilleri

This comment has been minimized.

Show comment
Hide comment
@acapilleri

acapilleri Dec 23, 2012

Contributor

@jeyb can you squash the commits, please?

Contributor

acapilleri commented Dec 23, 2012

@jeyb can you squash the commits, please?

@jeyb

This comment has been minimized.

Show comment
Hide comment
@jeyb

jeyb Dec 29, 2012

Contributor

@steveklabnik @jonleighton @tenderlove any comments on this PR?

Contributor

jeyb commented Dec 29, 2012

@steveklabnik @jonleighton @tenderlove any comments on this PR?

@mtrpcic

This comment has been minimized.

Show comment
Hide comment
@mtrpcic

mtrpcic Feb 10, 2013

Is there any status on the acceptance of this PR? I am encountering this exact issue, and would much appreciate this getting merged.

mtrpcic commented Feb 10, 2013

Is there any status on the acceptance of this PR? I am encountering this exact issue, and would much appreciate this getting merged.

@iurifq

This comment has been minimized.

Show comment
Hide comment
@iurifq

iurifq Aug 7, 2013

I've experienced the very same issue just now with rails 3.2.13. Any feedback for this?

iurifq commented Aug 7, 2013

I've experienced the very same issue just now with rails 3.2.13. Any feedback for this?

@laurocaetano

This comment has been minimized.

Show comment
Hide comment
@laurocaetano

laurocaetano Feb 6, 2014

Contributor

Closing for the same reason as #4568 and #11851

Contributor

laurocaetano commented Feb 6, 2014

Closing for the same reason as #4568 and #11851

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment