From a2be7c02940667c1cc84640fd06b290c959520a9 Mon Sep 17 00:00:00 2001 From: Presley Pizzo <1290996+presleyp@users.noreply.github.com> Date: Fri, 6 May 2022 10:02:17 -0400 Subject: [PATCH] fix: create and read workspace page (#1294) * Change name of existing workspace call * Add new api call (has handler already) * WorkspacesPage -> WorkspacePage * starting to replace swr * Add other api calls * Fix api call * Replace swr with xstate * Format * Test - wip * Fix route in template page * Fix endpoint in create workspace * Fix tests * Lint --- site/src/AppRouter.tsx | 2 +- site/src/api/index.ts | 19 ++- site/src/api/types.ts | 1 + site/src/forms/CreateWorkspaceForm.test.tsx | 11 +- site/src/forms/CreateWorkspaceForm.tsx | 9 +- .../TemplatePage/CreateWorkspacePage.tsx | 14 +- .../TemplatePage/TemplatePage.tsx | 5 +- .../WorkspacePage/WorkspacePage.test.tsx | 14 ++ .../src/pages/WorkspacePage/WorkspacePage.tsx | 42 ++++++ .../pages/WorkspacesPage/WorkspacesPage.tsx | 54 ------- site/src/testHelpers/index.tsx | 13 +- site/src/xServices/StateContext.tsx | 3 + .../xServices/terminal/terminalXService.ts | 2 +- .../xServices/workspace/workspaceXService.ts | 138 ++++++++++++++++++ 14 files changed, 261 insertions(+), 66 deletions(-) create mode 100644 site/src/pages/WorkspacePage/WorkspacePage.test.tsx create mode 100644 site/src/pages/WorkspacePage/WorkspacePage.tsx delete mode 100644 site/src/pages/WorkspacesPage/WorkspacesPage.tsx create mode 100644 site/src/xServices/workspace/workspaceXService.ts diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index af9f7826bc..ed3d299c97 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -19,7 +19,7 @@ import { TemplatePage } from "./pages/TemplatesPages/OrganizationPage/TemplatePa import { TemplatesPage } from "./pages/TemplatesPages/TemplatesPage" import { CreateUserPage } from "./pages/UsersPage/CreateUserPage/CreateUserPage" import { UsersPage } from "./pages/UsersPage/UsersPage" -import { WorkspacePage } from "./pages/WorkspacesPage/WorkspacesPage" +import { WorkspacePage } from "./pages/WorkspacePage/WorkspacePage" const TerminalPage = React.lazy(() => import("./pages/TerminalPage/TerminalPage")) diff --git a/site/src/api/index.ts b/site/src/api/index.ts index 2c42d96a2e..d0e275009f 100644 --- a/site/src/api/index.ts +++ b/site/src/api/index.ts @@ -20,7 +20,7 @@ export const provisioners: Types.Provisioner[] = [ export namespace Workspace { export const create = async (request: Types.CreateWorkspaceRequest): Promise => { - const response = await fetch(`/api/v2/users/me/workspaces`, { + const response = await fetch(`/api/v2/organizations/${request.organization_id}/workspaces`, { method: "POST", headers: { "Content-Type": "application/json", @@ -80,12 +80,27 @@ export const getUsers = async (): Promise => { return response.data } +export const getOrganization = async (organizationId: string): Promise => { + const response = await axios.get(`/api/v2/organizations/${organizationId}`) + return response.data +} + export const getOrganizations = async (): Promise => { const response = await axios.get("/api/v2/users/me/organizations") return response.data } -export const getWorkspace = async ( +export const getTemplate = async (templateId: string): Promise => { + const response = await axios.get(`/api/v2/templates/${templateId}`) + return response.data +} + +export const getWorkspace = async (workspaceId: string): Promise => { + const response = await axios.get(`/api/v2/workspaces/${workspaceId}`) + return response.data +} + +export const getWorkspaceByOwnerAndName = async ( organizationID: string, username = "me", workspaceName: string, diff --git a/site/src/api/types.ts b/site/src/api/types.ts index f2308aeb9b..380859514e 100644 --- a/site/src/api/types.ts +++ b/site/src/api/types.ts @@ -61,6 +61,7 @@ export interface CreateTemplateRequest { export interface CreateWorkspaceRequest { name: string template_id: string + organization_id: string } export interface WorkspaceBuild { diff --git a/site/src/forms/CreateWorkspaceForm.test.tsx b/site/src/forms/CreateWorkspaceForm.test.tsx index eed545dddc..59a7cd3ac7 100644 --- a/site/src/forms/CreateWorkspaceForm.test.tsx +++ b/site/src/forms/CreateWorkspaceForm.test.tsx @@ -1,6 +1,6 @@ import { render, screen } from "@testing-library/react" import React from "react" -import { MockTemplate, MockWorkspace } from "../testHelpers" +import { MockOrganization, MockTemplate, MockWorkspace } from "../testHelpers" import { CreateWorkspaceForm } from "./CreateWorkspaceForm" describe("CreateWorkspaceForm", () => { @@ -10,7 +10,14 @@ describe("CreateWorkspaceForm", () => { const onCancel = () => Promise.resolve() // When - render() + render( + , + ) // Then // Simple smoke test to verify form renders diff --git a/site/src/forms/CreateWorkspaceForm.tsx b/site/src/forms/CreateWorkspaceForm.tsx index 0fd40c1418..7bdd19bd67 100644 --- a/site/src/forms/CreateWorkspaceForm.tsx +++ b/site/src/forms/CreateWorkspaceForm.tsx @@ -15,13 +15,19 @@ export interface CreateWorkspaceForm { template: Template onSubmit: (request: CreateWorkspaceRequest) => Promise onCancel: () => void + organization_id: string } const validationSchema = Yup.object({ name: Yup.string().required("Name is required"), }) -export const CreateWorkspaceForm: React.FC = ({ template, onSubmit, onCancel }) => { +export const CreateWorkspaceForm: React.FC = ({ + template, + onSubmit, + onCancel, + organization_id, +}) => { const styles = useStyles() const form: FormikContextType<{ name: string }> = useFormik<{ name: string }>({ @@ -34,6 +40,7 @@ export const CreateWorkspaceForm: React.FC = ({ template, o return onSubmit({ template_id: template.id, name: name, + organization_id, }) }, }) diff --git a/site/src/pages/TemplatesPages/OrganizationPage/TemplatePage/CreateWorkspacePage.tsx b/site/src/pages/TemplatesPages/OrganizationPage/TemplatePage/CreateWorkspacePage.tsx index 66aff05bbe..1524ee58a0 100644 --- a/site/src/pages/TemplatesPages/OrganizationPage/TemplatePage/CreateWorkspacePage.tsx +++ b/site/src/pages/TemplatesPages/OrganizationPage/TemplatePage/CreateWorkspacePage.tsx @@ -1,5 +1,6 @@ import { makeStyles } from "@material-ui/core/styles" -import React, { useCallback } from "react" +import { useSelector } from "@xstate/react" +import React, { useCallback, useContext } from "react" import { useNavigate, useParams } from "react-router-dom" import useSWR from "swr" import * as API from "../../../../api" @@ -8,12 +9,17 @@ import { ErrorSummary } from "../../../../components/ErrorSummary/ErrorSummary" import { FullScreenLoader } from "../../../../components/Loader/FullScreenLoader" import { CreateWorkspaceForm } from "../../../../forms/CreateWorkspaceForm" import { unsafeSWRArgument } from "../../../../util" +import { selectOrgId } from "../../../../xServices/auth/authSelectors" +import { XServiceContext } from "../../../../xServices/StateContext" export const CreateWorkspacePage: React.FC = () => { const { organization: organizationName, template: templateName } = useParams() const navigate = useNavigate() const styles = useStyles() + const xServices = useContext(XServiceContext) + const myOrgId = useSelector(xServices.authXService, selectOrgId) + const { data: organizationInfo, error: organizationError } = useSWR( () => `/api/v2/users/me/organizations/${organizationName}`, ) @@ -44,9 +50,13 @@ export const CreateWorkspacePage: React.FC = () => { return } + if (!myOrgId) { + return + } + return (
- +
) } diff --git a/site/src/pages/TemplatesPages/OrganizationPage/TemplatePage/TemplatePage.tsx b/site/src/pages/TemplatesPages/OrganizationPage/TemplatePage/TemplatePage.tsx index da78bbf26b..a92335da7e 100644 --- a/site/src/pages/TemplatesPages/OrganizationPage/TemplatePage/TemplatePage.tsx +++ b/site/src/pages/TemplatesPages/OrganizationPage/TemplatePage/TemplatePage.tsx @@ -26,7 +26,10 @@ export const TemplatePage: React.FC = () => { // This just grabs all workspaces... and then later filters them to match the // current template. - const { data: workspaces, error: workspacesError } = useSWR(() => `/api/v2/users/me/workspaces`) + + const { data: workspaces, error: workspacesError } = useSWR( + () => `/api/v2/organizations/${unsafeSWRArgument(organizationInfo).id}/workspaces`, + ) if (organizationError) { return diff --git a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx new file mode 100644 index 0000000000..4f2795b32b --- /dev/null +++ b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx @@ -0,0 +1,14 @@ +import { screen } from "@testing-library/react" +import React from "react" +import { MockTemplate, MockWorkspace, renderWithAuth } from "../../testHelpers" +import { WorkspacePage } from "./WorkspacePage" + +describe("Workspace Page", () => { + it("shows a workspace", async () => { + renderWithAuth(, { route: `/workspaces/${MockWorkspace.id}`, path: "/workspaces/:workspace" }) + const workspaceName = await screen.findByText(MockWorkspace.name) + const templateName = await screen.findByText(MockTemplate.name) + expect(workspaceName).toBeDefined() + expect(templateName).toBeDefined() + }) +}) diff --git a/site/src/pages/WorkspacePage/WorkspacePage.tsx b/site/src/pages/WorkspacePage/WorkspacePage.tsx new file mode 100644 index 0000000000..730953651b --- /dev/null +++ b/site/src/pages/WorkspacePage/WorkspacePage.tsx @@ -0,0 +1,42 @@ +import { useActor } from "@xstate/react" +import React, { useContext, useEffect } from "react" +import { useParams } from "react-router-dom" +import { ErrorSummary } from "../../components/ErrorSummary/ErrorSummary" +import { FullScreenLoader } from "../../components/Loader/FullScreenLoader" +import { Margins } from "../../components/Margins/Margins" +import { Stack } from "../../components/Stack/Stack" +import { Workspace } from "../../components/Workspace/Workspace" +import { firstOrItem } from "../../util/array" +import { XServiceContext } from "../../xServices/StateContext" + +export const WorkspacePage: React.FC = () => { + const { workspace: workspaceQueryParam } = useParams() + const workspaceId = firstOrItem(workspaceQueryParam, null) + + const xServices = useContext(XServiceContext) + const [workspaceState, workspaceSend] = useActor(xServices.workspaceXService) + const { workspace, template, organization, getWorkspaceError, getTemplateError, getOrganizationError } = + workspaceState.context + + /** + * Get workspace, template, and organization on mount and whenever workspaceId changes. + * workspaceSend should not change. + */ + useEffect(() => { + workspaceId && workspaceSend({ type: "GET_WORKSPACE", workspaceId }) + }, [workspaceId, workspaceSend]) + + if (workspaceState.matches("error")) { + return + } else if (!workspace || !template || !organization) { + return + } else { + return ( + + + + + + ) + } +} diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx deleted file mode 100644 index c8446dff91..0000000000 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ /dev/null @@ -1,54 +0,0 @@ -import React from "react" -import { useParams } from "react-router-dom" -import useSWR from "swr" -import * as Types from "../../api/types" -import { ErrorSummary } from "../../components/ErrorSummary/ErrorSummary" -import { FullScreenLoader } from "../../components/Loader/FullScreenLoader" -import { Margins } from "../../components/Margins/Margins" -import { Stack } from "../../components/Stack/Stack" -import { Workspace } from "../../components/Workspace/Workspace" -import { unsafeSWRArgument } from "../../util" -import { firstOrItem } from "../../util/array" - -export const WorkspacePage: React.FC = () => { - const { workspace: workspaceQueryParam } = useParams() - - const { data: workspace, error: workspaceError } = useSWR(() => { - const workspaceParam = firstOrItem(workspaceQueryParam, null) - - return `/api/v2/workspaces/${workspaceParam}` - }) - - // Fetch parent template - const { data: template, error: templateError } = useSWR(() => { - return `/api/v2/templates/${unsafeSWRArgument(workspace).template_id}` - }) - - const { data: organization, error: organizationError } = useSWR(() => { - return `/api/v2/organizations/${unsafeSWRArgument(template).organization_id}` - }) - - if (workspaceError) { - return - } - - if (templateError) { - return - } - - if (organizationError) { - return - } - - if (!workspace || !template || !organization) { - return - } - - return ( - - - - - - ) -} diff --git a/site/src/testHelpers/index.tsx b/site/src/testHelpers/index.tsx index 4ff9e2f780..ed7e89a29b 100644 --- a/site/src/testHelpers/index.tsx +++ b/site/src/testHelpers/index.tsx @@ -26,13 +26,22 @@ export const render = (component: React.ReactElement): RenderResult => { type RenderWithAuthResult = RenderResult & { user: typeof MockUser } -export function renderWithAuth(ui: JSX.Element, { route = "/" }: { route?: string } = {}): RenderWithAuthResult { +/** + * + * @param ui The component to render and test + * @param options Can contain `route`, the URL to use, such as /users/user1, and `path`, + * such as /users/:userid. When there are no parameters, they are the same and you can just supply `route`. + */ +export function renderWithAuth( + ui: JSX.Element, + { route = "/", path }: { route?: string; path?: string } = {}, +): RenderWithAuthResult { const renderResult = wrappedRender( - {ui}} /> + {ui}} /> diff --git a/site/src/xServices/StateContext.tsx b/site/src/xServices/StateContext.tsx index d35573ef23..a94c4ced34 100644 --- a/site/src/xServices/StateContext.tsx +++ b/site/src/xServices/StateContext.tsx @@ -5,11 +5,13 @@ import { ActorRefFrom } from "xstate" import { authMachine } from "./auth/authXService" import { buildInfoMachine } from "./buildInfo/buildInfoXService" import { usersMachine } from "./users/usersXService" +import { workspaceMachine } from "./workspace/workspaceXService" interface XServiceContextType { authXService: ActorRefFrom buildInfoXService: ActorRefFrom usersXService: ActorRefFrom + workspaceXService: ActorRefFrom } /** @@ -34,6 +36,7 @@ export const XServiceProvider: React.FC = ({ children }) => { authXService: useInterpret(authMachine), buildInfoXService: useInterpret(buildInfoMachine), usersXService: useInterpret(() => usersMachine.withConfig({ actions: { redirectToUsersPage } })), + workspaceXService: useInterpret(workspaceMachine), }} > {children} diff --git a/site/src/xServices/terminal/terminalXService.ts b/site/src/xServices/terminal/terminalXService.ts index a308836842..d522611451 100644 --- a/site/src/xServices/terminal/terminalXService.ts +++ b/site/src/xServices/terminal/terminalXService.ts @@ -154,7 +154,7 @@ export const terminalMachine = if (!context.organizations || !context.workspaceName) { throw new Error("organizations or workspace not set") } - return API.getWorkspace(context.organizations[0].id, context.username, context.workspaceName) + return API.getWorkspaceByOwnerAndName(context.organizations[0].id, context.username, context.workspaceName) }, getWorkspaceAgent: async (context) => { if (!context.workspace || !context.workspaceName) { diff --git a/site/src/xServices/workspace/workspaceXService.ts b/site/src/xServices/workspace/workspaceXService.ts new file mode 100644 index 0000000000..34137ce5f0 --- /dev/null +++ b/site/src/xServices/workspace/workspaceXService.ts @@ -0,0 +1,138 @@ +import { assign, createMachine } from "xstate" +import * as API from "../../api" +import * as Types from "../../api/types" + +interface WorkspaceContext { + workspace?: Types.Workspace + template?: Types.Template + organization?: Types.Organization + getWorkspaceError?: Error | unknown + getTemplateError?: Error | unknown + getOrganizationError?: Error | unknown +} + +type WorkspaceEvent = { type: "GET_WORKSPACE"; workspaceId: string } + +export const workspaceMachine = createMachine( + { + tsTypes: {} as import("./workspaceXService.typegen").Typegen0, + schema: { + context: {} as WorkspaceContext, + events: {} as WorkspaceEvent, + services: {} as { + getWorkspace: { + data: Types.Workspace + } + getTemplate: { + data: Types.Template + } + getOrganization: { + data: Types.Organization + } + }, + }, + id: "workspaceState", + initial: "idle", + states: { + idle: { + on: { + GET_WORKSPACE: "gettingWorkspace", + }, + }, + gettingWorkspace: { + invoke: { + src: "getWorkspace", + id: "getWorkspace", + onDone: { + target: "gettingTemplate", + actions: ["assignWorkspace", "clearGetWorkspaceError"], + }, + onError: { + target: "error", + actions: "assignGetWorkspaceError", + }, + }, + tags: "loading", + }, + gettingTemplate: { + invoke: { + src: "getTemplate", + id: "getTemplate", + onDone: { + target: "gettingOrganization", + actions: ["assignTemplate", "clearGetTemplateError"], + }, + onError: { + target: "error", + actions: "assignGetTemplateError", + }, + }, + tags: "loading", + }, + gettingOrganization: { + invoke: { + src: "getOrganization", + id: "getOrganization", + onDone: { + target: "idle", + actions: ["assignOrganization", "clearGetOrganizationError"], + }, + onError: { + target: "error", + actions: "assignGetOrganizationError", + }, + }, + tags: "loading", + }, + error: { + on: { + GET_WORKSPACE: "gettingWorkspace", + }, + }, + }, + }, + { + actions: { + assignWorkspace: assign({ + workspace: (_, event) => event.data, + }), + assignGetWorkspaceError: assign({ + getWorkspaceError: (_, event) => event.data, + }), + clearGetWorkspaceError: (context) => assign({ ...context, getWorkspaceError: undefined }), + assignTemplate: assign({ + template: (_, event) => event.data, + }), + assignGetTemplateError: assign({ + getTemplateError: (_, event) => event.data, + }), + clearGetTemplateError: (context) => assign({ ...context, getTemplateError: undefined }), + assignOrganization: assign({ + organization: (_, event) => event.data, + }), + assignGetOrganizationError: assign({ + getOrganizationError: (_, event) => event.data, + }), + clearGetOrganizationError: (context) => assign({ ...context, getOrganizationError: undefined }), + }, + services: { + getWorkspace: async (_, event) => { + return await API.getWorkspace(event.workspaceId) + }, + getTemplate: async (context) => { + if (context.workspace) { + return await API.getTemplate(context.workspace.template_id) + } else { + throw Error("Cannot get template without workspace") + } + }, + getOrganization: async (context) => { + if (context.template) { + return await API.getOrganization(context.template.organization_id) + } else { + throw Error("Cannot get organization without template") + } + }, + }, + }, +)