Comments and changes to this ticket

• 

  Ryan Bigg April 25th, 2010 @ 12:23 AM

  Rather than using "foo", "bar" and "baz", could you use real terms so we can think how you're thinking? if foo is indeed a foreign_key then you should be using the association builders to assign it, not find_or_create.

• 

  Ryan Bigg April 26th, 2010 @ 09:16 AM

  • Assigned user set to “Pratik”

• You flagged this item as spam.
  

  Marc-André Lafortune April 26th, 2010 @ 06:14 PM

  I am not sure what you mean by "should be using the association builders", so maybe I am missing something.

  To be a bit more convincing that this is a bug, let me add:

  0)
  

  If Tag#name is protected:

  Tag.find_or_create_by_name("Summer") # assigns name
  Tag.find_or_create_by_name(:name =>"Summer") # doesn't assign name

  I believe both forms should be equivalent, especially that there is no way to:
  Tag.find_or_create_by_name("Summer", :extra_info => "blah")

  That later form would be nice, BTW, but I guess I'll concentrate on arguing about this current bug.

  An examples with an association I can think of:

  Summary.belongs_to :original_content

  summary = Summary.find_or_create_by_original_content_id(:original_content_id => 42, &:summarize)

  Moreover, I can't think of an example where it would not be desirable or prudent to assign 'foo'.

  Thanks

• You flagged this item as spam.
  

  Marc-André Lafortune April 26th, 2010 @ 06:22 PM

  Actually, is it only me, or the example from the doc will not work (protected fields or not)?

  User.find_or_create_by_name('Bob', :age => 40) # this won't set the age in Rails 2.3.5

• You flagged this item as spam.
  

  Tanel Suurhans April 30th, 2010 @ 10:04 PM

  Tested this on 2-3-stable.
  

  author = Author.find_or_create_by_name(:name => "bm", :author_address_id => 1)
  author.inspect # => #<Author id: 10, :name => "bm", :author_address_id => 1, :author_address_extra_id => nil>
  

  Also tested on 2.3.5 on one of my applications and indeed it does not seem to work.
  

  page = Page.find_or_create_by_name("my page", :content => "ponies here", :state => Page::STATE_HIDDEN)
  page.inspect # => #<Page id: 1, :state => "PUBLIC", :name => "my page", :content => nil>
  

  I see test cases covering additional values, as well as hash values for find_or_create_by in 2-3-stable, so i would assume its all working there.

  As for your second set of examples - in my opinion this behavior is expected. The attr_protected protects attributes against mass(hash) assignment.
  In your first example, its an explicit assignment, in your second one its basically a mass-assignment. So it's consistent with the rest of the API.

  Piece of the documentation from ActiveRecord::Base
  

  class Customer < ActiveRecord::Base
    attr_protected :credit_rating
  end
  customer = Customer.new("name" => David, "credit_rating" => "Excellent")
  customer.credit_rating # => nil
  customer.attributes = { "description" => "Jolly fellow", "credit_rating" => "Superb" }
  customer.credit_rating # => nil
  customer.credit_rating = "Average"
  customer.credit_rating # => "Average"
  

• You flagged this item as spam.
  

  Marc-André Lafortune May 4th, 2010 @ 07:03 PM

  I'm fine with the arugment protection, as long as the other form find_or_create_by_name("Marc-André", :whatever => "value") is fixed.

• 

  Jeremy Kemper May 7th, 2010 @ 01:22 AM

  • State changed from “new” to “open”
  • Assigned user changed from “Pratik” to “Santiago Pastorino”
• 

  Santiago Pastorino May 7th, 2010 @ 04:40 AM

  • Tag set to “activerecord, find_or_create, patch”

  Patch for master tomorrow i will backport it for 2-3-stable

  • find_or_instantiator_by_attributes-master.patch 3.8 KB
• 

  Repository May 7th, 2010 @ 05:52 PM

  • State changed from “open” to “committed”

  (from [9aaef5935660ba13531512fb7def4b8bdf14511d]) Make find_or_create and find_or_initialize work mixing explicit parameters and a hash [#4457 state:committed]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/9aaef5935660ba13531512fb7def4b...

• 

  Santiago Pastorino May 7th, 2010 @ 06:22 PM

  • Milestone set to “2.3.6”
  • State changed from “committed” to “open”

  I'm going to bakport to 2-3-stable

• 

  Santiago Pastorino May 7th, 2010 @ 09:01 PM

  2-3-stable backport

  • find_or_initialize-2-3-stable.patch 4.3 KB
• 

  Santiago Pastorino May 7th, 2010 @ 09:05 PM

  Sorry i think Marc-André Lafortune deserves a mention on the patch, i forgot it twice.
  Here is the 2-3-stable patch with the mention

  • find_or_initialize-2-3-stable.patch 4.3 KB
• 

  Santiago Pastorino May 7th, 2010 @ 09:07 PM

  WTF i send the same thing again, sorry.
  This is the right one.
  Thanks Marc-André.

  • find_or_initialize-2-3-stable.patch 4.5 KB
• 

  Repository May 7th, 2010 @ 09:32 PM

  • State changed from “open” to “committed”

  (from [f967b352d2abdb88b704266cdb06b53f7d8ab401]) Make find_or_create and find_or_initialize work mixing explicit parameters and a hash. ht: Marc-André Lafortune

  [#4457 state:committed]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/f967b352d2abdb88b704266cdb06b5...

• You flagged this item as spam.
  

  Marc-André Lafortune May 8th, 2010 @ 05:20 AM

  Great! Thanks Santiago.

  I actually felt bad not taking the time to write a patch for this...

• 

  Santiago Pastorino May 8th, 2010 @ 09:11 PM

  Thanks to you Marc-André, and give you some time to try the next issue ;).