fix: Improve executions list polling performance (#6355)

* refactor: move auto-refresh logic to executions list * fix: improve auto-refresh polling * fix: update executions list view to use same interval mechanism as executions sidebar * chore: fix linting issue * fix: fix executions list test * fix: fix linting issue
n8n-io · Jun 1, 2023 · b5cabfe · b5cabfe
1 parent 33e7ff8
commit b5cabfe
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 69 deletions.
diff --git a/packages/editor-ui/src/components/ExecutionsList.vue b/packages/editor-ui/src/components/ExecutionsList.vue
@@ -311,6 +311,12 @@ export default defineComponent({
 		ExecutionTime,
 		ExecutionFilter,
 	},
+	props: {
+		autoRefreshEnabled: {
+			type: Boolean,
+			default: true,
+		},
+	},
 	setup() {
 		return {
 			...useToast(),
@@ -326,8 +332,8 @@ export default defineComponent({
 
 			allVisibleSelected: false,
 			allExistingSelected: false,
-			autoRefresh: true,
-			autoRefreshInterval: undefined as undefined | NodeJS.Timer,
+			autoRefresh: this.autoRefreshEnabled,
+			autoRefreshTimeout: undefined as undefined | NodeJS.Timer,
 
 			filter: {} as ExecutionFilterType,
 
@@ -343,11 +349,12 @@ export default defineComponent({
 	},
 	mounted() {
 		setPageTitle(`n8n - ${this.pageTitle}`);
+
+		void this.handleAutoRefreshToggle();
 		document.addEventListener('visibilitychange', this.onDocumentVisibilityChange);
 	},
 	async created() {
 		await this.loadWorkflows();
-		this.handleAutoRefreshToggle();
 
 		void this.$externalHooks().run('executionsList.openDialog');
 		this.$telemetry.track('User opened Executions log', {
@@ -410,9 +417,9 @@ export default defineComponent({
 			});
 			window.open(route.href, '_blank');
 		},
-		handleAutoRefreshToggle() {
+		async handleAutoRefreshToggle() {
 			this.stopAutoRefreshInterval(); // Clear any previously existing intervals (if any - there shouldn't)
-			this.startAutoRefreshInterval();
+			void this.startAutoRefreshInterval();
 		},
 		handleCheckAllExistingChange() {
 			this.allExistingSelected = !this.allExistingSelected;
@@ -488,7 +495,7 @@ export default defineComponent({
 			});
 
 			this.handleClearSelection();
-			this.refreshData();
+			await this.refreshData();
 		},
 		handleClearSelection(): void {
 			this.allVisibleSelected = false;
@@ -501,14 +508,14 @@ export default defineComponent({
 			this.handleClearSelection();
 			this.isMounting = false;
 		},
-		handleActionItemClick(commandData: { command: string; execution: IExecutionsSummary }) {
+		async handleActionItemClick(commandData: { command: string; execution: IExecutionsSummary }) {
 			if (['currentlySaved', 'original'].includes(commandData.command)) {
 				let loadWorkflow = false;
 				if (commandData.command === 'currentlySaved') {
 					loadWorkflow = true;
 				}
 
-				this.retryExecution(commandData.execution, loadWorkflow);
+				await this.retryExecution(commandData.execution, loadWorkflow);
 
 				this.$telemetry.track('User clicked retry execution button', {
 					workflow_id: this.workflowsStore.workflowId,
@@ -517,7 +524,7 @@ export default defineComponent({
 				});
 			}
 			if (commandData.command === 'delete') {
-				this.deleteExecution(commandData.execution);
+				await this.deleteExecution(commandData.execution);
 			}
 		},
 		getWorkflowName(workflowId: string): string | undefined {
@@ -568,8 +575,11 @@ export default defineComponent({
 			);
 			let lastId = 0;
 			const gaps = [] as number[];
-			for (let i = results[0].results.length - 1; i >= 0; i--) {
-				const currentItem = results[0].results[i];
+
+			const pastExecutions = results[0] || { results: [], count: 0, estimated: false };
+
+			for (let i = pastExecutions.results.length - 1; i >= 0; i--) {
+				const currentItem = pastExecutions.results[i];
 				const currentId = parseInt(currentItem.id, 10);
 				if (lastId !== 0 && !isNaN(currentId)) {
 					// We are doing this iteration to detect possible gaps.
@@ -620,8 +630,8 @@ export default defineComponent({
 				(execution) =>
 					!gaps.includes(parseInt(execution.id, 10)) && lastId >= parseInt(execution.id, 10),
 			);
-			this.finishedExecutionsCount = results[0].count;
-			this.finishedExecutionsCountEstimated = results[0].estimated;
+			this.finishedExecutionsCount = pastExecutions.count;
+			this.finishedExecutionsCountEstimated = pastExecutions.estimated;
 
 			Vue.set(this, 'finishedExecutions', alreadyPresentExecutionsFiltered);
 			this.workflowsStore.addToCurrentExecutions(alreadyPresentExecutionsFiltered);
@@ -864,7 +874,7 @@ export default defineComponent({
 					type: 'success',
 				});
 
-				this.refreshData();
+				await this.refreshData();
 			} catch (error) {
 				this.showError(
 					error,
@@ -919,22 +929,25 @@ export default defineComponent({
 				this.selectAllVisibleExecutions();
 			}
 		},
-		startAutoRefreshInterval() {
+		async startAutoRefreshInterval() {
 			if (this.autoRefresh) {
-				this.autoRefreshInterval = setInterval(() => this.loadAutoRefresh(), 4 * 1000); // refresh data every 4 secs
+				await this.loadAutoRefresh();
+				this.autoRefreshTimeout = setTimeout(() => {
+					void this.startAutoRefreshInterval();
+				}, 4 * 1000); // refresh data every 4 secs
 			}
 		},
 		stopAutoRefreshInterval() {
-			if (this.autoRefreshInterval) {
-				clearInterval(this.autoRefreshInterval);
-				this.autoRefreshInterval = undefined;
+			if (this.autoRefreshTimeout) {
+				clearTimeout(this.autoRefreshTimeout);
+				this.autoRefreshTimeout = undefined;
 			}
 		},
 		onDocumentVisibilityChange() {
 			if (document.visibilityState === 'hidden') {
 				this.stopAutoRefreshInterval();
 			} else {
-				this.startAutoRefreshInterval();
+				void this.startAutoRefreshInterval();
 			}
 		},
 	},

diff --git a/packages/editor-ui/src/components/ExecutionsView/ExecutionsList.vue b/packages/editor-ui/src/components/ExecutionsView/ExecutionsList.vue
@@ -5,11 +5,12 @@
 			:loading="loading && !executions.length"
 			:loadingMore="loadingMore"
 			:temporaryExecution="temporaryExecution"
+			:auto-refresh="autoRefresh"
+			@update:autoRefresh="onAutoRefreshToggle"
 			@reloadExecutions="setExecutions"
 			@filterUpdated="onFilterUpdated"
 			@loadMore="onLoadMore"
 			@retryExecution="onRetryExecution"
-			@refresh="loadAutoRefresh"
 		/>
 		<div :class="$style.content" v-if="!hidePreview">
 			<router-view
@@ -83,6 +84,8 @@ export default defineComponent({
 			loadingMore: false,
 			filter: {} as ExecutionFilterType,
 			temporaryExecution: null as IExecutionsSummary | null,
+			autoRefresh: false,
+			autoRefreshTimeout: undefined as undefined | NodeJS.Timer,
 		};
 	},
 	setup() {
@@ -187,8 +190,17 @@ export default defineComponent({
 				await this.setExecutions();
 			}
 		}
+
+		this.autoRefresh = this.uiStore.executionSidebarAutoRefresh === true;
+		this.startAutoRefreshInterval();
+		document.addEventListener('visibilitychange', this.onDocumentVisibilityChange);
+
 		this.loading = false;
 	},
+	beforeDestroy() {
+		this.stopAutoRefreshInterval();
+		document.removeEventListener('visibilitychange', this.onDocumentVisibilityChange);
+	},
 	methods: {
 		async initView(loadWorkflow: boolean): Promise<void> {
 			if (loadWorkflow) {
@@ -325,14 +337,41 @@ export default defineComponent({
 				);
 			}
 		},
-		async onFilterUpdated(filter: ExecutionFilterType): void {
+		async onFilterUpdated(filter: ExecutionFilterType) {
 			this.filter = filter;
 			await this.setExecutions();
 		},
 		async setExecutions(): Promise<void> {
 			this.workflowsStore.currentWorkflowExecutions = await this.loadExecutions();
 			await this.setActiveExecution();
 		},
+
+		async startAutoRefreshInterval() {
+			if (this.autoRefresh) {
+				await this.loadAutoRefresh();
+				this.autoRefreshTimeout = setTimeout(() => this.startAutoRefreshInterval(), 4000);
+			}
+		},
+		stopAutoRefreshInterval() {
+			if (this.autoRefreshTimeout) {
+				clearTimeout(this.autoRefreshTimeout);
+				this.autoRefreshTimeout = undefined;
+			}
+		},
+		onAutoRefreshToggle(value: boolean): void {
+			this.autoRefresh = value;
+			this.uiStore.executionSidebarAutoRefresh = this.autoRefresh;
+
+			this.stopAutoRefreshInterval(); // Clear any previously existing intervals (if any - there shouldn't)
+			this.startAutoRefreshInterval();
+		},
+		onDocumentVisibilityChange() {
+			if (document.visibilityState === 'hidden') {
+				this.stopAutoRefreshInterval();
+			} else {
+				this.startAutoRefreshInterval();
+			}
+		},
 		async loadAutoRefresh(): Promise<void> {
 			// Most of the auto-refresh logic is taken from the `ExecutionsList` component
 			const fetchedExecutions: IExecutionsSummary[] = await this.loadExecutions();

diff --git a/packages/editor-ui/src/components/ExecutionsView/ExecutionsSidebar.vue b/packages/editor-ui/src/components/ExecutionsView/ExecutionsSidebar.vue
@@ -11,8 +11,8 @@
 		</div>
 		<div :class="$style.controls">
 			<el-checkbox
-				v-model="autoRefresh"
-				@change="onAutoRefreshToggle"
+				:value="autoRefresh"
+				@input="$emit('update:autoRefresh', $event)"
 				data-test-id="auto-refresh-checkbox"
 			>
 				{{ $locale.baseText('executionsList.autoRefresh') }}
@@ -39,7 +39,6 @@
 				:ref="`execution-${temporaryExecution.id}`"
 				:data-test-id="`execution-details-${temporaryExecution.id}`"
 				:showGap="true"
-				@refresh="onRefresh"
 				@retryExecution="onRetryExecution"
 			/>
 			<execution-card
@@ -48,7 +47,6 @@
 				:execution="execution"
 				:ref="`execution-${execution.id}`"
 				:data-test-id="`execution-details-${execution.id}`"
-				@refresh="onRefresh"
 				@retryExecution="onRetryExecution"
 			/>
 			<div v-if="loadingMore" class="mr-m">
@@ -85,6 +83,10 @@ export default defineComponent({
 		ExecutionFilter,
 	},
 	props: {
+		autoRefresh: {
+			type: Boolean,
+			default: false,
+		},
 		executions: {
 			type: Array as PropType<IExecutionsSummary[]>,
 			required: true,
@@ -106,8 +108,6 @@ export default defineComponent({
 		return {
 			VIEWS,
 			filter: {} as ExecutionFilterType,
-			autoRefresh: false,
-			autoRefreshInterval: undefined as undefined | NodeJS.Timer,
 		};
 	},
 	computed: {
@@ -126,39 +126,12 @@ export default defineComponent({
 		},
 	},
 	mounted() {
-		this.autoRefresh = this.uiStore.executionSidebarAutoRefresh === true;
-		this.startAutoRefreshInterval();
-
 		// On larger screens, we need to load more then first page of executions
 		// for the scroll bar to appear and infinite scrolling is enabled
 		this.checkListSize();
 		this.scrollToActiveCard();
-
-		document.addEventListener('visibilitychange', this.onDocumentVisibilityChange);
-	},
-	beforeDestroy() {
-		this.stopAutoRefreshInterval();
-		document.removeEventListener('visibilitychange', this.onDocumentVisibilityChange);
 	},
 	methods: {
-		startAutoRefreshInterval() {
-			if (this.autoRefresh) {
-				this.autoRefreshInterval = setInterval(() => this.onRefresh(), 4000);
-			}
-		},
-		stopAutoRefreshInterval() {
-			if (this.autoRefreshInterval) {
-				clearInterval(this.autoRefreshInterval);
-				this.autoRefreshInterval = undefined;
-			}
-		},
-		onDocumentVisibilityChange() {
-			if (document.visibilityState === 'hidden') {
-				this.stopAutoRefreshInterval();
-			} else {
-				this.startAutoRefreshInterval();
-			}
-		},
 		loadMore(limit = 20): void {
 			if (!this.loading) {
 				const executionsListRef = this.$refs.executionList as HTMLElement | undefined;
@@ -184,12 +157,6 @@ export default defineComponent({
 		reloadExecutions(): void {
 			this.$emit('reloadExecutions');
 		},
-		onAutoRefreshToggle(): void {
-			this.uiStore.executionSidebarAutoRefresh = this.autoRefresh;
-
-			this.stopAutoRefreshInterval(); // Clear any previously existing intervals (if any - there shouldn't)
-			this.startAutoRefreshInterval();
-		},
 		checkListSize(): void {
 			const sidebarContainerRef = this.$refs.container as HTMLElement | undefined;
 			const currentExecutionCardRefs = this.$refs[

diff --git a/packages/editor-ui/src/components/__tests__/ExecutionsList.test.ts b/packages/editor-ui/src/components/__tests__/ExecutionsList.test.ts
@@ -13,7 +13,7 @@ import { executionHelpers } from '@/mixins/executionsHelpers';
 import { i18nInstance } from '@/plugins/i18n';
 import type { IWorkflowDb } from '@/Interface';
 import type { IExecutionsSummary } from 'n8n-workflow';
-import { waitAllPromises } from '@/__tests__/utils';
+import { retry, waitAllPromises } from '@/__tests__/utils';
 import { useWorkflowsStore } from '@/stores';
 
 const workflowDataFactory = (): IWorkflowDb => ({
@@ -70,6 +70,9 @@ const renderOptions = {
 			},
 		},
 	}),
+	propsData: {
+		autoRefreshEnabled: false,
+	},
 	i18n: i18nInstance,
 	stubs: ['font-awesome-icon'],
 	mixins: [externalHooks, genericHelpers, executionHelpers],
@@ -134,16 +137,18 @@ describe('ExecutionsList.vue', () => {
 			.mockResolvedValueOnce(executionsData[1]);
 
 		const { getByTestId, getAllByTestId, queryByTestId } = await renderComponent();
-		await userEvent.click(getByTestId('execution-auto-refresh-checkbox'));
+
+		expect(storeSpy).toHaveBeenCalledTimes(1);
 
 		await userEvent.click(getByTestId('select-visible-executions-checkbox'));
 
-		expect(storeSpy).toHaveBeenCalledTimes(1);
-		expect(
-			getAllByTestId('select-execution-checkbox').filter((el) =>
-				el.contains(el.querySelector(':checked')),
-			).length,
-		).toBe(10);
+		await retry(() =>
+			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);