Skip to content
This repository
Browse code

Let ActiveModel instances define partial paths.

Deprecate ActiveModel::Name#partial_path. Now you
should call #to_path directly on ActiveModel
instances.
  • Loading branch information...
commit bf812074fd55e7dcfa426d6c9bfd4d8d68922194 1 parent 8e00611
authored July 08, 2011
8  actionpack/lib/action_view/helpers/form_helper.rb
@@ -1227,8 +1227,12 @@ def multipart=(multipart)
1227 1227
         parent_builder.multipart = multipart if parent_builder
1228 1228
       end
1229 1229
 
1230  
-      def self.model_name
1231  
-        @model_name ||= Struct.new(:partial_path).new(name.demodulize.underscore.sub!(/_builder$/, ''))
  1230
+      def self.to_path
  1231
+        @_to_path ||= name.demodulize.underscore.sub!(/_builder$/, '')
  1232
+      end
  1233
+
  1234
+      def to_path
  1235
+        self.class.to_path
1232 1236
       end
1233 1237
 
1234 1238
       def to_model
43  actionpack/lib/action_view/renderer/partial_renderer.rb
@@ -206,13 +206,6 @@ module ActionView
206 206
   #     <%- end -%>
207 207
   #   <% end %>
208 208
   class PartialRenderer < AbstractRenderer #:nodoc:
209  
-    PARTIAL_NAMES = Hash.new {|h,k| h[k] = {} }
210  
-
211  
-    def initialize(*)
212  
-      super
213  
-      @partial_names = PARTIAL_NAMES[@lookup_context.prefixes.first]
214  
-    end
215  
-
216 209
     def render(context, options, block)
217 210
       setup(context, options, block)
218 211
 
@@ -359,28 +352,36 @@ def collection_without_template
359 352
       segments
360 353
     end
361 354
 
  355
+    PARTIAL_PATHS = {}
  356
+
362 357
     def partial_path(object = @object)
363  
-      @partial_names[object.class.name] ||= begin
364  
-        object = object.to_model if object.respond_to?(:to_model)
365  
-        object.class.model_name.partial_path.dup.tap do |partial|
366  
-          path = @lookup_context.prefixes.first
367  
-          merge_path_into_partial(path, partial)
368  
-        end
  358
+      object = object.to_model if object.respond_to?(:to_model)
  359
+
  360
+      path = if object.respond_to?(:to_path)
  361
+               object.to_path
  362
+             else
  363
+               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."
  364
+               object.class.model_name.partial_path
  365
+             end
  366
+
  367
+      prefix = @lookup_context.prefixes.first
  368
+      PARTIAL_PATHS[ [path, prefix] ] ||= path.dup.tap do |object_path|
  369
+        merge_prefix_into_object_path(prefix, object_path)
369 370
       end
370 371
     end
371 372
 
372  
-    def merge_path_into_partial(path, partial)
373  
-      if path.include?(?/) && partial.include?(?/)
  373
+    def merge_prefix_into_object_path(prefix, object_path)
  374
+      if prefix.include?(?/) && object_path.include?(?/)
374 375
         overlap = []
375  
-        path_array = File.dirname(path).split('/')
376  
-        partial_array = partial.split('/')[0..-3] # skip model dir & partial
  376
+        prefix_array = File.dirname(prefix).split('/')
  377
+        object_path_array = object_path.split('/')[0..-3] # skip model dir & partial
377 378
 
378  
-        path_array.each_with_index do |dir, index|
379  
-          overlap << dir if dir == partial_array[index]
  379
+        prefix_array.each_with_index do |dir, index|
  380
+          overlap << dir if dir == object_path_array[index]
380 381
         end
381 382
 
