diff --git a/.changeset/27999-save-with-warnings-one-toast.md b/.changeset/27999-save-with-warnings-one-toast.md new file mode 100644 index 00000000000..9610feafdae --- /dev/null +++ b/.changeset/27999-save-with-warnings-one-toast.md @@ -0,0 +1,11 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/console': patch +--- + +fix(console): a record save whose answer carries warnings shows one toast, the record-named save message carrying the warning lines + +Clause-②: no +Metadata-protocol: none + +A save from the record form (the overlay dialog, its child-form task, or the full-page form) that passed a `severity: 'warning'` validation rule showed two toasts: the write-warning channel's "Saved — with warnings" with the rule messages, then the form's own `OBJECT "TITLE" updated`. It now shows one: the record-named save message, on the warning variant, with each distinct rule message on its own line. While a record form is open, the write-warning channel hands the answer warnings of the write that form makes (that record's update, or a create of that object) to the form instead of toasting them; the stripped-fields notice keeps its own toast, and any other write keeps the "Saved — with warnings" toast. diff --git a/ui/packages/app-shell/src/console/AppContent.tsx b/ui/packages/app-shell/src/console/AppContent.tsx index cf41e0a1cee..671d52d9b18 100644 --- a/ui/packages/app-shell/src/console/AppContent.tsx +++ b/ui/packages/app-shell/src/console/AppContent.tsx @@ -22,6 +22,7 @@ import type { AppAccessVerdict, ConnectionState } from '@object-ui/data-objectst import { useAuth, useWorkspaceAdminStatus } from '@object-ui/auth'; import { useMetadata } from '../providers/MetadataProvider.js'; import { useAdapter } from '../providers/AdapterProvider.js'; +import { claimAnswerWarnings, toastRecordSave, type AnswerWarningClaim } from '../providers/writeWarningToast.js'; import { usePreviewDrafts } from '../preview/PreviewModeContext.js'; import { PreviewDraftEmptyState } from '../preview/PreviewDraftEmptyState.js'; import { @@ -764,7 +765,9 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = runner.registerHandler('crud_success', async (action: any) => { closeRecordForm(); setRefreshKey(k => k + 1); - toast.success(action.params?.message ?? 'Record saved successfully'); + // `warnings`: the save's own advisory warning lines, which this one + // toast carries (objectstack-ai/objectstack#27999). + toastRecordSave(toast, action.params?.message ?? 'Record saved successfully', action.params?.warnings); return { success: true, reload: true }; }); @@ -901,10 +904,30 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = : t('form.createSuccess', { object: label, defaultValue: `${label} created successfully` }); }, [editingRecord, formObjectDef, objectLabel, t]); - const handleCrudSuccess = useCallback((saved?: any) => { + // objectstack-ai/objectstack#27999 — the open record form reports its own + // write's advisory warnings: while the overlay is open, the write-warning + // channel parks the answer warnings of the write it names (this record's + // update, or a create of this object) on the claim instead of toasting them, + // and the save toast carries them. Keyed on the URL's primitives, not on the + // memoised `editingRecord` / `formObjectDef` (ui/AGENTS.md #10). + const answerWarningClaim = useRef(null); + const formObjectName: string | undefined = formObjectDef?.name; + useEffect(() => { + if (!dataSource || !recordFormParam || !formObjectName) return; + const claim = claimAnswerWarnings(dataSource, recordFormParam === 'new' + ? { operation: 'create', resource: formObjectName } + : { operation: 'update', resource: formObjectName, id: recordFormParam }); + answerWarningClaim.current = claim; + return () => { + if (answerWarningClaim.current === claim) answerWarningClaim.current = null; + claim.release(); + }; + }, [dataSource, recordFormParam, formObjectName]); + + const handleCrudSuccess = useCallback((saved?: any, warnings?: string[]) => { executeAction({ type: 'crud_success', - params: { message: recordSaveMessage(saved) }, + params: { message: recordSaveMessage(saved), warnings }, }); }, [executeAction, recordSaveMessage]); @@ -925,12 +948,13 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = // the child's open related lists refetch on their own via the // invalidation bus (#2269): the dataSource write already emitted a // MutationEvent that the bridge fans out — no manual notify needed. + const warnings = answerWarningClaim.current?.take(saved) ?? []; if (isChildFormTask) { - toast.success(recordSaveMessage(saved)); + toastRecordSave(toast, recordSaveMessage(saved), warnings); closeRecordForm(); return; } - handleCrudSuccess(saved); + handleCrudSuccess(saved, warnings); if (editingRecord || !currentObjectDef) return; const target = resolvePostCreateTarget({ objectName: currentObjectDef.name, diff --git a/ui/packages/app-shell/src/console/__tests__/AppContent.recordSaveOneToast-27999.test.tsx b/ui/packages/app-shell/src/console/__tests__/AppContent.recordSaveOneToast-27999.test.tsx new file mode 100644 index 00000000000..f94271d1be6 --- /dev/null +++ b/ui/packages/app-shell/src/console/__tests__/AppContent.recordSaveOneToast-27999.test.tsx @@ -0,0 +1,282 @@ +/** + * 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. + */ + +/** + * objectstack-ai/objectstack#27999 — an overlay record save whose answer + * carries advisory warnings (a `severity: 'warning'` validation rule) shows ONE + * toast: the record-named save message (objectstack-ai/objectstack#26172), + * carrying the warning lines. The write-warning channel's "Saved — with + * warnings" toast is not raised for that write; its stripped-fields notice + * still is, and a write the overlay did not make keeps the channel toast. + * + * Measured from a real `AppContent` mount (the + * `AppContent.recordSaveToastNamesRecord-26172` harness): the global record-form + * modal is a stand-in, and the save does what the real chain does, in its + * order: the adapter emits the write-warning event INSIDE the write (here the + * channel's `emitWriteWarning`, handed the adapter as `AdapterProvider` hands + * it, and not awaited), then the form hands the write's result to `onSuccess`. + * The runner runs the handler `AppContent` registered, so the main path's + * toast is the real `crud_success` one; the child-form task toasts directly. + * The channel's sink is the same sonner `toast`, so every toast is counted. + * + * Direction, written before the run: with the channel ignoring the overlay's + * claim, every "one toast" cell RED (a second "Saved — with warnings" toast, + * and a success toast carrying no warning line); the CONTROL cells GREEN on + * both sides. + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach, afterEach, afterAll } from 'vitest'; +import React from 'react'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { toast } from 'sonner'; +import { createI18n, I18nProvider } from '@object-ui/i18n'; +import type { WriteWarningEvent } from '@object-ui/data-objectstack'; +import { emitWriteWarning } from '../../providers/writeWarningToast'; + +/** The part of the modal's schema this file reads. */ +interface ModalSchemaProbe { + objectName?: string; + onSuccess: (saved: Record) => Promise | void; +} +const { modalSchemas, handlers, execute } = vi.hoisted(() => { + const handlers = new Map Promise>(); + return { + modalSchemas: [] as ModalSchemaProbe[], + handlers, + execute: vi.fn(async (action: { type?: string }) => handlers.get(action?.type ?? '')?.(action)), + }; +}); + +/** The global record-form modal: its schema carries the `onSuccess` under test. */ +vi.mock('@object-ui/plugin-form', async (importOriginal) => ({ + ...(await importOriginal>()), + ModalForm: ({ schema }: { schema: ModalSchemaProbe }) => { + modalSchemas.push(schema); + return
; + }, +})); + +vi.mock('@object-ui/plugin-designer', async (importOriginal) => ({ + ...(await importOriginal()), + CreateAppPage: () =>
, + EditAppPage: () =>
, + DashboardDesignPage: () =>
, +})); + +vi.mock('../../layout/ConsoleLayout', () => ({ + ConsoleLayout: ({ children }: { children?: React.ReactNode }) => ( +
{children}
+ ), +})); +vi.mock('../../chrome/CommandPalette', () => ({ CommandPalette: () => null })); +vi.mock('../../chrome/KeyboardShortcutsDialog', () => ({ KeyboardShortcutsDialog: () => null })); +vi.mock('../../chrome/OnboardingWalkthrough', () => ({ OnboardingWalkthrough: () => null })); +vi.mock('../../views/ObjectView', () => ({ ObjectView: () =>
})); +vi.mock('../../views/RecordDetailView', () => ({ RecordDetailView: () =>
})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ + user: { id: 'u1', name: 'Ada', email: 'ada@example.com', role: 'user' }, + getAuthConfig: async () => ({ features: {} }), + activeOrganization: null, + }), + useWorkspaceAdminStatus: () => ({ isAdmin: true, isResolved: true }), +})); + +// The adapter the overlay claims on, and the one the channel is handed as +// `AdapterProvider` hands it: the same object. +const dataSourceStub = { + onConnectionStateChange: () => () => {}, + getConnectionState: () => 'connected', + getObjectSchema: async () => ({ fields: {} as Record }), +}; +vi.mock('../../providers/AdapterProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useAdapter: () => dataSourceStub, +})); + +const OBJECTS = [ + // Declares its name field: a saved task is named by `subject`. + { + name: 'showcase_task', + label: 'Task', + nameField: 'subject', + fields: { subject: { type: 'text', label: 'Subject' }, done: { type: 'boolean', label: 'Done' } }, + }, + // The parent the child-form task opens over. + { name: 'showcase_project', label: 'Project', fields: { name: { type: 'text', label: 'Name' } } }, +]; + +const APPS = [{ name: 'crm', label: 'CRM', isDefault: true, navigation: [] }]; + +// One value for the whole run: the route's effects key on these members. +const METADATA = { + apps: APPS, + objects: OBJECTS, + loading: false, + ensureType: undefined, + error: null, + refresh: async () => {}, +}; +vi.mock('../../providers/MetadataProvider', async (importOriginal) => ({ + ...(await importOriginal>()), + useMetadata: () => METADATA, +})); + +// The runner runs what `AppContent` registered, so the `crud_success` toast is +// the real handler's. +const ACTION_RUNNER = { + execute, + runner: { + registerHandler: (type: string, fn: (action: unknown) => Promise) => { handlers.set(type, fn); }, + getContext: () => ({}), + }, +}; +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + useActionRunner: () => ACTION_RUNNER, + useGlobalUndo: () => {}, + useMutationInvalidationBridge: () => {}, +})); + +// objectstack#27138 — an edit dialog reads the approvals state of the record it +// edits (`useRecordApprovals`). No approval runs in this file: the approvals +// routes answer empty from one double, installed for the whole file so no read +// can reach a socket after a test body returns, and handed back in `afterAll`. +vi.stubGlobal('fetch', vi.fn(async () => new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, +}))); +afterAll(() => vi.unstubAllGlobals()); + +import { AppContent } from '../AppContent'; + +/** The channel's translator: the English default, interpolated. */ +const t = (_key: string, opts?: Record) => + String(opts?.defaultValue ?? '').replace('{{fields}}', String(opts?.fields ?? '')); +const identityLabel = (_o: string, _f: string, fallback: string) => fallback; + +const MAIN_CREATE = '/apps/crm/showcase_task?form=new'; +const MAIN_EDIT = '/apps/crm/showcase_task?form=t1'; +const CHILD_EDIT = '/apps/crm/showcase_project/record/p1?form=t1&formObject=showcase_task'; + +const ADVISORY = { + field: 'subject', + code: 'validation_rule', + message: 'Reviews are usually scheduled a week ahead', + rule: 'review_lead_time', + severity: 'warning' as const, +}; +const SECOND = { ...ADVISORY, field: 'done', rule: 'done_owner', message: 'Done tasks usually name an owner' }; +const NAMED = { id: 't1', subject: 'Quarterly review' }; + +let toastSuccess: ReturnType; +let toastWarning: ReturnType; +beforeEach(() => { + modalSchemas.length = 0; + execute.mockClear(); + toastSuccess = vi.spyOn(toast, 'success'); + toastWarning = vi.spyOn(toast, 'warning'); +}); +afterEach(() => { + toastSuccess.mockRestore(); + toastWarning.mockRestore(); + cleanup(); +}); + +/** Mount at `url`, save `saved` with `event` emitted by the write, and settle every toast. */ +async function saveFrom(url: string, event: WriteWarningEvent, saved: Record): Promise { + const i18n = createI18n({ defaultLanguage: 'en', detectBrowserLanguage: false }); + render( + + + + } /> + + + , + ); + await waitFor(() => expect(modalSchemas.length).toBeGreaterThan(0)); + // The overlay's claim is opened by an effect of the mounted, open dialog. + await waitFor(() => expect(handlers.has('crud_success')).toBe(true)); + const schema = modalSchemas[modalSchemas.length - 1]; + expect(schema.objectName).toBe('showcase_task'); + const channel = emitWriteWarning(event, t, dataSourceStub as never, identityLabel, toast); + await schema.onSuccess(saved); + await channel; + await waitFor(() => expect(toastSuccess.mock.calls.length + toastWarning.mock.calls.length).toBeGreaterThan(0)); +} + +const update = (over: Partial = {}): WriteWarningEvent => ({ + operation: 'update', + resource: 'showcase_task', + id: 't1', + droppedFields: [], + warnings: [ADVISORY], + ...over, +}); + +describe('AppContent — an overlay save with warnings shows one record-named toast (objectstack-ai/objectstack#27999)', () => { + it('update (crud_success): one toast, the record-named message carrying the warning line', async () => { + await saveFrom(MAIN_EDIT, update(), NAMED); + + expect(toastSuccess).not.toHaveBeenCalled(); + expect(toastWarning.mock.calls).toEqual([ + ['Task "Quarterly review" updated', { description: ADVISORY.message }], + ]); + }); + + it('create (crud_success): one toast, each distinct warning on its own line', async () => { + await saveFrom( + MAIN_CREATE, + { operation: 'create', resource: 'showcase_task', id: 't1', droppedFields: [], warnings: [ADVISORY, SECOND] }, + NAMED, + ); + + expect(toastSuccess).not.toHaveBeenCalled(); + expect(toastWarning.mock.calls).toEqual([ + ['Task "Quarterly review" created', { description: `${ADVISORY.message}\n${SECOND.message}` }], + ]); + }); + + it('child-form task: the direct toast is the one toast', async () => { + await saveFrom(CHILD_EDIT, update(), NAMED); + + expect(toastSuccess).not.toHaveBeenCalled(); + expect(toastWarning.mock.calls).toEqual([ + ['Task "Quarterly review" updated', { description: ADVISORY.message }], + ]); + }); + + it('the stripped-fields notice keeps its own toast beside the save toast', async () => { + await saveFrom( + MAIN_EDIT, + update({ droppedFields: [{ object: 'showcase_task', fields: ['done'], reason: 'readonly' }] }), + NAMED, + ); + + expect(toastSuccess).not.toHaveBeenCalled(); + expect(toastWarning.mock.calls.map((call: unknown[]) => call[0]).sort()).toEqual([ + 'Saved — but some fields did not take effect', + 'Task "Quarterly review" updated', + ]); + expect(toastWarning.mock.calls.find((call: unknown[]) => call[0] === 'Task "Quarterly review" updated')?.[1]).toEqual({ + description: ADVISORY.message, + }); + }); + + it('CONTROL: a write the overlay did not make keeps the channel toast, and the save toast stays a success', async () => { + await saveFrom(MAIN_EDIT, update({ id: 't9' }), NAMED); + + expect(toastSuccess.mock.calls).toEqual([['Task "Quarterly review" updated']]); + expect(toastWarning.mock.calls).toEqual([['Saved — with warnings', { description: ADVISORY.message }]]); + }); +}); diff --git a/ui/packages/app-shell/src/providers/writeWarningToast.test.ts b/ui/packages/app-shell/src/providers/writeWarningToast.test.ts index eaaf323cdce..f2fe3c2645b 100644 --- a/ui/packages/app-shell/src/providers/writeWarningToast.test.ts +++ b/ui/packages/app-shell/src/providers/writeWarningToast.test.ts @@ -36,7 +36,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { DroppedFieldsEventSchema } from '@objectstack/spec/data'; -import { emitWriteWarning, type WriteWarningSink } from './writeWarningToast'; +import { claimAnswerWarnings, emitWriteWarning, type WriteWarningSink } from './writeWarningToast'; import { ObjectStackAdapter } from '@object-ui/data-objectstack'; import { createI18n, loadBuiltInLocale, type TranslateFn } from '@object-ui/i18n'; import type { DroppedFieldsEvent, WriteWarningEvent } from '@object-ui/data-objectstack'; @@ -487,3 +487,104 @@ describe('emitWriteWarning — write answer warnings (objectui#12107)', () => { expect(adapter.getObjectSchema).not.toHaveBeenCalled(); }); }); + +// ── a record save surface reports its own write's warnings (objectstack-ai/objectstack#27999) ── +// +// Same real chain as above. The claim is keyed on the object the listener +// hands `emitWriteWarning` as its adapter — in `AdapterProvider` that is the +// adapter itself, the one the save surfaces read through `useAdapter`. + +describe('claimAnswerWarnings — the save surface reports its write (objectstack-ai/objectstack#27999)', () => { + const WARNED = 'Saved — with warnings'; + + beforeEach(() => { + vi.clearAllMocks(); + adapter.getObjectSchema.mockResolvedValue(ANDON_SCHEMA); + }); + + it('an update the claim names: the channel shows nothing, the surface takes the message', async () => { + const update = vi.fn().mockResolvedValue({ record: { id: 'r1' }, warnings: [ADVISORY] }); + const { ds, calls, settle } = wiredAdapter({ update }); + const claim = claimAnswerWarnings(adapter, { operation: 'update', resource: 'quote', id: 'r1' }); + + const saved = await ds.update('quote', 'r1', { discount: 40 }); + // Taken as the surface's success handler would, before the channel settles. + expect(claim.take(saved)).toEqual([ADVISORY.message]); + await settle(); + claim.release(); + + expect(calls).toEqual([]); + }); + + it('a create the claim names: its own record id is the surface\'s, another create\'s keeps the channel toast', async () => { + const other = { ...ADVISORY, message: 'Another create, another rule' }; + const create = vi.fn() + .mockResolvedValueOnce({ record: { id: 'q9' }, warnings: [other] }) + .mockResolvedValueOnce({ record: { id: 'q1' }, warnings: [ADVISORY] }); + const { ds, calls, settle } = wiredAdapter({ create }); + const claim = claimAnswerWarnings(adapter, { operation: 'create', resource: 'quote' }); + + await ds.create('quote', { discount: 50 }); + const saved = await ds.create('quote', { discount: 40 }); + await settle(); + expect(calls).toEqual([]); + + expect(claim.take(saved)).toEqual([ADVISORY.message]); + expect(calls).toEqual([{ title: WARNED, description: other.message }]); + claim.release(); + }); + + it('the stripped-fields notice keeps its own toast while the warnings are claimed', async () => { + const update = vi.fn().mockResolvedValue({ + record: { id: 'r1' }, + droppedFields: [{ object: 'andon', fields: ['type', 'source_method'], reason: 'readonly_when' }], + warnings: [ADVISORY], + }); + const { ds, calls, settle } = wiredAdapter({ update }); + const strippedOnly = makeSink(); + await emitWriteWarning(EVENT, t, adapter as never, identityLabel, strippedOnly.sink); + const claim = claimAnswerWarnings(adapter, { operation: 'update', resource: 'andon', id: 'r1' }); + + const saved = await ds.update('andon', 'r1', { type: 'x', source_method: 'y' }); + expect(claim.take(saved)).toEqual([ADVISORY.message]); + await settle(); + claim.release(); + + expect(calls).toEqual([strippedOnly.calls[0]]); + }); + + it('CONTROL: a write the claim does not name keeps the channel toast', async () => { + const update = vi.fn().mockResolvedValue({ record: { id: 'r2' }, warnings: [ADVISORY] }); + const create = vi.fn().mockResolvedValue({ record: { id: 'o1' }, warnings: [ADVISORY] }); + const { ds, calls, settle } = wiredAdapter({ update, create }); + const claim = claimAnswerWarnings(adapter, { operation: 'update', resource: 'quote', id: 'r1' }); + + await ds.update('quote', 'r2', { discount: 40 }); + await ds.create('quote', { discount: 40 }); + await ds.update('order', 'r1', { discount: 40 }); + await settle(); + claim.release(); + + expect(calls).toEqual([ + { title: WARNED, description: ADVISORY.message }, + { title: WARNED, description: ADVISORY.message }, + { title: WARNED, description: ADVISORY.message }, + ]); + }); + + it('a claim closed without a take shows what it parked, and stops claiming', async () => { + const update = vi.fn().mockResolvedValue({ record: { id: 'r1' }, warnings: [ADVISORY] }); + const { ds, calls, settle } = wiredAdapter({ update }); + const claim = claimAnswerWarnings(adapter, { operation: 'update', resource: 'quote', id: 'r1' }); + + await ds.update('quote', 'r1', { discount: 40 }); + await settle(); + expect(calls).toEqual([]); + claim.release(); + expect(calls).toEqual([{ title: WARNED, description: ADVISORY.message }]); + + await ds.update('quote', 'r1', { discount: 40 }); + await settle(); + expect(calls).toHaveLength(2); + }); +}); diff --git a/ui/packages/app-shell/src/providers/writeWarningToast.ts b/ui/packages/app-shell/src/providers/writeWarningToast.ts index 07a3ef56d2a..af23c2681f5 100644 --- a/ui/packages/app-shell/src/providers/writeWarningToast.ts +++ b/ui/packages/app-shell/src/providers/writeWarningToast.ts @@ -237,6 +237,16 @@ function lineFor(reason: DroppedFieldsNotice['reason']): StrippedLine { * wrote it (it is the rule author's text); the only copy this module adds is * the title. An event without `warnings` produces exactly the toast it always * did. + * + * Except for a write a record save surface reports itself + * (objectstack-ai/objectstack#27999): when an open {@link claimAnswerWarnings} + * claim on `adapter` matches the event, its `warnings` are parked on that claim + * instead, and the surface's own record-named save toast carries them. The + * match is decided SYNCHRONOUSLY, before this function's first `await`: the + * adapter emits the event inside the write, before the write's promise settles, + * so the claim holds the warnings by the time the surface's success handler + * runs, whatever the label lookup below costs. The stripped-fields toast is + * never claimed. */ export async function emitWriteWarning( ev: WriteWarningEvent, @@ -245,19 +255,28 @@ export async function emitWriteWarning( fieldLabel: FieldLabelFn, sink: WriteWarningSink, ): Promise { + // Before the first `await`: see the claim paragraph above. + const messages = answerWarningLines(ev); + const claim = messages.length > 0 && adapter ? matchingClaim(adapter, ev) : undefined; + if (claim) claim.parked.push({ ev, show: () => emitAnswerWarnings(messages, t, sink) }); await emitStrippedFields(ev, t, adapter, fieldLabel, sink); - emitAnswerWarnings(ev, t, sink); + if (!claim) emitAnswerWarnings(messages, t, sink); } -/** - * The write answer's advisory `warnings`, one line per distinct message, under - * one title. Nothing when the event carries none. - */ -function emitAnswerWarnings(ev: WriteWarningEvent, t: TranslateFn, sink: WriteWarningSink): void { +/** The write answer's advisory `warnings`, one entry per distinct message. */ +function answerWarningLines(ev: WriteWarningEvent): string[] { const messages: string[] = []; for (const w of ev.warnings ?? []) { if (w.message && !messages.includes(w.message)) messages.push(w.message); } + return messages; +} + +/** + * The write answer's advisory `warnings`, one line per distinct message, under + * one title. Nothing when the event carries none. + */ +function emitAnswerWarnings(messages: string[], t: TranslateFn, sink: WriteWarningSink): void { if (messages.length === 0) return; sink.warning( t('detail.writeWarningsTitle', { @@ -267,6 +286,111 @@ function emitAnswerWarnings(ev: WriteWarningEvent, t: TranslateFn, sink: WriteWa ); } +/** + * The writes a record save surface reports itself: the record form an open + * overlay or page saves (objectstack-ai/objectstack#27999). An update names + * the record it writes; a create's record id is only known from its answer. + */ +export interface AnswerWarningScope { + operation: WriteWarningEvent['operation']; + resource: string; + id?: string | number; +} + +/** A record save surface's hold on the answer warnings of its own write. */ +export interface AnswerWarningClaim { + /** + * The distinct warning messages of the write that answered `saved`, removed + * from the claim. Every other event the claim parked is shown on the + * channel's own toast at once, so a write the surface did not make keeps + * the toast it always had. + */ + take(saved: unknown): string[]; + /** Close the claim; whatever it still parks is shown on the channel's toast. */ + release(): void; +} + +interface ParkedAnswerWarnings { + ev: WriteWarningEvent; + show: () => void; +} + +interface ClaimEntry { + scope: AnswerWarningScope; + parked: ParkedAnswerWarnings[]; +} + +/** Open claims per adapter instance: the one `AdapterProvider` subscribes to. */ +const CLAIMS = new WeakMap>(); + +const sameId = (a: unknown, b: unknown): boolean => + a === undefined || a === null ? b === undefined || b === null : String(a) === String(b); + +function matchingClaim(adapter: object, ev: WriteWarningEvent): ClaimEntry | undefined { + for (const entry of CLAIMS.get(adapter) ?? []) { + const { scope } = entry; + if (scope.operation !== ev.operation || scope.resource !== ev.resource) continue; + if (scope.operation === 'update' && !sameId(scope.id, ev.id)) continue; + return entry; + } + return undefined; +} + +/** + * Claim the answer warnings of the writes `scope` names on `adapter`, for as + * long as the surface that will report them is open. The surface reads its + * write's warnings with {@link AnswerWarningClaim.take} when the save succeeds + * and closes the claim with {@link AnswerWarningClaim.release} when it closes; + * no parked warning is dropped by either. + */ +export function claimAnswerWarnings(adapter: object, scope: AnswerWarningScope): AnswerWarningClaim { + const entry: ClaimEntry = { scope, parked: [] }; + let open = CLAIMS.get(adapter); + if (!open) CLAIMS.set(adapter, (open = new Set())); + open.add(entry); + const showAll = (list: ParkedAnswerWarnings[]) => { + for (const p of list) p.show(); + }; + return { + take(saved) { + const record = saved && typeof saved === 'object' ? (saved as { id?: unknown; _id?: unknown }) : undefined; + const savedId = record?.id ?? record?._id; + // An update's parked events already name its record; a create's are its + // own only when the answer's record id is the saved record's. + const own = entry.parked.filter((p) => scope.operation === 'update' || sameId(p.ev.id, savedId)); + const others = entry.parked.filter((p) => !own.includes(p)); + entry.parked = []; + showAll(others); + const messages: string[] = []; + for (const p of own) { + for (const m of answerWarningLines(p.ev)) if (!messages.includes(m)) messages.push(m); + } + return messages; + }, + release() { + open?.delete(entry); + const left = entry.parked; + entry.parked = []; + showAll(left); + }, + }; +} + +/** Where a record save toast goes. Structurally satisfied by sonner's `toast`. */ +export interface RecordSaveSink extends WriteWarningSink { + success(title: string): void; +} + +/** + * The one toast of a record save (objectstack-ai/objectstack#27999): the + * surface's record-named message, carrying its write's advisory warnings one + * per line on the warning variant when there are any. + */ +export function toastRecordSave(sink: RecordSaveSink, message: string, warnings: readonly string[] = []): void { + if (warnings.length > 0) sink.warning(message, { description: warnings.join('\n') }); + else sink.success(message); +} + /** The stripped-fields toast — the channel's original and unchanged message. */ async function emitStrippedFields( ev: WriteWarningEvent, diff --git a/ui/packages/app-shell/src/views/RecordFormPage.saveToastCarriesWarnings-27999.test.tsx b/ui/packages/app-shell/src/views/RecordFormPage.saveToastCarriesWarnings-27999.test.tsx new file mode 100644 index 00000000000..ac0d8fab187 --- /dev/null +++ b/ui/packages/app-shell/src/views/RecordFormPage.saveToastCarriesWarnings-27999.test.tsx @@ -0,0 +1,219 @@ +/** + * 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. + */ + +/** + * objectstack-ai/objectstack#27999 — a page-mode save whose answer carries + * advisory warnings (a `severity: 'warning'` validation rule) shows ONE toast: + * the record-named save message (objectstack-ai/objectstack#26461), carrying + * the warning lines. The write-warning channel's "Saved — with warnings" toast + * is not raised for that write; its stripped-fields notice still is, and a + * write the page did not make keeps the channel toast. + * + * The form is a stand-in whose button does what the real chain does, in its + * order: the adapter emits the write-warning event INSIDE the write (here the + * channel's `emitWriteWarning`, called with the adapter as `AdapterProvider` + * calls it, and not awaited), then `ObjectForm` hands the write's result to + * `schema.onSuccess`. The channel's sink and the page's toast are the same + * sonner `toast`, so every toast the user would see is counted. + * + * Direction, written before the run: with the channel ignoring the page's + * claim, every "one toast" cell RED (a second "Saved — with warnings" toast, and + * a success toast carrying no warning line); the CONTROL cells GREEN on both + * sides. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor, fireEvent } from '@testing-library/react'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { I18nProvider } from '@object-ui/i18n'; +import type { WriteWarningEvent } from '@object-ui/data-objectstack'; +import { emitWriteWarning } from '../providers/writeWarningToast'; +import { RecordFormPage } from './RecordFormPage'; + +const h = React.createElement; + +const { toastSuccess, toastWarning, saveState, ADAPTER } = vi.hoisted(() => ({ + toastSuccess: vi.fn(), + toastWarning: vi.fn(), + saveState: { event: undefined as unknown, saved: undefined as Record | undefined }, + ADAPTER: { getObjectSchema: async () => ({ fields: {} as Record }) }, +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: toastSuccess, + error: vi.fn(), + info: vi.fn(), + warning: toastWarning, + loading: vi.fn(), + dismiss: vi.fn(), + }), +})); + +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ + user: { id: 'u1', name: 'Ada', email: 'ada@example.com', role: 'admin' }, + getAuthConfig: async () => ({ features: {} }), + activeOrganization: null, + }), +})); + +/** The channel's translator: the English default, interpolated. */ +const t = (_key: string, opts?: Record) => + String(opts?.defaultValue ?? '').replace('{{fields}}', String(opts?.fields ?? '')); +const identityLabel = (_o: string, _f: string, fallback: string) => fallback; + +/** Every channel run the stand-in started, so a test can let them settle. */ +const pending: Promise[] = []; + +// The write, then `onSuccess` with its result — the channel run is NOT awaited +// in between, as the adapter's listener does not await it either. +vi.mock('@object-ui/plugin-form', async (importOriginal) => { + const { toast } = await import('sonner'); + return { + ...(await importOriginal>()), + ObjectForm: ({ schema }: { schema: { onSuccess?: (saved?: unknown) => void } }) => + h( + 'button', + { + type: 'button', + 'data-testid': 'fire-success', + onClick: () => { + if (saveState.event) { + pending.push(emitWriteWarning(saveState.event as WriteWarningEvent, t, ADAPTER, identityLabel, toast)); + } + schema.onSuccess?.(saveState.saved); + }, + }, + 'ok', + ), + }; +}); + +const TASK = { + name: 'showcase_task', + label: 'Task', + nameField: 'subject', + fields: { subject: { type: 'text', label: 'Subject' }, done: { type: 'boolean', label: 'Done' } }, +}; +const METADATA = { objects: [TASK], loading: false }; +vi.mock('../providers/MetadataProvider', () => ({ + useMetadata: () => METADATA, +})); +vi.mock('../providers/AdapterProvider', () => ({ + useAdapter: () => ADAPTER, +})); + +const ADVISORY = { + field: 'subject', + code: 'validation_rule', + message: 'Reviews are usually scheduled a week ahead', + rule: 'review_lead_time', + severity: 'warning' as const, +}; +const SECOND = { ...ADVISORY, field: 'done', rule: 'done_owner', message: 'Done tasks usually name an owner' }; +const NAMED = { id: 't1', subject: 'Quarterly review' }; + +beforeEach(() => { + toastSuccess.mockClear(); + toastWarning.mockClear(); + pending.length = 0; + saveState.event = undefined; + saveState.saved = undefined; +}); +afterEach(cleanup); + +/** Mount the page in `mode`, save through the form with `event` emitted by the write, and settle. */ +async function saveOn(mode: 'create' | 'edit', event: WriteWarningEvent, saved: Record) { + saveState.event = event; + saveState.saved = saved; + const path = + mode === 'create' ? '/apps/crm/showcase_task/new' : '/apps/crm/showcase_task/record/t1/edit'; + const route = + mode === 'create' + ? '/apps/:appName/:objectName/new' + : '/apps/:appName/:objectName/record/:recordId/edit'; + render( + h(I18nProvider, { + config: { defaultLanguage: 'en', detectBrowserLanguage: false }, + children: h( + MemoryRouter, + { initialEntries: [path] }, + h(Routes, null, h(Route, { path: route, element: h(RecordFormPage, { mode }) })), + ), + }), + ); + fireEvent.click(screen.getByTestId('fire-success')); + await waitFor(() => expect(toastSuccess.mock.calls.length + toastWarning.mock.calls.length).toBeGreaterThan(0)); + await Promise.all(pending); +} + +const update = (over: Partial = {}): WriteWarningEvent => ({ + operation: 'update', + resource: 'showcase_task', + id: 't1', + droppedFields: [], + warnings: [ADVISORY], + ...over, +}); + +describe('RecordFormPage — a save with warnings shows one record-named toast (objectstack-ai/objectstack#27999)', () => { + it('update: one toast, the record-named message carrying the warning line', async () => { + await saveOn('edit', update(), NAMED); + + expect(toastSuccess).not.toHaveBeenCalled(); + expect(toastWarning.mock.calls).toEqual([ + ['Task "Quarterly review" updated', { description: ADVISORY.message }], + ]); + }); + + it('create: one toast, each distinct warning on its own line', async () => { + await saveOn( + 'create', + { operation: 'create', resource: 'showcase_task', id: 't1', droppedFields: [], warnings: [ADVISORY, SECOND, ADVISORY] }, + NAMED, + ); + + expect(toastSuccess).not.toHaveBeenCalled(); + expect(toastWarning.mock.calls).toEqual([ + ['Task "Quarterly review" created', { description: `${ADVISORY.message}\n${SECOND.message}` }], + ]); + }); + + it('the stripped-fields notice keeps its own toast beside the save toast', async () => { + await saveOn( + 'edit', + update({ droppedFields: [{ object: 'showcase_task', fields: ['done'], reason: 'readonly' }] }), + NAMED, + ); + + expect(toastSuccess).not.toHaveBeenCalled(); + expect(toastWarning.mock.calls.map(([title]) => title)).toEqual([ + 'Task "Quarterly review" updated', + 'Saved — but some fields did not take effect', + ]); + expect(toastWarning.mock.calls[0][1]).toEqual({ description: ADVISORY.message }); + }); + + it('CONTROL: a write the page did not make keeps the channel toast, and the save toast stays a success', async () => { + await saveOn('edit', update({ resource: 'showcase_project', id: 'p1' }), NAMED); + + expect(toastSuccess.mock.calls).toEqual([['Task "Quarterly review" updated']]); + expect(toastWarning.mock.calls).toEqual([['Saved — with warnings', { description: ADVISORY.message }]]); + }); + + it('CONTROL: a save without warnings is the success toast it always was', async () => { + await saveOn('edit', update({ warnings: undefined, droppedFields: [] }), NAMED); + + expect(toastSuccess.mock.calls).toEqual([['Task "Quarterly review" updated']]); + expect(toastWarning).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/packages/app-shell/src/views/RecordFormPage.tsx b/ui/packages/app-shell/src/views/RecordFormPage.tsx index 4cf71998331..40793714af9 100644 --- a/ui/packages/app-shell/src/views/RecordFormPage.tsx +++ b/ui/packages/app-shell/src/views/RecordFormPage.tsx @@ -28,7 +28,7 @@ * @module views/RecordFormPage */ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useParams, useNavigate, useSearchParams, Link } from 'react-router-dom'; import { ObjectForm } from '@object-ui/plugin-form'; import { Button, Empty, EmptyTitle, EmptyDescription } from '@object-ui/components'; @@ -38,6 +38,7 @@ import { getRecordDisplayName } from '@object-ui/core'; import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n'; import { useMetadata } from '../providers/MetadataProvider.js'; import { useAdapter } from '../providers/AdapterProvider.js'; +import { claimAnswerWarnings, toastRecordSave, type AnswerWarningClaim } from '../providers/writeWarningToast.js'; import { ExpressionProvider, createExpressionEvaluator, @@ -174,7 +175,28 @@ export function RecordFormPage({ mode }: RecordFormPageProps) { // (`Record #`, or the empty fallback for an id-less row) is not a name, // so a record with nothing nameable keeps the object-label toast. The two // named keys are in all ten packs, so they pass bare like every key above. + // + // objectstack-ai/objectstack#27999 — and it is the save's ONE toast: while + // this page is open, the write-warning channel parks the answer warnings of + // the write the page makes (this record's update, or a create of this + // object) on the claim below instead of toasting them, and this toast + // carries them. + const answerWarningClaim = useRef(null); + useEffect(() => { + if (!dataSource || !objectName) return; + if (mode === 'edit' && !recordId) return; + const claim = claimAnswerWarnings(dataSource, mode === 'edit' + ? { operation: 'update', resource: objectName, id: recordId } + : { operation: 'create', resource: objectName }); + answerWarningClaim.current = claim; + return () => { + if (answerWarningClaim.current === claim) answerWarningClaim.current = null; + claim.release(); + }; + }, [dataSource, mode, objectName, recordId]); + const handleSuccess = useCallback((saved?: any) => { + const warnings = answerWarningClaim.current?.take(saved) ?? []; const resolved = saved && typeof saved === 'object' ? getRecordDisplayName(objectDef, saved, { fallback: '' }) : ''; @@ -183,13 +205,13 @@ export function RecordFormPage({ mode }: RecordFormPageProps) { ? resolved : undefined; if (mode === 'create') { - toast.success(title + toastRecordSave(toast, title ? t('form.createdNamed', { object: label, title }) - : t('form.createSuccess', { object: label })); + : t('form.createSuccess', { object: label }), warnings); } else { - toast.success(title + toastRecordSave(toast, title ? t('form.updatedNamed', { object: label, title }) - : t('form.updateSuccess', { object: label })); + : t('form.updateSuccess', { object: label }), warnings); } goBack(); }, [mode, t, label, objectDef, goBack]);