-
Notifications
You must be signed in to change notification settings - Fork 43
feat: shared workspaces support #957
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ import { | |
| } from "./workspace/workspacesProvider"; | ||
|
|
||
| const MY_WORKSPACES_TREE_ID = "myWorkspaces"; | ||
| const SHARED_WORKSPACES_TREE_ID = "sharedWorkspaces"; | ||
| const ALL_WORKSPACES_TREE_ID = "allWorkspaces"; | ||
|
|
||
| export async function activate(ctx: vscode.ExtensionContext): Promise<void> { | ||
|
|
@@ -174,6 +175,19 @@ async function doActivate( | |
| ); | ||
| ctx.subscriptions.push(allWorkspacesProvider); | ||
|
|
||
| const sharedWorkspacesProvider = new WorkspaceProvider( | ||
| WorkspaceQuery.Shared, | ||
| client, | ||
| output, | ||
| isAuthenticated, | ||
| undefined, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to poll for refreshes or keep it like |
||
| // Filter out workspaces owned by the current user. The deployment | ||
| // manager is created below; we capture it via the closure and read it | ||
| // lazily, since the callback only fires when workspaces are fetched. | ||
| () => deploymentManager.getCurrentUserId(), | ||
| ); | ||
| ctx.subscriptions.push(sharedWorkspacesProvider); | ||
|
|
||
| // createTreeView, unlike registerTreeDataProvider, gives us the tree view API | ||
| // (so we can see when it is visible) but otherwise they have the same effect. | ||
| const myWsTree = vscode.window.createTreeView(MY_WORKSPACES_TREE_ID, { | ||
|
|
@@ -189,6 +203,19 @@ async function doActivate( | |
| ctx.subscriptions, | ||
| ); | ||
|
|
||
| const sharedWsTree = vscode.window.createTreeView(SHARED_WORKSPACES_TREE_ID, { | ||
| treeDataProvider: sharedWorkspacesProvider, | ||
| }); | ||
| ctx.subscriptions.push(sharedWsTree); | ||
| sharedWorkspacesProvider.setVisibility(sharedWsTree.visible); | ||
| sharedWsTree.onDidChangeVisibility( | ||
| (event) => { | ||
| sharedWorkspacesProvider.setVisibility(event.visible); | ||
| }, | ||
| undefined, | ||
| ctx.subscriptions, | ||
| ); | ||
|
|
||
| const allWsTree = vscode.window.createTreeView(ALL_WORKSPACES_TREE_ID, { | ||
| treeDataProvider: allWorkspacesProvider, | ||
| }); | ||
|
|
@@ -207,7 +234,7 @@ async function doActivate( | |
| serviceContainer, | ||
| client, | ||
| oauthSessionManager, | ||
| [myWorkspacesProvider, allWorkspacesProvider], | ||
| [myWorkspacesProvider, sharedWorkspacesProvider, allWorkspacesProvider], | ||
| ); | ||
| ctx.subscriptions.push(deploymentManager); | ||
|
|
||
|
|
@@ -313,12 +340,16 @@ async function doActivate( | |
| ); | ||
| commandManager.register("coder.refreshWorkspaces", () => { | ||
| void myWorkspacesProvider.fetchAndRefresh(); | ||
| void sharedWorkspacesProvider.fetchAndRefresh(); | ||
| void allWorkspacesProvider.fetchAndRefresh(); | ||
| }); | ||
| commandManager.register("coder.viewLogs", commands.viewLogs.bind(commands)); | ||
| commandManager.register("coder.searchMyWorkspaces", async () => | ||
| showTreeViewSearch(MY_WORKSPACES_TREE_ID), | ||
| ); | ||
| commandManager.register("coder.searchSharedWorkspaces", async () => | ||
| showTreeViewSearch(SHARED_WORKSPACES_TREE_ID), | ||
| ); | ||
| commandManager.register("coder.searchAllWorkspaces", async () => | ||
| showTreeViewSearch(ALL_WORKSPACES_TREE_ID), | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,11 @@ import { type Logger } from "../logging/logger"; | |
| export enum WorkspaceQuery { | ||
| Mine = "owner:me", | ||
| All = "", | ||
| // Shared returns workspaces the user has access to via sharing but does not | ||
| // own. The server-side `shared:true` filter also includes workspaces the | ||
| // user owns and has shared out, so the provider filters those out | ||
| // client-side using the current user's id. | ||
|
Comment on lines
+26
to
+29
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agents tend to over explain with what and not why. I'd trim this a bit more (same for all of the new comments here) |
||
| Shared = "shared:true", | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -53,6 +58,10 @@ export class WorkspaceProvider | |
| private readonly logger: Logger, | ||
| private readonly isAuthenticated: () => boolean, | ||
| private readonly timerSeconds?: number, | ||
| // Returns the id of the currently authenticated user. Used by the Shared | ||
| // query to filter out workspaces owned by the current user. | ||
| private readonly getCurrentUserId: () => string | undefined = () => | ||
| undefined, | ||
|
Comment on lines
+63
to
+64
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather decouple this, like maybe pass |
||
| ) { | ||
| // No initialization. | ||
| } | ||
|
|
@@ -112,6 +121,18 @@ export class WorkspaceProvider | |
| q: this.getWorkspacesQuery, | ||
| }); | ||
|
|
||
| // `shared:true` also matches workspaces the current user shared out; | ||
| // keep only the ones owned by someone else. | ||
| let workspaces = resp.workspaces; | ||
| if (this.getWorkspacesQuery === WorkspaceQuery.Shared) { | ||
| const currentUserId = this.getCurrentUserId(); | ||
| if (currentUserId) { | ||
| workspaces = workspaces.filter( | ||
| (workspace) => workspace.owner_id !== currentUserId, | ||
| ); | ||
| } | ||
| } | ||
|
Comment on lines
+126
to
+134
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we make this a filter then we can just make it a |
||
|
|
||
| // We could have logged out while waiting for the query, or logged into a | ||
| // different deployment. | ||
| const url2 = this.client.getAxiosInstance().defaults.baseURL; | ||
|
|
@@ -133,7 +154,7 @@ export class WorkspaceProvider | |
| // have this separate map held outside the tree. | ||
| const showMetadata = this.getWorkspacesQuery === WorkspaceQuery.Mine; | ||
| if (showMetadata) { | ||
| const agents = extractAllAgents(resp.workspaces); | ||
| const agents = extractAllAgents(workspaces); | ||
| for (const agent of agents) { | ||
| // If we have an existing watcher, re-use it. | ||
| const oldWatcher = this.agentWatchers.get(agent.id); | ||
|
|
@@ -159,11 +180,15 @@ export class WorkspaceProvider | |
| } | ||
| } | ||
|
|
||
| // Show the owner alongside the workspace name when the list may contain | ||
| // workspaces owned by other users. | ||
| const showOwner = this.getWorkspacesQuery !== WorkspaceQuery.Mine; | ||
|
|
||
| // Create tree items for each workspace | ||
| const workspaceTreeItems = resp.workspaces.map((workspace: Workspace) => { | ||
| const workspaceTreeItems = workspaces.map((workspace: Workspace) => { | ||
| const workspaceTreeItem = new WorkspaceTreeItem( | ||
| workspace, | ||
| this.getWorkspacesQuery === WorkspaceQuery.All, | ||
| showOwner, | ||
| showMetadata, | ||
| ); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe call it
#authedUser: User | nullso it's more explicit, we can keep the derivedgetCurrentUserId