From fcf559b93ddf8e333d30cf61ca34538330281489 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Thu, 26 Jun 2025 15:03:33 +0200 Subject: [PATCH] chore: Refactor node parameter assignment logic out of NodeSettings (no-changelog) (#16665) --- .../editor-ui/src/components/NodeSettings.vue | 315 +----------------- .../useNodeSettingsParameters.test.ts | 69 ++++ .../composables/useNodeSettingsParameters.ts | 231 +++++++++++++ .../editor-ui/src/stores/nodeTypes.store.ts | 2 +- .../src/utils/nodeSettingsUtils.test.ts | 89 ++++- .../editor-ui/src/utils/nodeSettingsUtils.ts | 116 ++++++- .../editor-ui/src/utils/parameterUtils.ts | 0 7 files changed, 515 insertions(+), 307 deletions(-) create mode 100644 packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.test.ts create mode 100644 packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.ts create mode 100644 packages/frontend/editor-ui/src/utils/parameterUtils.ts diff --git a/packages/frontend/editor-ui/src/components/NodeSettings.vue b/packages/frontend/editor-ui/src/components/NodeSettings.vue index 9d3ae4d5ed6..9ba51fa1cd0 100644 --- a/packages/frontend/editor-ui/src/components/NodeSettings.vue +++ b/packages/frontend/editor-ui/src/components/NodeSettings.vue @@ -8,15 +8,7 @@ import type { NodeParameterValue, INodeCredentialDescription, } from 'n8n-workflow'; -import { - NodeHelpers, - NodeConnectionTypes, - deepCopy, - isINodePropertyCollectionList, - isINodePropertiesList, - isINodePropertyOptionsList, - displayParameter, -} from 'n8n-workflow'; +import { NodeHelpers, NodeConnectionTypes, deepCopy } from 'n8n-workflow'; import type { CurlToJSONResponse, INodeUi, @@ -33,8 +25,6 @@ import NodeSettingsTabs from '@/components/NodeSettingsTabs.vue'; import NodeWebhooks from '@/components/NodeWebhooks.vue'; import NDVSubConnections from '@/components/NDVSubConnections.vue'; import get from 'lodash/get'; -import set from 'lodash/set'; -import unset from 'lodash/unset'; import NodeExecuteButton from './NodeExecuteButton.vue'; import { isCommunityPackageName } from '@/utils/nodeTypesUtils'; @@ -51,9 +41,8 @@ import { useI18n } from '@n8n/i18n'; import { useTelemetry } from '@/composables/useTelemetry'; import { importCurlEventBus, ndvEventBus } from '@/event-bus'; import { ProjectTypes } from '@/types/projects.types'; -import { updateDynamicConnections } from '@/utils/nodeSettingsUtils'; import FreeAiCreditsCallout from '@/components/FreeAiCreditsCallout.vue'; -import { useCanvasOperations } from '@/composables/useCanvasOperations'; +import { useNodeSettingsParameters } from '@/composables/useNodeSettingsParameters'; import { N8nIconButton } from '@n8n/design-system'; const props = withDefaults( @@ -104,22 +93,11 @@ const telemetry = useTelemetry(); const nodeHelpers = useNodeHelpers(); const externalHooks = useExternalHooks(); const i18n = useI18n(); -const canvasOperations = useCanvasOperations(); +const nodeSettingsParameters = useNodeSettingsParameters(); +const nodeValues = nodeSettingsParameters.nodeValues; const nodeValid = ref(true); const openPanel = ref<'params' | 'settings'>('params'); -const nodeValues = ref({ - color: '#ff0000', - alwaysOutputData: false, - executeOnce: false, - notesInFlow: false, - onError: 'stopWorkflow', - retryOnFail: false, - maxTries: 3, - waitBetweenTries: 1000, - notes: '', - parameters: {}, -}); // Used to prevent nodeValues from being overwritten by defaults on reopening ndv const nodeValuesInitialized = ref(false); @@ -257,126 +235,6 @@ const credentialOwnerName = computed(() => { return credentialsStore.getCredentialOwnerName(credential); }); -const setValue = (name: string, value: NodeParameterValue) => { - const nameParts = name.split('.'); - let lastNamePart: string | undefined = nameParts.pop(); - - let isArray = false; - if (lastNamePart !== undefined && lastNamePart.includes('[')) { - // It includes an index so we have to extract it - const lastNameParts = lastNamePart.match(/(.*)\[(\d+)\]$/); - if (lastNameParts) { - nameParts.push(lastNameParts[1]); - lastNamePart = lastNameParts[2]; - isArray = true; - } - } - - // Set the value so that everything updates correctly in the UI - if (nameParts.length === 0) { - // Data is on top level - if (value === null) { - // Property should be deleted - if (lastNamePart) { - const { [lastNamePart]: removedNodeValue, ...remainingNodeValues } = nodeValues.value; - nodeValues.value = remainingNodeValues; - } - } else { - // Value should be set - nodeValues.value = { - ...nodeValues.value, - [lastNamePart as string]: value, - }; - } - } else { - // Data is on lower level - if (value === null) { - // Property should be deleted - let tempValue = get(nodeValues.value, nameParts.join('.')) as - | INodeParameters - | INodeParameters[]; - - if (lastNamePart && !Array.isArray(tempValue)) { - const { [lastNamePart]: removedNodeValue, ...remainingNodeValues } = tempValue; - tempValue = remainingNodeValues; - } - - if (isArray && Array.isArray(tempValue) && tempValue.length === 0) { - // If a value from an array got delete and no values are left - // delete also the parent - lastNamePart = nameParts.pop(); - tempValue = get(nodeValues.value, nameParts.join('.')) as INodeParameters; - if (lastNamePart) { - const { [lastNamePart]: removedArrayNodeValue, ...remainingArrayNodeValues } = tempValue; - tempValue = remainingArrayNodeValues; - } - } - } else { - // Value should be set - if (typeof value === 'object') { - set( - get(nodeValues.value, nameParts.join('.')) as Record, - lastNamePart as string, - deepCopy(value), - ); - } else { - set( - get(nodeValues.value, nameParts.join('.')) as Record, - lastNamePart as string, - value, - ); - } - } - } - - nodeValues.value = { ...nodeValues.value }; -}; - -/** - * Removes node values that are not valid options for the given parameter. - * This can happen when there are multiple node parameters with the same name - * but different options and display conditions - * @param nodeType The node type description - * @param nodeParameterValues Current node parameter values - * @param updatedParameter The parameter that was updated. Will be used to determine which parameters to remove based on their display conditions and option values - */ -const removeMismatchedOptionValues = ( - nodeType: INodeTypeDescription, - nodeParameterValues: INodeParameters | null, - updatedParameter: { name: string; value: NodeParameterValue }, -) => { - nodeType.properties.forEach((prop) => { - const displayOptions = prop.displayOptions; - // Not processing parameters that are not set or don't have options - if (!nodeParameterValues?.hasOwnProperty(prop.name) || !displayOptions || !prop.options) { - return; - } - // Only process the parameters that depend on the updated parameter - const showCondition = displayOptions.show?.[updatedParameter.name]; - const hideCondition = displayOptions.hide?.[updatedParameter.name]; - if (showCondition === undefined && hideCondition === undefined) { - return; - } - - let hasValidOptions = true; - - // Every value should be a possible option - if (isINodePropertyCollectionList(prop.options) || isINodePropertiesList(prop.options)) { - hasValidOptions = Object.keys(nodeParameterValues).every( - (key) => (prop.options ?? []).find((option) => option.name === key) !== undefined, - ); - } else if (isINodePropertyOptionsList(prop.options)) { - hasValidOptions = !!prop.options.find( - (option) => option.value === nodeParameterValues[prop.name], - ); - } - - if (!hasValidOptions && displayParameter(nodeParameterValues, prop, node.value, nodeType)) { - unset(nodeParameterValues as object, prop.name); - } - }); -}; - const valueChanged = (parameterData: IUpdateInformation) => { let newValue: NodeParameterValue; @@ -439,37 +297,16 @@ const valueChanged = (parameterData: IUpdateInformation) => { nodeParameters = deepCopy(nodeParameters); if (parameterData.value && typeof parameterData.value === 'object') { - for (const parameterName of Object.keys(parameterData.value)) { - //@ts-ignore - newValue = parameterData.value[parameterName]; + for (const [parameterName, parameterValue] of Object.entries(parameterData.value)) { + newValue = parameterValue; - // Remove the 'parameters.' from the beginning to just have the - // actual parameter name - const parameterPath = parameterName.split('.').slice(1).join('.'); - - // Check if the path is supposed to change an array and if so get - // the needed data like path and index - const parameterPathArray = parameterPath.match(/(.*)\[(\d+)\]$/); - - // Apply the new value - //@ts-ignore - if (parameterData[parameterName] === undefined && parameterPathArray !== null) { - // Delete array item - const path = parameterPathArray[1]; - const index = parameterPathArray[2]; - const data = get(nodeParameters, path); - - if (Array.isArray(data)) { - data.splice(parseInt(index, 10), 1); - set(nodeParameters as object, path, data); - } - } else { - if (newValue === undefined) { - unset(nodeParameters as object, parameterPath); - } else { - set(nodeParameters as object, parameterPath, newValue); - } - } + const parameterPath = nodeSettingsParameters.updateParameterByPath( + parameterName, + newValue, + nodeParameters, + nodeType, + _node.typeVersion, + ); void externalHooks.run('nodeSettings.valueChanged', { parameterPath, @@ -492,8 +329,8 @@ const valueChanged = (parameterData: IUpdateInformation) => { ); for (const key of Object.keys(nodeParameters as object)) { - if (nodeParameters && nodeParameters[key] !== null && nodeParameters[key] !== undefined) { - setValue(`parameters.${key}`, nodeParameters[key] as string); + if (nodeParameters?.[key] !== null && nodeParameters?.[key] !== undefined) { + nodeSettingsParameters.setValue(`parameters.${key}`, nodeParameters[key] as string); } } @@ -508,127 +345,9 @@ const valueChanged = (parameterData: IUpdateInformation) => { nodeHelpers.updateNodeParameterIssuesByName(_node.name); nodeHelpers.updateNodeCredentialIssuesByName(_node.name); } - } else if (parameterData.name.startsWith('parameters.')) { + } else if (nodeSettingsParameters.nameIsParameter(parameterData)) { // A node parameter changed - - const nodeType = nodeTypesStore.getNodeType(_node.type, _node.typeVersion); - if (!nodeType) { - return; - } - - // Get only the parameters which are different to the defaults - let nodeParameters = NodeHelpers.getNodeParameters( - nodeType.properties, - _node.parameters, - false, - false, - _node, - nodeType, - ); - - const oldNodeParameters = Object.assign({}, nodeParameters); - - // Copy the data because it is the data of vuex so make sure that - // we do not edit it directly - nodeParameters = deepCopy(nodeParameters); - - // Remove the 'parameters.' from the beginning to just have the - // actual parameter name - const parameterPath = parameterData.name.split('.').slice(1).join('.'); - - // Check if the path is supposed to change an array and if so get - // the needed data like path and index - const parameterPathArray = parameterPath.match(/(.*)\[(\d+)\]$/); - - // Apply the new value - if (parameterData.value === undefined && parameterPathArray !== null) { - // Delete array item - const path = parameterPathArray[1]; - const index = parameterPathArray[2]; - const data = get(nodeParameters, path); - - if (Array.isArray(data)) { - data.splice(parseInt(index, 10), 1); - set(nodeParameters as object, path, data); - } - } else { - if (newValue === undefined) { - unset(nodeParameters as object, parameterPath); - } else { - set(nodeParameters as object, parameterPath, newValue); - } - // If value is updated, remove parameter values that have invalid options - // so getNodeParameters checks don't fail - removeMismatchedOptionValues(nodeType, nodeParameters, { - name: parameterPath, - value: newValue, - }); - } - - // Get the parameters with the now new defaults according to the - // from the user actually defined parameters - nodeParameters = NodeHelpers.getNodeParameters( - nodeType.properties, - nodeParameters as INodeParameters, - true, - false, - _node, - nodeType, - ); - - if (isToolNode.value) { - const updatedDescription = NodeHelpers.getUpdatedToolDescription( - props.nodeType, - nodeParameters, - node.value?.parameters, - ); - - if (updatedDescription && nodeParameters) { - nodeParameters.toolDescription = updatedDescription; - } - } - - if (NodeHelpers.isDefaultNodeName(_node.name, nodeType, node.value?.parameters ?? {})) { - const newName = NodeHelpers.makeNodeName(nodeParameters ?? {}, nodeType); - // Account for unique-ified nodes with `` - if (!_node.name.startsWith(newName)) { - // We need a timeout here to support events reacting to the valueChange based on node names - setTimeout(async () => await canvasOperations.renameNode(_node.name, newName)); - } - } - - for (const key of Object.keys(nodeParameters as object)) { - if (nodeParameters && nodeParameters[key] !== null && nodeParameters[key] !== undefined) { - setValue(`parameters.${key}`, nodeParameters[key] as string); - } - } - - // Update the data in vuex - const updateInformation: IUpdateInformation = { - name: _node.name, - value: nodeParameters, - }; - - const connections = workflowsStore.allConnections; - - const updatedConnections = updateDynamicConnections(_node, connections, parameterData); - - if (updatedConnections) { - workflowsStore.setConnections(updatedConnections, true); - } - - workflowsStore.setNodeParameters(updateInformation); - - void externalHooks.run('nodeSettings.valueChanged', { - parameterPath, - newValue, - parameters: parameters.value, - oldNodeParameters, - }); - - nodeHelpers.updateNodeParameterIssuesByName(_node.name); - nodeHelpers.updateNodeCredentialIssuesByName(_node.name); - telemetry.trackNodeParametersValuesChange(nodeType.name, parameterData); + nodeSettingsParameters.updateNodeParameter(parameterData, newValue, _node, isToolNode.value); } else { // A property on the node itself changed diff --git a/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.test.ts b/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.test.ts new file mode 100644 index 00000000000..f0e5beb5896 --- /dev/null +++ b/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.test.ts @@ -0,0 +1,69 @@ +import { createTestingPinia } from '@pinia/testing'; +import { setActivePinia } from 'pinia'; +import { useNodeSettingsParameters } from './useNodeSettingsParameters'; + +describe('useNodeSettingsParameters', () => { + beforeEach(() => { + setActivePinia(createTestingPinia()); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + describe('nameIsParameter', () => { + it.each([ + ['', false], + ['parameters', false], + ['parameters.', true], + ['parameters.path.to.some', true], + ['', false], + ])('%s should be %s', (input, expected) => { + const { nameIsParameter } = useNodeSettingsParameters(); + const result = nameIsParameter({ name: input } as never); + expect(result).toBe(expected); + }); + + it('should reject path on other input', () => { + const { nameIsParameter } = useNodeSettingsParameters(); + const result = nameIsParameter({ + name: 'aName', + value: 'parameters.path.to.parameters', + } as never); + expect(result).toBe(false); + }); + }); + + describe('setValue', () => { + it('mutates nodeValues as expected', () => { + const nodeSettingsParameters = useNodeSettingsParameters(); + + expect(nodeSettingsParameters.nodeValues.value.color).toBe('#ff0000'); + expect(nodeSettingsParameters.nodeValues.value.parameters).toEqual({}); + + nodeSettingsParameters.setValue('color', '#ffffff'); + + expect(nodeSettingsParameters.nodeValues.value.color).toBe('#ffffff'); + expect(nodeSettingsParameters.nodeValues.value.parameters).toEqual({}); + + nodeSettingsParameters.setValue('parameters.key', 3); + + expect(nodeSettingsParameters.nodeValues.value.parameters).toEqual({ key: 3 }); + + nodeSettingsParameters.nodeValues.value = { parameters: { some: { nested: {} } } }; + nodeSettingsParameters.setValue('parameters.some.nested.key', true); + + expect(nodeSettingsParameters.nodeValues.value.parameters).toEqual({ + some: { nested: { key: true } }, + }); + + nodeSettingsParameters.setValue('parameters', null); + + expect(nodeSettingsParameters.nodeValues.value.parameters).toBe(undefined); + + nodeSettingsParameters.setValue('newProperty', 'newValue'); + + expect(nodeSettingsParameters.nodeValues.value.newProperty).toBe('newValue'); + }); + }); +}); diff --git a/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.ts b/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.ts new file mode 100644 index 00000000000..fd28fd8f193 --- /dev/null +++ b/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.ts @@ -0,0 +1,231 @@ +import type { IUpdateInformation } from '@/Interface'; +import get from 'lodash/get'; +import set from 'lodash/set'; +import { + type INode, + type INodeParameters, + type NodeParameterValue, + NodeHelpers, + deepCopy, +} from 'n8n-workflow'; +import { useTelemetry } from './useTelemetry'; +import { useWorkflowsStore } from '@/stores/workflows.store'; +import { useNodeHelpers } from './useNodeHelpers'; +import { useCanvasOperations } from './useCanvasOperations'; +import { useExternalHooks } from './useExternalHooks'; +import { ref } from 'vue'; +import { updateDynamicConnections, updateParameterByPath } from '@/utils/nodeSettingsUtils'; +import { useNodeTypesStore } from '@/stores/nodeTypes.store'; + +export function useNodeSettingsParameters() { + const workflowsStore = useWorkflowsStore(); + const nodeTypesStore = useNodeTypesStore(); + const telemetry = useTelemetry(); + const nodeHelpers = useNodeHelpers(); + const canvasOperations = useCanvasOperations(); + const externalHooks = useExternalHooks(); + + const nodeValues = ref({ + color: '#ff0000', + alwaysOutputData: false, + executeOnce: false, + notesInFlow: false, + onError: 'stopWorkflow', + retryOnFail: false, + maxTries: 3, + waitBetweenTries: 1000, + notes: '', + parameters: {}, + }); + + function setValue(name: string, value: NodeParameterValue) { + const nameParts = name.split('.'); + let lastNamePart: string | undefined = nameParts.pop(); + + let isArray = false; + if (lastNamePart !== undefined && lastNamePart.includes('[')) { + // It includes an index so we have to extract it + const lastNameParts = lastNamePart.match(/(.*)\[(\d+)\]$/); + if (lastNameParts) { + nameParts.push(lastNameParts[1]); + lastNamePart = lastNameParts[2]; + isArray = true; + } + } + + // Set the value so that everything updates correctly in the UI + if (nameParts.length === 0) { + // Data is on top level + if (value === null) { + // Property should be deleted + if (lastNamePart) { + const { [lastNamePart]: removedNodeValue, ...remainingNodeValues } = nodeValues.value; + nodeValues.value = remainingNodeValues; + } + } else { + // Value should be set + nodeValues.value = { + ...nodeValues.value, + [lastNamePart as string]: value, + }; + } + } else { + // Data is on lower level + if (value === null) { + // Property should be deleted + let tempValue = get(nodeValues.value, nameParts.join('.')) as + | INodeParameters + | INodeParameters[]; + + if (lastNamePart && !Array.isArray(tempValue)) { + const { [lastNamePart]: removedNodeValue, ...remainingNodeValues } = tempValue; + tempValue = remainingNodeValues; + } + + if (isArray && Array.isArray(tempValue) && tempValue.length === 0) { + // If a value from an array got delete and no values are left + // delete also the parent + lastNamePart = nameParts.pop(); + tempValue = get(nodeValues.value, nameParts.join('.')) as INodeParameters; + if (lastNamePart) { + const { [lastNamePart]: removedArrayNodeValue, ...remainingArrayNodeValues } = + tempValue; + tempValue = remainingArrayNodeValues; + } + } + } else { + // Value should be set + if (typeof value === 'object') { + set( + get(nodeValues.value, nameParts.join('.')) as Record, + lastNamePart as string, + deepCopy(value), + ); + } else { + set( + get(nodeValues.value, nameParts.join('.')) as Record, + lastNamePart as string, + value, + ); + } + } + } + + nodeValues.value = { ...nodeValues.value }; + } + + function nameIsParameter( + parameterData: IUpdateInformation, + ): parameterData is IUpdateInformation & { name: `parameters.${string}` } { + return parameterData.name.startsWith('parameters.'); + } + + function updateNodeParameter( + parameterData: IUpdateInformation & { name: `parameters.${string}` }, + newValue: NodeParameterValue, + node: INode, + isToolNode: boolean, + ) { + const nodeTypeDescription = nodeTypesStore.getNodeType(node.type, node.typeVersion); + if (!nodeTypeDescription) { + return; + } + + // Get only the parameters which are different to the defaults + let nodeParameters = NodeHelpers.getNodeParameters( + nodeTypeDescription.properties, + node.parameters, + false, + false, + node, + nodeTypeDescription, + ); + + const oldNodeParameters = Object.assign({}, nodeParameters); + + // Copy the data because it is the data of vuex so make sure that + // we do not edit it directly + nodeParameters = deepCopy(nodeParameters); + + const parameterPath = updateParameterByPath( + parameterData.name, + newValue, + nodeParameters, + nodeTypeDescription, + node.typeVersion, + ); + + // Get the parameters with the now new defaults according to the + // from the user actually defined parameters + nodeParameters = NodeHelpers.getNodeParameters( + nodeTypeDescription.properties, + nodeParameters as INodeParameters, + true, + false, + node, + nodeTypeDescription, + ); + + if (isToolNode) { + const updatedDescription = NodeHelpers.getUpdatedToolDescription( + nodeTypeDescription, + nodeParameters, + node.parameters, + ); + + if (updatedDescription && nodeParameters) { + nodeParameters.toolDescription = updatedDescription; + } + } + + if (NodeHelpers.isDefaultNodeName(node.name, nodeTypeDescription, node.parameters ?? {})) { + const newName = NodeHelpers.makeNodeName(nodeParameters ?? {}, nodeTypeDescription); + // Account for unique-ified nodes with `` + if (!node.name.startsWith(newName)) { + // We need a timeout here to support events reacting to the valueChange based on node names + setTimeout(async () => await canvasOperations.renameNode(node.name, newName)); + } + } + + for (const [key, value] of Object.entries(nodeParameters as object)) { + if (value !== null && value !== undefined) { + setValue(`parameters.${key}`, value as string); + } + } + + // Update the data in vuex + const updateInformation: IUpdateInformation = { + name: node.name, + value: nodeParameters, + }; + + const connections = workflowsStore.allConnections; + + const updatedConnections = updateDynamicConnections(node, connections, parameterData); + + if (updatedConnections) { + workflowsStore.setConnections(updatedConnections, true); + } + + workflowsStore.setNodeParameters(updateInformation); + + void externalHooks.run('nodeSettings.valueChanged', { + parameterPath, + newValue, + parameters: nodeTypeDescription.properties, + oldNodeParameters, + }); + + nodeHelpers.updateNodeParameterIssuesByName(node.name); + nodeHelpers.updateNodeCredentialIssuesByName(node.name); + telemetry.trackNodeParametersValuesChange(nodeTypeDescription.name, parameterData); + } + + return { + nodeValues, + setValue, + updateParameterByPath, + updateNodeParameter, + nameIsParameter, + }; +} diff --git a/packages/frontend/editor-ui/src/stores/nodeTypes.store.ts b/packages/frontend/editor-ui/src/stores/nodeTypes.store.ts index 0617a29f448..3730b21be0d 100644 --- a/packages/frontend/editor-ui/src/stores/nodeTypes.store.ts +++ b/packages/frontend/editor-ui/src/stores/nodeTypes.store.ts @@ -155,7 +155,7 @@ export const useNodeTypesStore = defineStore(STORES.NODE_TYPES, () => { return outputTypes.includes(NodeConnectionTypes.AiTool); } else { - return nodeType?.outputs.includes(NodeConnectionTypes.AiTool); + return nodeType?.outputs.includes(NodeConnectionTypes.AiTool) ?? false; } }; }); diff --git a/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.test.ts b/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.test.ts index 09bf72a082d..c4bb90d53cc 100644 --- a/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.test.ts +++ b/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.test.ts @@ -1,7 +1,12 @@ import { describe, it, expect, afterAll } from 'vitest'; import { mock } from 'vitest-mock-extended'; -import type { IConnections, NodeParameterValueType, IDataObject } from 'n8n-workflow'; -import { updateDynamicConnections } from './nodeSettingsUtils'; +import type { + IConnections, + NodeParameterValueType, + IDataObject, + INodeTypeDescription, +} from 'n8n-workflow'; +import { updateDynamicConnections, updateParameterByPath } from './nodeSettingsUtils'; import { SWITCH_NODE_TYPE } from '@/constants'; import type { INodeUi, IUpdateInformation } from '@/Interface'; @@ -197,3 +202,83 @@ describe('updateDynamicConnections', () => { expect(result).toBeNull(); }); }); + +describe('updateParameterByPath', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it('should update a parameter value by path', () => { + const nodeParameters = { + rules: { values: [{ id: 'rule1' }, { id: 'rule2' }] }, + }; + + const nodeType = mock({ + properties: [], + }); + + const parameterPath = 'parameters.rules.values[1].id'; + const newValue = 'updatedRule2'; + + const updatedPath = updateParameterByPath(parameterPath, newValue, nodeParameters, nodeType, 1); + + expect(updatedPath).toBe('rules.values[1].id'); + expect(nodeParameters.rules.values[1].id).toBe('updatedRule2'); + }); + + it('should remove a parameter value if newValue is undefined', () => { + const nodeParameters = { + rules: { values: [{ id: 'rule1' }, { id: 'rule2' }] }, + }; + + const nodeType = mock({ + properties: [], + }); + + const parameterPath = 'parameters.rules.values[1]'; + const newValue = undefined; + + const updatedPath = updateParameterByPath(parameterPath, newValue, nodeParameters, nodeType, 1); + + expect(updatedPath).toBe('rules.values[1]'); + expect(nodeParameters.rules.values).toHaveLength(1); + expect(nodeParameters.rules.values[0].id).toBe('rule1'); + }); + + it('should add a new parameter value if path does not exist', () => { + const nodeParameters = { + rules: { values: [{ id: 'rule1' }] }, + }; + + const nodeType = mock({ + properties: [], + }); + + const parameterPath = 'parameters.rules.values[1].id'; + const newValue = 'newRule'; + + const updatedPath = updateParameterByPath(parameterPath, newValue, nodeParameters, nodeType, 1); + + expect(updatedPath).toBe('rules.values[1].id'); + expect(nodeParameters.rules.values[1].id).toBe('newRule'); + }); + + it('should handle array deletion when newValue is undefined and path is an array', () => { + const nodeParameters = { + arrayParam: ['value1', 'value2', 'value3'], + }; + + const nodeType = mock({ + properties: [], + }); + + const parameterPath = 'parameters.arrayParam[1]'; + const newValue = undefined; + + const updatedPath = updateParameterByPath(parameterPath, newValue, nodeParameters, nodeType, 1); + + expect(updatedPath).toBe('arrayParam[1]'); + expect(nodeParameters.arrayParam).toHaveLength(2); + expect(nodeParameters.arrayParam).toEqual(['value1', 'value3']); + }); +}); diff --git a/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.ts b/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.ts index 986c8278a3f..29d857043e3 100644 --- a/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.ts +++ b/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.ts @@ -1,13 +1,24 @@ -import type { - IConnection, - IConnections, - IDataObject, - NodeInputConnections, - NodeParameterValueType, +import { + type IConnection, + type IConnections, + type IDataObject, + type NodeInputConnections, + type NodeParameterValueType, + type INodeTypeDescription, + type INode, + type INodeParameters, + type NodeParameterValue, + isINodePropertyCollectionList, + isINodePropertiesList, + isINodePropertyOptionsList, + displayParameter, } from 'n8n-workflow'; import type { INodeUi, IUpdateInformation } from '@/Interface'; import { SWITCH_NODE_TYPE } from '@/constants'; import isEqual from 'lodash/isEqual'; +import get from 'lodash/get'; +import set from 'lodash/set'; +import unset from 'lodash/unset'; import { captureException } from '@sentry/vue'; @@ -131,3 +142,96 @@ export function updateDynamicConnections( return null; } + +/** + * Removes node values that are not valid options for the given parameter. + * This can happen when there are multiple node parameters with the same name + * but different options and display conditions + * @param nodeType The node type description + * @param nodeParameterValues Current node parameter values + * @param updatedParameter The parameter that was updated. Will be used to determine which parameters to remove based on their display conditions and option values + */ +export function removeMismatchedOptionValues( + nodeType: INodeTypeDescription, + nodeTypeVersion: INode['typeVersion'], + nodeParameterValues: INodeParameters | null, + updatedParameter: { name: string; value: NodeParameterValue }, +) { + nodeType.properties.forEach((prop) => { + const displayOptions = prop.displayOptions; + // Not processing parameters that are not set or don't have options + if (!nodeParameterValues?.hasOwnProperty(prop.name) || !displayOptions || !prop.options) { + return; + } + // Only process the parameters that depend on the updated parameter + const showCondition = displayOptions.show?.[updatedParameter.name]; + const hideCondition = displayOptions.hide?.[updatedParameter.name]; + if (showCondition === undefined && hideCondition === undefined) { + return; + } + + let hasValidOptions = true; + + // Every value should be a possible option + if (isINodePropertyCollectionList(prop.options) || isINodePropertiesList(prop.options)) { + hasValidOptions = Object.keys(nodeParameterValues).every( + (key) => (prop.options ?? []).find((option) => option.name === key) !== undefined, + ); + } else if (isINodePropertyOptionsList(prop.options)) { + hasValidOptions = !!prop.options.find( + (option) => option.value === nodeParameterValues[prop.name], + ); + } + + if ( + !hasValidOptions && + displayParameter(nodeParameterValues, prop, { typeVersion: nodeTypeVersion }, nodeType) + ) { + unset(nodeParameterValues as object, prop.name); + } + }); +} + +export function updateParameterByPath( + parameterName: string, + newValue: NodeParameterValue, + nodeParameters: INodeParameters | null, + nodeType: INodeTypeDescription, + nodeTypeVersion: INode['typeVersion'], +) { + // Remove the 'parameters.' from the beginning to just have the + // actual parameter name + const parameterPath = parameterName.split('.').slice(1).join('.'); + + // Check if the path is supposed to change an array and if so get + // the needed data like path and index + const parameterPathArray = parameterPath.match(/(.*)\[(\d+)\]$/); + + // Apply the new value + if (newValue === undefined && parameterPathArray !== null) { + // Delete array item + const path = parameterPathArray[1]; + const index = parameterPathArray[2]; + const data = get(nodeParameters, path); + + if (Array.isArray(data)) { + data.splice(parseInt(index, 10), 1); + set(nodeParameters as object, path, data); + } + } else { + if (newValue === undefined) { + unset(nodeParameters as object, parameterPath); + } else { + set(nodeParameters as object, parameterPath, newValue); + } + + // If value is updated, remove parameter values that have invalid options + // so getNodeParameters checks don't fail + removeMismatchedOptionValues(nodeType, nodeTypeVersion, nodeParameters, { + name: parameterPath, + value: newValue, + }); + } + + return parameterPath; +} diff --git a/packages/frontend/editor-ui/src/utils/parameterUtils.ts b/packages/frontend/editor-ui/src/utils/parameterUtils.ts new file mode 100644 index 00000000000..e69de29bb2d