gistfile1.diff

From d6cf71ff546cb28a0278ea7c8c45cd378ead42e1 Mon Sep 17 00:00:00 2001
From: Eloy Duran <[email protected]>
Date: Thu, 31 Dec 2009 14:03:04 +0100
Subject: [PATCH 1/5] Add AssociationReflection#collection_association? which returns true if it's for a has_many or has_and_belongs_to_many association.

---
 activerecord/lib/active_record/reflection.rb |    9 ++++++++-
 activerecord/test/cases/reflection_test.rb   |    9 +++++++++
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index b751c9a..69772bf 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -252,10 +252,17 @@ module ActiveRecord
         end
       end
 
+      # Returns whether or not this association reflection is for a collection
+      # association. Returns +true+ if the +macro+ is one of +has_many+ or
+      # +has_and_belongs_to_many+, +false+ otherwise.
+      def collection_association?
+        [:has_many, :has_and_belongs_to_many].include?(macro)
+      end
+
       private
         def derive_class_name
           class_name = name.to_s.camelize
-          class_name = class_name.singularize if [ :has_many, :has_and_belongs_to_many ].include?(macro)
+          class_name = class_name.singularize if collection_association?
           class_name
         end
 
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index acd214e..38469c4 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -4,6 +4,7 @@ require 'models/customer'
 require 'models/company'
 require 'models/company_in_module'
 require 'models/subscriber'
+require 'models/ship'
 require 'models/pirate'
 require 'models/price_estimate'
 
@@ -190,6 +191,14 @@ class ReflectionTest < ActiveRecord::TestCase
     assert_kind_of ActiveRecord::Reflection::ThroughReflection, Subscriber.reflect_on_association(:books)
   end
 
+  def test_collection_association?
+    assert Pirate.reflect_on_association(:birds).collection_association?
+    assert Pirate.reflect_on_association(:parrots).collection_association?
+
+    assert !Pirate.reflect_on_association(:ship).collection_association?
+    assert !Ship.reflect_on_association(:pirate).collection_association?
+  end
+
   private
     def assert_reflection(klass, association, options)
       assert reflection = klass.reflect_on_association(association)
-- 
1.6.4.4


From 632fde104522a37d1a28acfc5ebe454688aa0c48 Mon Sep 17 00:00:00 2001
From: Eloy Duran <[email protected]>
Date: Thu, 31 Dec 2009 14:07:56 +0100
Subject: [PATCH 2/5] Cleanup some code in NestedAttributes & AutosaveAssociation with AssociationReflection#collection_association?.

---
 .../lib/active_record/autosave_association.rb      |    3 +--
 .../lib/active_record/nested_attributes.rb         |    8 +-------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index d563fe5..6214399 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -167,8 +167,7 @@ module ActiveRecord
         validation_method = "validate_associated_records_for_#{reflection.name}"
         force_validation = (reflection.options[:validate] == true || reflection.options[:autosave] == true)
 
-        case reflection.macro
-        when :has_many, :has_and_belongs_to_many
+        if reflection.collection_association?
           unless method_defined?(save_method)
             before_save :before_save_collection_association
 
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index ae2a3e4..4493718 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -242,16 +242,10 @@ module ActiveRecord
 
         attr_names.each do |association_name|
           if reflection = reflect_on_association(association_name)
-            type = case reflection.macro
-            when :has_one, :belongs_to
-              :one_to_one
-            when :has_many, :has_and_belongs_to_many
-              :collection
-            end
-
             reflection.options[:autosave] = true
             add_autosave_association_callbacks(reflection)
             nested_attributes_options[association_name.to_sym] = options
+            type = (reflection.collection_association? ? :collection : :one_to_one)
 
             # def pirate_attributes=(attributes)
             #   assign_nested_attributes_for_one_to_one_association(:pirate, attributes)
-- 
1.6.4.4


From f13542619f0b0e7bf615fa35699a96ace23cfd0f Mon Sep 17 00:00:00 2001
From: Eloy Duran <[email protected]>
Date: Thu, 31 Dec 2009 14:41:47 +0100
Subject: [PATCH 3/5] Add Associations#build_association as a simple API to build an associated record for collection and to-one associations.

