fix: Remove unused workspace routes in favor of list with filter

This consolidates the workspace routes into a single place. It allows users to fetch a workspace by their username and workspace name, which will be used by the frontend for routing.
coder · kylecarbs · Jun 3, 2022 · Jun 3, 2022 · Jun 3, 2022 · Jun 3, 2022
commit fb60891b89e9cb758264fc847a311b5ce7942bcd
diff --git a/cli/configssh.go b/cli/configssh.go
@@ -48,10 +48,6 @@ func configSSH() *cobra.Command {
 			if err != nil {
 				return err
 			}
-			organization, err := currentOrganization(cmd, client)
-			if err != nil {
-				return err
-			}
 			if strings.HasPrefix(sshConfigFile, "~/") {
 				dirname, _ := os.UserHomeDir()
 				sshConfigFile = filepath.Join(dirname, sshConfigFile[2:])
@@ -65,7 +61,9 @@ func configSSH() *cobra.Command {
 				sshConfigContent = sshConfigContent[:startIndex-1] + sshConfigContent[endIndex+len(sshEndToken):]
 			}
 
-			workspaces, err := client.WorkspacesByOwner(cmd.Context(), organization.ID, codersdk.Me)
+			workspaces, err := client.Workspaces(cmd.Context(), codersdk.WorkspaceFilter{
+				Owner: codersdk.Me,
+			})
 			if err != nil {
 				return err
 			}

diff --git a/cli/server.go b/cli/server.go
@@ -412,11 +412,9 @@ func server() *cobra.Command {
 					"Interrupt caught, gracefully exiting.  Use ctrl+\\ to force quit"))
 
 			if dev {
-				organizations, err := client.OrganizationsByUser(cmd.Context(), codersdk.Me)
-				if err != nil {
-					return xerrors.Errorf("get organizations: %w", err)
-				}
-				workspaces, err := client.WorkspacesByOwner(cmd.Context(), organizations[0].ID, codersdk.Me)
+				workspaces, err := client.Workspaces(cmd.Context(), codersdk.WorkspaceFilter{
+					Owner: codersdk.Me,
+				})
 				if err != nil {
 					return xerrors.Errorf("get workspaces: %w", err)
 				}

diff --git a/cli/ssh.go b/cli/ssh.go
@@ -194,7 +194,9 @@ func getWorkspaceAndAgent(cmd *cobra.Command, client *codersdk.Client, orgID uui
 		err            error
 	)
 	if shuffle {
-		workspaces, err := client.WorkspacesByOwner(cmd.Context(), orgID, userID)
+		workspaces, err := client.Workspaces(cmd.Context(), codersdk.WorkspaceFilter{
+			Owner: codersdk.Me,
+		})
 		if err != nil {
 			return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, err
 		}

diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -154,7 +154,6 @@ func New(options *Options) *API {
 				})
 				r.Route("/workspaces", func(r chi.Router) {
 					r.Post("/", api.postWorkspacesByOrganization)
-					r.Get("/", api.workspacesByOrganization)
 					r.Route("/{user}", func(r chi.Router) {
 						r.Use(httpmw.ExtractUserParam(options.Database))
 						r.Get("/{workspacename}", api.workspaceByOwnerAndName)
@@ -259,6 +258,7 @@ func New(options *Options) *API {
 						r.Get("/", api.organizationsByUser)
 						r.Get("/{organizationname}", api.organizationByUserAndName)
 					})
+					r.Get("/workspace/{workspacename}", api.workspaceByOwnerAndName)
 					r.Get("/gitsshkey", api.gitSSHKey)
 					r.Put("/gitsshkey", api.regenerateGitSSHKey)
 				})

diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -100,35 +100,6 @@ func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
 	httpapi.Write(rw, http.StatusOK, convertWorkspace(workspace, build, job, template, owner))
 }
 
-func (api *API) workspacesByOrganization(rw http.ResponseWriter, r *http.Request) {
-	organization := httpmw.OrganizationParam(r)
-	workspaces, err := api.Database.GetWorkspacesWithFilter(r.Context(), database.GetWorkspacesWithFilterParams{
-		OrganizationID: organization.ID,
-		Deleted:        false,
-	})
-	if errors.Is(err, sql.ErrNoRows) {
-		err = nil
-	}
-	if err != nil {
-		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("get workspaces: %s", err),
-		})
-		return
-	}
-
-	// Rbac filter
-	workspaces = AuthorizeFilter(api, r, rbac.ActionRead, workspaces)
-
-	apiWorkspaces, err := convertWorkspaces(r.Context(), api.Database, workspaces)
-	if err != nil {
-		httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
-			Message: fmt.Sprintf("convert workspaces: %s", err),
-		})
-		return
-	}
-	httpapi.Write(rw, http.StatusOK, apiWorkspaces)
-}
-
 // workspaces returns all workspaces a user can read.
 // Optional filters with query params
 func (api *API) workspaces(rw http.ResponseWriter, r *http.Request) {
@@ -223,7 +194,6 @@ func (api *API) workspacesByOwner(rw http.ResponseWriter, r *http.Request) {
 
 func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request) {
 	owner := httpmw.UserParam(r)
-	organization := httpmw.OrganizationParam(r)
 	workspaceName := chi.URLParam(r, "workspacename")
 
 	workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{
@@ -241,14 +211,6 @@ func (api *API) workspaceByOwnerAndName(rw http.ResponseWriter, r *http.Request)
 		})
 		return
 	}
