mudhorn/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2024-12-24 04:04:06 -08:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

fix(editor): Fix execution list item selection (#5606)

Browse source 
* fix(editor): Fix execution list item selection

* fix(editor): Delete only selected executions

* fix(editor): Fix clear selection

* fix(editor): Fix clear selection

* fix(editor): Fix clear selection

* feat(editor): Add select all existing executions checkbox

* fix(editor): Do not mark later loaded executions selected

* test(editor): Add execution list unit test

* fix(editor): Fix selection

* test(editor): update execution selection test

* fix(editor): Handle UI state when there is no execution

* fix(editor): Remove unnecessary logic

* test(editor): Add more execution list unit tests and fake data generation

* test(editor): Add more execution list unit tests

* test(editor): Simplifying test setup

* chore: update pnpm lock after resolving merge conflocts

* chore: fix package version

* fix: Improved executions deletion to prevent crashing and fixed removal of failed executions

* fix: Add comment to clarify why change was needed

* fix: fix executions list bug when selecting all and changing filter

* fix: fix execution lists running execution showing up on different workflow id

* fix(editor): Deleting an execution while all are selected

* fix(editor): Deleting an execution while all are selected

---------

Co-authored-by: Omar Ajoue <krynble@gmail.com>
Co-authored-by: Alex Grozav <alex@grozav.com>
This commit is contained in:
Csaba Tuncsik 2023-03-17 06:18:23 +01:00 committed by GitHub
parent 3718612bd7
commit 7a352efff9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 306 additions and 79 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
packages/cli/src/executions/executions.service.ts
View file

@ -541,6 +541,9 @@ export class ExecutionsService {
// delete executions by date, if user may access the underlying workflows
where.startedAt = LessThanOrEqual(deleteBefore);
Object.assign(where, requestFilters);
if (where.status) {
where.status = In(requestFiltersRaw!.status as string[]);
}
} else if (ids) {
// delete executions by IDs, if user may access the underlying workflows
where.id = In(ids);
@ -568,6 +571,10 @@ export class ExecutionsService {
idsToDelete.map(async (id) => binaryDataManager.deleteBinaryDataByExecutionId(id)),
);
await Db.collections.Execution.delete(idsToDelete);
do {
// Delete in batches to avoid "SQLITE_ERROR: Expression tree is too large (maximum depth 1000)" error
const batch = idsToDelete.splice(0, 500);
await Db.collections.Execution.delete(batch);
} while (idsToDelete.length > 0);
}
}

1
packages/editor-ui/package.json
View file

@ -86,6 +86,7 @@
"@faker-js/faker": "^7.6.0",
"@pinia/testing": "^0.0.14",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/user-event": "^14.4.3",
"@testing-library/vue": "^5.8.3",
"@types/dateformat": "^3.0.0",
"@types/express": "^4.17.6",

193
packages/editor-ui/src/components/ExecutionsList.vue
View file

