From 884a7e23f84258756d8dcdd2dfe933bdedf61adc Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Tue, 4 Feb 2025 12:16:00 +0100 Subject: [PATCH] fix(editor): Actually enforce the version and don't break for old values in local storage (#13025) Co-authored-by: Csaba Tuncsik --- .../src/stores/settings.store.test.ts | 7 +++ .../editor-ui/src/stores/settings.store.ts | 6 ++ .../src/stores/workflows.store.test.ts | 61 ++++++++++++++++++- .../editor-ui/src/stores/workflows.store.ts | 5 +- 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/packages/editor-ui/src/stores/settings.store.test.ts b/packages/editor-ui/src/stores/settings.store.test.ts index 27db7c72cc2..4d7d00c76ca 100644 --- a/packages/editor-ui/src/stores/settings.store.test.ts +++ b/packages/editor-ui/src/stores/settings.store.test.ts @@ -142,6 +142,13 @@ describe('settings.store', () => { userVersion: 1, result: 2, }, + { + name: 'handle values that used to be allowed in local storage', + default: 1 as const, + enforce: false, + userVersion: 0, + result: 1, + }, ])('%name', async ({ default: defaultVersion, userVersion, enforce, result }) => { const settingsStore = useSettingsStore(); diff --git a/packages/editor-ui/src/stores/settings.store.ts b/packages/editor-ui/src/stores/settings.store.ts index 6f0eab341d2..55165358b9f 100644 --- a/packages/editor-ui/src/stores/settings.store.ts +++ b/packages/editor-ui/src/stores/settings.store.ts @@ -112,6 +112,12 @@ export const useSettingsStore = defineStore(STORES.SETTINGS, () => { ? defaultVersion : userVersion; + // For backwards compatibility, e.g. if the user has 0 in their local + // storage, which used to be allowed, but not anymore. + if (![1, 2].includes(version)) { + return 1; + } + return version; }); diff --git a/packages/editor-ui/src/stores/workflows.store.test.ts b/packages/editor-ui/src/stores/workflows.store.test.ts index a612be7441b..d60cd8f127a 100644 --- a/packages/editor-ui/src/stores/workflows.store.test.ts +++ b/packages/editor-ui/src/stores/workflows.store.test.ts @@ -16,10 +16,15 @@ import type { IPinData, ExecutionSummary, IConnection, INodeExecutionData } from import { stringSizeInBytes } from '@/utils/typesUtils'; import { dataPinningEventBus } from '@/event-bus'; import { useUIStore } from '@/stores/ui.store'; -import type { PushPayload } from '@n8n/api-types'; +import type { PushPayload, FrontendSettings } from '@n8n/api-types'; import { flushPromises } from '@vue/test-utils'; import { useNDVStore } from '@/stores/ndv.store'; import { mock } from 'vitest-mock-extended'; +import { mockedStore, type MockedStore } from '@/__tests__/utils'; +import * as apiUtils from '@/utils/apiUtils'; +import { useSettingsStore } from '@/stores/settings.store'; +import { useLocalStorage } from '@vueuse/core'; +import { ref } from 'vue'; vi.mock('@/stores/ndv.store', () => ({ useNDVStore: vi.fn(() => ({ @@ -45,14 +50,24 @@ vi.mock('@/composables/useTelemetry', () => ({ useTelemetry: () => ({ track }), })); +vi.mock('@vueuse/core', async (importOriginal) => { + const actual = await importOriginal<{}>(); + return { + ...actual, + useLocalStorage: vi.fn().mockReturnValue({ value: undefined }), + }; +}); + describe('useWorkflowsStore', () => { let workflowsStore: ReturnType; let uiStore: ReturnType; + let settingsStore: MockedStore; beforeEach(() => { setActivePinia(createPinia()); workflowsStore = useWorkflowsStore(); uiStore = useUIStore(); + settingsStore = mockedStore(useSettingsStore); track.mockReset(); }); @@ -662,6 +677,50 @@ describe('useWorkflowsStore', () => { expect(workflowsStore.nodeMetadata[nodeName].parametersLastUpdatedAt).toBe(undefined); }); }); + + test.each([ + // enforce true cases - the version is always the defaultVersion + [-1, 1, true, 1], // enforce true, use default (1) + [0, 1, true, 1], // enforce true, use default (1) + [1, 1, true, 1], // enforce true, use default (1) + [2, 1, true, 1], // enforce true, use default (1) + [-1, 2, true, 2], // enforce true, use default (2) + [0, 2, true, 2], // enforce true, use default (2) + [1, 2, true, 2], // enforce true, use default (2) + [2, 2, true, 2], // enforce true, use default (2) + + // enforce false cases - check userVersion behavior + [-1, 1, false, 1], // userVersion -1, use default (1) + [0, 1, false, 1], // userVersion 0, invalid, use default (1) + [1, 1, false, 1], // userVersion 1, valid, use userVersion (1) + [2, 1, false, 2], // userVersion 2, valid, use userVersion (2) + [-1, 2, false, 2], // userVersion -1, use default (2) + [0, 2, false, 1], // userVersion 0, invalid, use default (2) + [1, 2, false, 1], // userVersion 1, valid, use userVersion (1) + [2, 2, false, 2], // userVersion 2, valid, use userVersion (2) + ] as Array<[number, 1 | 2, boolean, number]>)( + 'when { userVersion:%s, defaultVersion:%s, enforced:%s } run workflow should use partial execution version %s', + async (userVersion, defaultVersion, enforce, expectedVersion) => { + vi.mocked(useLocalStorage).mockReturnValueOnce(ref(userVersion)); + settingsStore.settings = { + partialExecution: { version: defaultVersion, enforce }, + } as FrontendSettings; + + const workflowData = { id: '1', nodes: [], connections: {} }; + const makeRestApiRequestSpy = vi + .spyOn(apiUtils, 'makeRestApiRequest') + .mockImplementation(async () => ({})); + + await workflowsStore.runWorkflow({ workflowData }); + + expect(makeRestApiRequestSpy).toHaveBeenCalledWith( + { baseUrl: '/rest', pushRef: expect.any(String) }, + 'POST', + `/workflows/1/run?partialExecutionVersion=${expectedVersion}`, + { workflowData }, + ); + }, + ); }); function getMockEditFieldsNode() { diff --git a/packages/editor-ui/src/stores/workflows.store.ts b/packages/editor-ui/src/stores/workflows.store.ts index e09de1dc3ba..6e0f2eecd11 100644 --- a/packages/editor-ui/src/stores/workflows.store.ts +++ b/packages/editor-ui/src/stores/workflows.store.ts @@ -124,8 +124,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { const nodeHelpers = useNodeHelpers(); const usersStore = useUsersStore(); - const version = settingsStore.partialExecutionVersion; - + const version = computed(() => settingsStore.partialExecutionVersion); const workflow = ref(createEmptyWorkflow()); const usedCredentials = ref>({}); @@ -1470,7 +1469,7 @@ export const useWorkflowsStore = defineStore(STORES.WORKFLOWS, () => { return await makeRestApiRequest( rootStore.restApiContext, 'POST', - `/workflows/${startRunData.workflowData.id}/run?partialExecutionVersion=${version}`, + `/workflows/${startRunData.workflowData.id}/run?partialExecutionVersion=${version.value}`, startRunData as unknown as IDataObject, ); } catch (error) {