From 14a16844bbb3ba9edb14269ce2d0b61c9a43637e Mon Sep 17 00:00:00 2001 From: David Dollar Date: Mon, 14 Jul 2008 11:57:05 -0400 Subject: [PATCH] Adds the ability to update existing entries on :accessible => true associations --- activerecord/lib/active_record/associations.rb | 12 +++++- .../associations/association_collection.rb | 44 ++++++++++++++++++- activerecord/test/cases/associations_test.rb | 35 ++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index d43e07a..5075054 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1113,7 +1113,17 @@ module ActiveRecord association = association_proxy_class.new(self, reflection) end - new_value = reflection.klass.new(new_value) if reflection.options[:accessible] && new_value.is_a?(Hash) + if reflection.options[:accessible] && new_value.is_a?(Hash) + if association.id.nil? + new_value = reflection.klass.new(new_value) + else + new_value.each do |k, v| + association.send("#{k}=", v) + end + association.save! + new_value = association + end + end if association_proxy_class == HasOneThroughAssociation association.create_through_record(new_value) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index a28be9e..cbd1ac5 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -97,7 +97,21 @@ module ActiveRecord @owner.transaction do flatten_deeper(records).each do |record| - record = @reflection.klass.new(record) if @reflection.options[:accessible] && record.is_a?(Hash) + + if @reflection.options[:accessible] && record.is_a?(Hash) + if record.has_key?(:id) + association = @owner.send(@reflection.name).find(record[:id]) + record.each do |k, v| + association.send("#{k}=", v) + end + update_record_for_target_with_callbacks(association) do |r| + r.save! + end + next + else + record = @reflection.klass.new(record) + end + end raise_on_type_mismatch(record) add_record_to_target_with_callbacks(record) do |r| @@ -231,8 +245,20 @@ module ActiveRecord # Replace this collection with +other_array+ # This will perform a diff and delete/add only records that have changed. def replace(other_array) - other_array.map! do |val| - val.is_a?(Hash) ? @reflection.klass.new(val) : val + other_array.map! do |record| + if record.is_a?(Hash) + if record.has_key?(:id) + association = @owner.send(@reflection.name).find(record[:id]) + record.each do |k, v| + association.send("#{k}=", v) + end + association.save! + record = association + else + record = @reflection.klass.new(record) + end + end + record end if @reflection.options[:accessible] other_array.each { |val| raise_on_type_mismatch(val) } @@ -349,6 +375,18 @@ module ActiveRecord record end + def update_record_for_target_with_callbacks(record) + callback(:before_update, record) + yield(record) if block_given? + @target ||= [] unless loaded? + require 'pp' + @target.map! do |target_record| + target_record.id == record.id ? record : target_record + end + callback(:after_update, record) + record + end + def callback(method, record) callbacks_for(method).each do |callback| ActiveSupport::Callbacks::Callback.new(method, callback, record).call(@owner, record) diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 4904fee..cecc209 100755 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -190,6 +190,7 @@ class AssociationProxyTest < ActiveRecord::TestCase end def test_belongs_to_mass_assignment + post = posts(:welcome) post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' } author_attributes = { :name => 'David Dollar' } @@ -203,9 +204,17 @@ class AssociationProxyTest < ActiveRecord::TestCase post = Post.create(post_attributes.merge({:creatable_author => author_attributes})) assert_equal post.creatable_author.name, author_attributes[:name] end + + assert_no_difference 'Author.count' do + author_id = post.creatable_author.id + author_attributes_update = { :id => author_id, :name => 'New Name' } + post.creatable_author = author_attributes_update + assert_equal post.creatable_author.name, author_attributes_update[:name] + end end def test_has_one_mass_assignment + post = posts(:welcome) post_attributes = { :title => 'Associations', :body => 'Are They Accessible?' } comment_attributes = { :body => 'Setter Takes Hash' } @@ -219,6 +228,14 @@ class AssociationProxyTest < ActiveRecord::TestCase post = Post.create(post_attributes.merge({:creatable_comment => comment_attributes})) assert_equal post.creatable_comment.body, comment_attributes[:body] end + + assert_no_difference 'Comment.count' do + comment_id = post.creatable_comment.id + comment_attributes_update = { :id => comment_id, :body => 'New Body' } + post.creatable_comment.destroy + post.creatable_comment = comment_attributes_update + assert_equal post.creatable_comment.body, comment_attributes_update[:body] + end end def test_has_many_mass_assignment @@ -247,6 +264,15 @@ class AssociationProxyTest < ActiveRecord::TestCase post.creatable_comments = [comment_attributes, comment_attributes] assert_equal post.creatable_comments.count, 2 + + assert_no_difference 'Comment.count' do + comments = post.creatable_comments + updated_comment = comments.pop + comment_attributes_update = { :id => updated_comment.id, :body => 'Newer Body' } + comments << comment_attributes_update + post.creatable_comments = comments + assert_equal post.creatable_comments.find(updated_comment.id).body, comment_attributes_update[:body] + end end def test_has_and_belongs_to_many_mass_assignment @@ -275,6 +301,15 @@ class AssociationProxyTest < ActiveRecord::TestCase post.creatable_categories = [category_attributes, category_attributes] assert_equal post.creatable_categories.count, 2 + + assert_no_difference 'Category.count' do + categories = post.creatable_categories + updated_category = categories.pop + category_attributes_update = { :id => updated_category.id, :name => 'New Name' } + categories << category_attributes_update + post.creatable_categories = categories + assert_equal post.creatable_categories.find(updated_category.id).name, category_attributes_update[:name] + end end def test_association_proxy_setter_can_take_hash -- 1.5.5.1