feat: add extra workspace actions in the workspaces table #17775

BrunoQuaresma · 2025-05-12T19:55:01Z

Demo:

bcpeinhardt · 2025-05-13T16:44:37Z

site/src/modules/workspaces/WorkspaceMoreActions/DownloadLogsDialog.tsx

@@ -221,8 +221,9 @@ const DownloadingItem: FC<DownloadingItemProps> = ({ file, giveUpTimeMs }) => {
 function humanBlobSize(size: number) {
 	const BLOB_SIZE_UNITS = ["B", "KB", "MB", "GB", "TB"] as const;
 	let i = 0;
-	while (size > 1024 && i < BLOB_SIZE_UNITS.length) {
-		size /= 1024;
+	let sizeIterator = size;


I'm not the biggest fan of changing parameters either, but in this case, this would never be a bug, since numbers are primitive values

I have to say it was a Biome catch haha.

Parkreiner

Nothing major, but I do think there are some bugs and edge cases we need to address. I also left some comments about things that I think we really should figure out pretty soon (even if they don't happen in this PR)

Parkreiner · 2025-05-13T15:40:59Z

site/src/api/api.ts

@@ -482,10 +482,10 @@ class ApiMethods {
 		return response.data;
 	};

-	checkAuthorization = async (
+	checkAuthorization = async <TResponse extends TypesGen.AuthorizationResponse>(


I'm concerned about why we have a generic for a function that checks anything auth-based. Does the server really give back multiple return types based on what parameters we give it?

Also, I'm a little concerned that with how the function is set up right now, we force anyone calling this function to pass in the correct type parameter. To some degree that's okay, since we have such a specific type constraint on here, I have to wonder if this is asking for trouble, not just as far as complexity, but also in terms of some other employee passing the wrong type in a few months from now

Two things come to mind:

I'm wondering if there's a way to redefine params so that we can infer the return type automatically. With less need to pass in types manually, there's less room for things to go wrong

I'm also wondering if it's worth updating the generic definition to TResponse extends TypesGen.AuthorizationResponse = TypesGen.AuthorizationResponse. That way, most users get back the base AuthorizationResponse type by default (which I'd imagine would be okay 99% of the time, since we haven't needed this constraint until now), and then if someone needs something more specific, they have the ability to pass in the type, and override the default return value

That is not possible right now since the response is dynamic and depends on the request parameters. However, I do agree it would be better to have something more predictable to avoid these kinda of errors.

Parkreiner · 2025-05-13T15:52:08Z

site/src/api/queries/deployment.ts

@@ -6,6 +6,7 @@ export const deploymentConfig = () => {
 	return {
 		queryKey: deploymentConfigQueryKey,
 		queryFn: API.getDeploymentConfig,
+		staleTime: Number.POSITIVE_INFINITY,


Just making sure: are we guaranteed that the config won't ever change after the initial request from the server? I know we already disable a lot of the RQ background refetch functionality, so I'm wondering if there's any harm in letting the data be marked as stale, since I'd think the data would still be refetched relatively infrequently

Yeap! To change these deployment configs even coderd would need to be restarted 😄

Parkreiner · 2025-05-13T15:55:48Z

site/src/api/queries/workspaces.ts

@@ -390,3 +395,14 @@ export const workspaceUsage = (options: WorkspaceUsageOptions) => {
 		refetchIntervalInBackground: true,
 	};
 };
+
+export const workspacePermissions = (workspace?: Workspace) => {


Could we add an explicit return type here? There's enough going on that I think it'd help communicate what this is supposed to be

I don't see too much value on that since it is dynamic and other parts of the application would throw an error. But, if it is a must for you, I can do it for sure.

Parkreiner · 2025-05-13T16:00:27Z

site/src/modules/workspaces/WorkspaceMoreActions/ChangeWorkspaceVersionDialog.tsx

-	defaultTemplateVersion,
-	...dialogProps
-}) => {
+export const ChangeWorkspaceVersionDialog: FC<


I don't think it's worth blocking on this, but I think it's worth thinking about for the future: do we want to limit the number of versions we show by default? Right now, we're loading everything, which makes the UI chug for a second, and I think that realistically, most users aren't going to be switching to anything that's more than 10 versions older than the most recent one

We have a lot of versions from the past two years, and I could see some customers having more than us, just because their companies are so much bigger

I totally agree with you, but I have to say it is not on the scope for this PR. Feel free to open an issue tho

Parkreiner · 2025-05-13T16:02:26Z

site/src/modules/workspaces/WorkspaceMoreActions/ChangeWorkspaceVersionDialog.stories.tsx

+export const ShortMessage: Story = {
+	args: {
+		workspace: {
+			...MockWorkspace,
+			template_active_version_id: MockTemplateVersion.id,
+		},
+	},
+};
+
+export const LongMessage: Story = {
+	args: {
+		workspace: {
+			...MockWorkspace,
+			template_active_version_id: MockTemplateVersionWithMarkdownMessage.id,
+		},
+	},
+};


Could we rename these stories? When I'm looking at them in the list of stories, it's not clear why they're different, especially when the only input that changed is a template ID

Maybe something like PlaintextMessage and MarkdownMessage?

Sure, just be aware it is not in the scope of this PR. It was just a renaming.

Parkreiner · 2025-05-13T17:01:49Z

site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceMoreActions.tsx

+						disabled={disabled}
+					>
+						<EllipsisVertical aria-hidden="true" />
+						<span className="sr-only">Workspace actions</span>


Appreciate the screenreader support text!

Parkreiner · 2025-05-13T17:07:05Z

site/src/modules/workspaces/WorkspaceMoreActions/WorkspaceMoreActions.tsx

+};
+
+const isMissingBuildParameters = (e: unknown): e is MissingBuildParameters => {
+	return Boolean(e && e instanceof MissingBuildParameters);


I don't think we need this function. We can get rid of the Boolean(e &&) part, because:

instanceof works with null and undefined values no problem – they'd just always evaluate to false

instanceof always evaluates to a boolean

So this can be cleaned up to

const isMissingBuildParameters = (e: unknown): e is MissingBuildParameters => { return e instanceof MissingBuildParameters; };

And that point, I think we can get away with just using the instanceof operator directly. It still has support for type-narrowing, but unlike a custom predicate function, there's no risk of getting the predicate logic wrong, and accidentally lying to the type system

That is true 👍

Parkreiner · 2025-05-13T17:12:59Z

site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx

-		{ canDebug, isOwner },
+		{
+			canDebug: !!deployment?.config.enable_terraform_debug_mode,
+			isOwner: !!user.roles.find((role) => role.name === "owner"),


This can be changed to user.roles.some((role) => role.name === "owner"), which is a little more clear, and will have less memory usage

Parkreiner · 2025-05-13T17:16:15Z

site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.tsx

@@ -50,14 +50,13 @@ const WorkspaceParametersPage: FC = () => {
 	const template = templateQuery.data;

 	// Permissions
-	const checks =
-		workspace && template ? workspaceChecks(workspace, template) : {};
+	const checks = workspace && template ? workspaceChecks(workspace) : {};


If we're only using the workspace to define the checks, do we still need template to be part of the ternary?

Not at all!

One less query 👏

Parkreiner · 2025-05-13T17:23:04Z

site/src/pages/WorkspacesPage/WorkspacesTable.tsx

+		<TableCell
+			onClick={(e) => {
+				// Prevent the click in the actions to trigger the row click
+				e.stopPropagation();
+			}}
+		>


This is something that I was going to bring up in the comment for submitting the review, but think this is a better place. I really think that we need to go back to the drawing board on the UI, because I don't think the way it works is obvious anymore:

This is what I mean:

For a new user, I don't think the row is very clear with these changes. The little arrow at least gave the user a place to click (even though the whole card was valid), but now, they have to just know that clicking the background will "magically" do something

Copying the comments from the image, just in case the image ever gets deleted down the line:

Red areas are interactive [referring to the checkbox on the left, as well as all the agent actions + MoreActions button on the right]

The logos for the template and user look exactly the same as the interactive buttons on the right, but have zero interactivity

You have to know to click the "background" of a card (which has no visual indicators that it's interactive, especially since this PR removes the arrow) to go to the Workspace Details page

As a bandaid, since it seems like we're pushing users to do more operations from the workspaces table row rather than the whole details page, I'm wondering if we could remove the link on the row entirely, and add a new option to the MoreActions menu

And to be clear, I'm okay with addressing that in a separate PR, but I think this PR is degrading the UX in some deceptively big ways. Even though it's a minor aesthetic change, it's affecting one of the biggest user flows in the entire product

I think this PR is degrading the UX in some deceptively big ways.

I kinda disagree with that, even agreeing with your points. Until we have data, it is just an opinion (I think Christin put some thoughts on that, so I'm kinda comfortable to move forward), so we only can verify that when ppl start using it.

IMO, the link should be in the workspace name only as GitHub does with issues table, and many other products. I'm against clickable rows in general because things like this.

…tions-workspaces

Applied critical changes

BrunoQuaresma · 2025-05-13T21:54:39Z

@Parkreiner I think I solved the most critical issues you pointed out, so I'm merging the PR, but please, let me know if I forgot anything or if you want to keep the discussions, I'm totally open for it.

BrunoQuaresma added 9 commits May 12, 2025 17:57

Refactor base menu component

496473f

Refactor change workspace version

22759e3

Refactor delete workspace

2f3cce0

Refactor duplicate and clean up code

f7e9ba9

Move components and fix build errors

a3c4908

Minor visual fixes

d5ae7e1

Fix debug check

0c65d4f

Add more actions to workspaces row

3b5f20b

Minor fixes

207094a

BrunoQuaresma requested a review from a team May 12, 2025 19:55

BrunoQuaresma self-assigned this May 12, 2025

BrunoQuaresma requested review from bcpeinhardt and removed request for a team May 12, 2025 19:55

BrunoQuaresma added 3 commits May 12, 2025 22:07

Run FMT

c79952b

Fix lint and tests

be967dd

Fix stories

139548c

bcpeinhardt approved these changes May 13, 2025

View reviewed changes

bcpeinhardt reviewed May 13, 2025

View reviewed changes

Parkreiner previously requested changes May 13, 2025

View reviewed changes

BrunoQuaresma requested a review from Parkreiner May 13, 2025 20:03

BrunoQuaresma added 2 commits May 13, 2025 20:49

Apply PR reviews

84b65a8

Merge branch 'main' of https://github.com/coder/coder into bq/more-ac…

96a6ffd

…tions-workspaces

Fix stories

62413e0

BrunoQuaresma merged commit 67e4024 into main May 13, 2025
34 checks passed

BrunoQuaresma deleted the bq/more-actions-workspaces branch May 13, 2025 21:53

github-actions bot locked and limited conversation to collaborators May 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add extra workspace actions in the workspaces table #17775

feat: add extra workspace actions in the workspaces table #17775

BrunoQuaresma commented May 12, 2025

bcpeinhardt May 13, 2025

Parkreiner May 13, 2025 •

edited

Loading

BrunoQuaresma May 13, 2025

Parkreiner left a comment

Parkreiner May 13, 2025

Parkreiner May 13, 2025

BrunoQuaresma May 13, 2025

Parkreiner May 13, 2025

BrunoQuaresma May 13, 2025

Parkreiner May 13, 2025

BrunoQuaresma May 13, 2025

Parkreiner May 13, 2025

BrunoQuaresma May 13, 2025

Parkreiner May 13, 2025

BrunoQuaresma May 13, 2025

Parkreiner May 13, 2025

Parkreiner May 13, 2025 •

edited

Loading

BrunoQuaresma May 13, 2025

Parkreiner May 13, 2025

Parkreiner May 13, 2025

BrunoQuaresma May 13, 2025

BrunoQuaresma May 13, 2025

Parkreiner May 13, 2025

Parkreiner May 13, 2025

Parkreiner May 13, 2025 •

edited

Loading

BrunoQuaresma May 13, 2025 •

edited

Loading

BrunoQuaresma commented May 13, 2025

feat: add extra workspace actions in the workspaces table #17775

feat: add extra workspace actions in the workspaces table #17775

Conversation

BrunoQuaresma commented May 12, 2025

Choose a reason for hiding this comment

Parkreiner May 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkreiner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkreiner May 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkreiner May 13, 2025 • edited Loading

Choose a reason for hiding this comment

BrunoQuaresma May 13, 2025 • edited Loading

Choose a reason for hiding this comment

BrunoQuaresma commented May 13, 2025

Parkreiner May 13, 2025 •

edited

Loading

Parkreiner May 13, 2025 •

edited

Loading

Parkreiner May 13, 2025 •

edited

Loading

BrunoQuaresma May 13, 2025 •

edited

Loading