Let ActiveModel instances define partial paths.

Deprecate ActiveModel::Name#partial_path. Now you
should call #to_path directly on ActiveModel
instances.

4 files changed, 69 insertions, 23 deletions

diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index 52a640abf3..7ea2ea2d5a 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -1227,8 +1227,12 @@ module ActionView
         parent_builder.multipart = multipart if parent_builder
       end
 
-      def self.model_name
-        @model_name ||= Struct.new(:partial_path).new(name.demodulize.underscore.sub!(/_builder$/, ''))
+      def self.to_path
+        @_to_path ||= name.demodulize.underscore.sub!(/_builder$/, '')
+      end
+
+      def to_path
+        self.class.to_path
       end
 
       def to_model
diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb
index c0ac332c4e..fc8a6b3107 100644
--- a/actionpack/lib/action_view/renderer/partial_renderer.rb
+++ b/actionpack/lib/action_view/renderer/partial_renderer.rb
@@ -206,13 +206,6 @@ module ActionView
   #     <%- end -%>
   #   <% end %>
   class PartialRenderer < AbstractRenderer #:nodoc:
-    PARTIAL_NAMES = Hash.new {|h,k| h[k] = {} }
-
-    def initialize(*)
-      super
-      @partial_names = PARTIAL_NAMES[@lookup_context.prefixes.first]
-    end
-
     def render(context, options, block)
       setup(context, options, block)
 
@@ -359,28 +352,36 @@ module ActionView
       segments
     end
 
+    PARTIAL_PATHS = {}
+
     def partial_path(object = @object)
-      @partial_names[object.class.name] ||= begin
-        object = object.to_model if object.respond_to?(:to_model)
-        object.class.model_name.partial_path.dup.tap do |partial|
-          path = @lookup_context.prefixes.first
-          merge_path_into_partial(path, partial)
-        end
+      object = object.to_model if object.respond_to?(:to_model)
+
+      path = if object.respond_to?(:to_path)
+               object.to_path
+             else
+               ActiveSupport::Deprecation.warn "ActiveModel-compatible objects whose classes return a #model_name that responds to #partial_path are deprecated. Please respond to #to_path directly instead."
+               object.class.model_name.partial_path
+             end
+
+      prefix = @lookup_context.prefixes.first
+      PARTIAL_PATHS[ [path, prefix] ] ||= path.dup.tap do |object_path|
+        merge_prefix_into_object_path(prefix, object_path)
       end
     end
 
-    def merge_path_into_partial(path, partial)
-      if path.include?(?/) && partial.include?(?/)
+    def merge_prefix_into_object_path(prefix, object_path)
+      if prefix.include?(?/) && object_path.include?(?/)
         overlap = []
-        path_array = File.dirname(path).split('/')
-        partial_array = partial.split('/')[0..-3] # skip model dir & partial
+        prefix_array = File.dirname(prefix).split('/')
+        object_path_array = object_path.split('/')[0..-3] # skip model dir & partial
 
-        path_array.each_with_index do |dir, index|
-          overlap << dir if dir == partial_array[index]
+        prefix_array.each_with_index do |dir, index|
+          overlap << dir if dir == object_path_array[index]
         end
 
-        partial.gsub!(/^#{overlap.join('/')}\//,'')
-        partial.insert(0, "#{File.dirname(path)}/")
+        object_path.gsub!(/^#{overlap.join('/')}\//,'')
+        object_path.insert(0, "#{File.dirname(prefix)}/")
       end
     end
 
diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb
index aca2dc9e4d..71a2c46d92 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -1891,6 +1891,17 @@ class FormHelperTest < ActionView::TestCase
     assert_equal LabelledFormBuilder, klass
   end
 
+  def test_form_for_with_labelled_builder_path
+    path = nil
+
+    form_for(@post, :builder => LabelledFormBuilder) do |f|
+      path = f.to_path
+      ''
+    end
+
+    assert_equal 'labelled_form', path
+  end
+
   class LabelledFormBuilderSubclass < LabelledFormBuilder; end
 
   def test_form_for_with_labelled_builder_with_nested_fields_for_with_custom_builder
diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
index 68b2ed45d1..0b91e55091 100644
--- a/actionpack/test/template/render_test.rb
+++ b/actionpack/test/template/render_test.rb
@@ -201,6 +201,36 @@ module RenderTestCases
       @controller_view.render(customers, :greeting => "Hello")
   end
 
+  class CustomerWithDeprecatedPartialPath
+    attr_reader :name
+
+    def self.model_name
+      Struct.new(:partial_path).new("customers/customer")
+    end
+
+    def initialize(name)
+      @name = name
+    end
+  end
+
+  def test_render_partial_using_object_with_deprecated_partial_path
+    assert_deprecated(/#model_name.*#partial_path.*#to_path/) do
+      assert_equal "Hello: nertzy",
+        @controller_view.render(CustomerWithDeprecatedPartialPath.new("nertzy"), :greeting => "Hello")
+    end
+  end
+
+  def test_render_partial_using_collection_with_deprecated_partial_path
+    assert_deprecated(/#model_name.*#partial_path.*#to_path/) do
+      customers = [
+        CustomerWithDeprecatedPartialPath.new("nertzy"),
+        CustomerWithDeprecatedPartialPath.new("peeja")
+      ]
+      assert_equal "Hello: nertzyHello: peeja",
+        @controller_view.render(customers, :greeting => "Hello")
+    end
+  end
+
   # TODO: The reason for this test is unclear, improve documentation
   def test_render_partial_and_fallback_to_layout
     assert_equal "Before (Josh)\n\nAfter", @view.render(:partial => "test/layout_for_partial", :locals => { :name => "Josh" })