Skip to content
This repository
Browse code

Removed map.resources :only/:except inheritance

It's very rare for these options to apply identically to nested child resources, and with this inheritance on it's very difficult to have a child resource with more actions than the parent.

This reverts commit 2ecec60.

Signed-off-by: Michael Koziarski <michael@koziarski.com>

[#1826 state:committed]
  • Loading branch information...
commit 80747e9db16ec60eb0d95b510baf051612ec0768 1 parent ec8f045
Tarmo Tänav authored January 30, 2009 NZKoz committed February 01, 2009
35  actionpack/lib/action_controller/resources.rb
@@ -42,7 +42,7 @@ module ActionController
42 42
   #
43 43
   # Read more about REST at http://en.wikipedia.org/wiki/Representational_State_Transfer
44 44
   module Resources
45  
-    INHERITABLE_OPTIONS = :namespace, :shallow, :actions
  45
+    INHERITABLE_OPTIONS = :namespace, :shallow
46 46
 
47 47
     class Resource #:nodoc:
48 48
       DEFAULT_ACTIONS = :index, :create, :new, :edit, :show, :update, :destroy
@@ -119,7 +119,7 @@ def uncountable?
119 119
       end
120 120
 
121 121
       def has_action?(action)
122  
-        !DEFAULT_ACTIONS.include?(action) || @options[:actions].nil? || @options[:actions].include?(action)
  122
+        !DEFAULT_ACTIONS.include?(action) || action_allowed?(action)
123 123
       end
124 124
 
125 125
       protected
@@ -135,24 +135,29 @@ def add_default_actions
135 135
         end
136 136
 
137 137
         def set_allowed_actions
138  
-          only    = @options.delete(:only)
139  
-          except  = @options.delete(:except)
  138
+          only, except = @options.values_at(:only, :except)
  139
+          @allowed_actions ||= {}
140 140
 
141  
-          if only && except
142  
-            raise ArgumentError, 'Please supply either :only or :except, not both.'
143  
-          elsif only == :all || except == :none
144  
-            options[:actions] = DEFAULT_ACTIONS
  141
+          if only == :all || except == :none
  142
+            only = nil
  143
+            except = []
145 144
           elsif only == :none || except == :all
146  
-            options[:actions] = []
147  
-          elsif only
148  
-            options[:actions] = DEFAULT_ACTIONS & Array(only).map(&:to_sym)
  145
+            only = []
  146
+            except = nil
  147
+          end
  148
+
  149
+          if only
  150
+            @allowed_actions[:only] = Array(only).map(&:to_sym)
149 151
           elsif except
150  
-            options[:actions] = DEFAULT_ACTIONS - Array(except).map(&:to_sym)
151  
-          else
152  
-            # leave options[:actions] alone
  152
+            @allowed_actions[:except] = Array(except).map(&:to_sym)
153 153
           end
154 154
         end
155 155
 
  156
+        def action_allowed?(action)
  157
+          only, except = @allowed_actions.values_at(:only, :except)
  158
+          (!only || only.include?(action)) && (!except || !except.include?(action))
  159
+        end
  160
+
156 161
         def set_prefixes
157 162
           @path_prefix = options.delete(:path_prefix)
158 163
           @name_prefix = options.delete(:name_prefix)
@@ -403,8 +408,6 @@ def initialize(entity, options)
403 408
     #   # --> POST /posts/1/comments (maps to the CommentsController#create action)
404 409
     #   # --> PUT /posts/1/comments/1 (fails)
405 410
     #
406  
-    # The <tt>:only</tt> and <tt>:except</tt> options are inherited by any nested resource(s).
407  
-    #
408 411
     # If <tt>map.resources</tt> is called with multiple resources, they all get the same options applied.
409 412
     #
410 413
     # Examples:
43  actionpack/test/controller/resources_test.rb
@@ -942,19 +942,6 @@ def test_singleton_resource_has_only_member_action
942 942
     end
943 943
   end
944 944
 
945  
-  def test_nested_resource_inherits_only_show_action
946  
-    with_routing do |set|
947  
-      set.draw do |map|
948  
-        map.resources :products, :only => :show do |product|
949  
-          product.resources :images
950  
-        end
951  
-      end
952  
-
953  
-      assert_resource_allowed_routes('images', { :product_id => '1' },                    { :id => '2' }, :show, [:index, :new, :create, :edit, :update, :destroy], 'products/1/images')
954  
-      assert_resource_allowed_routes('images', { :product_id => '1', :format => 'xml' },  { :id => '2' }, :show, [:index, :new, :create, :edit, :update, :destroy], 'products/1/images')
955  
-    end
956  
-  end
957  
-
958 945
   def test_nested_resource_has_only_show_and_member_action
959 946
     with_routing do |set|
960 947
       set.draw do |map|
@@ -971,7 +958,7 @@ def test_nested_resource_has_only_show_and_member_action
971 958
     end
972 959
   end
973 960
 
974  
-  def test_nested_resource_ignores_only_option
  961
+  def test_nested_resource_does_not_inherit_only_option
975 962
     with_routing do |set|
976 963
       set.draw do |map|
977 964
         map.resources :products, :only => :show do |product|
@@ -984,7 +971,20 @@ def test_nested_resource_ignores_only_option
984 971
     end
985 972
   end
986 973
 
987  
-  def test_nested_resource_ignores_except_option
  974
+  def test_nested_resource_does_not_inherit_only_option_by_default
  975
+    with_routing do |set|
  976
+      set.draw do |map|
  977
+        map.resources :products, :only => :show do |product|
  978
+          product.resources :images
  979
+        end
  980
+      end
  981
+
  982
+      assert_resource_allowed_routes('images', { :product_id => '1' },                    { :id => '2' }, [:index, :new, :create, :show, :edit, :update, :destory], [], 'products/1/images')
  983
+      assert_resource_allowed_routes('images', { :product_id => '1', :format => 'xml' },  { :id => '2' }, [:index, :new, :create, :show, :edit, :update, :destroy], [], 'products/1/images')
  984
+    end
  985
+  end
  986
+
  987
+  def test_nested_resource_does_not_inherit_except_option
988 988
     with_routing do |set|
989 989
       set.draw do |map|
990 990
         map.resources :products, :except => :show do |product|
@@ -997,6 +997,19 @@ def test_nested_resource_ignores_except_option
997 997
     end
998 998
   end
999 999
 
  1000
+  def test_nested_resource_does_not_inherit_except_option_by_default
  1001
+    with_routing do |set|
  1002
+      set.draw do |map|
  1003
+        map.resources :products, :except => :show do |product|
  1004
+          product.resources :images
  1005
+        end
  1006
+      end
  1007
+
  1008
+      assert_resource_allowed_routes('images', { :product_id => '1' },                    { :id => '2' }, [:index, :new, :create, :show, :edit, :update, :destroy], [], 'products/1/images')
  1009
+      assert_resource_allowed_routes('images', { :product_id => '1', :format => 'xml' },  { :id => '2' }, [:index, :new, :create, :show, :edit, :update, :destroy], [], 'products/1/images')
  1010
+    end
  1011
+  end
  1012
+
1000 1013
   def test_default_singleton_restful_route_uses_get
1001 1014
     with_routing do |set|
1002 1015
       set.draw do |map|

0 notes on commit 80747e9

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