diff options
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/Rakefile | 4 | ||||
-rw-r--r-- | activerecord/lib/active_record/migration.rb | 6 | ||||
-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, 369 insertions, 181 deletions
diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 1c7e2603ee..8b3b97bac4 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -30,7 +30,9 @@ desc 'Run mysql, sqlite, and postgresql tests by default' task :default => :test desc 'Run mysql, sqlite, and postgresql tests' -task :test => %w(test_mysql test_sqlite3 test_postgresql) +task :test => defined?(JRUBY_VERSION) ? + %w(test_jdbcmysql test_jdbcsqlite3 test_jdbcpostgresql) : + %w(test_mysql test_sqlite3 test_postgresql) for adapter in %w( mysql postgresql sqlite sqlite3 firebird db2 oracle sybase openbase frontbase jdbcmysql jdbcpostgresql jdbcsqlite3 jdbcderby jdbch2 jdbchsqldb ) Rake::TestTask.new("test_#{adapter}") { |t| diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 15350cf1e1..657acd6dc0 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -338,6 +338,10 @@ module ActiveRecord self.verbose = save end + def connection + ActiveRecord::Base.connection + end + def method_missing(method, *arguments, &block) arg_list = arguments.map(&:inspect) * ', ' @@ -345,7 +349,7 @@ module ActiveRecord unless arguments.empty? || method == :execute arguments[0] = Migrator.proper_table_name(arguments.first) end - ActiveRecord::Base.connection.send(method, *arguments, &block) + connection.send(method, *arguments, &block) end end end 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 |