Skip to content

fix(admin): show the server's reason on the field that caused it - #888

Open
ajslater wants to merge 1 commit into
fix/validation-types-and-lengthsfrom
fix/validation-server-field-errors
Open

ajslater wants to merge 1 commit into
fix/validation-types-and-lengthsfrom
fix/validation-server-field-errors

Conversation

@ajslater

Copy link
Copy Markdown
Owner

Implements P3e of tasks/followups-implementation-plan.md §5.5.

Stacked on #887#886#885. Retarget as each merges.

This is bigger than D6 implied, and the reason is the point

D6 called item (e) "a deletion, not an addition" — stop scanning locally and surface the server's error. But there was no server-field-error plumbing to surface it into, on either renderer, and neither wire shape is the one a reader would expect.

Admin resource viewsets render through AdminJSONAPIRenderer. For any 4xx get_resource_name returns "errors" and format_errors is literally return {"errors": data} — so the body is {"errors": {"username": [...]}}, a dict, not the JSON:API error list, because DJA's own exception handler is not installed. The xior interceptor only wraps when Array.isArray(body.errors), so it stays a plain error, and getErrors scanned ERROR_KEYS at the top level while the real key sits under errors. It fell through to errors = [data] and SubmitFooter rendered an object, which Vue stringified as JSON.

Envelope endpoints emit {"errors": [{status, title, detail}]}, which does wrap into an APIError — and APIError has no .response, so the same parser logged "Unable to parse error" and returned ["Unknown error"]. email-tab.vue showed no save-form errors at all.

Measured, not inferred:

ENVELOPE: {"data":null,"meta":{},"errors":[{"status":"400","title":"Bad Request",
           "detail":{"port":[...],"fromAddress":["Enter a valid email address."]}}]}
JSONAPI:  {"errors":{"username":["A user with that username already exists."]}}

Keys arrive camelCased, matching the client's model keys — so fieldErrors.fromAddress binds directly to draft.fromAddress with no translation layer.

Order of work

tests/test_admin_error_shapes.py was written and passing before any frontend change, per §5.5. Four cases pin both renderers' shapes across users, groups, libraries and the email singleton. That is what makes the normalizer safe to write.

The change

fieldErrorMap in the common store handles both shapes plus a bare DRF body, and form.fieldErrors joins form.errors in state. getErrors now flattens the same map, naming the field when more than one reported — so the summary is never a stringified object again.

:error-messages binds per field in the user (username, email), group (name) and library (path) inputs. The folder picker prefers the field-specific reason over the whole message list.

flag-card.setError was broken in a third way: it did Reflect.get(this.formErrors[0], field) against a list of message strings, so it was always undefined and a flag error could never render. It reads the normalized map now.

email-tab.vue binds host/port/timeout/from-address and shows anything left over above the action bar.

The client scans stay, demoted and documented

They are kept as a fast local pre-check — #870 made them getter-based so they re-read after a websocket reload — but they were never the authority, and now say so: loadTable reads only body.results and never follows next, so with page_size = 200 a duplicate on row 201 is not in the set. The server has always been authoritative (username, Group.name and Library.path are all unique=True, and ModelSerializer attaches UniqueValidator automatically — no explicit one is written anywhere in codex), so the server side needed no change at all.

library-create-update-inputs.vue never had an equality check, only parent/child overlap, so it gains correct behaviour it never had.

Tests

pytesttest_admin_error_shapes.py, 4 cases pinning both wire shapes.

vitestcommon-store-errors.test.js, 8 cases: both shapes normalizing to the same map, the multi-field summary, a lone detail staying a bare sentence, an explicit assertion that no message is ever a stringified object, the unparseable fallback, and clearing on both clearErrors and setSuccess.

vitestadmin-field-errors.test.js, 4 cases mounting the real dialogs: a duplicate username renders on the username field, an email error renders on the email field and not on the username, a duplicate group name renders on its own field, and no error means no message.

None of these existed before.

make fix && make lint && make ty clean. Full make test green: 1283 pytest (+4), 595 vitest (+12).

NEWS, under Fixes: "Admin forms show the server's reason when a name is already taken."

Verification debt

§8 notes #870's manual check re-runs after this PR, not before — this changes that surface.

🤖 Generated with Claude Code

D6 called this "a deletion, not an addition" -- stop scanning locally
and surface the server's error. But there was no server-field-error
plumbing to surface it into, on either of the two renderers, and
neither shape is the one a reader would expect.

Admin resource viewsets render through AdminJSONAPIRenderer, whose
`format_errors` is literally `{"errors": data}` -- so a 400 body is
`{"errors": {"username": [...]}}`, a dict rather than the JSON:API error
list, because DJA's own exception handler is not installed. The
interceptor only wraps when `errors` is an array, so it stays a plain
xior error, and `getErrors` scanned its known keys at the TOP level and
fell through to `errors = [data]`. SubmitFooter then rendered an object,
which Vue stringified as JSON.

Envelope endpoints emit `{"errors": [{status, title, detail}]}`, which
DOES wrap into an APIError -- and APIError has no `.response`, so the
same parser logged "Unable to parse error" and returned ["Unknown
error"]. The email tab showed no save errors at all.

Both shapes are pinned by a pytest written before any of this, and both
now normalize into one `{field: [messages]}` map that inputs bind
per-field. Keys arrive camelCased, matching the client's model keys.

`flag-card.setError` read `Reflect.get(this.formErrors[0], field)`
against a list of message strings, so it was always undefined and a
flag error could never render at all.

The client uniqueness scans stay as a fast pre-check rather than being
deleted -- #870 made them getter-based so they re-read after a
websocket reload -- but they were never the authority and are now
documented as such: `loadTable` reads only `body.results` and never
follows `next`, so past the first page of 200 they cannot see the
duplicate the server will reject. `library-create-update-inputs.vue`
never had an equality check at all, so it gains correct behaviour it
never had.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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