diff --git a/addons/internal/connector-ui/src/ApprovalFields.jsx b/addons/internal/connector-ui/src/ApprovalFields.jsx index 45c350fe88..456e291a4e 100644 --- a/addons/internal/connector-ui/src/ApprovalFields.jsx +++ b/addons/internal/connector-ui/src/ApprovalFields.jsx @@ -1,7 +1,9 @@ /* eslint-disable no-unused-vars -- components used only inside JSX look unused to this config, which has no eslint-plugin-react */ -// How the triggers that the agent proposes in a chat are approved. The same on -// every platform, so the words live here as defaults, like the other shared +// How the triggers that the agent proposes in a chat are approved. It is shown +// under the permission "Propose triggers" (see PermissionFields), as an option +// of it. The same on every platform, so the words live here as defaults, like the other shared // fields. The server enforces the limits; the page only bounds what is typed. +import { useId } from 'react' import { Field, RadioCards, TextInput } from './controls' import { APPROVAL_LIMITS, APPROVAL_MODES, approvalProblem } from './connector' @@ -19,24 +21,33 @@ export const APPROVAL_TEXT = { 'approval.invalid': 'Use a whole number from 1 to {{max}}, or leave it empty.', 'approval.default': 'Default: {{value}}.', 'approval.summary': 'Triggers: {{mode}}', + 'approval.effect': 'Proposals: {{mode}}', + 'approval.limits': 'Limits', 'approval.short.ask': 'asks first', 'approval.short.auto': 'set up at once', 'approval.short.auto_readonly': 'at once if read-only', } -export default function ApprovalFields({ form, setForm, s, triggersOff = false }) { +/** + * The approval mode and its limits, as an option under "Propose triggers". + * With off set (the permission is not ticked) the choice is greyed, with the + * reason, and nothing can be changed. The limits sit in a closed disclosure. + */ +export default function ApprovalOptions({ form, setForm, s, off = false }) { const a = form.approval + const reasonId = useId() const set = (patch) => setForm({ ...form, approval: { ...a, ...patch } }) - const auto = a.mode !== 'ask' const bad = approvalProblem(a) return ( -
- {s('approval.title')} +
+ {s('approval.title')} +

{s('approval.title')}

{s('approval.help')}

