diff options
author | Lance Ivy <lance@cainlevy.net> | 2009-02-08 14:23:35 +0100 |
---|---|---|
committer | Michael Koziarski <michael@koziarski.com> | 2009-02-13 21:47:56 +1300 |
commit | 5dbc9d40a49f5f0f50c2f3ebe6dda942f0e61562 (patch) | |
tree | 90f4a5d54190ada928712a285c56bcfe6da334fc | |
parent | a6508527570cd3f7225a7030218447bcc5824224 (diff) | |
download | rails-5dbc9d40a49f5f0f50c2f3ebe6dda942f0e61562.tar.gz rails-5dbc9d40a49f5f0f50c2f3ebe6dda942f0e61562.tar.bz2 rails-5dbc9d40a49f5f0f50c2f3ebe6dda942f0e61562.zip |
Changed API of NestedAttributes to take an array, or hash with index keys, of hashes that have the id on the inside of the attributes hash and updated the FormBuilder to produce such hashes. Also fixed NestedAttributes with composite ids.
Signed-off-by: Michael Koziarski <michael@koziarski.com>
Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
[#1892 state:committed]
-rw-r--r-- | actionpack/lib/action_view/helpers/form_helper.rb | 26 | ||||
-rw-r--r-- | actionpack/test/template/form_helper_test.rb | 38 | ||||
-rw-r--r-- | activerecord/lib/active_record/nested_attributes.rb | 246 | ||||
-rw-r--r-- | activerecord/test/cases/nested_attributes_test.rb | 288 | ||||
-rw-r--r-- | activerecord/test/models/pirate.rb | 2 | ||||
-rw-r--r-- | activerecord/test/models/ship.rb | 4 |
6 files changed, 409 insertions, 195 deletions
diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 3925978217..568687e9e0 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -964,24 +964,34 @@ module ActionView def fields_for_with_nested_attributes(association_name, args, block) name = "#{object_name}[#{association_name}_attributes]" association = @object.send(association_name) + explicit_object = args.first if args.first.respond_to?(:new_record?) if association.is_a?(Array) - children = args.first.respond_to?(:new_record?) ? [args.first] : association + children = explicit_object ? [explicit_object] : association + explicit_child_index = args.last[:child_index] if args.last.is_a?(Hash) children.map do |child| - child_name = "#{name}[#{ child.new_record? ? new_child_id : child.id }]" - @template.fields_for(child_name, child, *args, &block) + fields_for_nested_model("#{name}[#{explicit_child_index || nested_child_index}]", child, args, block) end.join else - object = args.first.respond_to?(:new_record?) ? args.first : association + fields_for_nested_model(name, explicit_object || association, args, block) + end + end + + def fields_for_nested_model(name, object, args, block) + if object.new_record? @template.fields_for(name, object, *args, &block) + else + @template.fields_for(name, object, *args) do |builder| + @template.concat builder.hidden_field(:id) + block.call(builder) + end end end - def new_child_id - value = (@child_counter ||= 1) - @child_counter += 1 - "new_#{value}" + def nested_child_index + @nested_child_index ||= -1 + @nested_child_index += 1 end end end diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index b7e4a933e1..5cc81b4afb 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -607,6 +607,7 @@ class FormHelperTest < ActionView::TestCase expected = '<form action="http://www.example.com" method="post">' + '<input name="post[title]" size="30" type="text" id="post_title" value="Hello World" />' + + '<input id="post_author_attributes_id" name="post[author_attributes][id]" type="hidden" value="321" />' + '<input id="post_author_attributes_name" name="post[author_attributes][name]" size="30" type="text" value="author #321" />' + '</form>' @@ -627,8 +628,10 @@ class FormHelperTest < ActionView::TestCase expected = '<form action="http://www.example.com" method="post">' + '<input name="post[title]" size="30" type="text" id="post_title" value="Hello World" />' + - '<input id="post_comments_attributes_1_name" name="post[comments_attributes][1][name]" size="30" type="text" value="comment #1" />' + - '<input id="post_comments_attributes_2_name" name="post[comments_attributes][2][name]" size="30" type="text" value="comment #2" />' + + '<input id="post_comments_attributes_0_id" name="post[comments_attributes][0][id]" type="hidden" value="1" />' + + '<input id="post_comments_attributes_0_name" name="post[comments_attributes][0][name]" size="30" type="text" value="comment #1" />' + + '<input id="post_comments_attributes_1_id" name="post[comments_attributes][1][id]" type="hidden" value="2" />' + + '<input id="post_comments_attributes_1_name" name="post[comments_attributes][1][name]" size="30" type="text" value="comment #2" />' + '</form>' assert_dom_equal expected, output_buffer @@ -648,8 +651,8 @@ class FormHelperTest < ActionView::TestCase expected = '<form action="http://www.example.com" method="post">' + '<input name="post[title]" size="30" type="text" id="post_title" value="Hello World" />' + - '<input id="post_comments_attributes_new_1_name" name="post[comments_attributes][new_1][name]" size="30" type="text" value="new comment" />' + - '<input id="post_comments_attributes_new_2_name" name="post[comments_attributes][new_2][name]" size="30" type="text" value="new comment" />' + + '<input id="post_comments_attributes_0_name" name="post[comments_attributes][0][name]" size="30" type="text" value="new comment" />' + + '<input id="post_comments_attributes_1_name" name="post[comments_attributes][1][name]" size="30" type="text" value="new comment" />' + '</form>' assert_dom_equal expected, output_buffer @@ -669,8 +672,9 @@ class FormHelperTest < ActionView::TestCase expected = '<form action="http://www.example.com" method="post">' + '<input name="post[title]" size="30" type="text" id="post_title" value="Hello World" />' + - '<input id="post_comments_attributes_321_name" name="post[comments_attributes][321][name]" size="30" type="text" value="comment #321" />' + - '<input id="post_comments_attributes_new_1_name" name="post[comments_attributes][new_1][name]" size="30" type="text" value="new comment" />' + + '<input id="post_comments_attributes_0_id" name="post[comments_attributes][0][id]" type="hidden" value="321" />' + + '<input id="post_comments_attributes_0_name" name="post[comments_attributes][0][name]" size="30" type="text" value="comment #321" />' + + '<input id="post_comments_attributes_1_name" name="post[comments_attributes][1][name]" size="30" type="text" value="new comment" />' + '</form>' assert_dom_equal expected, output_buffer @@ -690,14 +694,32 @@ class FormHelperTest < ActionView::TestCase expected = '<form action="http://www.example.com" method="post">' + '<input name="post[title]" size="30" type="text" id="post_title" value="Hello World" />' + - '<input id="post_comments_attributes_321_name" name="post[comments_attributes][321][name]" size="30" type="text" value="comment #321" />' + - '<input id="post_comments_attributes_new_1_name" name="post[comments_attributes][new_1][name]" size="30" type="text" value="new comment" />' + + '<input id="post_comments_attributes_0_id" name="post[comments_attributes][0][id]" type="hidden" value="321" />' + + '<input id="post_comments_attributes_0_name" name="post[comments_attributes][0][name]" size="30" type="text" value="comment #321" />' + + '<input id="post_comments_attributes_1_name" name="post[comments_attributes][1][name]" size="30" type="text" value="new comment" />' + '</form>' assert_dom_equal expected, output_buffer assert_equal yielded_comments, @post.comments end + def test_nested_fields_for_with_child_index_option_override_on_a_nested_attributes_collection_association + @post.comments = [] + + form_for(:post, @post) do |f| + f.fields_for(:comments, Comment.new(321), :child_index => 'abc') do |cf| + concat cf.text_field(:name) + end + end + + expected = '<form action="http://www.example.com" method="post">' + + '<input id="post_comments_attributes_abc_id" name="post[comments_attributes][abc][id]" type="hidden" value="321" />' + + '<input id="post_comments_attributes_abc_name" name="post[comments_attributes][abc][name]" size="30" type="text" value="comment #321" />' + + '</form>' + + assert_dom_equal expected, output_buffer + end + def test_fields_for fields_for(:post, @post) do |f| concat f.text_field(:title) diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 5778223c74..e3122d195a 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -41,15 +41,16 @@ module ActiveRecord # Enabling nested attributes on a one-to-one association allows you to # create the member and avatar in one go: # - # params = { 'member' => { 'name' => 'Jack', 'avatar_attributes' => { 'icon' => 'smiling' } } } + # params = { :member => { :name => 'Jack', :avatar_attributes => { :icon => 'smiling' } } } # member = Member.create(params) - # member.avatar.icon #=> 'smiling' + # member.avatar.id # => 2 + # member.avatar.icon # => 'smiling' # # It also allows you to update the avatar through the member: # - # params = { 'member' => { 'avatar_attributes' => { 'icon' => 'sad' } } } + # params = { :member' => { :avatar_attributes => { :id => '2', :icon => 'sad' } } } # member.update_attributes params['member'] - # member.avatar.icon #=> 'sad' + # member.avatar.icon # => 'sad' # # By default you will only be able to set and update attributes on the # associated model. If you want to destroy the associated model through the @@ -64,7 +65,7 @@ module ActiveRecord # Now, when you add the <tt>_delete</tt> key to the attributes hash, with a # value that evaluates to +true+, you will destroy the associated model: # - # member.avatar_attributes = { '_delete' => '1' } + # member.avatar_attributes = { :id => '2', :_delete => '1' } # member.avatar.marked_for_destruction? # => true # member.save # member.avatar #=> nil @@ -77,59 +78,80 @@ module ActiveRecord # # class Member < ActiveRecord::Base # has_many :posts - # accepts_nested_attributes_for :posts, :reject_if => proc { |attributes| attributes['title'].blank? } + # accepts_nested_attributes_for :posts # end # # You can now set or update attributes on an associated post model through # the attribute hash. # - # For each key in the hash that starts with the string 'new' a new model - # will be instantiated. When the proc given with the <tt>:reject_if</tt> - # option evaluates to +false+ for a certain attribute hash no record will - # be built for that hash. (Rejecting new records can alternatively be done - # by utilizing the <tt>'_delete'</tt> key. Scroll down for more info.) - # - # params = { 'member' => { - # 'name' => 'joe', 'posts_attributes' => { - # 'new_12345' => { 'title' => 'Kari, the awesome Ruby documentation browser!' }, - # 'new_54321' => { 'title' => 'The egalitarian assumption of the modern citizen' }, - # 'new_67890' => { 'title' => '' } # This one matches the :reject_if proc and will not be instantiated. - # } + # For each hash that does _not_ have an <tt>id</tt> key a new record will + # be instantiated, unless the hash also contains a <tt>_delete</tt> key + # that evaluates to +true+. + # + # params = { :member => { + # :name => 'joe', :posts_attributes => [ + # { :title => 'Kari, the awesome Ruby documentation browser!' }, + # { :title => 'The egalitarian assumption of the modern citizen' }, + # { :title => '', :_delete => '1' } # this will be ignored + # ] # }} # # member = Member.create(params['member']) - # member.posts.length #=> 2 - # member.posts.first.title #=> 'Kari, the awesome Ruby documentation browser!' - # member.posts.second.title #=> 'The egalitarian assumption of the modern citizen' + # member.posts.length # => 2 + # member.posts.first.title # => 'Kari, the awesome Ruby documentation browser!' + # member.posts.second.title # => 'The egalitarian assumption of the modern citizen' + # + # You may also set a :reject_if proc to silently ignore any new record + # hashes if they fail to pass your criteria. For example, the previous + # example could be rewritten as: + # + # class Member < ActiveRecord::Base + # has_many :posts + # accepts_nested_attributes_for :posts, :reject_if => proc { |attributes| attributes['title'].blank? } + # end + # + # params = { :member => { + # :name => 'joe', :posts_attributes => [ + # { :title => 'Kari, the awesome Ruby documentation browser!' }, + # { :title => 'The egalitarian assumption of the modern citizen' }, + # { :title => '' } # this will be ignored because of the :reject_if proc + # ] + # }} + # + # member = Member.create(params['member']) + # member.posts.length # => 2 + # member.posts.first.title # => 'Kari, the awesome Ruby documentation browser!' + # member.posts.second.title # => 'The egalitarian assumption of the modern citizen' # - # When the key for post attributes is an integer, the associated post with - # that ID will be updated: + # If the hash contains an <tt>id</tt> key that matches an already + # associated record, the matching record will be modified: # # member.attributes = { - # 'name' => 'Joe', - # 'posts_attributes' => { - # '1' => { 'title' => '[UPDATED] An, as of yet, undisclosed awesome Ruby documentation browser!' }, - # '2' => { 'title' => '[UPDATED] other post' } - # } + # :name => 'Joe', + # :posts_attributes => [ + # { :id => 1, :title => '[UPDATED] An, as of yet, undisclosed awesome Ruby documentation browser!' }, + # { :id => 2, :title => '[UPDATED] other post' } + # ] # } # - # By default the associated models are protected from being destroyed. If - # you want to destroy any of the associated models through the attributes - # hash, you have to enable it first using the <tt>:allow_destroy</tt> - # option. + # member.posts.first.title # => '[UPDATED] An, as of yet, undisclosed awesome Ruby documentation browser!' + # member.posts.second.title # => '[UPDATED] other post' # - # This will allow you to specify which models to destroy in the attributes - # hash by setting the '_delete' attribute to a value that evaluates to - # +true+: + # By default the associated records are protected from being destroyed. If + # you want to destroy any of the associated records through the attributes + # hash, you have to enable it first using the <tt>:allow_destroy</tt> + # option. This will allow you to also use the <tt>_delete</tt> key to + # destroy existing records: # # class Member < ActiveRecord::Base # has_many :posts # accepts_nested_attributes_for :posts, :allow_destroy => true # end # - # params = {'member' => { 'name' => 'joe', 'posts_attributes' => { - # '2' => { '_delete' => '1' } - # }}} + # params = { :member => { + # :posts_attributes => [{ :id => '2', :_delete => '1' }] + # }} + # # member.attributes = params['member'] # member.posts.detect { |p| p.id == 2 }.marked_for_destruction? # => true # member.posts.length #=> 2 @@ -143,24 +165,27 @@ module ActiveRecord # the parent model is saved. This happens inside the transaction initiated # by the parents save method. See ActiveRecord::AutosaveAssociation. module ClassMethods - # Defines an attributes writer for the specified association(s). + # Defines an attributes writer for the specified association(s). If you + # are using <tt>attr_protected</tt> or <tt>attr_accessible</tt>, then you + # will need to add the attribute writer to the allowed list. # # Supported options: # [:allow_destroy] # If true, destroys any members from the attributes hash with a - # <tt>_delete</tt> key and a value that converts to +true+ + # <tt>_delete</tt> key and a value that evaluates to +true+ # (eg. 1, '1', true, or 'true'). This option is off by default. # [:reject_if] # Allows you to specify a Proc that checks whether a record should be # built for a certain attribute hash. The hash is passed to the Proc # and the Proc should return either +true+ or +false+. When no Proc - # is specified a record will be built for all attribute hashes. + # is specified a record will be built for all attribute hashes that + # do not have a <tt>_delete</tt> that evaluates to true. # # Examples: - # accepts_nested_attributes_for :avatar - # accepts_nested_attributes_for :avatar, :allow_destroy => true - # accepts_nested_attributes_for :avatar, :reject_if => proc { ... } - # accepts_nested_attributes_for :avatar, :posts, :allow_destroy => true, :reject_if => proc { ... } + # # creates avatar_attributes= + # accepts_nested_attributes_for :avatar, :reject_if => proc { |attributes| attributes['name'].blank? } + # # creates avatar_attributes= and posts_attributes= + # accepts_nested_attributes_for :avatar, :posts, :allow_destroy => true def accepts_nested_attributes_for(*attr_names) options = { :allow_destroy => false } options.update(attr_names.extract_options!) @@ -193,9 +218,9 @@ module ActiveRecord end end - # Returns ActiveRecord::AutosaveAssociation::marked_for_destruction? - # It's used in conjunction with fields_for to build a form element - # for the destruction of this association. + # Returns ActiveRecord::AutosaveAssociation::marked_for_destruction? It's + # used in conjunction with fields_for to build a form element for the + # destruction of this association. # # See ActionView::Helpers::FormHelper::fields_for for more info. def _delete @@ -204,80 +229,101 @@ module ActiveRecord private - # Assigns the given attributes to the association. An association will be - # build if it doesn't exist yet. + # Attribute hash keys that should not be assigned as normal attributes. + # These hash keys are nested attributes implementation details. + UNASSIGNABLE_KEYS = %w{ id _delete } + + # Assigns the given attributes to the association. + # + # If the given attributes include an <tt>:id</tt> that matches the existing + # record’s id, then the existing record will be modified. Otherwise a new + # record will be built. + # + # If the given attributes include a matching <tt>:id</tt> attribute _and_ a + # <tt>:_delete</tt> key set to a truthy value, then the existing record + # will be marked for destruction. def assign_nested_attributes_for_one_to_one_association(association_name, attributes, allow_destroy) - if should_destroy_nested_attributes_record?(allow_destroy, attributes) - send(association_name).mark_for_destruction - else - (send(association_name) || send("build_#{association_name}")).attributes = attributes + attributes = attributes.stringify_keys + + if attributes['id'].blank? + unless reject_new_record?(association_name, attributes) + send("build_#{association_name}", attributes.except(*UNASSIGNABLE_KEYS)) + end + elsif (existing_record = send(association_name)) && existing_record.id.to_s == attributes['id'].to_s + assign_to_or_mark_for_destruction(existing_record, attributes, allow_destroy) end end # Assigns the given attributes to the collection association. # - # Keys containing an ID for an associated record will update that record. - # Keys starting with <tt>new</tt> will instantiate a new record for that - # association. + # Hashes with an <tt>:id</tt> value matching an existing associated record + # will update that record. Hashes without an <tt>:id</tt> value will build + # a new record for the association. Hashes with a matching <tt>:id</tt> + # value and a <tt>:_delete</tt> key set to a truthy value will mark the + # matched record for destruction. # # For example: # # assign_nested_attributes_for_collection_association(:people, { - # '1' => { 'name' => 'Peter' }, - # 'new_43' => { 'name' => 'John' } + # '1' => { :id => '1', :name => 'Peter' }, + # '2' => { :name => 'John' }, + # '3' => { :id => '2', :_delete => true } # }) # - # Will update the name of the Person with ID 1 and create a new associated - # person with the name 'John'. - def assign_nested_attributes_for_collection_association(association_name, attributes, allow_destroy) - unless attributes.is_a?(Hash) - raise ArgumentError, "Hash expected, got #{attributes.class.name} (#{attributes.inspect})" + # Will update the name of the Person with ID 1, build a new associated + # person with the name `John', and mark the associatied Person with ID 2 + # for destruction. + # + # Also accepts an Array of attribute hashes: + # + # assign_nested_attributes_for_collection_association(:people, [ + # { :id => '1', :name => 'Peter' }, + # { :name => 'John' }, + # { :id => '2', :_delete => true } + # ]) + def assign_nested_attributes_for_collection_association(association_name, attributes_collection, allow_destroy) + unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array) + raise ArgumentError, "Hash or Array expected, got #{attributes_collection.class.name} (#{attributes_collection.inspect})" end - # Make sure any new records sorted by their id before they're build. - sorted_by_id = attributes.sort_by { |id, _| id.is_a?(String) ? id.sub(/^new_/, '').to_i : id } - - sorted_by_id.each do |id, record_attributes| - if id.acts_like?(:string) && id.starts_with?('new_') - build_new_nested_attributes_record(association_name, record_attributes) - else - assign_to_or_destroy_nested_attributes_record(association_name, id, record_attributes, allow_destroy) - end + if attributes_collection.is_a? Hash + attributes_collection = attributes_collection.sort_by { |index, _| index.to_i }.map { |_, attributes| attributes } end - end - # Returns +true+ if <tt>allow_destroy</tt> is enabled and the attributes - # contains a truthy value for the key <tt>'_delete'</tt>. - # - # It will _always_ remove the <tt>'_delete'</tt> key, if present. - def should_destroy_nested_attributes_record?(allow_destroy, attributes) - ConnectionAdapters::Column.value_to_boolean(attributes.delete('_delete')) && allow_destroy - end + attributes_collection.each do |attributes| + attributes = attributes.stringify_keys - # Builds a new record with the given attributes. - # - # If a <tt>:reject_if</tt> proc exists for this association, it will be - # called with the attributes as its argument. If the proc returns a truthy - # value, the record is _not_ build. - # - # Alternatively, you can specify the <tt>'_delete'</tt> key to _not_ build - # a record. See should_destroy_nested_attributes_record? for more info. - def build_new_nested_attributes_record(association_name, attributes) - if reject_proc = self.class.reject_new_nested_attributes_procs[association_name] - return if reject_proc.call(attributes) + if attributes['id'].blank? + unless reject_new_record?(association_name, attributes) + send(association_name).build(attributes.except(*UNASSIGNABLE_KEYS)) + end + elsif existing_record = send(association_name).detect { |record| record.id.to_s == attributes['id'].to_s } + assign_to_or_mark_for_destruction(existing_record, attributes, allow_destroy) + end end - send(association_name).build(attributes) unless should_destroy_nested_attributes_record?(true, attributes) end - # Assigns the attributes to the record specified by +id+. Or marks it for - # destruction if #should_destroy_nested_attributes_record? returns +true+. - def assign_to_or_destroy_nested_attributes_record(association_name, id, attributes, allow_destroy) - record = send(association_name).detect { |record| record.id == id.to_i } - if should_destroy_nested_attributes_record?(allow_destroy, attributes) + # Updates a record with the +attributes+ or marks it for destruction if + # +allow_destroy+ is +true+ and has_delete_flag? returns +true+. + def assign_to_or_mark_for_destruction(record, attributes, allow_destroy) + if has_delete_flag?(attributes) && allow_destroy record.mark_for_destruction else - record.attributes = attributes + record.attributes = attributes.except(*UNASSIGNABLE_KEYS) end end + + # Determines if a hash contains a truthy _delete key. + def has_delete_flag?(hash) + ConnectionAdapters::Column.value_to_boolean hash['_delete'] + end + + # Determines if a new record should be build by checking for + # has_delete_flag? or if a <tt>:reject_if</tt> proc exists for this + # association and evaluates to +true+. + def reject_new_record?(association_name, attributes) + has_delete_flag?(attributes) || + self.class.reject_new_nested_attributes_procs[association_name].try(:call, attributes) + end end -end
\ No newline at end of file +end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 2e531a284e..cd6277c24b 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -23,7 +23,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase include AssertRaiseWithMessage def teardown - Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true + Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } end def test_base_should_have_an_empty_reject_new_nested_attributes_procs @@ -71,7 +71,7 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase assert_respond_to @pirate, :ship_attributes= end - def test_should_automatically_instantiate_an_associated_model_if_there_is_none + def test_should_build_a_new_record_if_there_is_no_id @ship.destroy @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger' } @@ -79,49 +79,106 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name end - def test_should_take_a_hash_and_assign_the_attributes_to_the_existing_associated_model - @pirate.ship_attributes = { :name => 'Davy Jones Gold Dagger' } - assert !@pirate.ship.new_record? + def test_should_not_build_a_new_record_if_there_is_no_id_and_delete_is_truthy + @ship.destroy + @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger', :_delete => '1' } + + assert_nil @pirate.ship + end + + def test_should_not_build_a_new_record_if_a_reject_if_proc_returns_false + @ship.destroy + @pirate.reload.ship_attributes = {} + + assert_nil @pirate.ship + end + + def test_should_replace_an_existing_record_if_there_is_no_id + @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger' } + + assert @pirate.ship.new_record? assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name + assert_equal 'Nights Dirty Lightning', @ship.name end - def test_should_also_work_with_a_HashWithIndifferentAccess - @pirate.ship_attributes = HashWithIndifferentAccess.new(:name => 'Davy Jones Gold Dagger') - assert !@pirate.ship.new_record? + def test_should_not_replace_an_existing_record_if_there_is_no_id_and_delete_is_truthy + @pirate.reload.ship_attributes = { :name => 'Davy Jones Gold Dagger', :_delete => '1' } + + assert_equal @ship, @pirate.ship + assert_equal 'Nights Dirty Lightning', @pirate.ship.name + end + + def test_should_modify_an_existing_record_if_there_is_a_matching_id + @pirate.reload.ship_attributes = { :id => @ship.id, :name => 'Davy Jones Gold Dagger' } + + assert_equal @ship, @pirate.ship assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name end - def test_should_work_with_update_attributes_as_well - @pirate.update_attributes({ :catchphrase => 'Arr', :ship_attributes => { :name => 'Mister Pablo' } }) - @pirate.reload + def test_should_take_a_hash_with_string_keys_and_update_the_associated_model + @pirate.reload.ship_attributes = { 'id' => @ship.id, 'name' => 'Davy Jones Gold Dagger' } - assert_equal 'Arr', @pirate.catchphrase - assert_equal 'Mister Pablo', @pirate.ship.name + assert_equal @ship, @pirate.ship + assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name end - def test_should_be_possible_to_destroy_the_associated_model + def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id + @ship.stubs(:id).returns('ABC1X') + @pirate.ship_attributes = { :id => @ship.id, :name => 'Davy Jones Gold Dagger' } + + assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name + end + + def test_should_delete_an_existing_record_if_there_is_a_matching_id_and_delete_is_truthy @pirate.ship.destroy - ['1', 1, 'true', true].each do |true_variable| + [1, '1', true, 'true'].each do |truth| @pirate.reload.create_ship(:name => 'Mister Pablo') assert_difference('Ship.count', -1) do - @pirate.update_attributes(:ship_attributes => { '_delete' => true_variable }) + @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_delete => truth }) end end end - def test_should_not_destroy_the_associated_model_with_a_non_truthy_argument - [nil, '0', 0, 'false', false].each do |false_variable| + def test_should_not_delete_an_existing_record_if_delete_is_not_truthy + [nil, '0', 0, 'false', false].each do |not_truth| assert_no_difference('Ship.count') do - @pirate.update_attributes(:ship_attributes => { '_delete' => false_variable }) + @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_delete => not_truth }) end end end + def test_should_not_delete_an_existing_record_if_allow_destroy_is_false + Pirate.accepts_nested_attributes_for :ship, :allow_destroy => false, :reject_if => proc { |attributes| attributes.empty? } + + assert_no_difference('Ship.count') do + @pirate.update_attribute(:ship_attributes, { :id => @pirate.ship.id, :_delete => '1' }) + end + + Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + end + + def test_should_also_work_with_a_HashWithIndifferentAccess + @pirate.ship_attributes = HashWithIndifferentAccess.new(:id => @ship.id, :name => 'Davy Jones Gold Dagger') + + assert !@pirate.ship.new_record? + assert_equal 'Davy Jones Gold Dagger', @pirate.ship.name + end + + def test_should_work_with_update_attributes_as_well + @pirate.update_attributes({ :catchphrase => 'Arr', :ship_attributes => { :id => @ship.id, :name => 'Mister Pablo' } }) + @pirate.reload + + assert_equal 'Arr', @pirate.catchphrase + assert_equal 'Mister Pablo', @pirate.ship.name + end + def test_should_not_destroy_the_associated_model_until_the_parent_is_saved assert_no_difference('Ship.count') do - @pirate.attributes = { :ship_attributes => { '_delete' => true } } + @pirate.attributes = { :ship_attributes => { :id => @ship.id, :_delete => '1' } } + end + assert_difference('Ship.count', -1) do + @pirate.save end - assert_difference('Ship.count', -1) { @pirate.save } end def test_should_automatically_enable_autosave_on_the_association @@ -131,15 +188,16 @@ end class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def setup - @ship = Ship.create!(:name => 'Nights Dirty Lightning') - @pirate = @ship.create_pirate(:catchphrase => "Don' botharrr talkin' like one, savvy?") + @ship = Ship.new(:name => 'Nights Dirty Lightning') + @pirate = @ship.build_pirate(:catchphrase => 'Aye') + @ship.save! end def test_should_define_an_attribute_writer_method_for_the_association assert_respond_to @ship, :pirate_attributes= end - def test_should_automatically_instantiate_an_associated_model_if_there_is_none + def test_should_build_a_new_record_if_there_is_no_id @pirate.destroy @ship.reload.pirate_attributes = { :catchphrase => 'Arr' } @@ -147,47 +205,95 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase assert_equal 'Arr', @ship.pirate.catchphrase end - def test_should_take_a_hash_and_assign_the_attributes_to_the_existing_associated_model - @ship.pirate_attributes = { :catchphrase => 'Arr' } - assert !@ship.pirate.new_record? + def test_should_not_build_a_new_record_if_there_is_no_id_and_delete_is_truthy + @pirate.destroy + @ship.reload.pirate_attributes = { :catchphrase => 'Arr', :_delete => '1' } + + assert_nil @ship.pirate + end + + def test_should_not_build_a_new_record_if_a_reject_if_proc_returns_false + @pirate.destroy + @ship.reload.pirate_attributes = {} + + assert_nil @ship.pirate + end + + def test_should_replace_an_existing_record_if_there_is_no_id + @ship.reload.pirate_attributes = { :catchphrase => 'Arr' } + + assert @ship.pirate.new_record? assert_equal 'Arr', @ship.pirate.catchphrase + assert_equal 'Aye', @pirate.catchphrase end - def test_should_also_work_with_a_HashWithIndifferentAccess - @ship.pirate_attributes = HashWithIndifferentAccess.new(:catchphrase => 'Arr') - assert !@ship.pirate.new_record? + def test_should_not_replace_an_existing_record_if_there_is_no_id_and_delete_is_truthy + @ship.reload.pirate_attributes = { :catchphrase => 'Arr', :_delete => '1' } + + assert_equal @pirate, @ship.pirate + assert_equal 'Aye', @ship.pirate.catchphrase + end + + def test_should_modify_an_existing_record_if_there_is_a_matching_id + @ship.reload.pirate_attributes = { :id => @pirate.id, :catchphrase => 'Arr' } + + assert_equal @pirate, @ship.pirate assert_equal 'Arr', @ship.pirate.catchphrase end - def test_should_work_with_update_attributes_as_well - @ship.update_attributes({ :name => 'Mister Pablo', :pirate_attributes => { :catchphrase => 'Arr' } }) - @ship.reload + def test_should_take_a_hash_with_string_keys_and_update_the_associated_model + @ship.reload.pirate_attributes = { 'id' => @pirate.id, 'catchphrase' => 'Arr' } - assert_equal 'Mister Pablo', @ship.name + assert_equal @pirate, @ship.pirate assert_equal 'Arr', @ship.pirate.catchphrase end - def test_should_be_possible_to_destroy_the_associated_model + def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id + @pirate.stubs(:id).returns('ABC1X') + @ship.pirate_attributes = { :id => @pirate.id, :catchphrase => 'Arr' } + + assert_equal 'Arr', @ship.pirate.catchphrase + end + + def test_should_delete_an_existing_record_if_there_is_a_matching_id_and_delete_is_truthy @ship.pirate.destroy - ['1', 1, 'true', true].each do |true_variable| + [1, '1', true, 'true'].each do |truth| @ship.reload.create_pirate(:catchphrase => 'Arr') assert_difference('Pirate.count', -1) do - @ship.update_attributes(:pirate_attributes => { '_delete' => true_variable }) + @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_delete => truth }) end end end - def test_should_not_destroy_the_associated_model_with_a_non_truthy_argument - [nil, '', '0', 0, 'false', false].each do |false_variable| + def test_should_not_delete_an_existing_record_if_delete_is_not_truthy + [nil, '0', 0, 'false', false].each do |not_truth| assert_no_difference('Pirate.count') do - @ship.update_attributes(:pirate_attributes => { '_delete' => false_variable }) + @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_delete => not_truth }) end end end + def test_should_not_delete_an_existing_record_if_allow_destroy_is_false + Ship.accepts_nested_attributes_for :pirate, :allow_destroy => false, :reject_if => proc { |attributes| attributes.empty? } + + assert_no_difference('Pirate.count') do + @ship.update_attribute(:pirate_attributes, { :id => @ship.pirate.id, :_delete => '1' }) + end + + Ship.accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + end + + def test_should_work_with_update_attributes_as_well + @ship.update_attributes({ :name => 'Mister Pablo', :pirate_attributes => { :catchphrase => 'Arr' } }) + @ship.reload + + assert_equal 'Mister Pablo', @ship.name + assert_equal 'Arr', @ship.pirate.catchphrase + end + def test_should_not_destroy_the_associated_model_until_the_parent_is_saved assert_no_difference('Pirate.count') do - @ship.attributes = { :pirate_attributes => { '_delete' => true } } + @ship.attributes = { :pirate_attributes => { :id => @ship.pirate.id, '_delete' => true } } end assert_difference('Pirate.count', -1) { @ship.save } end @@ -210,21 +316,43 @@ module NestedAttributesOnACollectionAssociationTests assert_equal ['Grace OMalley', 'Privateers Greed'], [@child_1.reload.name, @child_2.reload.name] end + def test_should_take_an_array_and_assign_the_attributes_to_the_associated_models + @pirate.send(association_setter, @alternate_params[association_getter].values) + @pirate.save + assert_equal ['Grace OMalley', 'Privateers Greed'], [@child_1.reload.name, @child_2.reload.name] + end + def test_should_also_work_with_a_HashWithIndifferentAccess - @pirate.send(association_setter, HashWithIndifferentAccess.new(@child_1.id => HashWithIndifferentAccess.new(:name => 'Grace OMalley'))) + @pirate.send(association_setter, HashWithIndifferentAccess.new('foo' => HashWithIndifferentAccess.new(:id => @child_1.id, :name => 'Grace OMalley'))) @pirate.save assert_equal 'Grace OMalley', @child_1.reload.name end - def test_should_take_a_hash_with_integer_keys_and_assign_the_attributes_to_the_associated_models + def test_should_take_a_hash_and_assign_the_attributes_to_the_associated_models @pirate.attributes = @alternate_params assert_equal 'Grace OMalley', @pirate.send(@association_name).first.name assert_equal 'Privateers Greed', @pirate.send(@association_name).last.name end - def test_should_automatically_build_new_associated_models_for_each_entry_in_a_hash_where_the_id_starts_with_the_string_new_ + def test_should_take_a_hash_with_composite_id_keys_and_assign_the_attributes_to_the_associated_models + @child_1.stubs(:id).returns('ABC1X') + @child_2.stubs(:id).returns('ABC2X') + + @pirate.attributes = { + association_getter => [ + { :id => @child_1.id, :name => 'Grace OMalley' }, + { :id => @child_2.id, :name => 'Privateers Greed' } + ] + } + + assert_equal ['Grace OMalley', 'Privateers Greed'], [@child_1.name, @child_2.name] + end + + def test_should_automatically_build_new_associated_models_for_each_entry_in_a_hash_where_the_id_is_missing @pirate.send(@association_name).destroy_all - @pirate.reload.attributes = { association_getter => { 'new_1' => { :name => 'Grace OMalley' }, 'new_2' => { :name => 'Privateers Greed' }}} + @pirate.reload.attributes = { + association_getter => { 'foo' => { :name => 'Grace OMalley' }, 'bar' => { :name => 'Privateers Greed' }} + } assert @pirate.send(@association_name).first.new_record? assert_equal 'Grace OMalley', @pirate.send(@association_name).first.name @@ -233,24 +361,36 @@ module NestedAttributesOnACollectionAssociationTests assert_equal 'Privateers Greed', @pirate.send(@association_name).last.name end - def test_should_remove_delete_key_from_arguments_hash_of_new_records + def test_should_not_assign_delete_key_to_a_record assert_nothing_raised ActiveRecord::UnknownAttributeError do - @pirate.send(association_setter, { 'new_1' => { '_delete' => '0' }}) + @pirate.send(association_setter, { 'foo' => { '_delete' => '0' }}) end end def test_should_ignore_new_associated_records_with_truthy_delete_attribute @pirate.send(@association_name).destroy_all - @pirate.reload.attributes = { association_getter => { 'new_1' => { :name => 'Grace OMalley' }, 'new_2' => { :name => 'Privateers Greed', '_delete' => '1' }}} + @pirate.reload.attributes = { + association_getter => { + 'foo' => { :name => 'Grace OMalley' }, + 'bar' => { :name => 'Privateers Greed', '_delete' => '1' } + } + } assert_equal 1, @pirate.send(@association_name).length assert_equal 'Grace OMalley', @pirate.send(@association_name).first.name end + def test_should_ignore_new_associated_records_if_a_reject_if_proc_returns_false + @alternate_params[association_getter]['baz'] = {} + assert_no_difference("@pirate.send(@association_name).length") do + @pirate.attributes = @alternate_params + end + end + def test_should_sort_the_hash_by_the_keys_before_building_new_associated_models attributes = ActiveSupport::OrderedHash.new - attributes['new_123726353'] = { :name => 'Grace OMalley' } - attributes['new_2'] = { :name => 'Privateers Greed' } # 2 is lower then 123726353 + attributes['123726353'] = { :name => 'Grace OMalley' } + attributes['2'] = { :name => 'Privateers Greed' } # 2 is lower then 123726353 @pirate.send(association_setter, attributes) assert_equal ['Posideons Killer', 'Killer bandita Dionne', 'Privateers Greed', 'Grace OMalley'].to_set, @pirate.send(@association_name).map(&:name).to_set @@ -260,28 +400,20 @@ module NestedAttributesOnACollectionAssociationTests assert_nothing_raised(ArgumentError) { @pirate.send(association_setter, {}) } assert_nothing_raised(ArgumentError) { @pirate.send(association_setter, ActiveSupport::OrderedHash.new) } - assert_raise_with_message ArgumentError, 'Hash expected, got String ("foo")' do + assert_raise_with_message ArgumentError, 'Hash or Array expected, got String ("foo")' do @pirate.send(association_setter, "foo") end - assert_raise_with_message ArgumentError, 'Hash expected, got Array ([:foo, :bar])' do - @pirate.send(association_setter, [:foo, :bar]) - end end def test_should_work_with_update_attributes_as_well - @pirate.update_attributes({ :catchphrase => 'Arr', association_getter => { @child_1.id => { :name => 'Grace OMalley' }}}) - assert_equal 'Grace OMalley', @child_1.reload.name - end + @pirate.update_attributes(:catchphrase => 'Arr', + association_getter => { 'foo' => { :id => @child_1.id, :name => 'Grace OMalley' }}) - def test_should_automatically_reject_any_new_record_if_a_reject_if_proc_exists_and_returns_false - @alternate_params[association_getter]["new_12345"] = {} - assert_no_difference("@pirate.send(@association_name).length") do - @pirate.attributes = @alternate_params - end + assert_equal 'Grace OMalley', @child_1.reload.name end - def test_should_update_existing_records_and_add_new_ones_that_have_an_id_that_start_with_the_string_new_ - @alternate_params[association_getter]['new_12345'] = { :name => 'Buccaneers Servant' } + def test_should_update_existing_records_and_add_new_ones_that_have_no_id + @alternate_params[association_getter]['baz'] = { :name => 'Buccaneers Servant' } assert_difference('@pirate.send(@association_name).count', +1) do @pirate.update_attributes @alternate_params end @@ -292,7 +424,7 @@ module NestedAttributesOnACollectionAssociationTests ['1', 1, 'true', true].each do |true_variable| record = @pirate.reload.send(@association_name).create!(:name => 'Grace OMalley') @pirate.send(association_setter, - @alternate_params[association_getter].merge(record.id => { '_delete' => true_variable }) + @alternate_params[association_getter].merge('baz' => { :id => record.id, '_delete' => true_variable }) ) assert_difference('@pirate.send(@association_name).count', -1) do @@ -303,7 +435,7 @@ module NestedAttributesOnACollectionAssociationTests def test_should_not_destroy_the_associated_model_with_a_non_truthy_argument [nil, '', '0', 0, 'false', false].each do |false_variable| - @alternate_params[association_getter][@child_1.id]['_delete'] = false_variable + @alternate_params[association_getter]['foo']['_delete'] = false_variable assert_no_difference('@pirate.send(@association_name).count') do @pirate.update_attributes(@alternate_params) end @@ -312,7 +444,7 @@ module NestedAttributesOnACollectionAssociationTests def test_should_not_destroy_the_associated_model_until_the_parent_is_saved assert_no_difference('@pirate.send(@association_name).count') do - @pirate.send(association_setter, @alternate_params[association_getter].merge(@child_1.id => { '_delete' => true })) + @pirate.send(association_setter, @alternate_params[association_getter].merge('baz' => { :id => @child_1.id, '_delete' => true })) end assert_difference('@pirate.send(@association_name).count', -1) { @pirate.save } end @@ -338,13 +470,15 @@ class TestNestedAttributesOnAHasManyAssociation < ActiveRecord::TestCase @association_name = :birds @pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") - @child_1 = @pirate.birds.create!(:name => 'Posideons Killer') - @child_2 = @pirate.birds.create!(:name => 'Killer bandita Dionne') + @pirate.birds.create!(:name => 'Posideons Killer') + @pirate.birds.create!(:name => 'Killer bandita Dionne') + + @child_1, @child_2 = @pirate.birds @alternate_params = { :birds_attributes => { - @child_1.id => { :name => 'Grace OMalley' }, - @child_2.id => { :name => 'Privateers Greed' } + 'foo' => { :id => @child_1.id, :name => 'Grace OMalley' }, + 'bar' => { :id => @child_2.id, :name => 'Privateers Greed' } } } end @@ -358,16 +492,18 @@ class TestNestedAttributesOnAHasAndBelongsToManyAssociation < ActiveRecord::Test @association_name = :parrots @pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?") - @child_1 = @pirate.parrots.create!(:name => 'Posideons Killer') - @child_2 = @pirate.parrots.create!(:name => 'Killer bandita Dionne') + @pirate.parrots.create!(:name => 'Posideons Killer') + @pirate.parrots.create!(:name => 'Killer bandita Dionne') + + @child_1, @child_2 = @pirate.parrots @alternate_params = { :parrots_attributes => { - @child_1.id => { :name => 'Grace OMalley' }, - @child_2.id => { :name => 'Privateers Greed' } + 'foo' => { :id => @child_1.id, :name => 'Grace OMalley' }, + 'bar' => { :id => @child_2.id, :name => 'Privateers Greed' } } } end include NestedAttributesOnACollectionAssociationTests -end
\ No newline at end of file +end diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 6a2416a05c..7bc50e0e7b 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -10,7 +10,7 @@ class Pirate < ActiveRecord::Base has_many :birds accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } - accepts_nested_attributes_for :ship, :allow_destroy => true + accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } validates_presence_of :catchphrase end diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index c46e27f3ae..06759d64b8 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -4,7 +4,7 @@ class Ship < ActiveRecord::Base belongs_to :pirate has_many :parts, :class_name => 'ShipPart', :autosave => true - accepts_nested_attributes_for :pirate, :allow_destroy => true + accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } validates_presence_of :name -end
\ No newline at end of file +end |