diff --git a/packages/webui/src/public/workflows.js b/packages/webui/src/public/workflows.js index 724d0ec6b..04cf3df1d 100644 --- a/packages/webui/src/public/workflows.js +++ b/packages/webui/src/public/workflows.js @@ -523,10 +523,13 @@ function compose(org, name, q, info) { const attr = name === null ? 'data-wfg-list' : 'data-wfg-page'; const wrapIt = (title, sub, body) => `
${head(title, sub)}${info.unread}${body}
`; + // Review P3: a change held for this organization that the page cannot draw now is said to be kept. + const held = [...drafts].filter(([k, d]) => JSON.parse(k)[0] === org && (changed(d) || editDirty(d.edit))).map(([k]) => JSON.parse(k)[2]); + const keptNote = held.length ? `

Unsaved change to ${held.map(n => esc(n)).join(', ')} is kept on this page. It comes back when the workflows can be read.

` : ''; // E2E6 P2-8: a stopped bus is not read; the note above says why. - if (info.stopped) return wrapIt(name === null ? 'Workflows' : esc(name), `Workflows in ${esc(info.label)}.`, ''); + if (info.stopped) return wrapIt(name === null ? 'Workflows' : esc(name), `Workflows in ${esc(info.label)}.`, keptNote); const s = states.get(org); - if (!s?.data) return wrapIt(name === null ? 'Workflows' : esc(name), `Workflows in ${esc(info.label)}.`, `

${esc(s?.error ?? 'Workflows could not be read.')}

`); + if (!s?.data) return wrapIt(name === null ? 'Workflows' : esc(name), `Workflows in ${esc(info.label)}.`, `

${esc(s?.error ?? 'Workflows could not be read.')}

${keptNote}`); if (name === null) return wrapIt('Workflows', `The steps a task goes through in ${esc(info.label)}, in order, with the roles that do each.${s.data.writable ? ' Open one to change it.' : ''}`, listBody(org, s, info.label)); const g = graphBody(org, s, name, q); return wrapIt(g.title, g.sub, g.body); @@ -631,14 +634,16 @@ async function save({ org, name, scope, s, key }) { const d = drafts.get(key); if (!d || d.busy) return; - const refused = refusal(s, { isNew: d.isNew, scope, name, steps: d.flow.steps }); + // Review P2: the open step form's edits are not in the workflow yet; saving now would drop them unsaid. + const refused = (editDirty(d.edit) ? `Not saved: step ${d.edit.i + 1} has edits not applied. Apply or cancel the step first.` : '') + || refusal(s, { isNew: d.isNew, scope, name, steps: d.flow.steps }); if (refused) { d.status = refused; say(refused); return paint('wfg-save'); } d.busy = true; d.status = 'Saving…'; paint(); try { await saveFlow(org, s, { scope, name, revision: d.revision, workflow: shape(d.flow) }); drafts.delete(key); notes.set(key, `Saved ${name}.`); - try { await read(org, s); } catch { /* the next poll reads it */ } + try { await read(org, states.get(org) ?? s); } catch { /* the next poll reads it */ } say(`Saved ${name}.`); paint(selected() ? `wfg-step:${selected()}` : 'wfg-delete'); } catch (error) { @@ -653,8 +658,9 @@ if (!d || d.busy) return; d.busy = true; paint(); try { - // The revision of the page's last read: a change made since then is refused, not deleted over. - await removeFlow(org, s, { scope, name, revision: s.data.revision }); + // The revision of the workflow the page shows (settle moves it on only while this workflow is unchanged): + // a change made to it since is refused, not deleted over. + await removeFlow(org, s, { scope, name, revision: d.revision }); drafts.delete(key); notes.set(dkey(org, '', ''), `Deleted ${name}.`); say(`Deleted ${name}.`); @@ -662,7 +668,7 @@ location.hash = `${orgHash(org)}/workflows`; } catch (error) { d.busy = false; d.confirm = false; - d.status = `Not deleted: ${error.message}`; + d.status = error.code === 'revision-conflict' ? 'Not deleted: the organization file changed this workflow since the page read it. Discard to load the current version.' : `Not deleted: ${error.message}`; say(d.status); paint('wfg-delete'); } diff --git a/packages/webui/tests/workflows-browser.test.mjs b/packages/webui/tests/workflows-browser.test.mjs index 94c45c52d..8c211cfd9 100644 --- a/packages/webui/tests/workflows-browser.test.mjs +++ b/packages/webui/tests/workflows-browser.test.mjs @@ -73,7 +73,7 @@ function organizations(reads, st) { members: async () => LIST.map(member), resolve: async id => { const org = LIST.find(o => o.id === id); if (!org) return null; - const workflowsRead = async () => { reads.push(id); return { revision: st.revs[id], document: structuredClone(st.docs[id] ?? {}), templates: ['writer'] }; }; + const workflowsRead = async () => { reads.push(id); if (st.failRead) throw Object.assign(new Error('org-unavailable'), { code: 'org-unavailable' }); return { revision: st.revs[id], document: structuredClone(st.docs[id] ?? {}), templates: ['writer'] }; }; if (!org.busHealthy) return { busHealthy: false, reads: null, bus: {}, workflowsRead, chat: { projects: async () => [], threads: async () => [] } }; return { busHealthy: true, workflowsRead, packsWrite: st.writable ? async (verb, args) => st.write(id, verb, args) : null, decisionWrite: async () => ({}), send: async () => ({}), @@ -595,3 +595,63 @@ test('switching organization with a pending workflow change asks first, through assert.equal(await b.evaluate(`window.MosaicWorkflows.hasDraft()`), false); assert.equal(st.writes.length, 0); }); + +test('Save with a step form holding edits not applied refuses in words and sends nothing; after Apply it sends them', { timeout: 60000 }, async t => { + const { b, wait, st } = await setup(t, { path: '#/orgs/acme/workflows/change?project=site&step=2' }); + await wait(`document.querySelector('[data-wfg-up]')`, 'step 2'); + // One change pending, so Save shows; then a step form is opened and typed in, not applied. + await b.evaluate(press('[data-wfg-up]')); + await wait(`${titles}[0] === 'Ship' && document.querySelector('[data-wfg-save]')`, 'moved'); + await b.evaluate(press('[data-wfg-edit]')); + await wait(`document.querySelector('[data-wfg-form]')`, 'form'); + await b.evaluate(fill('[data-wfg-form] input[name="title"]', 'Ship the release')); + await b.evaluate(press('[data-wfg-save]')); + await wait(`/Apply or cancel the step first/.test(${text('[data-wfg-status]')})`, 'save refused'); + assert.equal(await b.evaluate(text('[data-wfg-status]')), 'Not saved: step 1 has edits not applied. Apply or cancel the step first.'); + assert.equal(st.writes.length, 0); + assert.equal(await b.evaluate(`document.querySelector('[data-wfg-form] input[name="title"]')?.value`), 'Ship the release', 'the typed title stays'); + await b.evaluate(press('[data-wfg-apply]')); + await wait(`${titles}[0] === 'Ship the release'`, 'applied'); + await b.evaluate(press('[data-wfg-save]')); + await wait(`/Saved change\\./.test(${text('[data-wfg-status]')})`, 'saved'); + assert.equal(st.writes.length, 1); + assert.deepEqual(st.writes[0].args.workflow.steps.map(s => s.title), ['Ship the release', 'Build']); +}); + +test('Delete on a view another writer has changed is refused, not deleted over', { timeout: 60000 }, async t => { + const { b, wait, st, reads } = await setup(t, { path: '#/orgs/acme/workflows/change?project=site&step=1' }); + await wait(`document.querySelector('[data-wfg-remove]')`, 'step 1'); + await b.evaluate(press('[data-wfg-remove]')); + await wait(`${boxes}.length === 1 && document.querySelector('[data-wfg-unsaved]')`, 'pending removal'); + st.change('acme', doc => { doc.projects.site.workflows.change.description = 'Changed elsewhere.'; }); + const n = reads.length; + await b.evaluate(`document.dispatchEvent(new CustomEvent('mosaic-workspace-render'));true`); + await wait(`!!document.querySelector('[data-wfg-stale]')`, 'stale note'); + assert.ok(reads.length > n); + await b.evaluate(press('[data-wfg-delete]')); + await wait(`document.querySelector('[data-wfg-delete-yes]')`, 'confirm'); + await b.evaluate(press('[data-wfg-delete-yes]')); + await wait(`/^Not deleted/.test(${text('[data-wfg-status]')})`, 'delete refused'); + assert.equal(await b.evaluate(text('[data-wfg-status]')), 'Not deleted: the organization file changed this workflow since the page read it. Discard to load the current version.'); + assert.equal(st.writes.length, 1); + assert.equal(st.writes[0].args.revision, REV_A); + assert.equal(st.writes[0].args.workflow, null); + assert.equal(st.docs.acme.projects.site.workflows.change.description, 'Changed elsewhere.', 'the newer workflow is still in the file'); + assert.ok(await b.evaluate(`${boxes}.length === 1 && !!document.querySelector('[data-wfg-unsaved]')`), 'the pending change stays'); +}); + +test('a failed read says a pending change is kept, and it comes back when the read works', { timeout: 60000 }, async t => { + const { b, wait, st } = await setup(t, { path: '#/orgs/acme/workflows/change?project=site&step=1' }); + await wait(`document.querySelector('[data-wfg-remove]')`, 'step 1'); + await b.evaluate(press('[data-wfg-remove]')); + await wait(`${boxes}.length === 1 && document.querySelector('[data-wfg-unsaved]')`, 'pending removal'); + st.failRead = true; + await b.evaluate(`document.dispatchEvent(new CustomEvent('mosaic-workspace-render'));true`); + await wait(`!!document.querySelector('[data-wfg-error]') && !!document.querySelector('[data-wfg-held]')`, 'error with kept note'); + assert.equal(await b.evaluate(text('[data-wfg-held]')), 'Unsaved change to change is kept on this page. It comes back when the workflows can be read.'); + assert.equal(await b.evaluate(`window.MosaicWorkflows.hasDraft()`), true); + st.failRead = false; + await b.evaluate(`document.dispatchEvent(new CustomEvent('mosaic-workspace-render'));true`); + await wait(`${boxes}.length === 1 && !!document.querySelector('[data-wfg-unsaved]') && !document.querySelector('[data-wfg-held]')`, 'back'); + assert.equal(st.writes.length, 0); +});