-
-	if workspace.OrganizationID != organization.ID {
-		httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
-			Message: fmt.Sprintf("workspace is not owned by organization %q", organization.Name),
-		})
-		return
-	}
-
 	if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
 		return
 	}

diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -210,73 +210,12 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
 	})
 }
 
-func TestWorkspacesByOrganization(t *testing.T) {
-	t.Parallel()
-	t.Run("ListEmpty", func(t *testing.T) {
-		t.Parallel()
-		client := coderdtest.New(t, nil)
-		user := coderdtest.CreateFirstUser(t, client)
-		_, err := client.WorkspacesByOrganization(context.Background(), user.OrganizationID)
-		require.NoError(t, err)
-	})
-    	t.Run("List", func(t *testing.T) {
-		t.Parallel()
-		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
-		user := coderdtest.CreateFirstUser(t, client)
-		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
-		coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
-		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
-		ws := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
-		_ = coderdtest.AwaitWorkspaceBuildJob(t, client, ws.LatestBuild.ID)
-		workspaces, err := client.WorkspacesByOrganization(context.Background(), user.OrganizationID)
-		require.NoError(t, err)
-		require.Len(t, workspaces, 1)
-	})
-}
-
-func TestWorkspacesByOwner(t *testing.T) {
-	t.Parallel()
-	t.Run("ListEmpty", func(t *testing.T) {
-		t.Parallel()
-		client := coderdtest.New(t, nil)
-		user := coderdtest.CreateFirstUser(t, client)
-		_, err := client.WorkspacesByOwner(context.Background(), user.OrganizationID, codersdk.Me)
-		require.NoError(t, err)
-	})
-
-	t.Run("ListMine", func(t *testing.T) {
-		t.Parallel()
-		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
-		user := coderdtest.CreateFirstUser(t, client)
-		me, err := client.User(context.Background(), codersdk.Me)
-		require.NoError(t, err)
-
-		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
-		coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
-		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
-		_ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
-
-		// Create noise workspace that should be filtered out
-		other := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
-		_ = coderdtest.CreateWorkspace(t, other, user.OrganizationID, template.ID)
-
-		// Use a username
-		workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{
-			OrganizationID: user.OrganizationID,
-			Owner:          me.Username,
-		})
-		require.NoError(t, err)
-		require.Len(t, workspaces, 1)
-	})
-}
-
 func TestWorkspaceByOwnerAndName(t *testing.T) {
 	t.Parallel()
 	t.Run("NotFound", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
-		user := coderdtest.CreateFirstUser(t, client)
-		_, err := client.WorkspaceByOwnerAndName(context.Background(), user.OrganizationID, codersdk.Me, "something")
+		_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, "something")
 		var apiErr *codersdk.Error
 		require.ErrorAs(t, err, &apiErr)
 		require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
@@ -289,7 +228,7 @@ func TestWorkspaceByOwnerAndName(t *testing.T) {
 		coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
 		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
 		workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
-		_, err := client.WorkspaceByOwnerAndName(context.Background(), user.OrganizationID, codersdk.Me, workspace.Name)
+		_, err := client.WorkspaceByOwnerAndName(context.Background(), codersdk.Me, workspace.Name)
 		require.NoError(t, err)
 	})
 }
