fix(editor): Actually enforce the version and don't break for old values in local storage (#13025)

Co-authored-by: Csaba Tuncsik <csaba@n8n.io>

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();
 

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;
 	});
 

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<typeof useWorkflowsStore>;
 	let uiStore: ReturnType<typeof useUIStore>;
+	let settingsStore: MockedStore<typeof useSettingsStore>;
 
 	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() {

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<IWorkflowDb>(createEmptyWorkflow());
 	const usedCredentials = ref<Record<string, IUsedCredential>>({});
 
@@ -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) {