From 063a46ad4ae2aeb4af2c31b9e908d85b54dcefed Mon Sep 17 00:00:00 2001
From: wendrul <53628737+wendrul@users.noreply.github.com>
Date: Mon, 24 Jun 2024 15:24:25 +0200
Subject: [PATCH] fix: Improve cancel selected jobs action (#3960)

* Improve cancel selected action

* Add a `Select all` button
* Make the confirmation clearer by adding
* Fix bug where you could select uncancelable jobs throught the graph

* Switch buttons place
---
 frontend/src/lib/components/RunChart.svelte   | 10 +-
 .../src/lib/components/runs/RunsTable.svelte  | 93 ++++++++++++------
 .../(logged)/runs/[...path]/+page.svelte      | 94 ++++++++++---------
 3 files changed, 121 insertions(+), 76 deletions(-)

diff --git a/frontend/src/lib/components/RunChart.svelte b/frontend/src/lib/components/RunChart.svelte
index 47453689ab..097f2aa3c9 100644
--- a/frontend/src/lib/components/RunChart.svelte
+++ b/frontend/src/lib/components/RunChart.svelte
@@ -23,6 +23,7 @@
 	export let minTimeSet: string | undefined = undefined
 	export let maxTimeSet: string | undefined = undefined
 	export let selectedIds: string[] = []
+	export let canSelect: boolean = true
 
 	const dispatch = createEventDispatcher()
 	const SUCCESS_COLOR = '#4ade80'
@@ -142,7 +143,7 @@
 	}
 
 	function highlightSelectedPoints(ids: string[]) {
-		if (ids.length === 0) {
+		if (!canSelect || ids.length === 0) {
 			data.datasets[0].backgroundColor = FAIL_COLOR
 			data.datasets[1].backgroundColor = SUCCESS_COLOR
 		} else {
@@ -237,8 +238,10 @@
 			}
 		},
 		onClick: (e, u) => {
-			const ids = u.map((j) => data.datasets[j.datasetIndex].data[j.index].id)
-			selectedIds = ids
+			if (canSelect) {
+				const ids = u.map((j) => data.datasets[j.datasetIndex].data[j.index].id)
+				selectedIds = ids
+			}
 		},
 
 		scales: {
@@ -265,7 +268,6 @@
 	} as any
 
 	$: data && scatterOptions && highlightSelectedPoints(selectedIds)
-
 </script>
 
 <!-- {JSON.stringify(minTime)}
diff --git a/frontend/src/lib/components/runs/RunsTable.svelte b/frontend/src/lib/components/runs/RunsTable.svelte
index 9f979cfa05..6a39d36fd8 100644
--- a/frontend/src/lib/components/runs/RunsTable.svelte
+++ b/frontend/src/lib/components/runs/RunsTable.svelte
@@ -7,6 +7,7 @@
 	import { AlertTriangle } from 'lucide-svelte'
 	import Popover from '../Popover.svelte'
 	import { workspaceStore } from '$lib/stores'
+	import { twMerge } from 'tailwind-merge'
 	//import InfiniteLoading from 'svelte-infinite-loading'
 
 	export let jobs: Job[] | undefined = undefined
@@ -136,6 +137,24 @@
 		}
 	}
 	*/