@@ -409,7 +348,9 @@ func TestPostWorkspaceBuild(t *testing.T) {
 		require.Equal(t, workspace.LatestBuild.BuildNumber+1, build.BuildNumber)
 		coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
 
-		workspaces, err := client.WorkspacesByOwner(context.Background(), user.OrganizationID, user.UserID.String())
+		workspaces, err := client.Workspaces(context.Background(), codersdk.WorkspaceFilter{
+			Owner: user.UserID.String(),
+		})
 		require.NoError(t, err)
 		require.Len(t, workspaces, 0)
 	})

diff --git a/codersdk/organizations.go b/codersdk/organizations.go
@@ -200,51 +200,3 @@ func (c *Client) CreateWorkspace(ctx context.Context, organizationID uuid.UUID,
 	var workspace Workspace
 	return workspace, json.NewDecoder(res.Body).Decode(&workspace)
 }
-
-// WorkspacesByOrganization returns all workspaces in the specified organization.
-func (c *Client) WorkspacesByOrganization(ctx context.Context, organizationID uuid.UUID) ([]Workspace, error) {
-	res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/workspaces", organizationID), nil)
-	if err != nil {
-		return nil, err
-	}
-	defer res.Body.Close()
-
-	if res.StatusCode != http.StatusOK {
-		return nil, readBodyAsError(res)
-	}
-
-	var workspaces []Workspace
-	return workspaces, json.NewDecoder(res.Body).Decode(&workspaces)
-}
-
-// WorkspacesByOwner returns all workspaces contained in the organization owned by the user.
-func (c *Client) WorkspacesByOwner(ctx context.Context, organizationID uuid.UUID, user string) ([]Workspace, error) {
-	res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/workspaces/%s", organizationID, user), nil)
-	if err != nil {
-		return nil, err
-	}
-	defer res.Body.Close()
-
-	if res.StatusCode != http.StatusOK {
-		return nil, readBodyAsError(res)
-	}
-
-	var workspaces []Workspace
-	return workspaces, json.NewDecoder(res.Body).Decode(&workspaces)
-}
-
-// WorkspaceByOwnerAndName returns a workspace by the owner's UUID and the workspace's name.
-func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, organization uuid.UUID, owner string, name string) (Workspace, error) {
-	res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/workspaces/%s/%s", organization, owner, name), nil)
-	if err != nil {
-		return Workspace{}, err
-	}
-	defer res.Body.Close()
-
-	if res.StatusCode != http.StatusOK {
-		return Workspace{}, readBodyAsError(res)
-	}
-
-	var workspace Workspace
-	return workspace, json.NewDecoder(res.Body).Decode(&workspace)
-}
diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go
@@ -234,3 +234,19 @@ func (c *Client) Workspaces(ctx context.Context, filter WorkspaceFilter) ([]Work
 	var workspaces []Workspace
 	return workspaces, json.NewDecoder(res.Body).Decode(&workspaces)
 }
