Skip to content

feat: add shared filter to workspaces query#19807

Merged
brettkolodny merged 5 commits into
mainfrom
brett/i972
Sep 16, 2025
Merged

feat: add shared filter to workspaces query#19807
brettkolodny merged 5 commits into
mainfrom
brett/i972

Conversation

@brettkolodny

@brettkolodny brettkolodny commented Sep 12, 2025

Copy link
Copy Markdown
Contributor

Adds a shared:<boolean> search query to the /workspaces [get] endpoint

CleanShot.2025-09-15.at.11.13.25.mp4

Closes coder/internal#972

@brettkolodny brettkolodny marked this pull request as ready for review September 15, 2025 15:15

@aslilac aslilac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great! could we add some tests that actually hit the database tho?

@aslilac aslilac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these tests are actually testing much right now, since asRequestOption didn't get wired up but they're still passing. 😅

nvm, I didn't see that they're actually failing

Comment thread codersdk/workspaces.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. WorkspaceFilter has an asRequestOption method that also needs to be updated. that's where the q param actually gets serialized.

  2. this field probably needs to be *bool so that you can set shared:false as distinct from "no shared: filter"

Comment thread codersdk/workspaces.go Outdated
Comment on lines 519 to 522

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think this being last was intentional since it doesn't quite "fit in", and that should be preserved

@brettkolodny

Copy link
Copy Markdown
Contributor Author

I don't think these tests are actually testing much right now, since asRequestOption didn't get wired up but they're still passing. 😅

nvm, I didn't see that they're actually failing

Yeah I'm still working on it, I'll re-request review when it's ready

@brettkolodny brettkolodny marked this pull request as draft September 15, 2025 21:20
@brettkolodny brettkolodny force-pushed the brett/i972 branch 3 times, most recently from c0dc8e8 to bf4b351 Compare September 15, 2025 21:48
@brettkolodny brettkolodny marked this pull request as ready for review September 15, 2025 23:06
Comment thread coderd/workspaces_test.go
Comment on lines +1846 to +1850
workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{
Shared: ptr.Ref(true),
})
require.NoError(t, err, "fetch workspaces")
require.Equal(t, 1, workspaces.Count, "expected only one workspace")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since there's only one workspace here, could we also run this with Shared: ptr.Ref(false) and check the length to be 0?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am creating two workspaces if you look at the setup

Comment on lines +3619 to +3624
workspaces, err := client.Workspaces(ctx, codersdk.WorkspaceFilter{
Shared: ptr.Ref(true),
})
require.NoError(t, err, "fetch workspaces")
require.Equal(t, 1, workspaces.Count, "expected only one workspace")
require.Equal(t, workspaces.Workspaces[0].ID, sharedWorkspace.ID)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and perhaps the same thing here for the groups case

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, a second workspace is being created but not shared

@brettkolodny brettkolodny merged commit e6b04d1 into main Sep 16, 2025
48 of 50 checks passed
@brettkolodny brettkolodny deleted the brett/i972 branch September 16, 2025 16:37
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add shared: filter to /workspaces endpoint

2 participants