+	function isJobCancelable(j: Job): boolean {
+		return j.type === 'QueuedJob' && !j.schedule_path
+	}
+
+	let allSelected: boolean = false
+
+	function selectAll() {
+		if (allSelected) {
+			allSelected = false
+			selectedIds = []
+		} else {
+			allSelected = true
+			selectedIds = jobs?.filter(isJobCancelable).map((j) => j.id) ?? []
+		}
+	}
+	let cancelableJobCount: number = 0
+	$: isSelectingJobsToCancel && (allSelected = selectedIds.length === cancelableJobCount)
+	$: isSelectingJobsToCancel && (cancelableJobCount = jobs?.filter(isJobCancelable).length ?? 0)
 
 	function jobCountString(jobCount: number) {
 		const jc = jobCount
@@ -160,38 +179,54 @@
 	id="runs-table-wrapper"
 	bind:clientWidth={containerWidth}
 >
-	<div
-		class="flex flex-row bg-surface-secondary sticky top-0 w-full p-2 pr-4"
-		bind:clientHeight={header}
-	>
-		{#if showExternalJobs && externalJobs.length > 0}
-			<div class="w-1/12 text-2xs">
-				<div class="flex flex-row">
-					{jobs && jobCountString(jobs.length + externalJobs.length)}<Tooltip
-						>{externalJobs.length} jobs obscured</Tooltip
-					>
+	<div bind:clientHeight={header}>
+		{#if isSelectingJobsToCancel && cancelableJobCount != 0}
+			<!-- svelte-ignore a11y-click-events-have-key-events -->
+			<!-- svelte-ignore a11y-no-static-element-interactions -->
+			<div
+				class={twMerge(
+					'hover:bg-surface-hover bg-surface-primary cursor-pointer',
+					allSelected ? 'bg-blue-50 dark:bg-blue-900/50' : '',
+					'flex flex-row items-center sticky w-full p-2 pr-4 top-0 font-semibold border-t text-sm'
+				)}
+				on:click={selectAll}
+			>
+				<div class="px-2">
+					<input on:focus type="checkbox" checked={allSelected} />
 				</div>
+				Select all
 			</div>
-		{:else if $workspaceStore !== 'admins' && omittedObscuredJobs}
-			<div class="w-1/12 text-2xs flex flex-row">
-				{jobs && jobCountString(jobs.length)}
-				<Popover>
-					<AlertTriangle size={16} class="ml-0.5 text-yellow-500" />
-					<svelte:fragment slot="text">
-						Too specific filtering may have caused the omission of obscured jobs. This is done for
-						security reasons. To see obscured jobs, try removing some filters.
-					</svelte:fragment>
-				</Popover>
-			</div>
-		{:else}
-			<div class="w-1/12 text-2xs">{jobs && jobCountString(jobs.length)}</div>
 		{/if}
-		<div class="w-4/12 text-xs font-semibold">Timestamp</div>
-		<div class="w-4/12 text-xs font-semibold">Path</div>
-		{#if containsLabel}
-			<div class="w-3/12 text-xs font-semibold">Label</div>
-		{/if}
-		<div class="w-3/12 text-xs font-semibold">Triggered by</div>
+		<div class="flex flex-row bg-surface-secondary sticky top-0 w-full p-2 pr-4">
+			{#if showExternalJobs && externalJobs.length > 0}
+				<div class="w-1/12 text-2xs">
+					<div class="flex flex-row">
+						{jobs && jobCountString(jobs.length + externalJobs.length)}<Tooltip
+							>{externalJobs.length} jobs obscured</Tooltip
+						>
+					</div>
+				</div>
+			{:else if $workspaceStore !== 'admins' && omittedObscuredJobs}
+				<div class="w-1/12 text-2xs flex flex-row">
+					{jobs && jobCountString(jobs.length)}
+					<Popover>
+						<AlertTriangle size={16} class="ml-0.5 text-yellow-500" />
+						<svelte:fragment slot="text">
+							Too specific filtering may have caused the omission of obscured jobs. This is done for
+							security reasons. To see obscured jobs, try removing some filters.
+						</svelte:fragment>
+					</Popover>
+				</div>
+			{:else}
+				<div class="w-1/12 text-2xs">{jobs && jobCountString(jobs.length)}</div>
+			{/if}
+			<div class="w-4/12 text-xs font-semibold">Timestamp</div>
+			<div class="w-4/12 text-xs font-semibold">Path</div>
+			{#if containsLabel}
+				<div class="w-3/12 text-xs font-semibold">Label</div>
+			{/if}
+			<div class="w-3/12 text-xs font-semibold">Triggered by</div>
+		</div>
 	</div>
 
 	<VirtualList
diff --git a/frontend/src/routes/(root)/(logged)/runs/[...path]/+page.svelte b/frontend/src/routes/(root)/(logged)/runs/[...path]/+page.svelte
index d548dbc644..5049fc6af6 100644
--- a/frontend/src/routes/(root)/(logged)/runs/[...path]/+page.svelte
+++ b/frontend/src/routes/(root)/(logged)/runs/[...path]/+page.svelte
@@ -340,37 +340,35 @@
 	async function cancelVisibleJobs() {
 		isSelectingJobsToCancel = true
 		selectedIds = jobs?.filter(isJobCancelable).map((j) => j.id) ?? []
-		if (selectedIds.length === 0 ) {
-			sendUserToast("There are no visible jobs that can be canceled", true)
+		if (selectedIds.length === 0) {
+			sendUserToast('There are no visible jobs that can be canceled', true)
 		}
 	}
 	async function cancelFilteredJobs() {
 		isCancelingFilteredJobs = true
 		fetchingFilteredJobs = true
 		const selectedFilters = {
-				workspace: $workspaceStore ?? '',
-				startedBefore: maxTs,
-				startedAfter: minTs,
-				schedulePath,
-				scriptPathExact: path === null || path === '' ? undefined : path,
-				createdBy: user === null || user === '' ? undefined : user,
-				scriptPathStart: folder === null || folder === '' ? undefined : `f/${folder}/`,
-				jobKinds,
-				success: success == 'success' ? true : success == 'failure' ? false : undefined,
-				running: success == 'running' ? true : undefined,
-				isNotSchedule: showSchedules == false ? true : undefined,
-				scheduledForBeforeNow: showFutureJobs == false ? true : undefined,
-				args:
-					argFilter && argFilter != '{}' && argFilter != '' && argError == ''
-						? argFilter
-						: undefined,
-				result:
-					resultFilter && resultFilter != '{}' && resultFilter != '' && resultError == ''
-						? resultFilter
-						: undefined,
-				allWorkspaces: allWorkspaces ? true : undefined,
-				concurrencyKey: concurrencyKey ?? undefined
-			}
+			workspace: $workspaceStore ?? '',
+			startedBefore: maxTs,
+			startedAfter: minTs,
+			schedulePath,
+			scriptPathExact: path === null || path === '' ? undefined : path,
+			createdBy: user === null || user === '' ? undefined : user,
+			scriptPathStart: folder === null || folder === '' ? undefined : `f/${folder}/`,
+			jobKinds,
+			success: success == 'success' ? true : success == 'failure' ? false : undefined,
+			running: success == 'running' ? true : undefined,
+			isNotSchedule: showSchedules == false ? true : undefined,
+			scheduledForBeforeNow: showFutureJobs == false ? true : undefined,
+			args:
+				argFilter && argFilter != '{}' && argFilter != '' && argError == '' ? argFilter : undefined,
+			result:
+				resultFilter && resultFilter != '{}' && resultFilter != '' && resultError == ''
+					? resultFilter
+					: undefined,
+			allWorkspaces: allWorkspaces ? true : undefined,
+			concurrencyKey: concurrencyKey ?? undefined
+		}
 
 		selectedFiltersString = JSON.stringify(selectedFilters, null, 4)
 		jobIdsToCancel = await JobService.listFilteredUuids(selectedFilters)
@@ -386,6 +384,11 @@
 		return j.type === 'QueuedJob' && !j.schedule_path
 	}
 
+	function jobCountString(count: number) {
+
+		return `${count} ${count == 1 ? 'job': 'jobs'}`
+	}
+
 	const warnJobLimitMsg =
 		'The exact number of concurrent job at the beginning of the time range may be incorrect as only the last 1000 jobs are taken into account: a job that was started earlier than this limit will not be taken into account'
 
@@ -394,9 +397,6 @@
 		extendedJobs !== undefined &&
 		extendedJobs.jobs.length + extendedJobs.obscured_jobs.length >= 1000
 
-	$: if (selectedIds.length === 0) {
-		isSelectingJobsToCancel = false
-	}
 </script>
 
 <JobLoader
@@ -454,7 +454,7 @@
 </ConfirmationModal>
 
 <ConfirmationModal
-	title={`Confirm cancelling the jobs visible on this page`}
+	title={`Confirm cancelling the selected jobs`}
 	confirmationText={`Cancel ${jobIdsToCancel.length} jobs`}
 	open={isCancelingVisibleJobs}
 	on:confirmed={async () => {
@@ -556,6 +556,7 @@
 			{#if graph === 'RunChart'}
 				<RunChart
 					bind:selectedIds
+					canSelect={!isSelectingJobsToCancel}
 					minTimeSet={minTs}
 					maxTimeSet={maxTs}
 					maxIsNow={maxTs == undefined}
@@ -586,13 +587,6 @@
 				<div class="flex flex-row">
 					{#if isSelectingJobsToCancel}
 						<div class="mt-1 p-2 h-8 flex flex-row items-center gap-1">
-							<Button
-								startIcon={{ icon: Check }}
-								size="xs"
-								color="red"
-								variant="contained"
-								on:click={cancelSelectedJobs}
-							/>
 							<Button
 								startIcon={{ icon: X }}
 								size="xs"
@@ -603,6 +597,16 @@
 									selectedIds = []
 								}}
 							/>
+							<Button
+								disabled={selectedIds.length == 0}
+								startIcon={{ icon: Check }}
+								size="xs"
+								color="red"
+								variant="contained"
+								on:click={cancelSelectedJobs}
+							>
+								Cancel {jobCountString(selectedIds.length)}
+							</Button>
 						</div>
 					{:else if !$userStore?.is_admin && !$superadmin}
 						<DropdownV2
@@ -839,6 +843,7 @@
 			{#if graph === 'RunChart'}
 				<RunChart
 					bind:selectedIds
+					canSelect={!isSelectingJobsToCancel}
 					minTimeSet={minTs}
 					maxTimeSet={maxTs}
 					maxIsNow={maxTs == undefined}
@@ -871,13 +876,6 @@
 				<div class="flex flex-row">
 					{#if isSelectingJobsToCancel}
 						<div class="mt-1 p-2 h-8 flex flex-row items-center gap-1">
-							<Button
-								startIcon={{ icon: Check }}
-								size="xs"
-								color="red"
-								variant="contained"
-								on:click={cancelSelectedJobs}
-							/>
 							<Button
 								startIcon={{ icon: X }}
 								size="xs"
@@ -888,6 +886,16 @@
 									selectedIds = []
 								}}
 							/>
+							<Button
+								disabled={selectedIds.length == 0}
+								startIcon={{ icon: Check }}
+								size="xs"
+								color="red"
+								variant="contained"
+								on:click={cancelSelectedJobs}
+							>
+								Cancel {jobCountString(selectedIds.length)}
+							</Button>
 						</div>
 					{:else if !$userStore?.is_admin && !$superadmin}
 						<DropdownV2