provide a more robust fix to prevent filter conditions from being del…

…eted

This was spurred on by #2832, and effectively replaces #2523.

TL;DR it was the filter form's problem, so the resource extension
shouldn't have been trying to compensate.

lib/active_admin/filters/forms.rb

@@ -65,12 +65,10 @@ def active_admin_filters_form_for(search, filters, options = {})
 
         form_for search, options do |f|
           filters.each do |attribute, opts|
-            should   = opts.delete(:if)     || proc{ true }
-            shouldnt = opts.delete(:unless) || proc{ false }
+            return if opts.key?(:if)     && !call_method_or_proc_on(self, opts[:if])
+            return if opts.key?(:unless) &&  call_method_or_proc_on(self, opts[:unless])
 
-            if call_method_or_proc_on(self, should) && !call_method_or_proc_on(self, shouldnt)
-              f.filter attribute, opts
-            end
+            f.filter attribute, opts.except(:if, :unless)
           end
 
           buttons = content_tag :div, :class => "buttons" do

lib/active_admin/filters/resource_extension.rb

@@ -17,10 +17,9 @@ def initialize(*)
       # Returns the filters for this resource. If filters are not enabled,
       # it will always return an empty array.
       #
-      # @return [Array] Filters that apply for this resource
+      # @return [Hash] Filters that apply for this resource
       def filters
-        return [] unless filters_enabled?
-        filter_lookup
+        filters_enabled? ? filter_lookup : {}
       end
 
       # Setter to enable / disable filters on this resource.
@@ -76,7 +75,7 @@ def reset_filters!
       # Collapses the waveform, if you will, of which filters should be displayed.
       # Removes filters and adds in default filters as desired.
       def filter_lookup
-        filters = @filters.try(:deep_dup) || {}
+        filters = @filters.try(:dup) || {}
 
         if filters.empty? || preserve_default_filters?
           default_filters.each do |f|
@@ -101,7 +100,7 @@ def default_association_filters
         if resource_class.respond_to?(:reflect_on_all_associations)
           poly, not_poly = resource_class.reflect_on_all_associations.partition{ |r| r.macro == :belongs_to && r.options[:polymorphic] }
 
-          # remove associations nested more than twice
+          # remove deeply nested associations
           not_poly.reject!{ |r| r.chain.length > 2 }
 
           filters = poly.map(&:foreign_type) + not_poly.map(&:name)

spec/unit/filters/filter_form_builder_spec.rb

@@ -370,27 +370,26 @@ def view.a_helper_method
   end
 
   describe "conditional display" do
-    context "with :if block" do
-      it "should be displayed if true" do
-        body = filter :body,   if: proc{true}
-        expect(body).to have_tag "input", attributes: {name: "q[body_contains]"}
-      end
-
-      it "should NOT be displayed if false" do
-        body = filter :author, if: proc{false}
-        expect(body).to_not have_tag "select", attributes: {name: "q[author_id_eq]"}
-      end
-    end
-
-    context "with :unless block" do
-      it "should be displayed if false" do
-        body = filter :created_at, unless: proc{false}
-        expect(body).to     have_tag "input", attributes: {name: "q[created_at_gteq]"}
-      end
-
-      it "should NOT be displayed if true" do
-        body = filter :updated_at, unless: proc{true}
-        expect(body).to_not have_tag "input", attributes: {name: "q[updated_at_gteq]"}
+    [:if, :unless].each do |verb|
+      should   = verb == :if ? "should" : "shouldn't"
+      if_true  = verb == :if ? :to      : :to_not
+      if_false = verb == :if ? :to_not  : :to
+      context "with #{verb.inspect} proc" do
+        it "#{should} be displayed if true" do
+          body = filter :body, verb => proc{ true }
+          expect(body).send if_true,  have_tag("input", attributes: {name: "q[body_contains]"})
+        end
+        it "#{should} be displayed if false" do
+          body = filter :body, verb => proc{ false }
+          expect(body).send if_false, have_tag("input", attributes: {name: "q[body_contains]"})
+        end
+        it "should still be hidden on the second render" do
+          filters = {body: { verb => proc{ verb == :unless }}}
+          2.times do
+            body = render_filter scope, filters
+            expect(body).not_to have_tag "input", attributes: {name: "q[body_contains]"}
+          end
+        end
       end
     end
   end

spec/unit/filters/resource_spec.rb

@@ -7,6 +7,10 @@
     namespace.register(Post)
   end
 
+  it "should return a Hash" do
+    expect(resource.filters).to be_a Hash
+  end
+
   it "should return the defaults if no filters are set" do
     expect(resource.filters.keys).to match_array([
       :author, :body, :category, :created_at, :published_at, :starred, :taggings, :title, :updated_at
@@ -80,16 +84,6 @@
       expect(resource.filters).to eq title: {three: :four}
     end
 
-    it "should keep specified options" do
-      resource.add_filter :title, one: :two
-
-      resource.filters.each do |attribute, opts|
-        opts.delete(:one)
-      end
-
-      expect(resource.filters).to eq title: {one: :two}
-    end
-
     it "should preserve default filters" do
       resource.preserve_default_filters!
       resource.add_filter :count, as: :string