Changeset 7138

Timestamp:

06/27/07 08:38:55 (2 years ago)

Author:

bitsweat

Message:

Prefix nested resource named routes with their action name, e.g. new_group_user_path(@group) instead of group_new_user_path(@group). The old nested action named route is deprecated in Rails 1.2.4. Closes #8558.

Files:

• trunk/actionpack/CHANGELOG (modified) (1 diff)
• trunk/actionpack/lib/action_controller/resources.rb (modified) (3 diffs)
• trunk/actionpack/lib/action_controller/routing.rb (modified) (6 diffs)
• trunk/actionpack/test/controller/resources_test.rb (modified) (9 diffs)
• trunk/actionpack/test/controller/routing_test.rb (modified) (1 diff)

trunk/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Prefix nested resource named routes with their action name, e.g. new_group_user_path(@group) instead of group_new_user_path(@group). The old nested action named route is deprecated in Rails 1.2.4.  #8558 [David Chelimsky]
-
-* Allow sweepers to be created solely for expiring after controller actions, not model changes [DHH]

trunk/actionpack/lib/action_controller/resources.rb

@@ -427 +427 @@
-          actions.each do |action|
-            action_options = action_options_for(action, resource, method)
-
-
+
+
+
+
-          end
-        end
@@ -454 +456 @@
-            action_options = action_options_for(action, resource, method)
-            if action == :new
-
-
+
+
+
-            else
-
-
+
+
+
-            end
-          end
@@ -468 +472 @@
-          actions.each do |action|
-            action_options = action_options_for(action, resource, method)
-
-
+
+
+
-          end
-        end

trunk/actionpack/lib/action_controller/routing.rb

@@ -1021 +1021 @@
-        end
-        
+        def deprecated_named_route(name, deprecated_name, path, options = {})
+          named_route(name, path, options)
+          @set.add_deprecated_named_route(name, deprecated_name, path, options) unless deprecated_name == name
+        end
+        
-        # Enables the use of resources in a module by setting the name_prefix, path_prefix, and namespace for the model.
-        # Example:
@@ -1053 +1058 @@
-        include Enumerable
-
-
+, :deprecated_named_routes
-
-        def initialize
@@ -1062 +1067 @@
-          @routes = {}
-          @helpers = []
+          @deprecated_named_routes = {}
-          
-          @module ||= Module.new
@@ -1075 +1081 @@
-
-        def get(name)
+          if @deprecated_named_routes.has_key?(name.to_sym)
+            ActiveSupport::Deprecation.warn(
+              "The named route \"#{name}\" uses a format that has been deprecated. " +
+              "You should use \"#{@deprecated_named_routes[name]}\" instead", caller
+            )
+          end
-          routes[name.to_sym]
-        end
@@ -1170 +1182 @@
-        self.named_routes = NamedRouteCollection.new
-      end
-
+
-      # Subclasses and plugins may override this method to specify a different
-      # RouteBuilder instance, so that other route DSL's can be created.
@@ -1223 +1235 @@
-  
-      def add_named_route(name, path, options = {})
+        # TODO - is options EVER used?
-        name = options[:name_prefix] + name.to_s if options[:name_prefix]
-        named_routes[name.to_sym] = add_route(path, options)
+      end
+  
+      def add_deprecated_named_route(name, deprecated_name, path, options = {})
+        add_named_route(deprecated_name, path, options)
+        named_routes.deprecated_named_routes[deprecated_name.to_sym] = name
-      end
-  

trunk/actionpack/test/controller/resources_test.rb

