Repository navigation
Leave boolean predicates out of a value policy - #6
Merged
Merged
Conversation
A value policy swept every request key naming its attribute, boolean predicates included. `?q[customer_id_null]=true` is answered by "true", not by a value of the column's own type: `sanitize: :digits` emptied it, `validate: :integer` refused what was left, and the :reset dropped the key - so a drill-down link that spelled out "no customer" came back as the whole table. The sweep now skips a sibling whose Ransack predicate is of type :boolean (`null`, `not_null`, `present`, `blank`). A boolean predicate the filter declares itself stays policed: that one its author asked for.
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix and regression test have no identified blocking issues; tests were not executed during review.
No open issues 🎉
What changed in this PR
Prevents value policies from dropping boolean sibling predicates while preserving enforcement for explicitly declared predicates.
Changes:
- Uses Ransack predicate types to exclude boolean siblings.
- Adds a regression test for
position_null=true. - Bumps the version to 1.0.1.
| File | Description |
|---|---|
spec/integration/numeric_builtins_spec.rb |
Tests preservation of boolean sibling filtering. |
lib/active_admin_filter_extensions/version.rb |
Updates the version to 1.0.1. |
lib/active_admin_filter_extensions/policies.rb |
Excludes undeclared boolean siblings from value policies. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
A value policy (
sanitize:/validate:) swept every request key naming its attribute, including the predicates Ransack answers with a boolean rather than with a value of the column's own type —null,not_null,present,blank.So for
a request of
?q[position_null]=truewent:"true"→:digits→""→:integerrefuses →:resetdrops the key → the redirect lands on the bare index. A link meaning "the rows with no position set" came back as the whole table, silently including the rest.The fix
sibling_predicate_keysnow skips a sibling whose Ransack predicate is of type:boolean, asking Ransack itself (Ransack::Predicate.named(...).type) rather than matching suffixes, so an app's own boolean predicate is covered too.A boolean predicate the filter declares stays policed —
filter :x_null, extensions: { validate: ... }is a policy its author asked for, and the derived key is still inkeys.Test
spec/integration/numeric_builtins_spec.rb, under "sibling predicates" — a post with noposition, fetched through?q[position_null]=trueon theIntegerPostresource whoseposition_eqcarries both numeric policies.Red before the fix:
(the whole
qwas gone — the condition had been dropped). Green after. Full suite: 196 examples, 0 failures.Version bumped to 1.0.1.