refactor(core): Include execution progress in save settings (no-changelog) (#7769)

packages/cli/src/WorkflowExecuteAdditionalData.ts

@@ -358,18 +358,9 @@ export function hookFunctionsPreExecute(parentProcessMode?: string): IWorkflowEx
 				data: ITaskData,
 				executionData: IRunExecutionData,
 			): Promise<void> {
-				const saveExecutionProgress = config.getEnv('executions.saveExecutionProgress');
+				const saveSettings = toSaveSettings(this.workflowData.settings);
-				const workflowSettings = this.workflowData.settings;
+
-				if (workflowSettings !== undefined) {
+				if (!saveSettings.progress) return;
-					if (workflowSettings.saveExecutionProgress === false) {
-						return;
-					}
-					if (workflowSettings.saveExecutionProgress !== true && !saveExecutionProgress) {
-						return;
-					}
-				} else if (!saveExecutionProgress) {
-					return;
-				}
 
 				try {
 					logger.debug(

packages/cli/src/executionLifecycleHooks/toSaveSettings.ts

@@ -2,14 +2,19 @@ import config from '@/config';
 import type { IWorkflowSettings } from 'n8n-workflow';
 
 /**
- * Return whether a workflow execution is configured to be saved or not,
+ * Return whether a workflow execution is configured to be saved or not:
- * for error executions, success executions, and manual executions.
+ *
+ * - `error`: Whether to save failed executions in production.
+ * - `success`: Whether to successful executions in production.
+ * - `manual`: Whether to save successful or failed manual executions.
+ * - `progress`: Whether to save execution progress, i.e. after each node's execution.
  */
 export function toSaveSettings(workflowSettings: IWorkflowSettings = {}) {
 	const DEFAULTS = {
 		ERROR: config.getEnv('executions.saveDataOnError'),
 		SUCCESS: config.getEnv('executions.saveDataOnSuccess'),
 		MANUAL: config.getEnv('executions.saveDataManualExecutions'),
+		PROGRESS: config.getEnv('executions.saveExecutionProgress'),
 	};
 
 	return {
@@ -19,6 +24,13 @@ export function toSaveSettings(workflowSettings: IWorkflowSettings = {}) {
 		success: workflowSettings.saveDataSuccessExecution
 			? workflowSettings.saveDataSuccessExecution !== 'none'
 			: DEFAULTS.SUCCESS !== 'none',
-		manual: workflowSettings?.saveManualExecutions ?? DEFAULTS.MANUAL,
+		manual:
+			workflowSettings === undefined || workflowSettings.saveManualExecutions === 'DEFAULT'
+				? DEFAULTS.MANUAL
+				: workflowSettings.saveManualExecutions ?? DEFAULTS.MANUAL,
+		progress:
+			workflowSettings === undefined || workflowSettings.saveExecutionProgress === 'DEFAULT'
+				? DEFAULTS.PROGRESS
+				: workflowSettings.saveExecutionProgress ?? DEFAULTS.PROGRESS,
 	};
 }

packages/cli/test/unit/execution-lifecyle/toSaveSettings.test.ts

@@ -0,0 +1,154 @@
+import config from '@/config';
+import { toSaveSettings } from '@/executionLifecycleHooks/toSaveSettings';
+
+afterEach(() => {
+	config.load(config.default);
+});
+
+describe('failed production executions', () => {
+	it('should favor workflow settings over defaults', () => {
+		config.set('executions.saveDataOnError', 'none');
+
+		const saveSettings = toSaveSettings({ saveDataErrorExecution: 'all' });
+
+		expect(saveSettings.error).toBe(true);
+
+		config.set('executions.saveDataOnError', 'all');
+
+		const _saveSettings = toSaveSettings({ saveDataErrorExecution: 'none' });
+
+		expect(_saveSettings.error).toBe(false);
+	});
+
+	it('should fall back to default if no workflow setting', () => {
+		config.set('executions.saveDataOnError', 'all');
+
+		const saveSettings = toSaveSettings();
+
+		expect(saveSettings.error).toBe(true);
+
+		config.set('executions.saveDataOnError', 'none');
+
+		const _saveSettings = toSaveSettings();
+
+		expect(_saveSettings.error).toBe(false);
+	});
+});
+
+describe('sucessful production executions', () => {
+	it('should favor workflow settings over defaults', () => {
+		config.set('executions.saveDataOnSuccess', 'none');
+
+		const saveSettings = toSaveSettings({ saveDataSuccessExecution: 'all' });
+
+		expect(saveSettings.success).toBe(true);
+
+		config.set('executions.saveDataOnSuccess', 'all');
+
+		const _saveSettings = toSaveSettings({ saveDataSuccessExecution: 'none' });
+
+		expect(_saveSettings.success).toBe(false);
+	});
+
+	it('should fall back to default if no workflow setting', () => {
+		config.set('executions.saveDataOnSuccess', 'all');
+
+		const saveSettings = toSaveSettings();
+
+		expect(saveSettings.success).toBe(true);
+
+		config.set('executions.saveDataOnSuccess', 'none');
+
+		const _saveSettings = toSaveSettings();
+
+		expect(_saveSettings.success).toBe(false);
+	});
+});
+
+describe('manual executions', () => {
+	it('should favor workflow setting over default', () => {
+		config.set('executions.saveDataManualExecutions', false);
+
+		const saveSettings = toSaveSettings({ saveManualExecutions: true });
+
+		expect(saveSettings.manual).toBe(true);
+
+		config.set('executions.saveDataManualExecutions', true);
+
+		const _saveSettings = toSaveSettings({ saveManualExecutions: false });
+
+		expect(_saveSettings.manual).toBe(false);
+	});
+
+	it('should favor fall back to default if workflow setting is explicit default', () => {
+		config.set('executions.saveDataManualExecutions', true);
+
+		const saveSettings = toSaveSettings({ saveManualExecutions: 'DEFAULT' });
+
+		expect(saveSettings.manual).toBe(true);
+
+		config.set('executions.saveDataManualExecutions', false);
+
+		const _saveSettings = toSaveSettings({ saveManualExecutions: 'DEFAULT' });
+
+		expect(_saveSettings.manual).toBe(false);
+	});
+
+	it('should fall back to default if no workflow setting', () => {
+		config.set('executions.saveDataManualExecutions', true);
+
+		const saveSettings = toSaveSettings();
+
+		expect(saveSettings.manual).toBe(true);
+
+		config.set('executions.saveDataManualExecutions', false);
+
+		const _saveSettings = toSaveSettings();
+
+		expect(_saveSettings.manual).toBe(false);
+	});
+});
+
+describe('execution progress', () => {
+	it('should favor workflow setting over default', () => {
+		config.set('executions.saveExecutionProgress', false);
+
+		const saveSettings = toSaveSettings({ saveExecutionProgress: true });
+
+		expect(saveSettings.progress).toBe(true);
+
+		config.set('executions.saveExecutionProgress', true);
+
+		const _saveSettings = toSaveSettings({ saveExecutionProgress: false });
+
+		expect(_saveSettings.progress).toBe(false);
+	});
+
+	it('should favor fall back to default if workflow setting is explicit default', () => {
+		config.set('executions.saveExecutionProgress', true);
+
+		const saveSettings = toSaveSettings({ saveExecutionProgress: 'DEFAULT' });
+
+		expect(saveSettings.progress).toBe(true);
+
+		config.set('executions.saveExecutionProgress', false);
+
+		const _saveSettings = toSaveSettings({ saveExecutionProgress: 'DEFAULT' });
+
+		expect(_saveSettings.progress).toBe(false);
+	});
+
+	it('should fall back to default if no workflow setting', () => {
+		config.set('executions.saveExecutionProgress', true);
+
+		const saveSettings = toSaveSettings();
+
+		expect(saveSettings.progress).toBe(true);
+
+		config.set('executions.saveExecutionProgress', false);
+
+		const _saveSettings = toSaveSettings();
+
+		expect(_saveSettings.progress).toBe(false);
+	});
+});

packages/cli/test/unit/execution.lifecycle.test.ts

@@ -1,7 +1,7 @@
 import { restoreBinaryDataId } from '@/executionLifecycleHooks/restoreBinaryDataId';
 import { BinaryDataService } from 'n8n-core';
 import { mockInstance } from '../shared/mocking';
-import { toSaveSettings } from '@/executionLifecycleHooks/toSaveSettings';
+
 import type { IRun } from 'n8n-workflow';
 import config from '@/config';
 
@@ -140,100 +140,3 @@ for (const mode of ['filesystem-v2', 's3'] as const) {
 		expect(binaryDataService.rename).not.toHaveBeenCalled();
 	});
 }
-
-describe('toSaveSettings()', () => {
-	afterEach(() => {
-		jest.restoreAllMocks();
-		config.load(config.default);
-	});
-
-	describe('when setting `error`', () => {
-		it('should favor workflow settings over defaults', () => {
-			config.set('executions.saveDataOnError', 'none');
-
-			const saveSettings = toSaveSettings({ saveDataErrorExecution: 'all' });
-
-			expect(saveSettings.error).toBe(true);
-
-			config.set('executions.saveDataOnError', 'all');
-
-			const _saveSettings = toSaveSettings({ saveDataErrorExecution: 'none' });
-
-			expect(_saveSettings.error).toBe(false);
-		});
-
-		it('should fall back to default if no workflow setting', () => {
-			config.set('executions.saveDataOnError', 'all');
-
-			const saveSettings = toSaveSettings();
-
-			expect(saveSettings.error).toBe(true);
-
-			config.set('executions.saveDataOnError', 'none');
-
-			const _saveSettings = toSaveSettings();
-
-			expect(_saveSettings.error).toBe(false);
-		});
-	});
-
-	describe('when setting `success`', () => {
-		it('should favor workflow settings over defaults', () => {
-			config.set('executions.saveDataOnSuccess', 'none');
-
-			const saveSettings = toSaveSettings({ saveDataSuccessExecution: 'all' });
-
-			expect(saveSettings.success).toBe(true);
-
-			config.set('executions.saveDataOnSuccess', 'all');
-
-			const _saveSettings = toSaveSettings({ saveDataSuccessExecution: 'none' });
-
-			expect(_saveSettings.success).toBe(false);
-		});
-
-		it('should fall back to default if no workflow setting', () => {
-			config.set('executions.saveDataOnSuccess', 'all');
-
-			const saveSettings = toSaveSettings();
-
-			expect(saveSettings.success).toBe(true);
-
-			config.set('executions.saveDataOnSuccess', 'none');
-
-			const _saveSettings = toSaveSettings();
-
-			expect(_saveSettings.success).toBe(false);
-		});
-	});
-
-	describe('when setting `manual`', () => {
-		it('should favor workflow settings over defaults', () => {
-			config.set('executions.saveDataManualExecutions', false);
-
-			const saveSettings = toSaveSettings({ saveManualExecutions: true });
-
-			expect(saveSettings.manual).toBe(true);
-
-			config.set('executions.saveDataManualExecutions', true);
-
-			const _saveSettings = toSaveSettings({ saveManualExecutions: false });
-
-			expect(_saveSettings.manual).toBe(false);
-		});
-
-		it('should fall back to default if no workflow setting', () => {
-			config.set('executions.saveDataManualExecutions', true);
-
-			const saveSettings = toSaveSettings();
-
-			expect(saveSettings.manual).toBe(true);
-
-			config.set('executions.saveDataManualExecutions', false);
-
-			const _saveSettings = toSaveSettings();
-
-			expect(_saveSettings.manual).toBe(false);
-		});
-	});
-});