From ec69bcc3fd841781de433cd31709afc63707f539 Mon Sep 17 00:00:00 2001 From: Charlie Kolb Date: Tue, 15 Jul 2025 11:14:07 +0200 Subject: [PATCH] chore: Refactor nodeValues back to NodeSettings (no-changelog) (#17300) --- .../editor-ui/src/components/FocusPanel.vue | 3 +- .../editor-ui/src/components/NodeSettings.vue | 28 +++++- .../useNodeSettingsParameters.test.ts | 37 -------- .../composables/useNodeSettingsParameters.ts | 93 +------------------ .../src/utils/nodeSettingsUtils.test.ts | 47 ++++++++++ .../editor-ui/src/utils/nodeSettingsUtils.ts | 79 ++++++++++++++++ 6 files changed, 157 insertions(+), 130 deletions(-) diff --git a/packages/frontend/editor-ui/src/components/FocusPanel.vue b/packages/frontend/editor-ui/src/components/FocusPanel.vue index 709fc1bb5b..d27d01124c 100644 --- a/packages/frontend/editor-ui/src/components/FocusPanel.vue +++ b/packages/frontend/editor-ui/src/components/FocusPanel.vue @@ -2,7 +2,7 @@ import { useFocusPanelStore } from '@/stores/focusPanel.store'; import { useNodeTypesStore } from '@/stores/nodeTypes.store'; import { N8nText, N8nInput, N8nResizeWrapper } from '@n8n/design-system'; -import { computed, nextTick, ref, watch } from 'vue'; +import { computed, nextTick, ref, watch, toRef } from 'vue'; import { useI18n } from '@n8n/i18n'; import { formatAsExpression, @@ -184,6 +184,7 @@ function valueChanged(value: string) { } nodeSettingsParameters.updateNodeParameter( + toRef(resolvedParameter.value.node.parameters), { value, name: resolvedParameter.value.parameterPath as `parameters.${string}` }, value, resolvedParameter.value.node, diff --git a/packages/frontend/editor-ui/src/components/NodeSettings.vue b/packages/frontend/editor-ui/src/components/NodeSettings.vue index 0f387d85b4..b971d3f510 100644 --- a/packages/frontend/editor-ui/src/components/NodeSettings.vue +++ b/packages/frontend/editor-ui/src/components/NodeSettings.vue @@ -94,6 +94,19 @@ const emit = defineEmits<{ const slots = defineSlots<{ actions?: {} }>(); +const nodeValues = ref({ + color: '#ff0000', + alwaysOutputData: false, + executeOnce: false, + notesInFlow: false, + onError: 'stopWorkflow', + retryOnFail: false, + maxTries: 3, + waitBetweenTries: 1000, + notes: '', + parameters: {}, +}); + const nodeTypesStore = useNodeTypesStore(); const ndvStore = useNDVStore(); const workflowsStore = useWorkflowsStore(); @@ -106,7 +119,6 @@ const nodeHelpers = useNodeHelpers(); const externalHooks = useExternalHooks(); const i18n = useI18n(); const nodeSettingsParameters = useNodeSettingsParameters(); -const nodeValues = nodeSettingsParameters.nodeValues; const nodeParameterWrapper = useTemplateRef('nodeParameterWrapper'); const shouldShowStaticScrollbar = ref(false); @@ -355,7 +367,11 @@ const valueChanged = (parameterData: IUpdateInformation) => { for (const key of Object.keys(nodeParameters as object)) { if (nodeParameters?.[key] !== null && nodeParameters?.[key] !== undefined) { - nodeSettingsParameters.setValue(`parameters.${key}`, nodeParameters[key] as string); + nodeSettingsParameters.setValue( + nodeValues, + `parameters.${key}`, + nodeParameters[key] as string, + ); } } @@ -372,7 +388,13 @@ const valueChanged = (parameterData: IUpdateInformation) => { } } else if (nameIsParameter(parameterData)) { // A node parameter changed - nodeSettingsParameters.updateNodeParameter(parameterData, newValue, _node, isToolNode.value); + nodeSettingsParameters.updateNodeParameter( + nodeValues, + 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 index 83e5d14e73..9d15616d3d 100644 --- a/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.test.ts +++ b/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.test.ts @@ -18,43 +18,6 @@ describe('useNodeSettingsParameters', () => { setActivePinia(createTestingPinia()); }); - describe('setValue', () => { - afterEach(() => { - vi.clearAllMocks(); - }); - - 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'); - }); - }); - describe('handleFocus', () => { let ndvStore: MockedStore; let focusPanelStore: MockedStore; diff --git a/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.ts b/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.ts index 98d310c7b6..f50e13f028 100644 --- a/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.ts +++ b/packages/frontend/editor-ui/src/composables/useNodeSettingsParameters.ts @@ -1,6 +1,6 @@ import get from 'lodash/get'; import set from 'lodash/set'; -import { ref } from 'vue'; +import type { Ref } from 'vue'; import { type INode, type INodeParameters, @@ -17,6 +17,7 @@ import { useExternalHooks } from './useExternalHooks'; import type { INodeUi, IUpdateInformation } from '@/Interface'; import { mustHideDuringCustomApiCall, + setValue, updateDynamicConnections, updateParameterByPath, } from '@/utils/nodeSettingsUtils'; @@ -25,7 +26,6 @@ import { useFocusPanelStore } from '@/stores/focusPanel.store'; import { useNDVStore } from '@/stores/ndv.store'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { KEEP_AUTH_IN_NDV_FOR_NODES } from '@/constants'; -import { omitKey } from '@/utils/objectUtils'; import { getMainAuthField, getNodeAuthFields, @@ -41,92 +41,8 @@ export function useNodeSettingsParameters() { 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?.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) { - nodeValues.value = omitKey(nodeValues.value, lastNamePart); - } - } 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)) { - tempValue = omitKey(tempValue, lastNamePart); - } - - 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) { - tempValue = omitKey(tempValue, lastNamePart); - } - } - } 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 updateNodeParameter( + nodeValues: Ref, parameterData: IUpdateInformation & { name: `parameters.${string}` }, newValue: NodeParameterValue, node: INode, @@ -195,7 +111,7 @@ export function useNodeSettingsParameters() { for (const [key, value] of Object.entries(nodeParameters as object)) { if (value !== null && value !== undefined) { - setValue(`parameters.${key}`, value as string); + setValue(nodeValues, `parameters.${key}`, value as string); } } @@ -353,7 +269,6 @@ export function useNodeSettingsParameters() { } return { - nodeValues, setValue, shouldDisplayNodeParameter, updateParameterByPath, diff --git a/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.test.ts b/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.test.ts index a0e0644c8b..84a5ea37a0 100644 --- a/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.test.ts +++ b/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.test.ts @@ -6,6 +6,7 @@ import type { IDataObject, INodeTypeDescription, INodePropertyOptions, + INodeParameters, INodeProperties, } from 'n8n-workflow'; import { @@ -14,10 +15,12 @@ import { nameIsParameter, formatAsExpression, parseFromExpression, + setValue, shouldSkipParamValidation, } from './nodeSettingsUtils'; import { CUSTOM_API_CALL_KEY, SWITCH_NODE_TYPE } from '@/constants'; import type { INodeUi, IUpdateInformation } from '@/Interface'; +import { type Ref, ref } from 'vue'; describe('updateDynamicConnections', () => { afterAll(() => { @@ -553,3 +556,47 @@ describe('shouldSkipParamValidation', () => { }); }); }); + +describe('setValue', () => { + let nodeValues: Ref; + beforeEach(() => { + nodeValues = ref({ + color: '#ff0000', + alwaysOutputData: false, + executeOnce: false, + notesInFlow: false, + onError: 'stopWorkflow', + retryOnFail: false, + maxTries: 3, + waitBetweenTries: 1000, + notes: '', + parameters: {}, + }); + }); + + it('mutates nodeValues as expected', () => { + setValue(nodeValues, 'color', '#ffffff'); + + expect(nodeValues.value.color).toBe('#ffffff'); + expect(nodeValues.value.parameters).toEqual({}); + + setValue(nodeValues, 'parameters.key', 3); + + expect(nodeValues.value.parameters).toEqual({ key: 3 }); + + nodeValues.value = { parameters: { some: { nested: {} } } }; + setValue(nodeValues, 'parameters.some.nested.key', true); + + expect(nodeValues.value.parameters).toEqual({ + some: { nested: { key: true } }, + }); + + setValue(nodeValues, 'parameters', null); + + expect(nodeValues.value.parameters).toBe(undefined); + + setValue(nodeValues, 'newProperty', 'newValue'); + + expect(nodeValues.value.newProperty).toBe('newValue'); + }); +}); diff --git a/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.ts b/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.ts index 4e5c9ccea6..5dbac008b9 100644 --- a/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.ts +++ b/packages/frontend/editor-ui/src/utils/nodeSettingsUtils.ts @@ -17,6 +17,7 @@ import { isINodePropertyOptionsList, displayParameter, isResourceLocatorValue, + deepCopy, } from 'n8n-workflow'; import type { INodeUi, IUpdateInformation } from '@/Interface'; import { CUSTOM_API_CALL_KEY, SWITCH_NODE_TYPE } from '@/constants'; @@ -27,6 +28,84 @@ import unset from 'lodash/unset'; import { captureException } from '@sentry/vue'; import { isPresent } from './typesUtils'; +import type { Ref } from 'vue'; +import { omitKey } from './objectUtils'; + +export function setValue( + nodeValues: Ref, + name: string, + value: NodeParameterValue, +) { + const nameParts = name.split('.'); + let lastNamePart: string | undefined = nameParts.pop(); + + let isArray = false; + if (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) { + nodeValues.value = omitKey(nodeValues.value, lastNamePart); + } + } 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)) { + tempValue = omitKey(tempValue, lastNamePart); + } + + 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) { + tempValue = omitKey(tempValue, lastNamePart); + } + } + } 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 }; +} export function updateDynamicConnections( node: INodeUi,