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

Arguments for creates_unique? #1035

Closed
cheerfulstoic opened this issue Nov 4, 2015 · 6 comments
Closed

Arguments for creates_unique? #1035

cheerfulstoic opened this issue Nov 4, 2015 · 6 comments

Comments

@cheerfulstoic
Copy link
Contributor

I just started using ActiveRel.creates_unique and I thought it would always only result in one relationship, but it only does that if all of the properties that I'm giving are equal to another relationship's properties. It would be nice to be able to say which properties it should match on. Then the ActiveRel code can pull all of the rest out into a ON CREATE SET

@subvertallchris
Copy link
Contributor

As a wise man once said... https://www.youtube.com/watch?v=UaU6NQzEGcg

That's really unfortunate.

@subvertallchris
Copy link
Contributor

Is this worth backporting into 5.2.x? It reminds of me that find_or_create issue, where we're changing something that's not technically a bug but it defies expectations.

@cheerfulstoic
Copy link
Contributor Author

It's a funny one because I don't think it would defy expectations if you know what CREATES UNIQUE is doing. Though if we don't, what are the chances that somebody else will?

Regarding backporting, it's tempting, but I'd also like to have reasons for people to want to update to 6.0 ;)

@subvertallchris
Copy link
Contributor

Yeah, for sure. It makes sense once you know about it...

Since there's so much room for confusion, I think should make the syntax more explicit so you can't accidentally end up with unwanted behavior. In 6.0, maybe we require an argument of nil or :none, :all, on: [keys], or except: [keys]. nil would give the behavior we expected now, the others would do as expected?

I'm going to do a quick spike of this -- already have nil behavior working -- and we can tighten up the options tonight and tomorrow.

@subvertallchris
Copy link
Contributor

That PR is a proposed implementation of the comment above.

@subvertallchris
Copy link
Contributor

Ooh, we can close this.

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

No branches or pull requests

2 participants