Changeset 6062

Timestamp:

01/28/07 08:20:04 (2 years ago)

Author:

bitsweat

Message:

Resource member routes require :id, eliminating the ambiguous overlap with collection routes. Closes #7229.

Files:

• trunk/actionpack/CHANGELOG (modified) (1 diff)
• trunk/actionpack/lib/action_controller/resources.rb (modified) (2 diffs)
• trunk/actionpack/test/controller/resources_test.rb (modified) (7 diffs)

trunk/actionpack/CHANGELOG

@@ -1 +1 @@
-*SVN*
+
+* Resource member routes require :id, eliminating the ambiguous overlap with collection routes.  #7229 [dkubb]
-
-* Remove deprecated assertions.  [Jeremy Kemper]

trunk/actionpack/lib/action_controller/resources.rb

@@ -323 +323 @@
-      def map_collection_actions(map, resource)
-        resource.collection_methods.each do |method, actions|
-          route_options = requirements_for(method)
-
-          actions.each do |action|
-
-
-
-
-
-
-
-
-
-
-
+
+
+
-          end
-        end
@@ -342 +332 @@
-
-      def map_default_collection_actions(map, resource)
-
-
-
-
-
+
+
+
+
+
+
+
-      end
-
-      def map_default_singleton_actions(map, resource)
-
-
+
+
+
-      end
-
-      def map_new_actions(map, resource)
-        resource.new_methods.each do |method, actions|
-          route_options = requirements_for(method)
-          actions.each do |action|
+            action_options = action_options_for(action, resource, method)
-            if action == :new
-route_options.merge(:action => "new")
-route_options.merge(:action => "new")
+action_options
+action_options
-            else
-route_options.merge(:action => action.to_s)
-route_options.merge(:action => action.to_s)
+action_options
+action_options
-            end
-          end
-        end
-      end
-
+
-      def map_member_actions(map, resource)
-        resource.member_methods.each do |method, actions|
-          route_options = requirements_for(method)
-
-          actions.each do |action|
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-      end
-  end

trunk/actionpack/test/controller/resources_test.rb

@@ -26 +26 @@
-    assert_resource_methods [:upload, :fix],          resource, :member,     :post
-    assert_resource_methods [:new, :preview, :draft], resource, :new,        :get
+  end
+
+  def test_should_resource_controller_name_equal_resource_name_by_default
+    resource = ActionController::Resources::Resource.new(:messages, {})
+    assert_equal 'messages', resource.controller
+  end
+
+  def test_should_resource_controller_name_equal_controller_option
+    resource = ActionController::Resources::Resource.new(:messages, :controller => 'posts')
+    assert_equal 'posts', resource.controller
+  end
+
+  def test_should_all_singleton_paths_be_the_same
+    [ :path, :nesting_path_prefix, :member_path ].each do |method|
+      resource = ActionController::Resources::SingletonResource.new(:messages, :path_prefix => 'admin')
+      assert_equal 'admin/messages', resource.send(method)
+    end
-  end
-
@@ -117 +134 @@
-  end
-
-
-  def test_with_new_action
-    with_restful_routing :messages, :new => { :preview => :post } do
@@ -281 +297 @@
-  end
-
+  def test_should_not_allow_delete_or_put_on_collection_path
+    controller_name = :messages
+    with_restful_routing controller_name do
+      options = { :controller => controller_name.to_s }
+      collection_path = "/#{controller_name}"
+
+      assert_raises(ActionController::RoutingError) do
+        assert_recognizes(options.merge(:action => 'update'), :path => collection_path, :method => :put)
+      end
+
+      assert_raises(ActionController::RoutingError) do
+        assert_recognizes(options.merge(:action => 'destroy'), :path => collection_path, :method => :delete)
+      end
+    end
+  end
+
-  protected
-    def with_restful_routing(*args)
@@ -310 +342 @@
-      (options[:options] ||= {})[:controller] = controller_name.to_s
-
-
-
-
+
+
+
+
+
-
-      with_options(options[:options]) do |controller|
-        controller.assert_routing collection_path,            :action => 'index'
-        controller.assert_routing "#{collection_path}.xml" ,  :action => 'index', :format => 'xml'
-        controller.assert_routing new_path,                   :action => 'new'
-        controller.assert_routing member_path,                :action => 'show', :id => '1'
-
+
+
+
-        controller.assert_routing "#{member_path}.xml",       :action => 'show', :id => '1', :format => 'xml'
-
-
-
-
-
-
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
-
-      yield options[:options] if block_given?
@@ -355 +395 @@
-      name_prefix = options[:name_prefix]
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
-      yield options[:options] if block_given?
-    end
@@ -366 +408 @@
-    def assert_singleton_routes_for(singleton_name, options = {})
-      (options[:options] ||= {})[:controller] ||= singleton_name.to_s
-
-
+
+
+
+
+
+
-      with_options options[:options] do |controller|
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
+
+
-      assert_recognizes(options[:options].merge(:action => 'create'),  :path => full_path, :method => :post)
-      assert_recognizes(options[:options].merge(:action => 'update'),  :path => full_path, :method => :put)
-      assert_recognizes(options[:options].merge(:action => 'destroy'), :path => full_path, :method => :delete)
+
+      assert_recognizes(options[:options].merge(:action => 'show',    :format => 'xml'), :path => "#{full_path}.xml",  :method => :get)
+      assert_recognizes(options[:options].merge(:action => 'new',     :format => 'xml'), :path => "#{new_path}.xml",   :method => :get)
+      assert_recognizes(options[:options].merge(:action => 'edit',    :format => 'xml'), :path => formatted_edit_path, :method => :get)
+      assert_recognizes(options[:options].merge(:action => 'create',  :format => 'xml'), :path => "#{full_path}.xml",  :method => :post)
+      assert_recognizes(options[:options].merge(:action => 'update',  :format => 'xml'), :path => "#{full_path}.xml",  :method => :put)
+      assert_recognizes(options[:options].merge(:action => 'destroy', :format => 'xml'), :path => "#{full_path}.xml",  :method => :delete)
-
-      yield options[:options] if block_given?
@@ -389 +447 @@
-      get :show, options[:options]
-      options[:options].delete :action
-
+
-      full_path = "/#{options[:path_prefix]}#{singleton_name}"
-
-
-
-
-
+
+
+
+
+
+
+
-    end
-