feat: filter tasks that are waiting for user input by BrunoQuaresma · Pull Request #19377 · coder/coder

BrunoQuaresma · 2025-08-15T17:56:27Z

Closes #19324

johnstcn · 2025-08-18T07:34:01Z

+// TODO: This is a temporary solution while the BE does not return the Task in a
+// right shape with a custom name. This should be removed once the BE is fixed.


coder/internal#904

johnstcn

LGTM, but deferring approval to FE domain experts.

aslilac · 2025-08-18T17:25:03Z

+	value?: string;
+};
+
+function useUsersOptions({ query, value }: UseUsersOptionsOptions) {


this doesn't really need to be a hook imo. it should be a query function like we generally use, and it can take user as an argument.

I know, but IMO, it encapsulates the intention quite well and improve the readability. It makes easier for me to know where and for what things are being used for. Eg. It is clear for me for what the useQuery and usesAuthenticated are being used for.

Unfortunately, you can't use hooks inside of regular functions, but I'm happy to change the code to use any existent convention.

the entirety of this hook except for const { user } = useAuthenticated(); can be encapsulated in a query options function tho! and that better matches how we do it everywhere else.

Unfortunately, you can't use hooks inside of regular functions

and this is part of why I am so against custom hooks in 90% of use cases. they're viral function coloring, and should be avoided unless they are actually necessary to encapsulate stateful logic. I don't think that's the case here, as all of the logic is independent of the state.

I think the new React docs on custom hooks are very insightful. Even they don't think people should really be making custom hooks most of the time!

https://react.dev/learn/reusing-logic-with-custom-hooks#there-is-more-than-one-way-to-do-it

aslilac · 2025-08-18T17:31:39Z

+export const data = {
+	async createTask(
+		prompt: string,
+		userId: string,
+		templateVersionId: string,
+		presetId: string | undefined,
+	): Promise<Task> {
+		const workspace = await API.experimental.createTask(userId, {
+			name: `task-${generateWorkspaceName()}`,
+			template_version_id: templateVersionId,
+			template_version_preset_id: presetId,
+			prompt,
+		});
+
+		return {
+			workspace,
+			prompt,
+		};
+	},
+};


I don't understand the need for this data wrapper. why not just export the function directly?

Because of some spyOn limitations 😢. I'm not able to make it work when doing the following:

import * as data from "./data" spyOn(data, "createTask")

Cannot spy on export "createTask". Module namespace is not configurable in ESM. See: https://vitest.dev/guide/browser/#limitations

But it should be temporary, since the create task API should handle that.

aslilac · 2025-08-18T17:33:07Z

+	};
+
+	const sortSelectedFirst = (a: User) =>
+		value && a.username === value ? -1 : 0;


this doesn't hand the case where a.username === a.username

Not sure if I got it 🤔

I can't even blame you, writing sorting functions is really unintuitive. 😅

if a.username === a.username and a.username === value you'll still return -1 when you should return 0. they're both the selected user, so you shouldn't be preferring one over the other. although I guess saying it out loud, that case probably won't ever come up except maybe in tests that don't generate unique usernames.

Would it be ok if we let this as it is?

Co-authored-by: ケイラ <[email protected]>

aslilac · 2025-08-20T15:26:43Z

+		const [firstTask, ...otherTasks] = MockTasks;
+		spyOn(API, "getTemplates").mockResolvedValue([MockTemplate]);
+		spyOn(API.experimental, "getTasks").mockResolvedValue([
+			{
+				...firstTask,
+				workspace: {
+					...firstTask.workspace,
+					latest_app_status: {
+						...firstTask.workspace.latest_app_status,
+						state: "idle" as const,
+					},
+				},
+			},
+			...otherTasks,
+		]);


this might be a nice opportunity to use structuredClone to mask some noise:

Suggested change

const [firstTask, ...otherTasks] = MockTasks;

spyOn(API, "getTemplates").mockResolvedValue([MockTemplate]);

spyOn(API.experimental, "getTasks").mockResolvedValue([

{

...firstTask,

workspace: {

...firstTask.workspace,

latest_app_status: {

...firstTask.workspace.latest_app_status,

state: "idle" as const,

},

},

},

...otherTasks,

]);

spyOn(API, "getTemplates").mockResolvedValue([MockTemplate]);

const tasks = structuredClone(MockTasks);

tasks[0].workspace.latest_app_status.state = "idle";

spyOn(API.experimental, "getTasks").mockResolvedValue(tasks);

There is an issue with that, structuredClone keep the read only structure causing an TS error.

Cannot assign to 'state' because it is a read-only property.

