From e6f63d7dbf5e3cecfc29d86c017f39a0f525ccea Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 18 Aug 2022 16:52:18 +0000 Subject: [PATCH 1/7] feat: Add template settings page --- site/src/AppRouter.tsx | 9 ++ site/src/api/api.ts | 8 ++ .../WorkspaceStats/WorkspaceStats.test.tsx | 5 +- .../CreateWorkspacePage.test.tsx | 1 - .../pages/TemplatePage/TemplatePage.test.tsx | 5 +- .../pages/TemplatePage/TemplatePageView.tsx | 27 ++++-- .../TemplateSettingsForm.tsx | 87 +++++++++++++++++++ .../TemplateSettingsPage.test.tsx | 70 +++++++++++++++ .../TemplateSettingsPage.tsx | 45 ++++++++++ .../TemplateSettingsPageView.tsx | 31 +++++++ .../WorkspaceBuildPage.test.tsx | 1 - .../WorkspacePage/WorkspacePage.test.tsx | 2 - site/src/testHelpers/entities.ts | 2 +- site/src/testHelpers/handlers.ts | 3 + site/src/testHelpers/renderHelpers.tsx | 31 +++---- .../templateSettingsXService.ts | 86 ++++++++++++++++++ 16 files changed, 374 insertions(+), 39 deletions(-) create mode 100644 site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx create mode 100644 site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx create mode 100644 site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx create mode 100644 site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx create mode 100644 site/src/xServices/templateSettings/templateSettingsXService.ts diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index d10db7b1557c2..5d1be6af3365d 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -1,5 +1,6 @@ import { useSelector } from "@xstate/react" import { SetupPage } from "pages/SetupPage/SetupPage" +import { TemplateSettingsPage } from "pages/TemplateSettingsPage/TemplateSettingsPage" import { FC, lazy, Suspense, useContext } from "react" import { Navigate, Route, Routes } from "react-router-dom" import { selectPermissions } from "xServices/auth/authSelectors" @@ -97,6 +98,14 @@ export const AppRouter: FC = () => { } /> + + + + } + /> diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 5d40a668400f3..6d8d84cd00801 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -145,6 +145,14 @@ export const getTemplateVersions = async ( return response.data } +export const updateTemplateMeta = async ( + templateId: string, + data: TypesGen.UpdateTemplateMeta, +): Promise => { + const response = await axios.patch(`/api/v2/templates/${templateId}`, data) + return response.data +} + export const getWorkspace = async ( workspaceId: string, params?: TypesGen.WorkspaceOptions, diff --git a/site/src/components/WorkspaceStats/WorkspaceStats.test.tsx b/site/src/components/WorkspaceStats/WorkspaceStats.test.tsx index d7d8fc9fef3b1..cefc389fbbc31 100644 --- a/site/src/components/WorkspaceStats/WorkspaceStats.test.tsx +++ b/site/src/components/WorkspaceStats/WorkspaceStats.test.tsx @@ -14,10 +14,7 @@ describe("WorkspaceStats", () => { const handleUpdateMock = jest.fn() renderWithAuth( , - { - route: `/@${MockOutdatedWorkspace.owner_name}/${MockOutdatedWorkspace.name}`, - path: "/@:username/:workspace", - }, + { route: `/@${MockOutdatedWorkspace.owner_name}/${MockOutdatedWorkspace.name}` }, ) const tooltipButton = await screen.findByRole("button") fireEvent.click(tooltipButton) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 962583b4a44e3..13fbf6596b9be 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -10,7 +10,6 @@ import { Language } from "./CreateWorkspacePageView" const renderCreateWorkspacePage = () => { return renderWithAuth(, { route: "/templates/" + MockTemplate.name + "/workspace", - path: "/templates/:template/workspace", }) } diff --git a/site/src/pages/TemplatePage/TemplatePage.test.tsx b/site/src/pages/TemplatePage/TemplatePage.test.tsx index 9dac040cbab08..08a0da6b96fef 100644 --- a/site/src/pages/TemplatePage/TemplatePage.test.tsx +++ b/site/src/pages/TemplatePage/TemplatePage.test.tsx @@ -14,10 +14,7 @@ describe("TemplatePage", () => { const mock = jest.spyOn(CreateDayString, "createDayString") mock.mockImplementation(() => "a minute ago") - renderWithAuth(, { - route: `/templates/${MockTemplate.id}`, - path: "/templates/:template", - }) + renderWithAuth(, { route: `/templates/${MockTemplate.id}` }) await screen.findByText(MockTemplate.name) screen.getByTestId("markdown") screen.getByText(MockWorkspaceResource.name) diff --git a/site/src/pages/TemplatePage/TemplatePageView.tsx b/site/src/pages/TemplatePage/TemplatePageView.tsx index 6eab044d0f688..ff7e1ba2b2864 100644 --- a/site/src/pages/TemplatePage/TemplatePageView.tsx +++ b/site/src/pages/TemplatePage/TemplatePageView.tsx @@ -2,6 +2,7 @@ import Button from "@material-ui/core/Button" import Link from "@material-ui/core/Link" import { makeStyles } from "@material-ui/core/styles" import AddCircleOutline from "@material-ui/icons/AddCircleOutline" +import SettingsOutlined from "@material-ui/icons/SettingsOutlined" import frontMatter from "front-matter" import { FC } from "react" import ReactMarkdown from "react-markdown" @@ -20,6 +21,7 @@ import { VersionsTable } from "../../components/VersionsTable/VersionsTable" import { WorkspaceSection } from "../../components/WorkspaceSection/WorkspaceSection" const Language = { + settingsButton: "Settings", createButton: "Create workspace", noDescription: "", readmeTitle: "README", @@ -51,13 +53,24 @@ export const TemplatePageView: FC = ({ - - + + + + + + + + } > {template.name} diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx new file mode 100644 index 0000000000000..960fb977a7040 --- /dev/null +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx @@ -0,0 +1,87 @@ +import TextField from "@material-ui/core/TextField" +import { Template, UpdateTemplateMeta } from "api/typesGenerated" +import { FormFooter } from "components/FormFooter/FormFooter" +import { Stack } from "components/Stack/Stack" +import { FormikContextType, useFormik } from "formik" +import { FC } from "react" +import { getFormHelpersWithError, nameValidator, onChangeTrimmed } from "util/formUtils" +import * as Yup from "yup" + +export const Language = { + nameLabel: "Name", + descriptionLabel: "Description", + maxTtlLabel: "Max TTL", + // This is the same from the CLI on https://github.com/coder/coder/blob/546157b63ef9204658acf58cb653aa9936b70c49/cli/templateedit.go#L59 + maxTtlHelperText: "Edit the template maximum time before shutdown in milliseconds", + formAriaLabel: "Template settings form", +} + +export const validationSchema = Yup.object({ + name: nameValidator(Language.nameLabel), + description: Yup.string(), + max_ttl_ms: Yup.number(), +}) + +export interface TemplateSettingsForm { + template: Template + onSubmit: (data: UpdateTemplateMeta) => void + onCancel: () => void +} + +export const TemplateSettingsForm: FC = ({ + template, + onSubmit, + onCancel, +}) => { + const form: FormikContextType = useFormik({ + initialValues: { + name: template.name, + description: template.description, + max_ttl_ms: template.max_ttl_ms, + }, + validationSchema, + onSubmit: (data) => { + form.setSubmitting(true) + onSubmit(data) + }, + }) + const getFieldHelpers = getFormHelpersWithError(form) + + return ( +
+ + + + + + + + + + + ) +} diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx new file mode 100644 index 0000000000000..6f65d589679a1 --- /dev/null +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx @@ -0,0 +1,70 @@ +import { screen, waitFor } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import { UpdateTemplateMeta } from "api/typesGenerated" +import { Language as FooterFormLanguage } from "components/FormFooter/FormFooter" +import { rest } from "msw" +import { server } from "testHelpers/server" +import { MockTemplate } from "../../testHelpers/entities" +import { history, renderWithAuth } from "../../testHelpers/renderHelpers" +import { Language as FormLanguage } from "./TemplateSettingsForm" +import { TemplateSettingsPage } from "./TemplateSettingsPage" +import { Language as ViewLanguage } from "./TemplateSettingsPageView" + +const renderTemplateSettingsPage = async () => { + const renderResult = renderWithAuth(, { + route: `/templates/${MockTemplate.name}/settings`, + }) + // Wait the form to be rendered + await screen.findAllByLabelText(FormLanguage.nameLabel) + return renderResult +} + +const fillAndSubmitForm = async ({ + name, + description, + max_ttl_ms, +}: Omit, "min_autostart_interval_ms">) => { + const nameField = await screen.findByLabelText(FormLanguage.nameLabel) + await userEvent.clear(nameField) + await userEvent.type(nameField, name) + + const descriptionField = await screen.findByLabelText(FormLanguage.descriptionLabel) + await userEvent.clear(descriptionField) + await userEvent.type(descriptionField, description) + + const maxTtlField = await screen.findByLabelText(FormLanguage.maxTtlLabel) + await userEvent.clear(maxTtlField) + await userEvent.type(maxTtlField, max_ttl_ms.toString()) + + const submitButton = await screen.findByText(FooterFormLanguage.defaultSubmitLabel) + await userEvent.click(submitButton) +} + +describe("TemplateSettingsPage", () => { + it("renders", async () => { + await renderTemplateSettingsPage() + const element = await screen.findByText(ViewLanguage.title) + expect(element).toBeDefined() + }) + + it("succeeds", async () => { + await renderTemplateSettingsPage() + + const newTemplateSettings = { + name: "edited-template-name", + description: "Edited description", + max_ttl_ms: 4000, + } + // Return patch with new data + server.use( + rest.patch("/api/v2/templates/:templateId", (req, res, ctx) => { + return res(ctx.status(200), ctx.json({ ...MockTemplate, ...newTemplateSettings })) + }), + ) + await fillAndSubmitForm(newTemplateSettings) + + await waitFor(() => + expect(history.location.pathname).toEqual(`/templates/${newTemplateSettings.name}`), + ) + }) +}) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx new file mode 100644 index 0000000000000..5ece110404352 --- /dev/null +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx @@ -0,0 +1,45 @@ +import { useMachine } from "@xstate/react" +import { useOrganizationId } from "hooks/useOrganizationId" +import { FC } from "react" +import { Helmet } from "react-helmet" +import { useNavigate, useParams } from "react-router-dom" +import { pageTitle } from "util/page" +import { templateSettingsMachine } from "xServices/templateSettings/templateSettingsXService" +import { TemplateSettingsPageView } from "./TemplateSettingsPageView" + +const Language = { + title: "Template Settings", +} + +export const TemplateSettingsPage: FC = () => { + const { template: templateName } = useParams() as { template: string } + const navigate = useNavigate() + const organizationId = useOrganizationId() + const [state, send] = useMachine(templateSettingsMachine, { + context: { templateName, organizationId }, + actions: { + onSave: (_, { data }) => { + // Use the data.name because the template name can be changed + navigate(`/templates/${data.name}`) + }, + }, + }) + const { templateSettings: template } = state.context + + return ( + <> + + {pageTitle(Language.title)} + + { + navigate(`/templates/${templateName}`) + }} + onSubmit={(templateSettings) => { + send({ type: "SAVE", templateSettings }) + }} + /> + + ) +} diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx new file mode 100644 index 0000000000000..f93ac736af52a --- /dev/null +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx @@ -0,0 +1,31 @@ +import { Template, UpdateTemplateMeta } from "api/typesGenerated" +import { FullPageForm } from "components/FullPageForm/FullPageForm" +import { Loader } from "components/Loader/Loader" +import { FC } from "react" +import { TemplateSettingsForm } from "./TemplateSettingsForm" + +export const Language = { + title: "Template settings", +} + +export interface TemplateSettingsPageView { + template?: Template + onSubmit: (data: UpdateTemplateMeta) => void + onCancel: () => void +} + +export const TemplateSettingsPageView: FC = ({ + template, + onCancel, + onSubmit, +}) => { + return ( + + {template ? ( + + ) : ( + + )} + + ) +} diff --git a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPage.test.tsx b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPage.test.tsx index cd0faa8354a18..257ccc2d1b6ab 100644 --- a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPage.test.tsx +++ b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPage.test.tsx @@ -8,7 +8,6 @@ describe("WorkspaceBuildPage", () => { const server = new WS(`ws://localhost/api/v2/workspacebuilds/${MockWorkspaceBuild.id}/logs`) renderWithAuth(, { route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}/builds/${MockWorkspace.latest_build.build_number}`, - path: "/@:username/:workspace/builds/:buildNumber", }) await server.connected const log = { diff --git a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx index 25296de6001b3..5eac82ec41d0d 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx @@ -29,7 +29,6 @@ import { WorkspacePage } from "./WorkspacePage" const renderWorkspacePage = async () => { renderWithAuth(, { route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}`, - path: "/@:username/:workspace", }) await screen.findByText(MockWorkspace.name) } @@ -183,7 +182,6 @@ describe("Workspace Page", () => { it("shows the status of each agent in each resource", async () => { renderWithAuth(, { route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}`, - path: "/@:username/:workspace", }) const agent1Names = await screen.findAllByText(MockWorkspaceAgent.name) expect(agent1Names.length).toEqual(2) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 738c29459ff4a..5c484b7e89662 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -146,7 +146,7 @@ export const MockTemplate: TypesGen.Template = { created_at: "2022-05-17T17:39:01.382927298Z", updated_at: "2022-05-18T17:39:01.382927298Z", organization_id: MockOrganization.id, - name: "Test Template", + name: "test-template", provisioner: MockProvisioner.provisioners[0], active_version_id: MockTemplateVersion.id, workspace_owner_count: 1, diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index da7b0ebeda7a7..ae4f23d3b08db 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -28,6 +28,9 @@ export const handlers = [ rest.get("/api/v2/templates/:templateId/versions", async (req, res, ctx) => { return res(ctx.status(200), ctx.json([M.MockTemplateVersion])) }), + rest.patch("/api/v2/templates/:templateId", async (req, res, ctx) => { + return res(ctx.status(200), ctx.json(M.MockTemplate)) + }), rest.get("/api/v2/templateversions/:templateVersionId", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockTemplateVersion)) }), diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx index 93f0e5bbc2d09..2c1a21e60758e 100644 --- a/site/src/testHelpers/renderHelpers.tsx +++ b/site/src/testHelpers/renderHelpers.tsx @@ -2,12 +2,7 @@ import ThemeProvider from "@material-ui/styles/ThemeProvider" import { render as wrappedRender, RenderResult } from "@testing-library/react" import { createMemoryHistory } from "history" import { FC, ReactElement } from "react" -import { - MemoryRouter, - Route, - Routes, - unstable_HistoryRouter as HistoryRouter, -} from "react-router-dom" +import { unstable_HistoryRouter as HistoryRouter } from "react-router-dom" import { RequireAuth } from "../components/RequireAuth/RequireAuth" import { dark } from "../theme" import { XServiceProvider } from "../xServices/StateContext" @@ -25,7 +20,15 @@ export const WrapperComponent: FC = ({ children }) => { ) } -export const render = (component: ReactElement): RenderResult => { +interface RenderOptions { + route?: string +} + +export const render = ( + component: ReactElement, + { route = "/" }: RenderOptions = {}, +): RenderResult => { + history.replace(route) return wrappedRender({component}) } @@ -39,19 +42,9 @@ type RenderWithAuthResult = RenderResult & { user: typeof MockUser } */ export function renderWithAuth( ui: JSX.Element, - { route = "/", path }: { route?: string; path?: string } = {}, + renderOptions?: RenderOptions, ): RenderWithAuthResult { - const renderResult = wrappedRender( - - - - - {ui}} /> - - - - , - ) + const renderResult = render({ui}, renderOptions) return { user: MockUser, diff --git a/site/src/xServices/templateSettings/templateSettingsXService.ts b/site/src/xServices/templateSettings/templateSettingsXService.ts new file mode 100644 index 0000000000000..7c55d45e46da6 --- /dev/null +++ b/site/src/xServices/templateSettings/templateSettingsXService.ts @@ -0,0 +1,86 @@ +import { getTemplateByName, updateTemplateMeta } from "api/api" +import { Template, UpdateTemplateMeta } from "api/typesGenerated" +import { createMachine } from "xstate" +import { assign } from "xstate/lib/actions" + +export const templateSettingsMachine = + /** @xstate-layout N4IgpgJg5mDOIC5QBcwFsAOAbAhqgymMsgJYB2UsAdFgPY4TlQDEEtZYV5AbrQNadUmXASKkK1OgyYIetAMZ4S7ANoAGALqJQGWrBKl22kAA9EANgCMVAKwB2AByWATJbWWAnABYv55w-MAGhAAT0RnAGZnKgibGw9nD3M1JJsHNQiAX0zgoWw8MEJiJmpIAyZmfABBADUAUWNdfUMyYzMESx9bSK8IhwdncwcbF2dgsIRejypzX2dXf09zCLUbbNz0fNFiiSpYHG4Ktg4uMl4BKjyRQrESvYOZOUUW9S0kECbyo3f25KoHOxeDwODx9EYeNTzcYWGxqKgeGw+SJ2cx+VZebI5EBkWgQODGK4FIriSg0eiMCiNPRfVo-RBeMahRAQqhqNnuWERFFeOyedYgQnbEmlRgkqnNZS00DtSwRcz-EY2PzeeYOLk2aEIWJ2WwOIEBNIeBG8-mCm47Un7Q6U96fFptenWRJDIZy+y9AFBJkIEbRbxeWUBRwIyx2U2ba7Eu5WyDimkOhAI2yxSx+foMpWWTV2RLJgIrPoA4bh4RE24SOP2ukdHXOgJq8zuvoozWWBys9nzSydNt2NQYzFAA */ + createMachine( + { + initial: "loading", + schema: {} as { + context: { + organizationId: string + templateName: string + templateSettings?: Template + } + services: { + getTemplateSettings: { + data: Template + } + saveTemplateSettings: { + data: Template + } + } + events: { type: "SAVE"; templateSettings: UpdateTemplateMeta } + }, + tsTypes: {} as import("./templateSettingsXService.typegen").Typegen0, + states: { + loading: { + invoke: { + src: "getTemplateSettings", + onDone: [ + { + actions: "assignTemplateSettings", + target: "editing", + }, + ], + }, + }, + editing: { + on: { + SAVE: { + target: "saving", + }, + }, + }, + saving: { + invoke: { + src: "saveTemplateSettings", + onDone: [ + { + target: "saved", + }, + ], + }, + }, + saved: { + entry: "onSave", + type: "final", + }, + }, + id: "templateSettings", + }, + { + services: { + getTemplateSettings: async ({ organizationId, templateName }) => { + return getTemplateByName(organizationId, templateName) + }, + saveTemplateSettings: async ( + { templateSettings }, + { templateSettings: newTemplateSettings }, + ) => { + if (!templateSettings) { + throw new Error("templateSettins is not loaded yet.") + } + + return updateTemplateMeta(templateSettings.id, newTemplateSettings) + }, + }, + actions: { + assignTemplateSettings: assign({ + templateSettings: (_, { data }) => data, + }), + }, + }, + ) From b79c947e7b7fb07dcc5c520a13d4b88c8c26a377 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 18 Aug 2022 17:04:49 +0000 Subject: [PATCH 2/7] Add storybook --- .../TemplateSettingsPageView.stories.tsx | 20 +++++++++++++++++++ .../TemplateSettingsPageView.tsx | 4 ++-- 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx new file mode 100644 index 0000000000000..55b40b598ed6b --- /dev/null +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx @@ -0,0 +1,20 @@ +import { action } from "@storybook/addon-actions" +import { Story } from "@storybook/react" +import * as Mocks from "../../testHelpers/renderHelpers" +import { TemplateSettingsPageView, TemplateSettingsPageViewProps } from "./TemplateSettingsPageView" + +export default { + title: "pages/TemplateSettingsPageView", + component: TemplateSettingsPageView, +} + +const Template: Story = (args) => ( + +) + +export const Example = Template.bind({}) +Example.args = { + template: Mocks.MockTemplate, + onSubmit: action("onSubmit"), + onCancel: action("cancel"), +} diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx index f93ac736af52a..3c8770910b928 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx @@ -8,13 +8,13 @@ export const Language = { title: "Template settings", } -export interface TemplateSettingsPageView { +export interface TemplateSettingsPageViewProps { template?: Template onSubmit: (data: UpdateTemplateMeta) => void onCancel: () => void } -export const TemplateSettingsPageView: FC = ({ +export const TemplateSettingsPageView: FC = ({ template, onCancel, onSubmit, From aaa7b7a5c864477aa84dc87ad5bcf41eabf63d78 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 18 Aug 2022 17:22:35 +0000 Subject: [PATCH 3/7] Handle errors --- .../TemplateSettingsForm.tsx | 4 ++- .../TemplateSettingsPage.tsx | 6 +++- .../TemplateSettingsPageView.stories.tsx | 32 +++++++++++++++++++ .../TemplateSettingsPageView.tsx | 21 +++++++++--- .../templateSettingsXService.ts | 16 ++++++++++ 5 files changed, 73 insertions(+), 6 deletions(-) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx index 960fb977a7040..14eaa940eadbd 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx @@ -26,12 +26,14 @@ export interface TemplateSettingsForm { template: Template onSubmit: (data: UpdateTemplateMeta) => void onCancel: () => void + error?: unknown } export const TemplateSettingsForm: FC = ({ template, onSubmit, onCancel, + error, }) => { const form: FormikContextType = useFormik({ initialValues: { @@ -45,7 +47,7 @@ export const TemplateSettingsForm: FC = ({ onSubmit(data) }, }) - const getFieldHelpers = getFormHelpersWithError(form) + const getFieldHelpers = getFormHelpersWithError(form, error) return (
diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx index 5ece110404352..6d27191fdeb85 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx @@ -24,7 +24,7 @@ export const TemplateSettingsPage: FC = () => { }, }, }) - const { templateSettings: template } = state.context + const { templateSettings: template, saveTemplateSettingsError, getTemplateError } = state.context return ( <> @@ -33,6 +33,10 @@ export const TemplateSettingsPage: FC = () => { { navigate(`/templates/${templateName}`) }} diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx index 55b40b598ed6b..1ded6a16a9fe1 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx @@ -1,6 +1,7 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" import * as Mocks from "../../testHelpers/renderHelpers" +import { makeMockApiError } from "../../testHelpers/renderHelpers" import { TemplateSettingsPageView, TemplateSettingsPageViewProps } from "./TemplateSettingsPageView" export default { @@ -18,3 +19,34 @@ Example.args = { onSubmit: action("onSubmit"), onCancel: action("cancel"), } + +export const GetTemplateError = Template.bind({}) +GetTemplateError.args = { + template: undefined, + errors: { + getTemplateError: makeMockApiError({ + message: "Failed to fetch the template.", + detail: "You do not have permission to access this resource.", + }), + }, + onSubmit: action("onSubmit"), + onCancel: action("cancel"), +} + +export const SaveTemplateSettingsError = Template.bind({}) +SaveTemplateSettingsError.args = { + template: Mocks.MockTemplate, + errors: { + saveTemplateSettingsError: makeMockApiError({ + message: 'Template "test" already exists.', + validations: [ + { + field: "name", + detail: "This value is already in use and should be unique.", + }, + ], + }), + }, + onSubmit: action("onSubmit"), + onCancel: action("cancel"), +} diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx index 3c8770910b928..f1eb8305a89ce 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx @@ -1,4 +1,5 @@ import { Template, UpdateTemplateMeta } from "api/typesGenerated" +import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" import { FullPageForm } from "components/FullPageForm/FullPageForm" import { Loader } from "components/Loader/Loader" import { FC } from "react" @@ -12,19 +13,31 @@ export interface TemplateSettingsPageViewProps { template?: Template onSubmit: (data: UpdateTemplateMeta) => void onCancel: () => void + errors?: { + getTemplateError?: unknown + saveTemplateSettingsError?: unknown + } } export const TemplateSettingsPageView: FC = ({ template, onCancel, onSubmit, + errors = {}, }) => { + const isLoading = !template && !errors.getTemplateError + return ( - {template ? ( - - ) : ( - + {errors.getTemplateError && } + {isLoading && } + {template && ( + )} ) diff --git a/site/src/xServices/templateSettings/templateSettingsXService.ts b/site/src/xServices/templateSettings/templateSettingsXService.ts index 7c55d45e46da6..c8930978dc899 100644 --- a/site/src/xServices/templateSettings/templateSettingsXService.ts +++ b/site/src/xServices/templateSettings/templateSettingsXService.ts @@ -13,6 +13,8 @@ export const templateSettingsMachine = organizationId: string templateName: string templateSettings?: Template + getTemplateError?: unknown + saveTemplateSettingsError?: unknown } services: { getTemplateSettings: { @@ -35,6 +37,10 @@ export const templateSettingsMachine = target: "editing", }, ], + onError: { + target: "error", + actions: "assignGetTemplateError", + }, }, }, editing: { @@ -52,12 +58,16 @@ export const templateSettingsMachine = target: "saved", }, ], + onError: [{ target: "editing", actions: ["assignSaveTemplateSettingsError"] }], }, }, saved: { entry: "onSave", type: "final", }, + error: { + type: "final", + }, }, id: "templateSettings", }, @@ -81,6 +91,12 @@ export const templateSettingsMachine = assignTemplateSettings: assign({ templateSettings: (_, { data }) => data, }), + assignGetTemplateError: assign({ + getTemplateError: (_, { data }) => data, + }), + assignSaveTemplateSettingsError: assign({ + saveTemplateSettingsError: (_, { data }) => data, + }), }, }, ) From b6bdd925a35b91d6778af5d48119377b1b8750c0 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 18 Aug 2022 17:41:53 +0000 Subject: [PATCH 4/7] Update isloading --- .../TemplateSettingsPage/TemplateSettingsForm.tsx | 11 ++++++----- .../TemplateSettingsPage/TemplateSettingsPage.tsx | 1 + .../TemplateSettingsPage/TemplateSettingsPageView.tsx | 3 +++ .../templateSettings/templateSettingsXService.ts | 2 ++ 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx index 14eaa940eadbd..05a4f4bab5d15 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx @@ -26,6 +26,7 @@ export interface TemplateSettingsForm { template: Template onSubmit: (data: UpdateTemplateMeta) => void onCancel: () => void + isSubmitting: boolean error?: unknown } @@ -34,6 +35,7 @@ export const TemplateSettingsForm: FC = ({ onSubmit, onCancel, error, + isSubmitting, }) => { const form: FormikContextType = useFormik({ initialValues: { @@ -43,7 +45,6 @@ export const TemplateSettingsForm: FC = ({ }, validationSchema, onSubmit: (data) => { - form.setSubmitting(true) onSubmit(data) }, }) @@ -54,7 +55,7 @@ export const TemplateSettingsForm: FC = ({ = ({ = ({ = ({ /> - + ) } diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx index 6d27191fdeb85..883cebba092b4 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.tsx @@ -32,6 +32,7 @@ export const TemplateSettingsPage: FC = () => { {pageTitle(Language.title)} void onCancel: () => void + isSubmitting: boolean errors?: { getTemplateError?: unknown saveTemplateSettingsError?: unknown @@ -23,6 +24,7 @@ export const TemplateSettingsPageView: FC = ({ template, onCancel, onSubmit, + isSubmitting, errors = {}, }) => { const isLoading = !template && !errors.getTemplateError @@ -33,6 +35,7 @@ export const TemplateSettingsPageView: FC = ({ {isLoading && } {template && ( Date: Thu, 18 Aug 2022 16:00:38 -0300 Subject: [PATCH 5/7] Update site/src/xServices/templateSettings/templateSettingsXService.ts Co-authored-by: Presley Pizzo <1290996+presleyp@users.noreply.github.com> --- site/src/xServices/templateSettings/templateSettingsXService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/xServices/templateSettings/templateSettingsXService.ts b/site/src/xServices/templateSettings/templateSettingsXService.ts index c59fea7534d8f..de80fbcedc842 100644 --- a/site/src/xServices/templateSettings/templateSettingsXService.ts +++ b/site/src/xServices/templateSettings/templateSettingsXService.ts @@ -83,7 +83,7 @@ export const templateSettingsMachine = { templateSettings: newTemplateSettings }, ) => { if (!templateSettings) { - throw new Error("templateSettins is not loaded yet.") + throw new Error("templateSettings is not loaded yet.") } return updateTemplateMeta(templateSettings.id, newTemplateSettings) From d5a9f5e5e22550ab33b431b48ec7e5aa08f18089 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 18 Aug 2022 19:33:11 +0000 Subject: [PATCH 6/7] Fix tests --- .../WorkspaceStats/WorkspaceStats.test.tsx | 5 ++- .../CreateWorkspacePage.test.tsx | 1 + .../pages/TemplatePage/TemplatePage.test.tsx | 5 ++- .../TemplateSettingsPage.test.tsx | 20 +++++------- .../WorkspaceBuildPage.test.tsx | 1 + .../WorkspacePage/WorkspacePage.test.tsx | 2 ++ site/src/testHelpers/renderHelpers.tsx | 31 ++++++++++++------- 7 files changed, 39 insertions(+), 26 deletions(-) diff --git a/site/src/components/WorkspaceStats/WorkspaceStats.test.tsx b/site/src/components/WorkspaceStats/WorkspaceStats.test.tsx index cefc389fbbc31..d7d8fc9fef3b1 100644 --- a/site/src/components/WorkspaceStats/WorkspaceStats.test.tsx +++ b/site/src/components/WorkspaceStats/WorkspaceStats.test.tsx @@ -14,7 +14,10 @@ describe("WorkspaceStats", () => { const handleUpdateMock = jest.fn() renderWithAuth( , - { route: `/@${MockOutdatedWorkspace.owner_name}/${MockOutdatedWorkspace.name}` }, + { + route: `/@${MockOutdatedWorkspace.owner_name}/${MockOutdatedWorkspace.name}`, + path: "/@:username/:workspace", + }, ) const tooltipButton = await screen.findByRole("button") fireEvent.click(tooltipButton) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 13fbf6596b9be..962583b4a44e3 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -10,6 +10,7 @@ import { Language } from "./CreateWorkspacePageView" const renderCreateWorkspacePage = () => { return renderWithAuth(, { route: "/templates/" + MockTemplate.name + "/workspace", + path: "/templates/:template/workspace", }) } diff --git a/site/src/pages/TemplatePage/TemplatePage.test.tsx b/site/src/pages/TemplatePage/TemplatePage.test.tsx index 08a0da6b96fef..9dac040cbab08 100644 --- a/site/src/pages/TemplatePage/TemplatePage.test.tsx +++ b/site/src/pages/TemplatePage/TemplatePage.test.tsx @@ -14,7 +14,10 @@ describe("TemplatePage", () => { const mock = jest.spyOn(CreateDayString, "createDayString") mock.mockImplementation(() => "a minute ago") - renderWithAuth(, { route: `/templates/${MockTemplate.id}` }) + renderWithAuth(, { + route: `/templates/${MockTemplate.id}`, + path: "/templates/:template", + }) await screen.findByText(MockTemplate.name) screen.getByTestId("markdown") screen.getByText(MockWorkspaceResource.name) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx index 6f65d589679a1..f3e7bdf81fd49 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx @@ -1,11 +1,10 @@ import { screen, waitFor } from "@testing-library/react" import userEvent from "@testing-library/user-event" +import * as API from "api/api" import { UpdateTemplateMeta } from "api/typesGenerated" import { Language as FooterFormLanguage } from "components/FormFooter/FormFooter" -import { rest } from "msw" -import { server } from "testHelpers/server" import { MockTemplate } from "../../testHelpers/entities" -import { history, renderWithAuth } from "../../testHelpers/renderHelpers" +import { renderWithAuth } from "../../testHelpers/renderHelpers" import { Language as FormLanguage } from "./TemplateSettingsForm" import { TemplateSettingsPage } from "./TemplateSettingsPage" import { Language as ViewLanguage } from "./TemplateSettingsPageView" @@ -13,6 +12,7 @@ import { Language as ViewLanguage } from "./TemplateSettingsPageView" const renderTemplateSettingsPage = async () => { const renderResult = renderWithAuth(, { route: `/templates/${MockTemplate.name}/settings`, + path: `/templates/:templateId/settings`, }) // Wait the form to be rendered await screen.findAllByLabelText(FormLanguage.nameLabel) @@ -55,16 +55,12 @@ describe("TemplateSettingsPage", () => { description: "Edited description", max_ttl_ms: 4000, } - // Return patch with new data - server.use( - rest.patch("/api/v2/templates/:templateId", (req, res, ctx) => { - return res(ctx.status(200), ctx.json({ ...MockTemplate, ...newTemplateSettings })) - }), - ) + jest.spyOn(API, "updateTemplateMeta").mockResolvedValueOnce({ + ...MockTemplate, + ...newTemplateSettings, + }) await fillAndSubmitForm(newTemplateSettings) - await waitFor(() => - expect(history.location.pathname).toEqual(`/templates/${newTemplateSettings.name}`), - ) + await waitFor(() => expect(API.updateTemplateMeta).toBeCalledTimes(1)) }) }) diff --git a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPage.test.tsx b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPage.test.tsx index 257ccc2d1b6ab..cd0faa8354a18 100644 --- a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPage.test.tsx +++ b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPage.test.tsx @@ -8,6 +8,7 @@ describe("WorkspaceBuildPage", () => { const server = new WS(`ws://localhost/api/v2/workspacebuilds/${MockWorkspaceBuild.id}/logs`) renderWithAuth(, { route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}/builds/${MockWorkspace.latest_build.build_number}`, + path: "/@:username/:workspace/builds/:buildNumber", }) await server.connected const log = { diff --git a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx index 5eac82ec41d0d..25296de6001b3 100644 --- a/site/src/pages/WorkspacePage/WorkspacePage.test.tsx +++ b/site/src/pages/WorkspacePage/WorkspacePage.test.tsx @@ -29,6 +29,7 @@ import { WorkspacePage } from "./WorkspacePage" const renderWorkspacePage = async () => { renderWithAuth(, { route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}`, + path: "/@:username/:workspace", }) await screen.findByText(MockWorkspace.name) } @@ -182,6 +183,7 @@ describe("Workspace Page", () => { it("shows the status of each agent in each resource", async () => { renderWithAuth(, { route: `/@${MockWorkspace.owner_name}/${MockWorkspace.name}`, + path: "/@:username/:workspace", }) const agent1Names = await screen.findAllByText(MockWorkspaceAgent.name) expect(agent1Names.length).toEqual(2) diff --git a/site/src/testHelpers/renderHelpers.tsx b/site/src/testHelpers/renderHelpers.tsx index 2c1a21e60758e..93f0e5bbc2d09 100644 --- a/site/src/testHelpers/renderHelpers.tsx +++ b/site/src/testHelpers/renderHelpers.tsx @@ -2,7 +2,12 @@ import ThemeProvider from "@material-ui/styles/ThemeProvider" import { render as wrappedRender, RenderResult } from "@testing-library/react" import { createMemoryHistory } from "history" import { FC, ReactElement } from "react" -import { unstable_HistoryRouter as HistoryRouter } from "react-router-dom" +import { + MemoryRouter, + Route, + Routes, + unstable_HistoryRouter as HistoryRouter, +} from "react-router-dom" import { RequireAuth } from "../components/RequireAuth/RequireAuth" import { dark } from "../theme" import { XServiceProvider } from "../xServices/StateContext" @@ -20,15 +25,7 @@ export const WrapperComponent: FC = ({ children }) => { ) } -interface RenderOptions { - route?: string -} - -export const render = ( - component: ReactElement, - { route = "/" }: RenderOptions = {}, -): RenderResult => { - history.replace(route) +export const render = (component: ReactElement): RenderResult => { return wrappedRender({component}) } @@ -42,9 +39,19 @@ type RenderWithAuthResult = RenderResult & { user: typeof MockUser } */ export function renderWithAuth( ui: JSX.Element, - renderOptions?: RenderOptions, + { route = "/", path }: { route?: string; path?: string } = {}, ): RenderWithAuthResult { - const renderResult = render({ui}, renderOptions) + const renderResult = wrappedRender( + + + + + {ui}} /> + + + + , + ) return { user: MockUser, From 4cb969eb6d6fcad893b49afe3546a90971ac562a Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 18 Aug 2022 19:40:39 +0000 Subject: [PATCH 7/7] Fix error on storybook --- .../src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx | 6 +++++- .../TemplateSettingsPageView.stories.tsx | 3 +++ .../pages/TemplateSettingsPage/TemplateSettingsPageView.tsx | 5 ++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx index 05a4f4bab5d15..0598d0d87b946 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx @@ -2,7 +2,7 @@ import TextField from "@material-ui/core/TextField" import { Template, UpdateTemplateMeta } from "api/typesGenerated" import { FormFooter } from "components/FormFooter/FormFooter" import { Stack } from "components/Stack/Stack" -import { FormikContextType, useFormik } from "formik" +import { FormikContextType, FormikTouched, useFormik } from "formik" import { FC } from "react" import { getFormHelpersWithError, nameValidator, onChangeTrimmed } from "util/formUtils" import * as Yup from "yup" @@ -28,6 +28,8 @@ export interface TemplateSettingsForm { onCancel: () => void isSubmitting: boolean error?: unknown + // Helpful to show field errors on Storybook + initialTouched?: FormikTouched } export const TemplateSettingsForm: FC = ({ @@ -36,6 +38,7 @@ export const TemplateSettingsForm: FC = ({ onCancel, error, isSubmitting, + initialTouched, }) => { const form: FormikContextType = useFormik({ initialValues: { @@ -47,6 +50,7 @@ export const TemplateSettingsForm: FC = ({ onSubmit: (data) => { onSubmit(data) }, + initialTouched, }) const getFieldHelpers = getFormHelpersWithError(form, error) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx index 1ded6a16a9fe1..671fca00a4bec 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.stories.tsx @@ -47,6 +47,9 @@ SaveTemplateSettingsError.args = { ], }), }, + initialTouched: { + name: true, + }, onSubmit: action("onSubmit"), onCancel: action("cancel"), } diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx index 88ce66f6d75f3..46c69a4eb533c 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPageView.tsx @@ -2,7 +2,7 @@ import { Template, UpdateTemplateMeta } from "api/typesGenerated" import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" import { FullPageForm } from "components/FullPageForm/FullPageForm" import { Loader } from "components/Loader/Loader" -import { FC } from "react" +import { ComponentProps, FC } from "react" import { TemplateSettingsForm } from "./TemplateSettingsForm" export const Language = { @@ -18,6 +18,7 @@ export interface TemplateSettingsPageViewProps { getTemplateError?: unknown saveTemplateSettingsError?: unknown } + initialTouched?: ComponentProps["initialTouched"] } export const TemplateSettingsPageView: FC = ({ @@ -26,6 +27,7 @@ export const TemplateSettingsPageView: FC = ({ onSubmit, isSubmitting, errors = {}, + initialTouched, }) => { const isLoading = !template && !errors.getTemplateError @@ -35,6 +37,7 @@ export const TemplateSettingsPageView: FC = ({ {isLoading && } {template && (