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

find_or_create_by attributes not working #61

Closed
juddblair opened this issue Jun 5, 2013 · 4 comments
Closed

find_or_create_by attributes not working #61

juddblair opened this issue Jun 5, 2013 · 4 comments

Comments

@juddblair
Copy link
Contributor

Me again, with another weird edge case issue:

In my app, our hierarchical tags are scoped to a particular client and we indicate this with a 'client_name' field in our TagDefinition object.

I'm calling find_or_create_by_path thusly

TagDefinition.find_or_create_by_path(hierarchies, {'client_name' => client_name, 'context' => TagDefinition::HIERARCHY_CONTEXT})

where 'hierarchies' is an array of tag_definition names. The problem is this - every single one of our clients has a root tag called "All". When I run the above snippet for a hierarchies value of ['All','North','South'] for a client_name of 'aperture_science', closure tree happily finds the root tag 'All' - for another client, 'acme', and then creates all the other tags with the incorrect tag as the root.

The problem is this:

In both the class and instance methods of find_or_create_by_path, you accept a hash of attributes that get passed to active record. This is all groovy, except that the hash of attributes is only honored in the insert/update, NOT in the where clause.

I updated my fork https://github.com/juddblair/closure_tree with a quick and dirty fix (merged in the attributes hash before the where clause) and ran it against our app, seemed to fix the problem.

@mceachen
Copy link
Collaborator

mceachen commented Jun 6, 2013

So your "name" column isn't unique?

I'm hesitant to use a conditions hash, because multiple rows may match
those conditions. In that case, do we pick the first entry? What
constitutes "first"?

Seems like for your case, unless I'm misunderstanding (which is likely),
this should work:

root = TagDefinition.roots.
  where(:client_name => 'aperture_science').
  find_or_create_by_path(...)

Matthew
On Jun 5, 2013 1:09 PM, "Judd Blair" notifications@github.com wrote:

Me again, with another weird edge case issue:

In my app, our hierarchical tags are scoped to a particular client and we
indicate this with a 'client_name' field in our TagDefinition object.

I'm calling find_or_create_by_path thusly

TagDefinition.find_or_create_by_path(hierarchies, {'client_name' =>
client_name, 'context' => TagDefinition::HIERARCHY_CONTEXT})

where 'hierarchies' is an array of tag_definition names. The problem is
this - every single one of our clients has a root tag called "All". When I
run the above snippet for a hierarchies value of ['All','North','South']
for a client_name of 'aperture_science', closure tree happily finds the
root tag 'All' - for another client, 'acme', and then creates all the other
tags with the incorrect tag as the root.

The problem is this:

In both the class and instance methods of find_or_create_by_path, you
accept a hash of attributes that get passed to active record. This is all
groovy, except that the hash of attributes is only honored in the
insert/update, NOT in the where clause.

I updated my fork https://github.com/juddblair/closure_tree with a quick
and dirty fix (merged in the attributes hash before the where clause) and
ran it against our app, seemed to fix the problem.


Reply to this email directly or view it on GitHubhttps://github.com//issues/61
.

@juddblair
Copy link
Contributor Author

Nope, name is not unique.

It's actually not even unique to a client, since you could potentially have a hierarchy that goes 'All'->'Northeast'->'NYC'->'North' and another that goes 'All'->'Canada'->'Northeast'->'Halifax'->'North'...you get the idea. To make matters more confusing each client has multiple roots: TagDefinitions include both the hierarchical case I've described (a regional hierarchy of store locations, essentially), but can also include flat 'filter' tags such as brand, store type, etc. that, being flat, come up as roots.

You're right though - I can pop the first element off the list for the name of the root node and find_or_create_by given the name, client name, context, and level (the root node isn't guaranteed to be there If I'm on-boarding a new client), and then find_or_create_by_path the remainder of the list from the root node. Thanks a lot for that.

That said, I suppose the confusing part for me is that the create honors the conditions hash, but the find doesn't, which I didn't expect - in the example in the docs, you have:

child = Tag.find_or_create_by_path(%w{home chuck Photos"}, {:tag_type => "File"})

If I'm not mistaken, let's assume you have another tag_type of "Picture", which also has a "home" tag - depending on how your DB orders it, child = Tag.find_or_create_by_path(%w{home larry pics pic.jpg"}, {:tag_type => "Picture"}) could find the 'home' tag with the tag_type of "File" and create the rest of the children for that incorrect root tag - wouldn't adding the conditions hash to the select clause alleviate this problem?

@mceachen
Copy link
Collaborator

mceachen commented Jun 9, 2013

You convinced me—thanks for your help. https://travis-ci.org/mceachen/closure_tree/builds/7928772 is building 2760c03

@mceachen
Copy link
Collaborator

This will drop in v4.2.0, being released shortly.

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