refactoring and improvements

README.md

@@ -24,21 +24,26 @@ Pre-1.0 and not on RubyGems yet: the option name and the shape of its value may
 
 ## Usage
 
-**A scalar** applies to the filter name as it stands, which is what a name that already carries
-its predicate needs:
+A default is a **value**. Which Ransack key it lands under is the input's business, and the
+input is asked - so a `:select` gets `_eq`, a `:check_boxes` gets `_in` under the association's
+primary key, and a `:string` gets whichever predicate heads its dropdown, including when the
+resource or the namespace has reordered that list:
 
 ```ruby
-filter :state_eq, as: :select, collection: %w[active archived], default: "active"
+filter :status,     as: :select,      default: "active"
+filter :author,     as: :check_boxes, default: [1, 2]
+filter :created_at, as: :date_range,  default: -> { 1.week.ago..Time.current }
+filter :created_at, as: :date_range,  default: -> { 1.week.ago.. }       # lower bound only
+filter :title,                        default: "acme"
 ```
 
-**A Hash** is keyed by predicate, for inputs that render more than one field. A `:date_range`
-renders a `gteq` and an `lteq` field, and a `:check_boxes` submits as `q[author_id_in][]`, so the
-filter name on its own does not identify a search key:
+**A Range** fills a two-ended input, one bound per end; leave an end off and that end is left to
+the admin. Handing a single value to a two-ended input raises, rather than picking an end for you.
 
-```ruby
-filter :created_at, as: :date_range,  default: { gteq: -> { 1.week.ago.to_date } }
-filter :author,     as: :check_boxes, default: { id_in: [1, 2] }
-```
+Anything relative to now belongs in a Proc, as above. `filter` runs when the resource file is
+loaded, so `default: 1.week.ago..` would pin the window to the moment the process booted and let
+it drift for as long as that process lives - quietly, and worst on the long-running ones. A Proc
+is re-read on every request.
 
 **A Proc** is evaluated against the controller on every request, and a `nil` result applies no
 filter, which is how a default is made conditional or read off the signed-in admin:
