Skip to content

Commit d62fb1f

Browse files
os-zhuangclaude
andauthored
feat(app-shell): toast when a save silently dropped read-only fields (framework #3431/#3455) (#2814)
The framework surfaces LEGALLY-stripped write fields (readonly #2948 / readonlyWhen #3042 / #3043 create ingress) as a `droppedFields` payload on the create/update response. The console discarded it — a value typed into a locked field vanished on save with a success toast and no explanation. This surfaces it. - data-objectstack: ObjectStackAdapter emits a WriteWarningEvent after a create/update whose response carried droppedFields, via a new onWriteWarning() subscription (mirrors the onMutation bus). Field is read structurally, so an older client / never-dropping backend is a no-op. Exports WriteWarningEvent, WriteWarningListener, DroppedFieldsEvent. - app-shell: AdapterProvider subscribes and raises a toast.warning so the strip is visible instead of silent. The write still succeeded; behaviour unchanged. Verified: 6 unit cases (emit on create/update, no-emit when absent/empty/ malformed, unsubscribe, listener isolation) + onMutation regression green; data-objectstack & app-shell type-check clean; toast rendered in the real console (singular + plural grammar) through the live ConsoleToaster. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 4dfd14f commit d62fb1f

4 files changed

Lines changed: 257 additions & 2 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@object-ui/data-objectstack": minor
3+
"@object-ui/app-shell": patch
4+
---
5+
6+
feat(app-shell): toast when a save silently dropped read-only fields (framework #3431/#3455)
7+
8+
The framework now reports fields it LEGALLY stripped from a write (a non-system
9+
caller can't seed a `readonly` field, a `readonlyWhen` predicate locked it, …)
10+
via a `droppedFields` payload on the create/update response. Previously the
11+
console discarded it: a value the user typed into a locked field just vanished on
12+
save with a success toast and no explanation.
13+
14+
- **data-objectstack:** `ObjectStackAdapter` now emits a `WriteWarningEvent`
15+
after a create/update whose response carried `droppedFields`, exposed through a
16+
new `onWriteWarning(cb)` subscription (mirrors the existing `onMutation` bus).
17+
Reads the field structurally, so an older client or a backend that never drops
18+
is a no-op. New exported types: `WriteWarningEvent`, `WriteWarningListener`,
19+
`DroppedFieldsEvent`.
20+
- **app-shell:** `AdapterProvider` subscribes and raises a `toast.warning`
21+
("Some fields were not saved — the read-only field … could not be changed"),
22+
so the strip is visible instead of silent. The write itself still succeeded;
23+
status/behaviour are unchanged.

packages/app-shell/src/providers/AdapterProvider.tsx

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,30 @@
88
*/
99

1010
import { useState, useEffect, type ReactNode } from 'react';
11-
import { ObjectStackAdapter } from '@object-ui/data-objectstack';
11+
import { toast } from 'sonner';
12+
import { ObjectStackAdapter, type WriteWarningEvent } from '@object-ui/data-objectstack';
1213
import { createAuthenticatedFetch } from '@object-ui/auth';
1314
import { AdapterCtx } from '@object-ui/react';
1415
import { installSettleSignalGlobal, withSettleSignal } from '../observability/settleSignal';
1516

1617
export { useAdapter } from '@object-ui/react';
1718

19+
/**
20+
* Turn a write-warning (framework #3431/#3455) into a human toast. The write
21+
* SUCCEEDED — some caller-supplied fields were legally stripped (read-only /
22+
* locked by state), so we tell the user rather than let it pass silently.
23+
*/
24+
function toastWriteWarning(ev: WriteWarningEvent): void {
25+
const fields = Array.from(new Set(ev.droppedFields.flatMap((d) => d.fields)));
26+
if (fields.length === 0) return;
27+
const list = fields.join(', ');
28+
const description =
29+
fields.length === 1
30+
? `The read-only field “${list}” could not be changed and was not saved.`
31+
: `${fields.length} read-only fields could not be changed and were not saved: ${list}.`;
32+
toast.warning('Some fields were not saved', { description });
33+
}
34+
1835
interface AdapterProviderProps {
1936
children: ReactNode;
2037
/** Optional pre-created adapter (useful for testing). */
@@ -35,6 +52,7 @@ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterP
3552
}
3653

3754
let cancelled = false;
55+
let unsubscribeWriteWarning: (() => void) | undefined;
3856

3957
// Expose window.__objectui.{pendingRequests,idle,whenIdle} so an automated
4058
// (AI) browser driver has one "is the app settled?" predicate (ADR-0054 C5).
@@ -52,6 +70,10 @@ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterP
5270
cache: { maxSize: 50, ttl: 300_000 },
5371
});
5472

73+
// Surface silently-stripped write fields (#3431/#3455) as a toast so a
74+
// read-only value the user typed doesn't just vanish on save.
75+
unsubscribeWriteWarning = a.onWriteWarning(toastWriteWarning);
76+
5577
await a.connect();
5678

5779
if (!cancelled) {
@@ -65,7 +87,10 @@ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterP
6587
}
6688

6789
init();
68-
return () => { cancelled = true; };
90+
return () => {
91+
cancelled = true;
92+
unsubscribeWriteWarning?.();
93+
};
6994
}, [externalAdapter]);
7095

