Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Brandon Dimcheff December 11th, 2010 @ 06:57 AM

  I guess lighthouse won't let me upload a patch, so here it is

• 

  Piotr Sarnacki December 11th, 2010 @ 01:30 PM

  • Milestone set to “2.x”
  • State changed from “new” to “open”
  • Tag set to “bug activerecord find_or_create_by”
  • Importance changed from “” to “Low”
• 

  Ryan Bigg December 11th, 2010 @ 11:53 PM

  This is not supported by Rails at the moment. Please try my gem find_by_hash: http://github.com/radar/find_by_hash to get this functionality.

• You flagged this item as spam.
  

  Brandon Dimcheff December 12th, 2010 @ 03:34 AM

  @ryan - This as worked for me in rails 2 for a while. In the docs it shows that you can do this with an ActiveRecord class directly:

  Tag.find_or_create_by_name(:name => "rails", :creator => current_user)
  

  Now I realize that I'm doing this with an association instead of directly on the class, but why should the API be any different?

• You flagged this item as spam.
  

  Brandon Dimcheff December 12th, 2010 @ 03:36 AM

  I just edited the ticket to fix an error - find_or_create works properly on a model class, as far as I can tell. It does not work properly when called on an association.

• You flagged this item as spam.
  

  James Badger December 15th, 2010 @ 10:25 PM

  I have also experienced this bug, and I am working on a patch.

  I tried to use your test Brandon, but I was unable to properly set the 'type' attribute for the Comment model, it would always evaluate to nil. This error is also present in test_find_or_create_by_with_additional_parameters (from #1108), which does not check to see if the 'type' was actually set (so no error or failure is raised). As 'type' is reserved for STI in this context, and 'test' is not a valid subclass for Comment.

  For that reason, I rewrote them with the Author-Post association instead, which may fit the 'additional attributes setting' part of the test.

• You flagged this item as spam.
  

  Brandon Dimcheff December 15th, 2010 @ 11:07 PM

  ah, ok yeah that makes sense. Since I never got that test passing, I didn't run into that problem. I just used the model objects that were already being used in the other activerecord tests. I think those test the type field, but maybe they do so without reloading the object, so the type is deceptively set correctly...

• You flagged this item as spam.
  

  James Badger December 16th, 2010 @ 02:02 AM

  https://gist.github.com/742909

  Here is my patch, which (as the test case shows) attempts to fix a few scenarios:

  • Finding an existing model using find_or_create_by_title("A fine post")
  • Creating a new model using find_or_create_by_title(:title => "A wood post", :body => "It is made of wood.")
  • Creating a new model using find_or_create_by_title("An iron post", :body => "A heavy post made of iron.")
  • Creating a new model using find_or_create_by_title_and_body("A witness post", "The other post was near here.")

  The one in which I am not sure what type of behaviour would be preferred is the last one, where
  
find_or_create_by_title_and_body("The last post", "If only they were this easy to find.", {:title => "Maybe not the last post", :body => "Another post."})

  will use the hash-defined values over the string values. This could be reversed to prefer the string values by using a custom merge! block in the patch.

• You flagged this item as spam.
  

  Pat George February 9th, 2011 @ 05:57 PM

  1. I just updated an old Rails 2.3.4 project to 2.3.11 (for the security update) only to discover all our "object.association.find_or_create_by_attr1_and_attr2(hash)" calls no longer work.

  I went back to 2.3.5 and tested it each Rails version. It stopped working all together in 2.3.9 throwing an "ArgumentError: Unknown key(s)" error (which I believe is what issue 1108 was about). In 2.3.11 it no longer throws the error but instead assigns the entire hash to the first attribute:

  account = Account.first; params = { :url => "google2311.com", :token => "foo2311", :user_id => account.users.first.id } account.links.find_or_create_by_url_and_token params _.url => {:token=>"foo2311", :user_id=>1, :url=>"google2311.com"}

  If this isn't supported anymore, fine. But know that it's a regression.

• You flagged this item as spam.
  

  Pat George February 9th, 2011 @ 05:59 PM

  Sorry for the bad code block.

  >> account = Account.first; params = { :url => "google2311.com", :token => "foo2311", :user_id => account.users.first.id }
  >> account.links.find_or_create_by_url_and_token params
  >> _.url
  => {:token=>"foo2311", :user_id=>1, :url=>"google2311.com"}
  

• You flagged this item as spam.
  

  Brandon Dimcheff February 10th, 2011 @ 04:38 PM

  @pat, yes that was my experience as well. I stuck a git checkout of rails in vendor/rails and git bisected until I found the commit that caused the problem, which wasn't released until 2.3.9, I think. I haven't tried James' patch yet, but if it works, maybe we can get it accepted...

  This is definitely a regression, and it logically should work, considering it works when it's not using an association proxy.

• You flagged this item as spam.
  

  Brian Finney March 8th, 2011 @ 02:58 AM

  Bump, also running into this issue. This is also blocking for us to update to the latest rails-2.3 from 2.3.8

  @James, That patch is close but misses a few edge cases that are handled by ActiveRecord::Base#find_or_create_by_*.

  Attaching a patch that incorporates James' tests, plus a few more edge cases, and fixes this bug.

  Also as pull request at https://github.com/rails/rails/pull/207

  Thanks
  Brian

  • 0001-fix-6147-find_or_create-vi.patch 4.8 KB

• You flagged this item as spam.
  

  Brian Finney March 11th, 2011 @ 09:34 PM

  Updated patch from pull request

  • 0001-fix-6147-find_or_create-vi.patch 4.8 KB

• You flagged this item as spam.
  

  bingbing March 26th, 2011 @ 01:37 AM

  omega watch

• You flagged this item as spam.
  

  jeff (at snowmoonsoftware) May 4th, 2011 @ 06:16 PM

  Bump. Brian, I applied your patch to rails 2.3.11, and still see a problem with find_or_create_by with multiple parameters:

  p.niod_metrics.find_or_create_by_metric_type_id_and_month(666, Date.parse('2011-06-01'))
  
  NiodMetric Load (0.5ms)   SELECT * FROM 'niod_metrics' WHERE ('niod_metrics'.'month' IS NULL AND 
  'niod_metrics'.'metric_type_id' IN( 666,'2011-06-01' )) AND ('niod_metrics'.niodmetrics_id = 1750 AND 
  'niod_metrics'.niodmetrics_type = 'Profile') LIMIT 1
  
  
  NiodMetric Create (0.2ms)   INSERT INTO 'niod_metrics' ('created_at', 'month', 'updated_at', 'last_activity_at', 
  'niodmetrics_type', 'value', 'metric_type_id', 'niodmetrics_id') VALUES('2011-05-04 17:10:44', '2011-06-01', 
  '2011-05-04 17:10:44', NULL, 'Profile', NULL, 666, 1750)
  

  The insert is correct, but the find isn't. This is blocking a move from rails 2.3.4 to 2.3.11.

• You flagged this item as spam.
  

  Brian Finney May 10th, 2011 @ 12:49 AM

  Hey Jeff

  I believe the issue you're seeing is unrelated to this patch. This bug is specific to using hash parameters to effect the create_by statement and the patch only effects the insert statement. To find the root cause of the select being incorrect, you need to follow the calls through pull_finder_args_from(DynamicFinderMatch.match(method).attribute_names, *args) on line 383 of activerecord/lib/active_record/associations/association_collection.rb.

  Thanks