From d460eb28ca8223b5d7941d7e70df6d2b845ccec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Castro?= Date: Fri, 16 Oct 2026 11:51:01 +0000 Subject: [PATCH] fix(editor): Explain why copy, cut, duplicate and pin refuse a restricted node (no-changelog) (#40997) Co-authored-by: Claude Opus 5 --- .../composables/useCanvasOperations.test.ts | 63 +++++++++++++++++++ .../app/composables/useCanvasOperations.ts | 25 +++++++- .../editor-ui/src/app/views/NodeView.test.ts | 27 ++++++++ .../editor-ui/src/app/views/NodeView.vue | 2 +- 4 files changed, 113 insertions(+), 4 deletions(-) diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts index 628639a4afb..79057ef144b 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts @@ -2739,6 +2739,40 @@ describe('useCanvasOperations', () => { expect(setDataMock).not.toHaveBeenCalled(); expect(unsetDataMock).not.toHaveBeenCalled(); }); + + it('should not pin a selection that contains a restricted node type', () => { + const historyStore = mockedStore(useHistoryStore); + const toast = useToast(); + + const restrictedNode = createTestNode({ + id: '1', + name: 'RestrictedNode', + type: 'n8n-nodes-base.slack', + }); + const pinnableNode = createTestNode({ id: '2', name: 'PinnableNode' }); + + vi.spyOn(workflowDocumentStoreInstance, 'getNodesByIds').mockReturnValue([ + restrictedNode, + pinnableNode, + ]); + canPinNodeMock.mockReturnValue(true); + getInputDataWithPinnedMock.mockReturnValue([{ json: { test: 'data' } }]); + mockedStore(useTypeAvailabilityPoliciesStore).getNodeTypeAvailability.mockImplementation( + (name) => ({ name, available: name !== 'n8n-nodes-base.slack' }), + ); + + useCanvasOperations().toggleNodesPinned(['1', '2'], 'pin-icon-click'); + + expect(setDataMock).not.toHaveBeenCalled(); + expect(unsetDataMock).not.toHaveBeenCalled(); + expect(historyStore.startRecordingUndo).not.toHaveBeenCalled(); + expect(toast.showMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'warning', + message: expect.stringContaining('n8n-nodes-base.slack'), + }), + ); + }); }); describe('addConnections', () => { @@ -5110,6 +5144,7 @@ describe('useCanvasOperations', () => { }); it('should not copy a selection that contains a restricted node type', async () => { + const toast = useToast(); const nodes = buildImportNodes(); nodes[1].type = 'n8n-nodes-base.slack'; workflowDocumentStoreInstance.allNodes = nodes; @@ -5123,6 +5158,12 @@ describe('useCanvasOperations', () => { expect(copied).toBe(false); expect(useClipboard().copy).not.toHaveBeenCalled(); + expect(toast.showMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'warning', + message: expect.stringContaining('n8n-nodes-base.slack'), + }), + ); }); it('should include nodeGroups when all group members are copied', async () => { @@ -7404,6 +7445,28 @@ describe('useCanvasOperations', () => { expect(workflowDocumentStoreInstance.createGroup).not.toHaveBeenCalled(); }); + it('should not duplicate a selection that contains a restricted node type', async () => { + const toast = useToast(); + const nodes = buildImportNodes(); + nodes[1].type = 'n8n-nodes-base.slack'; + workflowDocumentStoreInstance.allNodes = nodes; + vi.spyOn(workflowDocumentStoreInstance, 'getNodesByIds').mockReturnValue(nodes); + mockedStore(useTypeAvailabilityPoliciesStore).getNodeTypeAvailability.mockImplementation( + (name) => ({ name, available: name !== 'n8n-nodes-base.slack' }), + ); + + const duplicatedNodeIds = await useCanvasOperations().duplicateNodes(['1', '2']); + + expect(duplicatedNodeIds).toEqual([]); + expect(workflowDocumentStoreInstance.addNode).not.toHaveBeenCalled(); + expect(toast.showMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'warning', + message: expect.stringContaining('n8n-nodes-base.slack'), + }), + ); + }); + it('should show max node type error when duplicating nodes that exceed maxNodes limit', async () => { const toast = useToast(); const nodeTypesStore = useNodeTypesStore(); diff --git a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts index 4307b866be0..792e578215a 100644 --- a/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts +++ b/packages/frontend/editor-ui/src/app/composables/useCanvasOperations.ts @@ -172,6 +172,7 @@ import { useRoute, useRouter } from 'vue-router'; import { useTemplatesStore } from '@/features/workflows/templates/templates.store'; import { isValidNodeConnectionType } from '@/app/utils/typeGuards'; import { removePreviewToken } from '@/features/shared/nodeCreator/nodeCreator.utils'; +import { useRestrictedNodeWarning } from '@/features/shared/nodeCreator/composables/useRestrictedNodeWarning'; import { useSetupPanelStore } from '@/features/setupPanel/setupPanel.store'; import { useEmptyCanvasGroupsFlag } from '@/features/workflows/canvas/composables/useEmptyCanvasGroupsFlag'; import { clearAllNodeResourceLocatorValues } from '@/features/workflows/templates/utils/templateTransforms'; @@ -273,6 +274,7 @@ export function useCanvasOperations() { const workflowHelpers = useWorkflowHelpers(); const nodeHelpers = useNodeHelpers(); const aiSimulatedDataGuard = useAiSimulatedDataGuard(); + const { warnIfRestricted } = useRestrictedNodeWarning(); const { requireNodeTypeDescription, resolveNodeParameters, @@ -1138,6 +1140,7 @@ export function useCanvasOperations() { { trackHistory = true, trackBulk = true } = {}, ) { const nodes = workflowDocumentStore.value.getNodesByIds(ids); + if (refuseRestrictedNodes(nodes)) return; // Filter to only pinnable nodes const pinnableNodesWithPinnedData = nodes @@ -3667,8 +3670,24 @@ export function useCanvasOperations() { return filteredConnections; } + /** + * Refuses an operation on a restricted node, and says why. + * The context menu disables its own items, but the keyboard shortcuts cannot, + * so the refusal must explain itself. + */ + function refuseRestrictedNodes(nodes: INodeUi[]): boolean { + const restricted = nodes.find((node) => isNodeTypeRestricted(node.type)); + if (!restricted) return false; + + warnIfRestricted(restricted.type); + return true; + } + async function duplicateNodes(ids: string[], options: { viewport?: ViewportBoundaries } = {}) { - const workflowData = deepCopy(getNodesToSave(workflowDocumentStore.value.getNodesByIds(ids))); + const nodes = workflowDocumentStore.value.getNodesByIds(ids); + if (refuseRestrictedNodes(nodes)) return []; + + const workflowData = deepCopy(getNodesToSave(nodes)); const result = await importWorkflowData(workflowData, 'duplicate', { viewport: options.viewport, importTags: false, @@ -3676,10 +3695,10 @@ export function useCanvasOperations() { return result.nodes?.map((node) => node.id).filter(isPresent) ?? []; } + async function copyNodes(ids: string[]): Promise { const nodes = workflowDocumentStore.value.getNodesByIds(ids); - const hasRestrictedNode = nodes.some((node) => isNodeTypeRestricted(node.type)); - if (hasRestrictedNode) return false; + if (refuseRestrictedNodes(nodes)) return false; const workflowData = deepCopy(getNodesToSave(nodes)); if (!emptyCanvasGroupsEnabled.value) { diff --git a/packages/frontend/editor-ui/src/app/views/NodeView.test.ts b/packages/frontend/editor-ui/src/app/views/NodeView.test.ts index bbf1e182e1f..bd5e06a2885 100644 --- a/packages/frontend/editor-ui/src/app/views/NodeView.test.ts +++ b/packages/frontend/editor-ui/src/app/views/NodeView.test.ts @@ -1143,6 +1143,33 @@ describe('NodeView', () => { expect(copiedText[0]).toContain('"name": "Existing"'); }); + it('refuses to copy a restricted node and does not claim success', async () => { + vi.spyOn(useTypeAvailabilityPoliciesStore(), 'getNodeTypeAvailability').mockReturnValue({ + name: MANUAL_TRIGGER_NODE_TYPE, + available: false, + }); + mockShowMessage.mockClear(); + const { findByTestId } = renderNodeView(); + + await userEvent.click(await findByTestId('canvas-stub-copy')); + + await waitFor(() => + expect(mockMcpJsonNudgeGate).toHaveBeenCalledWith('copy', expect.any(Function)), + ); + await deferred?.(); + + expect(copiedText).toEqual([]); + expect(mockShowMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'warning', + message: expect.stringContaining(MANUAL_TRIGGER_NODE_TYPE), + }), + ); + expect(mockShowMessage).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'success' }), + ); + }); + it('drops a deferred copy once NodeView has unmounted', async () => { const { findByTestId, unmount } = renderNodeView(); diff --git a/packages/frontend/editor-ui/src/app/views/NodeView.vue b/packages/frontend/editor-ui/src/app/views/NodeView.vue index 8df947c0374..7937e0b0f68 100644 --- a/packages/frontend/editor-ui/src/app/views/NodeView.vue +++ b/packages/frontend/editor-ui/src/app/views/NodeView.vue @@ -625,7 +625,7 @@ async function onCopyNodes(ids: string[]) { return; } - await copyNodes(ids); + if (!(await copyNodes(ids))) return; toast.showMessage({ title: i18n.baseText('generic.copiedToClipboard'), type: 'success' }); };