From 32df17104c13b713a36057ab9aaeef3fd03d9d24 Mon Sep 17 00:00:00 2001 From: oleg Date: Fri, 5 Apr 2024 15:00:31 +0200 Subject: [PATCH] fix(editor): Allow pinning of AI root nodes (#9060) Signed-off-by: Oleg Ivaniv --- cypress/e2e/13-pinning.cy.ts | 2 +- cypress/e2e/14-mapping.cy.ts | 2 +- cypress/e2e/24-ndv-paired-item.cy.ts | 2 +- cypress/pages/ndv.ts | 11 ++ packages/editor-ui/src/components/RunData.vue | 175 +++++++++--------- .../src/composables/useContextMenu.ts | 30 +-- .../src/composables/usePinnedData.ts | 26 ++- 7 files changed, 127 insertions(+), 121 deletions(-) diff --git a/cypress/e2e/13-pinning.cy.ts b/cypress/e2e/13-pinning.cy.ts index 709000b5e6..a9ccc78818 100644 --- a/cypress/e2e/13-pinning.cy.ts +++ b/cypress/e2e/13-pinning.cy.ts @@ -134,7 +134,7 @@ describe('Data pinning', () => { ndv.getters.pinDataButton().should('not.exist'); ndv.getters.editPinnedDataButton().should('be.visible'); - ndv.actions.setPinnedData([ + ndv.actions.pastePinnedData([ { test: '1'.repeat(Cypress.env('MAX_PINNED_DATA_SIZE')), }, diff --git a/cypress/e2e/14-mapping.cy.ts b/cypress/e2e/14-mapping.cy.ts index b797a10aca..4029336c11 100644 --- a/cypress/e2e/14-mapping.cy.ts +++ b/cypress/e2e/14-mapping.cy.ts @@ -206,7 +206,7 @@ describe('Data mapping', () => { workflowPage.actions.addInitialNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); workflowPage.getters.canvasNodeByName(MANUAL_TRIGGER_NODE_DISPLAY_NAME).click(); workflowPage.actions.openNode(MANUAL_TRIGGER_NODE_DISPLAY_NAME); - ndv.actions.setPinnedData([ + ndv.actions.pastePinnedData([ { input: [ { diff --git a/cypress/e2e/24-ndv-paired-item.cy.ts b/cypress/e2e/24-ndv-paired-item.cy.ts index 382be75bf3..1b2b4f1efe 100644 --- a/cypress/e2e/24-ndv-paired-item.cy.ts +++ b/cypress/e2e/24-ndv-paired-item.cy.ts @@ -324,7 +324,7 @@ describe('NDV', () => { ]; /* prettier-ignore */ workflowPage.actions.openNode('Get thread details1'); - ndv.actions.setPinnedData(PINNED_DATA); + ndv.actions.pastePinnedData(PINNED_DATA); ndv.actions.close(); workflowPage.actions.executeWorkflow(); diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index 8460282ade..f9cb5b1304 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -155,6 +155,17 @@ export class NDV extends BasePage { this.actions.savePinnedData(); }, + pastePinnedData: (data: object) => { + this.getters.editPinnedDataButton().click(); + + this.getters.pinnedDataEditor().click(); + this.getters + .pinnedDataEditor() + .type('{selectall}{backspace}', { delay: 0 }) + .paste(JSON.stringify(data)); + + this.actions.savePinnedData(); + }, clearParameterInput: (parameterName: string) => { this.getters.parameterInput(parameterName).type(`{selectall}{backspace}`); }, diff --git a/packages/editor-ui/src/components/RunData.vue b/packages/editor-ui/src/components/RunData.vue index db685a2e21..5adafcd71e 100644 --- a/packages/editor-ui/src/components/RunData.vue +++ b/packages/editor-ui/src/components/RunData.vue @@ -217,8 +217,8 @@
@@ -725,45 +725,6 @@ export default defineComponent({ search: '', }; }, - mounted() { - this.init(); - - if (!this.isPaneTypeInput) { - this.showPinDataDiscoveryTooltip(this.jsonData); - } - this.ndvStore.setNDVBranchIndex({ - pane: this.paneType as 'input' | 'output', - branchIndex: this.currentOutputIndex, - }); - - if (this.paneType === 'output') { - this.setDisplayMode(); - this.activatePane(); - } - - if (this.hasRunError) { - const error = this.workflowRunData?.[this.node.name]?.[this.runIndex]?.error; - const errorsToTrack = ['unknown error']; - - if (error && errorsToTrack.some((e) => error.message.toLowerCase().includes(e))) { - this.$telemetry.track( - `User encountered an error: "${error.message}"`, - { - node: this.node.type, - errorMessage: error.message, - nodeVersion: this.node.typeVersion, - n8nVersion: this.rootStore.versionCli, - }, - { - withPostHog: true, - }, - ); - } - } - }, - beforeUnmount() { - this.hidePinDataDiscoveryTooltip(); - }, computed: { ...mapStores( useNodeTypesStore, @@ -803,21 +764,14 @@ export default defineComponent({ return this.nodeTypesStore.isTriggerNode(this.node.type); }, canPinData(): boolean { - // Only "main" inputs can pin data - if (this.node === null) { return false; } - const workflow = this.workflowsStore.getCurrentWorkflow(); - const workflowNode = workflow.getNode(this.node.name); - const inputs = NodeHelpers.getNodeInputs(workflow, workflowNode!, this.nodeType!); - const inputNames = NodeHelpers.getConnectionTypes(inputs); - - const nonMainInputs = !!inputNames.find((inputName) => inputName !== NodeConnectionType.Main); + const canPinNode = usePinnedData(this.node).canPinNode(false); return ( - !nonMainInputs && + canPinNode && !this.isPaneTypeInput && this.pinnedData.isValidNodeType.value && !(this.binaryData && this.binaryData.length > 0) @@ -1035,6 +989,87 @@ export default defineComponent({ return this.hasNodeRun && !this.inputData.length && this.search; }, }, + watch: { + node(newNode: INodeUi, prevNode: INodeUi) { + if (newNode.id === prevNode.id) return; + this.init(); + }, + hasNodeRun() { + if (this.paneType === 'output') this.setDisplayMode(); + }, + inputDataPage: { + handler(data: INodeExecutionData[]) { + if (this.paneType && data) { + this.ndvStore.setNDVPanelDataIsEmpty({ + panel: this.paneType as 'input' | 'output', + isEmpty: data.every((item) => isEmpty(item.json)), + }); + } + }, + immediate: true, + deep: true, + }, + jsonData(data: IDataObject[], prevData: IDataObject[]) { + if (isEqual(data, prevData)) return; + this.refreshDataSize(); + this.showPinDataDiscoveryTooltip(data); + }, + binaryData(newData: IBinaryKeyData[], prevData: IBinaryKeyData[]) { + if (newData.length && !prevData.length && this.displayMode !== 'binary') { + this.switchToBinary(); + } else if (!newData.length && this.displayMode === 'binary') { + this.onDisplayModeChange('table'); + } + }, + currentOutputIndex(branchIndex: number) { + this.ndvStore.setNDVBranchIndex({ + pane: this.paneType as 'input' | 'output', + branchIndex, + }); + }, + search(newSearch: string) { + this.$emit('search', newSearch); + }, + }, + mounted() { + this.init(); + + if (!this.isPaneTypeInput) { + this.showPinDataDiscoveryTooltip(this.jsonData); + } + this.ndvStore.setNDVBranchIndex({ + pane: this.paneType as 'input' | 'output', + branchIndex: this.currentOutputIndex, + }); + + if (this.paneType === 'output') { + this.setDisplayMode(); + this.activatePane(); + } + + if (this.hasRunError) { + const error = this.workflowRunData?.[this.node.name]?.[this.runIndex]?.error; + const errorsToTrack = ['unknown error']; + + if (error && errorsToTrack.some((e) => error.message.toLowerCase().includes(e))) { + this.$telemetry.track( + `User encountered an error: "${error.message}"`, + { + node: this.node.type, + errorMessage: error.message, + nodeVersion: this.node.typeVersion, + n8nVersion: this.rootStore.versionCli, + }, + { + withPostHog: true, + }, + ); + } + } + }, + beforeUnmount() { + this.hidePinDataDiscoveryTooltip(); + }, methods: { getResolvedNodeOutputs() { if (this.node && this.nodeType) { @@ -1500,48 +1535,6 @@ export default defineComponent({ document.dispatchEvent(new KeyboardEvent('keyup', { key: '/' })); }, }, - watch: { - node(newNode: INodeUi, prevNode: INodeUi) { - if (newNode.id === prevNode.id) return; - this.init(); - }, - hasNodeRun() { - if (this.paneType === 'output') this.setDisplayMode(); - }, - inputDataPage: { - handler(data: INodeExecutionData[]) { - if (this.paneType && data) { - this.ndvStore.setNDVPanelDataIsEmpty({ - panel: this.paneType as 'input' | 'output', - isEmpty: data.every((item) => isEmpty(item.json)), - }); - } - }, - immediate: true, - deep: true, - }, - jsonData(data: IDataObject[], prevData: IDataObject[]) { - if (isEqual(data, prevData)) return; - this.refreshDataSize(); - this.showPinDataDiscoveryTooltip(data); - }, - binaryData(newData: IBinaryKeyData[], prevData: IBinaryKeyData[]) { - if (newData.length && !prevData.length && this.displayMode !== 'binary') { - this.switchToBinary(); - } else if (!newData.length && this.displayMode === 'binary') { - this.onDisplayModeChange('table'); - } - }, - currentOutputIndex(branchIndex: number) { - this.ndvStore.setNDVBranchIndex({ - pane: this.paneType as 'input' | 'output', - branchIndex, - }); - }, - search(newSearch: string) { - this.$emit('search', newSearch); - }, - }, }); diff --git a/packages/editor-ui/src/composables/useContextMenu.ts b/packages/editor-ui/src/composables/useContextMenu.ts index ebf78bdb67..b4aa25b9e3 100644 --- a/packages/editor-ui/src/composables/useContextMenu.ts +++ b/packages/editor-ui/src/composables/useContextMenu.ts @@ -1,20 +1,15 @@ import type { XYPosition } from '@/Interface'; -import { - NOT_DUPLICATABE_NODE_TYPES, - PIN_DATA_NODE_TYPES_DENYLIST, - STICKY_NODE_TYPE, -} from '@/constants'; +import { NOT_DUPLICATABE_NODE_TYPES, STICKY_NODE_TYPE } from '@/constants'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { useSourceControlStore } from '@/stores/sourceControl.store'; import { useUIStore } from '@/stores/ui.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import type { IActionDropdownItem } from 'n8n-design-system/src/components/N8nActionDropdown/ActionDropdown.vue'; -import { NodeHelpers, NodeConnectionType } from 'n8n-workflow'; import type { INode, INodeTypeDescription } from 'n8n-workflow'; import { computed, ref, watch } from 'vue'; import { getMousePosition } from '../utils/nodeViewUtils'; import { useI18n } from './useI18n'; -import { useDataSchema } from './useDataSchema'; +import { usePinnedData } from './usePinnedData'; export type ContextMenuTarget = | { source: 'canvas' } @@ -47,7 +42,7 @@ export const useContextMenu = (onAction: ContextMenuActionCallback = () => {}) = const nodeTypesStore = useNodeTypesStore(); const workflowsStore = useWorkflowsStore(); const sourceControlStore = useSourceControlStore(); - const { getInputDataWithPinned } = useDataSchema(); + const i18n = useI18n(); const isReadOnly = computed( @@ -83,13 +78,6 @@ export const useContextMenu = (onAction: ContextMenuActionCallback = () => {}) = return canAddNodeOfType(nodeType); }; - const canPinNode = (node: INode): boolean => { - const nodeType = nodeTypesStore.getNodeType(node.type, node.typeVersion); - const dataToPin = getInputDataWithPinned(node); - if (!nodeType || dataToPin.length === 0) return false; - return nodeType.outputs.length === 1 && !PIN_DATA_NODE_TYPES_DENYLIST.includes(node.type); - }; - const hasPinData = (node: INode): boolean => { return !!workflowsStore.pinDataByNodeName(node.name); }; @@ -159,16 +147,6 @@ export const useContextMenu = (onAction: ContextMenuActionCallback = () => {}) = ...selectionActions, ]; } else { - const nonMainInputs = (node: INode) => { - const workflow = workflowsStore.getCurrentWorkflow(); - const workflowNode = workflow.getNode(node.name); - const nodeType = nodeTypesStore.getNodeType(node.type, node.typeVersion); - const inputs = NodeHelpers.getNodeInputs(workflow, workflowNode!, nodeType!); - const inputNames = NodeHelpers.getConnectionTypes(inputs); - - return !!inputNames.find((inputName) => inputName !== NodeConnectionType.Main); - }; - const menuActions: IActionDropdownItem[] = [ !onlyStickies && { id: 'toggle_activation', @@ -184,7 +162,7 @@ export const useContextMenu = (onAction: ContextMenuActionCallback = () => {}) = ? i18n.baseText('contextMenu.unpin', i18nOptions) : i18n.baseText('contextMenu.pin', i18nOptions), shortcut: { keys: ['p'] }, - disabled: nodes.some(nonMainInputs) || isReadOnly.value || !nodes.every(canPinNode), + disabled: isReadOnly.value || !nodes.every((n) => usePinnedData(n).canPinNode(true)), }, { id: 'copy', diff --git a/packages/editor-ui/src/composables/usePinnedData.ts b/packages/editor-ui/src/composables/usePinnedData.ts index a35fdaa9ba..1dce344656 100644 --- a/packages/editor-ui/src/composables/usePinnedData.ts +++ b/packages/editor-ui/src/composables/usePinnedData.ts @@ -1,7 +1,7 @@ import { useToast } from '@/composables/useToast'; import { useI18n } from '@/composables/useI18n'; import type { INodeExecutionData, IPinData } from 'n8n-workflow'; -import { jsonParse, jsonStringify } from 'n8n-workflow'; +import { jsonParse, jsonStringify, NodeConnectionType, NodeHelpers } from 'n8n-workflow'; import { MAX_EXPECTED_REQUEST_SIZE, MAX_PINNED_DATA_SIZE, @@ -18,6 +18,8 @@ import { computed, unref } from 'vue'; import { useRootStore } from '@/stores/n8nRoot.store'; import { storeToRefs } from 'pinia'; import { useNodeType } from '@/composables/useNodeType'; +import { useDataSchema } from './useDataSchema'; +import { useNodeTypesStore } from '@/stores/nodeTypes.store'; export type PinDataSource = | 'pin-icon-click' @@ -47,6 +49,7 @@ export function usePinnedData( const i18n = useI18n(); const telemetry = useTelemetry(); const externalHooks = useExternalHooks(); + const { getInputDataWithPinned } = useDataSchema(); const { pushRef } = storeToRefs(rootStore); const { isSubNodeType, isMultipleOutputsNodeType } = useNodeType({ @@ -73,6 +76,26 @@ export function usePinnedData( ); }); + function canPinNode(checkDataEmpty = false) { + const targetNode = unref(node); + if (targetNode === null) return false; + + const nodeType = useNodeTypesStore().getNodeType(targetNode.type, targetNode.typeVersion); + const dataToPin = getInputDataWithPinned(targetNode); + + if (!nodeType || (checkDataEmpty && dataToPin.length === 0)) return false; + + const workflow = workflowsStore.getCurrentWorkflow(); + const outputs = NodeHelpers.getNodeOutputs(workflow, targetNode, nodeType); + const mainOutputs = outputs.filter((output) => + typeof output === 'string' + ? output === NodeConnectionType.Main + : output.type === NodeConnectionType.Main, + ); + + return mainOutputs.length === 1 && !PIN_DATA_NODE_TYPES_DENYLIST.includes(targetNode.type); + } + function isValidJSON(data: string): boolean { try { JSON.parse(data); @@ -246,6 +269,7 @@ export function usePinnedData( data, hasData, isValidNodeType, + canPinNode, setData, onSetDataSuccess, onSetDataError,