Merge commit 'alloy/master-nested_attributes_and_autosave'

actionpack/lib/action_view/helpers/form_helper.rb

@@ -449,6 +449,15 @@ def apply_form_for_options!(object_or_array, options) #:nodoc:
       #     <% end %>
       #   <% end %>
       #
+      # Or a collection to be used:
+      #
+      #   <% form_for @person, :url => { :action => "update" } do |person_form| %>
+      #     ...
+      #     <% person_form.fields_for :projects, @active_projects do |project_fields| %>
+      #       Name: <%= project_fields.text_field :name %>
+      #     <% end %>
+      #   <% end %>
+      #
       # When projects is already an association on Person you can use
       # +accepts_nested_attributes_for+ to define the writer method for you:
       #
@@ -1037,18 +1046,21 @@ def nested_attributes_association?(association_name)
 
         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.to_model if args.first.respond_to?(:to_model)
+          association = args.first.to_model if args.first.respond_to?(:to_model)
+
+          if association.respond_to?(:new_record?)
+            association = [association] if @object.send(association_name).is_a?(Array)
+          elsif !association.is_a?(Array)
+            association = @object.send(association_name)
+          end
 
           if association.is_a?(Array)
-            children = explicit_object ? [explicit_object] : association
             explicit_child_index = args.last[:child_index] if args.last.is_a?(Hash)
-
-            children.map do |child|
+            association.map do |child|
               fields_for_nested_model("#{name}[#{explicit_child_index || nested_child_index(name)}]", child, args, block)
             end.join
-          else
-            fields_for_nested_model(name, explicit_object || association, args, block)
+          elsif association
+            fields_for_nested_model(name, association, args, block)
           end
         end
 

actionpack/test/template/form_helper_test.rb

@@ -784,6 +784,42 @@ def test_nested_fields_for_with_existing_and_new_records_on_a_nested_attributes_
     assert_dom_equal expected, output_buffer
   end
 
+  def test_nested_fields_for_with_an_empty_supplied_attributes_collection
+    form_for(:post, @post) do |f|
+      concat f.text_field(:title)
+      f.fields_for(:comments, []) do |cf|
+        concat cf.text_field(:name)
+      end
+    end
+
+    expected = '<form action="http://www.example.com" method="post">' +
+               '<input name="post[title]" size="30" type="text" id="post_title" value="Hello World" />' +
+               '</form>'
+
+    assert_dom_equal expected, output_buffer
+  end
+
+  def test_nested_fields_for_with_existing_records_on_a_supplied_nested_attributes_collection
+    @post.comments = Array.new(2) { |id| Comment.new(id + 1) }
+
+    form_for(:post, @post) do |f|
+      concat f.text_field(:title)
+      f.fields_for(:comments, @post.comments) do |cf|
+        concat cf.text_field(:name)
+      end
+    end
+
+    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_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
+  end
+
   def test_nested_fields_for_on_a_nested_attributes_collection_association_yields_only_builder
     @post.comments = [Comment.new(321), Comment.new]
     yielded_comments = []

activerecord/lib/active_record/associations.rb

@@ -280,9 +280,10 @@ def association_instance_set(name, association)
     # You can manipulate objects and associations before they are saved to the database, but there is some special behavior you should be
     # aware of, mostly involving the saving of associated objects.
     #
-    # Unless you enable the :autosave option on a <tt>has_one</tt>, <tt>belongs_to</tt>,
-    # <tt>has_many</tt>, or <tt>has_and_belongs_to_many</tt> association,
-    # in which case the members are always saved.
+    # Unless you set the :autosave option on a <tt>has_one</tt>, <tt>belongs_to</tt>,
+    # <tt>has_many</tt>, or <tt>has_and_belongs_to_many</tt> association. Setting it
+    # to +true+ will _always_ save the members, whereas setting it to +false+ will
+    # _never_ save the members.
     #
     # === One-to-one associations
     #

activerecord/lib/active_record/autosave_association.rb

@@ -158,7 +158,7 @@ def #{type}(name, options = {})
       def add_autosave_association_callbacks(reflection)
         save_method = "autosave_associated_records_for_#{reflection.name}"
         validation_method = "validate_associated_records_for_#{reflection.name}"
-        validate validation_method
+        force_validation = (reflection.options[:validate] == true || reflection.options[:autosave] == true)
 
         case reflection.macro
         when :has_many, :has_and_belongs_to_many
@@ -169,7 +169,10 @@ def add_autosave_association_callbacks(reflection)
           after_create save_method
           after_update save_method
 
