diff --git a/.changeset/29019-form-refusal-text.md b/.changeset/29019-form-refusal-text.md new file mode 100644 index 00000000000..cddfdfd8da5 --- /dev/null +++ b/.changeset/29019-form-refusal-text.md @@ -0,0 +1,18 @@ +--- +'@object-ui/console': patch +--- + +fix(console): a refused form submit says a localized sentence, never the server's response body + +Clause-②: no +Metadata-protocol: none + +The Console form page (`/forms/NAME`, the in-app form and the public `/f/SLUG`) drew a refused write as the verb, the status and the raw response body, so a member without create rights read `Create failed (403): {"error":…,"code":"PERMISSION_DENIED","object":…}` in English on the banner and the toast. The page now reads the refusal the way the record dialog does, and the banner and the toast say the same sentence: + +- a sentence the producer marked for the end user (`userMessage`), or a delegated administrator's scope refusal, as written; +- any other denial (`403`, `PERMISSION_DENIED`, `FORBIDDEN`) as the localized "You don't have permission to save this record."; +- the per-rule lines no input carries, one per line, as before; +- the server's own sentence for any other `4xx`, read out of the envelope (`error`, or the wrapped `error.message`), never the body itself; +- a `5xx`, or a refusal with no sentence (a proxy's markup page), as the localized "Could not save. Please try again." + +The fields a refusal names are still marked under their inputs. No i18n key is added: both lines are the existing `form.noPermissionToSave` and `form.submitFailed`, present in all ten packs. diff --git a/ui/apps/console/src/components/FormPage.nativeInvalid-27496.test.tsx b/ui/apps/console/src/components/FormPage.nativeInvalid-27496.test.tsx index f1628455806..9ac2497a409 100644 --- a/ui/apps/console/src/components/FormPage.nativeInvalid-27496.test.tsx +++ b/ui/apps/console/src/components/FormPage.nativeInvalid-27496.test.tsx @@ -103,6 +103,7 @@ describe('#27496 — a submit the browser blocks replaces the stale banner', () stubPublic(false, 400, { error: 'Validation failed', code: 'VALIDATION_FAILED' }); await refuseThenSubmitBadEmail(); await fixEmailAndSubmit(); - expect(await screen.findByText(/Submit failed \(400\)/)).toBeInTheDocument(); + // The server's sentence, out of the envelope, never the body (#29019). + expect(await screen.findByText('Validation failed')).toBeInTheDocument(); }); }); diff --git a/ui/apps/console/src/components/FormPage.refusalMarks-27417.test.tsx b/ui/apps/console/src/components/FormPage.refusalMarks-27417.test.tsx index 210c204e589..95db4a078ab 100644 --- a/ui/apps/console/src/components/FormPage.refusalMarks-27417.test.tsx +++ b/ui/apps/console/src/components/FormPage.refusalMarks-27417.test.tsx @@ -17,8 +17,9 @@ * error to, the same refusal in the runtime dispatcher's wrapped envelope, and * the flat `409 UNIQUE_VIOLATION` naming its one `field`. * - * The banner and the toast are unchanged: a refusal that names no field marks - * no row and still says what it said before. + * A refusal that names no field marks no row and still says the server's + * sentence; what the banner says for every other refusal is pinned in + * `FormPage.refusalText-29019.test.tsx`. */ import type React from 'react'; @@ -180,9 +181,10 @@ describe('#27417 — a server refusal naming fields marks each of them', () => { expect(describedText(email)).toContain('Email is not a valid email address'); // The row the refusal does not name stays unmarked. expect(screen.getByLabelText('Company')).not.toHaveAttribute('aria-invalid', 'true'); - // The banner and the toast are unchanged. + // Every named row carries its reason, so the banner and the toast say the + // server's sentence — never the response body (#29019). expect(vi.mocked(toast.error)).toHaveBeenCalledTimes(1); - expect(String(vi.mocked(toast.error).mock.calls[0][0])).toContain('Submit failed (400)'); + expect(vi.mocked(toast.error).mock.calls[0][0]).toBe('Validation failed'); }); it('internal route: the wrapped dispatcher envelope marks both named fields', async () => { diff --git a/ui/apps/console/src/components/FormPage.refusalText-29019.test.tsx b/ui/apps/console/src/components/FormPage.refusalText-29019.test.tsx new file mode 100644 index 00000000000..4d8aa87c58f --- /dev/null +++ b/ui/apps/console/src/components/FormPage.refusalText-29019.test.tsx @@ -0,0 +1,256 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack-ai/objectstack#29019 — a refused write says a sentence in the + * reader's language, never the response body. + * + * A member with read but no create right opened a form link, pressed Submit, + * and the banner and the toast both read the server's answer verbatim: + * `Create failed (403): {"error":"You do not have permission…","code": + * "PERMISSION_DENIED","object":"…"}` — English JSON, with the object's machine + * name, under a page whose report twin says "You don't have access". + * + * The page now reads the refusal the way the record dialog does: a denial is + * the localized `form.noPermissionToSave` line, a server fault (`5xx`) the + * localized `form.submitFailed` line, a `4xx` the server's own sentence out of + * the envelope, and the rows a refusal names are still marked. The fixtures + * are the envelopes the server answers: the flat body the data routes answer, + * the runtime dispatcher's wrapped one, and a proxy's markup page. + */ + +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { toast } from 'sonner'; +import { createI18n } from '@object-ui/i18n'; +import { FormPage } from './FormPage'; + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +const FIELDS = { + name: { type: 'text', label: 'Name' }, + email: { type: 'text', label: 'Email' }, +}; + +/** The QA run's refusal, byte for byte: a `POST /data/:object` without create. */ +const PERMISSION_DENIED = { + status: 403, + body: { + error: 'You do not have permission to perform this action. Contact your administrator if you need access.', + code: 'PERMISSION_DENIED', + object: 'qar8_fsb', + }, +}; + +let posts = 0; + +type Refusal = { status: number; body: unknown }; + +/** One answer of the stubbed server; a string body goes out as it is. */ +function answer(ok: boolean, status: number, body: unknown): Response { + const text = typeof body === 'string' ? body : JSON.stringify(body); + return { + ok, + status, + statusText: ok ? 'OK' : 'Refused', + json: async () => JSON.parse(text), + text: async () => text, + } as unknown as Response; +} + +/** The internal route `/forms/:name`, refusing its `POST /data/:object` with `refusal`. */ +function stubInternal(refusal: Refusal) { + const view = { + name: 'qar8_fsb.form', + object: 'qar8_fsb', + viewKind: 'form', + label: 'Feedback', + config: { type: 'simple', sections: [{ fields: ['name', 'email'] }] }, + }; + const object = { name: 'qar8_fsb', label: 'Feedback', fields: FIELDS }; + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string, init?: RequestInit) => { + if (init?.method === 'POST') { + posts += 1; + return answer(false, refusal.status, refusal.body); + } + if (String(url).includes('/meta/view/')) return answer(true, 200, view); + if (String(url).includes('/meta/object/')) return answer(true, 200, object); + throw new Error(`unstubbed fetch: ${url}`); + }), + ); + return render( + + + } /> + + , + ); +} + +/** The public route `/f/:slug`, refusing its `POST /forms/:slug/submit` with `refusal`. */ +function stubPublic(refusal: Refusal) { + const payload = { + slug: 'feedback', + object: 'qar8_fsb', + label: 'Feedback', + form: { type: 'simple', sections: [{ fields: ['name', 'email'] }] }, + objectSchema: { name: 'qar8_fsb', fields: FIELDS }, + }; + vi.stubGlobal( + 'fetch', + vi.fn(async (url: string, init?: RequestInit) => { + if (init?.method === 'POST') { + posts += 1; + return answer(false, refusal.status, refusal.body); + } + if (!String(url).includes('/forms/')) throw new Error(`unstubbed fetch: ${url}`); + return answer(true, 200, payload); + }), + ); + return render( + + + } /> + + , + ); +} + +async function fillAndSubmit(submitName: RegExp | string = /Submit/) { + await userEvent.type(await screen.findByLabelText('Name'), 'Ada'); + await userEvent.click(screen.getByRole('button', { name: submitName })); +} + +/** The banner's text, once the refusal has drawn it. */ +async function bannerText(): Promise { + await waitFor(() => expect(document.querySelector('[id^="form-error:"]')).not.toBeNull()); + return document.querySelector('[id^="form-error:"]')?.textContent ?? ''; +} + +/** The one refusal toast's content, as a string. */ +function toastText(): string { + expect(vi.mocked(toast.error)).toHaveBeenCalledTimes(1); + return String(vi.mocked(toast.error).mock.calls[0][0]); +} + +/** Neither the body nor the transport's prose reached the reader. */ +function expectNoRawBody(text: string, status: number) { + expect(text).not.toContain('{'); + expect(text).not.toContain(String(status)); + expect(text).not.toMatch(/failed \(/); +} + +beforeEach(() => { + posts = 0; + vi.mocked(toast.error).mockClear(); + createI18n({ defaultLanguage: 'en', detectBrowserLanguage: false }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('#29019 — a denied create reads the localized denial', () => { + it('internal route: the 403 PERMISSION_DENIED banner and toast say the same localized sentence', async () => { + stubInternal(PERMISSION_DENIED); + await fillAndSubmit(); + + const banner = await bannerText(); + expect(posts).toBe(1); + expect(banner).toBe("You don't have permission to save this record."); + expect(toastText()).toBe(banner); + expectNoRawBody(banner, 403); + // No row is to blame for a denial. + expect(document.querySelectorAll('[id$="-error"]')).toHaveLength(0); + }); + + it('zh: the same refusal reads in the reader\'s language', async () => { + createI18n({ defaultLanguage: 'zh', detectBrowserLanguage: false }); + stubInternal(PERMISSION_DENIED); + await fillAndSubmit('提交'); + + const banner = await bannerText(); + expect(banner).toBe('您没有权限保存这条记录。'); + expect(toastText()).toBe(banner); + expectNoRawBody(banner, 403); + }); + + it('public route: the dispatcher\'s wrapped 403 envelope reads the same denial', async () => { + stubPublic({ + status: 403, + body: { + success: false, + error: { code: 'PERMISSION_DENIED', message: 'Access denied for qar8_fsb', httpStatus: 403 }, + }, + }); + await fillAndSubmit(); + + const banner = await bannerText(); + expect(banner).toBe("You don't have permission to save this record."); + expect(toastText()).toBe(banner); + expectNoRawBody(banner, 403); + }); + + it('a sentence the producer marked for the end user is said as written', async () => { + stubInternal({ + status: 403, + body: { ...PERMISSION_DENIED.body, userMessage: 'Feedback closes on Fridays.' }, + }); + await fillAndSubmit(); + + expect(await bannerText()).toBe('Feedback closes on Fridays.'); + expect(toastText()).toBe('Feedback closes on Fridays.'); + }); +}); + +describe('#29019 — every other refusal', () => { + it('a 400 with field details still marks its fields, and says no body', async () => { + stubInternal({ + status: 400, + body: { + error: 'Validation failed', + code: 'VALIDATION_FAILED', + fields: [{ field: 'email', code: 'required', message: 'Email is required' }], + }, + }); + await fillAndSubmit(); + + const email = screen.getByLabelText('Email'); + await waitFor(() => expect(email).toHaveAttribute('aria-invalid', 'true')); + expect(document.getElementById(email.getAttribute('aria-describedby') ?? '')?.textContent).toContain( + 'Email is required', + ); + expect(screen.getByLabelText('Name')).not.toHaveAttribute('aria-invalid', 'true'); + const banner = await bannerText(); + expect(banner).toBe('Validation failed'); + expect(toastText()).toBe(banner); + expectNoRawBody(banner, 400); + }); + + it('a 500 reads the localized generic sentence, not the server fault', async () => { + stubInternal({ + status: 500, + body: { error: 'SQLITE_ERROR: no such table: qar8_fsb', code: 'INTERNAL_ERROR' }, + }); + await fillAndSubmit(); + + const banner = await bannerText(); + expect(banner).toBe('Could not save. Please try again.'); + expect(toastText()).toBe(banner); + expectNoRawBody(banner, 500); + expect(banner).not.toContain('SQLITE'); + }); + + it('a proxy\'s markup page reads the localized generic sentence', async () => { + stubPublic({ status: 502, body: '

502 Bad Gateway

' }); + await fillAndSubmit(); + + const banner = await bannerText(); + expect(banner).toBe('Could not save. Please try again.'); + expect(toastText()).toBe(banner); + expect(banner).not.toContain('<'); + }); +}); diff --git a/ui/apps/console/src/components/FormPage.tsx b/ui/apps/console/src/components/FormPage.tsx index a4ac3681315..f5afeaa3fec 100644 --- a/ui/apps/console/src/components/FormPage.tsx +++ b/ui/apps/console/src/components/FormPage.tsx @@ -161,7 +161,10 @@ import { } from '@object-ui/components'; import { SchemaRendererContext, + declaredUserMessage, + delegationScopeMessage, extractFieldErrors, + isPermissionError, unmarkedRefusalLines, usePredicateScope, type ApiFetch, @@ -1925,7 +1928,7 @@ async function submitPublic(slug: string, data: Record): Promis }); if (!res.ok) { const body = await res.text().catch(() => ''); - throw refusedWriteError(`Submit failed (${res.status}): ${body || res.statusText}`, body); + throw new WriteRefusal('Submit', res.status, body); } return res.json().catch(() => ({})); } @@ -1965,42 +1968,120 @@ async function submitInternal( ); if (!res.ok) { const body = await res.text().catch(() => ''); - throw refusedWriteError( - `${recordId ? 'Update' : 'Create'} failed (${res.status}): ${body || res.statusText}`, - body, - ); + throw new WriteRefusal(recordId ? 'Update' : 'Create', res.status, body); } return res.json().catch(() => ({})); } +/** The first argument that is a string with something in it after trimming. */ +function firstSentence(...vals: unknown[]): string | null { + for (const v of vals) { + if (typeof v === 'string' && v.trim()) return v.trim(); + } + return null; +} + /** * The error a refused submit throws (objectstack-ai/objectstack#27417): the - * message this page has always shown, plus the response body attached the way - * `@objectstack/client` attaches it to its own errors — the semantic `code`, - * and `details` from the wrapped envelope's `error.details`, else the whole - * flat body. - * - * That is what lets the record dialog's reader of the server's field paths, - * `extractFieldErrors` (`@object-ui/react`), read this page's refusals exactly - * as it reads the client's: the flat `400 VALIDATION_FAILED` with `fields[]` - * the submit routes answer, and the `409 UNIQUE_VIOLATION` naming its one - * `field`. ⛔ No field path is read here; a body that is not JSON attaches - * nothing, and the refusal marks no field. + * response attached the way `@objectstack/client` attaches it to its own + * errors — the HTTP `status`, the semantic `code`, `details` from the wrapped + * envelope's `error.details` (else the whole flat body), and the producer's + * marked `userMessage` from either dialect. + * + * That is what lets the record dialog's readers (`@object-ui/react`) read this + * page's refusals exactly as they read the client's: `extractFieldErrors` the + * flat `400 VALIDATION_FAILED` with `fields[]` the submit routes answer and the + * `409 UNIQUE_VIOLATION` naming its one `field`; `isPermissionError` a `403` or + * a `PERMISSION_DENIED`; `declaredUserMessage` the marked sentence. ⛔ No field + * path is read here; a body that is not JSON attaches nothing, and the refusal + * marks no field. + * + * The message is the server's own sentence — the envelope's `error` (or the + * wrapped `error.message`), or a plain-text body — and never the body itself: + * the end user once read `Create failed (403): {"error":…,"code":…}` verbatim + * (objectstack-ai/objectstack#29019). Which text reaches the page is + * {@link refusalText}'s call, not this message's. */ -function refusedWriteError(message: string, body: string): Error { - const error = new Error(message) as Error & { code?: string; details?: unknown }; - let parsed: unknown; - try { - parsed = JSON.parse(body); - } catch { - return error; +class WriteRefusal extends Error { + readonly status: number; + /** The response body as received — kept for a debugger, never drawn. */ + readonly body: string; + /** The server's sentence for this refusal, or `null` when it gave none. */ + readonly reason: string | null; + code?: string; + details?: unknown; + userMessage?: string; + + constructor(verb: 'Submit' | 'Create' | 'Update', status: number, body: string) { + let parsed: unknown; + try { + parsed = JSON.parse(body); + } catch { + parsed = undefined; + } + const envelope = + parsed && typeof parsed === 'object' + ? (parsed as { + code?: unknown; + details?: unknown; + message?: unknown; + userMessage?: unknown; + error?: unknown; + }) + : null; + const wrapped = + envelope?.error && typeof envelope.error === 'object' + ? (envelope.error as { code?: unknown; details?: unknown; message?: unknown; userMessage?: unknown }) + : null; + // A body that is not JSON is the server's sentence when it reads as one: + // plain text, not a markup page a proxy answered in its place. + const plain = parsed === undefined && !body.trim().startsWith('<') ? firstSentence(body) : null; + const reason = envelope + ? firstSentence(envelope.error, wrapped?.message, envelope.message) + : plain; + super(reason ?? `${verb} failed (${status})`); + this.name = 'WriteRefusal'; + this.status = status; + this.body = body; + this.reason = reason; + if (!envelope) return; + const code = firstSentence(envelope.code, wrapped?.code); + if (code) this.code = code; + this.details = envelope.details ?? wrapped?.details ?? envelope; + const marked = [envelope.userMessage, wrapped?.userMessage].find( + (m): m is string => typeof m === 'string' && m.trim() !== '', + ); + if (marked) this.userMessage = marked; } - if (!parsed || typeof parsed !== 'object') return error; - const envelope = parsed as { code?: unknown; details?: unknown; error?: { code?: unknown; details?: unknown } }; - const code = [envelope.code, envelope.error?.code].find((c) => typeof c === 'string' && c); - if (typeof code === 'string') error.code = code; - error.details = envelope.details ?? envelope.error?.details ?? envelope; - return error; +} + +/** + * What the banner and the toast say for a refused write — the record dialog's + * reading (`@object-ui/components`' form renderer), so a refusal reads the same + * on both surfaces: + * + * 1. the sentence the producer MARKED for the end user (`userMessage`), or a + * delegated administrator's scope refusal, which names only their own scope; + * 2. any other denial (`403`, `PERMISSION_DENIED`, `FORBIDDEN`) as the fixed + * localized line — the server's text names objects and policies, not + * something the submitter can act on (objectstack#3821, #24914 B′); + * 3. the per-rule lines no row carries (`unmarkedRefusalLines`); + * 4. the server's sentence of a `4xx` — it answers the request the submitter + * made, and they can change it; + * 5. otherwise — a `5xx`, or a refusal that gave no sentence — the localized + * generic line: a server fault's text is a diagnostic, not a reason. + */ +function refusalText( + err: WriteRefusal, + unmarked: string[] | null, + t: (key: string) => string, +): string | string[] { + const marked = declaredUserMessage(err) ?? delegationScopeMessage(err); + if (marked) return marked; + if (isPermissionError(err)) return t('form.noPermissionToSave'); + if (unmarked && unmarked.length > 1) return unmarked; + if (unmarked?.[0]) return unmarked[0]; + return err.status < 500 && err.reason ? err.reason : t('form.submitFailed'); } /** @@ -2936,7 +3017,7 @@ export function FormPage({ mode, recordPath, cancelPath }: FormPageProps) { } } catch (err: any) { // The fields the server's refusal names, read by the record dialog's - // reader (see `refusedWriteError`), marked where this form draws them. A + // reader (see `WriteRefusal`), marked where this form draws them. A // field the form does not draw marks nothing; the banner still says it. const drawn = sections.flatMap((s) => s.fields); const named = (extractFieldErrors(err) ?? []).filter((fe) => @@ -2945,10 +3026,13 @@ export function FormPage({ mode, recordPath, cancelPath }: FormPageProps) { // A refusal listing one entry per failed rule says each entry no row // carries on its own line, not the envelope's `; `-joined summary. const unmarked = unmarkedRefusalLines(err, (field) => named.some((fe) => fe.field === field)); + // A refused write says a sentence in the reader's language, never the + // response body (objectstack-ai/objectstack#29019); anything else thrown + // on the way says its own message, as before. const msg: string | string[] = - unmarked && unmarked.length > 1 - ? unmarked - : unmarked?.[0] ?? err?.message ?? String(err); + err instanceof WriteRefusal + ? refusalText(err, unmarked, t) + : err?.message ?? String(err); setError(msg); toast.error(refusalContent(msg), { id: outcomeToastId }); setFieldErrors(Object.fromEntries(