Skip to content
This repository
Browse code

Make inheritance of map.resources :only/:except options behave more p…

…redictably

Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information...
commit 2ecec6052f7f290252a9fd9cc27ec804c7aad36c 1 parent 94d6716
Tom Stuart authored November 13, 2008 NZKoz committed November 14, 2008
33  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, :only, :except
  45
+    INHERITABLE_OPTIONS = :namespace, :shallow, :actions
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) || action_allowed?(action)
  122
+        !DEFAULT_ACTIONS.include?(action) || @options[:actions].nil? || @options[:actions].include?(action)
123 123
       end
124 124
 
125 125
       protected
@@ -135,29 +135,24 @@ def add_default_actions
135 135
         end
136 136
 
137 137
         def set_allowed_actions
138  
-          only, except = @options.values_at(:only, :except)
139  
-          @allowed_actions ||= {}
  138
+          only    = @options.delete(:only)
  139
+          except  = @options.delete(:except)
140 140
 
141  
-          if only == :all || except == :none
142  
-            only = nil
143  
-            except = []
  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
144 145
           elsif only == :none || except == :all
145  
-            only = []
146  
-            except = nil
147  
-          end
148  
-
149  
-          if only
150  
-            @allowed_actions[:only] = Array(only).map(&:to_sym)
  146
+            options[:actions] = []
  147
+          elsif only
  148
+            options[:actions] = DEFAULT_ACTIONS & Array(only).map(&:to_sym)
151 149
           elsif except
152  
-            @allowed_actions[:except] = Array(except).map(&:to_sym)
  150
+            options[:actions] = DEFAULT_ACTIONS - Array(except).map(&:to_sym)
  151
+          else
  152
+            # leave options[:actions] alone
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  
-
161 156
         def set_prefixes
162 157
           @path_prefix = options.delete(:path_prefix)
163 158
           @name_prefix = options.delete(:name_prefix)
26  actionpack/test/controller/resources_test.rb
@@ -971,6 +971,32 @@ def test_nested_resource_has_only_show_and_member_action
971 971
     end
972 972
   end
973 973
 
  974
+  def test_nested_resource_ignores_only_option
  975
+    with_routing do |set|
  976
+      set.draw do |map|
  977
+        map.resources :products, :only => :show do |product|
  978
+          product.resources :images, :except => :destroy
  979
+        end
  980
+      end
  981
+
  982
+      assert_resource_allowed_routes('images', { :product_id => '1' },                    { :id => '2' }, [:index, :new, :create, :show, :edit, :update], :destroy, '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_ignores_except_option
  988
+    with_routing do |set|
  989
+      set.draw do |map|
  990
+        map.resources :products, :except => :show do |product|
  991
+          product.resources :images, :only => :destroy
  992
+        end
  993
+      end
  994
+
  995
+      assert_resource_allowed_routes('images', { :product_id => '1' },                    { :id => '2' }, :destroy, [:index, :new, :create, :show, :edit, :update], 'products/1/images')
  996
+      assert_resource_allowed_routes('images', { :product_id => '1', :format => 'xml' },  { :id => '2' }, :destroy, [:index, :new, :create, :show, :edit, :update], 'products/1/images')
  997
+    end
  998
+  end
  999
+
974 1000
   protected
975 1001
     def with_restful_routing(*args)
976 1002
       with_routing do |set|

1 note on commit 2ecec60

Vitaly Omelchenko

ыба!

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