Honor skipping filters conditionally for only certain actions even wh…

…en the parent class sets that filter to conditionally be executed only for the same actions. Closes #4522.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4167 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

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]

actionpack/lib/action_controller/filters.rb

@@ -255,6 +255,7 @@ def prepend_around_filter(*filters)
       # 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 @@ def skip_before_filter(*filters)
       # 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 @@ def add_action_conditions(filters, conditions)
         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:

actionpack/test/controller/filters_test.rb

@@ -126,7 +126,23 @@ def change_password
       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 @@ def test_conditional_skipping_of_filters
     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