-          define_method(validation_method) { validate_collection_association(reflection) }
+          if force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false)
+            define_method(validation_method) { validate_collection_association(reflection) }
+            validate validation_method
+          end
         else
           case reflection.macro
           when :has_one
@@ -179,7 +182,11 @@ def add_autosave_association_callbacks(reflection)
             define_method(save_method) { save_belongs_to_association(reflection) }
             before_save save_method
           end
-          define_method(validation_method) { validate_single_association(reflection) }
+
+          if force_validation
+            define_method(validation_method) { validate_single_association(reflection) }
+            validate validation_method
+          end
         end
       end
     end
@@ -223,18 +230,16 @@ def associated_records_to_validate_or_save(association, new_record, autosave)
     # Validate the association if <tt>:validate</tt> or <tt>:autosave</tt> is
     # turned on for the association specified by +reflection+.
     def validate_single_association(reflection)
-      if reflection.options[:validate] == true || reflection.options[:autosave] == true
-        if (association = association_instance_get(reflection.name)) && !association.target.nil?
-          association_valid?(reflection, association)
-        end
+      if (association = association_instance_get(reflection.name)) && !association.target.nil?
+        association_valid?(reflection, association)
       end
     end
 
     # Validate the associated records if <tt>:validate</tt> or
     # <tt>:autosave</tt> is turned on for the association specified by
     # +reflection+.
     def validate_collection_association(reflection)
-      if reflection.options[:validate] != false && association = association_instance_get(reflection.name)
+      if association = association_instance_get(reflection.name)
         if records = associated_records_to_validate_or_save(association, new_record?, reflection.options[:autosave])
           records.each { |record| association_valid?(reflection, record) }
         end
@@ -243,15 +248,15 @@ def validate_collection_association(reflection)
 
     # Returns whether or not the association is valid and applies any errors to
     # the parent, <tt>self</tt>, if it wasn't. Skips any <tt>:autosave</tt>
-    # enabled records if they're marked_for_destruction?.
+    # enabled records if they're marked_for_destruction? or destroyed.
     def association_valid?(reflection, association)
+      return true if association.destroyed? || association.marked_for_destruction?
+
       unless valid = association.valid?
         if reflection.options[:autosave]
-          unless association.marked_for_destruction?
-            association.errors.each do |attribute, message|
-              attribute = "#{reflection.name}_#{attribute}"
-              errors[attribute] << message if errors[attribute].empty?
-            end
+          association.errors.each do |attribute, message|
+            attribute = "#{reflection.name}_#{attribute}"
+            errors[attribute] << message if errors[attribute].empty?
           end
         else
           errors.add(reflection.name)
@@ -281,9 +286,11 @@ def save_collection_association(reflection)
 
         if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave)
           records.each do |record|
+            next if record.destroyed?
+
             if autosave && record.marked_for_destruction?
               association.destroy(record)
-            elsif @new_record_before_save || record.new_record?
+            elsif autosave != false && (@new_record_before_save || record.new_record?)
               if autosave
                 association.send(:insert_record, record, false, false)
               else
@@ -309,14 +316,17 @@ def save_collection_association(reflection)
     # This all happens inside a transaction, _if_ the Transactions module is included into
     # ActiveRecord::Base after the AutosaveAssociation module, which it does by default.
     def save_has_one_association(reflection)
-      if (association = association_instance_get(reflection.name)) && !association.target.nil?
+      if (association = association_instance_get(reflection.name)) && !association.target.nil? && !association.destroyed?
         autosave = reflection.options[:autosave]
 
         if autosave && association.marked_for_destruction?
           association.destroy
-        elsif new_record? || association.new_record? || association[reflection.primary_key_name] != id || autosave
-          association[reflection.primary_key_name] = id
-          association.save(!autosave)
+        else
+          key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id
+          if autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != key || autosave)
+            association[reflection.primary_key_name] = key
+            association.save(!autosave)
+          end
         end
       end
     end
@@ -330,12 +340,12 @@ def save_has_one_association(reflection)
     # This all happens inside a transaction, _if_ the Transactions module is included into
     # ActiveRecord::Base after the AutosaveAssociation module, which it does by default.
     def save_belongs_to_association(reflection)
-      if association = association_instance_get(reflection.name)
+      if (association = association_instance_get(reflection.name)) && !association.destroyed?
         autosave = reflection.options[:autosave]
 
         if autosave && association.marked_for_destruction?
           association.destroy
-        else
+        elsif autosave != false
           association.save(!autosave) if association.new_record? || autosave
 
           if association.updated?