Skip to content

Commit cabe2ed

Browse files
fix(app-shell): a screen-flow wizard step toasts "Saved — next step" only when it saved a record (#26656)
<!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbJSZXnd6faVLEnwoaLHNWWh)_ Part of #26578. This lands the toast half; the step-button half stays open on the card, because it needs a signal the wire does not carry (see Acceptance notes). Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:屏幕流向导里,纯字段步骤之间不再弹「已保存 — 下一步」;只有 `object-form` 步骤(它真的保存了一条记录)还弹。流程结束时的提示不变。 - **为什么改**:hotclm 的发起合同向导在第 3 步才建记录,第 1、2 步却都提示「已保存」,用户误以为已经提交。 - **风险与回滚**:只动 `FlowRunner.tsx` 的一个分支,纯前端文案行为;回滚即 revert 本 PR。 - **先看**:`ui/packages/app-shell/src/views/FlowRunner.tsx`(`resumeWith` 的 `savedRecord` 参数与 paused 分支),`FlowRunner.betweenStepsToast-26578.test.tsx`,`FlowRunner.actionSuccessMessage-12234.test.tsx` 的改动。 - **两分钟验证**:`pnpm --dir ui exec vitest run packages/app-shell/src/views/__tests__/FlowRunner`,预期 15 个文件、94 个测试全绿。 - **需要你做的**:无,席位落地。 ## What changes - `FlowRunner`'s `resumeWith` takes a `savedRecord` flag. The flat-field Submit passes `false`; the `object-form` save callback passes `true`. - When the resume answers `paused`, the `flowRunner.nextStep` toast ("Saved — next step") fires only when `savedRecord` is true. A flat step that only collected variables now advances to the next screen with no toast. An `object-form` step keeps it: `ObjectForm` calls `onSuccess` only after its write lands, and passing `onSuccess` silences the form's own confirmation, so this toast is the user's only one. - Terminal toasts (the action's `successMessage`, the flow's, the catalog sentence), the refused and failed paths, and the step button are unchanged. - The FlowRunner header gains a section stating the rule and the measured limit on the button label. ## Why Reported on hotclm's five-step contract wizard: every step toasted 「已保存 — 下一步」, including steps that saved nothing, so a requester believed the contract was submitted on step 1. ## Tests - New `FlowRunner.betweenStepsToast-26578.test.tsx`: a flat step that pauses again toasts nothing; an `object-form` step that pauses again toasts exactly `Saved — next step` (full call list); a flat step that ends the run still gets the completion toast. - `FlowRunner.actionSuccessMessage-12234.test.tsx`: its multi-step pin asserted the old between-steps toast on flat screens; it now asserts the action message alone, and its docblock says why. ## Acceptance notes - **The step button still reads Submit on every flat step. This is a measured limit, not a choice.** Showing "Next" on a step that is not the last needs to know which step is the last. Measured on origin/main: the paused screen (`ScreenSpec` in `packages/spec/src/contracts/automation-service.ts`) carries `nodeId`, `title`, `description`, `fields`, `kind`, `objectName`, `mode`, `recordId`, `defaults` and `idVariable`, and no final-step or step-count member. The engine's screen executor (`packages/services/service-automation/src/builtin/screen-nodes.ts`) emits exactly those. A flow can branch after any screen, so only the engine could answer it, from the graph. The only client-side signal is after the fact: a resume that answers `paused` proves the previous step was not the last. - The same gap keeps a step counter out: the run never knows a total. - Closing the button half needs a server-computed wire member on `ScreenSpec` (for example, whether any screen node is reachable downstream of this one). That is a contract addition in `packages/spec`, so it is reported to the seat rather than built here. An author-declared button label would be a protocol addition, which triage ruled goes back to triage first. ## Verification Head `a746e566`. <details><summary>FlowRunner suites (ui root, under os-verify-lock)</summary> `pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/views/__tests__/FlowRunner` → `Test Files 15 passed (15)`, `Tests 94 passed (94)`, `VERDICT command-exit 0`. </details> <details><summary>Type-check</summary> `pnpm --filter @object-ui/app-shell type-check` (runs `tsc --noEmit` and `tsc -p tsconfig.test.json`, the latter covering `__tests__`) → `VERDICT command-exit 0`. </details> <details><summary>Ablation (one, on the changed behaviour)</summary> Through `scripts/ablation-replace.mjs`: anchor `if (savedRecord) toast.success(` replaced by `if (savedRecord || true) toast.success(` (anchor 1 → 0, blob `1ed20f953095` → `244560514f7f`). Running the new pin file and the 12234 file: `Tests 2 failed | 10 passed`. The flat-step pin fails on `toast.success` called once; the 12234 multi-step pin fails with `[['已保存 — 下一步'], ['工单已升级']]` against `[['工单已升级']]`. Restored: blob equals HEAD and `git diff HEAD` is empty. A first attempt used an undeclared identifier in the replacement, which threw instead of toasting; it was discarded as invalid. </details> <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 34 commands, each run with its exit code recorded. `--ran` at `a746e566`: `34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN`; every exit code 0. The ui `scripts/` suite in that list: `Test Files 178 passed | 2 skipped (180)`. `pnpm check:new-line-citations` (ui): `0 new citation(s)`. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jQzKBjxZXgyVnom2V3)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 55b9273 commit cabe2ed

