Skip to content

fix(datable): ignore non-hash bounds instead of raising - #8

Merged
jplot merged 1 commit into
mainfrom
fix/datable-non-hash-bounds
Jul 3, 2026
Merged

fix(datable): ignore non-hash bounds instead of raising#8
jplot merged 1 commit into
mainfrom
fix/datable-non-hash-bounds

Conversation

@jplot

@jplot jplot commented Jul 3, 2026

Copy link
Copy Markdown
Member

Summary

A declared datable attribute given a scalar or array value — e.g. ?filters[value_date]=2026-01-01 straight from a query string — raised TypeError when the date filters read bounds[:after] on a String, violating the gem's never-raise contract.

Changes

  • Datable.columns now drops attributes whose value is not a hash of bounds, so a malformed shape narrows nothing (single guard shared by the four date filters)
  • specs: scalar and array values, plain hash and ActionController::Parameters, no longer raise and return the untouched relation

- `filters[value_date]=2026-01-01` from a plain query string reached
  `bounds[:after]` on a String and raised TypeError, breaking the
  never-raise contract
- drop attributes whose value is not a hash of bounds in
  Datable.columns, the single path shared by the four date filters
Copilot AI review requested due to automatic review settings July 3, 2026 09:05
@fluence-ci

fluence-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

🛡️ Documentation Quality: 100.0% (0.0%)

████████████████████ 100.0% — 180/180 checks passing

Base PR Δ
Coverage 100.0% 100.0% ±0%
Failing 0 0 ±0

Documentation quality above threshold.

Changed files

File Failing Δ vs base
🟢 lib/filterable/datable.rb 0 ±0

@fluence-ci

fluence-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

🛡️ RuboCop: 0 offenses (0)

Base PR Δ
Offenses 0 0 ±0

No RuboCop offenses.

@fluence-ci

fluence-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

🛡️ Coverage Report: 100.0% (0.0%)

████████████████████ 100.0% — 144/144 lines covered

Base PR Δ
Total 100.0% 100.0% ±0%

All changes are tested.

Changed files

File Coverage Δ vs base Lines
🟢 lib/filterable/datable.rb 100.0% ±0% 13/13

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.

Pull request overview

This PR hardens the Filterable::Datable date filters so malformed filters shapes (e.g., a scalar/array where a bounds hash is expected) are ignored rather than raising, preserving the gem’s “never-raise” contract when consuming raw query-string params.

Changes:

  • Add a single guard in Filterable::Datable.columns to drop non-Hash bounds before the four date filters read bounds[:...].
  • Extend specs to cover scalar/array malformed bounds for both plain hashes and ActionController::Parameters, asserting no exception and an unchanged relation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
spec/filterable_spec.rb Adds regression coverage ensuring malformed date-filter shapes don’t raise and don’t narrow results.
lib/filterable/datable.rb Filters out non-hash bounds in Datable.columns to prevent TypeError from bounds[:key] on scalars/arrays.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fluence-ci

fluence-ci Bot commented Jul 3, 2026

Copy link
Copy Markdown

📊 Code Quality Report

Tool Base PR Δ
Reek 9 10 smells 🔴 +1
Flog total 191.4 193.9 🔴 +2.5
Flay total 153 153 ±0

🔍 1 new reek smell introduced — review below.

🔍 1 new reek smell introduced by this PR
Smell File Context Lines Message
TooManyStatements lib/filterable/datable.rb Filterable::Datable#columns 37 has approx 6 statements
🔍 All 10 reek smells on this PR
Smell File Context Lines Message
DuplicateMethodCall lib/filterable/concern.rb Filterable::Concern#filterable 49, 51 calls 'params[:filters]' 2 times
DuplicateMethodCall lib/filterable/sortable.rb Filterable::Sortable#call 20, 22 calls 'params[:sort]' 2 times
ManualDispatch lib/filterable/concern.rb Filterable::Concern#add_filter 33 manually dispatches method call
ManualDispatch lib/filterable/concern.rb Filterable::Concern#filters 21 manually dispatches method call
ManualDispatch lib/filterable/concern.rb Filterable::Concern#indifferent? 63 manually dispatches method call
ManualDispatch lib/filterable/datable.rb Filterable::Datable#columns 40 manually dispatches method call
TooManyStatements lib/filterable/datable.rb Filterable::Datable#columns 37 has approx 6 statements
TooManyStatements lib/filterable/datable/range.rb Filterable::Datable::Range#call 17 has approx 6 statements
TooManyStatements lib/filterable/sortable.rb Filterable::Sortable#call 19 has approx 6 statements
UtilityFunction lib/filterable/concern.rb Filterable::Concern#indifferent? 62 doesn't depend on instance state (maybe move it to another class?)
📊 Flog complexity breakdown
193.9: flog total
     6.7: flog/method average

    17.4: Filterable::Datable::Range#call  lib/filterable/datable/range.rb:17-26
    16.3: Filterable::Concern#filterable   lib/filterable/concern.rb:47-52
    15.5: main#none
    14.8: Filterable::Sortable#call        lib/filterable/sortable.rb:19-29
    11.9: Filterable::Datable::After#call  lib/filterable/datable/after.rb:16-23
    11.9: Filterable::Datable::Before#call lib/filterable/datable/before.rb:16-23
    11.9: Filterable::Datable::Since#call  lib/filterable/datable/since.rb:16-23
    10.9: Filterable::Datable#columns      lib/filterable/datable.rb:37-42
     9.3: Filterable::Railtie::initializer#filterable.active_record lib/filterable/railtie.rb:18-24
📊 Flay duplication breakdown
Total score (lower is better) = 153

1) Similar code found in :module (mass = 105)
  lib/filterable/datable/after.rb:4
  lib/filterable/datable/before.rb:4
  lib/filterable/datable/since.rb:4

2) Similar code found in :iter (mass = 48)
  lib/filterable/concerns/datable.rb:18
  lib/filterable/concerns/sortable.rb:15

@jplot
jplot merged commit 23b2334 into main Jul 3, 2026
17 checks passed
@jplot
jplot deleted the fix/datable-non-hash-bounds branch July 3, 2026 09:20
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