Permalink
Browse files

Let ActiveModel instances define partial paths.

Deprecate ActiveModel::Name#partial_path. Now you
should call #to_path directly on ActiveModel
instances.
  • Loading branch information...
1 parent a636a93 commit 6e671a8536f17b6f23b5251652015ce8c1557f1f Grant Hutchins & Peter Jaros committed with fxn Jul 8, 2011
@@ -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
@@ -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
@@ -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
@@ -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" })
@@ -1,9 +1,12 @@
+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
@@ -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
@@ -1,12 +1,15 @@
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)
@@ -1,5 +1,6 @@
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
+
+ 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
@@ -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
@@ -0,0 +1,3 @@
+class Helicopter
+ include ActiveModel::Conversion
+end

0 comments on commit 6e671a8

Please sign in to comment.