diff options
| -rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/filters.rb | 13 | ||||
| -rw-r--r-- | actionpack/test/controller/filters_test.rb | 23 |
3 files changed, 37 insertions, 1 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index da981e6669..0dd7f3c9e0 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Honor skipping filters conditionally for only certain actions even when the parent class sets that filter to conditionally be executed only for the same actions. #4522 [Marcel Molina Jr.] + * Delegate xml_http_request in integration tests to the session instance. [Jamis Buck] * Update the diagnostics template skip the useless '<controller not set>' text. [Nicholas Seckar] diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb index 2b260b80d7..1c957e7120 100644 --- a/actionpack/lib/action_controller/filters.rb +++ b/actionpack/lib/action_controller/filters.rb @@ -255,6 +255,7 @@ module ActionController #:nodoc: # just like when you apply the filters. def skip_before_filter(*filters) if conditions = extract_conditions!(filters) + remove_contradicting_conditions!(filters, conditions) conditions[:only], conditions[:except] = conditions[:except], conditions[:only] add_action_conditions(filters, conditions) else @@ -272,6 +273,7 @@ module ActionController #:nodoc: # just like when you apply the filters. def skip_after_filter(*filters) if conditions = extract_conditions!(filters) + remove_contradicting_conditions!(filters, conditions) conditions[:only], conditions[:except] = conditions[:except], conditions[:only] add_action_conditions(filters, conditions) else @@ -332,6 +334,17 @@ module ActionController #:nodoc: def condition_hash(filters, *actions) filters.inject({}) {|hash, filter| hash.merge(filter => actions.flatten.map {|action| action.to_s})} end + + def remove_contradicting_conditions!(filters, conditions) + return unless conditions[:only] + filters.each do |filter| + next unless included_actions_for_filter = included_actions[filter] + [*conditions[:only]].each do |conditional_action| + conditional_action = conditional_action.to_s + included_actions_for_filter.delete(conditional_action) if included_actions_for_filter.include?(conditional_action) + end + end + end end module InstanceMethods # :nodoc: diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 59bff76536..2459236002 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -126,7 +126,23 @@ class FilterTest < Test::Unit::TestCase render :inline => "ran action" end end - + + class ConditionalParentOfConditionalSkippingController < ConditionalFilterController + before_filter :conditional_in_parent, :only => [:show, :another_action] + after_filter :conditional_in_parent, :only => [:show, :another_action] + + private + + def conditional_in_parent + @ran_filter ||= [] + @ran_filter << 'conditional_in_parent' + end + end + + class ChildOfConditionalParentController < ConditionalParentOfConditionalSkippingController + skip_before_filter :conditional_in_parent, :only => :another_action + skip_after_filter :conditional_in_parent, :only => :another_action + end class ProcController < PrependingController before_filter(proc { |c| c.assigns["ran_proc_filter"] = true }) @@ -372,6 +388,11 @@ class FilterTest < Test::Unit::TestCase assert_equal %w( clean_up ), test_process(ConditionalSkippingController, "change_password").template.controller.instance_variable_get("@ran_after_filter") end + def test_conditional_skipping_of_filters_when_parent_filter_is_also_conditional + assert_equal %w( conditional_in_parent conditional_in_parent ), test_process(ChildOfConditionalParentController).template.assigns['ran_filter'] + assert_nil test_process(ChildOfConditionalParentController, 'another_action').template.assigns['ran_filter'] + end + private def test_process(controller, action = "show") request = ActionController::TestRequest.new |