- {triggersOff &&

{s('perm.triggersNeed')}

} + {off &&

{s('perm.triggersNeed')}

} set({ mode })} options={APPROVAL_MODES.map((m) => ({ value: m, label: s(`approval.mode.${m}`) }))} /> - {auto && (<> +
+ {s('approval.limits')}

{s('approval.limitsHelp')}

{APPROVAL_LIMITS.map(({ key, max, fallback }) => ( @@ -44,7 +55,7 @@ export default function ApprovalFields({ form, setForm, s, triggersOff = false } value={a[key]} onChange={(e) => set({ [key]: e.target.value })} />} ))} - )} +
) } diff --git a/addons/internal/connector-ui/src/ConnectorPage.jsx b/addons/internal/connector-ui/src/ConnectorPage.jsx index 7b8a73b9d3..d265a05685 100644 --- a/addons/internal/connector-ui/src/ConnectorPage.jsx +++ b/addons/internal/connector-ui/src/ConnectorPage.jsx @@ -10,7 +10,7 @@ import ConfirmDialog from '../../host-compat/src/components/ConfirmDialog' import Icon from '../../host-compat/src/components/Icon' import AnswerFields, { answerSummary, answerT } from './AnswerFields' import AccessView from './AccessView' -import ApprovalFields from './ApprovalFields' +import ApprovalOptions from './ApprovalFields' import FilesFields from './FilesFields' import { Check, Field, Notice, RadioCards, Select, Switch, TextInput } from './controls' import NameList from './NameList' @@ -259,7 +259,8 @@ export default function ConnectorPage({ kind, ns, testId, TokenFields = SingleTo a.id === form.agent_id)} channelsOnly={channelOnly(form, lists)} /> - 0 && !(form.caps || []).includes('triggers.propose')} /> + {/* The approval of proposed triggers sits under the permission Propose triggers; an older host with no capability list has no such row, so it shows here. */} + {capabilities.length === 0 && } {capabilities.length > 0 && } )}
diff --git a/addons/internal/connector-ui/src/PermissionFields.jsx b/addons/internal/connector-ui/src/PermissionFields.jsx index 32d243ba27..c2ff3092bc 100644 --- a/addons/internal/connector-ui/src/PermissionFields.jsx +++ b/addons/internal/connector-ui/src/PermissionFields.jsx @@ -10,6 +10,7 @@ import { Field, Hint, Notice, Switch, TextInput } from './controls' import Icon from '../../host-compat/src/components/Icon' import { MAX_CHANNELS, MAX_CHANNEL_ID, MAX_CHANNEL_LABEL, agentHas, agentHeld, channelsProblem, copyFromAgent, grantableFor, hasNoCaps, presetBlock, roleHas, setCap, setGrant, unpairedSensitive, usualFor, withAddedCaps, withCopiedCaps } from './connector' import PairingFields from './PairingFields' +import ApprovalOptions from './ApprovalFields' export const PERMISSION_TEXT = { 'perm.title': 'What senders may do', @@ -68,14 +69,14 @@ export const PERMISSION_TEXT = { 'perm.channelProblem.label': 'A name has at most {{max}} characters.', 'perm.channelProblem.repeat': 'A channel is listed twice.', 'perm.channelProblem.count': 'At most {{max}} channels.', - 'perm.triggersNeed': 'These settings apply only while "Propose triggers" is on above.', + 'perm.triggersNeed': 'Turn on Propose triggers to use this.', } export function permissionT(t) { return (key, opts = {}) => t(key, { defaultValue: PERMISSION_TEXT[key], ...opts }) } -function Cap({ cap, on, onChange, s, lacks }) { +function Cap({ cap, on, onChange, s, lacks, children, effect }) { // A permission the agent does not have cannot be switched on; one that is // already on can still be switched off. const blocked = lacks && !on @@ -86,6 +87,8 @@ function Cap({ cap, on, onChange, s, lacks }) { {cap.sensitive && {s('perm.sensitive')}} {cap.description}{cap.sensitive ? ` ${s('perm.sensitiveHelp')}` : ''} + {effect &&

{effect}

} + {children} {lacks &&

{s('perm.agentLacks')}

} ) @@ -303,7 +306,10 @@ export default function PermissionFields({ form, setForm, kind, capabilities, s, )}
    - {caps.map((c) => setForm(setCap(form, c.name, v))} />)} + {caps.map((c) => setForm(setCap(form, c.name, v))}> + {c.name === 'triggers.propose' && form.approval && } + )}