---
 activerecord/lib/active_record/associations.rb |   30 +++++++++++++++++++++++
 activerecord/test/cases/associations_test.rb   |   31 +++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 1 deletions(-)

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index f0bad6c..59acc7f 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -108,6 +108,36 @@ module ActiveRecord
       end unless self.new_record?
     end
 
+    # Naively builds a new record for the given +association_name+. It does
+    # nothing more than calling the correct builder method:
+    #
+    # For a has_one or belongs_to association:
+    #
+    #   parent.build_association(:child)
+    #
+    # Is the equivalent of:
+    #
+    #   parent.build_child
+    #
+    # For a has_many or has_and_belongs_to_many association:
+    #
+    #   parent.build_association(:children)
+    #
+    # Is the equivalent of:
+    #
+    #   parent.children.build
+    #
+    # Note that this method is used as a, for now, unofficial ActiveModel API
+    # to build new records from the view. For instance, in +fields_for+.
+    def build_association(association_name)
+      reflection = self.class.reflect_on_association(association_name.to_sym)
+      if reflection.collection_association?
+        send(association_name).build
+      else
+        send("build_#{association_name}")
+      end
+    end
+
     private
       # Gets the specified association instance if it responds to :loaded?, nil otherwise.
       def association_instance_get(name)
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index 9bc34bd..f54d660 100644
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -16,9 +16,11 @@ require 'models/tag'
 require 'models/tagging'
 require 'models/person'
 require 'models/reader'
+require 'models/bird'
 require 'models/parrot'
 require 'models/pirate'
 require 'models/treasure'
+require 'models/ship'
 require 'models/price_estimate'
 require 'models/club'
 require 'models/member'
@@ -64,7 +66,7 @@ class AssociationsTest < ActiveRecord::TestCase
     assert !firm.clients(true).empty?, "New firm should have reloaded client objects"
     assert_equal 1, firm.clients(true).size, "New firm should have reloaded clients count"
   end
-  
+
   def test_force_reload_is_uncached
     firm = Firm.create!("name" => "A New Firm, Inc")
     client = Client.create!("name" => "TheClient.com", :firm => firm)
@@ -75,6 +77,33 @@ class AssociationsTest < ActiveRecord::TestCase
     end
   end
 
+  def test_simple_association_build_for_collection_associations
+    pirate = Pirate.create!(:catchphrase => 'Yarr!')
+    existing_bird = pirate.birds.create!(:name => 'Killer bandita Dionne')
+    existing_parrot = pirate.parrots.create!(:name => 'Posideons Killer')
+
+    new_bird = pirate.build_association(:birds)
+    assert new_bird.new_record?
+    assert_equal [existing_bird, new_bird], pirate.birds
+    assert_equal [existing_bird], pirate.reload.birds
+
+    new_parrot = pirate.build_association('parrots')
+    assert new_parrot.new_record?
+    assert_equal [existing_parrot, new_parrot], pirate.parrots
+    assert_equal [existing_parrot], pirate.reload.parrots
+  end
+
+  def test_simple_association_build_for_to_one_associations
+    pirate = Pirate.create!(:catchphrase => 'Yarr!')
+    ship = pirate.build_association(:ship)
+    assert ship.new_record?
+    assert_equal ship, pirate.ship
+
+    ship = Ship.create!(:name => 'Nights Dirty Lightning')
+    pirate = ship.build_association('pirate')
+    assert pirate.new_record?
+    assert_equal pirate, ship.pirate
+  end
 end
 
 class AssociationProxyTest < ActiveRecord::TestCase
-- 
1.6.4.4


From 7ead79a6355535000469aa6dd5e9d98b8350e82a Mon Sep 17 00:00:00 2001
From: Eloy Duran <[email protected]>
Date: Thu, 31 Dec 2009 15:06:54 +0100
Subject: [PATCH 4/5] Adds :build_if_blank option to fields_for that will initialize associated records for models using nested_attributes_for.

