Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/26891-unique-refusal-marks-field.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@object-ui/react': patch
---

fix(react): a create or edit form marks the field a unique-index refusal names

Clause-②: no
Metadata-protocol: none

Saving a record whose value collides with a unique index (a product SKU, an account name that must be unique) used to show "A record with this value already exists" as a banner and a toast, with no field marked. The user had to guess which of the form's unique fields collided.

The server's `409 UNIQUE_VIOLATION` answer names the one field on `field`, with a sentence that names it by its label ("A record with this SKU already exists"). `extractFieldErrors` only read lists of field errors, so it answered nothing for this refusal. It now returns one entry for it: the named field, carrying the server's sentence. The form renderer writes that sentence under the input, as it does for a `required` field, and shows no banner. Inline edit on a record page reads the same helper, so its save bar attributes the refusal to that field too.

When the server cannot tell which column collided (an index name, a composite key), the answer has no `field` and nothing changes: the banner stays.

What to do: nothing.
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,49 @@ describe('form renderer — server-rejected fields', () => {
expect(scrollSpy).not.toHaveBeenCalled();
});
});

/**
* A unique-index refusal names ONE field on `details.field` (not a list): the
* 409 `UNIQUE_VIOLATION` body `@objectstack/rest` sends, as
* `@objectstack/client` decorates it.
*/
function uniqueViolation(field?: string) {
const error = field ? 'A record with this SKU already exists' : 'A record with this value already exists';
return Object.assign(new Error(error), {
code: 'UNIQUE_VIOLATION',
httpStatus: 409,
details: { error, code: 'UNIQUE_VIOLATION', ...(field ? { field } : {}), object: 'product' },
});
}

const PRODUCT_FIELDS = [
{ name: 'name', label: 'Name', type: 'input' },
{ name: 'sku', label: 'SKU', type: 'input' },
];

