diff --git a/changelog.d/2237-assign-dialog-refusal.fixed.md b/changelog.d/2237-assign-dialog-refusal.fixed.md new file mode 100644 index 000000000..17b8844e3 --- /dev/null +++ b/changelog.d/2237-assign-dialog-refusal.fixed.md @@ -0,0 +1 @@ +A refused Hand a task to a Workflow now says so inside the dialog that stays open: the host's sentence is the only `role="alert"` for that refusal, the page draws no second copy behind the mask, and re-opening the dialog starts clean instead of inheriting the previous attempt's sentence (#2237). diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index ea468a8a6..15d2373a3 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -14,7 +14,7 @@ Settings → Workflows (#1364) is this plugin's list. The page lives in the core A host call on that page that throws, rejects, or never answers ends on the page's own `.wf-err` alert instead of staying on `Reading workflows…` or looking saved: a failed read offers `Try again`, and a failed save or model change clears the `Changes are saved automatically` hint. A successful call clears the alert again. -A confirmation dialog the host refuses — Rename, Delete, or Create — keeps the dialog open and prints the host's own sentence inside the dialog as the **only** `role="alert"` for that refusal: while one of those three dialogs holds the sentence the section draws no `.wf-err` copy of it, because a live region behind the mask is still read out and one refusal would be announced twice (#2193, #2202). Opening any of the three clears the previous refusal, a refusal with no dialog open still reports once on the page, and the success path is unchanged. +A confirmation dialog the host refuses — Rename, Delete, Create, or Hand a task to a Workflow — keeps the dialog open and prints the host's own sentence inside the dialog as the **only** `role="alert"` for that refusal: while one of those four dialogs holds the sentence the section draws no `.wf-err` copy of it, because a live region behind the mask is still read out and one refusal would be announced twice (#2193, #2202, #2237). Opening any of the four clears the previous refusal; closing the assign dialog drops its refusal rather than leaving it on the page. A refusal with no dialog open still reports once on the page, and the success path is unchanged. The **Add link** row applies the drawer's pair rule before it saves (#2173): a pair the graph already lists is a no-op, another kind over that same pair names the kind already there instead of submitting, and the **To** list marks the targets the chosen node already reaches. The host still refuses a duplicated pair — the form is what stops producing one. diff --git a/plugins/workflow/client.js b/plugins/workflow/client.js index f762fc73f..7091a2b51 100644 --- a/plugins/workflow/client.js +++ b/plugins/workflow/client.js @@ -1414,7 +1414,7 @@ window.__ModuleLoader__.load({ h('button', { type: 'button', className: 'wf-btn wf-danger', onClick: onConfirm }, tr('Delete'))))) } - function AssignModal({ workflows, tasks, tasksStatus, tasksError, roster, workflowId, onClose, onConfirm, onRetry }) { + function AssignModal({ workflows, tasks, tasksStatus, tasksError, error, roster, workflowId, onClose, onConfirm, onRetry }) { const [taskId, setTaskId] = useState(tasks[0]?.id ?? '') const [target, setTarget] = useState(workflowId || workflows[0]?.id || '') // #1457: the dialog opens before loadTasks() answers, so the first client render can have no @@ -1454,6 +1454,9 @@ window.__ModuleLoader__.load({ h('div', { className: 'wf-id' }, `${shortId(task.id)} · ${priorityOf(task)}`), h('div', { className: 'wf-tt' }, task.title), h('div', { className: 'wf-kcol' }, `● ${tr(COLUMN_KEY[columnOf(task)] ?? 'Not started')}`)))), + // #2237: a refused assign stays in this dialog, so the sentence the host sent is + // drawn here; the page-level copy is suppressed while this dialog is open. + error ? h('div', { className: 'wf-err', role: 'alert' }, error) : null, h('div', { className: 'wf-step-h' }, h('b', null, '2'), tr('Pick who runs it')), h('div', { className: 'wf-targets' }, workflows.map(item => h('button', { @@ -2020,7 +2023,7 @@ window.__ModuleLoader__.load({ // door must not open the dialog it no longer offers. root.openWorkflowAssign = () => { if (locked) return - setAssignOpen(true); setOpenId(id => id); void loadTasks() + setAssignOpen(true); setOpenId(id => id); setError(''); void loadTasks() } return () => { root.openWorkflowAssign = prev } }, [locked]) @@ -2140,6 +2143,9 @@ window.__ModuleLoader__.load({ const confirmAssign = async ({ taskId, target }) => { const task = tasks.find(item => item.id === taskId) if (!task) return + // #2237: a retry starts clean, so a refusal from the previous attempt does not sit + // beside the answer to this one. + setError('') if (String(target).startsWith('bot:')) { const botId = String(target).slice(4) const value = await call('workflow/assign', { botId, taskId: task.id, title: task.title }) @@ -2229,14 +2235,14 @@ window.__ModuleLoader__.load({ // lanes may carry the same name. Hidden text keeps the count without taking those // away — sighted readers still get the dot and the digit alone. (item.activeCount ?? 0) > 0 ? h('span', { className: 'wf-sr' }, tr('{count} running', { count: item.activeCount })) : null)), - locked ? null : h('button', { type: 'button', className: 'wf-row wf-assign-row', onClick: () => { setAssignOpen(true); void loadTasks() } }, + locked ? null : h('button', { type: 'button', className: 'wf-row wf-assign-row', onClick: () => { setAssignOpen(true); setError(''); void loadTasks() } }, h('span', { className: 'wf-ico' }, glyph()), h('span', { className: 'wf-txt' }, h('span', { className: 'wf-t1' }, tr('Hand a task to a Workflow')))), // #2202: `role="alert"` is a live region, so a copy behind the mask is still read out. - // While Rename, Delete or Create holds this same refusal in its own dialog, that dialog - // carries the only announcement; the section draws no second copy of the same sentence. - // A refusal with no dialog open is reported here as before. - error && !renaming && !deleting && !creating ? h('div', { className: 'wf-err', role: 'alert' }, error) : null, + // While Rename, Delete, Create or Hand a task to a Workflow holds this same refusal in + // its own dialog, that dialog carries the only announcement; the section draws no second + // copy of the same sentence. A refusal with no dialog open is reported here as before. + error && !renaming && !deleting && !creating && !assignOpen ? h('div', { className: 'wf-err', role: 'alert' }, error) : null, readError ? h('div', { className: 'wf-err', role: 'alert' }, readError) : null, open ? portal(h(Stage, { workflow: open, roster, locked, readOnlyReason, stageLeft, drawer, tab, focusedRunId, @@ -2261,7 +2267,7 @@ window.__ModuleLoader__.load({ if (value?.workflows) setPack(value) }) }, - onAssign: () => { setAssignOpen(true); void loadTasks() }, + onAssign: () => { setAssignOpen(true); setError(''); void loadTasks() }, onTip: setTip, onPause: (runId) => { void stageCall('workflow/pause', { workflowId: open.id, runId }, 'Could not pause this run').then(value => { @@ -2287,8 +2293,10 @@ window.__ModuleLoader__.load({ onRematch: rematchTask, })) : null, assignOpen ? portal(h(AssignModal, { - workflows, tasks, tasksStatus, tasksError, roster, workflowId: openId || workflows[0]?.id, - onClose: () => setAssignOpen(false), onConfirm: confirmAssign, onRetry: loadTasks, + workflows, tasks, tasksStatus, tasksError, error, roster, workflowId: openId || workflows[0]?.id, + // #2237: this dialog owns its refusal, so closing it drops the sentence rather than + // leaving it on the page for the next open to inherit. + onClose: () => { setAssignOpen(false); setError('') }, onConfirm: confirmAssign, onRetry: loadTasks, })) : null, creating ? portal(h(CreateModal, { roster, templates: pack?.templates, error, onClose: () => setCreating(false), onCreate: create, diff --git a/plugins/workflow/test/client.test.js b/plugins/workflow/test/client.test.js index 9c7f056d7..debf8a53c 100644 --- a/plugins/workflow/test/client.test.js +++ b/plugins/workflow/test/client.test.js @@ -4049,3 +4049,91 @@ test('#2202: the plugin Settings page draws no read-only sentence of its own', a assert.deepEqual(calls.filter(item => item.name !== 'workflow/state'), [], 'and still writes nothing') settings.unmount() }) + +// #2237: Hand a task to a Workflow is the fourth dialog on this surface. A refused assign put the +// host's sentence in the page-level `error`, whose only copy was drawn behind the mask; the dialog +// itself painted nothing, so it stayed open saying nothing. These count over the whole tree first +// (`alertsOf`) and only then look inside the open dialog — the #2202 shape, for the reason its +// comment gives: counting inside the dialog is exactly what let the duplicate through. +const ASSIGN_TASK = { id: 'task-live-1', title: 'Real board card one', status: 'open', priority: 'P1' } + +async function withTaskHost(fn) { + const prev = globalThis.__flybot__ + globalThis.__flybot__ = { host: async (channel) => (channel === 'tasks' ? [ASSIGN_TASK] : []) } + try { return await fn() } finally { globalThis.__flybot__ = prev } +} + +function assignCall(graph = { ...delivery(), id: 'lane-a', name: 'Lane A' }, outcome = () => ({ ok: false, error: refusal })) { + const calls = [] + const call = async (name, body) => { + calls.push({ name, body }) + if (name === 'workflow/state') return { workflows: [graph], templates: [] } + if (name === 'workflow/assign') return outcome() + return { ok: true, workflows: [graph] } + } + return { call, calls } +} + +const dialogOf = view => findAll(view.tree, node => node.props?.role === 'dialog')[0] +const assignOf = tree => findAll(tree, node => node.type === 'button' && textOf(node) === 'Assign')[0] + +async function openAssign(view) { + findAll(view.tree, node => node.props?.className === 'wf-row wf-assign-row')[0].props.onClick() + await settle() + return dialogOf(view) +} + +test('#2237: a refused assign is announced once, inside the open dialog', async () => { + const { call } = assignCall() + await withTaskHost(async () => { + const view = mount(client.Section, { roster: { bots: [] }, call, actions: {} }) + await settle() + let dialog = await openAssign(view) + assert.ok(dialog, 'the assign dialog opens') + assignOf(dialog).props.onClick() + await settle() + dialog = dialogOf(view) + assert.ok(dialog, 'a refused assign leaves the dialog open so the user can retry or cancel') + assert.deepEqual(alertsOf(view), [refusal], 'the whole tree carries the refusal exactly once') + assert.deepEqual(findAll(dialog, node => node.props?.role === 'alert').map(textOf), [refusal], 'and that one announcement is inside the open dialog, not only behind the mask') + assert.equal(findAll(dialog, node => String(node.props?.className ?? '').includes('wf-assign-fail')).length, 0, 'the read-failure slot is not lit for a refused assign') + view.unmount() + }) +}) + +test('#2237: a successful assign closes the dialog and leaves no alert', async () => { + const graph = { ...delivery(), id: 'lane-a', name: 'Lane A' } + const { call, calls } = assignCall(graph, () => ({ ok: true, workflows: [graph] })) + await withTaskHost(async () => { + const view = mount(client.Section, { roster: { bots: [] }, call, actions: {} }) + await settle() + const dialog = await openAssign(view) + assert.ok(dialog, 'the assign dialog opens') + assignOf(dialog).props.onClick() + await settle() + assert.equal(calls.filter(item => item.name === 'workflow/assign').length, 1, 'the click posts one assign') + assert.equal(dialogOf(view), undefined, 'a successful assign closes the dialog') + assert.deepEqual(alertsOf(view), [], 'and the tree carries no alert') + view.unmount() + }) +}) + +test('#2237: re-opening the assign dialog after a refusal starts clean', async () => { + const { call } = assignCall() + await withTaskHost(async () => { + const view = mount(client.Section, { roster: { bots: [] }, call, actions: {} }) + await settle() + let dialog = await openAssign(view) + assignOf(dialog).props.onClick() + await settle() + assert.deepEqual(alertsOf(view), [refusal], 'precondition: the refusal reached the screen') + dialog = dialogOf(view) + findAll(dialog, node => node.type === 'button' && textOf(node) === 'Cancel')[0].props.onClick() + await settle() + assert.equal(dialogOf(view), undefined, 'Cancel closes the dialog') + dialog = await openAssign(view) + assert.ok(dialog, 'the dialog re-opens') + assert.deepEqual(alertsOf(view), [], 'no sentence is left over from the previous attempt') + view.unmount() + }) +})