4 files changed

Lines changed: 190 additions & 12 deletions

File tree

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): a screen-flow wizard step toasts "Saved — next step" only when it saved a record
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
In a screen-flow wizard, every step that led to another screen showed the toast "Saved — next step", even a step that only collected input. In a wizard that creates its record on step 3, steps 1 and 2 told the user something was saved when nothing was.
11+
12+
What changes:
13+
14+
- A step with plain input fields that leads to another screen now shows no toast. The next screen appearing is the answer.
15+
- An `object-form` step still shows "Saved — next step", because it saved its record before the run moved on.
16+
- The toast at the end of the run is unchanged.
17+
18+
The step button still reads "Submit" on every step. The paused screen the server sends does not say whether it is the last one, so the dialog cannot tell a middle step from the final one yet.
19+
20+
What to do: nothing. Nothing a flow declares changes.

‎ui/packages/app-shell/src/views/FlowRunner.tsx‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@
116116
* dispatches a declared action runs it through `useActionTextLocalizer`
117117
* first, and the handler receives that copy, so there is no second
118118
* localization step here. Nothing else moves: the pause stays silent, an
119-
* intermediate step still toasts `flowRunner.nextStep`, and a `refused` or
120-
* failed end still shows no success toast.
119+
* intermediate step toasts only a record it saved (objectstack#26578, below),
120+
* and a `refused` or failed end still shows no success toast.
121121
*
122122
* The order of the first two is not this file's: it is `flowSuccessCopy` in
123123
* `utils/flowResponse`, which a run that goes straight through follows too
@@ -219,6 +219,25 @@
219219
* The values then go out through `screenResumeValues`, which reduces each file
220220
* field to its bare id: the engine reads `'file'` as a value domain and refuses
221221
* anything else.
222+
*
223+
* ## A step between screens toasts only a record it saved (objectstack#26578)
224+
*
225+
* Every step of a wizard used to answer its pause with `Saved — next step`,
226+
* including a flat step that only collected variables: in a five-step wizard
227+
* whose record is created on step 3, steps 1 and 2 told the user something was
228+
* saved when nothing was. So the between-steps toast now follows what this
229+
* dialog itself persisted: an `object-form` step has written its record (the
230+
* form calls `onSuccess` only after the write lands, and passing `onSuccess`
231+
* is what silences the form's own confirmation), so it keeps the toast; a flat
232+
* step wrote nothing here, and the next screen appearing is its whole answer.
233+
* The terminal toasts are untouched.
234+
*
235+
* The flat footer's button still reads Submit on every step, and that is a
236+
* measured limit, not a choice. "Next" on a step that is not the last needs to
237+
* know which step IS the last, and nothing on the wire says so: the paused
238+
* screen (`ScreenSpec` in `@objectstack/spec/contracts`) carries no final-step
239+
* or step-count member, and a flow can branch after any screen, so only the
240+
* engine could tell from the graph. The same gap keeps a step counter out.
222241
*/
223242
import { Suspense, useEffect, useState } from 'react';
224243
import {
@@ -479,8 +498,10 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
479498

480499
// Resume the paused run with `inputs` (applied as bare flow variables) and
481500
// advance: render the next screen (multi-step wizard) or finish + refresh.
482-
// Shared by the flat-field submit and the object-form save callback.
483-
const resumeWith = async (inputs: Record<string, unknown>): Promise<void> => {
501+
// Shared by the flat-field submit and the object-form save callback;
502+
// `savedRecord` says whether this step persisted a record before resuming,
503+
// which only an `object-form` step does (objectstack#26578 — see the header).
504+
const resumeWith = async (inputs: Record<string, unknown>, savedRecord: boolean): Promise<void> => {
484505
const res = await authFetch(
485506
`${baseUrl}/api/v1/automation/${encodeURIComponent(flowName)}/runs/${encodeURIComponent(runId)}/resume`,
486507
{ method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ inputs }) },
@@ -527,7 +548,9 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
527548
setRunId(outcome.runId || runId);
528549
setFlowLabel(outcome.flowLabel);
529550
setValues(initialScreenValues(outcome.screen));
530-
toast.success(t('flowRunner.nextStep', { defaultValue: 'Saved — next step' }));
551+
// Only a step that saved a record says so; a flat step's answer is the
552+
// next screen itself (objectstack#26578 — see the header).
553+
if (savedRecord) toast.success(t('flowRunner.nextStep', { defaultValue: 'Saved — next step' }));
531554
} else {
532555
// Terminal success — the invoking action's own `successMessage` first
533556
// (objectui#12234 — see the header), then the flow's declared completion
@@ -596,7 +619,7 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
596619
setSubmitting(true);
597620
try {
598621
// File fields go out as their bare `sys_file` id (objectstack#25801).
599-
await resumeWith(screenResumeValues(screen, values));
622+
await resumeWith(screenResumeValues(screen, values), false);
600623
} catch (err) {
601624
// The request never produced a response (network / abort), so the
602625
// suspension was not consumed — this one IS retryable.
@@ -617,7 +640,7 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
617640
const inputs = screen.idVariable && id != null ? { [screen.idVariable]: id } : {};
618641
setSubmitting(true);
619642
try {
620-
await resumeWith(inputs);
643+
await resumeWith(inputs, true);
621644
} catch (err) {
622645
const message = (err as Error).message;
623646
toast.error(message);

‎ui/packages/app-shell/src/views/__tests__/FlowRunner.actionSuccessMessage-12234.test.tsx‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
* `ScreenFlowState.actionSuccessMessage`, it is the terminal toast — and it
2323
* wins over the flow's own `successMessage`, because it is what the same
2424
* click shows when the run does not pause.
25-
* 2. **Only the TERMINAL toast moves.** A multi-step run still toasts the
26-
* next-step notice between screens; a run that ends `refused` or fails after
27-
* the pause still shows no success toast at all.
25+
* 2. **Only the TERMINAL toast moves.** A multi-step run of flat screens shows
26+
* nothing between them (objectstack#26578: a flat step saved no record, so
27+
* it has no "Saved" to say) and closes with the action message alone; a run
28+
* that ends `refused` or fails after the pause still shows no success toast
29+
* at all.
2830
* 3. **The controls keep today's toast.** No action copy (absent, blank, or an
2931
* `I18nLabel` map no localizer resolved) ⇒ the flow's own message, else the
3032
* catalog sentence — asserted in zh with the reported wording, so the
@@ -106,7 +108,7 @@ describe("objectui#12234 — the invoking action's successMessage closes the run
106108
expect(toastSuccess.mock.calls).toEqual([[ACTION_MESSAGE]]);
107109
});
108110

109-
it('a multi-step run keeps the next-step notice between screens and closes with the action message', async () => {
111+
it('a multi-step run shows nothing between flat screens and closes with the action message', async () => {
110112
const { onComplete } = mount(stateWith(ACTION_MESSAGE), [
111113
jsonResponse({
112114
success: true,
@@ -119,7 +121,7 @@ describe("objectui#12234 — the invoking action's successMessage closes the run
119121
await submit();
120122

121123
await waitFor(() => expect(onComplete).toHaveBeenCalledTimes(1));
122-
expect(toastSuccess.mock.calls).toEqual([['已保存 — 下一步'], [ACTION_MESSAGE]]);
124+
expect(toastSuccess.mock.calls).toEqual([[ACTION_MESSAGE]]);
123125
});
124126

125127
it('a run that ends refused after the pause still shows no success toast', async () => {
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#26578 — a step between screens toasts only a record it saved.
5+
*
6+
* A wizard step used to answer every pause with `Saved — next step`, so a flat
7+
* step that only collected variables told the user something was saved. The
8+
* toast now follows what the dialog itself persisted:
9+
*
10+
* 1. A flat step that pauses again draws the next screen and toasts nothing.
11+
* 2. An `object-form` step that pauses again has written its record, so it
12+
* keeps the toast — the form's own confirmation is silenced by `onSuccess`,
13+
* which makes this the only one the user gets.
14+
* 3. A flat step that ends the run still gets the completion toast: only the
15+
* between-steps toast moved.
16+
*
17+
* The real `ScreenView` draws every flat screen; an `object-form` step is a
18+
* stub whose button stands in for the form's Save (as in
19+
* `FlowRunner.refusedMessageOnly-13342.test.tsx`).
20+
*/
21+
import { describe, it, expect, vi, beforeEach } from 'vitest';
22+
import { render, screen, waitFor } from '@testing-library/react';
23+
import userEvent from '@testing-library/user-event';
24+
25+
const toastError = vi.fn();
26+
const toastSuccess = vi.fn();
27+
vi.mock('sonner', () => ({ toast: { error: toastError, success: toastSuccess } }));
28+
29+
vi.mock('../ScreenView.js', async (importOriginal) => {
30+
const actual = await importOriginal<typeof import('../ScreenView.js')>();
31+
type Props = Parameters<typeof actual.ScreenView>[0];
32+
return {
33+
...actual,
34+
ScreenView: (props: Props) =>
35+
actual.isObjectFormScreen(props.screen) ? (
36+
<button type="button" onClick={() => props.objectForm?.onSuccess?.({ id: 'ctr_1' })}>
37+
Stub save
38+
</button>
39+
) : (
40+
<actual.ScreenView {...props} />
41+
),
42+
};
43+
});
44+
45+
const { FlowRunner } = await import('../FlowRunner');
46+
type ScreenFlowState = import('../FlowRunner').ScreenFlowState;
47+
type ScreenSpec = ScreenFlowState['screen'];
48+
49+
const INFO_SCREEN: ScreenSpec = {
50+
nodeId: 'screen_info',
51+
title: 'Contract details',
52+
fields: [{ name: 'contract_title', label: 'Title', type: 'text', required: true }],
53+
};
54+
55+
const CONTRACT_FORM_SCREEN: ScreenSpec = {
56+
nodeId: 'screen_contract',
57+
title: 'New contract',
58+
kind: 'object-form',
59+
objectName: 'contract',
60+
idVariable: 'contractId',
61+
};
62+
63+
const NEXT_SCREEN: ScreenSpec = {
64+
nodeId: 'screen_submit',
65+
title: 'Send to legal',
66+
fields: [{ name: 'notes', label: 'Notes', type: 'text' }],
67+
};
68+
69+
const pausedBody = { success: true, data: { success: true, status: 'paused', runId: 'run_1', screen: NEXT_SCREEN } };
70+
const completedBody = { success: true, data: { success: true } };
71+
72+
function jsonResponse(body: unknown): Response {
73+
return new Response(JSON.stringify(body), { status: 200, headers: { 'Content-Type': 'application/json' } });
74+
}
75+
76+
function mount(s: ScreenSpec, body: unknown) {
77+
const onComplete = vi.fn();
78+
const authFetch = vi.fn(async () => jsonResponse(body));
79+
render(
80+
<FlowRunner
81+
state={{ flowName: 'start_contract', runId: 'run_1', screen: s }}
82+
authFetch={authFetch}
83+
baseUrl=""
84+
onClose={vi.fn()}
85+
onComplete={onComplete}
86+
/>,
87+
);
88+
return { onComplete, authFetch };
89+
}
90+
91+
async function typeAndSubmit() {
92+
const user = userEvent.setup();
93+
await user.type(screen.getByRole('textbox'), 'Supply agreement');
94+
await user.click(screen.getByRole('button', { name: 'Submit' }));
95+
}
96+
97+
beforeEach(() => {
98+
toastError.mockClear();
99+
toastSuccess.mockClear();
100+
});
101+
102+
describe('objectstack#26578 — a step between screens toasts only a record it saved', () => {
103+
it('a flat step that pauses again draws the next screen and toasts nothing', async () => {
104+
const { authFetch, onComplete } = mount(INFO_SCREEN, pausedBody);
105+
await typeAndSubmit();
106+
107+
expect(await screen.findByRole('heading', { name: 'Send to legal' })).toBeInTheDocument();
108+
expect(authFetch).toHaveBeenCalledTimes(1);
109+
expect(toastSuccess).not.toHaveBeenCalled();
110+
expect(toastError).not.toHaveBeenCalled();
111+
expect(onComplete).not.toHaveBeenCalled();
112+
});
113+
114+
it('an `object-form` step that pauses again keeps the toast for the record it saved', async () => {
115+
const { authFetch } = mount(CONTRACT_FORM_SCREEN, pausedBody);
116+
await userEvent.setup().click(screen.getByRole('button', { name: 'Stub save' }));
117+
118+
expect(await screen.findByRole('heading', { name: 'Send to legal' })).toBeInTheDocument();
119+
expect(authFetch).toHaveBeenCalledWith(
120+
'/api/v1/automation/start_contract/runs/run_1/resume',
121+
expect.objectContaining({ body: JSON.stringify({ inputs: { contractId: 'ctr_1' } }) }),
122+
);
123+
expect(toastSuccess.mock.calls).toEqual([['Saved — next step']]);
124+
});
125+
126+
it('a flat step that ends the run still gets the completion toast', async () => {
127+
const { onComplete } = mount(INFO_SCREEN, completedBody);
128+
await typeAndSubmit();
129+
130+
await waitFor(() => expect(onComplete).toHaveBeenCalledTimes(1));
131+
expect(toastSuccess.mock.calls).toEqual([['Flow "start_contract" completed']]);
132+
});
133+
});

0 commit comments

Comments
 (0)