@@ -101 +101 @@
-  end
-
-
+p
-    with_restful_routing :messages, :comments, :path_prefix => '/thread/:thread_id' do
-      assert_simply_restful_for :messages, :path_prefix => 'thread/5/', :options => { :thread_id => '5' }
@@ -114 +114 @@
-  end
-
-
-
-
-
-
-
+
+
+
+
-      assert_restful_routes_for :messages do |options|
-        assert_routing rss_path, options.merge(rss_options)
-
-        actions.each do |action, method|
-          assert_recognizes(options.merge(:action => action), :path => "/messages/#{action}", :method => method)
@@ -129 +125 @@
-
-      assert_restful_named_routes_for :messages do |options|
-        assert_named_route rss_path, :rss_messages_path, rss_options
-        actions.keys.each do |action|
-          assert_named_route "/messages/#{action}", "#{action}_messages_path", :action => action
+        end
+      end
+    end
+  end
+
+  def test_with_collection_actions_and_name_prefix
+    actions = { 'a' => :get, 'b' => :put, 'c' => :post, 'd' => :delete }
+  
+    with_restful_routing :messages, :path_prefix => '/threads/:thread_id', :name_prefix => "thread_", :collection => actions do
+      assert_restful_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options|
+        actions.each do |action, method|
+          assert_recognizes(options.merge(:action => action), :path => "/threads/1/messages/#{action}", :method => method)
+        end
+      end
+  
+      assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options|
+        actions.keys.each do |action|
+          assert_deprecated_named_route /thread_#{action}_messages/, "/threads/1/messages/#{action}", "thread_#{action}_messages_path", :action => action
+          assert_named_route "/threads/1/messages/#{action}", "#{action}_thread_messages_path", :action => action
+        end
+      end
+    end
+  end
+
+  def test_with_collection_action_and_name_prefix_and_formatted
+    actions = { 'a' => :get, 'b' => :put, 'c' => :post, 'd' => :delete }
+  
+    with_restful_routing :messages, :path_prefix => '/threads/:thread_id', :name_prefix => "thread_", :collection => actions do
+      assert_restful_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options|
+        actions.each do |action, method|
+          assert_recognizes(options.merge(:action => action, :format => 'xml'), :path => "/threads/1/messages/#{action}.xml", :method => method)
+        end
+      end
+  
+      assert_restful_named_routes_for :messages, :path_prefix => 'threads/1/', :name_prefix => 'thread_', :options => { :thread_id => '1' } do |options|
+        actions.keys.each do |action|
+          assert_deprecated_named_route /formatted_thread_#{action}_messages/, "/threads/1/messages/#{action}.xml", "formatted_thread_#{action}_messages_path", :action => action, :format => 'xml'
+          assert_named_route "/threads/1/messages/#{action}.xml", "formatted_#{action}_thread_messages_path", :action => action, :format => 'xml'
-        end
-      end
@@ -184 +217 @@
-    end
-  end
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-  def test_override_new_method
-    with_restful_routing :messages do
@@ -245 +308 @@
-  end
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-
-  def test_should_create_singleton_resource_routes
@@ -542 +613 @@
-      full_prefix = "/#{options[:path_prefix]}#{controller_name}"
-      name_prefix = options[:name_prefix]
-
+
-      assert_named_route "#{full_prefix}",            "#{name_prefix}#{controller_name}_path",              options[:options]
-/new",        "#{name_prefix}new_#{singular_name}_path",            options[:options]
+.xml",        "formatted_#{name_prefix}#{controller_name}_path",    options[:options].merge(            :format => 'xml')
-      assert_named_route "#{full_prefix}/1",          "#{name_prefix}#{singular_name}_path",                options[:options].merge(:id => '1')
-      assert_named_route "#{full_prefix}/1/edit",     "#{name_prefix}edit_#{singular_name}_path",           options[:options].merge(:id => '1')
-      assert_named_route "#{full_prefix}.xml",        "formatted_#{name_prefix}#{controller_name}_path",    options[:options].merge(            :format => 'xml')
-      assert_named_route "#{full_prefix}/new.xml",    "formatted_#{name_prefix}new_#{singular_name}_path",  options[:options].merge(            :format => 'xml')
-      assert_named_route "#{full_prefix}/1.xml",      "formatted_#{name_prefix}#{singular_name}_path",      options[:options].merge(:id => '1', :format => 'xml')
-
+
+
+
+
+
+
+
+
+
+
+
-      yield options[:options] if block_given?
-    end
-
+
-    def assert_singleton_routes_for(singleton_name, options = {})
-      options[:options] ||= {}
@@ -601 +679 @@
-
-      assert_named_route "#{full_path}",          "#{name_prefix}#{singleton_name}_path",                options[:options]
-      assert_named_route "#{full_path}/new",      "#{name_prefix}new_#{singleton_name}_path",            options[:options]
-      assert_named_route "#{full_path}/edit",     "#{name_prefix}edit_#{singleton_name}_path",           options[:options]
-      assert_named_route "#{full_path}.xml",      "formatted_#{name_prefix}#{singleton_name}_path",      options[:options].merge(:format => 'xml')
-
-
+
+
+
+
+
+
+
+
+
+
-    end
-
@@ -611 +695 @@
-      actual =  @controller.send(route, options) rescue $!.class.name
-      assert_equal expected, actual, "Error on route: #{route}(#{options.inspect})"
+    end
+    
+    def assert_deprecated_named_route(message, expected, route, options)
+      assert_deprecated message do
+        assert_named_route expected, route, options
+      end
+    end
+
+    # These are only deprecated if they have a name_prefix.
+    # See http://dev.rubyonrails.org/ticket/8251
+    def assert_potentially_deprecated_named_route(name_prefix, message, expected, route, options)
+      if name_prefix
+        assert_deprecated_named_route(message, expected, route, options)
+      else
+        assert_named_route expected, route, options
+      end
-    end
-
@@ -629 +729 @@
-      true
-    end
-
-end

trunk/actionpack/test/controller/routing_test.rb

@@ -1025 +1025 @@
-end
-
+class MapperDeprecatedRouteTest < Test::Unit::TestCase
+  def setup
+    @set = mock("set")
+    @mapper = ActionController::Routing::RouteSet::Mapper.new(@set)    
+  end
+  
+  def test_should_add_new_and_deprecated_named_routes
+    @set.expects(:add_named_route).with("new", "path", {:a => "b"})
+    @set.expects(:add_deprecated_named_route).with("new", "old", "path", {:a => "b"})
+    @mapper.deprecated_named_route("new", "old", "path", {:a => "b"})
+  end
+  
+  def test_should_not_add_deprecated_named_route_if_both_are_the_same
+    @set.expects(:add_named_route).with("new", "path", {:a => "b"})
+    @set.expects(:add_deprecated_named_route).with("new", "new", "path", {:a => "b"}).never
+    @mapper.deprecated_named_route("new", "new", "path", {:a => "b"})
+  end
+end
+
+class RouteSetDeprecatedRouteTest < Test::Unit::TestCase
+  def setup
+    @set = ActionController::Routing::RouteSet.new
+  end
+
+  def test_should_add_deprecated_route
+    @set.expects(:add_named_route).with("old", "path", {:a => "b"})
+    @set.add_deprecated_named_route("new", "old", "path", {:a => "b"})
+  end
+  
+  def test_should_fire_deprecation_warning_on_access
+    @set.add_deprecated_named_route("new_outer_inner_path", "outer_new_inner_path", "/outers/:outer_id/inners/new", :controller => :inners)
+    ActiveSupport::Deprecation.expects(:warn)
+    @set.named_routes["outer_new_inner_path"]
+  end
+end
+
-end # uses_mocha
-