+
+// WorkspaceByOwnerAndName returns a workspace by the owner's UUID and the workspace's name.
+func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name string) (Workspace, error) {
+	res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/users/%s/workspace/%s", owner, name), nil)
+	if err != nil {
+		return Workspace{}, err
+	}
+	defer res.Body.Close()
+
+	if res.StatusCode != http.StatusOK {
+		return Workspace{}, readBodyAsError(res)
+	}
+
+	var workspace Workspace
+	return workspace, json.NewDecoder(res.Body).Decode(&workspace)
+}
@@ -136,13 +136,10 @@ export const getWorkspaces = async (filter?: TypesGen.WorkspaceFilter): Promise<
 }
 
 export const getWorkspaceByOwnerAndName = async (
-  organizationID: string,
   username = "me",
   workspaceName: string,
 ): Promise<TypesGen.Workspace> => {
-  const response = await axios.get<TypesGen.Workspace>(
-    `/api/v2/organizations/${organizationID}/workspaces/${username}/${workspaceName}`,
-  )
+  const response = await axios.get<TypesGen.Workspace>(`/api/v2/users/${username}/workspace/${workspaceName}`)
   return response.data
 }
 

@@ -54,25 +54,10 @@ describe("TerminalPage", () => {
     history.push(`/some-user/${MockWorkspace.name}/terminal`)
   })
 
-  it("shows an error if fetching organizations fails", async () => {
-    // Given
-    server.use(
-      rest.get("/api/v2/users/me/organizations", async (req, res, ctx) => {
-        return res(ctx.status(500), ctx.json({ message: "nope" }))
-      }),
-    )
-
-    // When
-    const { container } = renderTerminal()
-
-    // Then
-    await expectTerminalText(container, Language.organizationsErrorMessagePrefix)
-  })
-
   it("shows an error if fetching workspace fails", async () => {
     // Given
     server.use(
-      rest.get("/api/v2/organizations/:organizationId/workspaces/:userName/:workspaceName", (req, res, ctx) => {
+      rest.get("/api/v2/users/:userName/workspace/:workspaceName", (req, res, ctx) => {
         return res(ctx.status(500), ctx.json({ id: "workspace-id" }))
       }),
     )

@@ -11,7 +11,6 @@ import { MONOSPACE_FONT_FAMILY } from "../../theme/constants"
 import { terminalMachine } from "../../xServices/terminal/terminalXService"
 
 export const Language = {
-  organizationsErrorMessagePrefix: "Unable to fetch organizations: ",
   workspaceErrorMessagePrefix: "Unable to fetch workspace: ",
   workspaceAgentErrorMessagePrefix: "Unable to fetch workspace agent: ",
   websocketErrorMessagePrefix: "WebSocket failed: ",
@@ -58,8 +57,7 @@ const TerminalPage: FC<{
   })
   const isConnected = terminalState.matches("connected")
   const isDisconnected = terminalState.matches("disconnected")
-  const { organizationsError, workspaceError, workspaceAgentError, workspaceAgent, websocketError } =
-    terminalState.context
+  const { workspaceError, workspaceAgentError, workspaceAgent, websocketError } = terminalState.context
 
   // Create the terminal!
   useEffect(() => {
@@ -145,9 +143,6 @@ const TerminalPage: FC<{
       terminal.options = {
         disableStdin: true,
       }
-      if (organizationsError instanceof Error) {
-        terminal.writeln(Language.organizationsErrorMessagePrefix + organizationsError.message)
-      }
       if (workspaceError instanceof Error) {
         terminal.writeln(Language.workspaceErrorMessagePrefix + workspaceError.message)
       }
@@ -180,17 +175,7 @@ const TerminalPage: FC<{
         width: terminal.cols,
       },
     })
-  }, [
-    workspaceError,
-    organizationsError,
-    workspaceAgentError,
-    websocketError,
-    workspaceAgent,
-    terminal,
-    fitAddon,
-    isConnected,
-    sendEvent,
-  ])
+  }, [workspaceError, workspaceAgentError, websocketError, workspaceAgent, terminal, fitAddon, isConnected, sendEvent])
 
   return (
     <>