7196
return (

packages/data-objectstack/src/index.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,36 @@ export type ConnectionStateListener = (event: ConnectionStateEvent) => void;
379379
*/
380380
export type BatchProgressListener = (event: BatchProgressEvent) => void;
381381

382+
/**
383+
* One server-reported write-strip: caller-supplied fields the backend LEGALLY
384+
* removed from a write before persisting (a non-system caller cannot seed a
385+
* `readonly` field, a `readonlyWhen` predicate locked it, etc.). Mirrors the
386+
* framework `DroppedFieldsEvent` (spec `DroppedFieldsEventSchema`) structurally
387+
* so we don't pin a client type version — `reason` is kept as a widened string
388+
* for forward-compatibility with reasons added server-side.
389+
*/
390+
export interface DroppedFieldsEvent {
391+
object: string;
392+
fields: string[];
393+
reason: string;
394+
}
395+
396+
/**
397+
* Emitted after a create/update whose response carried `droppedFields`
398+
* (framework #3431/#3455). The write SUCCEEDED — this is a warning that some
399+
* supplied fields never landed, so the UI can tell the user rather than let it
400+
* pass silently. Subscribe via {@link ObjectStackAdapter.onWriteWarning}.
401+
*/
402+
export interface WriteWarningEvent {
403+
operation: 'create' | 'update';
404+
resource: string;
405+
id?: string | number;
406+
droppedFields: DroppedFieldsEvent[];
407+
}
408+
409+
/** Event listener type for write-warning (dropped-fields) events. */
410+
export type WriteWarningListener = (event: WriteWarningEvent) => void;
411+
382412
// Re-export FileUploadResult from types for consumers
383413
export type { FileUploadResult } from '@object-ui/types';
384414

@@ -471,6 +501,11 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
471501
// inline-edit "Save All" writes without a manual reload.
472502
private mutationListeners = new Set<(event: MutationEvent<T>) => void>();
473503

504+
// Subscribers registered via onWriteWarning(). Emitted after a create/update
505+
// whose response carried `droppedFields` (framework #3431/#3455) so the app
506+
// shell can surface a toast instead of the strip passing silently.
507+
private writeWarningListeners = new Set<WriteWarningListener>();
508+
474509
constructor(config: {
475510
baseUrl: string;
476511
token?: string;
@@ -902,10 +937,59 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
902937
};
903938
}
904939

940+
/**
941+
* Notify all write-warning subscribers. Isolated like {@link emitMutation}: a
942+
* throwing listener must not break the write or starve the others.
943+
*/
944+
private emitWriteWarning(event: WriteWarningEvent): void {
945+
for (const listener of this.writeWarningListeners) {
946+
try {
947+
listener(event);
948+
} catch (err) {
949+
console.warn('ObjectStackAdapter: write-warning listener error', err);
950+
}
951+
}
952+
}
953+
954+
/**
955+
* Read `droppedFields` off a create/update response (framework #3431/#3455)
956+
* and, when present, notify write-warning subscribers. Tolerant of a client
957+
* whose response type predates `droppedFields`: the field is read structurally
958+
* and validated, so an older client (or a backend that never drops) is a no-op.
959+
*/
960+
private notifyDroppedFields(
961+
operation: 'create' | 'update',
962+
resource: string,
963+
result: unknown,
964+
id?: string | number,
965+
): void {
966+
const dropped = (result as { droppedFields?: unknown } | null | undefined)?.droppedFields;
967+
if (!Array.isArray(dropped) || dropped.length === 0) return;
968+
const droppedFields = dropped.filter(
969+
(e): e is DroppedFieldsEvent =>
970+
!!e && typeof e === 'object' && Array.isArray((e as DroppedFieldsEvent).fields) && (e as DroppedFieldsEvent).fields.length > 0,
971+
);
972+
if (droppedFields.length === 0) return;
973+
this.emitWriteWarning({ operation, resource, ...(id !== undefined ? { id } : {}), droppedFields });
974+
}
975+
976+
/**
977+
* Subscribe to write-warning events (a create/update dropped caller-supplied
978+
* fields — #3431/#3455). Returns an unsubscribe function. The app shell uses
979+
* this to toast the user; the write itself already succeeded.
980+
*/
981+
onWriteWarning(callback: WriteWarningListener): () => void {
982+
this.writeWarningListeners.add(callback);
983+
return () => {
984+
this.writeWarningListeners.delete(callback);
985+
};
986+
}
987+
905988
async create(resource: string, data: Partial<T>): Promise<T> {
906989
await this.connect();
907990
const result = await this.client.data.create<T>(resource, data);
908991
this.emitMutation({ type: 'create', resource, record: { ...result.record } });
992+
this.notifyDroppedFields('create', resource, result, (result.record as { id?: string | number } | undefined)?.id);
909993
return result.record;
910994
}
911995

@@ -935,6 +1019,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
9351019
opts?.ifMatch ? { ifMatch: opts.ifMatch } : undefined,
9361020
);
9371021
this.emitMutation({ type: 'update', resource, id, record: { ...result.record } });
1022+
this.notifyDroppedFields('update', resource, result, id);
9381023
return result.record;
9391024
} catch (err) {
9401025
throw normaliseClientError(err);
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* ObjectStackAdapter must surface the framework's write-observability channel
11+
* (#3431/#3455): when a create/update response carries `droppedFields` — fields
12+
* the backend LEGALLY stripped (read-only / locked-by-state) — the adapter emits
13+
* a WriteWarningEvent so the app shell can toast the user instead of letting the
14+
* dropped value vanish silently. The write itself still succeeded.
15+
*/
16+
import { describe, it, expect, vi } from 'vitest';
17+
import { ObjectStackAdapter } from './index';
18+
import type { WriteWarningEvent } from './index';
19+
20+
function makeDS(stub: Record<string, any>) {
21+
const ds: any = new ObjectStackAdapter({
22+
baseUrl: 'http://test.local',
23+
fetch: vi.fn(async () =>
24+
new Response(JSON.stringify({ success: true, data: { capabilities: {}, routes: {} } }), {
25+
status: 200,
26+
headers: { 'Content-Type': 'application/json' },
27+
}),
28+
),
29+
});
30+
ds.connected = true;
31+
ds.connectionState = 'connected';
32+
ds.client = { data: stub };
33+
return ds;
34+
}
35+
36+
const ONE_DROP = [{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' }];
37+
38+
describe('ObjectStackAdapter.onWriteWarning', () => {
39+
it('emits a create write-warning carrying the dropped fields and the new id', async () => {
40+
const create = vi.fn().mockResolvedValue({ record: { id: 'r1', title: 'A' }, droppedFields: ONE_DROP });
41+
const ds = makeDS({ create });
42+
const events: WriteWarningEvent[] = [];
43+
ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
44+
45+
const rec = await ds.create('approval_case', { title: 'A', approval_status: 'approved' });
46+
47+
// The write still returns the plain record (unchanged behaviour).
48+
expect(rec).toEqual({ id: 'r1', title: 'A' });
49+
expect(events).toEqual([
50+
{ operation: 'create', resource: 'approval_case', id: 'r1', droppedFields: ONE_DROP },
51+
]);
52+
});
53+
54+
it('emits an update write-warning carrying the target id', async () => {
55+
const update = vi.fn().mockResolvedValue({ record: { id: 'r1' }, droppedFields: ONE_DROP });
56+
const ds = makeDS({ update });
57+
const events: WriteWarningEvent[] = [];
58+
ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
59+
60+
await ds.update('approval_case', 'r1', { approval_status: 'approved' });
61+
62+
expect(events).toEqual([
63+
{ operation: 'update', resource: 'approval_case', id: 'r1', droppedFields: ONE_DROP },
64+
]);
65+
});
66+
67+
it('does NOT emit when the response carries no droppedFields (the common case)', async () => {
68+
const create = vi.fn().mockResolvedValue({ record: { id: 'r1', title: 'A' } });
69+
const update = vi.fn().mockResolvedValue({ record: { id: 'r1' } });
70+
const ds = makeDS({ create, update });
71+
const events: WriteWarningEvent[] = [];
72+
ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
73+
74+
await ds.create('approval_case', { title: 'A' });
75+
await ds.update('approval_case', 'r1', { title: 'B' });
76+
77+
expect(events).toEqual([]);
78+
});
79+
80+
it('does NOT emit for an empty or malformed droppedFields payload', async () => {
81+
const create = vi
82+
.fn()
83+
.mockResolvedValueOnce({ record: { id: 'r1' }, droppedFields: [] })
84+
.mockResolvedValueOnce({ record: { id: 'r2' }, droppedFields: 'nope' })
85+
.mockResolvedValueOnce({ record: { id: 'r3' }, droppedFields: [{ object: 'x', fields: [] }] });
86+
const ds = makeDS({ create });
87+
const events: WriteWarningEvent[] = [];
88+
ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
89+
90+
await ds.create('x', {});
91+
await ds.create('x', {});
92+
await ds.create('x', {}); // event present but its fields[] is empty → filtered out
93+
94+
expect(events).toEqual([]);
95+
});
96+
97+
it('stops delivering after unsubscribe', async () => {
98+
const create = vi.fn().mockResolvedValue({ record: { id: 'r1' }, droppedFields: ONE_DROP });
99+
const ds = makeDS({ create });
100+
const events: WriteWarningEvent[] = [];
101+
const unsub = ds.onWriteWarning((e: WriteWarningEvent) => events.push(e));
102+
103+
await ds.create('approval_case', {});
104+
unsub();
105+
await ds.create('approval_case', {});
106+
107+
expect(events).toHaveLength(1);
108+
});
109+
110+
it('isolates a throwing listener so the write still resolves and other listeners fire', async () => {
111+
const update = vi.fn().mockResolvedValue({ record: { id: 'r1' }, droppedFields: ONE_DROP });
112+
const ds = makeDS({ update });
113+
const good = vi.fn();
114+
ds.onWriteWarning(() => { throw new Error('boom'); });
115+
ds.onWriteWarning(good);
116+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
117+
118+
await expect(ds.update('approval_case', 'r1', { approval_status: 'x' })).resolves.toBeTruthy();
119+
expect(good).toHaveBeenCalledTimes(1);
120+
warn.mockRestore();
121+
});
122+
});

0 commit comments

Comments
 (0)