From 9e0e4b40d73cf44b0556917642840e0dfcbd686c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 16:48:17 +0000 Subject: [PATCH 1/2] fix(ui): let a producer-marked refusal reach the drag-write surfaces The kanban card-move toast, the calendar reschedule toast and the OCC conflict dialog each substituted a generic string for a refusal the PRODUCER had marked as user-facing (`userMessage`, objectstack#9934), so a user was told "Save failed" where the author had written a sentence for them. All three now read the marking through the shared `declaredUserMessage` reader, which knows both places the adapter boundary parks it: the typed member on `ConcurrentUpdateError` and the details bag on `DataApiValidationError`. Nothing unmarked reaches the user - the reader answers null for it and every existing substitution stands, so objectstack#3821's protection holds by construction. The two toasts substitute; the conflict dialog augments. Its description also explains what the destructive "Overwrite" button does, which is affordance copy this surface owns rather than a refusal message, so the marking leads and that paragraph stays. Applies the objectui#5210 ruling, already implemented for the console form. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- ...ObjectCalendar.markedRefusalToast.test.tsx | 199 ++++++++++++++ .../plugin-calendar/src/ObjectCalendar.tsx | 16 +- .../src/occSave.markedRefusal.test.tsx | 179 +++++++++++++ packages/plugin-form/src/occSave.tsx | 33 +++ .../ObjectKanban.markedRefusalToast.test.tsx | 243 ++++++++++++++++++ packages/plugin-kanban/src/ObjectKanban.tsx | 16 +- 6 files changed, 680 insertions(+), 6 deletions(-) create mode 100644 packages/plugin-calendar/src/ObjectCalendar.markedRefusalToast.test.tsx create mode 100644 packages/plugin-form/src/occSave.markedRefusal.test.tsx create mode 100644 packages/plugin-kanban/src/ObjectKanban.markedRefusalToast.test.tsx diff --git a/packages/plugin-calendar/src/ObjectCalendar.markedRefusalToast.test.tsx b/packages/plugin-calendar/src/ObjectCalendar.markedRefusalToast.test.tsx new file mode 100644 index 0000000000..f538c112ab --- /dev/null +++ b/packages/plugin-calendar/src/ObjectCalendar.markedRefusalToast.test.tsx @@ -0,0 +1,199 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * A rejected drag-to-reschedule must render the refusal text the PRODUCER + * marked as user-facing, instead of substituting a generic string for it. + * objectui#5902 — the calendar half of the same defect the kanban card-move + * toast carries, inheriting the objectui#5210 ruling. + * + * Same structure, and the same reasons, as + * `plugin-kanban/src/ObjectKanban.markedRefusalToast.test.tsx`: fixtures are + * built wire-shaped and pushed through the real `normaliseClientError` + * boundary, three MARKED arms are RED before the fix, three UNMARKED arms are + * GREEN before and after and exist so a fix that just printed `String(error)` + * cannot pass (objectstack#3821). + * + * The drop is driven through `CalendarView`'s real MonthView drag handlers — + * the same synthetic `DataTransfer` `CalendarView.dnd.test.tsx` uses, because + * jsdom does not round-trip a real one. `ObjectCalendar` is mounted WITHOUT an + * `onEventDrop` prop, which is precisely the condition that routes the drop + * into `handleEventDropDefault` — the persist path under test. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { toast } from '@object-ui/components'; +import { normaliseClientError } from '@object-ui/data-objectstack'; +import { ObjectCalendar } from './ObjectCalendar'; + +const MARKED = 'This visit cannot move outside the technician’s on-call window.'; + +const TITLE = 'Site visit'; + +/** + * Anchor both the record and the drop target inside the CURRENT month, so the + * month grid `ObjectCalendar` opens on always contains them — day 1 and day 8 + * are in every month's grid, whatever today's date is when the suite runs. + */ +const today = new Date(); +const dayInThisMonth = (d: number) => + new Date(today.getFullYear(), today.getMonth(), d, 9, 0, 0, 0); +const SOURCE_DAY = dayInThisMonth(1); +const TARGET_DAY = dayInThisMonth(8); + +/** The cell's own aria-label, built the way `MonthView` builds it. */ +const cellLabel = (d: Date) => + d.toLocaleDateString('default', { + weekday: 'long', + month: 'long', + day: 'numeric', + year: 'numeric', + }); + +const record = () => ({ + id: 'v1', + name: TITLE, + starts_at: SOURCE_DAY.toISOString(), +}); + +const schema = { + type: 'object-calendar', + objectName: 'visit', + calendar: { startDateField: 'starts_at', titleField: 'name' }, +} as never; + +const conflict = (userMessage?: string) => + normaliseClientError( + Object.assign(new Error('Record was modified by another user'), { + code: 'CONCURRENT_UPDATE', + httpStatus: 409, + details: { + currentVersion: '2026-05-22T07:14:00.000Z', + ...(userMessage ? { userMessage } : {}), + }, + }), + ); + +const validation = (userMessage?: string) => + normaliseClientError( + Object.assign(new Error('Validation failed'), { + code: 'VALIDATION_FAILED', + httpStatus: 400, + details: { + code: 'VALIDATION_FAILED', + fields: [{ field: 'starts_at', message: 'Outside the allowed window' }], + ...(userMessage ? { userMessage } : {}), + }, + }), + ); + +const forbidden = (userMessage?: string) => + normaliseClientError( + Object.assign(new Error('FORBIDDEN: insufficient privileges to update visit v1'), { + code: 'FORBIDDEN', + httpStatus: 403, + ...(userMessage ? { userMessage } : {}), + }), + ); + +function makeDataSource(rejectWith: unknown) { + return { + getObjectSchema: vi.fn(async () => ({ + name: 'visit', + fields: { name: { type: 'text' }, starts_at: { type: 'datetime' } }, + })), + find: vi.fn(async () => ({ value: [record()] })), + update: vi.fn(async () => { + throw rejectWith; + }), + }; +} + +/** jsdom has no DataTransfer round-trip; carry the payload in a synthetic one. */ +function performDnd(source: Element, target: Element) { + const store: Record = {}; + const dataTransfer = { + effectAllowed: '' as string, + dropEffect: '' as string, + setData: (k: string, v: string) => { + store[k] = v; + }, + getData: (k: string) => store[k] ?? '', + setDragImage: () => {}, + types: ['text/plain'], + }; + fireEvent.dragStart(source, { dataTransfer }); + fireEvent.dragOver(target, { dataTransfer }); + fireEvent.drop(target, { dataTransfer }); + fireEvent.dragEnd(source, { dataTransfer }); +} + +/** + * Mount the calendar on its OWN data (no `data` prop, no `onEventDrop`), drag + * the event a week forward, and return the single toast text. + */ +async function rescheduleAndReadToast(rejectWith: unknown): Promise { + const ds = makeDataSource(rejectWith); + render(); + + const pill = await screen.findByLabelText(TITLE); + const targetCell = screen.getByLabelText(new RegExp(`^${cellLabel(TARGET_DAY)}`)); + performDnd(pill, targetCell); + + await waitFor(() => expect(ds.update).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + return (toast.error as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][0]; +} + +beforeEach(() => { + vi.spyOn(toast, 'error').mockImplementation(() => 'toast-id' as never); + // The surface logs the raw error for the console; keep the run readable. + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe('ObjectCalendar — a marked refusal reaches the reschedule toast (#5902)', () => { + it('renders the marking on a 409 CONCURRENT_UPDATE (typed top-level member)', async () => { + expect(await rescheduleAndReadToast(conflict(MARKED))).toBe(MARKED); + }); + + it('renders the marking on a 400 VALIDATION_FAILED (details bag)', async () => { + expect(await rescheduleAndReadToast(validation(MARKED))).toBe(MARKED); + }); + + it('renders the marking on a 403, ahead of the "not authorized" substitution', async () => { + expect(await rescheduleAndReadToast(forbidden(MARKED))).toBe(MARKED); + }); + + it('keeps the generic string for an UNMARKED 409', async () => { + const said = await rescheduleAndReadToast(conflict()); + expect(said).not.toBe(MARKED); + expect(said).toBe('Record was modified by another user'); + }); + + it('keeps the generic string for an UNMARKED 400', async () => { + const said = await rescheduleAndReadToast(validation()); + expect(said).not.toBe(MARKED); + expect(said).toBe('Validation failed'); + }); + + it('keeps the localized substitution for an UNMARKED 403', async () => { + // objectstack#3821: an unmarked permission denial must NOT dump the server + // text ("…insufficient privileges to update visit v1") at the user. + const said = await rescheduleAndReadToast(forbidden()); + expect(said).toBe('You are not authorized to perform this action.'); + expect(String(said)).not.toContain('insufficient privileges'); + }); +}); diff --git a/packages/plugin-calendar/src/ObjectCalendar.tsx b/packages/plugin-calendar/src/ObjectCalendar.tsx index cdd1e879bc..7f97229e36 100644 --- a/packages/plugin-calendar/src/ObjectCalendar.tsx +++ b/packages/plugin-calendar/src/ObjectCalendar.tsx @@ -31,6 +31,7 @@ import { useSafeTranslate, extractWriteErrorMessage, isPermissionError, + declaredUserMessage, } from '@object-ui/react'; import { RecordDetailDrawer, deriveRecordPageHref } from '@object-ui/plugin-detail'; import { @@ -432,10 +433,19 @@ export const ObjectCalendar: React.FC = ({ // Surface the failure — never silently snap the event back. A row-level // security denial (403) is the common case: the user lacks permission to // reschedule this record. (cloud#864) + // …unless the AUTHOR opted in. `userMessage` (objectstack#9934) is the + // producer-side marking: a field set at throw time to say "this text is + // for the end user". It is a SEPARATE field from `message`, so nothing + // unmarked can reach here — the substitution below still governs every + // platform diagnostic and #3821 holds by construction rather than by us + // guessing what a body contains. Status-agnostic on purpose: 403 is + // where this was reported (objectui#5210/#5902), not a fence the + // contract draws — a marked 409 or 400 renders identically. toast.error( - isPermissionError(err) - ? tt('errors.unauthorized', 'You are not authorized to perform this action.') - : extractWriteErrorMessage(err) ?? tt('table.saveFailed', 'Save failed'), + declaredUserMessage(err) ?? + (isPermissionError(err) + ? tt('errors.unauthorized', 'You are not authorized to perform this action.') + : extractWriteErrorMessage(err) ?? tt('table.saveFailed', 'Save failed')), ); } }, [calendarConfig, schema.objectName, dataSource, data, tt]); diff --git a/packages/plugin-form/src/occSave.markedRefusal.test.tsx b/packages/plugin-form/src/occSave.markedRefusal.test.tsx new file mode 100644 index 0000000000..76ff353cdc --- /dev/null +++ b/packages/plugin-form/src/occSave.markedRefusal.test.tsx @@ -0,0 +1,179 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The THIRD drag-/record-write surface objectui#5902 covers, and the one its + * card did not name: `useOccSave`'s conflict dialog. + * + * The census the card carried was two (kanban card-move, calendar reschedule). + * `occSave.tsx` already reached into the conflict error for `currentVersion` + * but never for the producer's `userMessage`, so a 409 an author had marked + * showed the same canned sentence as every other 409 — the objectui#5210 + * defect, on a dialog instead of a toast. + * + * ── Why this surface renders the marking ADDITIVELY, not as a replacement ── + * The toast surfaces substitute: one slot, one string, the marking wins it. + * This dialog's description does two different jobs in one paragraph — it says + * WHY the write was refused, and it explains what the DESTRUCTIVE button will + * do ("Overwriting will replace their changes with yours."). `userMessage` is + * a refusal message; it is not affordance copy for a button this surface owns. + * Replacing the whole description would therefore leave "Overwrite" unexplained + * on the one surface where the choice is irreversible. So the marking is + * rendered first and in its own right, and the existing copy stays under it. + * Pinned below in both directions so the choice is visible rather than implied. + * + * ── Direction of these pins ─────────────────────────────────────────────── + * - marked 409 → RED before the fix (the marking was never rendered). + * - unmarked 409 → GREEN before and after: nothing the producer did not opt + * into may reach the user (objectstack#3821). + * - marked 400 → GREEN before and after. `saveWithOcc` rethrows everything + * that is not a conflict, and the caller (form.tsx, already + * fixed by objectui#5210) renders the marking. Pinned here so + * a future change to this seam cannot quietly swallow or + * re-wrap the marking on its way past. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor, act } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { declaredUserMessage } from '@object-ui/react'; +import { normaliseClientError } from '@object-ui/data-objectstack'; +import { useOccSave, type OccSaveOutcome } from './occSave'; + +const MARKED = 'Reload first — the quote was re-priced while you were editing.'; + +/** A fragment of the canned copy, distinguishable from the marking above. */ +const GENERIC = /This record was changed by someone else while you were editing/; +/** The destructive button's own explanation, which must survive the fix. */ +const AFFORDANCE = /Overwriting will replace their changes with yours/; + +const conflict = (userMessage?: string) => + normaliseClientError( + Object.assign(new Error('Record was modified by another user'), { + code: 'CONCURRENT_UPDATE', + httpStatus: 409, + details: { + currentVersion: '2026-05-22T07:14:00.000Z', + ...(userMessage ? { userMessage } : {}), + }, + }), + ); + +const validation = (userMessage?: string) => + normaliseClientError( + Object.assign(new Error('Validation failed'), { + code: 'VALIDATION_FAILED', + httpStatus: 400, + details: { + code: 'VALIDATION_FAILED', + fields: [{ field: 'amount', message: 'Amount is not allowed' }], + ...(userMessage ? { userMessage } : {}), + }, + }), + ); + +/** + * The smallest honest host for the hook: it renders the real `conflictDialog` + * and drives the real `saveWithOcc`, with nothing between them and the test. + */ +function harness(rejectWith: unknown) { + const outcome: { value?: OccSaveOutcome; error?: unknown } = {}; + const dataSource = { + update: vi.fn(async () => { + throw rejectWith; + }), + }; + + const Host: React.FC = () => { + const { saveWithOcc, conflictDialog } = useOccSave(); + return ( +
+ + {conflictDialog} +
+ ); + }; + + render(); + return { outcome, dataSource }; +} + +const save = async () => { + await act(async () => { + fireEvent.click(screen.getByText('save')); + }); +}; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe('useOccSave — a marked refusal reaches the conflict dialog (#5902)', () => { + it('renders the marking on a MARKED 409, above the copy it does not replace', async () => { + harness(conflict(MARKED)); + await save(); + + // The gate: pre-fix the dialog opens with the canned copy only. + expect(await screen.findByText(MARKED)).toBeInTheDocument(); + // …and the destructive button keeps its explanation. The marking is a + // refusal message, not affordance copy — it augments, it does not evict. + expect(screen.getByText(AFFORDANCE, { exact: false })).toBeInTheDocument(); + + // Settle the pending decision so nothing is left awaiting at teardown. + fireEvent.click(screen.getByText('Keep editing')); + await waitFor(() => expect(screen.queryByText('Keep editing')).toBeNull()); + }); + + it('says nothing extra on an UNMARKED 409', async () => { + harness(conflict()); + await save(); + + expect(await screen.findByText(GENERIC, { exact: false })).toBeInTheDocument(); + expect(screen.queryByText(MARKED)).toBeNull(); + + fireEvent.click(screen.getByText('Keep editing')); + await waitFor(() => expect(screen.queryByText('Keep editing')).toBeNull()); + }); + + it('rethrows a MARKED non-conflict rejection with the marking intact', async () => { + // The second error shape never reaches this surface's own rendering — it + // is rethrown for the caller. What this seam owes is that it passes it on + // UNCHANGED, so the caller's `declaredUserMessage` still finds the marking. + const marked = validation(MARKED); + const { outcome } = harness(marked); + await save(); + + await waitFor(() => expect(outcome.error).toBeDefined()); + expect(outcome.error).toBe(marked); + expect(declaredUserMessage(outcome.error)).toBe(MARKED); + // No conflict dialog for a shape that is not a conflict. + expect(screen.queryByText('Keep editing')).toBeNull(); + }); +}); diff --git a/packages/plugin-form/src/occSave.tsx b/packages/plugin-form/src/occSave.tsx index caf7f01334..cf0c8c4a8c 100644 --- a/packages/plugin-form/src/occSave.tsx +++ b/packages/plugin-form/src/occSave.tsx @@ -42,6 +42,7 @@ import { } from '@object-ui/components'; import { AlertTriangle } from 'lucide-react'; import { createSafeTranslation } from '@object-ui/i18n'; +import { declaredUserMessage } from '@object-ui/react'; // Localized strings for the conflict dialog. Falls back to English when no // i18n provider is mounted (createSafeTranslation handles that). @@ -101,6 +102,15 @@ export interface OccSaveArgs { interface ConflictState { /** Latest server-side version from the 409, for the overwrite retry. */ currentVersion?: string; + /** + * The refusal text the PRODUCER marked as addressed to the end user + * (`userMessage`, objectstack#9934), or `undefined` when the 409 carried no + * marking. Read through `declaredUserMessage`, never duck-typed here: the + * marking lands in two different places depending on which error the adapter + * built (a typed member on `ConcurrentUpdateError`, the details bag on + * `DataApiValidationError`), and that reader is the one place that knows both. + */ + userMessage?: string; } /** Backend ships SQL-style "YYYY-MM-DD HH:mm:ss.SSS"; normalise for Date. */ @@ -149,10 +159,18 @@ export function useOccSave(): { } catch (err) { if (!isConcurrentUpdateError(err)) throw err; const currentVersion = (err as { currentVersion?: unknown }).currentVersion; + // A 409 an author MARKED says something this dialog's canned copy + // cannot — which record, which team, what to do next. It used to be + // dropped here, so every conflict read identically no matter what the + // producer wrote (objectui#5902, inheriting objectui#5210's ruling). + // Everything unmarked still answers `null`, so objectstack#3821's + // protection is untouched. + const marked = declaredUserMessage(err); const overwrite = await new Promise((resolve) => { decisionRef.current = resolve; setConflict({ currentVersion: typeof currentVersion === 'string' ? currentVersion : undefined, + userMessage: marked ?? undefined, }); }); if (!overwrite) return { status: 'cancelled' }; @@ -182,6 +200,21 @@ export function useOccSave(): { {t('form.conflictTitle')} + {/* + The producer's marking leads, in its own right — and does NOT + replace the sentence under it. That sentence does two jobs in one + paragraph: it says why the write was refused, AND it explains what + the destructive button will do. `userMessage` is a refusal + message, not affordance copy this surface owns, so evicting the + paragraph would leave "Overwrite" unexplained on the one surface + where the choice is irreversible. Rendered verbatim: the author + already wrote and localized it for their user. + */} + {conflict?.userMessage && ( + + {conflict.userMessage} + + )} {t('form.conflictMessage')} {latestTime && ( diff --git a/packages/plugin-kanban/src/ObjectKanban.markedRefusalToast.test.tsx b/packages/plugin-kanban/src/ObjectKanban.markedRefusalToast.test.tsx new file mode 100644 index 0000000000..e95237a97f --- /dev/null +++ b/packages/plugin-kanban/src/ObjectKanban.markedRefusalToast.test.tsx @@ -0,0 +1,243 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * A rejected card move must render the refusal text the PRODUCER marked as + * user-facing, instead of substituting a generic string for it. objectui#5902, + * inheriting the objectui#5210 ruling (already implemented for the console + * form) onto this drag-write surface. + * + * ── Why this needs a surface pin and not a reader unit test ──────────────── + * `declaredUserMessage` already has unit tests, and + * `error-message.normalisation-boundary.test.ts` already pins that the marking + * survives the adapter's re-wraps. Neither of them can see this defect: the + * marking arrived at this file intact and the toast dropped it on the floor. + * The only thing that fails when a surface stops reading the marking is a pin + * ON that surface, driven end to end. + * + * ── Fixtures come through the real boundary ─────────────────────────────── + * Every error below is built as a wire-shaped rejection and pushed through + * `normaliseClientError`, the same adapter boundary a real `dataSource.update` + * failure crosses. Hand-rolling the post-boundary shape here would pin this + * surface against THIS file's idea of where the marking lives — which is the + * asymmetry that makes it worth pinning at all: `ConcurrentUpdateError` parks + * it on a typed top-level member, `DataApiValidationError` parks it in the + * details bag, and a 403 is passed through untouched. All three must reach the + * user identically, because the contract is status-agnostic. + * + * ── Direction of these pins ─────────────────────────────────────────────── + * - the three MARKED arms → RED before the fix (they read the generic + * substitution), GREEN after. These are the issue. + * - the three UNMARKED arms → GREEN before and after. Without them a fix that + * simply printed `String(error)` would pass, and + * that is the objectstack#3821 defect this + * surface's substitution exists to prevent. + * + * The board is driven through `DndContext`'s real `onDragEnd`, captured by the + * module mock below — the same harness `ObjectKanban.rejectedMoveRollback.test.tsx` + * uses, and for the same reason: dnd-kit's pointer sensors need layout and + * pointer-capture that jsdom does not provide. + */ + +import React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, act, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { registerAllFields } from '@object-ui/fields'; +import { toast } from '@object-ui/components'; +import { normaliseClientError } from '@object-ui/data-objectstack'; +import type { DataSource } from '@object-ui/types'; +import { ObjectKanban } from './ObjectKanban'; + +// Pay the board's lazy chunk at import time rather than racing it against a +// `findBy` budget (AGENTS.md §测试纪律); specifier byte-identical to `./index`'s +// so the component's own `React.lazy` factory resolves from the ESM cache. +import './KanbanImpl'; + +// `vi.hoisted` so the mock factory — hoisted above every import — can reach this +// box. A plain `const` would still be in its TDZ when `@dnd-kit/core` is first +// requested by `KanbanImpl`. +const dnd = vi.hoisted(() => ({ + onDragEnd: undefined as undefined | ((event: unknown) => void), +})); + +vi.mock('@dnd-kit/core', async (importOriginal) => { + const actual = await importOriginal(); + const ReactMod = await import('react'); + const CapturingDndContext = (props: Record) => { + dnd.onDragEnd = props.onDragEnd as (event: unknown) => void; + return ReactMod.createElement(actual.DndContext, props as never); + }; + return { ...actual, DndContext: CapturingDndContext }; +}); + +registerAllFields(); + +/** + * The sentence the application author wrote for their user. Deliberately + * unlike every generic string this surface can produce — "Save failed", "You + * are not authorized to perform this action.", and the raw server text — so an + * assertion on it cannot pass by accident on the pre-fix code path. + */ +const MARKED = 'Cards cannot leave Backlog until finance signs off.'; + +const objectDef = { + name: 'task', + fields: { + title: { type: 'text', label: 'Title' }, + status: { + type: 'picklist', + label: 'Status', + options: [ + { value: 'backlog', label: 'Backlog' }, + { value: 'in_progress', label: 'In Progress' }, + ], + }, + }, +}; + +const CARD = 'Fix the widget'; + +const schema = { + type: 'object-kanban', + objectName: 'task', + groupBy: 'status', + cardTitle: 'title', + columns: [ + { id: 'backlog', title: 'Backlog' }, + { id: 'in_progress', title: 'In Progress' }, + ], +} as never; + +const serverRecords = () => [{ id: 't1', title: CARD, status: 'backlog' }]; + +/** 409 CONCURRENT_UPDATE as it arrives on the wire, optionally marked. */ +const conflict = (userMessage?: string) => + normaliseClientError( + Object.assign(new Error('Record was modified by another user'), { + code: 'CONCURRENT_UPDATE', + httpStatus: 409, + details: { + currentVersion: '2026-05-22T07:14:00.000Z', + ...(userMessage ? { userMessage } : {}), + }, + }), + ); + +/** 400 VALIDATION_FAILED as it arrives on the wire, optionally marked. */ +const validation = (userMessage?: string) => + normaliseClientError( + Object.assign(new Error('Validation failed'), { + code: 'VALIDATION_FAILED', + httpStatus: 400, + details: { + code: 'VALIDATION_FAILED', + fields: [{ field: 'status', message: 'Invalid status transition' }], + ...(userMessage ? { userMessage } : {}), + }, + }), + ); + +/** + * 403 — returned by `normaliseClientError` untouched, so the marking sits on + * the error itself. This is the arm the ruling was reported on, and the one + * where the substitution is strongest: `isPermissionError` claims it, so the + * marked text has to win ahead of the "not authorized" string. + */ +const forbidden = (userMessage?: string) => + normaliseClientError( + Object.assign(new Error('FORBIDDEN: insufficient privileges to update task t1'), { + code: 'FORBIDDEN', + httpStatus: 403, + ...(userMessage ? { userMessage } : {}), + }), + ); + +function makeDataSource(rejectWith: unknown): DataSource { + return { + getObjectSchema: vi.fn(async () => objectDef), + find: vi.fn(async () => ({ value: serverRecords() })), + update: vi.fn(async () => { + throw rejectWith; + }), + } as unknown as DataSource; +} + +/** Mount the board and settle every async state update before the drag. */ +async function mountBoard(dataSource: DataSource) { + render(); + expect(await screen.findByText(CARD)).toBeInTheDocument(); + await act(async () => { + await Promise.resolve(); + await Promise.resolve(); + }); +} + +/** Drop the card onto "In Progress" and return the single toast text. */ +async function dropAndReadToast(): Promise { + expect(dnd.onDragEnd).toBeTypeOf('function'); + await act(async () => { + dnd.onDragEnd!({ active: { id: 't1' }, over: { id: 'in_progress' } }); + }); + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)); + return (toast.error as unknown as { mock: { calls: unknown[][] } }).mock.calls[0][0]; +} + +beforeEach(() => { + dnd.onDragEnd = undefined; + vi.spyOn(toast, 'error').mockImplementation(() => 'toast-id' as never); + // The surface logs the raw error for the console; keep the run readable. + vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe('ObjectKanban — a marked refusal reaches the card-move toast (#5902)', () => { + it('renders the marking on a 409 CONCURRENT_UPDATE (typed top-level member)', async () => { + await mountBoard(makeDataSource(conflict(MARKED))); + expect(await dropAndReadToast()).toBe(MARKED); + }); + + it('renders the marking on a 400 VALIDATION_FAILED (details bag)', async () => { + await mountBoard(makeDataSource(validation(MARKED))); + expect(await dropAndReadToast()).toBe(MARKED); + }); + + it('renders the marking on a 403, ahead of the "not authorized" substitution', async () => { + // Status-agnostic by contract: a 403 is where this was reported, not a + // fence the marking respects. + await mountBoard(makeDataSource(forbidden(MARKED))); + expect(await dropAndReadToast()).toBe(MARKED); + }); + + it('keeps the generic string for an UNMARKED 409', async () => { + await mountBoard(makeDataSource(conflict())); + const said = await dropAndReadToast(); + expect(said).not.toBe(MARKED); + expect(said).toBe('Record was modified by another user'); + }); + + it('keeps the generic string for an UNMARKED 400', async () => { + await mountBoard(makeDataSource(validation())); + const said = await dropAndReadToast(); + expect(said).not.toBe(MARKED); + expect(said).toBe('Validation failed'); + }); + + it('keeps the localized substitution for an UNMARKED 403', async () => { + // objectstack#3821: an unmarked permission denial must NOT dump the server + // text ("…insufficient privileges to update task t1") in front of the user. + await mountBoard(makeDataSource(forbidden())); + const said = await dropAndReadToast(); + expect(said).toBe('You are not authorized to perform this action.'); + expect(String(said)).not.toContain('insufficient privileges'); + }); +}); diff --git a/packages/plugin-kanban/src/ObjectKanban.tsx b/packages/plugin-kanban/src/ObjectKanban.tsx index 6e760e903a..2c99db839a 100644 --- a/packages/plugin-kanban/src/ObjectKanban.tsx +++ b/packages/plugin-kanban/src/ObjectKanban.tsx @@ -15,6 +15,7 @@ import { useSafeTranslate, extractWriteErrorMessage, isPermissionError, + declaredUserMessage, } from '@object-ui/react'; import { toast } from '@object-ui/components'; import { createSafeTranslation } from '@object-ui/i18n'; @@ -704,10 +705,19 @@ export const ObjectKanban: React.FC = ({ // Surface the failure — never silently snap the card back. A row-level // security denial (403) is the common case: the user lacks permission // to change this record's status. (cloud#864) + // …unless the AUTHOR opted in. `userMessage` (objectstack#9934) is the + // producer-side marking: a field set at throw time to say "this text is + // for the end user". It is a SEPARATE field from `message`, so nothing + // unmarked can reach here — the substitution below still governs every + // platform diagnostic and #3821 holds by construction rather than by us + // guessing what a body contains. Status-agnostic on purpose: 403 is + // where this was reported (objectui#5210/#5902), not a fence the + // contract draws — a marked 409 or 400 renders identically. toast.error( - isPermissionError(err) - ? tt('errors.unauthorized', 'You are not authorized to perform this action.') - : extractWriteErrorMessage(err) ?? tt('table.saveFailed', 'Save failed'), + declaredUserMessage(err) ?? + (isPermissionError(err) + ? tt('errors.unauthorized', 'You are not authorized to perform this action.') + : extractWriteErrorMessage(err) ?? tt('table.saveFailed', 'Save failed')), ); // Roll the optimistic move back, on BOTH data ownerships (#4138). // From e5fb2be3b4bc8b437cd0626beec6094543d26daf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 17:21:09 +0000 Subject: [PATCH 2/2] chore: declare the drag-write refusal fix as a patch release Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- ...5902-marked-refusal-drag-write-surfaces.md | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .changeset/5902-marked-refusal-drag-write-surfaces.md diff --git a/.changeset/5902-marked-refusal-drag-write-surfaces.md b/.changeset/5902-marked-refusal-drag-write-surfaces.md new file mode 100644 index 0000000000..831e3de14c --- /dev/null +++ b/.changeset/5902-marked-refusal-drag-write-surfaces.md @@ -0,0 +1,25 @@ +--- +'@object-ui/plugin-kanban': patch +'@object-ui/plugin-calendar': patch +'@object-ui/plugin-form': patch +--- + +Let a producer-marked refusal reach the drag-write surfaces (objectui#5902). + +The kanban card-move toast, the calendar drag-to-reschedule toast and the OCC +conflict dialog each substituted a generic string for a refusal the producer had +marked as user-facing (`userMessage`), so a user was told "Save failed" where the +application author had written a sentence addressed to them. All three now read +the marking through the shared `declaredUserMessage` reader, which covers both +places the adapter boundary parks it — the typed member on +`ConcurrentUpdateError` and the details bag on `DataApiValidationError`. + +Nothing unmarked changes: the reader answers `null` for it, so every existing +generic substitution — including the localized "not authorized" message that +keeps raw server diagnostics away from end users — still governs unmarked +refusals exactly as before. + +The two toasts substitute; the conflict dialog augments. Its description also +explains what the destructive "Overwrite" button does, which is affordance copy +that surface owns rather than a refusal message, so the marking leads and that +paragraph stays.