Skip to content

Fix 10 issues found in high-effort review of main#141

Merged
vsdudakov merged 2 commits into
mainfrom
fix/review-findings-main
Jul 8, 2026
Merged

Fix 10 issues found in high-effort review of main#141
vsdudakov merged 2 commits into
mainfrom
fix/review-findings-main

Conversation

@vsdudakov

Copy link
Copy Markdown
Owner

Fixes 10 verified findings from a multi-agent review of the latest main (the 0.8.1 filter-sanitization/date-parsing changes).

Correctness

  • Frontend boolean-string corruption: transformValueFromServer coerced 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.
  • SQLAlchemy relationship filters on PostgreSQL: the relationship-filter branch compared raw string query params against integer related pks (asyncpg raises DataError → HTTP 500; SQLite masked it via type affinity). Values are now int-coerced like the scalar path.
  • Silent operator collapse on relation fields: every condition except in degraded to pk equality (?participants__icontains=ann returned wrong data with HTTP 200). The service layer now rejects non-exact/in lookups on m2m fields with 422, and the adapter raises instead of guessing.
  • Flask error handler: HTTPExceptions with an attached response (abort(Response(...))) are passed through unchanged instead of becoming a generic 500; headers like Allow/WWW-Authenticate/Retry-After are preserved on JSON error responses; unhandled errors are logged with their traceback (previously only str(exc)).
  • IS NULL on text columns: field__exact=null maps to SQL NULL again (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.
  • PonyORM _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

  • PonyORM list search: was materializing every matching row as full entities once per search field just to collect pks, then re-filtering with a giant in-memory IN set. Now a single OR'd WHERE clause.

Structure

  • Fields without a change widget map to null (known non-date) instead of undefined, so they no longer fall back to shape-based date detection — closing the residual date-corruption hole for widget-less fields.
  • transformDataFromServer accepts the model configuration directly; the three call sites no longer duplicate the getChangeWidgetTypes(...) incantation, and a future call site can't silently reintroduce shape detection by forgetting the second argument.

Tests

  • Backend: 1990 passed; ruff, ruff format, ty clean.
  • Frontend: 245 passed across 43 files; tsc, eslint, biome clean.
  • Updated tests asserting the old text-field "null" literal behavior and the old transformDataFromServer signature; added coverage for boolean-widget gating, real-boolean passthrough, and known-absent-widget fields.

vsdudakov added 2 commits July 8, 2026 15:15
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).
@vsdudakov vsdudakov merged commit 0792b2b into main Jul 8, 2026
12 checks passed
@vsdudakov vsdudakov deleted the fix/review-findings-main branch July 8, 2026 11:39
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.

1 participant