aslilac · 2025-08-20T15:31:07Z

+		spyOn(API.experimental, "getTasks").mockResolvedValue([
+			{
+				...firstTask,
+				workspace: {
+					...firstTask.workspace,
+					latest_app_status: {
+						...firstTask.workspace.latest_app_status,
+						state: "idle" as const,
+					},
 				},
-				// biome-ignore lint/suspicious/noExplicitAny: Testing malformed data edge cases
-			] as any;
+			},
+			...otherTasks,
+		]);


all the same applies here

aslilac · 2025-08-20T15:33:40Z

-		);
-	}
-
+const PillButton = ({ className, active, ...props }: PillButtonProps) => {


FC?

also, seems like a good candidate for a general component. might be worth moving to components/.

seems like a good candidate for a general component.

Wdyt about waiting for more use cases using it?

aslilac · 2025-08-20T15:40:28Z

+	};
+
+	const sortSelectedFirst = (a: User) =>
+		value && a.username === value ? -1 : 0;


I can't even blame you, writing sorting functions is really unintuitive. 😅

if a.username === a.username and a.username === value you'll still return -1 when you should return 0. they're both the selected user, so you shouldn't be preferring one over the other. although I guess saying it out loud, that case probably won't ever come up except maybe in tests that don't generate unique usernames.

Co-authored-by: ケイラ <[email protected]>

BrunoQuaresma · 2025-08-20T17:55:00Z

@aslilac hooks refactored and other requests added. ✅

johnstcn

LGTM pending FE approval.

BrunoQuaresma added 6 commits August 13, 2025 19:43

chore: cleanup and organize code

b5ffb82

Merge branch 'main' of https://github.com/coder/coder into bq/19324

d502175

chore: fix biome settings for VSCode

d92ee0f

chore: user search params for user filtering

d3053e9

feat: add tabs

b4c861b

test: add stories

177f4b2

BrunoQuaresma requested a review from a team August 15, 2025 17:56

BrunoQuaresma self-assigned this Aug 15, 2025

BrunoQuaresma requested review from aqandrew and removed request for a team August 15, 2025 17:56

BrunoQuaresma requested a review from aslilac as a code owner August 15, 2025 17:56

BrunoQuaresma added 2 commits August 15, 2025 17:58

chore: fix fmt

061d4da

Merge branch 'main' of https://github.com/coder/coder into bq/19324

ab0e12c

johnstcn reviewed Aug 18, 2025

View reviewed changes

aslilac reviewed Aug 18, 2025

View reviewed changes

Bruno Quaresma and others added 2 commits August 19, 2025 08:54

Apply suggestions from code review

ed7d3b3

Co-authored-by: ケイラ <[email protected]>

Apply review suggestions

c93f279

BrunoQuaresma requested review from aslilac and removed request for aqandrew August 19, 2025 12:17

Bruno Quaresma and others added 3 commits August 19, 2025 09:17

Update site/src/pages/TasksPage/TasksPage.tsx

baf98e4

Co-authored-by: ケイラ <[email protected]>

Fix lint

654cbbf

Fix lint

bffefd5

aslilac reviewed Aug 20, 2025

View reviewed changes

Bruno Quaresma and others added 4 commits August 20, 2025 14:35

Update site/src/pages/TasksPage/TasksPage.stories.tsx

2a1f139

Co-authored-by: ケイラ <[email protected]>

Update site/src/pages/TasksPage/TasksTable.tsx

9ec150d

Co-authored-by: ケイラ <[email protected]>

Update site/src/api/api.ts

bdace80

Co-authored-by: ケイラ <[email protected]>

Apply review comments

1d3dc1c

Show tasks not being shown

cd31514

Merge branch 'main' of https://github.com/coder/coder into bq/19324

be423d4

johnstcn approved these changes Aug 21, 2025

View reviewed changes

BrunoQuaresma mentioned this pull request Aug 21, 2025

feat: display the number of idle tasks in the navbar #19471

Merged

Merge branch 'main' of https://github.com/coder/coder into bq/19324

d04abc5

BrunoQuaresma merged commit d77c3d0 into main Aug 21, 2025

BrunoQuaresma deleted the bq/19324 branch August 21, 2025 18:06

github-actions Bot locked and limited conversation to collaborators Aug 21, 2025

		// TODO: This is a temporary solution while the BE does not return the Task in a
		// right shape with a custom name. This should be removed once the BE is fixed.

Conversation

BrunoQuaresma commented Aug 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrunoQuaresma Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BrunoQuaresma commented Aug 20, 2025

Uh oh!

johnstcn left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BrunoQuaresma Aug 19, 2025 •

edited

Loading