@ -31,20 +31,39 @@
>
<n8n-option v-for="item in statuses" :key="item.id" :label="item.name" :value="item.id" />
</n8n-select>
<el-checkbox v-model="autoRefresh" @change="handleAutoRefreshToggle">
<el-checkbox
v-model="autoRefresh"
@change="handleAutoRefreshToggle"
data-testid="execution-auto-refresh-checkbox"
>
{{ $locale.baseText('executionsList.autoRefresh') }}
</el-checkbox>
</div>
<el-checkbox
v-if="allVisibleSelected && finishedExecutionsCount > 0"
:class="$style.selectAll"
:label="
$locale.baseText('executionsList.selectAll', {
adjustToNumber: finishedExecutionsCount,
interpolate: { executionNum: finishedExecutionsCount },
})
"
:value="allExistingSelected"
@change="handleCheckAllExistingChange"
data-testid="select-all-executions-checkbox"
/>
<table :class="$style.execTable">
<thead>
<tr>
<th>
<el-checkbox
:indeterminate="isIndeterminate"
v-model="checkAll"
@change="handleCheckAllChange"
:value="allVisibleSelected"
@change="handleCheckAllVisibleChange"
:disabled="finishedExecutionsCount < 1"
label=""
data-testid="select-visible-executions-checkbox"
/>
</th>
<th>{{ $locale.baseText('executionsList.name') }}</th>
@ -66,9 +85,10 @@
<td>
<el-checkbox
v-if="execution.stoppedAt !== undefined && execution.id"
:value="selectedItems[execution.id.toString()] || checkAll"
:value="selectedItems[execution.id] || allExistingSelected"
@change="handleCheckboxChanged(execution.id)"
label=""
data-testid="select-execution-checkbox"
/>
</td>
<td>
@ -213,9 +233,16 @@
</tbody>
</table>
<div
v-if="!combinedExecutions.length"
:class="$style.loadedAll"
data-testid="execution-list-empty"
>
{{ $locale.baseText('executionsList.empty') }}
</div>
<div
:class="$style.loadMore"
v-if="
v-else-if="
finishedExecutionsCount > finishedExecutions.length || finishedExecutionsCountEstimated
"
>
@ -225,23 +252,37 @@
:label="$locale.baseText('executionsList.loadMore')"
@click="loadMore()"
:loading="isDataLoading"
data-testid="load-more-button"
/>
</div>
<div v-else :class="$style.loadedAll">{{ $locale.baseText('executionsList.loadedAll') }}</div>
<div v-else :class="$style.loadedAll" data-testid="execution-all-loaded">
{{ $locale.baseText('executionsList.loadedAll') }}
</div>
</div>
<div v-if="checkAll === true || isIndeterminate === true" :class="$style.selectionOptions">
<div
v-if="numSelected > 0"
:class="$style.selectionOptions"
data-testid="selected-executions-info"
>
<span>
{{ $locale.baseText('executionsList.selected', { interpolate: { numSelected } }) }}
{{
$locale.baseText('executionsList.selected', {
adjustToNumber: numSelected,
interpolate: { numSelected },
})
}}
</span>
<n8n-button
:label="$locale.baseText('generic.delete')"
type="tertiary"
@click="handleDeleteSelected"
data-testid="delete-selected-button"
/>
<n8n-button
:label="$locale.baseText('executionsList.clearSelection')"
type="tertiary"
@click="handleClearSelection"
data-testid="clear-selection-button"
/>
</div>
</div>
@ -261,10 +302,9 @@ import {
IExecutionsCurrentSummaryExtended,
IExecutionDeleteFilter,
IExecutionsListResponse,
IExecutionsSummary,
IWorkflowShortResponse,
} from '@/Interface';
import type { ExecutionStatus, IDataObject } from 'n8n-workflow';
import type { IExecutionsSummary, ExecutionStatus, IDataObject } from 'n8n-workflow';
import { range as _range } from 'lodash-es';
import mixins from 'vue-typed-mixins';
import { mapStores } from 'pinia';
@ -285,7 +325,8 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
finishedExecutionsCount: 0,
finishedExecutionsCountEstimated: false,
checkAll: false,
allVisibleSelected: false,
allExistingSelected: false,
autoRefresh: true,
autoRefreshInterval: undefined as undefined | NodeJS.Timer,
@ -353,33 +394,28 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
return this.workflowsStore.activeExecutions;
},
combinedExecutions(): IExecutionsSummary[] {
const returnData: IExecutionsSummary[] = [];
const returnData = [];
if (['ALL', 'running'].includes(this.filter.status)) {
returnData.push(...this.activeExecutions);
returnData.push(...(this.activeExecutions as IExecutionsSummary[]));
}
if (['ALL', 'error', 'success', 'waiting'].includes(this.filter.status)) {
returnData.push(...this.finishedExecutions);
}
return returnData;
},
combinedExecutionsCount(): number {
return 0 + this.activeExecutions.length + this.finishedExecutionsCount;
return returnData.filter(
(execution) =>
this.filter.workflowId === 'ALL' || execution.workflowId === this.filter.workflowId,
);
},
numSelected(): number {
if (this.checkAll) {
if (this.allExistingSelected) {
return this.finishedExecutionsCount;
}
return Object.keys(this.selectedItems).length;
},
isIndeterminate(): boolean {
if (this.checkAll) {
return false;
}
return this.numSelected > 0;
},
workflowFilterCurrent(): IDataObject {
const filter: IDataObject = {};
if (this.filter.workflowId !== 'ALL') {
@ -434,9 +470,18 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
this.autoRefreshInterval = setInterval(() => this.loadAutoRefresh(), 4 * 1000); // refresh data every 4 secs
}
},
handleCheckAllChange() {
if (!this.checkAll) {
handleCheckAllExistingChange() {
this.allExistingSelected = !this.allExistingSelected;
this.allVisibleSelected = !this.allExistingSelected;
this.handleCheckAllVisibleChange();
},
handleCheckAllVisibleChange() {
this.allVisibleSelected = !this.allVisibleSelected;
if (!this.allVisibleSelected) {
this.allExistingSelected = false;
Vue.set(this, 'selectedItems', {});
} else {
this.selectAllVisibleExecutions();
}
},
handleCheckboxChanged(executionId: string) {
@ -445,6 +490,10 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
} else {
Vue.set(this.selectedItems, executionId, true);
}
this.allVisibleSelected =
Object.keys(this.selectedItems).length === this.combinedExecutions.length;
this.allExistingSelected =
Object.keys(this.selectedItems).length === this.finishedExecutionsCount;
},
async handleDeleteSelected() {
const deleteExecutions = await this.confirmMessage(
@ -464,7 +513,7 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
this.isDataLoading = true;
const sendData: IExecutionDeleteFilter = {};
if (this.checkAll) {
if (this.allExistingSelected) {
sendData.deleteBefore = this.finishedExecutions[0].startedAt as Date;
} else {
sendData.ids = Object.keys(this.selectedItems);
@ -474,45 +523,6 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
try {
await this.restApi().deleteExecutions(sendData);
let removedCurrentlyLoadedExecution = false;
let removedActiveExecution = false;
const currentWorkflow: string = this.workflowsStore.workflowId;
const activeExecution: IExecutionsSummary | null =
this.workflowsStore.activeWorkflowExecution;
// Also update current workflow executions view if needed
for (const selectedId of Object.keys(this.selectedItems)) {
const execution: IExecutionsSummary | undefined =
this.workflowsStore.getExecutionDataById(selectedId);
if (execution && execution.workflowId === currentWorkflow) {
this.workflowsStore.deleteExecution(execution);
removedCurrentlyLoadedExecution = true;
}
if (
execution !== undefined &&
activeExecution !== null &&
execution.id === activeExecution.id
) {
removedActiveExecution = true;
}
}
// Also update route if needed
if (removedCurrentlyLoadedExecution) {
const currentWorkflowExecutions: IExecutionsSummary[] =
this.workflowsStore.currentWorkflowExecutions;
if (currentWorkflowExecutions.length === 0) {
this.workflowsStore.activeWorkflowExecution = null;
this.$router.push({ name: VIEWS.EXECUTION_HOME, params: { name: currentWorkflow } });
} else if (removedActiveExecution) {
this.workflowsStore.activeWorkflowExecution = currentWorkflowExecutions[0];
this.$router
.push({
name: VIEWS.EXECUTION_PREVIEW,
params: { name: currentWorkflow, executionId: currentWorkflowExecutions[0].id },
})
.catch(() => {});
}
}
} catch (error) {
this.isDataLoading = false;
this.$showError(
@ -529,16 +539,15 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
type: 'success',
});
Vue.set(this, 'selectedItems', {});
this.checkAll = false;
this.handleClearSelection();
this.refreshData();
},
handleClearSelection() {
this.checkAll = false;
this.handleCheckAllChange();
handleClearSelection(): void {
this.allVisibleSelected = false;
this.allExistingSelected = false;
Vue.set(this, 'selectedItems', {});
},
handleFilterChanged() {
handleFilterChanged(): void {
this.refreshData();
},
handleActionItemClick(commandData: { command: string; execution: IExecutionsSummary }) {
@ -668,6 +677,8 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
Vue.set(this, 'finishedExecutions', alreadyPresentExecutionsFiltered);
this.workflowsStore.addToCurrentExecutions(alreadyPresentExecutionsFiltered);
this.adjustSelectionAfterMoreItemsLoaded();
},
async loadFinishedExecutions(): Promise<void> {
if (this.filter.status === 'running') {
@ -685,6 +696,10 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
this.finishedExecutionsCountEstimated = data.estimated;
this.workflowsStore.addToCurrentExecutions(data.results);
if (this.finishedExecutions.length === 0) {
this.handleClearSelection();
}
},
async loadMore() {
if (this.filter.status === 'running') {
@ -726,6 +741,8 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
this.isDataLoading = false;
this.workflowsStore.addToCurrentExecutions(data.results);
this.adjustSelectionAfterMoreItemsLoaded();
},
async loadWorkflows() {
try {
@ -919,6 +936,11 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
try {
await this.restApi().deleteExecutions({ ids: [execution.id] });
await this.refreshData();
if (this.allVisibleSelected) {
Vue.set(this, 'selectedItems', {});
this.selectAllVisibleExecutions();
}
} catch (error) {
this.$showError(
error,
@ -936,6 +958,17 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
isRunning(execution: IExecutionsSummary): boolean {
return this.getStatus(execution) === 'running';
},
selectAllVisibleExecutions() {
this.combinedExecutions.forEach((execution: IExecutionsSummary) => {
Vue.set(this.selectedItems, execution.id, true);
});
},
adjustSelectionAfterMoreItemsLoaded() {
if (this.allExistingSelected) {
this.allVisibleSelected = true;
this.selectAllVisibleExecutions();
}
},
},
},
);
@ -969,7 +1002,7 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
z-index: 2;
left: 50%;
transform: translateX(-50%);
bottom: var(--spacing-xl);
bottom: var(--spacing-3xl);
background: var(--color-background-dark);
border-radius: var(--border-radius-base);
color: var(--color-text-xlight);
@ -1141,7 +1174,7 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
}
.loadMore {
margin: var(--spacing-l) 0;
margin: var(--spacing-m) 0;
width: 100%;
text-align: center;
}
@ -1159,4 +1192,10 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi,
.retryAction + .deleteAction {
border-top: 1px solid var(--color-foreground-light);
}
.selectAll {
display: inline-block;
margin: 0 0 var(--spacing-s) var(--spacing-s);
color: var(--color-danger);
}
</style>

167
packages/editor-ui/src/components/__tests__/ExecutionsList.test.ts Normal file
View file

@ -0,0 +1,167 @@
import { vi, describe, it, expect } from 'vitest';
import Vue from 'vue';
import { PiniaVuePlugin } from 'pinia';
import { createTestingPinia } from '@pinia/testing';
import { render } from '@testing-library/vue';
import userEvent from '@testing-library/user-event';
import { faker } from '@faker-js/faker';
import { STORES } from '@/constants';
import ExecutionsList from '@/components/ExecutionsList.vue';
import { externalHooks } from '@/mixins/externalHooks';
import { genericHelpers } from '@/mixins/genericHelpers';
import { executionHelpers } from '@/mixins/executionsHelpers';
import { showMessage } from '@/mixins/showMessage';
import { i18nInstance } from '@/plugins/i18n';
import type { IWorkflowShortResponse } from '@/Interface';
import type { IExecutionsSummary } from 'n8n-workflow';
const waitAllPromises = () => new Promise((resolve) => setTimeout(resolve));
const workflowDataFactory = (): IWorkflowShortResponse => ({
createdAt: faker.date.past().toDateString(),
updatedAt: faker.date.past().toDateString(),
id: faker.datatype.uuid(),
name: faker.datatype.string(),
active: faker.datatype.boolean(),
tags: [],
});
const executionDataFactory = (): IExecutionsSummary => ({
id: faker.datatype.uuid(),
finished: faker.datatype.boolean(),
mode: faker.helpers.arrayElement(['manual', 'trigger']),
startedAt: faker.date.past(),
stoppedAt: faker.date.past(),
workflowId: faker.datatype.number().toString(),
workflowName: faker.datatype.string(),
status: faker.helpers.arrayElement(['failed', 'success']),
nodeExecutionStatus: {},
});
const workflowsData = Array.from({ length: 10 }, workflowDataFactory);
const executionsData = Array.from({ length: 2 }, () => ({
count: 20,
results: Array.from({ length: 10 }, executionDataFactory),
estimated: false,
}));
let getPastExecutionsSpy = vi.fn().mockResolvedValue({ count: 0, results: [], estimated: false });
const mockRestApiMixin = Vue.extend({
methods: {
restApi() {
return {
getWorkflows: vi.fn().mockResolvedValue(workflowsData),
getCurrentExecutions: vi.fn().mockResolvedValue([]),
getPastExecutions: getPastExecutionsSpy,
};
},
},
});
const renderOptions = {
pinia: createTestingPinia({
initialState: {
[STORES.SETTINGS]: {
settings: {
templates: {
enabled: true,
host: 'https://api.n8n.io/api/',
},
},
},
},
}),
i18n: i18nInstance,
stubs: ['font-awesome-icon'],
mixins: [externalHooks, genericHelpers, executionHelpers, showMessage, mockRestApiMixin],
};
function TelemetryPlugin(vue: typeof Vue): void {
Object.defineProperty(vue, '$telemetry', {
get() {
return {
track: () => {},
};
},
});
Object.defineProperty(vue.prototype, '$telemetry', {
get() {
return {
track: () => {},
};
},
});
}
const renderComponent = async () => {
const renderResult = render(ExecutionsList, renderOptions);
await waitAllPromises();
return renderResult;
};
Vue.use(TelemetryPlugin);
Vue.use(PiniaVuePlugin);
describe('ExecutionsList.vue', () => {
it('should render empty list', async () => {
const { queryAllByTestId, queryByTestId, getByTestId } = await renderComponent();
await userEvent.click(getByTestId('execution-auto-refresh-checkbox'));
expect(queryAllByTestId('select-execution-checkbox').length).toBe(0);
expect(queryByTestId('load-more-button')).not.toBeInTheDocument();
expect(queryByTestId('select-all-executions-checkbox')).not.toBeInTheDocument();
expect(getByTestId('execution-list-empty')).toBeInTheDocument();
});
it('should handle selection flow when loading more items', async () => {
getPastExecutionsSpy = vi
.fn()
.mockResolvedValueOnce(executionsData[0])
.mockResolvedValueOnce(executionsData[1]);
const { getByTestId, getAllByTestId, queryByTestId } = await renderComponent();
await userEvent.click(getByTestId('execution-auto-refresh-checkbox'));
await userEvent.click(getByTestId('select-visible-executions-checkbox'));
expect(getPastExecutionsSpy).toHaveBeenCalledTimes(1);
expect(
getAllByTestId('select-execution-checkbox').filter((el) =>
el.contains(el.querySelector(':checked')),
).length,
).toBe(10);
expect(getByTestId('select-all-executions-checkbox')).toBeInTheDocument();
expect(getByTestId('selected-executions-info').textContent).toContain(10);
await userEvent.click(getByTestId('load-more-button'));
expect(getAllByTestId('select-execution-checkbox').length).toBe(20);
expect(
getAllByTestId('select-execution-checkbox').filter((el) =>
el.contains(el.querySelector(':checked')),
).length,
).toBe(10);
await userEvent.click(getByTestId('select-all-executions-checkbox'));
expect(getAllByTestId('select-execution-checkbox').length).toBe(20);
expect(
getAllByTestId('select-execution-checkbox').filter((el) =>
el.contains(el.querySelector(':checked')),
).length,
).toBe(20);
expect(getByTestId('selected-executions-info').textContent).toContain(20);
await userEvent.click(getAllByTestId('select-execution-checkbox')[2]);
expect(getAllByTestId('select-execution-checkbox').length).toBe(20);
expect(
getAllByTestId('select-execution-checkbox').filter((el) =>
el.contains(el.querySelector(':checked')),
).length,
).toBe(19);
expect(getByTestId('selected-executions-info').textContent).toContain(19);
expect(getByTestId('select-visible-executions-checkbox')).toBeInTheDocument();
expect(queryByTestId('select-all-executions-checkbox')).not.toBeInTheDocument();
});
});

4
packages/editor-ui/src/plugins/i18n/locales/en.json
View file

@ -454,6 +454,7 @@
"executionsList.error": "Failed",
"executionsList.filters": "Filters",
"executionsList.loadMore": "Load More",
"executionsList.empty": "No executions",
"executionsList.loadedAll": "No more executions to fetch",
"executionsList.modes.error": "error",
"executionsList.modes.integrated": "integrated",
@ -471,7 +472,8 @@
"executionsList.succeeded": "Succeeded",
"executionsList.selectStatus": "Select Status",
"executionsList.selectWorkflow": "Select Workflow",
"executionsList.selected": "{numSelected} execution selected:",
"executionsList.selected": "{numSelected} execution selected: | {numSelected} executions selected:",
"executionsList.selectAll": "Select {executionNum} finished execution | Select all {executionNum} finished executions",
"executionsList.test": "Test execution",
"executionsList.showError.handleDeleteSelected.title": "Problem deleting executions",
"executionsList.showError.loadMore.title": "Problem loading executions",

11
pnpm-lock.yaml
View file

@ -573,6 +573,7 @@ importers:
'@jsplumb/util': ^5.13.2
'@pinia/testing': ^0.0.14
'@testing-library/jest-dom': ^5.16.5
'@testing-library/user-event': ^14.4.3
'@testing-library/vue': ^5.8.3
'@types/dateformat': ^3.0.0
'@types/express': ^4.17.6
@ -696,6 +697,7 @@ importers:
'@faker-js/faker': 7.6.0
'@pinia/testing': 0.0.14_pinia@2.0.23+vue@2.7.14
'@testing-library/jest-dom': 5.16.5
'@testing-library/user-event': 14.4.3_7izb363m7fjrh7ob6q4a2yqaqe
'@testing-library/vue': 5.8.3_rhqkolmkwunxzlyyxxsuwaiuri
'@types/dateformat': 3.0.1
'@types/express': 4.17.14
@ -5304,6 +5306,15 @@ packages:
redent: 3.0.0
dev: true
/@testing-library/user-event/14.4.3_7izb363m7fjrh7ob6q4a2yqaqe:
resolution: {integrity: sha512-kCUc5MEwaEMakkO5x7aoD+DLi02ehmEM2QCGWvNqAS1dV/fAvORWEjnjsEIvml59M7Y5kCkWN6fCCyPOe8OL6Q==}
engines: {node: '>=12', npm: '>=6'}
peerDependencies:
'@testing-library/dom': '>=7.21.4'
dependencies:
'@testing-library/dom': 7.31.2
dev: true
/@testing-library/vue/5.8.3_rhqkolmkwunxzlyyxxsuwaiuri:
resolution: {integrity: sha512-M6+QqP1xuFHixKOeXF9pCLbtiyJZRKfJRP+unBf6Ljm7aS1V2CSS95oTetFoblaj0W1+AC9XJgwmUDtlLoaakQ==}
engines: {node: '>10.18'}