aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actionpack/lib/action_view/helpers/form_helper.rb26
-rw-r--r--actionpack/test/template/form_helper_test.rb38
-rw-r--r--activerecord/lib/active_record/nested_attributes.rb246
-rw-r--r--activerecord/test/cases/nested_attributes_test.rb288
-rw-r--r--activerecord/test/models/pirate.rb2
-rw-r--r--activerecord/test/models/ship.rb4
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