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/2176-workflows-readonly-hides-edit.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A window that holds the team read-only no longer offers **Edit** on Settings → Workflows: the page draws the host's own read-only sentence in its place, drops rename and delete, and never asks the host to save a graph, so a layout is not built only for the save to be refused ([#2176](https://github.com/ThinkFlowLab/FlyBot/issues/2176)).
18 changes: 11 additions & 7 deletions core/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -16851,7 +16851,7 @@ function GraphCanvas({ graph, editing, roster, onCommit, width: knownWidth = 0,
)
);
}
function Row2({ item, source, onOpen, onRename, onDelete }) {
function Row2({ item, source, onOpen, onRename, onDelete, editable = true }) {
return (0, import_react37.createElement)(
"button",
{
Expand All @@ -16867,7 +16867,7 @@ function Row2({ item, source, onOpen, onRename, onDelete }) {
(0, import_react37.createElement)("span", null, item.name),
(0, import_react37.createElement)("span", { className: "bt-set-hint" }, item.preview || chainOf(item) || item.description || "")
),
source === "own" ? (0, import_react37.createElement)(
source === "own" && editable ? (0, import_react37.createElement)(
"span",
{ className: "bt-set-actions" },
(0, import_react37.createElement)("button", {
Expand Down Expand Up @@ -16903,7 +16903,9 @@ function WorkflowsView({ phase = "loading", pack = null, error = "", opened = nu
const templates = pack?.templates ?? [];
const templateIds = new Set(templates.map((item) => item.id));
const yours = (pack?.workflows ?? []).filter((item) => !templateIds.has(item.id));
const showUse = opened?.source === "factory" || opened?.fromFactory !== void 0;
const readOnly = typeof roster?.readOnly === "string" ? roster.readOnly.trim() : "";
const locked = readOnly !== "";
const showUse = !locked && (opened?.source === "factory" || opened?.fromFactory !== void 0);
return (0, import_react37.createElement)(
"div",
{ className: "bt-wf-page" },
Expand Down Expand Up @@ -16942,7 +16944,7 @@ function WorkflowsView({ phase = "loading", pack = null, error = "", opened = nu
(0, import_react37.createElement)(
"div",
{ className: "bt-set-card" },
yours.length === 0 ? (0, import_react37.createElement)("div", { className: "bt-set-row" }, (0, import_react37.createElement)("span", { className: "bt-set-hint" }, t("No workflows of your own yet."))) : yours.map((item) => (0, import_react37.createElement)(Row2, { key: item.id, item, source: "own", onOpen: () => onOpen?.(item, "own"), onRename, onDelete }))
yours.length === 0 ? (0, import_react37.createElement)("div", { className: "bt-set-row" }, (0, import_react37.createElement)("span", { className: "bt-set-hint" }, t("No workflows of your own yet."))) : yours.map((item) => (0, import_react37.createElement)(Row2, { key: item.id, item, source: "own", editable: !locked, onOpen: () => onOpen?.(item, "own"), onRename, onDelete }))
)
)
) : null,
Expand All @@ -16954,7 +16956,7 @@ function WorkflowsView({ phase = "loading", pack = null, error = "", opened = nu
{ className: "bt-wf-actions" },
(0, import_react37.createElement)("button", { type: "button", onClick: () => onBack?.() }, t("Back")),
showUse ? (0, import_react37.createElement)("button", { type: "button", onClick: () => onUse?.(opened.source === "factory" ? opened.id : opened.fromFactory) }, t("Use this")) : null,
opened?.source === "own" ? (0, import_react37.createElement)("button", { type: "button", onClick: () => onEditing?.(!editing) }, editing ? t("Finish editing") : t("Edit")) : null
locked ? (0, import_react37.createElement)("span", { className: "bt-wf-status", role: "status" }, hostText(readOnly)) : opened?.source === "own" ? (0, import_react37.createElement)("button", { type: "button", onClick: () => onEditing?.(!editing) }, editing ? t("Finish editing") : t("Edit")) : null
),
(0, import_react37.createElement)("h3", { className: "bt-wf-title" }, graph.name),
graph.description ? (0, import_react37.createElement)("p", { className: "bt-wf-hint" }, graph.description) : null,
Expand All @@ -16964,7 +16966,7 @@ function WorkflowsView({ phase = "loading", pack = null, error = "", opened = nu
onCommit,
width: canvasWidth,
cardHeights,
editing: opened?.source === "own" && editing === true
editing: opened?.source === "own" && editing === true && !locked
})
) : null
);
Expand Down Expand Up @@ -17016,6 +17018,7 @@ function DeleteModal({ workflow, onClose, onConfirm }) {
);
}
function WorkflowsPage({ call: call4, roster }) {
const readOnly = typeof roster?.readOnly === "string" && roster.readOnly.trim() ? roster.readOnly : "";
const [phase, setPhase] = (0, import_react37.useState)("loading");
const [pack, setPack] = (0, import_react37.useState)(null);
const [error, setError] = (0, import_react37.useState)("");
Expand Down Expand Up @@ -17084,7 +17087,7 @@ function WorkflowsPage({ call: call4, roster }) {
setError("");
if (made) {
setOpened({ source: "own", id: made.id });
setEditing(true);
setEditing(!readOnly);
}
}).catch((failure) => {
setError(workflowCallError(failure, t("Could not copy this workflow")));
Expand Down Expand Up @@ -17130,6 +17133,7 @@ function WorkflowsPage({ call: call4, roster }) {
};
const commit = (graph) => {
if (!opened || opened.source !== "own") return;
if (readOnly) return;
Promise.resolve(invoke("workflow/graph", { workflowId: opened.id, nodes: graph.nodes, edges: graph.edges })).then((value) => {
const settled = settleWorkflowRead(value);
if (settled.phase === "error") {
Expand Down
29 changes: 21 additions & 8 deletions core/src/client/workflows.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// sidebar run uses. A read always settles: a host that never answers becomes
// an error, an empty store says so, and the file path is not the page.
import { createElement as h, useEffect, useRef, useState } from 'react'
import { t } from './i18n.js'
import { hostText, t } from './i18n.js'

export const READ_MS = 8000

Expand Down Expand Up @@ -532,15 +532,15 @@ function GraphCanvas({ graph, editing, roster, onCommit, width: knownWidth = 0,
})))))
}

function Row({ item, source, onOpen, onRename, onDelete }) {
function Row({ item, source, onOpen, onRename, onDelete, editable = true }) {
return h('button', {
type: 'button', className: 'bt-set-row', 'data-workflow': item.id, 'data-source': source,
onClick: () => onOpen?.(item),
},
h('span', { className: 'bt-set-copy' },
h('span', null, item.name),
h('span', { className: 'bt-set-hint' }, item.preview || chainOf(item) || item.description || '')),
source === 'own' ? h('span', { className: 'bt-set-actions' },
source === 'own' && editable ? h('span', { className: 'bt-set-actions' },
h('button', {
type: 'button', className: 'bt-set-action', 'aria-label': t('Rename'), title: t('Rename'),
onClick: (event) => { event.stopPropagation(); onRename?.(item) },
Expand All @@ -562,7 +562,14 @@ export function WorkflowsView({ phase = 'loading', pack = null, error = '', open
const templates = pack?.templates ?? []
const templateIds = new Set(templates.map((item) => item.id))
const yours = (pack?.workflows ?? []).filter((item) => !templateIds.has(item.id))
const showUse = opened?.source === 'factory' || opened?.fromFactory !== undefined
// #2176: a window that holds the team read-only carries the host's own sentence. The page
// shows that sentence where Edit was and never opens the editor, so `workflow/graph` is not
// offered in the first place instead of being refused after the layout was built. `Use this`
// writes `workflow/create`, which the channel's READS list does not hold either, so a
// read-only window does not offer that one either.
const readOnly = typeof roster?.readOnly === 'string' ? roster.readOnly.trim() : ''
const locked = readOnly !== ''
const showUse = !locked && (opened?.source === 'factory' || opened?.fromFactory !== undefined)
return h('div', { className: 'bt-wf-page' },
h('p', { className: 'bt-wf-hint' }, t('A workflow is a group of your Bots. Drag the flowchart to change who hands work to whom.')),
error ? h('div', { className: 'bt-error', role: 'alert' }, alertCopy(error),
Expand All @@ -587,19 +594,21 @@ export function WorkflowsView({ phase = 'loading', pack = null, error = '', open
h('div', { className: 'bt-set-card' },
yours.length === 0
? h('div', { className: 'bt-set-row' }, h('span', { className: 'bt-set-hint' }, t('No workflows of your own yet.')))
: yours.map((item) => h(Row, { key: item.id, item, source: 'own', onOpen: () => onOpen?.(item, 'own'), onRename, onDelete }))))) : null,
: yours.map((item) => h(Row, { key: item.id, item, source: 'own', editable: !locked, onOpen: () => onOpen?.(item, 'own'), onRename, onDelete }))))) : null,
graph ? h('div', { className: 'bt-wf-editor' },
h('div', { className: 'bt-wf-actions' },
h('button', { type: 'button', onClick: () => onBack?.() }, t('Back')),
showUse ? h('button', { type: 'button', onClick: () => onUse?.(opened.source === 'factory' ? opened.id : opened.fromFactory) }, t('Use this')) : null,
opened?.source === 'own' ? h('button', { type: 'button', onClick: () => onEditing?.(!editing) }, editing ? t('Finish editing') : t('Edit')) : null),
locked
? h('span', { className: 'bt-wf-status', role: 'status' }, hostText(readOnly))
: opened?.source === 'own' ? h('button', { type: 'button', onClick: () => onEditing?.(!editing) }, editing ? t('Finish editing') : t('Edit')) : null),
h('h3', { className: 'bt-wf-title' }, graph.name),
graph.description ? h('p', { className: 'bt-wf-hint' }, graph.description) : null,
(graph.nodes ?? []).length === 0
? h('p', { className: 'bt-wf-status' }, t('This workflow has no flowchart yet.'))
: h(GraphCanvas, {
graph, roster, onCommit, width: canvasWidth, cardHeights,
editing: opened?.source === 'own' && editing === true,
editing: opened?.source === 'own' && editing === true && !locked,
})) : null)
}

Expand Down Expand Up @@ -627,6 +636,7 @@ function DeleteModal({ workflow, onClose, onConfirm }) {
}

export function WorkflowsPage({ call, roster }) {
const readOnly = typeof roster?.readOnly === 'string' && roster.readOnly.trim() ? roster.readOnly : ''
const [phase, setPhase] = useState('loading')
const [pack, setPack] = useState(null)
const [error, setError] = useState('')
Expand Down Expand Up @@ -691,7 +701,7 @@ export function WorkflowsPage({ call, roster }) {
setPack(settled.pack)
setPhase(settled.phase)
setError('')
if (made) { setOpened({ source: 'own', id: made.id }); setEditing(true) }
if (made) { setOpened({ source: 'own', id: made.id }); setEditing(!readOnly) }
})
.catch((failure) => { setError(workflowCallError(failure, t('Could not copy this workflow'))) })
}
Expand Down Expand Up @@ -734,6 +744,9 @@ export function WorkflowsPage({ call, roster }) {

const commit = (graph) => {
if (!opened || opened.source !== 'own') return
// #2176: the channel refuses `workflow/graph` for a read-only window; the page does not
// build the request at all.
if (readOnly) return
Promise.resolve(invoke('workflow/graph', { workflowId: opened.id, nodes: graph.nodes, edges: graph.edges }))
.then((value) => {
const settled = settleWorkflowRead(value)
Expand Down
87 changes: 87 additions & 0 deletions core/test/workflows-page.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1381,3 +1381,90 @@ test('a forward edge into the next row turns above the tallest CJK named card (#
const below = points.filter((point) => point[1] > cards.get('d').top)
assert.deepEqual(below, [], `a>d never draws into the next row (${route.d})`)
})

// #2176: a window that holds the team read-only still offered Edit on Settings → Workflows.
// The host's own sentence already arrives on the page as `roster.readOnly`; it must replace the
// control rather than show up after the fact, because `workflow/graph` is not in the channel's
// READS list (`core/src/host/channel.js:227`), so the host only refuses once the user has built
// the layout — and then the state is discarded with one technical line.
const READ_ONLY = 'another dsh holds the team, so this window is read-only'

test('#2176: a read-only window renders the host sentence instead of Edit and offers no rename or delete', () => {
const view = renderView({ phase: 'ready', pack, opened: { source: 'own', id: 'wf-mine' }, error: '', roster: { bots: [], readOnly: READ_ONLY } })
const editingControl = all(view).filter((entry) => entry.type === 'button' && (text(entry) === 'Edit' || text(entry) === 'Finish editing'))
assert.deepEqual(editingControl.map(text), [], 'a read-only window offers no editing control')
const sentence = all(view).find((entry) => entry.props?.className === 'bt-wf-status' && text(entry) === READ_ONLY)
assert.ok(sentence, 'the host sentence is rendered on the page')
assert.equal(text(sentence), READ_ONLY, 'the page repeats the host wording, it does not invent one')
assert.equal(all(view).some((entry) => entry.props?.['data-port']), false, 'no port handles to drag')
// The read side is untouched: the graph, its cards and the run chips still render.
assert.ok(all(view).some((entry) => entry.props?.['data-node'] === 'coder'), 'the opened graph still renders')

// Rename and Delete live on the list rows, so they are checked on the list the page shows
// before a graph is opened — that is where the user would reach them.
const list = renderView({ phase: 'ready', pack, opened: null, error: '', roster: { bots: [], readOnly: READ_ONLY } })
assert.equal(all(list).some((entry) => entry.type === 'button' && (entry.props?.['aria-label'] === 'Rename' || entry.props?.['aria-label'] === 'Delete')), false,
'rename and delete are not offered in a read-only window')
assert.ok(all(list).some((entry) => entry.props?.['data-workflow'] === 'wf-mine'), 'the list still renders')
})

test('#2176: a read-only window never asks the host to save a graph the user drags', async () => {
const calls = []
const page = mountPage({ call: async (method) => { calls.push(method); return pack }, roster: { bots: [], readOnly: READ_ONLY } })
await page.settle()
await openWorkflow(page, 'wf-mine')
// A user who is offered an editing control takes it; read-only must not offer one. On a page
// that does, this drives the very flow that ends in the host's refusal.
const edit = all(page.tree).find((entry) => entry.type === 'button' && (text(entry) === 'Edit' || text(entry) === 'Finish editing'))
if (edit) {
edit.props.onClick({ stopPropagation() {}, preventDefault() {}, target: {} })
await page.settle()
}
const card = all(page.tree).find((entry) => entry.props?.['data-node'] === 'coder')
assert.ok(card, 'the card the user would drag is on the canvas')
card.props.onPointerDown({ clientX: 10, clientY: 20, target: { dataset: {} } })
const canvas = all(page.tree).find((entry) => entry.props?.className === 'bt-wf-canvas')
canvas.props.onPointerUp({ clientX: 30, clientY: 50, target: { dataset: {} } })
await page.settle()
assert.deepEqual(calls.filter((method) => method === 'workflow/graph'), [], 'a read-only window never writes the graph')
assert.ok(calls.includes('workflow/state'), 'the page still reads')
})

test('#2176: a window that is not read-only keeps Edit, rename and delete', () => {
const view = renderView({ phase: 'ready', pack, opened: { source: 'own', id: 'wf-mine' }, error: '', roster: { bots: [] } })
assert.equal(all(view).some((entry) => entry.type === 'button' && text(entry) === 'Edit'), true, 'a normal window still offers Edit')
assert.equal(all(view).some((entry) => text(entry) === READ_ONLY), false, 'no read-only sentence in a normal window')
const list = renderView({ phase: 'ready', pack, opened: null, error: '', roster: { bots: [] } })
assert.equal(all(list).some((entry) => entry.type === 'button' && entry.props?.['aria-label'] === 'Rename'), true, 'rename is still offered')
assert.equal(all(list).some((entry) => entry.type === 'button' && entry.props?.['aria-label'] === 'Delete'), true, 'delete is still offered')
})

// #2176: `Use this` writes `workflow/create`, and the channel's READS list does not hold that
// method either (`core/src/host/channel.js:227`), so a read-only window that keeps the button
// offers the user a copy the host refuses. The button goes away with Edit; the read side stays.
test('#2176: a read-only window offers no Use this on a preset and never creates a copy', async () => {
const calls = []
const page = mountPage({ call: async (method) => { calls.push(method); return pack }, roster: { bots: [], readOnly: READ_ONLY } })
await page.settle()
await openWorkflow(page, 'delivery')
assert.equal(all(page.tree).some((entry) => entry.type === 'button' && text(entry) === 'Use this'), false,
'a read-only window offers no Use this')
assert.deepEqual(calls.filter((method) => method === 'workflow/create'), [], 'a read-only window never asks the host to copy a preset')
assert.ok(calls.includes('workflow/state'), 'the page still reads')
assert.ok(all(page.tree).some((entry) => entry.props?.['data-node'] === 'coder'), 'the opened graph still renders')
})

test('#2176: a window that is not read-only keeps Use this and copies the preset once', async () => {
const calls = []
const copied = { ...pack, workflows: [...pack.workflows, { ...factory, id: 'delivery-copy', name: 'Copy of delivery' }] }
const page = mountPage({
call: async (method) => { calls.push(method); return method === 'workflow/create' ? copied : pack },
roster: { bots: [] },
})
await page.settle()
await openWorkflow(page, 'delivery')
assert.equal(all(page.tree).some((entry) => entry.type === 'button' && text(entry) === 'Use this'), true, 'a normal window still offers Use this')
await page.click((entry) => text(entry) === 'Use this')
assert.deepEqual(calls.filter((method) => method === 'workflow/create'), ['workflow/create'], 'a normal window copies the preset once')
assert.match(text(page.tree), /Copy of delivery/, 'the copy opens')
})
Loading
Loading