Repository navigation
fix(row-model): accept the boolean filter operand the docs promise and the funnel sends - #361
Merged
Merged
Conversation
validateFilterOperand rejected string "true"/"false" for boolean isAnyOf/isNoneOf filters, throwing CompiledQueryValidationError even though: the React funnel's BOOLEAN_OPTIONS emits those exact string literals, content/docs/grid/filtering.mdx documents them as the required contract, and booleanValue() (the evaluator this validator guards) already coerces them. The validator was stricter than the contract it was meant to enforce. Widen it to accept real booleans plus the string literals "true"/ "false" - not 1/0/"1"/"0", which booleanValue() also coerces but the docs never promise, so accepting them would be validator-only drift. Removes the test row that pinned the bug as intended behavior and replaces it with positive tests that the accepted shapes validate and evaluate correctly. Adds a cross-boundary test (packages/react, which already depends on both @pretable/core and @pretable-internal/row-model) that pipes the real toColumnFilter()/operatorsForType() output into row-model's real compileQuery() for every column type and operator the filter menu can emit. Neither package's own tests exercised this seam, which is why the mismatch shipped.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Vercel preview readyPreview: https://pretable-37e1epn44-cacheplane.vercel.app Updated automatically by the |
3 tasks done
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.
Summary
Filtering a
type: "boolean"column threw an uncaught error and took the whole page down:validateFilterOperandrequired real JS booleans. Three things say it shouldn't:filter-operators.ts'sBOOLEAN_OPTIONSemits string"true"/"false".docs/grid/filtering.mdxdocuments that contract explicitly — a boolean column'soptionsmay relabel the two states, but their values must remain"true"and"false".booleanValue(), the evaluator this validator guards, already coerces those strings.The validator was stricter than the evaluator it protects and stricter than the documented contract. The docs and the React menu were right; the validator was the outlier.
Origin:
72e7d47d(#321) introducedcompiled-query.tswhole-cloth and deletedgrid-core/src/evaluate-filter.ts, which duck-typed booleans as strings with no strict validation. The React side was built against that permissive contract and never updated.Not UI-only. A caller who never opens the funnel and hand-writes a query following the docs got the identical throw, synchronously, out of public
@pretable/coresetQuery().Scope of the audit
The full column-type × operator matrix was executed before fixing anything: text, number, date, and enum all validate and evaluate correctly across every operator the menu can emit. Boolean
isAnyOf/isNoneOfwere the only failures, and there were no silently-wrong cases.The real deliverable
No test anywhere crossed the react → row-model boundary for filters.
filter-operators.test.tstestedtoColumnFilterin isolation; row-model's tests never consumed its output. That gap is why this shipped.packages/react/src/__tests__/filter-menu-row-model-boundary.test.tsnow pipes the realtoColumnFilter/operatorsForTypeoutput into the realcompileQueryfor all 31 (type, operator) pairs, asserting both that compilation succeeds and that matching is correct. A self-check keeps the case table in sync withoperatorsForType, so the two can't drift apart silently.It lives in
packages/reactbecause that package already depends on both sides; the reverse placement would invert the dependency graph.A test pinned the bug
compiled-query.test.tsasserted the validator rejects["true"]— encoding the broken behavior as intended. Replaced with positive tests proving["true"]/["false"]validate and match the right rows, plus a mixed[true, "false"]case. The rest of the rejection table is intact and still fails as expected.Deliberately not accepted
1/0/"1"/"0", whichbooleanValue()also coerces. The docs promise only the two string literals; accepting more would move the drift from "validator too strict" to "validator too loose".Test Plan
pnpm test— all greentypecheck,lint,format,build,api:check,typecheck:public— cleanCompiledQueryValidationErroraboveFollow-up
content/examples/column-filters/dropped its boolean column because of this crash; it should be restored once this merges.🤖 Generated with Claude Code