diff --git a/apps/client-e2e/specs/template-designer-w3.spec.ts b/apps/client-e2e/specs/template-designer-w3.spec.ts index 315f16e..e1c6594 100644 --- a/apps/client-e2e/specs/template-designer-w3.spec.ts +++ b/apps/client-e2e/specs/template-designer-w3.spec.ts @@ -12,8 +12,21 @@ interface UpdateTemplateDraftInput { readonly workflowDefinitionJson: string; } +interface MockMember { + readonly email: string; + readonly memberId: string; + readonly name: string; +} + interface MockTemplateGraphQlOptions { + // Opt-in so that adding directory entries for one spec cannot change the + // option ordering the keyboard-driven specs depend on. + readonly extraMembers?: readonly MockMember[]; readonly initialWorkflowDefinitionJson?: string; + // Lets a spec hold a `MemberOptions` response open until it deliberately + // releases it, so response ordering between two in-flight searches can be + // controlled deterministically instead of racing real network timing. + readonly memberSearchGate?: (searchText: string) => Promise; readonly onDraftUpdate?: (input: UpdateTemplateDraftInput) => void; } @@ -203,6 +216,172 @@ test.describe('M1 W3 template designer', () => { }); }); + test('does not offer a phantom row in the manager-fallback picker', async ({ + page, + }): Promise => { + await page.setViewportSize({ height: 1200, width: 1440 }); + await mockTemplateGraphQl(page); + + await page.goto(`/templates/${TEMPLATE_ID}/designer`); + await page.getByRole('combobox', { name: '未設定' }).click(); + await page.locator('[role="option"]').filter({ hasText: '所有人' }).click(); + await page.getByRole('button', { name: '簽核節點' }).click(); + + await page.getByRole('combobox', { name: '指定會員' }).click(); + await page + .locator('[role="option"]') + .filter({ hasText: '發起人主管' }) + .click(); + + await page.getByRole('combobox', { name: '停止流程並提示' }).click(); + await page + .locator('[role="option"]') + .filter({ hasText: '固定改派' }) + .click(); + + const fallbackMemberSearch = page.getByPlaceholder('搜尋姓名或信箱'); + + // Focus rather than fill: opening the picker before typing anything is + // exactly the moment `fallback.memberId` is still ''. + await fallbackMemberSearch.focus(); + + await expect( + page.locator('[role="option"]').filter({ hasText: '陳財務主管' }), + ).toBeVisible(); + // Nothing has been picked yet, so the picker must not offer a fake + // "未知會員" row built from the empty memberId as if it were a real + // person that could be selected. + await expect( + page.locator('[role="option"]').filter({ hasText: '未知會員' }), + ).toHaveCount(0); + }); + + test('narrows the approver picker to the latest search result', async ({ + page, + }): Promise => { + await page.setViewportSize({ height: 1200, width: 1440 }); + await mockTemplateGraphQl(page, { + extraMembers: [ + { + email: 'wang.engineer@example.internal', + memberId: 'member-201', + name: '王工程師', + }, + ], + }); + + await page.goto(`/templates/${TEMPLATE_ID}/designer`); + await page.getByRole('combobox', { name: '未設定' }).click(); + await page.locator('[role="option"]').filter({ hasText: '所有人' }).click(); + await page.getByRole('button', { name: '簽核節點' }).click(); + + const approverSearch = page.getByPlaceholder('搜尋姓名或信箱'); + + // Focus rather than click: the approver picker is multi-select and already + // carries the default approver's tag, which intercepts pointer events over + // the input. + await approverSearch.focus(); + await expect( + page.locator('[role="option"]').filter({ hasText: '王工程師' }), + ).toBeVisible(); + + await approverSearch.fill('chen'); + + await expect( + page.locator('[role="option"]').filter({ hasText: '陳財務主管' }), + ).toBeVisible(); + // The previous response must not linger beside the new one. + await expect( + page.locator('[role="option"]').filter({ hasText: '王工程師' }), + ).toHaveCount(0); + // The approver already on the node stays offered even though the search + // result does not contain it, otherwise its chip would lose the label. + await expect( + page.locator('[role="option"]').filter({ hasText: '林總經理' }), + ).toBeVisible(); + }); + + test('discards a stale search response that resolves after a newer one', async ({ + page, + }): Promise => { + const pendingSearches = new Map void>(); + const requestedSearchTexts: string[] = []; + + await page.setViewportSize({ height: 1200, width: 1440 }); + await mockTemplateGraphQl(page, { + extraMembers: [ + // Matches the broad 'ch' search but not the narrower 'chen' search, + // so it is the tell for whether a stale 'ch' response overwrote the + // already-landed 'chen' results. + { + email: 'chris.ops@example.internal', + memberId: 'member-301', + name: '資訊部克里斯', + }, + ], + memberSearchGate: (searchText): Promise => + new Promise((resolve) => { + requestedSearchTexts.push(searchText); + pendingSearches.set(searchText, resolve); + }), + }); + + await page.goto(`/templates/${TEMPLATE_ID}/designer`); + await page.getByRole('combobox', { name: '未設定' }).click(); + await page.locator('[role="option"]').filter({ hasText: '所有人' }).click(); + await page.getByRole('button', { name: '簽核節點' }).click(); + + const approverSearch = page.getByPlaceholder('搜尋姓名或信箱'); + + await approverSearch.focus(); + await expect.poll((): boolean => requestedSearchTexts.includes('')).toBe( + true, + ); + await resolveMemberSearch(page, pendingSearches, ''); + await expect( + page.locator('[role="option"]').filter({ hasText: '陳財務主管' }), + ).toBeVisible(); + + await approverSearch.fill('ch'); + await expect + .poll((): boolean => requestedSearchTexts.includes('ch')) + .toBe(true); + + await approverSearch.fill('chen'); + await expect + .poll((): boolean => requestedSearchTexts.includes('chen')) + .toBe(true); + + // Both 'ch' and 'chen' requests are now in flight. Resolve the + // later-issued, narrower 'chen' request first — a broad prefix is + // systematically slower than a narrow one in production, which is + // exactly the scenario this branch exists to guard against. + await resolveMemberSearch(page, pendingSearches, 'chen'); + await expect( + page.locator('[role="option"]').filter({ hasText: '陳財務主管' }), + ).toBeVisible(); + await expect( + page.locator('[role="option"]').filter({ hasText: '資訊部克里斯' }), + ).toHaveCount(0); + + // The stale 'ch' request resolves after. Its response must not + // overwrite the results already shown for the newer 'chen' search. + // `resolveMemberSearch` waits for the response to actually land before + // returning, and the extra settle time below gives React a chance to + // commit whatever state update that response triggers (a bug would + // apply, not deny, that update — this is not open-ended network + // polling, it bounds a same-tick render commit after a response we + // already know has landed). + await resolveMemberSearch(page, pendingSearches, 'ch'); + await page.waitForTimeout(200); + await expect( + page.locator('[role="option"]').filter({ hasText: '資訊部克里斯' }), + ).toHaveCount(0); + await expect( + page.locator('[role="option"]').filter({ hasText: '陳財務主管' }), + ).toBeVisible(); + }); + test('configures the return comment requirement and a business-day SLA', async ({ page, }): Promise => { @@ -615,6 +794,9 @@ async function mockTemplateGraphQl( if (query.includes('query MemberOptions')) { const searchText = readOptionalString(payload.variables?.searchText); + + await options.memberSearchGate?.(searchText); + await fulfillGraphQl(route, { searchMembers: [ { @@ -622,6 +804,7 @@ async function mockTemplateGraphQl( memberId: 'member-101', name: '陳財務主管', }, + ...(options.extraMembers ?? []), ].filter((member) => memberMatchesSearchText(member, searchText)), }); return; @@ -1031,6 +1214,40 @@ function isWorkflowEdgeRecord( ); } +// Resolves a gated `MemberOptions` request and waits for its response to +// actually reach the page, so the caller can rely on the request/response +// cycle having genuinely completed rather than racing an assertion against +// whenever React happens to re-render. +async function resolveMemberSearch( + page: Page, + pendingSearches: Map void>, + searchText: string, +): Promise { + const resolve = pendingSearches.get(searchText); + + if (!resolve) { + throw new Error(`No pending MemberOptions request for "${searchText}"`); + } + + const responsePromise = page.waitForResponse((response): boolean => { + if (!response.url().includes('/graphql')) { + return false; + } + + const body = response.request().postDataJSON() as + | { query?: string; variables?: { searchText?: string } } + | null; + + return ( + Boolean(body?.query?.includes('query MemberOptions')) && + readOptionalString(body?.variables?.searchText) === searchText + ); + }); + + resolve(); + await responsePromise; +} + function readGraphQlPayload(route: Route): GraphQlPayload { const payload = route.request().postDataJSON() as unknown; diff --git a/libs/bpm-core-react/src/views/templates/designer/TemplateDesignerView.tsx b/libs/bpm-core-react/src/views/templates/designer/TemplateDesignerView.tsx index b35ead0..bf06cf3 100644 --- a/libs/bpm-core-react/src/views/templates/designer/TemplateDesignerView.tsx +++ b/libs/bpm-core-react/src/views/templates/designer/TemplateDesignerView.tsx @@ -820,6 +820,19 @@ export function TemplateDesignerView({ const [memberOptions, setMemberOptions] = useState< readonly MemberSelectOption[] >([]); + // `memberOptions` accumulates every member the designer has ever seen, + // because it doubles as the id → display-name lookup for nodes elsewhere on + // the canvas. The pickers must not offer that accumulation as search + // results, so the latest response is tracked on its own. + const [memberSearchResults, setMemberSearchResults] = useState< + readonly MemberSelectOption[] + >([]); + // Guards `memberSearchResults` against out-of-order responses: typing + // narrows a search text while a broader, slower request for an earlier + // text is still in flight, and that stale response must not overwrite a + // response for a more recent search. `memberOptions` accumulation is + // order-insensitive (it only ever adds), so it does not need this guard. + const memberSearchSequenceRef = useRef(0); const [orgUnits, setOrgUnits] = useState([]); const [positions, setPositions] = useState([]); const [memberships, setMemberships] = useState( @@ -1184,10 +1197,20 @@ export function TemplateDesignerView({ setMemberLoading(true); setError(null); + const searchSequence = ++memberSearchSequenceRef.current; + try { const options = await searchMemberOptions(searchText); + const nextOptions = readMemberSelectOptions(options); + + // Only the freshest request may write the pickers' visible results; a + // slower, superseded request's response is dropped here. + if (searchSequence === memberSearchSequenceRef.current) { + setMemberSearchResults(nextOptions); + } + setMemberOptions((currentOptions) => - mergeMemberOptions(currentOptions, readMemberSelectOptions(options)), + mergeMemberOptions(currentOptions, nextOptions), ); } catch (requestError: unknown) { setError(readErrorMessage(requestError)); @@ -2252,8 +2275,12 @@ export function TemplateDesignerView({ resolver.type === 'ORG_MANAGER' || resolver.type === 'ORG_UNIT_MANAGER' ? (resolver.fallback ?? { type: 'NONE' as const }) : { type: 'NONE' as const }; + // An empty `memberId` means no one has been picked yet (the just-switched + // default). `readMemberSelectOption` never returns null, so without this + // guard it would resolve to the "未知會員" sentinel and offer that + // sentinel as a selectable, phantom row. const fallbackMember = - fallback.type === 'DIRECT' + fallback.type === 'DIRECT' && fallback.memberId ? readMemberSelectOption(memberOptions, fallback.memberId) : null; const resubmitStrategy = @@ -2315,10 +2342,16 @@ export function TemplateDesignerView({ onSearch={handleSearchMembers} onVisibilityChange={(open): void => { if (open) { + // Clear synchronously so the window between opening and the + // fetch landing never shows a different node's stale + // results. + setMemberSearchResults([]); void handleSearchMembers(''); } }} - options={[...memberOptions]} + options={[ + ...mergeMemberOptions(selectedMembers, memberSearchResults), + ]} placeholder="搜尋姓名或信箱" searchDebounceTime={300} value={selectedMembers} @@ -2470,10 +2503,25 @@ export function TemplateDesignerView({ onSearch={handleSearchMembers} onVisibilityChange={(open): void => { if (open) { + // Clear synchronously so the window between opening + // and the fetch landing never shows a different + // node's stale results. + setMemberSearchResults([]); void handleSearchMembers(''); } }} - options={[...memberOptions]} + options={[ + // The search results come first so that a genuine hit + // for the fallback member's id wins the dedupe inside + // `mergeMemberOptions` — the fallback member is only + // ever pre-fetched via search, never by id (see + // `readWorkflowDirectMemberIds`), so its own entry can + // still be a "未知會員" sentinel. + ...mergeMemberOptions( + memberSearchResults, + fallbackMember ? [fallbackMember] : [], + ), + ]} placeholder="搜尋姓名或信箱" searchDebounceTime={300} value={fallbackMember} @@ -2837,10 +2885,15 @@ export function TemplateDesignerView({ onSearch={handleSearchMembers} onVisibilityChange={(open): void => { if (open) { + // Clear synchronously so the window between opening and the + // fetch landing never shows a different node's stale results. + setMemberSearchResults([]); void handleSearchMembers(''); } }} - options={[...memberOptions]} + options={[ + ...mergeMemberOptions(selectedMembers, memberSearchResults), + ]} overflowStrategy="wrap" placeholder="搜尋姓名或信箱" searchDebounceTime={300}