Skip to content

Loading…

Dynamic find_or_create_by_x_and_y always creates new records in Rails 2.3.11 #331

Closed
wants to merge 1 commit into from

2 participants

@daphonz

This is a regression that was introduced in fdfc8e3.

Any create_or_find_by_x_and_y call on an association will aways create a new record.

To recreate:

Say you have two models, one has_many of another:

class Author < ActiveRecord::Base
  has_many :posts
end

class Post < ActiveRecord::Base
  belongs_to :author
end

Jump into the console and enable AR logging. Then run the dynamic find_or_create_by twice:

Author.first.posts.find_or_create_by_title_and_published('Post Title', true)
Author.first.posts.find_or_create_by_title_and_published('Post Title', true)

You'll see that the query looks generates something like

select * from posts where title IN ('Post Title', 1) and published = '' and author_id = 7

i.e. the title is getting passed an array of the input.

Failing test and patch included below in the merge request (9f7ff62).

@daphonz daphonz Fixing dynamic finders on associations to properly send arguments to …
…the find_by_* method. Closes issue #330.

Commit fdfc8e3 introduced a bugfix to prevent additional values passed
to a dynamic find_or_create_by_x methods from confusing the finder.
This patch also broke the essential behavior of this method on an
association by incorrectly sending arguments to the find_by_x methods.
The finder method would always see its inputs as a single array of
values instead of individual arguments, almost guaranteeing that the
finder call would be incorrect, and that we'd always create a new
record instead.

This patch adds a splat operator to the parameter array we send along to
the dynamic finder so that it receives its inputs correctly, and
includes an additional test to ensure that repeated calls to
find_or_create_by_x only creates one new record.
9f7ff62
@josevalim
Ruby on Rails member

Merged.

@josevalim josevalim closed this
@cch1 cch1 pushed a commit to cch1/rails that referenced this pull request
@josevalim josevalim Merged pull request #331 from daphonz/2-3-stable.
Dynamic find_or_create_by_x_and_y always creates new records in Rails 2.3.11
f424efe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 28, 2011
  1. @daphonz

    Fixing dynamic finders on associations to properly send arguments to …

    daphonz committed
    …the find_by_* method. Closes issue #330.
    
    Commit fdfc8e3 introduced a bugfix to prevent additional values passed
    to a dynamic find_or_create_by_x methods from confusing the finder.
    This patch also broke the essential behavior of this method on an
    association by incorrectly sending arguments to the find_by_x methods.
    The finder method would always see its inputs as a single array of
    values instead of individual arguments, almost guaranteeing that the
    finder call would be incorrect, and that we'd always create a new
    record instead.
    
    This patch adds a splat operator to the parameter array we send along to
    the dynamic finder so that it receives its inputs correctly, and
    includes an additional test to ensure that repeated calls to
    find_or_create_by_x only creates one new record.
View
2 activerecord/lib/active_record/associations/association_collection.rb
@@ -381,7 +381,7 @@ def method_missing(method, *args, &block)
when /^find_or_create_by_(.*)$/
rest = $1
find_args = pull_finder_args_from(DynamicFinderMatch.match(method).attribute_names, *args)
- return send("find_by_#{rest}", find_args) ||
+ return send("find_by_#{rest}", *find_args) ||
method_missing("create_by_#{rest}", *args, &block)
when /^create_by_(.*)$/
return create($1.split('_and_').zip(args).inject({}) { |h,kv| k,v=kv ; h[k] = v ; h }, &block)
View
9 activerecord/test/cases/associations/has_many_associations_test.rb
@@ -82,6 +82,15 @@ def test_find_or_create_by_with_additional_parameters
assert_equal 4, post.comments.length
end
+ def test_find_or_create_by_with_same_parameters_creates_a_single_record
+ author = Author.first
+ assert_difference "Post.count", +1 do
+ 2.times do
+ author.posts.find_or_create_by_body_and_title('one', 'two')
+ end
+ end
+ end
+
def test_find_or_create_by_with_block
post = Post.create! :title => 'test_find_or_create_by_with_additional_parameters', :body => 'this is the body'
comment = post.comments.find_or_create_by_body('other test comment body') { |comment| comment.type = 'test' }
Something went wrong with that request. Please try again.