aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLance Ivy <lance@cainlevy.net>2009-02-08 14:23:35 +0100
committerMichael Koziarski <michael@koziarski.com>2009-02-13 21:47:56 +1300
commit5dbc9d40a49f5f0f50c2f3ebe6dda942f0e61562 (patch)
tree90f4a5d54190ada928712a285c56bcfe6da334fc
parenta6508527570cd3f7225a7030218447bcc5824224 (diff)
downloadrails-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.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