From 3459eb6c2f8ed37cdce1ac4e9087b7060006d8b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 21 Nov 2023 17:33:44 +0100 Subject: [PATCH] refactor(core): Include execution progress in save settings (no-changelog) (#7769) --- .../cli/src/WorkflowExecuteAdditionalData.ts | 15 +- .../executionLifecycleHooks/toSaveSettings.ts | 18 +- .../execution-lifecyle/toSaveSettings.test.ts | 154 ++++++++++++++++++ .../cli/test/unit/execution.lifecycle.test.ts | 99 +---------- 4 files changed, 173 insertions(+), 113 deletions(-) create mode 100644 packages/cli/test/unit/execution-lifecyle/toSaveSettings.test.ts diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 73a533a2e0..556b92cd23 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -358,18 +358,9 @@ export function hookFunctionsPreExecute(parentProcessMode?: string): IWorkflowEx data: ITaskData, executionData: IRunExecutionData, ): Promise { - const saveExecutionProgress = config.getEnv('executions.saveExecutionProgress'); - const workflowSettings = this.workflowData.settings; - if (workflowSettings !== undefined) { - if (workflowSettings.saveExecutionProgress === false) { - return; - } - if (workflowSettings.saveExecutionProgress !== true && !saveExecutionProgress) { - return; - } - } else if (!saveExecutionProgress) { - return; - } + const saveSettings = toSaveSettings(this.workflowData.settings); + + if (!saveSettings.progress) return; try { logger.debug( diff --git a/packages/cli/src/executionLifecycleHooks/toSaveSettings.ts b/packages/cli/src/executionLifecycleHooks/toSaveSettings.ts index f81b028129..4b4aac2b85 100644 --- a/packages/cli/src/executionLifecycleHooks/toSaveSettings.ts +++ b/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, - * for error executions, success executions, and manual executions. + * Return whether a workflow execution is configured to be saved or not: + * + * - `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, }; } diff --git a/packages/cli/test/unit/execution-lifecyle/toSaveSettings.test.ts b/packages/cli/test/unit/execution-lifecyle/toSaveSettings.test.ts new file mode 100644 index 0000000000..6fc516a0ea --- /dev/null +++ b/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); + }); +}); diff --git a/packages/cli/test/unit/execution.lifecycle.test.ts b/packages/cli/test/unit/execution.lifecycle.test.ts index 59929708f8..c20bd07f6f 100644 --- a/packages/cli/test/unit/execution.lifecycle.test.ts +++ b/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); - }); - }); -});