From 9f7ff621bd62ade37b4ae4e608bdf24b7f7b1456 Mon Sep 17 00:00:00 2001 From: Casey Dreier Date: Wed, 27 Apr 2011 21:57:24 -0400 Subject: [PATCH] Fixing dynamic finders on associations to properly send arguments to the find_by_* method. Closes issue #330. Commit fdfc8e3b9 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. --- .../active_record/associations/association_collection.rb | 2 +- .../cases/associations/has_many_associations_test.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 3a602e49c4d97..c04110a994b6a 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/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) diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 3996b84705df2..2f715a4631cf6 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/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' }