Merge pull request #2034 from Casecommons/to_path

Allow ActiveModel-compatible instances to define their own partial paths

actionpack/lib/action_view/helpers/form_helper.rb

@@ -1227,8 +1227,12 @@ def multipart=(multipart)
         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

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 @@ def collection_without_template
       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
 

actionpack/test/template/form_helper_test.rb

@@ -1891,6 +1891,17 @@ def test_form_for_with_labelled_builder_with_nested_fields_for_with_options_hash
     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

actionpack/test/template/render_test.rb

@@ -201,6 +201,36 @@ def test_render_partial_using_collection
       @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" })

activemodel/lib/active_model/conversion.rb

+require 'active_support/concern'
+require 'active_support/inflector'
+
 module ActiveModel
   # == Active Model Conversions
   #
-  # Handles default conversions: to_model, to_key and to_param.
+  # Handles default conversions: to_model, to_key, to_param, and to_path.
   #
-  # Let's take for example this non persisted object.
+  # Let's take for example this non-persisted object.
   #
   #   class ContactMessage
   #     include ActiveModel::Conversion
@@ -18,8 +21,11 @@ module ActiveModel
   #   cm.to_model == self # => true
   #   cm.to_key           # => nil
   #   cm.to_param         # => nil
+  #   cm.to_path          # => "contact_messages/contact_message"
   #
   module Conversion
+    extend ActiveSupport::Concern
+
     # If your object is already designed to implement all of the Active Model
     # you can use the default <tt>:to_model</tt> implementation, which simply
     # returns self.
@@ -45,5 +51,21 @@ def to_key
     def to_param
       persisted? ? to_key.join('-') : nil
     end
+
+    # Returns a string identifying the path associated with the object.
+    # ActionPack uses this to find a suitable partial to represent the object.
+    def to_path
+      self.class.to_path
+    end
+
+    module ClassMethods
+      def to_path
+        @_to_path ||= begin
+          element = ActiveSupport::Inflector.underscore(ActiveSupport::Inflector.demodulize(self))
+          collection = ActiveSupport::Inflector.tableize(self)
+          "#{collection}/#{element}".freeze
+        end
+      end
+    end
   end
 end

activemodel/lib/active_model/lint.rb

@@ -43,6 +43,16 @@ def model.persisted?() false end
         assert model.to_param.nil?, "to_param should return nil when `persisted?` returns false"
       end
 
+      # == Responds to <tt>to_path</tt>
+      #
+      # Returns a string giving a relative path.  This is used for looking up
+      # partials. For example, a BlogPost model might return "blog_posts/blog_post"
+      #
+      def test_to_path
+        assert model.respond_to?(:to_path), "The model should respond to to_path"
+        assert_kind_of String, model.to_path
+      end
+
       # == Responds to <tt>valid?</tt>
       #
       # Returns a boolean that specifies whether the object is in a valid or invalid
@@ -66,15 +76,14 @@ def test_persisted?
 
       # == Naming
       #
-      # Model.model_name must return a string with some convenience methods as
-      # :human and :partial_path. Check ActiveModel::Naming for more information.
+      # Model.model_name must return a string with some convenience methods:
+      # :human, :singular, and :plural. Check ActiveModel::Naming for more information.
       #
       def test_model_naming
         assert model.class.respond_to?(:model_name), "The model should respond to model_name"
         model_name = model.class.model_name
         assert_kind_of String, model_name
         assert_kind_of String, model_name.human
-        assert_kind_of String, model_name.partial_path
         assert_kind_of String, model_name.singular
         assert_kind_of String, model_name.plural
       end

activemodel/lib/active_model/naming.rb

 require 'active_support/inflector'
 require 'active_support/core_ext/hash/except'
 require 'active_support/core_ext/module/introspection'
+require 'active_support/core_ext/module/deprecation'
 
 module ActiveModel
   class Name < String
     attr_reader :singular, :plural, :element, :collection, :partial_path, :route_key, :param_key, :i18n_key
     alias_method :cache_key, :collection
 
+    deprecate :partial_path => "ActiveModel::Name#partial_path is deprecated. Call #to_path on model instances directly instead."
+
     def initialize(klass, namespace = nil, name = nil)
       name ||= klass.name
       super(name)

activemodel/test/cases/conversion_test.rb

 require 'cases/helper'
 require 'models/contact'
+require 'models/helicopter'
 
 class ConversionTest < ActiveModel::TestCase
   test "to_model default implementation returns self" do
@@ -22,4 +23,10 @@ class ConversionTest < ActiveModel::TestCase
   test "to_param default implementation returns a string of ids for persisted records" do
     assert_equal "1", Contact.new(:id => 1).to_param
   end
-end
\ No newline at end of file
+
+  test "to_path default implementation returns a string giving a relative path" do
+    assert_equal "contacts/contact", Contact.new.to_path
+    assert_equal "helicopters/helicopter", Helicopter.new.to_path,
+      "ActiveModel::Conversion#to_path caching should be class-specific"
+  end
+end

activemodel/test/cases/naming_test.rb

@@ -26,7 +26,9 @@ def test_collection
   end
 
   def test_partial_path
-    assert_equal 'post/track_backs/track_back', @model_name.partial_path
+    assert_deprecated(/#partial_path.*#to_path/) do
+      assert_equal 'post/track_backs/track_back', @model_name.partial_path
+    end
   end
 
   def test_human
@@ -56,7 +58,9 @@ def test_collection
   end
 
   def test_partial_path
-    assert_equal 'blog/posts/post', @model_name.partial_path
+    assert_deprecated(/#partial_path.*#to_path/) do
+      assert_equal 'blog/posts/post', @model_name.partial_path
+    end
   end
 
   def test_human
@@ -98,7 +102,9 @@ def test_collection
   end
 
   def test_partial_path
-    assert_equal 'blog/posts/post', @model_name.partial_path
+    assert_deprecated(/#partial_path.*#to_path/) do
+      assert_equal 'blog/posts/post', @model_name.partial_path
+    end
   end
 
   def test_human
@@ -136,7 +142,9 @@ def test_collection
   end
 
   def test_partial_path
-    assert_equal 'articles/article', @model_name.partial_path
+    assert_deprecated(/#partial_path.*#to_path/) do
+      assert_equal 'articles/article', @model_name.partial_path
+    end
   end
 
   def test_human

activemodel/test/models/helicopter.rb

+class Helicopter
+  include ActiveModel::Conversion
+end