@@ -48,6 +53,28 @@ filter :author_id_eq, default: -> { current_admin_user.id unless current_admin_u
 filter :queue_eq,     default: -> { current_admin_user.default_queue }
 ```
 
+Asking the input means you get whatever that input actually submits, which is not always the
+predicate the docs of some other app would lead you to expect. A `:string` filter submits the
+head of its dropdown, and an app that re-registers Ransack's aliases - `contains`, `equals`,
+`starts_with` - reorders that list, so `default: "acme"` may search for equality rather than a
+substring. That is correct, since it is what an untouched form submits there, but check it
+rather than assume.
+
+Deriving needs a resource Ransack can search. A resource backed by something else - an
+ActiveResource model standing in for an HTTP API, say - has no `ransack`, and the derivation
+raises rather than guessing:
+
+    filter :created_at declares a `default:` but could not work out which search key it submits
+    (NoMethodError: undefined method 'ransack' ...) - name the predicate with a Hash instead
+
+**A Hash** names the predicates outright, for that case and for when the input's own predicate
+is not the one you want:
+
+```ruby
+filter :title, default: { eq: "acme" }                 # rather than the _cont it would submit
+filter :created_at, as: :date_range, default: { gteq: -> { 1.week.ago } }
+```
+
 A filter hidden by `:if` or `:unless` imposes no default either — it would filter the collection
 with no control on screen to undo it.
 

lib/active_admin_filters_defaults.rb

@@ -3,6 +3,7 @@
 require "activeadmin"
 require "active_admin_filters_defaults/version"
 require "active_admin_filters_defaults/data_access"
+require "active_admin_filters_defaults/filter_defaults"
 require "active_admin_filters_defaults/filters_form"
 
 module ActiveAdminFiltersDefaults
@@ -11,7 +12,11 @@ module ActiveAdminFiltersDefaults
 # `add_filter` stores whatever options it is given without a whitelist, so `default:` needs no
 # registration of its own.
 ActiveAdmin.before_load do |_app|
+  # Prepended, because it replaces Active Admin's own #apply_filtering.
   ActiveAdmin::ResourceController.prepend ActiveAdminFiltersDefaults::DataAccess
+  # Included, because it only adds - and a resource overriding one of its seams in a
+  # `controller do` block should win, which is what including gives.
+  ActiveAdmin::ResourceController.include ActiveAdminFiltersDefaults::FilterDefaults
   ActiveAdmin::ResourceController.helper_method :filter_default_values, :visible_filters
 
   ActiveAdmin::Resource.prepend ActiveAdminFiltersDefaults::ResourceExtension

lib/active_admin_filters_defaults/data_access.rb

@@ -1,8 +1,9 @@
 # frozen_string_literal: true
 
 module ActiveAdminFiltersDefaults
-  # Replaces ActiveAdmin::ResourceController::DataAccess#apply_filtering so that the collection
-  # is searched on #filtering_params rather than on `params[:q]` read directly.
+  # The single method this gem replaces: Active Admin searches the collection on `params[:q]`
+  # read directly, and it searches #filtering_params instead. Everything that method means is
+  # in FilterDefaults.
   #
   # The Ransack call is the Active Admin 3 one, which this gem targets: Active Admin 4 passes
   # `auth_object: active_admin_authorization` there as well.
@@ -13,84 +14,5 @@ def apply_filtering(chain)
       @search = chain.ransack(filtering_params)
       @search.result
     end
-
-    # The filter values the collection is searched with. Override to change what the index
-    # filters on without reaching into `params`.
-    #
-    # Whatever the request asked for wins over a default, so that an override of
-    # #filter_defaults_apply? which lets defaults through on a partly filtered request keeps
-    # the values that were actually asked for.
-    #
-    # @return [Hash, ActionController::Parameters] values passed to Ransack
-    def filtering_params
-      return params[:q] || {} unless filter_defaults_apply?
-
-      defaults = filter_default_values
-      return params[:q] || {} if defaults.blank?
-
-      requested = params[:q]
-      requested = requested.to_unsafe_h if requested.respond_to?(:to_unsafe_h)
-      defaults.merge(requested || {})
-    end
-
-    # Whether the request is one the declared defaults should apply to. Override to widen it -
-    # a resource that always carries its customer in `q`, say, still wants its defaults on the
-    # first visit:
-    #
-    #   def filter_defaults_apply?
-    #     super || params[:q].keys == %w[customer_id_eq]
-    #   end
-    #
-    # `commit` marks a submission of the filters form: submitting it with every field blank
-    # sends no `q` at all, because the form disables empty fields on submit, so `commit` is
-    # what tells "show me everything" apart from a first visit. Clearing the filters drops
-    # `commit` along with `q`, so Clear Filters returns the page to its declared defaults.
-    def filter_defaults_apply?
-      params[:q].blank? && params[:commit].blank?
-    end
-
-    # The filters of this resource that `:if` and `:unless` allow, which is both the set the
-    # filters form renders and the set that can impose a default - a filter that is not
-    # rendered does not filter the collection behind the admin's back.
-    #
-    # @return [Hash] filter attribute => filter options
-    def visible_filters
-      @visible_filters ||= active_admin_config.filters.reject do |_attribute, options|
-        (options.key?(:if) && !::MethodOrProcHelper.render_in_context(self, options[:if])) ||
-          (options.key?(:unless) && ::MethodOrProcHelper.render_in_context(self, options[:unless]))
-      end
-    end
-
-    # @return [Hash] the search values declared with `filter ..., default:`
-    def filter_default_values
-      @filter_default_values ||= visible_filters.each_with_object({}) do |(attribute, options), result|
-        next unless options.key?(:default)
-
-        add_filter_default_value(result, attribute, options[:default])
-      end
-    end
-
-    # A scalar default applies to the filter name as it stands, which is what filters whose
-    # name already carries a predicate need: `filter :status_eq, default: "active"`.
-    #
-    # A Hash is keyed by predicate, which is what an input rendering more than one field
-    # needs, since the filter name alone does not identify a search key:
-    # `filter :created_at, as: :date_range, default: { gteq: -> { 1.week.ago.to_date } }`.
-    #
-    # Only a Proc is called - unlike `:if` and `:unless`, a Symbol here is a value, not a
-    # method to send.
-    def add_filter_default_value(result, attribute, default)
-      default = instance_exec(&default) if default.is_a?(Proc)
-      return if default.nil?
-
-      if default.is_a?(Hash)
-        default.each do |predicate, value|
-          value = instance_exec(&value) if value.is_a?(Proc)
-          result["#{attribute}_#{predicate}"] = value unless value.nil?
-        end
-      else
-        result[attribute.to_s] = default
-      end
-    end
   end
 end

lib/active_admin_filters_defaults/filter_defaults.rb

@@ -0,0 +1,180 @@
+# frozen_string_literal: true
+
+module ActiveAdminFiltersDefaults
+  # Everything `filter ..., default:` means: which requests the declared defaults apply to, what
+  # they come to on this one, and which Ransack key each value lands under.
+  #
+  # Included rather than prepended - none of it replaces anything Active Admin defines, and a
+  # resource is meant to be able to override the seams here in its own `controller do` block.
+  # The one method that does replace Active Admin's is in DataAccess.
+  module FilterDefaults
+    # Read here and never handed to an input: `:default` is this gem's, and `:if` / `:unless`
+    # are Active Admin's but are resolved by #visible_filters before the form is given
+    # anything. Named once, because both the form and the derivation have to strip the same
+    # set and an option that drifts between them reaches Formtastic silently.
+    NOT_FOR_INPUT = %i[default if unless].freeze
+
+    protected
+
+    # The filter values the collection is searched with. Override to change what the index
+    # filters on without reaching into `params`.
+    #
+    # Whatever the request asked for wins over a default, so that an override of
+    # #filter_defaults_apply? which lets defaults through on a partly filtered request keeps
+    # the values that were actually asked for.
+    #
+    # @return [Hash, ActionController::Parameters] values passed to Ransack
+    def filtering_params
+      return params[:q] || {} unless filter_defaults_apply?
+
+      defaults = filter_default_values
+      return params[:q] || {} if defaults.blank?
+
+      requested = params[:q]
+      requested = requested.to_unsafe_h if requested.respond_to?(:to_unsafe_h)
+      defaults.merge(requested || {})
+    end
+
+    # Whether the request is one the declared defaults should apply to. Override to widen it -
+    # a resource that always carries its customer in `q`, say, still wants its defaults on the
+    # first visit:
+    #
+    #   def filter_defaults_apply?
+    #     super || params[:q].keys == %w[customer_id_eq]
+    #   end
+    #
+    # `commit` marks a submission of the filters form: submitting it with every field blank
+    # sends no `q` at all, because the form disables empty fields on submit, so `commit` is
+    # what tells "show me everything" apart from a first visit. Clearing the filters drops
+    # `commit` along with `q`, so Clear Filters returns the page to its declared defaults.
+    def filter_defaults_apply?
+      params[:q].blank? && params[:commit].blank?
+    end
+
+    # The filters of this resource that `:if` and `:unless` allow, which is both the set the
+    # filters form renders and the set that can impose a default - a filter that is not
+    # rendered does not filter the collection behind the admin's back.
+    #
+    # @return [Hash] filter attribute => filter options
+    def visible_filters
+      @visible_filters ||= active_admin_config.filters.reject do |_attribute, options|
+        (options.key?(:if) && !::MethodOrProcHelper.render_in_context(self, options[:if])) ||
+          (options.key?(:unless) && ::MethodOrProcHelper.render_in_context(self, options[:unless]))
+      end
+    end
+
+    # @return [Hash] the search values declared with `filter ..., default:`
+    def filter_default_values
+      @filter_default_values ||= visible_filters.each_with_object({}) do |(attribute, options), result|
+        next unless options.key?(:default)
+
+        add_filter_default_value(result, attribute, options)
+      end
+    end
+
+    private
+
+    # A default is a value, and where that value goes is the input's business - see SearchKeys.
+    #
+    #   filter :status, as: :select,      default: "active"
+    #   filter :author, as: :check_boxes, default: [1, 2]
+    #   filter :created_at, as: :date_range, default: -> { 1.week.ago.. }
+    #
+    # A Hash still says the predicates outright, for when the input's own is not the one you
+    # want (`default: { eq: "acme" }` on a string filter that would otherwise search `_cont`).
+    #
+    # Only a Proc is called - unlike `:if` and `:unless`, a Symbol here is a value, not a
+    # method to send.
+    def add_filter_default_value(result, attribute, options)
+      default = options[:default]
+      default = instance_exec(&default) if default.is_a?(Proc)
+      return if default.nil?
+
+      if default.is_a?(Hash)
+        default.each do |predicate, value|
+          value = instance_exec(&value) if value.is_a?(Proc)
+          result["#{attribute}_#{predicate}"] = value unless value.nil?
+        end
+      else
+        assign_derived_filter_default(result, attribute, options, default)
+      end
+    end
+    # A Range fills a two-ended input, one bound per end, and an endless or beginless one fills
+    # only the end it has. Anything else is a single value for a single-ended input.
+    def assign_derived_filter_default(result, attribute, options, default)
+      names = filter_search_keys(attribute, options)
+
+      if default.is_a?(Range)
+        unless names.size == 2
+          raise_filter_default_error(attribute, "a Range needs an input with two ends, and this one submits #{names.join(' and ')}")
+        end
+
+        result[names.first] = default.begin unless default.begin.nil?
+        result[names.last] = default.end unless default.end.nil?
+      else
+        unless names.size == 1
+          raise_filter_default_error(attribute, "this input submits #{names.join(' and ')}, so a single value cannot say which to fill - give a Range, or name the predicates with a Hash")
+        end
+
+        result[names.first] = default
+      end
+    end
+
+    # Asked against an empty search on purpose: `current_filter` otherwise answers with whatever
+    # predicate the current request happens to carry, and a default is about the request that
+    # carries none.
+    #
+    # @return [Array<String>] one key, or two for a range input
+    def filter_search_keys(attribute, options)
+      input = build_filter_input(attribute, options)
+
+      names =
+        if input.respond_to?(:gt_input_name)
+          [input.gt_input_name, input.lt_input_name]
+        elsif input.respond_to?(:current_filter) && !input.seems_searchable?
+          # `:string` and `:numeric` let the admin pick the predicate from a dropdown, and the
+          # head of that list is what an untouched form submits. Unless the filter name already
+          # carries a predicate, in which case there is no dropdown - and asking anyway raises,
+          # since `current_filter` would go looking for `title_eq_cont`.
+          [input.current_filter]
+        else
+          [input.input_name]
+        end
+
+      names.map { |name| name.to_s[/\Aq\[([^\]]+)\]/, 1] || name.to_s }
+    rescue StandardError => e
+      raise_filter_default_error(attribute, "could not work out which search key it submits (#{e.class}: #{e.message}) - name the predicate with a Hash instead")
+    end
+
+    def build_filter_input(attribute, options)
+      builder = filter_input_builder
+      options = filter_input_options(options, builder)
+      as = options[:as] || builder.send(:default_input_type, attribute)
+      builder.send(:namespaced_input_class, as)
+             .new(builder, builder.template, builder.object, :q, attribute, options)
+    end
+
+    # `:input_html` may be a Proc, which the filters form resolves against the view before it
+    # builds the input. Handing the Proc over instead is not merely incomplete, it is wrong in
+    # a way nothing reports: `Proc#[]` is `call`, so Formtastic asking `input_html[:multiple]`
+    # invokes it, gets a truthy Hash back, and a `:select` derives `_in` rather than `_eq`.
+    def filter_input_options(options, builder)
+      options = options.except(*NOT_FOR_INPUT)
+      return options unless options[:input_html].is_a?(Proc)
+
+      options.merge(input_html: builder.template.instance_exec(&options[:input_html]))
+    end
+
+    # One per request rather than one per filter: #view_context builds a fresh view class every
+    # time it is called, and every defaulted filter would pay for another one.
+    def filter_input_builder
+      @filter_input_builder ||= ::ActiveAdmin::Filters::FormBuilder.new(
+        :q, active_admin_config.resource_class.ransack({}), view_context, {}
+      )
+    end
+
+    def raise_filter_default_error(attribute, message)
+      raise ArgumentError, "filter :#{attribute} declares a `default:` but #{message}"
+    end
+  end
+end

lib/active_admin_filters_defaults/filters_form.rb

@@ -23,7 +23,7 @@ def filters_sidebar_section
   # method, so that this gem carries no copy of a method body that belongs to Active Admin.
   module ViewHelper
     def active_admin_filters_form_for(search, filters, options = {})
-      super(search, filters.transform_values { |opts| opts.except(:if, :unless, :default) }, options)
+      super(search, filters.transform_values { |opts| opts.except(*FilterDefaults::NOT_FOR_INPUT) }, options)
     end
   end