Fix 10 issues found in high-effort review of main#141
Merged
Conversation
Backend: - SQLAlchemy: coerce relationship (m2m) filter values to int for integer pks (500 on PostgreSQL/asyncpg) and reject unsupported relation lookups with 422 instead of silently collapsing them to pk equality - PonyORM: only rewrite <relation>_<pk> filter keys when the base attribute is a real relation, so plain columns ending in _id filter correctly - PonyORM: run list search as a single OR'd WHERE clause instead of materializing every matching row once per search field - Flask: pass through HTTPExceptions with an attached response, preserve error headers on JSON error responses, log unhandled errors with traceback - Filtering: text columns support IS NULL again via __exact=null while substring lookups keep the literal string Frontend: - only coerce "true"/"false" strings to booleans for boolean widgets, so text content that spells a boolean survives the change form - treat fields without a change widget as known non-date (null) instead of falling back to shape-based date detection - transformDataFromServer takes the model configuration directly, removing the duplicated getChangeWidgetTypes incantation at every call site
Covers the three new defensive branches: the service-layer 422 for substring lookups on m2m filter fields, the SQLAlchemy adapter's loud failure for unsupported relation conditions, and the Flask handler's passthrough of HTTPExceptions with an attached response (plus header preservation on JSON error responses).
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.
Fixes 10 verified findings from a multi-agent review of the latest main (the 0.8.1 filter-sanitization/date-parsing changes).
Correctness
transformValueFromServercoerced the literal strings"true"/"false"to booleans before the widget-type gate, so a Char field containing the word"false"was destroyed on the change form — the same corruption class 0.8.1 fixed for dates. Boolean coercion is now gated to Switch/Checkbox widgets; real booleans always pass through.DataError→ HTTP 500; SQLite masked it via type affinity). Values are now int-coerced like the scalar path.indegraded to pk equality (?participants__icontains=annreturned wrong data with HTTP 200). The service layer now rejects non-exact/inlookups on m2m fields with 422, and the adapter raises instead of guessing.HTTPExceptions with an attached response (abort(Response(...))) are passed through unchanged instead of becoming a generic 500; headers likeAllow/WWW-Authenticate/Retry-Afterare preserved on JSON error responses; unhandled errors are logged with their traceback (previously onlystr(exc)).field__exact=nullmaps to SQLNULLagain (it compared against the literal string'null'); substring lookups (contains/icontains, which the UI uses for text filters) keep the literal, so text content that spells "null" remains searchable._id-suffix rewrite: filtering a plain column whose name merely ends with_id(e.g.external_id) was rewritten to an invalid relation path and returned 500. The rewrite now fires only when the base attribute is a real relation, and replaces the suffix rather than the first occurrence.Performance
INset. Now a single OR'd WHERE clause.Structure
null(known non-date) instead ofundefined, so they no longer fall back to shape-based date detection — closing the residual date-corruption hole for widget-less fields.transformDataFromServeraccepts the model configuration directly; the three call sites no longer duplicate thegetChangeWidgetTypes(...)incantation, and a future call site can't silently reintroduce shape detection by forgetting the second argument.Tests
"null"literal behavior and the oldtransformDataFromServersignature; added coverage for boolean-widget gating, real-boolean passthrough, and known-absent-widget fields.