describe('form renderer — unique-index refusal', () => {
it('marks the field the refusal names, instead of a banner and a toast', async () => {
const onSubmit = vi.fn().mockRejectedValue(uniqueViolation('sku'));
const { container } = renderForm(PRODUCT_FIELDS, onSubmit);
const skuWrap = container.querySelector('[data-field="sku"]') as HTMLElement;
const nameWrap = container.querySelector('[data-field="name"]') as HTMLElement;

submit();

await waitFor(() => expect(skuWrap.textContent).toMatch(/A record with this SKU already exists/));
expect(skuWrap.querySelector('[aria-invalid="true"]')).toBeTruthy();
expect(nameWrap.textContent).not.toMatch(/already exists/);
expect(toastErrorSpy).not.toHaveBeenCalled();
});

it('keeps the banner when the refusal cannot name the field', async () => {
const onSubmit = vi.fn().mockRejectedValue(uniqueViolation());
const { container } = renderForm(PRODUCT_FIELDS, onSubmit);

submit();

await waitFor(() => expect(toastErrorSpy).toHaveBeenCalled());
expect(String(toastErrorSpy.mock.calls[0][0])).toMatch(/A record with this value already exists/);
expect(container.querySelector('[aria-invalid="true"]')).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { describe, it, expect } from 'vitest';

import { normaliseClientError } from '@object-ui/data-objectstack';
import { declaredUserMessage } from './error-message';
import { declaredUserMessage, extractFieldErrors } from './error-message';

/**
* The two halves of the `userMessage` channel, pinned where they meet.
Expand Down Expand Up @@ -94,3 +94,27 @@ describe('userMessage survives the adapter boundary', () => {
expect(declaredUserMessage(normaliseClientError(conflict))).toBeNull();
});
});

describe('a UNIQUE_VIOLATION field survives the adapter boundary', () => {
it('reaches extractFieldErrors through the passthrough arm', () => {
// `normaliseClientError` re-wraps only VALIDATION_FAILED and
// CONCURRENT_UPDATE; a unique-index 409 leaves it untouched, so the single
// `details.field` the client parked is what the form reads.
const raw = Object.assign(new Error('A record with this SKU already exists'), {
code: 'UNIQUE_VIOLATION',
httpStatus: 409,
details: {
error: 'A record with this SKU already exists',
code: 'UNIQUE_VIOLATION',
field: 'sku',
object: 'product',
},
});
const normalised = normaliseClientError(raw) as typeof raw;
expect(normalised.code).toBe('UNIQUE_VIOLATION');
expect(normalised.httpStatus).toBe(409);
expect(extractFieldErrors(normalised)).toEqual([
{ field: 'sku', message: 'A record with this SKU already exists' },
]);
});
});
49 changes: 49 additions & 0 deletions ui/packages/react/src/utils/error-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,55 @@ describe('extractFieldErrors', () => {
expect(extractFieldErrors('a string')).toBeNull();
expect(extractFieldErrors(new Error('plain'))).toBeNull();
});

// A unique-index refusal names ONE field on a single string `field`, not a
// list: `@objectstack/rest` answers 409 `{ error, code: 'UNIQUE_VIOLATION',
// developerMessage, field, object }`, and `@objectstack/client` puts that flat
// body on `error.details`.
function uniqueViolation(field?: string) {
const error = field ? 'A record with this SKU already exists' : 'A record with this value already exists';
return Object.assign(new Error(error), {
code: 'UNIQUE_VIOLATION',
httpStatus: 409,
details: {
error,
code: 'UNIQUE_VIOLATION',
developerMessage: 'Duplicate value for sku on product',
...(field ? { field } : {}),
object: 'product',
},
});
}

it("marks the field a UNIQUE_VIOLATION names, with the server's sentence", () => {
const err = uniqueViolation('sku');
expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.httpStatus).toBe(409);
expect(extractFieldErrors(err)).toEqual([
{ field: 'sku', message: 'A record with this SKU already exists' },
]);
});

it('reads a hand-rolled UNIQUE_VIOLATION carrying `field` directly', () => {
const err = { code: 'UNIQUE_VIOLATION', field: 'name', error: 'A record with this Name already exists' };
expect(extractFieldErrors(err)).toEqual([
{ field: 'name', message: 'A record with this Name already exists' },
]);
});

it('returns null for a UNIQUE_VIOLATION without `field` — the server could not tell which column', () => {
const err = uniqueViolation();
expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.httpStatus).toBe(409);
expect(extractFieldErrors(err)).toBeNull();
});

it('reads a single `field` only on a UNIQUE_VIOLATION', () => {
// `INVALID_FIELD` also carries `field`, but it names a field the request
// referenced that does not exist — there is no input to mark.
const invalid = { code: 'INVALID_FIELD', httpStatus: 400, details: { code: 'INVALID_FIELD', field: 'skuu' } };
expect(extractFieldErrors(invalid)).toBeNull();
});
});

/**
Expand Down
17 changes: 16 additions & 1 deletion ui/packages/react/src/utils/error-message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ export function delegationScopeMessage(err: unknown): string | null {
return extractWriteErrorMessage(err);
}

/** The ADR-0112 code a write answers (409) when a unique index refuses it. */
const UNIQUE_VIOLATION = 'UNIQUE_VIOLATION';

/** One field the server rejected, normalised for `form.setError`. */
export interface WriteFieldError {
/** Field name, matching the form control's `name`. */
Expand Down Expand Up @@ -204,6 +207,14 @@ export interface WriteFieldError {
*
* Entries without a usable `field` are dropped rather than guessed at: a wrong
* mark on an innocent input is worse than the generic toast we already show.
*
* One refusal is field-scoped without a list: a `409 UNIQUE_VIOLATION` names
* the ONE field whose unique index refused the write, on a single string
* `field` (`details.field` on the raw client error, `field` on a hand-rolled
* one), with the server's end-user sentence on `error`. It becomes a single
* entry carrying that sentence. The server omits `field` whenever it cannot
* tell which column collided (an index name, a composite key), and the same
* rule holds here: no `field`, no entry, and the caller keeps its banner.
*/
export function extractFieldErrors(err: unknown): WriteFieldError[] | null {
const e = err as Record<string, any> | null | undefined;
Expand All @@ -214,7 +225,11 @@ export function extractFieldErrors(err: unknown): WriteFieldError[] | null {
(Array.isArray(e.details?.fields) && e.details.fields) ||
(Array.isArray(e.fields) && e.fields) ||
null;
if (!raw || !Array.isArray(raw)) return null;
if (!raw || !Array.isArray(raw)) {
if (!errorCodeIs(e, UNIQUE_VIOLATION) && !errorCodeIs(e.details, UNIQUE_VIOLATION)) return null;
const field = firstString(e.details?.field, e.field);
return field ? [{ field, message: extractWriteErrorMessage(err) ?? '' }] : null;
}

const out: WriteFieldError[] = [];
for (const entry of raw) {
Expand Down
Loading