Skip to content

Commit d17f670

Browse files
fix(console): settings pages re-read their namespace after a save or action (objectui#13382) (#13415)
Closes #13382 Clause-②: no ## What changed `SettingsView` (`apps/console/src/pages/settings/SettingsView.tsx`) now reads its namespace again after a successful save or action, through the same `getSettingsNamespace` the load effect uses, and takes the whole payload, manifest included. After objectstack#25448 the server puts a live-status `info_banner` (id `live_status`) first in `manifest.specifiers` on every read, so Setup → Email's banner now follows a save without a reload. - **Save:** the PUT's values are still merged and the draft, field-error and crypto-refusal resets are unchanged. The success toast fires, then the re-read runs. - **Action:** the re-read runs only when the action answers `ok`. The draft is left as it is. - **Re-read fails:** it is caught and ignored, so the merged values and the success toast stay and the page does not turn into an error. - **No placeholder flash:** `loading` is not flipped. - **Stale guard:** the load effect has no cancellation of its own, so a read counter (`readSeqRef`) is added. The load effect bumps it, and each re-read applies only while it is still the latest. A re-read that lands after a namespace change, or after a newer re-read, is dropped. ## Tests - New `SettingsView.rereadAfterSave-13382.test.tsx`: a fake `fetch` whose first GET answers "No mail leaves this machine" and every later GET answers "Mail is delivered through SMTP (…)". Six cases: - control: the first load is unchanged and sends one GET; - after a save the new banner shows, with no remount (the same field-list node) and the saved value and toast present; - after a successful action the new banner shows; - an unsaved draft survives an action and its re-read; - a save whose re-read rejects still shows the saved value and the success toast, with no error toast and no load-failure card; - a re-read held in flight and released after a move to another namespace is dropped. - **Ablation:** with the `SettingsView.tsx` change reverted, 4 of the 6 cases fail and the control passes. The namespace-change case passes there because nothing re-reads. With only the load effect's counter bump removed, that case fails and the other 5 pass. - **Sibling fakes:** two existing fakes answered every GET with a fixed fixture while their PUT answered other values, a server that contradicts itself. The re-read exposed that. - `SettingsView.unsetAndReset-13145` and `SettingsView.languageKeepsDraft-13254`: each fake now stores what its PUT wrote, and later GETs return it. - The 13254 case "after a language change, Save sends the kept edit and the bar goes" still asserts that the language change sent no GET. After the save it now expects the one re-read, so it is a second pin for this fix: it fails with the fix reverted. - `pnpm exec vitest run apps/console/src/pages/settings/ --maxWorkers=2`: 29 files and 239 tests passed, at `5ee1459d6d`. - `turbo run type-check --filter=@object-ui/console...`: 71/71 successful. The console's `tsconfig.json` includes `src`, so the tests are type-checked too. - `eslint` on the four touched files: 0 errors and 3 `no-explicit-any` warnings, all on existing `catch (err: any)` lines. With `--no-inline-config` there is 1 error, `react-hooks/static-components` on the existing `getLazyIcon` line, whose inline disable that flag ignores. `origin/main`'s copy of the file gives the same error, so it is not new here. - `node scripts/check-changeset-presence.mjs` ✅, `check-changeset-no-major` ✅, `pnpm check:new-line-citations`: 0 new. ## Changeset `.changeset/13382-settings-reread-after-save.md`, a `@object-ui/console` patch. ## Acceptance notes - The load effect itself still has no stale guard: if namespace A's first GET resolves after a move to B, it could still land. This PR does not change that path (the card's control is "first load unchanged"), and it was not reproduced here. - I have not checked this in a running console against objectstack#25448. The pins drive the page over a stubbed `fetch`. - [x] Adds no product-specific widget, dialog, hook or string (see AGENTS.md › Product-neutral renderer) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WJozybDmRH8z2c5SD7vJkj Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4b1ddf3 commit d17f670

5 files changed

Lines changed: 358 additions & 8 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@object-ui/console': patch
3+
---
4+
5+
A Setup settings page reads its namespace again after a successful save or action, and takes the whole answer, manifest included (objectui#13382). The server computes parts of a namespace's manifest on every read: objectstack#25448 puts a live-status banner first on the page, so Setup → Email says whether mail leaves the machine. The page used to merge only the saved values and keep the manifest from its first load, so after an administrator configured SMTP the banner still said no mail leaves the machine until the page was reloaded.
6+
7+
The re-read does not show the loading placeholder. An action leaves the unsaved edits as they are. If the re-read fails, the page keeps the saved values and the success message, and does not turn into an error.

‎apps/console/src/pages/settings/SettingsView.tsx‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*
44
* Fetches manifest+values from `/api/settings/:namespace`, lays them
55
* out via <SettingsField>, tracks dirty state, and saves through PUT.
6+
* A successful save or action reads the namespace again, manifest included.
67
* Env-locked fields are read-only and show <EnvLockBadge>.
78
*/
89

@@ -275,6 +276,12 @@ export function SettingsView() {
275276
*/
276277
const [focusKey, setFocusKey] = useState<{ key: string } | null>(null);
277278
const fieldListRef = useRef<HTMLDivElement>(null);
279+
/**
280+
* Counts the reads this page starts (objectui#13382). A read applies its
281+
* payload only while it is still the latest one, so a re-read that resolves
282+
* after a namespace change, or after a newer re-read, is dropped.
283+
*/
284+
const readSeqRef = useRef(0);
278285

279286
useEffect(() => {
280287
if (!focusKey) return;
@@ -301,6 +308,8 @@ export function SettingsView() {
301308
// The draft resets when the namespace changes, on save and on discard.
302309
useEffect(() => {
303310
if (!namespace) return;
311+
// A re-read still in flight for the namespace being left is now stale.
312+
readSeqRef.current += 1;
304313
const loadNamespace = async () => {
305314
setLoading(true);
306315
setError(null);
@@ -382,6 +391,29 @@ export function SettingsView() {
382391
const title = labels.title(resolveLabel(manifest.label));
383392
const description = labels.description(manifest.description);
384393

394+
/**
395+
* Read the namespace again after a successful save or action and take the
396+
* whole payload, manifest included (objectui#13382). The server computes
397+
* parts of the manifest on every read: objectstack#25448 puts a live-status
398+
* `info_banner` (id `live_status`) first in `manifest.specifiers`, and a
399+
* save can change what it says. The PUT answers values only, so without
400+
* this the banner kept the first load's text until a reload.
401+
*
402+
* The draft, field errors and crypto refusal are left alone: the caller
403+
* decides those. `loading` is not flipped, so the form does not flash its
404+
* placeholder. A failed re-read keeps what is on screen: the save or action
405+
* itself succeeded, so the page must not turn into an error.
406+
*/
407+
const reread = async (ns: string) => {
408+
const seq = ++readSeqRef.current;
409+
try {
410+
const fresh = await getSettingsNamespace(ns);
411+
if (readSeqRef.current === seq) setPayload(fresh);
412+
} catch {
413+
// Keep the page as it is; the next load or save reads again.
414+
}
415+
};
416+
385417
const onSave = async () => {
386418
if (dirtyKeys.length === 0) return;
387419
// objectui#13239 — a visible required field left empty blocks the save
@@ -425,6 +457,7 @@ export function SettingsView() {
425457
setDraft({});
426458
setFieldErrors({});
427459
toast.success(t('console.settingsView.saved'));
460+
void reread(namespace);
428461
} catch (err: any) {
429462
const apiError = err?.payload?.error;
430463
if (apiError?.code === 'SETTINGS_LOCKED') {
@@ -501,8 +534,13 @@ export function SettingsView() {
501534
setSaving(true);
502535
try {
503536
const result = await runSettingsAction(namespace, actionId, draft);
504-
if (result.ok) toast.success(result.message ?? t('console.settingsView.actionSucceeded'));
505-
else toast.error(result.message ?? t('console.settingsView.actionFailed'));
537+
if (result.ok) {
538+
toast.success(result.message ?? t('console.settingsView.actionSucceeded'));
539+
// An action does not save the draft, so the draft stays as it is.
540+
void reread(namespace);
541+
} else {
542+
toast.error(result.message ?? t('console.settingsView.actionFailed'));
543+
}
506544
} catch (err: any) {
507545
toast.error(err?.message ?? t('console.settingsView.actionFailed'));
508546
} finally {

‎apps/console/src/pages/settings/__tests__/SettingsView.languageKeepsDraft-13254.test.tsx‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,24 @@ function ok(body: unknown): Response {
6565
return { ok: true, status: 200, statusText: 'OK', json: async () => body } as unknown as Response;
6666
}
6767

68-
/** GET answers the namespace named in the URL; PUT answers the values it was sent. */
68+
/**
69+
* GET answers the namespace named in the URL; PUT answers the values it was
70+
* sent and stores them, so a later GET answers the same: the page reads the
71+
* namespace again after a save (objectui#13382).
72+
*/
6973
function serve() {
74+
const stored = structuredClone(PAYLOADS) as Record<string, { values: Record<string, unknown> }>;
7075
fetchMock.mockImplementation(async (url: string, init?: RequestInit) => {
7176
const namespace = decodeURIComponent(String(url).split('/').pop() ?? '');
7277
if (init?.method === 'PUT') {
7378
const sent = JSON.parse(String(init.body)) as Record<string, unknown>;
7479
const values = Object.fromEntries(
7580
Object.entries(sent).map(([k, v]) => [k, { value: v, source: 'tenant', locked: false }]),
7681
);
82+
Object.assign(stored[namespace].values, structuredClone(values));
7783
return ok({ success: true, data: { values } });
7884
}
79-
return ok({ success: true, data: structuredClone(PAYLOADS[namespace]) });
85+
return ok({ success: true, data: structuredClone(stored[namespace]) });
8086
});
8187
}
8288

@@ -215,13 +221,16 @@ describe('the draft still resets where it should (objectui#13254 controls)', ()
215221
await editDefaultCountry();
216222
fireEvent.click(screen.getByRole('button', { name: 'switch to zh' }));
217223
expect(await screen.findByText('全部设置')).toBeTruthy();
224+
// The language change itself sent no GET.
225+
expect(getsOf('localization')).toBe(1);
218226

219227
fireEvent.click(screen.getByRole('button', { name: /保存更改/ }));
220228

221229
await waitFor(() => expect(screen.queryByTestId('settings-unsaved-count')).toBeNull());
222230
expect(puts()).toEqual([{ default_country: 'FR' }]);
231+
// The one GET after the save is the page's re-read (objectui#13382).
232+
await waitFor(() => expect(getsOf('localization')).toBe(2));
223233
expect(screen.getByDisplayValue('FR')).toBeTruthy();
224-
expect(getsOf('localization')).toBe(1);
225234
});
226235

227236
it('after a language change, Discard goes back to the stored value without a GET', async () => {

0 commit comments

Comments
 (0)