Apply PR reviews

coder · BrunoQuaresma · Feb 18, 2025 · Feb 5, 2025 · Feb 7, 2025 · Feb 7, 2025
commit 59539607bc6cd52c9cd3747c8b4cda674e2aa157
diff --git a/site/src/api/api.ts b/site/src/api/api.ts
@@ -1238,7 +1238,7 @@ class ApiMethods {
 	};
 
 	cancelTemplateVersionBuild = async (
-		templateVersionId: TypesGen.TemplateVersion["id"],
+		templateVersionId: string,
 	): Promise<TypesGen.Response> => {
 		const response = await this.axios.patch(
 			`/api/v2/templateversions/${templateVersionId}/cancel`,
@@ -1248,7 +1248,7 @@ class ApiMethods {
 	};
 
 	cancelTemplateVersionDryRun = async (
-		templateVersionId: TypesGen.TemplateVersion["id"],
+		templateVersionId: string,
 		jobId: string,
 	): Promise<TypesGen.Response> => {
 		const response = await this.axios.patch(

diff --git a/site/src/pages/OrganizationSettingsPage/ProvisionersPage/CancelJobButton.tsx b/site/src/pages/OrganizationSettingsPage/ProvisionersPage/CancelJobButton.tsx
@@ -1,6 +1,5 @@
 import type { ProvisionerJob } from "api/typesGenerated";
 import { Button } from "components/Button/Button";
-import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog";
 import {
 	Tooltip,
 	TooltipContent,
@@ -9,22 +8,25 @@ import {
 } from "components/Tooltip/Tooltip";
 import { BanIcon } from "lucide-react";
 import { type FC, useState } from "react";
+import { CancelJobConfirmationDialog } from "./CancelJobConfirmationDialog";
+
+const CANCELLABLE = ["pending", "running"];
 
 type CancelJobButtonProps = {
 	job: ProvisionerJob;
 };
 
 export const CancelJobButton: FC<CancelJobButtonProps> = ({ job }) => {
 	const [isDialogOpen, setIsDialogOpen] = useState(false);
-	const cancellable = ["pending", "running"].includes(job.status);
+	const isCancellable = CANCELLABLE.includes(job.status);
 
 	return (
 		<>
 			<TooltipProvider>
 				<Tooltip>
 					<TooltipTrigger asChild>
 						<Button
-							disabled={!cancellable}
+							disabled={!isCancellable}
 							aria-label="Cancel job"
 							size="icon"
 							variant="outline"
@@ -39,16 +41,12 @@ export const CancelJobButton: FC<CancelJobButtonProps> = ({ job }) => {
 				</Tooltip>
 			</TooltipProvider>
 
-			<ConfirmDialog
-				type="delete"
-				onClose={(): void => {
+			<CancelJobConfirmationDialog
+				open={isDialogOpen}
+				job={job}
+				onClose={() => {
 					setIsDialogOpen(false);
 				}}
-				open={isDialogOpen}
-				title="Cancel provisioner job"
-				description={`Are you sure you want to cancel the provisioner job "${job.id}"? This operation will result in the associated workspaces not getting created.`}
-				confirmText="Confirm"
-				cancelText="Discard"
 			/>
 		</>
 	);

diff --git a/site/src/pages/OrganizationSettingsPage/ProvisionersPage/CancelJobConfirmationDialog.tsx b/site/src/pages/OrganizationSettingsPage/ProvisionersPage/CancelJobConfirmationDialog.tsx
@@ -4,26 +4,16 @@ import {
 	provisionerJobQueryKey,
 } from "api/queries/organizations";
 import type { ProvisionerJob } from "api/typesGenerated";
-import {
-	ConfirmDialog,
-	type ConfirmDialogProps,
-} from "components/Dialogs/ConfirmDialog/ConfirmDialog";
+import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog";
 import { displayError, displaySuccess } from "components/GlobalSnackbar/utils";
 import type { FC } from "react";
 import { useMutation, useQueryClient } from "react-query";
 
-type CancelJobConfirmationDialogProps = Omit<
-	ConfirmDialogProps,
-	| "type"
-	| "title"
-	| "description"
-	| "confirmText"
-	| "cancelText"
-	| "onConfirm"
-	| "confirmLoading"
-> & {
+type CancelJobConfirmationDialogProps = {
+	open: boolean;
+	onClose: () => void;
 	job: ProvisionerJob;
-	cancelProvisionerJob: typeof API.cancelProvisionerJob;
+	cancelProvisionerJob?: typeof API.cancelProvisionerJob;
 };
 
 export const CancelJobConfirmationDialog: FC<

diff --git a/site/src/pages/OrganizationSettingsPage/ProvisionersPage/DataGrid.tsx b/site/src/pages/OrganizationSettingsPage/ProvisionersPage/DataGrid.tsx
@@ -1,17 +1,16 @@
 import type { FC, HTMLProps } from "react";
 import { cn } from "utils/cn";
 
-export const DataGrid: FC<HTMLProps<HTMLDivElement>> = ({
+export const DataGrid: FC<HTMLProps<HTMLDListElement>> = ({
 	className,
 	...props
 }) => {
 	return (
-		<div
+		<dl
 			{...props}
 			className={cn([
-				"grid grid-cols-[auto_1fr] gap-x-4 items-center",
-				"[&_span:nth-of-type(even)]:text-content-primary [&_span:nth-of-type(even)]:font-mono",
-				"[&_span:nth-of-type(even)]:leading-[22px]",
+				"m-0 grid grid-cols-[auto_1fr] gap-x-4 items-center",
+				"[&_dt]:text-content-primary [&_dt]:font-mono [&_dt]:leading-[22px]",
 				className,
 			])}
 		/>
@@ -22,7 +21,5 @@ export const DataGridSpace: FC<HTMLProps<HTMLDivElement>> = ({
 	className,
 	...props
 }) => {
-	return (
-		<div aria-hidden {...props} className={cn(["h-6 col-span-2", className])} />
-	);
+	return <div {...props} className={cn(["h-6 col-span-2", className])} />;
 };
diff --git a/site/src/pages/OrganizationSettingsPage/ProvisionersPage/JobStatusIndicator.tsx b/site/src/pages/OrganizationSettingsPage/ProvisionersPage/JobStatusIndicator.tsx
@@ -11,40 +11,50 @@ import {
 import { TriangleAlertIcon } from "lucide-react";
 import type { FC } from "react";
 
+const variantByStatus: Record<
+	ProvisionerJobStatus,
+	StatusIndicatorProps["variant"]
+> = {
+	succeeded: "success",
+	failed: "failed",
+	pending: "pending",
+	running: "pending",
+	canceling: "pending",
+	canceled: "inactive",
+	unknown: "inactive",
+};
+
 type JobStatusIndicatorProps = {
-	job: ProvisionerJob | ProvisionerDaemonJob;
+	job: ProvisionerJob;
 };
 
 export const JobStatusIndicator: FC<JobStatusIndicatorProps> = ({ job }) => {
-	const isProvisionerJob = "queue_position" in job;
 	return (
-		<StatusIndicator size="sm" variant={statusIndicatorVariant(job.status)}>
+		<StatusIndicator size="sm" variant={variantByStatus[job.status]}>
 			<StatusIndicatorDot />
 			<span className="[&:first-letter]:uppercase">{job.status}</span>
 			{job.status === "failed" && (
 				<TriangleAlertIcon className="size-icon-xs p-[1px]" />
 			)}
-			{job.status === "pending" &&
-				isProvisionerJob &&
-				`(${job.queue_position}/${job.queue_size})`}
+			{job.status === "pending" && `(${job.queue_position}/${job.queue_size})`}
 		</StatusIndicator>
 	);
 };
 
-function statusIndicatorVariant(
-	status: ProvisionerJobStatus,
-): StatusIndicatorProps["variant"] {
-	switch (status) {
-		case "succeeded":
-			return "success";
-		case "failed":
-			return "failed";
-		case "pending":
-		case "running":
-		case "canceling":
-			return "pending";
-		case "canceled":
-		case "unknown":
-			return "inactive";
-	}
-}
+type DaemonJobStatusIndicatorProps = {
+	job: ProvisionerDaemonJob;
+};
+
+export const DaemonJobStatusIndicator: FC<DaemonJobStatusIndicatorProps> = ({
+	job,
+}) => {
+	return (
+		<StatusIndicator size="sm" variant={variantByStatus[job.status]}>
+			<StatusIndicatorDot />
+			<span className="[&:first-letter]:uppercase">{job.status}</span>
+			{job.status === "failed" && (
+				<TriangleAlertIcon className="size-icon-xs p-[1px]" />
+			)}
+		</StatusIndicator>
+	);
+};
diff --git a/site/src/pages/OrganizationSettingsPage/ProvisionersPage/ProvisionerDaemonsPage.tsx b/site/src/pages/OrganizationSettingsPage/ProvisionersPage/ProvisionerDaemonsPage.tsx
@@ -24,20 +24,26 @@ import { cn } from "utils/cn";
 import { docs } from "utils/docs";
 import { relativeTime } from "utils/time";
 import { DataGrid, DataGridSpace } from "./DataGrid";
-import { JobStatusIndicator } from "./JobStatusIndicator";
-import { ShrinkTags, Tag, Tags } from "./Tags";
+import { DaemonJobStatusIndicator } from "./JobStatusIndicator";
+import { TruncateTags, Tag, Tags } from "./Tags";
+import { Button } from "components/Button/Button";
 
 type ProvisionerDaemonsPageProps = {
-	org: Organization;
+	orgId: string;
 };
 
 export const ProvisionerDaemonsPage: FC<ProvisionerDaemonsPageProps> = ({
-	org,
+	orgId,
 }) => {
-	const { data: daemons, isLoadingError } = useQuery({
-		...provisionerDaemons(org.id),
+	const {
+		data: daemons,
+		isLoadingError,
+		refetch,
+	} = useQuery({
+		...provisionerDaemons(orgId),
 		select: (data) =>
 			data.toSorted((a, b) => {
+				if (!a.last_seen_at && !b.last_seen_at) return 0;
 				if (!a.last_seen_at) return 1;
 				if (!b.last_seen_at) return -1;
 				return (
@@ -49,6 +55,7 @@ export const ProvisionerDaemonsPage: FC<ProvisionerDaemonsPageProps> = ({
 
 	return (
 		<section className="flex flex-col gap-8">
+			<h2 className="sr-only">Provisioner daemons</h2>
 			<p className="text-sm text-content-secondary m-0 mt-2">
 				Coder server runs provisioner daemons which execute terraform during
 				workspace and template builds.{" "}
@@ -85,7 +92,10 @@ export const ProvisionerDaemonsPage: FC<ProvisionerDaemonsPageProps> = ({
 					) : isLoadingError ? (
 						<TableRow>
 							<TableCell colSpan={999}>
-								<EmptyState message="Error loading the provisioner daemons" />
+								<EmptyState
+									message="Error loading the provisioner daemons"
+									cta={<Button onClick={() => refetch()}>Retry</Button>}
+								/>
 							</TableCell>
 						</TableRow>
 					) : (
@@ -128,6 +138,7 @@ const DaemonRow: FC<DaemonRowProps> = ({ daemon }) => {
 						) : (
 							<ChevronRightIcon className="size-icon-sm p-0.5" />
 						)}
+						<span className="sr-only">({isOpen ? "Hide" : "Show more"})</span>
 						<span className="[&:first-letter]:uppercase">
 							{relativeTime(
 								new Date(daemon.last_seen_at ?? new Date().toISOString()),
@@ -159,7 +170,7 @@ const DaemonRow: FC<DaemonRowProps> = ({ daemon }) => {
 					)}
 				</TableCell>
 				<TableCell>
-					<ShrinkTags tags={daemon.tags} />
+					<TruncateTags tags={daemon.tags} />
 				</TableCell>
 				<TableCell>
 					<StatusIndicator size="sm" variant={statusIndicatorVariant(daemon)}>
@@ -175,49 +186,49 @@ const DaemonRow: FC<DaemonRowProps> = ({ daemon }) => {
 				<TableRow>
 					<TableCell colSpan={999} className="p-4 border-t-0">
 						<DataGrid>
-							<span>Last seen:</span>
-							<span>{daemon.last_seen_at}</span>
+							<dt>Last seen:</dt>
+							<dd>{daemon.last_seen_at}</dd>
 
-							<span>Creation time:</span>
-							<span>{daemon.created_at}</span>
+							<dt>Creation time:</dt>
+							<dd>{daemon.created_at}</dd>
 
-							<span>Version:</span>
-							<span>{daemon.version}</span>
+							<dt>Version:</dt>
+							<dd>{daemon.version}</dd>
 
-							<span>Tags:</span>
-							<span>
+							<dt>Tags:</dt>
+							<dd>
 								<Tags>
 									{Object.entries(daemon.tags).map(([key, value]) => (
 										<Tag key={key} label={key} value={value} />
 									))}
 								</Tags>
-							</span>
+							</dd>
 
 							{daemon.current_job && (
 								<>
 									<DataGridSpace />
 
-									<span>Last job:</span>
-									<span>{daemon.current_job.id}</span>
+									<dt>Last job:</dt>
+									<dd>{daemon.current_job.id}</dd>
 
-									<span>Last job state:</span>
-									<span>
-										<JobStatusIndicator job={daemon.current_job} />
-									</span>
+									<dt>Last job state:</dt>
+									<dd>
+										<DaemonJobStatusIndicator job={daemon.current_job} />
+									</dd>
 								</>
 							)}
 
 							{daemon.previous_job && (
 								<>
 									<DataGridSpace />
 
-									<span>Previous job:</span>
-									<span>{daemon.previous_job.id}</span>
+									<dt>Previous job:</dt>
+									<dd>{daemon.previous_job.id}</dd>
 
-									<span>Previous job state:</span>
-									<span>
-										<JobStatusIndicator job={daemon.previous_job} />
-									</span>
+									<dt>Previous job state:</dt>
+									<dd>
+										<DaemonJobStatusIndicator job={daemon.previous_job} />
+									</dd>
 								</>
 							)}
 						</DataGrid>
@@ -240,8 +251,7 @@ function statusIndicatorVariant(
 			return "success";
 		case "busy":
 			return "pending";
-		case "offline":
-		case null:
+		default:
 			return "inactive";
 	}
 }
@@ -258,7 +268,7 @@ function statusLabel(daemon: ProvisionerDaemon) {
 			return "Busy...";
 		case "offline":
 			return "Disconnected";
-		case null:
+		default:
 			return "Unknown";
 	}
 }