From d045099f6691b1e409ad24160a17d8b618d09a5a Mon Sep 17 00:00:00 2001 From: Tom Stuart Date: Thu, 13 Nov 2008 20:00:11 +0000 Subject: [PATCH] Make inheritance of map.resources :only/:except options behave more predictably --- actionpack/lib/action_controller/resources.rb | 33 ++++++++++-------------- actionpack/test/controller/resources_test.rb | 26 +++++++++++++++++++ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/actionpack/lib/action_controller/resources.rb b/actionpack/lib/action_controller/resources.rb index d6cc4aa..7700b9d 100644 --- a/actionpack/lib/action_controller/resources.rb +++ b/actionpack/lib/action_controller/resources.rb @@ -42,7 +42,7 @@ module ActionController # # Read more about REST at http://en.wikipedia.org/wiki/Representational_State_Transfer module Resources - INHERITABLE_OPTIONS = :namespace, :shallow, :only, :except + INHERITABLE_OPTIONS = :namespace, :shallow, :actions class Resource #:nodoc: DEFAULT_ACTIONS = :index, :create, :new, :edit, :show, :update, :destroy @@ -119,7 +119,7 @@ module ActionController end def has_action?(action) - !DEFAULT_ACTIONS.include?(action) || action_allowed?(action) + !DEFAULT_ACTIONS.include?(action) || @options[:actions].nil? || @options[:actions].include?(action) end protected @@ -135,29 +135,24 @@ module ActionController end def set_allowed_actions - only, except = @options.values_at(:only, :except) - @allowed_actions ||= {} + only = @options.delete(:only) + except = @options.delete(:except) - if only == :all || except == :none - only = nil - except = [] + if only && except + raise ArgumentError, 'Please supply either :only or :except, not both.' + elsif only == :all || except == :none + options[:actions] = DEFAULT_ACTIONS elsif only == :none || except == :all - only = [] - except = nil - end - - if only - @allowed_actions[:only] = Array(only).map(&:to_sym) + options[:actions] = [] + elsif only + options[:actions] = DEFAULT_ACTIONS & Array(only).map(&:to_sym) elsif except - @allowed_actions[:except] = Array(except).map(&:to_sym) + options[:actions] = DEFAULT_ACTIONS - Array(except).map(&:to_sym) + else + # leave options[:actions] alone end end - def action_allowed?(action) - only, except = @allowed_actions.values_at(:only, :except) - (!only || only.include?(action)) && (!except || !except.include?(action)) - end - def set_prefixes @path_prefix = options.delete(:path_prefix) @name_prefix = options.delete(:name_prefix) diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 1f1f7b8..04f7a0a 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -971,6 +971,32 @@ class ResourcesTest < Test::Unit::TestCase end end + def test_nested_resource_ignores_only_option + with_routing do |set| + set.draw do |map| + map.resources :products, :only => :show do |product| + product.resources :images, :except => :destroy + end + end + + assert_resource_allowed_routes('images', { :product_id => '1' }, { :id => '2' }, [:index, :new, :create, :show, :edit, :update], :destroy, 'products/1/images') + assert_resource_allowed_routes('images', { :product_id => '1', :format => 'xml' }, { :id => '2' }, [:index, :new, :create, :show, :edit, :update], :destroy, 'products/1/images') + end + end + + def test_nested_resource_ignores_except_option + with_routing do |set| + set.draw do |map| + map.resources :products, :except => :show do |product| + product.resources :images, :only => :destroy + end + end + + assert_resource_allowed_routes('images', { :product_id => '1' }, { :id => '2' }, :destroy, [:index, :new, :create, :show, :edit, :update], 'products/1/images') + assert_resource_allowed_routes('images', { :product_id => '1', :format => 'xml' }, { :id => '2' }, :destroy, [:index, :new, :create, :show, :edit, :update], 'products/1/images') + end + end + protected def with_restful_routing(*args) with_routing do |set| -- 1.6.0.2