Skip to content

Add applied_filter_params, the values as a copy - #8

Merged
Fivell merged 1 commit into
mainfrom
applied-filter-params
Oct 20, 2026
Merged

Fivell merged 1 commit into
mainfrom
applied-filter-params

Conversation

@Fivell

@Fivell Fivell commented Oct 19, 2026

Copy link
Copy Markdown
Member

Why

filtering_params is documented as the values handed to Ransack, and that is exactly what it returns: a Hash with String keys, the same object the filters form and the canonicalizing redirect are computed from.

That is the right contract for the framework path, and an awkward one for a page that does its own work with those values. Such a page — one that overrides find_collection without super, talks to something that is not an ActiveRecord::Relation, or builds a drill-down or export link — usually has to translate the filters first: rename a key an input suffixed (time_start_gteq_datetime → time_start_gteq), turn a sentinel select value into a different predicate (status_eq=null → status_null=true). Done on the returned Hash, that edits what fills the filters form and what the redirect decision is computed from; done safely, every site repeats .with_indifferent_access or symbolize_keys plus a dup of its own.

What

# @return [ActiveSupport::HashWithIndifferentAccess]
def applied_filter_params
  filtering_params.with_indifferent_access
end

Registered as a helper next to filtering_params, so a view can call it too.

  • filtering_params is untouched — same contract, same type, same object semantics.
  • The new method's promise is the copy: a caller may read it by symbol and mutate it freely.

Tests

spec/data_access_spec.rb — one example reads by symbol a value the request spelled as a String; the other mutates the result the way a translating page does (applied[:position_gt] = applied.delete(:position_eq)) and asserts both that filtering_params is unchanged and that Ransack still searched the original.

Full suite: 200 examples, 0 failures. Version bumped to 1.2.0.

Also fixes a stale annotation: filtering_params was documented as [Hash, ActionController::Parameters], but requested_filtering_params unwraps Parameters via to_unsafe_h, so it is always a Hash.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Shared mutable String values allow edits through the new helper to alter the original filters.

1 open issue
What changed in this PR

Adds applied_filter_params for controllers and views to translate effective filters without changing the existing filtering_params API.

Changes:

  • Adds and exposes the indifferent-access copy helper.
  • Documents its intended usage and tests symbol access and key mutation.
  • Corrects a return-type annotation and bumps the version to 1.2.0.
File Description
spec/​data_access_spec.rb Tests helper access and key-mutation isolation.
README.md Documents the new helper.
lib/​active_admin_filter_extensions/​version.rb Bumps version to 1.2.0.
lib/​active_admin_filter_extensions/​filter_defaults.rb Implements the helper and corrects the annotation.
lib/​active_admin_filter_extensions.rb Exposes the helper to views.

Review effort: Balanced

Comment thread lib/active_admin_filter_extensions/filter_defaults.rb Outdated
A page that builds its own collection or its own links usually has to translate
the effective filters before passing them on: rename a key an input suffixed,
turn a sentinel value into a different predicate. Doing that to what
#filtering_params returns edits the Hash that also fills the filters form and
decides the canonicalizing redirect, and the String keys make every such site
reach for symbolize/stringify first.

#applied_filter_params is the same values as an indifferent deep copy - readable
by symbol, safe to mutate, down to the value objects. #filtering_params keeps
its contract untouched: the exact Hash handed to Ransack.
@Fivell
Fivell force-pushed the applied-filter-params branch from 80bc785 to 39cf196 Compare October 20, 2026 06:44
@Fivell
Fivell requested a balanced review from Copilot October 20, 2026 08:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused change preserves existing behavior and fixes the previously reported shared-value issue with regression coverage.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@Fivell
Fivell merged commit 3e93e1a into main Oct 20, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants