diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index f4f10472a2..442c749bf8 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -63,6 +63,7 @@ import { updateExistingExecution, } from './executionLifecycleHooks/shared/sharedHookFunctions'; import { restoreBinaryDataId } from './executionLifecycleHooks/restoreBinaryDataId'; +import { toSaveSettings } from './executionLifecycleHooks/toSaveSettings'; import { Logger } from './Logger'; const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType'); @@ -508,14 +509,9 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { } } - const workflowSettings = this.workflowData.settings; - let saveManualExecutions = config.getEnv('executions.saveDataManualExecutions'); - if (workflowSettings?.saveManualExecutions !== undefined) { - // Apply to workflow override - saveManualExecutions = workflowSettings.saveManualExecutions as boolean; - } + const saveSettings = toSaveSettings(this.workflowData.settings); - if (isManualMode && !saveManualExecutions && !fullRunData.waitTill) { + if (isManualMode && !saveSettings.manual && !fullRunData.waitTill) { await Container.get(ExecutionRepository).hardDelete({ workflowId: this.workflowData.id as string, executionId: this.executionId, @@ -524,24 +520,12 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { return; } - // Check config to know if execution should be saved or not - let saveDataErrorExecution = config.getEnv('executions.saveDataOnError') as string; - let saveDataSuccessExecution = config.getEnv('executions.saveDataOnSuccess') as string; - if (this.workflowData.settings !== undefined) { - saveDataErrorExecution = - (this.workflowData.settings.saveDataErrorExecution as string) || - saveDataErrorExecution; - saveDataSuccessExecution = - (this.workflowData.settings.saveDataSuccessExecution as string) || - saveDataSuccessExecution; - } + const executionStatus = determineFinalExecutionStatus(fullRunData); + const shouldNotSave = + (executionStatus === 'success' && !saveSettings.success) || + (executionStatus !== 'success' && !saveSettings.error); - const workflowStatusFinal = determineFinalExecutionStatus(fullRunData); - - if ( - (workflowStatusFinal === 'success' && saveDataSuccessExecution === 'none') || - (workflowStatusFinal !== 'success' && saveDataErrorExecution === 'none') - ) { + if (shouldNotSave) { if (!fullRunData.waitTill && !isManualMode) { executeErrorWorkflow( this.workflowData, @@ -564,7 +548,7 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks { const fullExecutionData = prepareExecutionDataForDbUpdate({ runData: fullRunData, workflowData: this.workflowData, - workflowStatusFinal, + workflowStatusFinal: executionStatus, retryOf: this.retryOf, }); @@ -1135,23 +1119,14 @@ export function getWorkflowHooksWorkerMain( fullRunData: IRun, newStaticData: IDataObject, ): Promise { - // Check config to know if execution should be saved or not - let saveDataErrorExecution = config.getEnv('executions.saveDataOnError') as string; - let saveDataSuccessExecution = config.getEnv('executions.saveDataOnSuccess') as string; - if (this.workflowData.settings !== undefined) { - saveDataErrorExecution = - (this.workflowData.settings.saveDataErrorExecution as string) || saveDataErrorExecution; - saveDataSuccessExecution = - (this.workflowData.settings.saveDataSuccessExecution as string) || - saveDataSuccessExecution; - } + const executionStatus = determineFinalExecutionStatus(fullRunData); + const saveSettings = toSaveSettings(this.workflowData.settings); - const workflowStatusFinal = determineFinalExecutionStatus(fullRunData); + const shouldNotSave = + (executionStatus === 'success' && !saveSettings.success) || + (executionStatus !== 'success' && !saveSettings.error); - if ( - (workflowStatusFinal === 'success' && saveDataSuccessExecution === 'none') || - (workflowStatusFinal !== 'success' && saveDataErrorExecution === 'none') - ) { + if (shouldNotSave) { await Container.get(ExecutionRepository).hardDelete({ workflowId: this.workflowData.id as string, executionId: this.executionId, diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 59df756b6d..f8496957b9 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -119,7 +119,7 @@ export const schema = { }, rejectUnauthorized: { doc: 'If unauthorized SSL connections should be rejected', - format: 'Boolean', + format: Boolean, default: true, env: 'DB_POSTGRESDB_SSL_REJECT_UNAUTHORIZED', }, @@ -215,7 +215,7 @@ export const schema = { }, onboardingFlowDisabled: { doc: 'Show onboarding flow in new workflow', - format: 'Boolean', + format: Boolean, default: false, env: 'N8N_ONBOARDING_FLOW_DISABLED', }, @@ -288,7 +288,7 @@ export const schema = { }, saveExecutionProgress: { doc: 'Whether or not to save progress for each node executed', - format: 'Boolean', + format: Boolean, default: false, env: 'EXECUTIONS_DATA_SAVE_ON_PROGRESS', }, @@ -300,7 +300,7 @@ export const schema = { // in the editor. saveDataManualExecutions: { doc: 'Save data of executions when started manually via editor', - format: 'Boolean', + format: Boolean, default: true, env: 'EXECUTIONS_DATA_SAVE_MANUAL_EXECUTIONS', }, @@ -312,7 +312,7 @@ export const schema = { // a future version. pruneData: { doc: 'Delete data of past executions on a rolling basis', - format: 'Boolean', + format: Boolean, default: true, env: 'EXECUTIONS_DATA_PRUNE', }, @@ -358,7 +358,7 @@ export const schema = { health: { active: { doc: 'If health checks should be enabled', - format: 'Boolean', + format: Boolean, default: false, env: 'QUEUE_HEALTH_CHECK_ACTIVE', }, @@ -420,7 +420,7 @@ export const schema = { env: 'QUEUE_BULL_REDIS_CLUSTER_NODES', }, tls: { - format: 'Boolean', + format: Boolean, default: false, env: 'QUEUE_BULL_REDIS_TLS', doc: 'Enable TLS on Redis connections. Default: false', @@ -558,7 +558,7 @@ export const schema = { }, metrics: { enable: { - format: 'Boolean', + format: Boolean, default: false, env: 'N8N_METRICS', doc: 'Enable /metrics endpoint. Default: false', diff --git a/packages/cli/src/executionLifecycleHooks/toSaveSettings.ts b/packages/cli/src/executionLifecycleHooks/toSaveSettings.ts new file mode 100644 index 0000000000..2b7ba09c83 --- /dev/null +++ b/packages/cli/src/executionLifecycleHooks/toSaveSettings.ts @@ -0,0 +1,20 @@ +import config from '@/config'; +import type { IWorkflowSettings } from 'n8n-workflow'; + +const DEFAULTS = { + ERROR: config.getEnv('executions.saveDataOnError'), + SUCCESS: config.getEnv('executions.saveDataOnSuccess'), + MANUAL: config.getEnv('executions.saveDataManualExecutions'), +}; + +/** + * Return whether a workflow execution is configured to be saved or not, + * for error executions, success executions, and manual executions. + */ +export function toSaveSettings(workflowSettings: IWorkflowSettings = {}) { + return { + error: workflowSettings.saveDataErrorExecution !== 'none' ?? DEFAULTS.ERROR !== 'none', + success: workflowSettings.saveDataSuccessExecution !== 'none' ?? DEFAULTS.SUCCESS !== 'none', + manual: workflowSettings?.saveManualExecutions ?? DEFAULTS.MANUAL, + }; +} diff --git a/packages/cli/test/unit/execution.lifecycle.test.ts b/packages/cli/test/unit/execution.lifecycle.test.ts index c41465c8e5..48fabfec36 100644 --- a/packages/cli/test/unit/execution.lifecycle.test.ts +++ b/packages/cli/test/unit/execution.lifecycle.test.ts @@ -1,6 +1,7 @@ import { restoreBinaryDataId } from '@/executionLifecycleHooks/restoreBinaryDataId'; import { BinaryDataService } from 'n8n-core'; import { mockInstance } from '../integration/shared/utils/mocking'; +import { toSaveSettings } from '@/executionLifecycleHooks/toSaveSettings'; import type { IRun } from 'n8n-workflow'; import config from '@/config'; @@ -129,4 +130,58 @@ for (const mode of ['filesystem-v2', 's3'] as const) { }); }); }); + + it('should do nothing on itemless case', async () => { + const executionId = '999'; + + const promise = restoreBinaryDataId(toIRun(), executionId); + + await expect(promise).resolves.not.toThrow(); + + expect(binaryDataService.rename).not.toHaveBeenCalled(); + }); } + +describe('toSaveSettings()', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('should set `error` based on workflow settings', () => { + const saveSettings = toSaveSettings({ saveDataErrorExecution: 'all' }); + + expect(saveSettings.error).toBe(true); + + const _saveSettings = toSaveSettings({ saveDataErrorExecution: 'none' }); + + expect(_saveSettings.error).toBe(false); + }); + + it('should set `success` based on workflow settings', () => { + const saveSettings = toSaveSettings({ saveDataSuccessExecution: 'all' }); + + expect(saveSettings.success).toBe(true); + + const _saveSettings = toSaveSettings({ saveDataSuccessExecution: 'none' }); + + expect(_saveSettings.success).toBe(false); + }); + + it('should set `manual` based on workflow settings', () => { + const saveSettings = toSaveSettings({ saveManualExecutions: true }); + + expect(saveSettings.manual).toBe(true); + + const _saveSettings = toSaveSettings({ saveManualExecutions: false }); + + expect(_saveSettings.manual).toBe(false); + }); + + it('should return defaults if no workflow settings', async () => { + const saveSettings = toSaveSettings(); + + expect(saveSettings.error).toBe(true); + expect(saveSettings.success).toBe(true); + expect(saveSettings.manual).toBe(true); + }); +});