diff --git a/addons/internal/connector-ui/src/connector.css b/addons/internal/connector-ui/src/connector.css index 850b6b13ea..30bd3b2a53 100644 --- a/addons/internal/connector-ui/src/connector.css +++ b/addons/internal/connector-ui/src/connector.css @@ -199,3 +199,11 @@ fieldset.cn-group { .cn-confirm__list { margin: 0; padding-left: var(--dk-space-5); font-size: var(--fs-small); line-height: var(--lh-small); } .cn-cap__lacks { margin: 2px 0 0; font-size: var(--fs-small); line-height: var(--lh-small); color: var(--color-text-muted); } .cn-cap[data-lacks="true"] .cn-cap__title { color: var(--color-text-muted); } + +/* The approval of proposed triggers, nested under the permission Propose triggers. */ +.cn-cap__effect { margin: 2px 0 0; font-size: var(--fs-small); line-height: var(--lh-small); color: var(--color-text-secondary); } +.cn-option { margin: var(--dk-space-2) 0 0 var(--dk-space-3); padding: 0 0 0 var(--dk-space-3); border: 0; border-left: 2px solid var(--color-border-subtle); min-width: 0; display: flex; flex-direction: column; gap: var(--dk-space-2); } +.cn-option[data-off="true"] { opacity: 0.6; } +.cn-option__title { margin: 0; font-size: var(--fs-small); line-height: var(--lh-small); font-weight: var(--font-weight-semibold); color: var(--color-text-primary); } +.cn-option__limits summary { cursor: pointer; font-size: var(--fs-small); color: var(--color-text-secondary); } +.cn-option__limits[open] summary { margin-bottom: var(--dk-space-2); } diff --git a/core/http/react-ui/e2e/connector-permissions.spec.js b/core/http/react-ui/e2e/connector-permissions.spec.js index 84e29dc237..af1ca95248 100644 --- a/core/http/react-ui/e2e/connector-permissions.spec.js +++ b/core/http/react-ui/e2e/connector-permissions.spec.js @@ -319,13 +319,41 @@ test.describe('Connector permissions', () => { await shot(page, 'perm-5-access') }) - test('trigger approval says it applies only while triggers are allowed', async ({ page }) => { - await api(page, [SET]).install() + test('the approval of proposed triggers sits under Propose triggers, shows its choice in the row and is greyed while the permission is off', async ({ page }) => { + const a = api(page, [{ ...LEGACY, caps: ['memory.write'], approval: { mode: 'auto_readonly' } }]) + await a.install() await page.goto(PAGE) await page.getByRole('button', { name: 'Edit' }).click() - await expect(page.getByTestId('connector-approval-off')).toBeVisible() - await page.getByTestId('connector-cap-switch-triggers.propose').click() - await expect(page.getByTestId('connector-approval-off')).toHaveCount(0) + const row = page.getByTestId('connector-cap-triggers.propose') + // One place only: nothing at the bottom of the page. + await expect(page.getByTestId('connector-approval')).toHaveCount(1) + await expect(row.getByTestId('connector-approval')).toBeVisible() + await expect(row.getByTestId('connector-cap-effect-triggers.propose')).toHaveText('Proposals: at once if read-only') + // Permission off: greyed, with the reason, and the radios cannot be used. + await expect(row.getByTestId('connector-approval-off')).toHaveText('Turn on Propose triggers to use this.') + await expect(row.getByRole('radio').first()).toBeDisabled() + await expect(row.getByTestId('connector-approval')).toHaveAccessibleDescription('Turn on Propose triggers to use this.') + await shot(page, 'perm-9-approval-greyed') + await row.getByTestId('connector-cap-switch-triggers.propose').click() + await expect(row.getByTestId('connector-approval-off')).toHaveCount(0) + await expect(row.getByRole('radio', { name: /Ask me/ })).toBeEnabled() + // The three modes change the line of the row. + await row.getByRole('radio', { name: /Ask me/ }).check() + await expect(row.getByTestId('connector-cap-effect-triggers.propose')).toHaveText('Proposals: asks first') + await shot(page, 'perm-10-approval-ask') + await row.getByRole('radio', { name: /Set it up at once/ }).check() + await expect(row.getByTestId('connector-cap-effect-triggers.propose')).toHaveText('Proposals: set up at once') + await shot(page, 'perm-11-approval-auto') + await row.getByRole('radio', { name: /At once when the runs only read/ }).check() + await expect(row.getByTestId('connector-cap-effect-triggers.propose')).toHaveText('Proposals: at once if read-only') + await shot(page, 'perm-12-approval-readonly') + // The limits are in a closed disclosure and are sent as before. + const limits = row.getByTestId('connector-approval-limits') + await expect(limits).not.toHaveAttribute('open', '') + await limits.getByText('Limits', { exact: true }).click() + await limits.getByTestId('connector-approval-max_pending').fill('3') + await page.getByRole('button', { name: 'Save' }).click() + await expect.poll(() => a.s.calls.find(c => c.method === 'PUT')?.body.approval).toEqual({ mode: 'auto_readonly', max_pending: 3, min_gap_minutes: 0, max_runs_per_day: 0, max_lifetime_days: 0 }) }) test('an older host that sends no capability list shows no permission section', async ({ page }) => { diff --git a/docs/content/features/chats-connectors.md b/docs/content/features/chats-connectors.md index 24393261d3..bd572a3133 100644 --- a/docs/content/features/chats-connectors.md +++ b/docs/content/features/chats-connectors.md @@ -666,7 +666,9 @@ trigger stays in Studio. Nothing runs before the owner confirms. ### Approval modes The owner chooses, per binding, what happens to a proposal. The choice is on the settings -page of the connector add-on, or in the `approval` field of the binding +page of the connector add-on, nested under the permission **Propose triggers** (greyed while +that permission is off, with its limits in a closed **Limits** list, and the current mode in +the line of the row), or in the `approval` field of the binding (`PUT /api/chats/connectors/{id}`), and is the same for every connector. | `mode` | Behavior |