adding support to the where handler for jsonb for date ranges in the …

[jakesanders]

…same format as other range filters

#80

Checklist:

• I have updated the necessary documentation
• I have updated the changelog, if necessary (CHANGELOG.md)
• I have signed off all my commits as required by DCO
• My build is green

[andrew-wheeler]

This potentially blocks the default behavior if there was a key / value with a value containing "..." which was not intended to be a date range. Suggested improvement: catch the StandardError from normalized_date_range and fall back to the default behavior in the else.

[mattl3w1s]

Agent review

[mattl3w1s]

This seems legit. From my agent:

====

[blocking] correctness / integration

This branch may be unreachable for real requests. Filter#parameterize calls ValueParser before passing the value to WhereHandler. For :jsonb, Parameter#supports_ranges? returns true (only :string, :text, :scope are excluded — see parameter.rb:31).

ValueParser#parse_as_range? checks supports_ranges && raw_value.to_s.include?("..."). Two realistic request shapes both fail:

• Nested params (filters[metadata][published_at]=2018-01-01...2018-01-02) → Rails gives a Hash like {"published_at"=>"2018-01-01...2018-01-02"}. hash.to_s.include?("...") is true, so parse_as_range? fires and calls value.split("...") on the Hash → NoMethodError.
• JSON string (filters[metadata]={"published_at":"2018-01-01...2018-01-02"}) → parse_as_range? fires on the whole JSON string, producing a Range from the first two ...-separated JSON fragments. That Range reaches apply_jsonb_conditions, which iterates it via String#succ — the latent bug described in issue #80.

The tests pass because they call handler.call directly, bypassing ValueParser. An integration test through Sift::Filtrator.filter (or the dummy controller) would expose this.

Fix: either guard parse_as_range? to skip when the value is a Hash, or handle the jsonb range detection inside ValueParser#parse_json_and_values so each individual key value is tested for the ... pattern rather than the entire outer value.

[mattl3w1s]

Good idea to add the timestampz type casting from agent (below)

======

[blocking] correctness

->> extracts the JSONB key as text. The BETWEEN comparison is therefore lexicographic, not type-aware:

• Dates: Works only when all stored values are ISO-8601 strings with the same timezone offset. '2018-01-01T00:00:00+05:30' and '2018-01-01T00:00:00Z' represent the same instant but compare differently as text.
• Numerics: '9' > '10' lexicographically, so a range like "9...100" would return wrong results.

Issue #80's acceptance criteria explicitly calls for type-casting: (metadata->>'price')::numeric BETWEEN 10 AND 100. The PR implements none of that — no key-type declaration, no cast in the SQL.

For datetime specifically, the cast would be:

(metadata->>'published_at')::timestamptz BETWEEN ? AND ?

This requires the caller to declare the key type (part of the feature spec that was left unimplemented).

[mattl3w1s]

Probably also legit

====

[warning] semantics

SQL BETWEEN a AND b is inclusive on both ends. Sift's ... separator follows Ruby's exclusive-range convention (exclusive upper bound). Issue #80 explicitly calls this out as an open question — the PR resolves it silently in favor of inclusive semantics without any documentation or changelog entry.

At minimum, add a comment explaining the deliberate choice. Ideally document it in the README alongside the other range filter docs so callers know "2018-01-01...2018-01-02" on a JSONB key matches records including 2018-01-02, unlike what the ... literal implies in Ruby.

[mattl3w1s]

Good suggestion

====

[warning] testing

This test calls handler.call directly with a pre-parsed Hash, bypassing Filter#parameterize → ValueParser. As noted in the inline comment on where_handler.rb:29, ValueParser#parse_as_range? would intercept the value before WhereHandler runs in a real request, making this code path unreachable in production.

Adding an integration test through Sift::Filtrator.filter (or the existing dummy PostsController) would catch that gap:

test "jsonb date range filter works end-to-end via Filtrator" do
  filters = [Sift::Filter.new(:metadata, :jsonb, :metadata, nil)]
  range_string = "2018-01-01T00:00:00+00:00...2018-01-02T00:00:00+00:00"
  params = { filters: { metadata: { "published_at" => range_string } } }
  relation = Sift::Filtrator.filter(Post.all, params, filters)
  assert_includes relation.to_sql, "BETWEEN"
end