382  
-        partial.gsub!(/^#{overlap.join('/')}\//,'')
383  
-        partial.insert(0, "#{File.dirname(path)}/")
  383
+        object_path.gsub!(/^#{overlap.join('/')}\//,'')
  384
+        object_path.insert(0, "#{File.dirname(prefix)}/")
384 385
       end
385 386
     end
386 387
 
11  actionpack/test/template/form_helper_test.rb
@@ -1874,6 +1874,17 @@ def test_form_for_with_labelled_builder_with_nested_fields_for_with_options_hash
1874 1874
     assert_equal LabelledFormBuilder, klass
1875 1875
   end
1876 1876
 
  1877
+  def test_form_for_with_labelled_builder_path
  1878
+    path = nil
  1879
+
  1880
+    form_for(@post, :builder => LabelledFormBuilder) do |f|
  1881
+      path = f.to_path
  1882
+      ''
  1883
+    end
  1884
+
  1885
+    assert_equal 'labelled_form', path
  1886
+  end
  1887
+
1877 1888
   class LabelledFormBuilderSubclass < LabelledFormBuilder; end
1878 1889
 
1879 1890
   def test_form_for_with_labelled_builder_with_nested_fields_for_with_custom_builder
30  actionpack/test/template/render_test.rb
@@ -201,6 +201,36 @@ def test_render_partial_using_collection
201 201
       @controller_view.render(customers, :greeting => "Hello")
202 202
   end
203 203
 
  204
+  class CustomerWithDeprecatedPartialPath
  205
+    attr_reader :name
  206
+
  207
+    def self.model_name
  208
+      Struct.new(:partial_path).new("customers/customer")
  209
+    end
  210
+
  211
+    def initialize(name)
  212
+      @name = name
  213
+    end
  214
+  end
  215
+
  216
+  def test_render_partial_using_object_with_deprecated_partial_path
  217
+    assert_deprecated(/#model_name.*#partial_path.*#to_path/) do
  218
+      assert_equal "Hello: nertzy",
  219
+        @controller_view.render(CustomerWithDeprecatedPartialPath.new("nertzy"), :greeting => "Hello")
  220
+    end
  221
+  end
  222
+
  223
+  def test_render_partial_using_collection_with_deprecated_partial_path
  224
+    assert_deprecated(/#model_name.*#partial_path.*#to_path/) do
  225
+      customers = [
  226
+        CustomerWithDeprecatedPartialPath.new("nertzy"),
  227
+        CustomerWithDeprecatedPartialPath.new("peeja")
  228
+      ]
  229
+      assert_equal "Hello: nertzyHello: peeja",
  230
+        @controller_view.render(customers, :greeting => "Hello")
  231
+    end
  232
+  end
  233
+
204 234
   # TODO: The reason for this test is unclear, improve documentation
205 235
   def test_render_partial_and_fallback_to_layout
206 236
     assert_equal "Before (Josh)\n\nAfter", @view.render(:partial => "test/layout_for_partial", :locals => { :name => "Josh" })
26  activemodel/lib/active_model/conversion.rb
... ...
@@ -1,9 +1,12 @@
  1
+require 'active_support/concern'
  2
+require 'active_support/inflector'
  3
+
1 4
 module ActiveModel
2 5
   # == Active Model Conversions
3 6
   #
4  
-  # Handles default conversions: to_model, to_key and to_param.
  7
+  # Handles default conversions: to_model, to_key, to_param, and to_path.
5 8
   #
6  
-  # Let's take for example this non persisted object.
  9
+  # Let's take for example this non-persisted object.
7 10
   #
8 11
   #   class ContactMessage
9 12
   #     include ActiveModel::Conversion
@@ -18,8 +21,11 @@ module ActiveModel
18 21
   #   cm.to_model == self # => true
19 22
   #   cm.to_key           # => nil
20 23
   #   cm.to_param         # => nil
  24
+  #   cm.to_path          # => "contact_messages/contact_message"
21 25
   #
22 26
   module Conversion
  27
+    extend ActiveSupport::Concern
  28
+
23 29
     # If your object is already designed to implement all of the Active Model
24 30
     # you can use the default <tt>:to_model</tt> implementation, which simply
25 31
     # returns self.
@@ -45,5 +51,21 @@ def to_key
45 51
     def to_param
46 52
       persisted? ? to_key.join('-') : nil
47 53
     end
  54
+
  55
+    # Returns a string identifying the path associated with the object.
  56
+    # ActionPack uses this to find a suitable partial to represent the object.
  57
+    def to_path
  58
+      self.class.to_path
  59
+    end
  60
+
  61
+    module ClassMethods
  62
+      def to_path
  63
+        @_to_path ||= begin
  64
+          element = ActiveSupport::Inflector.underscore(ActiveSupport::Inflector.demodulize(self))
  65
+          collection = ActiveSupport::Inflector.tableize(self)
  66
+          "#{collection}/#{element}".freeze
  67
+        end
  68
+      end
  69
+    end
48 70
   end
49 71
 end
15  activemodel/lib/active_model/lint.rb
@@ -43,6 +43,16 @@ def model.persisted?() false end
43 43
         assert model.to_param.nil?, "to_param should return nil when `persisted?` returns false"
44 44
       end
45 45
 
  46
+      # == Responds to <tt>to_path</tt>
  47
+      #
  48
+      # Returns a string giving a relative path.  This is used for looking up
  49
+      # partials. For example, a BlogPost model might return "blog_posts/blog_post"
  50
+      #
  51
+      def test_to_path
  52
+        assert model.respond_to?(:to_path), "The model should respond to to_path"
  53
+        assert_kind_of String, model.to_path
  54
+      end
  55
+
46 56
       # == Responds to <tt>valid?</tt>
47 57
       #
48 58
       # Returns a boolean that specifies whether the object is in a valid or invalid
@@ -66,15 +76,14 @@ def test_persisted?
66 76
 
67 77
       # == Naming
68 78
       #
69  
-      # Model.model_name must return a string with some convenience methods as
70  
-      # :human and :partial_path. Check ActiveModel::Naming for more information.
  79
+      # Model.model_name must return a string with some convenience methods:
  80
+      # :human, :singular, and :plural. Check ActiveModel::Naming for more information.
71 81
       #
72 82
       def test_model_naming
73 83
         assert model.class.respond_to?(:model_name), "The model should respond to model_name"
74 84
         model_name = model.class.model_name
75 85
         assert_kind_of String, model_name
76 86
         assert_kind_of String, model_name.human
77  
-        assert_kind_of String, model_name.partial_path
78 87
         assert_kind_of String, model_name.singular
79 88
         assert_kind_of String, model_name.plural
80 89
       end
3  activemodel/lib/active_model/naming.rb
... ...
@@ -1,12 +1,15 @@
1 1
 require 'active_support/inflector'
2 2
 require 'active_support/core_ext/hash/except'
3 3
 require 'active_support/core_ext/module/introspection'
  4
+require 'active_support/core_ext/module/deprecation'
4 5
 
5 6
 module ActiveModel
6 7
   class Name < String
7 8
     attr_reader :singular, :plural, :element, :collection, :partial_path, :route_key, :param_key, :i18n_key
8 9
     alias_method :cache_key, :collection
9 10
 
  11
+    deprecate :partial_path => "ActiveModel::Name#partial_path is deprecated. Call #to_path on model instances directly instead."
  12
+
10 13
     def initialize(klass, namespace = nil, name = nil)
11 14
       name ||= klass.name
12 15
       super(name)
9  activemodel/test/cases/conversion_test.rb
... ...
@@ -1,5 +1,6 @@
1 1
 require 'cases/helper'
2 2
 require 'models/contact'
  3
+require 'models/helicopter'
3 4
 
4 5
 class ConversionTest < ActiveModel::TestCase
5 6
   test "to_model default implementation returns self" do
@@ -22,4 +23,10 @@ class ConversionTest < ActiveModel::TestCase
22 23
   test "to_param default implementation returns a string of ids for persisted records" do
23 24
     assert_equal "1", Contact.new(:id => 1).to_param
24 25
   end
25  
-end
  26
+
  27
+  test "to_path default implementation returns a string giving a relative path" do
  28
+    assert_equal "contacts/contact", Contact.new.to_path
  29
+    assert_equal "helicopters/helicopter", Helicopter.new.to_path,
  30
+      "ActiveModel::Conversion#to_path caching should be class-specific"
  31
+  end
  32
+end
16  activemodel/test/cases/naming_test.rb
@@ -26,7 +26,9 @@ def test_collection
26 26
   end
27 27
 
28 28
   def test_partial_path
29  
-    assert_equal 'post/track_backs/track_back', @model_name.partial_path
  29
+    assert_deprecated(/#partial_path.*#to_path/) do
  30
+      assert_equal 'post/track_backs/track_back', @model_name.partial_path
  31
+    end
30 32
   end
31 33
 
32 34
   def test_human
@@ -56,7 +58,9 @@ def test_collection
56 58
   end
57 59
 
58 60
   def test_partial_path
59  
-    assert_equal 'blog/posts/post', @model_name.partial_path
  61
+    assert_deprecated(/#partial_path.*#to_path/) do
  62
+      assert_equal 'blog/posts/post', @model_name.partial_path
  63
+    end
60 64
   end
61 65
 
62 66
   def test_human
@@ -98,7 +102,9 @@ def test_collection
98 102
   end
99 103
 
100 104
   def test_partial_path
101  
-    assert_equal 'blog/posts/post', @model_name.partial_path
  105
+    assert_deprecated(/#partial_path.*#to_path/) do
  106
+      assert_equal 'blog/posts/post', @model_name.partial_path
  107
+    end
102 108
   end
103 109
 
104 110
   def test_human
@@ -136,7 +142,9 @@ def test_collection
136 142
   end
137 143
 
138 144
   def test_partial_path
139  
-    assert_equal 'articles/article', @model_name.partial_path
  145
+    assert_deprecated(/#partial_path.*#to_path/) do
  146
+      assert_equal 'articles/article', @model_name.partial_path
  147
+    end
140 148
   end
141 149
 
142 150
   def test_human
3  activemodel/test/models/helicopter.rb
... ...
@@ -0,0 +1,3 @@
  1
+class Helicopter
  2
+  include ActiveModel::Conversion
  3
+end

1 note on commit bf81207

José Valim
Owner

Notice there is a later commit that renames to_path to to_partial_path: dc8773b

Please sign in to comment.
Something went wrong with that request. Please try again.