Skip to content

Commit

Permalink
fix: Return waiting as part of the finished executions
Browse files Browse the repository at this point in the history
  • Loading branch information
krynble committed Apr 18, 2024
1 parent 50cdb51 commit a0f2559
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 27 deletions.
19 changes: 13 additions & 6 deletions packages/cli/src/executions/execution.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import type {
WorkflowExecuteMode,
ExecutionStatus,
} from 'n8n-workflow';
import { ApplicationError, Workflow, WorkflowOperationError } from 'n8n-workflow';
import {
ApplicationError,
ExecutionStatusList,
Workflow,
WorkflowOperationError,
} from 'n8n-workflow';
import { ActiveExecutions } from '@/ActiveExecutions';
import type {
ExecutionPayload,
Expand Down Expand Up @@ -354,15 +359,17 @@ export class ExecutionService {
* and whether the total is an estimate or not. Active executions are excluded
* from the total and count for pagination purposes.
*/
async findAllActiveAndLatestFinished(query: ExecutionSummaries.RangeQuery) {
const active: ExecutionStatus[] = ['new', 'running'];
const finished: ExecutionStatus[] = ['success', 'error', 'canceled'];
async findAllRunningAndLatest(query: ExecutionSummaries.RangeQuery) {
const currentlyRunningStatuses: ExecutionStatus[] = ['new', 'running'];
const allStatuses = new Set(ExecutionStatusList);
currentlyRunningStatuses.forEach((status) => allStatuses.delete(status));
const notRunningStatuses: ExecutionStatus[] = Array.from(allStatuses);

const [activeResult, finishedResult] = await Promise.all([
this.findRangeWithCount({ ...query, status: active }),
this.findRangeWithCount({ ...query, status: currentlyRunningStatuses }),
this.findRangeWithCount({
...query,
status: finished,
status: notRunningStatuses,
order: { stoppedAt: 'DESC' },
}),
]);
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/executions/executions.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class ExecutionsController {
const noRange = !query.range.lastId || !query.range.firstId;

if (noStatus && noRange) {
return await this.executionService.findAllActiveAndLatestFinished(query);
return await this.executionService.findAllRunningAndLatest(query);
}

return await this.executionService.findRangeWithCount(query);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ describe('ExecutionService', () => {
accessibleWorkflowIds: [workflow.id],
};

const output = await executionService.findAllActiveAndLatestFinished(query);
const output = await executionService.findAllRunningAndLatest(query);

expect(output.results).toHaveLength(23); // 3 active + 20 finished (excludes 21st)
expect(output.count).toBe(totalFinished); // 21 finished, excludes active
Expand All @@ -363,7 +363,7 @@ describe('ExecutionService', () => {
accessibleWorkflowIds: [workflow.id],
};

const output = await executionService.findAllActiveAndLatestFinished(query);
const output = await executionService.findAllRunningAndLatest(query);

expect(output.results).toHaveLength(totalFinished); // 5 finished
expect(output.count).toBe(totalFinished); // 5 finished, excludes active
Expand All @@ -385,7 +385,7 @@ describe('ExecutionService', () => {
accessibleWorkflowIds: [workflow.id],
};

const output = await executionService.findAllActiveAndLatestFinished(query);
const output = await executionService.findAllRunningAndLatest(query);

expect(output.results).toHaveLength(3); // 3 finished
expect(output.count).toBe(0); // 0 finished, excludes active
Expand All @@ -401,7 +401,7 @@ describe('ExecutionService', () => {
accessibleWorkflowIds: [workflow.id],
};

const output = await executionService.findAllActiveAndLatestFinished(query);
const output = await executionService.findAllRunningAndLatest(query);

expect(output.results).toHaveLength(0);
expect(output.count).toBe(0);
Expand Down
12 changes: 6 additions & 6 deletions packages/cli/test/unit/controllers/executions.controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ describe('ExecutionsController', () => {
'should fetch executions per query',
async (rangeQuery) => {
workflowSharingService.getSharedWorkflowIds.mockResolvedValue(['123']);
executionService.findAllActiveAndLatestFinished.mockResolvedValue(NO_EXECUTIONS);
executionService.findAllRunningAndLatest.mockResolvedValue(NO_EXECUTIONS);

const req = mock<ExecutionRequest.GetMany>({ rangeQuery });

await executionsController.getMany(req);

expect(executionService.findAllActiveAndLatestFinished).not.toHaveBeenCalled();
expect(executionService.findAllRunningAndLatest).not.toHaveBeenCalled();
expect(executionService.findRangeWithCount).toHaveBeenCalledWith(rangeQuery);
},
);
Expand All @@ -87,13 +87,13 @@ describe('ExecutionsController', () => {
'should fetch executions per query',
async (rangeQuery) => {
workflowSharingService.getSharedWorkflowIds.mockResolvedValue(['123']);
executionService.findAllActiveAndLatestFinished.mockResolvedValue(NO_EXECUTIONS);
executionService.findAllRunningAndLatest.mockResolvedValue(NO_EXECUTIONS);

const req = mock<ExecutionRequest.GetMany>({ rangeQuery });

await executionsController.getMany(req);

expect(executionService.findAllActiveAndLatestFinished).toHaveBeenCalled();
expect(executionService.findAllRunningAndLatest).toHaveBeenCalled();
expect(executionService.findRangeWithCount).not.toHaveBeenCalled();
},
);
Expand All @@ -102,7 +102,7 @@ describe('ExecutionsController', () => {
describe('if both status and range provided', () => {
it('should fetch executions per query', async () => {
workflowSharingService.getSharedWorkflowIds.mockResolvedValue(['123']);
executionService.findAllActiveAndLatestFinished.mockResolvedValue(NO_EXECUTIONS);
executionService.findAllRunningAndLatest.mockResolvedValue(NO_EXECUTIONS);

const rangeQuery: ExecutionSummaries.RangeQuery = {
kind: 'range',
Expand All @@ -115,7 +115,7 @@ describe('ExecutionsController', () => {

await executionsController.getMany(req);

expect(executionService.findAllActiveAndLatestFinished).not.toHaveBeenCalled();
expect(executionService.findAllRunningAndLatest).not.toHaveBeenCalled();
expect(executionService.findRangeWithCount).toHaveBeenCalledWith(rangeQuery);
});
});
Expand Down
23 changes: 13 additions & 10 deletions packages/workflow/src/ExecutionStatus.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
export type ExecutionStatus =
| 'canceled'
| 'crashed'
| 'error'
| 'new'
| 'running'
| 'success'
| 'unknown'
| 'waiting'
| 'warning';
export const ExecutionStatusList = [
'canceled' as const,
'crashed' as const,
'error' as const,
'new' as const,
'running' as const,
'success' as const,
'unknown' as const,
'waiting' as const,
'warning' as const,
];

export type ExecutionStatus = (typeof ExecutionStatusList)[number];

0 comments on commit a0f2559

Please sign in to comment.