Based on earlier work done by Mike Breen. [#2365 state:resolved]
---
 actionpack/lib/action_view/helpers/form_helper.rb |   18 ++++++
 actionpack/test/template/form_helper_test.rb      |   65 ++++++++++++++++++++-
 2 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index d0c66ed..314f5bf 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -1063,6 +1063,10 @@ module ActionView
             association = @object.send(association_name)
           end
 
+          if association.blank? && args.last.is_a?(Hash) && (times = args.last[:build_if_blank])
+            association = build_associated_models(association_name, times)
+          end
+
           if association.is_a?(Array)
             explicit_child_index = args.last[:child_index] if args.last.is_a?(Hash)
             association.map do |child|
@@ -1073,6 +1077,20 @@ module ActionView
           end
         end
 
+        def build_associated_models(association_name, times)
+          if @object.respond_to?(:build_association)
+            if @object.send(association_name).is_a?(Array)
+              Array.new(times.is_a?(Numeric) ? times : 1) do
+                @object.build_association(association_name)
+              end
+            else
+              @object.build_association(association_name)
+            end
+          else
+            raise NoMethodError, "Unable to build a nested model, because the object does not respond to `build_association'."
+          end
+        end
+
         def fields_for_nested_model(name, object, args, block)
           if object.new_record?
             @template.fields_for(name, object, *args, &block)
diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb
index 44734ab..1c64c34 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -448,7 +448,7 @@ class FormHelperTest < ActionView::TestCase
 
     assert_dom_equal expected, output_buffer
   end
-
+  
   def test_nested_fields_for_with_nested_collections
     form_for('post[]', @post) do |f|
       concat f.text_field(:title)
@@ -592,6 +592,65 @@ class FormHelperTest < ActionView::TestCase
     assert_dom_equal expected, output_buffer
   end
 
+  def test_nested_fields_for_with_build_if_blank_option_raises_if_object_is_not_compatible
+    assert_raises(NoMethodError) do
+      form_for(:post, @post) do |f|
+        f.fields_for(:author, :build_if_blank => true) {}
+      end
+    end
+
+    assert_raises(NoMethodError) do
+      @post.comments = []
+      form_for(:post, @post) do |f|
+        f.fields_for(:comments, :build_if_blank => 3) {}
+      end
+    end
+  end
+
+  def test_nested_fields_for_one_to_one_association_with_build_if_blank_option
+    def @post.build_association(name)
+      Author.new if name == :author
+    end
+
+    form_for(:post, @post) do |f|
+      concat f.text_field(:title)
+      f.fields_for(:author, :build_if_blank => true) do |af|
+        concat af.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_author_attributes_name" name="post[author_attributes][name]" size="30" type="text" value="new author" />' +
+               '</form>'
+
+    assert_dom_equal expected, output_buffer
+  end
+
+  def test_nested_fields_for_with_build_if_blank_option_for_collection_association
+    @post.comments = []
+
+    def @post.build_association(name)
+      Comment.new if name == :comments
+    end
+
+    form_for(:post, @post) do |f|
+      concat f.text_field(:title)
+      f.fields_for(:comments, :build_if_blank => 3) 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_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" />' +
+               '<input id="post_comments_attributes_2_name" name="post[comments_attributes][2][name]" size="30" type="text" value="new comment" />' +
+               '</form>'
+
+    assert_dom_equal expected, output_buffer
+  end
+
   def test_nested_fields_for_with_explicitly_passed_object_on_a_nested_attributes_one_to_one_association
     form_for(:post, @post) do |f|
       f.fields_for(:author, Author.new(123)) do |af|
@@ -606,7 +665,7 @@ class FormHelperTest < ActionView::TestCase
 
     form_for(:post, @post) do |f|
       concat f.text_field(:title)
-      f.fields_for(:author) do |af|
+      f.fields_for(:author, :build_if_blank => true) do |af|
         concat af.text_field(:name)
       end
     end
@@ -625,7 +684,7 @@ class FormHelperTest < ActionView::TestCase
 
     form_for(:post, @post) do |f|
       concat f.text_field(:title)
-      f.fields_for(:author) do |af|
+      f.fields_for(:author, :build_if_blank => 3) do |af|
         concat af.hidden_field(:id)
         concat af.text_field(:name)
       end
-- 
1.6.4.4


From 79fd8a2a8d3b8043834d0fb2452bd3b2100b98ed Mon Sep 17 00:00:00 2001
From: Eloy Duran <[email protected]>
Date: Thu, 31 Dec 2009 15:26:33 +0100
Subject: [PATCH 5/5] Add documentation about :build_if_blank and refactored a bit.

---
 actionpack/lib/action_view/helpers/form_helper.rb |   54 ++++++++++++++++-----
 1 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index 314f5bf..69dbff0 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -407,6 +407,16 @@ module ActionView
       #     <% end %>
       #   <% end %>
       #
+      # If you would like to build an associated model in case there is none,
+      # then you can use the <tt>:build_if_blank</tt> option:
+      #
+      #   <% form_for @person, :url => { :action => "update" } do |person_form| %>
+      #     ...
+      #     <% person_form.fields_for :address, :build_if_blank => true do |address_fields| %>
+      #       ...
+      #     <% end %>
+      #   <% end %>
+      #
       # ==== One-to-many
       #
       # Consider a Person class which returns an _array_ of Project instances
@@ -486,6 +496,17 @@ module ActionView
       #       Delete: <%= project_fields.check_box :_delete %>
       #     <% end %>
       #   <% end %>
+      #
+      # If you would like to build associated models in case there are none,
+      # then you can use the <tt>:build_if_blank</tt> option with the amount of
+      # models that are to be instantiated:
+      #
+      #   <% form_for @person, :url => { :action => "update" } do |person_form| %>
+      #     ...
+      #     <% person_form.fields_for :projects, :build_if_blank => 3 do |project_fields| %>
+      #       ...
+      #     <% end %>
+      #   <% end %>
       def fields_for(record_or_name_or_array, *args, &block)
         raise ArgumentError, "Missing block" unless block_given?
         options = args.extract_options!
@@ -1055,7 +1076,23 @@ module ActionView
 
         def fields_for_with_nested_attributes(association_name, args, block)
           name = "#{object_name}[#{association_name}_attributes]"
-          association = args.first.to_model if args.first.respond_to?(:to_model)
+          options = args.last.is_a?(Hash) ? args.last : {}
+          association = get_nested_association(association_name, args.first, options)
+
+          if association.is_a?(Array)
+            association.map do |child|
+              index = options[:child_index] || nested_child_index(name)
+              fields_for_nested_model("#{name}[#{index}]", child, args, block)
+            end.join
+          elsif association
+            fields_for_nested_model(name, association, args, block)
+          end
+        end
+
+        def get_nested_association(association_name, explicit_association, options)
+          if explicit_association.respond_to?(:to_model)
+            association = explicit_association.to_model
+          end
 
           if association.respond_to?(:new_record?)
             association = [association] if @object.send(association_name).is_a?(Array)
@@ -1063,21 +1100,14 @@ module ActionView
             association = @object.send(association_name)
           end
 
-          if association.blank? && args.last.is_a?(Hash) && (times = args.last[:build_if_blank])
-            association = build_associated_models(association_name, times)
+          if association.blank? && (times = options[:build_if_blank])
+            association = build_nested_models(association_name, times)
           end
 
-          if association.is_a?(Array)
-            explicit_child_index = args.last[:child_index] if args.last.is_a?(Hash)
-            association.map do |child|
-              fields_for_nested_model("#{name}[#{explicit_child_index || nested_child_index(name)}]", child, args, block)
-            end.join
-          elsif association
-            fields_for_nested_model(name, association, args, block)
-          end
+          association
         end
 
-        def build_associated_models(association_name, times)
+        def build_nested_models(association_name, times)
           if @object.respond_to?(:build_association)
             if @object.send(association_name).is_a?(Array)
               Array.new(times.is_a?(Numeric) ? times : 1) do
-- 
1.6.4.4