Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/2237-assign-dialog-refusal.fixed.md
Original file line number Diff line number Diff line change
@@ -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).
2 changes: 1 addition & 1 deletion docs/subsystems/workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
28 changes: 18 additions & 10 deletions plugins/workflow/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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', {
Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -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 })
Expand Down Expand Up @@ -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,
Expand All @@ -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 => {
Expand All @@ -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,
Expand Down
88 changes: 88 additions & 0 deletions plugins/workflow/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3009,7 +3009,7 @@
// The hint's text is read when the drag is refused, so the host translation is put in place
// after the drawer mounted: a stub standing in for the whole host at mount time is read by the
// page's own reads (avatars, tool links) and hides the workflow list.
globalThis.__flybot__ = { ...(previous ?? {}), t: (key) => (key === 'Tasks' ? '任务' : key) }

Check warning on line 3012 in plugins/workflow/test/client.test.js

View workflow job for this annotation

GitHub Actions / plugins (workflow)

unicorn(no-useless-fallback-in-spread)

Empty fallbacks in spreads are unnecessary
try {
nodeAt('r1').props.onPointerDown({ clientX: 40, clientY: 160, target: { dataset: {} } })
canvas().props.onPointerMove({ clientX: 96, clientY: 208 })
Expand Down Expand Up @@ -4049,3 +4049,91 @@
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()
})
})
Loading