From 542a5634c8d3330df64b65c3c3c7c1ba61e815f0 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 3 May 2023 18:27:00 +0000 Subject: [PATCH 1/9] fix(site): Fix template icon field validation --- .../AlertBanner/AlertBanner.stories.tsx | 4 +- .../SignInForm/SignInForm.stories.tsx | 4 +- .../Workspace/Workspace.stories.tsx | 6 +-- .../WorkspaceScheduleForm.stories.tsx | 4 +- .../CreateWorkspacePageView.stories.tsx | 15 +++--- .../GeneralSettingsPageView.stories.tsx | 6 ++- .../StarterTemplatePageView.stories.tsx | 4 +- .../StarterTemplatesPageView.stories.tsx | 4 +- .../TemplateSettingsForm.tsx | 5 +- .../TemplateSettingsPageView.stories.tsx | 4 +- .../TemplateVariablesPageView.stories.tsx | 4 +- .../TemplateVersionPageView.stories.tsx | 4 +- .../TemplatesPageView.stories.tsx | 4 +- .../SSHKeysPage/SSHKeysPageView.stories.tsx | 6 +-- .../TokensPage/TokensPageView.stories.tsx | 6 +-- .../WorspaceProxyView.stories.tsx | 6 +-- site/src/testHelpers/entities.ts | 6 ++- site/src/utils/formUtils.test.ts | 29 +++++++++--- site/src/utils/formUtils.ts | 46 +++++++------------ 19 files changed, 89 insertions(+), 78 deletions(-) diff --git a/site/src/components/AlertBanner/AlertBanner.stories.tsx b/site/src/components/AlertBanner/AlertBanner.stories.tsx index b46b60a4293e5..2a5f5f3a7969a 100644 --- a/site/src/components/AlertBanner/AlertBanner.stories.tsx +++ b/site/src/components/AlertBanner/AlertBanner.stories.tsx @@ -1,7 +1,7 @@ import { Story } from "@storybook/react" import { AlertBanner } from "./AlertBanner" import Button from "@material-ui/core/Button" -import { makeMockApiError } from "testHelpers/entities" +import { makeMockValidationApiError } from "testHelpers/entities" import { AlertBannerProps } from "./alertTypes" import Link from "@material-ui/core/Link" @@ -16,7 +16,7 @@ const ExampleAction = ( ) -const mockError = makeMockApiError({ +const mockError = makeMockValidationApiError({ message: "Email or password was invalid", detail: "Password is invalid", }) diff --git a/site/src/components/SignInForm/SignInForm.stories.tsx b/site/src/components/SignInForm/SignInForm.stories.tsx index c0fd17d6f6d70..04b70e3628878 100644 --- a/site/src/components/SignInForm/SignInForm.stories.tsx +++ b/site/src/components/SignInForm/SignInForm.stories.tsx @@ -1,5 +1,5 @@ import { Story } from "@storybook/react" -import { makeMockApiError } from "testHelpers/entities" +import { makeMockValidationApiError } from "testHelpers/entities" import { SignInForm, SignInFormProps } from "./SignInForm" export default { @@ -37,7 +37,7 @@ SigningIn.args = { export const WithError = Template.bind({}) WithError.args = { ...SignedOut.args, - error: makeMockApiError({ + error: makeMockValidationApiError({ message: "Email or password was invalid", validations: [ { diff --git a/site/src/components/Workspace/Workspace.stories.tsx b/site/src/components/Workspace/Workspace.stories.tsx index 23b5806f83eca..b952b6c2c819a 100644 --- a/site/src/components/Workspace/Workspace.stories.tsx +++ b/site/src/components/Workspace/Workspace.stories.tsx @@ -95,7 +95,7 @@ Failed.args = { ...Running.args, workspace: Mocks.MockFailedWorkspace, workspaceErrors: { - [WorkspaceErrors.BUILD_ERROR]: Mocks.makeMockApiError({ + [WorkspaceErrors.BUILD_ERROR]: Mocks.makeMockValidationApiError({ message: "A workspace build is already active.", }), }, @@ -152,7 +152,7 @@ export const GetBuildsError = Template.bind({}) GetBuildsError.args = { ...Running.args, workspaceErrors: { - [WorkspaceErrors.GET_BUILDS_ERROR]: Mocks.makeMockApiError({ + [WorkspaceErrors.GET_BUILDS_ERROR]: Mocks.makeMockValidationApiError({ message: "There is a problem fetching builds.", }), }, @@ -162,7 +162,7 @@ export const CancellationError = Template.bind({}) CancellationError.args = { ...Failed.args, workspaceErrors: { - [WorkspaceErrors.CANCELLATION_ERROR]: Mocks.makeMockApiError({ + [WorkspaceErrors.CANCELLATION_ERROR]: Mocks.makeMockValidationApiError({ message: "Job could not be canceled.", }), }, diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx index 47972cef3110e..742ff5270adc9 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx @@ -8,7 +8,7 @@ import { emptySchedule, } from "pages/WorkspaceSettingsPage/WorkspaceSchedulePage/schedule" import { emptyTTL } from "pages/WorkspaceSettingsPage/WorkspaceSchedulePage/ttl" -import { makeMockApiError } from "testHelpers/entities" +import { makeMockValidationApiError } from "testHelpers/entities" import { WorkspaceScheduleForm, WorkspaceScheduleFormProps, @@ -81,7 +81,7 @@ export const WithError = Template.bind({}) WithError.args = { initialValues: { ...defaultInitialValues, ttl: 100 }, initialTouched: { ttl: true }, - submitScheduleError: makeMockApiError({ + submitScheduleError: makeMockValidationApiError({ message: "Something went wrong.", validations: [{ field: "ttl_ms", detail: "Invalid time until shutdown." }], }), diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index b37f060a8713b..77823148099a9 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -1,6 +1,6 @@ import { ComponentMeta, Story } from "@storybook/react" import { - makeMockApiError, + makeMockValidationApiError, mockParameterSchema, MockParameterSchemas, MockTemplate, @@ -85,7 +85,7 @@ export const GetTemplatesError = Template.bind({}) GetTemplatesError.args = { ...Parameters.args, createWorkspaceErrors: { - [CreateWorkspaceErrors.GET_TEMPLATES_ERROR]: makeMockApiError({ + [CreateWorkspaceErrors.GET_TEMPLATES_ERROR]: makeMockValidationApiError({ message: "Failed to fetch templates.", detail: "You do not have permission to access this resource.", }), @@ -97,10 +97,11 @@ export const GetTemplateSchemaError = Template.bind({}) GetTemplateSchemaError.args = { ...Parameters.args, createWorkspaceErrors: { - [CreateWorkspaceErrors.GET_TEMPLATE_SCHEMA_ERROR]: makeMockApiError({ - message: 'Failed to fetch template schema for "docker-amd64".', - detail: "You do not have permission to access this resource.", - }), + [CreateWorkspaceErrors.GET_TEMPLATE_SCHEMA_ERROR]: + makeMockValidationApiError({ + message: 'Failed to fetch template schema for "docker-amd64".', + detail: "You do not have permission to access this resource.", + }), }, hasTemplateErrors: true, } @@ -109,7 +110,7 @@ export const CreateWorkspaceError = Template.bind({}) CreateWorkspaceError.args = { ...Parameters.args, createWorkspaceErrors: { - [CreateWorkspaceErrors.CREATE_WORKSPACE_ERROR]: makeMockApiError({ + [CreateWorkspaceErrors.CREATE_WORKSPACE_ERROR]: makeMockValidationApiError({ message: 'Workspace "test" already exists in the "docker-amd64" template.', validations: [ diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index ff806b522791b..a25c4b1aa6881 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -1,6 +1,6 @@ import { ComponentMeta, Story } from "@storybook/react" import { - makeMockApiError, + makeMockValidationApiError, MockDeploymentDAUResponse, } from "testHelpers/entities" import { @@ -43,5 +43,7 @@ NoDAUs.args = { export const DAUError = Template.bind({}) DAUError.args = { deploymentDAUs: undefined, - getDeploymentDAUsError: makeMockApiError({ message: "Error fetching DAUs." }), + getDeploymentDAUsError: makeMockValidationApiError({ + message: "Error fetching DAUs.", + }), } diff --git a/site/src/pages/StarterTemplatePage/StarterTemplatePageView.stories.tsx b/site/src/pages/StarterTemplatePage/StarterTemplatePageView.stories.tsx index fc23097efb5e4..1d852425119f8 100644 --- a/site/src/pages/StarterTemplatePage/StarterTemplatePageView.stories.tsx +++ b/site/src/pages/StarterTemplatePage/StarterTemplatePageView.stories.tsx @@ -1,6 +1,6 @@ import { Story } from "@storybook/react" import { - makeMockApiError, + makeMockValidationApiError, MockOrganization, MockTemplateExample, } from "testHelpers/entities" @@ -33,7 +33,7 @@ Error.args = { context: { exampleId: MockTemplateExample.id, organizationId: MockOrganization.id, - error: makeMockApiError({ + error: makeMockValidationApiError({ message: `Example ${MockTemplateExample.id} not found.`, }), starterTemplate: undefined, diff --git a/site/src/pages/StarterTemplatesPage/StarterTemplatesPageView.stories.tsx b/site/src/pages/StarterTemplatesPage/StarterTemplatesPageView.stories.tsx index 32a4aec2ede7c..1217759958e58 100644 --- a/site/src/pages/StarterTemplatesPage/StarterTemplatesPageView.stories.tsx +++ b/site/src/pages/StarterTemplatesPage/StarterTemplatesPageView.stories.tsx @@ -1,6 +1,6 @@ import { Story } from "@storybook/react" import { - makeMockApiError, + makeMockValidationApiError, MockOrganization, MockTemplateExample, MockTemplateExample2, @@ -36,7 +36,7 @@ export const Error = Template.bind({}) Error.args = { context: { organizationId: MockOrganization.id, - error: makeMockApiError({ + error: makeMockValidationApiError({ message: "Error on loading the template examples", }), starterTemplatesByTag: undefined, diff --git a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx index b51c889a32c24..5762eba17335a 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsForm.tsx @@ -7,6 +7,7 @@ import { nameValidator, templateDisplayNameValidator, onChangeTrimmed, + iconValidator, } from "utils/formUtils" import * as Yup from "yup" import i18next from "i18next" @@ -37,8 +38,8 @@ export const getValidationSchema = (): Yup.AnyObjectSchema => MAX_DESCRIPTION_CHAR_LIMIT, i18next.t("descriptionMaxError", { ns: "templateSettingsPage" }), ), - allow_user_cancel_workspace_jobs: Yup.boolean(), + icon: iconValidator, }) export interface TemplateSettingsForm { @@ -74,7 +75,7 @@ export const TemplateSettingsForm: FC = ({ onSubmit, initialTouched, }) - const getFieldHelpers = getFormHelpers(form, error) + const getFieldHelpers = getFormHelpers(form, error) const { t } = useTranslation("templateSettingsPage") const styles = useStyles() diff --git a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx index d4fa5a847360d..7ef92d0732202 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx @@ -1,6 +1,6 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" -import { makeMockApiError, MockTemplate } from "testHelpers/entities" +import { makeMockValidationApiError, MockTemplate } from "testHelpers/entities" import { TemplateSettingsPageView, TemplateSettingsPageViewProps, @@ -25,7 +25,7 @@ Example.args = {} export const SaveTemplateSettingsError = Template.bind({}) SaveTemplateSettingsError.args = { - submitError: makeMockApiError({ + submitError: makeMockValidationApiError({ message: 'Template "test" already exists.', validations: [ { diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx index 2638196cc8540..c2222c5388cf4 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx @@ -1,7 +1,7 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" import { - makeMockApiError, + makeMockValidationApiError, MockTemplateVersion, MockTemplateVersionVariable1, MockTemplateVersionVariable2, @@ -69,7 +69,7 @@ WithUpdateTemplateError.args = { MockTemplateVersionVariable4, ], errors: { - updateTemplateError: makeMockApiError({ + updateTemplateError: makeMockValidationApiError({ message: "Something went wrong.", }), }, diff --git a/site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx b/site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx index 33437b2cf9d19..e3a6732ab1dbe 100644 --- a/site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx +++ b/site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx @@ -2,7 +2,7 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" import { UseTabResult } from "hooks/useTab" import { - makeMockApiError, + makeMockValidationApiError, MockOrganization, MockTemplate, MockTemplateVersion, @@ -64,7 +64,7 @@ Error.args = { ...defaultArgs.context, currentVersion: undefined, currentFiles: undefined, - error: makeMockApiError({ + error: makeMockValidationApiError({ message: "Error on loading the template version", }), }, diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx index e5c20f501ef3e..46ad393847526 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx @@ -1,6 +1,6 @@ import { ComponentMeta, Story } from "@storybook/react" import { - makeMockApiError, + makeMockValidationApiError, MockOrganization, MockPermissions, MockTemplate, @@ -89,7 +89,7 @@ Error.args = { ...MockPermissions, createTemplates: false, }, - error: makeMockApiError({ + error: makeMockValidationApiError({ message: "Something went wrong fetching templates.", }), templates: undefined, diff --git a/site/src/pages/UserSettingsPage/SSHKeysPage/SSHKeysPageView.stories.tsx b/site/src/pages/UserSettingsPage/SSHKeysPage/SSHKeysPageView.stories.tsx index 4342dba6b8237..33d761a4ae285 100644 --- a/site/src/pages/UserSettingsPage/SSHKeysPage/SSHKeysPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/SSHKeysPage/SSHKeysPageView.stories.tsx @@ -1,5 +1,5 @@ import { Story } from "@storybook/react" -import { makeMockApiError } from "testHelpers/entities" +import { makeMockValidationApiError } from "testHelpers/entities" import { SSHKeysPageView, SSHKeysPageViewProps } from "./SSHKeysPageView" export default { @@ -39,7 +39,7 @@ export const WithGetSSHKeyError = Template.bind({}) WithGetSSHKeyError.args = { ...Example.args, hasLoaded: false, - getSSHKeyError: makeMockApiError({ + getSSHKeyError: makeMockValidationApiError({ message: "Failed to get SSH key", }), } @@ -47,7 +47,7 @@ WithGetSSHKeyError.args = { export const WithRegenerateSSHKeyError = Template.bind({}) WithRegenerateSSHKeyError.args = { ...Example.args, - regenerateSSHKeyError: makeMockApiError({ + regenerateSSHKeyError: makeMockValidationApiError({ message: "Failed to regenerate SSH key", }), } diff --git a/site/src/pages/UserSettingsPage/TokensPage/TokensPageView.stories.tsx b/site/src/pages/UserSettingsPage/TokensPage/TokensPageView.stories.tsx index 2387cc3c3c023..5ef3677c8fe88 100644 --- a/site/src/pages/UserSettingsPage/TokensPage/TokensPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/TokensPage/TokensPageView.stories.tsx @@ -1,5 +1,5 @@ import { Story } from "@storybook/react" -import { makeMockApiError, MockTokens } from "testHelpers/entities" +import { makeMockValidationApiError, MockTokens } from "testHelpers/entities" import { TokensPageView, TokensPageViewProps } from "./TokensPageView" export default { @@ -41,7 +41,7 @@ export const WithGetTokensError = Template.bind({}) WithGetTokensError.args = { ...Example.args, hasLoaded: false, - getTokensError: makeMockApiError({ + getTokensError: makeMockValidationApiError({ message: "Failed to get tokens.", }), } @@ -50,7 +50,7 @@ export const WithDeleteTokenError = Template.bind({}) WithDeleteTokenError.args = { ...Example.args, hasLoaded: false, - deleteTokenError: makeMockApiError({ + deleteTokenError: makeMockValidationApiError({ message: "Failed to delete token.", }), } diff --git a/site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorspaceProxyView.stories.tsx b/site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorspaceProxyView.stories.tsx index 74239927002ad..9e74e3b48bb21 100644 --- a/site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorspaceProxyView.stories.tsx +++ b/site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorspaceProxyView.stories.tsx @@ -1,6 +1,6 @@ import { Story } from "@storybook/react" import { - makeMockApiError, + makeMockValidationApiError, MockWorkspaceProxies, MockPrimaryWorkspaceProxy, MockHealthyWildWorkspaceProxy, @@ -61,7 +61,7 @@ export const WithProxiesError = Template.bind({}) WithProxiesError.args = { ...Example.args, hasLoaded: false, - getWorkspaceProxiesError: makeMockApiError({ + getWorkspaceProxiesError: makeMockValidationApiError({ message: "Failed to get proxies.", }), } @@ -70,7 +70,7 @@ export const WithSelectProxyError = Template.bind({}) WithSelectProxyError.args = { ...Example.args, hasLoaded: false, - selectProxyError: makeMockApiError({ + selectProxyError: makeMockValidationApiError({ message: "Failed to select proxy.", }), } diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 91dc24132f009..1ef3ed911b7f8 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -341,6 +341,8 @@ export const MockTemplate: TypesGen.Template = { created_by_name: "test_creator", icon: "/icon/code.svg", allow_user_cancel_workspace_jobs: true, + allow_user_autostart: false, + allow_user_autostop: false, } export const MockTemplateVersionFiles: TemplateVersionFiles = { @@ -1261,9 +1263,9 @@ type MockAPIOutput = { isAxiosError: boolean } -type MakeMockApiErrorFunction = (input: MockAPIInput) => MockAPIOutput +type makeMockValidationApiErrorFunction = (input: MockAPIInput) => MockAPIOutput -export const makeMockApiError: MakeMockApiErrorFunction = ({ +export const makeMockValidationApiError: makeMockValidationApiErrorFunction = ({ message, detail, validations, diff --git a/site/src/utils/formUtils.test.ts b/site/src/utils/formUtils.test.ts index 0950bea52f6fc..ebdc09e04a572 100644 --- a/site/src/utils/formUtils.test.ts +++ b/site/src/utils/formUtils.test.ts @@ -1,5 +1,6 @@ import { FormikContextType } from "formik/dist/types" import { getFormHelpers, nameValidator, onChangeTrimmed } from "./formUtils" +import { makeMockValidationApiError } from "testHelpers/entities" interface TestType { untouchedGoodField: string @@ -69,9 +70,17 @@ describe("form util functions", () => { }) describe("with API errors", () => { it("shows an error if there is only an API error", () => { - const getFieldHelpers = getFormHelpers(form, { - touchedGoodField: "API error!", - }) + const getFieldHelpers = getFormHelpers( + form, + makeMockValidationApiError({ + validations: [ + { + field: "touchedGoodField", + detail: "API error!", + }, + ], + }), + ) const result = getFieldHelpers("touchedGoodField") expect(result.error).toBeTruthy() expect(result.helperText).toEqual("API error!") @@ -83,9 +92,17 @@ describe("form util functions", () => { expect(result.helperText).toEqual("oops!") }) it("shows the API error if both are present", () => { - const getFieldHelpers = getFormHelpers(form, { - touchedBadField: "API error!", - }) + const getFieldHelpers = getFormHelpers( + form, + makeMockValidationApiError({ + validations: [ + { + field: "touchedBadField", + detail: "API error!", + }, + ], + }), + ) const result = getFieldHelpers("touchedBadField") expect(result.error).toBeTruthy() expect(result.helperText).toEqual("API error!") diff --git a/site/src/utils/formUtils.ts b/site/src/utils/formUtils.ts index f4ca69563f7c9..a92c8d4034e59 100644 --- a/site/src/utils/formUtils.ts +++ b/site/src/utils/formUtils.ts @@ -1,5 +1,5 @@ import { isApiValidationError, mapApiErrorToFieldErrors } from "api/errors" -import { FormikContextType, FormikErrors, getIn } from "formik" +import { FormikContextType, FormikErrors } from "formik" import { ChangeEvent, ChangeEventHandler, @@ -34,37 +34,23 @@ interface FormHelpers { } export const getFormHelpers = - (form: FormikContextType, error?: Error | unknown) => - ( - name: string, - HelperText: ReactNode = "", - backendErrorName?: string, - ): FormHelpers => { + (form: FormikContextType, error?: unknown) => + (fieldName: keyof TFormValues, helperText?: ReactNode): FormHelpers => { const apiValidationErrors = isApiValidationError(error) - ? (mapApiErrorToFieldErrors(error.response.data) as FormikErrors) - : // This should not return the error since it is not and api validation error but I didn't have time to fix this and tests - error - - if (typeof name !== "string") { - throw new Error( - `name must be type of string, instead received '${typeof name}'`, - ) - } - - const apiErrorName = backendErrorName ?? name - - // getIn is a util function from Formik that gets at any depth of nesting - // and is necessary for the types to work - const touched = getIn(form.touched, name) - const apiError = getIn(apiValidationErrors, apiErrorName) - const frontendError = getIn(form.errors, name) - const returnError = apiError ?? frontendError + ? (mapApiErrorToFieldErrors( + error.response.data, + ) as FormikErrors) + : undefined + const touched = Boolean(form.touched[fieldName]) + const apiError = apiValidationErrors?.[fieldName]?.toString() + const formError = form.errors[fieldName]?.toString() + const errorToDisplay = apiError ?? formError return { - ...form.getFieldProps(name), - id: name, - error: touched && Boolean(returnError), - helperText: touched ? returnError || HelperText : HelperText, + ...form.getFieldProps(fieldName), + id: fieldName.toString(), + error: touched && Boolean(errorToDisplay), + helperText: touched ? errorToDisplay ?? helperText : helperText, } } @@ -102,3 +88,5 @@ export const templateDisplayNameValidator = ( Language.nameTooLong(displayName, templateDisplayNameMaxLength), ) .optional() + +export const iconValidator = Yup.string().label("Icon").max(256) From 70dc4ae4dda1918464d6c7da2ad2dfc88894c9c7 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 3 May 2023 18:31:00 +0000 Subject: [PATCH 2/9] Remove unecessary type --- site/src/testHelpers/entities.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 1ef3ed911b7f8..f2c0b715c3e5a 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1263,13 +1263,11 @@ type MockAPIOutput = { isAxiosError: boolean } -type makeMockValidationApiErrorFunction = (input: MockAPIInput) => MockAPIOutput - -export const makeMockValidationApiError: makeMockValidationApiErrorFunction = ({ +export const makeMockValidationApiError = ({ message, detail, validations, -}) => ({ +}: MockAPIInput): MockAPIOutput => ({ response: { data: { message: message ?? "Something went wrong.", From 3e2679387f805b41e21c2b36a4b6afc25d8089ce Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 4 May 2023 14:28:08 +0000 Subject: [PATCH 3/9] Minor fixes --- site/src/api/errors.test.ts | 77 ++++++++----------- site/src/api/errors.ts | 28 ++----- .../AlertBanner/AlertBanner.stories.tsx | 4 +- .../SearchBarWithFilter.stories.tsx | 22 +++--- .../SettingsAccountForm.stories.tsx | 22 +++--- .../SettingsSecurityForm.stories.tsx | 22 +++--- .../SignInForm/SignInForm.stories.tsx | 4 +- .../Workspace/Workspace.stories.tsx | 6 +- .../WorkspaceScheduleForm.stories.tsx | 4 +- .../CreateWorkspacePageView.stories.tsx | 15 ++-- .../GeneralSettingsPageView.stories.tsx | 7 +- .../StarterTemplatePageView.stories.tsx | 4 +- .../StarterTemplatesPageView.stories.tsx | 4 +- .../TemplateSettingsPageView.stories.tsx | 4 +- .../TemplateVariablesPageView.stories.tsx | 4 +- .../TemplateVersionPageView.stories.tsx | 4 +- .../TemplatesPageView.stories.tsx | 4 +- .../AccountPage/AccountPage.test.tsx | 20 +++-- .../SSHKeysPage/SSHKeysPageView.stories.tsx | 6 +- .../SecurityPage/SecurityPage.test.tsx | 35 ++++----- .../TokensPage/TokensPageView.stories.tsx | 6 +- .../WorspaceProxyView.stories.tsx | 6 +- .../pages/UsersPage/UsersPageView.stories.tsx | 1 - site/src/testHelpers/entities.ts | 7 +- site/src/utils/formUtils.test.ts | 6 +- site/src/utils/formUtils.ts | 14 +++- 26 files changed, 144 insertions(+), 192 deletions(-) diff --git a/site/src/api/errors.test.ts b/site/src/api/errors.test.ts index fcf5032be491a..5fa94412f7334 100644 --- a/site/src/api/errors.test.ts +++ b/site/src/api/errors.test.ts @@ -1,3 +1,4 @@ +import { mockApiError } from "testHelpers/entities" import { getValidationErrorMessage, isApiError, @@ -7,17 +8,14 @@ import { describe("isApiError", () => { it("returns true when the object is an API Error", () => { expect( - isApiError({ - isAxiosError: true, - response: { - data: { - message: "Invalid entry", - errors: [ - { detail: "Username is already in use", field: "username" }, - ], - }, - }, - }), + isApiError( + mockApiError({ + message: "Invalid entry", + validations: [ + { detail: "Username is already in use", field: "username" }, + ], + }), + ), ).toBe(true) }) @@ -48,24 +46,21 @@ describe("mapApiErrorToFieldErrors", () => { describe("getValidationErrorMessage", () => { it("returns multiple validation messages", () => { expect( - getValidationErrorMessage({ - response: { - data: { - message: "Invalid user search query.", - validations: [ - { - field: "status", - detail: `Query param "status" has invalid value: "inactive" is not a valid user status`, - }, - { - field: "q", - detail: `Query element "role:a:e" can only contain 1 ':'`, - }, - ], - }, - }, - isAxiosError: true, - }), + getValidationErrorMessage( + mockApiError({ + message: "Invalid user search query.", + validations: [ + { + field: "status", + detail: `Query param "status" has invalid value: "inactive" is not a valid user status`, + }, + { + field: "q", + detail: `Query element "role:a:e" can only contain 1 ':'`, + }, + ], + }), + ), ).toEqual( `Query param "status" has invalid value: "inactive" is not a valid user status\nQuery element "role:a:e" can only contain 1 ':'`, ) @@ -73,28 +68,18 @@ describe("getValidationErrorMessage", () => { it("non-API error returns empty validation message", () => { expect( - getValidationErrorMessage({ - response: { - data: { - error: "Invalid user search query.", - }, - }, - isAxiosError: true, - }), + getValidationErrorMessage(new Error("Invalid user search query.")), ).toEqual("") }) it("no validations field returns empty validation message", () => { expect( - getValidationErrorMessage({ - response: { - data: { - message: "Invalid user search query.", - detail: `Query element "role:a:e" can only contain 1 ':'`, - }, - }, - isAxiosError: true, - }), + getValidationErrorMessage( + mockApiError({ + message: "Invalid user search query.", + detail: `Query element "role:a:e" can only contain 1 ':'`, + }), + ), ).toEqual("") }) }) diff --git a/site/src/api/errors.ts b/site/src/api/errors.ts index b581526803986..83c41cbcf4dd0 100644 --- a/site/src/api/errors.ts +++ b/site/src/api/errors.ts @@ -24,30 +24,16 @@ export type ApiError = AxiosError & { } export const isApiError = (err: unknown): err is ApiError => { - if (axios.isAxiosError(err)) { - const response = err.response?.data - if (!response) { - return false - } - - return ( - typeof response.message === "string" && - (typeof response.errors === "undefined" || Array.isArray(response.errors)) - ) - } - - return false + return axios.isAxiosError(err) && err.response !== undefined } -/** - * ApiErrors contain useful error messages in their response body. They contain an overall message - * and may also contain errors for specific form fields. - * @param error ApiError - * @returns true if the ApiError contains error messages for specific form fields. - */ export const hasApiFieldErrors = (error: ApiError): boolean => Array.isArray(error.response.data.validations) +export const isApiValidationError = (error: unknown): error is ApiError => { + return isApiError(error) && hasApiFieldErrors(error) +} + export const mapApiErrorToFieldErrors = ( apiErrorResponse: ApiErrorResponse, ): FieldErrors => { @@ -63,10 +49,6 @@ export const mapApiErrorToFieldErrors = ( return result } -export const isApiValidationError = (error: unknown): error is ApiError => { - return isApiError(error) && hasApiFieldErrors(error) -} - /** * * @param error diff --git a/site/src/components/AlertBanner/AlertBanner.stories.tsx b/site/src/components/AlertBanner/AlertBanner.stories.tsx index 2a5f5f3a7969a..967e331b31167 100644 --- a/site/src/components/AlertBanner/AlertBanner.stories.tsx +++ b/site/src/components/AlertBanner/AlertBanner.stories.tsx @@ -1,7 +1,7 @@ import { Story } from "@storybook/react" import { AlertBanner } from "./AlertBanner" import Button from "@material-ui/core/Button" -import { makeMockValidationApiError } from "testHelpers/entities" +import { mockApiError } from "testHelpers/entities" import { AlertBannerProps } from "./alertTypes" import Link from "@material-ui/core/Link" @@ -16,7 +16,7 @@ const ExampleAction = ( ) -const mockError = makeMockValidationApiError({ +const mockError = mockApiError({ message: "Email or password was invalid", detail: "Password is invalid", }) diff --git a/site/src/components/SearchBarWithFilter/SearchBarWithFilter.stories.tsx b/site/src/components/SearchBarWithFilter/SearchBarWithFilter.stories.tsx index 86f8bd61e9f62..98dde91134845 100644 --- a/site/src/components/SearchBarWithFilter/SearchBarWithFilter.stories.tsx +++ b/site/src/components/SearchBarWithFilter/SearchBarWithFilter.stories.tsx @@ -4,6 +4,7 @@ import { SearchBarWithFilter, SearchBarWithFilterProps, } from "./SearchBarWithFilter" +import { mockApiError } from "testHelpers/entities" export default { title: "components/SearchBarWithFilter", @@ -34,18 +35,13 @@ WithError.args = { { query: userFilterQuery.active, name: "Active users" }, { query: "random query", name: "Random query" }, ], - error: { - response: { - data: { - message: "Invalid user search query.", - validations: [ - { - field: "status", - detail: `Query param "status" has invalid value: "inactive" is not a valid user status`, - }, - ], + error: mockApiError({ + message: "Invalid user search query.", + validations: [ + { + field: "status", + detail: `Query param "status" has invalid value: "inactive" is not a valid user status`, }, - }, - isAxiosError: true, - }, + ], + }), } diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.stories.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.stories.tsx index 87947cfd42c56..3b49826232a5f 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.stories.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.stories.tsx @@ -1,5 +1,6 @@ import { Story } from "@storybook/react" import { AccountForm, AccountFormProps } from "./SettingsAccountForm" +import { mockApiError } from "testHelpers/entities" export default { title: "components/SettingsAccountForm", @@ -35,20 +36,15 @@ Loading.args = { export const WithError = Template.bind({}) WithError.args = { ...Example.args, - updateProfileError: { - response: { - data: { - message: "Username is invalid", - validations: [ - { - field: "username", - detail: "Username is too long.", - }, - ], + updateProfileError: mockApiError({ + message: "Username is invalid", + validations: [ + { + field: "username", + detail: "Username is too long.", }, - }, - isAxiosError: true, - }, + ], + }), initialTouched: { username: true, }, diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx index 1f7e878054787..d2ca25ff16bd0 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx @@ -1,5 +1,6 @@ import { Story } from "@storybook/react" import { SecurityForm, SecurityFormProps } from "./SettingsSecurityForm" +import { mockApiError } from "testHelpers/entities" export default { title: "components/SettingsSecurityForm", @@ -36,20 +37,15 @@ Loading.args = { export const WithError = Template.bind({}) WithError.args = { ...Example.args, - updateSecurityError: { - response: { - data: { - message: "Old password is incorrect", - validations: [ - { - field: "old_password", - detail: "Old password is incorrect.", - }, - ], + updateSecurityError: mockApiError({ + message: "Old password is incorrect", + validations: [ + { + field: "old_password", + detail: "Old password is incorrect.", }, - }, - isAxiosError: true, - }, + ], + }), initialTouched: { old_password: true, }, diff --git a/site/src/components/SignInForm/SignInForm.stories.tsx b/site/src/components/SignInForm/SignInForm.stories.tsx index 04b70e3628878..ea3610e7ba49d 100644 --- a/site/src/components/SignInForm/SignInForm.stories.tsx +++ b/site/src/components/SignInForm/SignInForm.stories.tsx @@ -1,5 +1,5 @@ import { Story } from "@storybook/react" -import { makeMockValidationApiError } from "testHelpers/entities" +import { mockApiError } from "testHelpers/entities" import { SignInForm, SignInFormProps } from "./SignInForm" export default { @@ -37,7 +37,7 @@ SigningIn.args = { export const WithError = Template.bind({}) WithError.args = { ...SignedOut.args, - error: makeMockValidationApiError({ + error: mockApiError({ message: "Email or password was invalid", validations: [ { diff --git a/site/src/components/Workspace/Workspace.stories.tsx b/site/src/components/Workspace/Workspace.stories.tsx index b952b6c2c819a..dd93520ba6a15 100644 --- a/site/src/components/Workspace/Workspace.stories.tsx +++ b/site/src/components/Workspace/Workspace.stories.tsx @@ -95,7 +95,7 @@ Failed.args = { ...Running.args, workspace: Mocks.MockFailedWorkspace, workspaceErrors: { - [WorkspaceErrors.BUILD_ERROR]: Mocks.makeMockValidationApiError({ + [WorkspaceErrors.BUILD_ERROR]: Mocks.mockApiError({ message: "A workspace build is already active.", }), }, @@ -152,7 +152,7 @@ export const GetBuildsError = Template.bind({}) GetBuildsError.args = { ...Running.args, workspaceErrors: { - [WorkspaceErrors.GET_BUILDS_ERROR]: Mocks.makeMockValidationApiError({ + [WorkspaceErrors.GET_BUILDS_ERROR]: Mocks.mockApiError({ message: "There is a problem fetching builds.", }), }, @@ -162,7 +162,7 @@ export const CancellationError = Template.bind({}) CancellationError.args = { ...Failed.args, workspaceErrors: { - [WorkspaceErrors.CANCELLATION_ERROR]: Mocks.makeMockValidationApiError({ + [WorkspaceErrors.CANCELLATION_ERROR]: Mocks.mockApiError({ message: "Job could not be canceled.", }), }, diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx index 742ff5270adc9..0ca9d30c69a3c 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx @@ -8,7 +8,7 @@ import { emptySchedule, } from "pages/WorkspaceSettingsPage/WorkspaceSchedulePage/schedule" import { emptyTTL } from "pages/WorkspaceSettingsPage/WorkspaceSchedulePage/ttl" -import { makeMockValidationApiError } from "testHelpers/entities" +import { mockApiError } from "testHelpers/entities" import { WorkspaceScheduleForm, WorkspaceScheduleFormProps, @@ -81,7 +81,7 @@ export const WithError = Template.bind({}) WithError.args = { initialValues: { ...defaultInitialValues, ttl: 100 }, initialTouched: { ttl: true }, - submitScheduleError: makeMockValidationApiError({ + submitScheduleError: mockApiError({ message: "Something went wrong.", validations: [{ field: "ttl_ms", detail: "Invalid time until shutdown." }], }), diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index 77823148099a9..c548580f35797 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -1,6 +1,6 @@ import { ComponentMeta, Story } from "@storybook/react" import { - makeMockValidationApiError, + mockApiError, mockParameterSchema, MockParameterSchemas, MockTemplate, @@ -85,7 +85,7 @@ export const GetTemplatesError = Template.bind({}) GetTemplatesError.args = { ...Parameters.args, createWorkspaceErrors: { - [CreateWorkspaceErrors.GET_TEMPLATES_ERROR]: makeMockValidationApiError({ + [CreateWorkspaceErrors.GET_TEMPLATES_ERROR]: mockApiError({ message: "Failed to fetch templates.", detail: "You do not have permission to access this resource.", }), @@ -97,11 +97,10 @@ export const GetTemplateSchemaError = Template.bind({}) GetTemplateSchemaError.args = { ...Parameters.args, createWorkspaceErrors: { - [CreateWorkspaceErrors.GET_TEMPLATE_SCHEMA_ERROR]: - makeMockValidationApiError({ - message: 'Failed to fetch template schema for "docker-amd64".', - detail: "You do not have permission to access this resource.", - }), + [CreateWorkspaceErrors.GET_TEMPLATE_SCHEMA_ERROR]: mockApiError({ + message: 'Failed to fetch template schema for "docker-amd64".', + detail: "You do not have permission to access this resource.", + }), }, hasTemplateErrors: true, } @@ -110,7 +109,7 @@ export const CreateWorkspaceError = Template.bind({}) CreateWorkspaceError.args = { ...Parameters.args, createWorkspaceErrors: { - [CreateWorkspaceErrors.CREATE_WORKSPACE_ERROR]: makeMockValidationApiError({ + [CreateWorkspaceErrors.CREATE_WORKSPACE_ERROR]: mockApiError({ message: 'Workspace "test" already exists in the "docker-amd64" template.', validations: [ diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index a25c4b1aa6881..2a027ef7e9c41 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -1,8 +1,5 @@ import { ComponentMeta, Story } from "@storybook/react" -import { - makeMockValidationApiError, - MockDeploymentDAUResponse, -} from "testHelpers/entities" +import { mockApiError, MockDeploymentDAUResponse } from "testHelpers/entities" import { GeneralSettingsPageView, GeneralSettingsPageViewProps, @@ -43,7 +40,7 @@ NoDAUs.args = { export const DAUError = Template.bind({}) DAUError.args = { deploymentDAUs: undefined, - getDeploymentDAUsError: makeMockValidationApiError({ + getDeploymentDAUsError: mockApiError({ message: "Error fetching DAUs.", }), } diff --git a/site/src/pages/StarterTemplatePage/StarterTemplatePageView.stories.tsx b/site/src/pages/StarterTemplatePage/StarterTemplatePageView.stories.tsx index 1d852425119f8..5f373e32ae958 100644 --- a/site/src/pages/StarterTemplatePage/StarterTemplatePageView.stories.tsx +++ b/site/src/pages/StarterTemplatePage/StarterTemplatePageView.stories.tsx @@ -1,6 +1,6 @@ import { Story } from "@storybook/react" import { - makeMockValidationApiError, + mockApiError, MockOrganization, MockTemplateExample, } from "testHelpers/entities" @@ -33,7 +33,7 @@ Error.args = { context: { exampleId: MockTemplateExample.id, organizationId: MockOrganization.id, - error: makeMockValidationApiError({ + error: mockApiError({ message: `Example ${MockTemplateExample.id} not found.`, }), starterTemplate: undefined, diff --git a/site/src/pages/StarterTemplatesPage/StarterTemplatesPageView.stories.tsx b/site/src/pages/StarterTemplatesPage/StarterTemplatesPageView.stories.tsx index 1217759958e58..b5c4f52b95a5c 100644 --- a/site/src/pages/StarterTemplatesPage/StarterTemplatesPageView.stories.tsx +++ b/site/src/pages/StarterTemplatesPage/StarterTemplatesPageView.stories.tsx @@ -1,6 +1,6 @@ import { Story } from "@storybook/react" import { - makeMockValidationApiError, + mockApiError, MockOrganization, MockTemplateExample, MockTemplateExample2, @@ -36,7 +36,7 @@ export const Error = Template.bind({}) Error.args = { context: { organizationId: MockOrganization.id, - error: makeMockValidationApiError({ + error: mockApiError({ message: "Error on loading the template examples", }), starterTemplatesByTag: undefined, diff --git a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx index 7ef92d0732202..d5f61f588a427 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateGeneralSettingsPage/TemplateSettingsPageView.stories.tsx @@ -1,6 +1,6 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" -import { makeMockValidationApiError, MockTemplate } from "testHelpers/entities" +import { mockApiError, MockTemplate } from "testHelpers/entities" import { TemplateSettingsPageView, TemplateSettingsPageViewProps, @@ -25,7 +25,7 @@ Example.args = {} export const SaveTemplateSettingsError = Template.bind({}) SaveTemplateSettingsError.args = { - submitError: makeMockValidationApiError({ + submitError: mockApiError({ message: 'Template "test" already exists.', validations: [ { diff --git a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx index c2222c5388cf4..2156893b62633 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateVariablesPage/TemplateVariablesPageView.stories.tsx @@ -1,7 +1,7 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" import { - makeMockValidationApiError, + mockApiError, MockTemplateVersion, MockTemplateVersionVariable1, MockTemplateVersionVariable2, @@ -69,7 +69,7 @@ WithUpdateTemplateError.args = { MockTemplateVersionVariable4, ], errors: { - updateTemplateError: makeMockValidationApiError({ + updateTemplateError: mockApiError({ message: "Something went wrong.", }), }, diff --git a/site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx b/site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx index e3a6732ab1dbe..cf7982d2f46fc 100644 --- a/site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx +++ b/site/src/pages/TemplateVersionPage/TemplateVersionPageView.stories.tsx @@ -2,7 +2,7 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" import { UseTabResult } from "hooks/useTab" import { - makeMockValidationApiError, + mockApiError, MockOrganization, MockTemplate, MockTemplateVersion, @@ -64,7 +64,7 @@ Error.args = { ...defaultArgs.context, currentVersion: undefined, currentFiles: undefined, - error: makeMockValidationApiError({ + error: mockApiError({ message: "Error on loading the template version", }), }, diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx index 46ad393847526..2c74729dbc75c 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.stories.tsx @@ -1,6 +1,6 @@ import { ComponentMeta, Story } from "@storybook/react" import { - makeMockValidationApiError, + mockApiError, MockOrganization, MockPermissions, MockTemplate, @@ -89,7 +89,7 @@ Error.args = { ...MockPermissions, createTemplates: false, }, - error: makeMockValidationApiError({ + error: mockApiError({ message: "Something went wrong fetching templates.", }), templates: undefined, diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx index 9a52ac71fcda2..b7576f58803c8 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx @@ -5,6 +5,7 @@ import { renderWithAuth } from "../../../testHelpers/renderHelpers" import * as AuthXService from "../../../xServices/auth/authXService" import { AccountPage } from "./AccountPage" import i18next from "i18next" +import { mockApiError } from "testHelpers/entities" const { t } = i18next @@ -54,17 +55,14 @@ describe("AccountPage", () => { describe("when the username is already taken", () => { it("shows an error", async () => { - jest.spyOn(API, "updateProfile").mockRejectedValueOnce({ - isAxiosError: true, - response: { - data: { - message: "Invalid profile", - validations: [ - { detail: "Username is already in use", field: "username" }, - ], - }, - }, - }) + jest.spyOn(API, "updateProfile").mockRejectedValueOnce( + mockApiError({ + message: "Invalid profile", + validations: [ + { detail: "Username is already in use", field: "username" }, + ], + }), + ) const { user } = renderPage() await fillAndSubmitForm() diff --git a/site/src/pages/UserSettingsPage/SSHKeysPage/SSHKeysPageView.stories.tsx b/site/src/pages/UserSettingsPage/SSHKeysPage/SSHKeysPageView.stories.tsx index 33d761a4ae285..9b51e231adb29 100644 --- a/site/src/pages/UserSettingsPage/SSHKeysPage/SSHKeysPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/SSHKeysPage/SSHKeysPageView.stories.tsx @@ -1,5 +1,5 @@ import { Story } from "@storybook/react" -import { makeMockValidationApiError } from "testHelpers/entities" +import { mockApiError } from "testHelpers/entities" import { SSHKeysPageView, SSHKeysPageViewProps } from "./SSHKeysPageView" export default { @@ -39,7 +39,7 @@ export const WithGetSSHKeyError = Template.bind({}) WithGetSSHKeyError.args = { ...Example.args, hasLoaded: false, - getSSHKeyError: makeMockValidationApiError({ + getSSHKeyError: mockApiError({ message: "Failed to get SSH key", }), } @@ -47,7 +47,7 @@ WithGetSSHKeyError.args = { export const WithRegenerateSSHKeyError = Template.bind({}) WithRegenerateSSHKeyError.args = { ...Example.args, - regenerateSSHKeyError: makeMockValidationApiError({ + regenerateSSHKeyError: mockApiError({ message: "Failed to regenerate SSH key", }), } diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx index f7aac577c7a9d..9a809d4b57602 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx @@ -4,6 +4,7 @@ import * as SecurityForm from "../../../components/SettingsSecurityForm/Settings import { renderWithAuth } from "../../../testHelpers/renderHelpers" import { SecurityPage } from "./SecurityPage" import i18next from "i18next" +import { mockApiError } from "testHelpers/entities" const { t } = i18next @@ -54,17 +55,14 @@ describe("SecurityPage", () => { describe("when the old_password is incorrect", () => { it("shows an error", async () => { - jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce({ - isAxiosError: true, - response: { - data: { - message: "Incorrect password.", - validations: [ - { detail: "Incorrect password.", field: "old_password" }, - ], - }, - }, - }) + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( + mockApiError({ + message: "Incorrect password.", + validations: [ + { detail: "Incorrect password.", field: "old_password" }, + ], + }), + ) const { user } = renderPage() await fillAndSubmitForm() @@ -79,15 +77,12 @@ describe("SecurityPage", () => { describe("when the password is invalid", () => { it("shows an error", async () => { - jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce({ - isAxiosError: true, - response: { - data: { - message: "Invalid password.", - validations: [{ detail: "Invalid password.", field: "password" }], - }, - }, - }) + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( + mockApiError({ + message: "Invalid password.", + validations: [{ detail: "Invalid password.", field: "password" }], + }), + ) const { user } = renderPage() await fillAndSubmitForm() diff --git a/site/src/pages/UserSettingsPage/TokensPage/TokensPageView.stories.tsx b/site/src/pages/UserSettingsPage/TokensPage/TokensPageView.stories.tsx index 5ef3677c8fe88..6748f53b74ba4 100644 --- a/site/src/pages/UserSettingsPage/TokensPage/TokensPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/TokensPage/TokensPageView.stories.tsx @@ -1,5 +1,5 @@ import { Story } from "@storybook/react" -import { makeMockValidationApiError, MockTokens } from "testHelpers/entities" +import { mockApiError, MockTokens } from "testHelpers/entities" import { TokensPageView, TokensPageViewProps } from "./TokensPageView" export default { @@ -41,7 +41,7 @@ export const WithGetTokensError = Template.bind({}) WithGetTokensError.args = { ...Example.args, hasLoaded: false, - getTokensError: makeMockValidationApiError({ + getTokensError: mockApiError({ message: "Failed to get tokens.", }), } @@ -50,7 +50,7 @@ export const WithDeleteTokenError = Template.bind({}) WithDeleteTokenError.args = { ...Example.args, hasLoaded: false, - deleteTokenError: makeMockValidationApiError({ + deleteTokenError: mockApiError({ message: "Failed to delete token.", }), } diff --git a/site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorspaceProxyView.stories.tsx b/site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorspaceProxyView.stories.tsx index 9e74e3b48bb21..56e2c4563fde4 100644 --- a/site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorspaceProxyView.stories.tsx +++ b/site/src/pages/UserSettingsPage/WorkspaceProxyPage/WorspaceProxyView.stories.tsx @@ -1,6 +1,6 @@ import { Story } from "@storybook/react" import { - makeMockValidationApiError, + mockApiError, MockWorkspaceProxies, MockPrimaryWorkspaceProxy, MockHealthyWildWorkspaceProxy, @@ -61,7 +61,7 @@ export const WithProxiesError = Template.bind({}) WithProxiesError.args = { ...Example.args, hasLoaded: false, - getWorkspaceProxiesError: makeMockValidationApiError({ + getWorkspaceProxiesError: mockApiError({ message: "Failed to get proxies.", }), } @@ -70,7 +70,7 @@ export const WithSelectProxyError = Template.bind({}) WithSelectProxyError.args = { ...Example.args, hasLoaded: false, - selectProxyError: makeMockValidationApiError({ + selectProxyError: mockApiError({ message: "Failed to select proxy.", }), } diff --git a/site/src/pages/UsersPage/UsersPageView.stories.tsx b/site/src/pages/UsersPage/UsersPageView.stories.tsx index 3449a0ea9192a..f51a4d4f013bc 100644 --- a/site/src/pages/UsersPage/UsersPageView.stories.tsx +++ b/site/src/pages/UsersPage/UsersPageView.stories.tsx @@ -54,6 +54,5 @@ Error.args = { ], }, }, - isAxiosError: true, }, } diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index f2c0b715c3e5a..46ce3b64935ab 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1253,6 +1253,7 @@ type MockAPIInput = { } type MockAPIOutput = { + isAxiosError: true response: { data: { message: string @@ -1260,14 +1261,15 @@ type MockAPIOutput = { validations: FieldError[] | undefined } } - isAxiosError: boolean } -export const makeMockValidationApiError = ({ +export const mockApiError = ({ message, detail, validations, }: MockAPIInput): MockAPIOutput => ({ + // This is how axios can check if it is an axios error when calling isAxiosError + isAxiosError: true, response: { data: { message: message ?? "Something went wrong.", @@ -1275,7 +1277,6 @@ export const makeMockValidationApiError = ({ validations: validations ?? undefined, }, }, - isAxiosError: true, }) export const MockEntitlements: TypesGen.Entitlements = { diff --git a/site/src/utils/formUtils.test.ts b/site/src/utils/formUtils.test.ts index ebdc09e04a572..ec495d50c1b20 100644 --- a/site/src/utils/formUtils.test.ts +++ b/site/src/utils/formUtils.test.ts @@ -1,6 +1,6 @@ import { FormikContextType } from "formik/dist/types" import { getFormHelpers, nameValidator, onChangeTrimmed } from "./formUtils" -import { makeMockValidationApiError } from "testHelpers/entities" +import { mockApiError } from "testHelpers/entities" interface TestType { untouchedGoodField: string @@ -72,7 +72,7 @@ describe("form util functions", () => { it("shows an error if there is only an API error", () => { const getFieldHelpers = getFormHelpers( form, - makeMockValidationApiError({ + mockApiError({ validations: [ { field: "touchedGoodField", @@ -94,7 +94,7 @@ describe("form util functions", () => { it("shows the API error if both are present", () => { const getFieldHelpers = getFormHelpers( form, - makeMockValidationApiError({ + mockApiError({ validations: [ { field: "touchedBadField", diff --git a/site/src/utils/formUtils.ts b/site/src/utils/formUtils.ts index a92c8d4034e59..2a01eb14c0b04 100644 --- a/site/src/utils/formUtils.ts +++ b/site/src/utils/formUtils.ts @@ -35,14 +35,22 @@ interface FormHelpers { export const getFormHelpers = (form: FormikContextType, error?: unknown) => - (fieldName: keyof TFormValues, helperText?: ReactNode): FormHelpers => { + ( + fieldName: keyof TFormValues, + helperText?: ReactNode, + // backendFieldName is used when the value in the form is named different from the backend + backendFieldName?: string, + ): FormHelpers => { const apiValidationErrors = isApiValidationError(error) ? (mapApiErrorToFieldErrors( error.response.data, - ) as FormikErrors) + ) as FormikErrors & { [key: string]: string }) : undefined const touched = Boolean(form.touched[fieldName]) - const apiError = apiValidationErrors?.[fieldName]?.toString() + // Since the field in the form can be diff from the backend, we need to + // check for both when getting the error + const apiField = backendFieldName ?? fieldName + const apiError = apiValidationErrors?.[apiField.toString()] const formError = form.errors[fieldName]?.toString() const errorToDisplay = apiError ?? formError From 2bbbb817b5f5c5017a8526ed4ea5f54a3bac4bb6 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 4 May 2023 14:39:45 +0000 Subject: [PATCH 4/9] Fix user test --- .../CreateUserForm/CreateUserForm.tsx | 11 ++-- .../CreateUserPage/CreateUserPage.test.tsx | 17 ++----- .../CreateUserPage/CreateUserPage.tsx | 11 +--- .../src/xServices/users/createUserXService.ts | 51 ++++--------------- 4 files changed, 19 insertions(+), 71 deletions(-) diff --git a/site/src/components/CreateUserForm/CreateUserForm.tsx b/site/src/components/CreateUserForm/CreateUserForm.tsx index ca1ad4c2bc406..3d00304c1efdd 100644 --- a/site/src/components/CreateUserForm/CreateUserForm.tsx +++ b/site/src/components/CreateUserForm/CreateUserForm.tsx @@ -1,6 +1,5 @@ -import FormHelperText from "@material-ui/core/FormHelperText" import TextField from "@material-ui/core/TextField" -import { FormikContextType, FormikErrors, useFormik } from "formik" +import { FormikContextType, useFormik } from "formik" import { FC } from "react" import * as Yup from "yup" import * as TypesGen from "../../api/typesGenerated" @@ -27,9 +26,8 @@ export const Language = { export interface CreateUserFormProps { onSubmit: (user: TypesGen.CreateUserRequest) => void onCancel: () => void - formErrors?: FormikErrors + error?: unknown isLoading: boolean - error?: string myOrgId: string } @@ -44,7 +42,7 @@ const validationSchema = Yup.object({ export const CreateUserForm: FC< React.PropsWithChildren -> = ({ onSubmit, onCancel, formErrors, isLoading, error, myOrgId }) => { +> = ({ onSubmit, onCancel, error, isLoading, myOrgId }) => { const form: FormikContextType = useFormik({ initialValues: { @@ -58,7 +56,7 @@ export const CreateUserForm: FC< }) const getFieldHelpers = getFormHelpers( form, - formErrors, + error, ) return ( @@ -92,7 +90,6 @@ export const CreateUserForm: FC< variant="outlined" /> - {error && {error}} diff --git a/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.test.tsx b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.test.tsx index c55e574b8cfb2..56b69d75a246d 100644 --- a/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.test.tsx +++ b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.test.tsx @@ -1,7 +1,6 @@ import { fireEvent, screen } from "@testing-library/react" import userEvent from "@testing-library/user-event" import { rest } from "msw" -import * as API from "../../../api/api" import { Language as FormLanguage } from "../../../components/CreateUserForm/CreateUserForm" import { Language as FooterLanguage } from "../../../components/FormFooter/FormFooter" import { @@ -14,7 +13,9 @@ import { Language as CreateUserLanguage } from "../../../xServices/users/createU import { CreateUserPage } from "./CreateUserPage" const renderCreateUserPage = async () => { - renderWithAuth() + renderWithAuth(, { + extraRoutes: [{ path: "/users", element:
Users Page
}], + }) await waitForLoaderToBeRemoved() } @@ -51,18 +52,6 @@ describe("Create User Page", () => { expect(errorMessage).toBeDefined() }) - it("shows generic error message", async () => { - jest.spyOn(API, "createUser").mockRejectedValueOnce({ - data: "unknown error", - }) - await renderCreateUserPage() - await fillForm({}) - const errorMessage = await screen.findByText( - CreateUserLanguage.createUserError, - ) - expect(errorMessage).toBeDefined() - }) - it("shows API error message", async () => { const fieldErrorMessage = "username already in use" server.use( diff --git a/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx index 67385e3de0630..1855b81cdc326 100644 --- a/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx +++ b/site/src/pages/UsersPage/CreateUserPage/CreateUserPage.tsx @@ -23,13 +23,7 @@ export const CreateUserPage: FC = () => { }, }, }) - const { createUserErrorMessage, createUserFormErrors } = - createUserState.context - // There is no field for organization id in Community Edition, so handle its field error like a generic error - const genericError = - createUserErrorMessage || - createUserFormErrors?.organization_id || - (!myOrgId ? Language.unknownError : undefined) + const { error } = createUserState.context return ( @@ -37,7 +31,7 @@ export const CreateUserPage: FC = () => { {pageTitle("Create User")} createUserSend({ type: "CREATE", user }) } @@ -46,7 +40,6 @@ export const CreateUserPage: FC = () => { navigate("/users") }} isLoading={createUserState.hasTag("loading")} - error={genericError} myOrgId={myOrgId} /> diff --git a/site/src/xServices/users/createUserXService.ts b/site/src/xServices/users/createUserXService.ts index d22a3754ff1e5..ddc0e0db08477 100644 --- a/site/src/xServices/users/createUserXService.ts +++ b/site/src/xServices/users/createUserXService.ts @@ -1,24 +1,14 @@ import { assign, createMachine } from "xstate" import * as API from "../../api/api" -import { - ApiError, - FieldErrors, - getErrorMessage, - hasApiFieldErrors, - isApiError, - mapApiErrorToFieldErrors, -} from "../../api/errors" import * as TypesGen from "../../api/typesGenerated" import { displaySuccess } from "../../components/GlobalSnackbar/utils" export const Language = { createUserSuccess: "Successfully created user.", - createUserError: "Error on creating the user.", } export interface CreateUserContext { - createUserErrorMessage?: string - createUserFormErrors?: FieldErrors + error?: unknown } export type CreateUserEvent = @@ -44,11 +34,11 @@ export const createUserMachine = createMachine( idle: { on: { CREATE: "creatingUser", - CANCEL_CREATE_USER: { actions: ["clearCreateUserError"] }, + CANCEL_CREATE_USER: { actions: ["clearError"] }, }, }, creatingUser: { - entry: "clearCreateUserError", + entry: "clearError", invoke: { src: "createUser", id: "createUser", @@ -56,17 +46,10 @@ export const createUserMachine = createMachine( target: "idle", actions: ["displayCreateUserSuccess", "redirectToUsersPage"], }, - onError: [ - { - target: "idle", - cond: "hasFieldErrors", - actions: ["assignCreateUserFormErrors"], - }, - { - target: "idle", - actions: ["assignCreateUserError"], - }, - ], + onError: { + target: "idle", + actions: ["assignError"], + }, }, tags: "loading", }, @@ -76,25 +59,11 @@ export const createUserMachine = createMachine( services: { createUser: (_, event) => API.createUser(event.user), }, - guards: { - hasFieldErrors: (_, event) => - isApiError(event.data) && hasApiFieldErrors(event.data), - }, actions: { - assignCreateUserError: assign({ - createUserErrorMessage: (_, event) => - getErrorMessage(event.data, Language.createUserError), - }), - assignCreateUserFormErrors: assign({ - // the guard ensures it is ApiError - createUserFormErrors: (_, event) => - mapApiErrorToFieldErrors((event.data as ApiError).response.data), + assignError: assign({ + error: (_, event) => event.data, }), - clearCreateUserError: assign((context: CreateUserContext) => ({ - ...context, - createUserErrorMessage: undefined, - createUserFormErrors: undefined, - })), + clearError: assign({ error: (_) => undefined }), displayCreateUserSuccess: () => { displaySuccess(Language.createUserSuccess) }, From 668092411c8376201296d33f415f4fcfb0f3069e Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 4 May 2023 14:47:28 +0000 Subject: [PATCH 5/9] Fix setup logic --- site/src/pages/SetupPage/SetupPage.test.tsx | 17 +------ site/src/pages/SetupPage/SetupPage.tsx | 6 +-- .../pages/SetupPage/SetupPageView.stories.tsx | 18 ++----- site/src/pages/SetupPage/SetupPageView.tsx | 14 ++---- site/src/xServices/setup/setupXService.ts | 48 ++++--------------- 5 files changed, 23 insertions(+), 80 deletions(-) diff --git a/site/src/pages/SetupPage/SetupPage.test.tsx b/site/src/pages/SetupPage/SetupPage.test.tsx index e3ec187649e17..616e3439372f6 100644 --- a/site/src/pages/SetupPage/SetupPage.test.tsx +++ b/site/src/pages/SetupPage/SetupPage.test.tsx @@ -1,12 +1,11 @@ import { fireEvent, screen, waitFor } from "@testing-library/react" import userEvent from "@testing-library/user-event" -import * as API from "api/api" import { rest } from "msw" -import { history, MockUser, render } from "testHelpers/renderHelpers" +import { history, render } from "testHelpers/renderHelpers" import { server } from "testHelpers/server" -import { Language as SetupLanguage } from "xServices/setup/setupXService" import { SetupPage } from "./SetupPage" import { Language as PageViewLanguage } from "./SetupPageView" +import { MockUser } from "testHelpers/entities" const fillForm = async ({ username = "someuser", @@ -47,18 +46,6 @@ describe("Setup Page", () => { expect(errorMessage).toBeDefined() }) - it("shows generic error message", async () => { - jest.spyOn(API, "createFirstUser").mockRejectedValueOnce({ - data: "unknown error", - }) - render() - await fillForm() - const errorMessage = await screen.findByText( - SetupLanguage.createFirstUserError, - ) - expect(errorMessage).toBeDefined() - }) - it("shows API error message", async () => { const fieldErrorMessage = "invalid username" server.use( diff --git a/site/src/pages/SetupPage/SetupPage.tsx b/site/src/pages/SetupPage/SetupPage.tsx index c243b9066ce91..56c9b9b78f5f0 100644 --- a/site/src/pages/SetupPage/SetupPage.tsx +++ b/site/src/pages/SetupPage/SetupPage.tsx @@ -22,8 +22,7 @@ export const SetupPage: FC = () => { }, }, }) - const { createFirstUserFormErrors, createFirstUserErrorMessage } = - setupState.context + const { error } = setupState.context useEffect(() => { if (authState.matches("signedIn")) { @@ -38,8 +37,7 @@ export const SetupPage: FC = () => { { setupSend({ type: "CREATE_FIRST_USER", firstUser }) }} diff --git a/site/src/pages/SetupPage/SetupPageView.stories.tsx b/site/src/pages/SetupPage/SetupPageView.stories.tsx index b3a5684806642..d39e62a2626c6 100644 --- a/site/src/pages/SetupPage/SetupPageView.stories.tsx +++ b/site/src/pages/SetupPage/SetupPageView.stories.tsx @@ -1,6 +1,7 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" import { SetupPageView, SetupPageViewProps } from "./SetupPageView" +import { mockApiError } from "testHelpers/entities" export default { title: "pages/SetupPageView", @@ -14,27 +15,18 @@ const Template: Story = (args: SetupPageViewProps) => ( export const Ready = Template.bind({}) Ready.args = { onSubmit: action("submit"), - isCreating: false, -} - -export const UnknownError = Template.bind({}) -UnknownError.args = { - onSubmit: action("submit"), - isCreating: false, - genericError: "Something went wrong", } export const FormError = Template.bind({}) FormError.args = { onSubmit: action("submit"), - isCreating: false, - formErrors: { - username: "Username taken", - }, + error: mockApiError({ + validations: [{ field: "username", detail: "Username taken" }], + }), } export const Loading = Template.bind({}) Loading.args = { onSubmit: action("submit"), - isCreating: true, + isLoading: true, } diff --git a/site/src/pages/SetupPage/SetupPageView.tsx b/site/src/pages/SetupPage/SetupPageView.tsx index fb000357f770f..cde67b896efe5 100644 --- a/site/src/pages/SetupPage/SetupPageView.tsx +++ b/site/src/pages/SetupPage/SetupPageView.tsx @@ -1,6 +1,5 @@ import Box from "@material-ui/core/Box" import Checkbox from "@material-ui/core/Checkbox" -import FormHelperText from "@material-ui/core/FormHelperText" import { makeStyles } from "@material-ui/core/styles" import TextField from "@material-ui/core/TextField" import Typography from "@material-ui/core/Typography" @@ -8,7 +7,7 @@ import { LoadingButton } from "components/LoadingButton/LoadingButton" import { SignInLayout } from "components/SignInLayout/SignInLayout" import { Stack } from "components/Stack/Stack" import { Welcome } from "components/Welcome/Welcome" -import { FormikContextType, FormikErrors, useFormik } from "formik" +import { FormikContextType, useFormik } from "formik" import { getFormHelpers, nameValidator, onChangeTrimmed } from "utils/formUtils" import * as Yup from "yup" import * as TypesGen from "../../api/typesGenerated" @@ -35,15 +34,13 @@ const validationSchema = Yup.object({ export interface SetupPageViewProps { onSubmit: (firstUser: TypesGen.CreateFirstUserRequest) => void - formErrors?: FormikErrors - genericError?: string + error?: unknown isLoading?: boolean } export const SetupPageView: React.FC = ({ onSubmit, - formErrors, - genericError, + error, isLoading, }) => { const form: FormikContextType = @@ -59,7 +56,7 @@ export const SetupPageView: React.FC = ({ }) const getFieldHelpers = getFormHelpers( form, - formErrors, + error, ) const styles = useStyles() @@ -93,9 +90,6 @@ export const SetupPageView: React.FC = ({ type="password" variant="outlined" /> - {genericError && ( - {genericError} - )}
diff --git a/site/src/xServices/setup/setupXService.ts b/site/src/xServices/setup/setupXService.ts index 9ae29c67681f0..e0ce5eecd8557 100644 --- a/site/src/xServices/setup/setupXService.ts +++ b/site/src/xServices/setup/setupXService.ts @@ -1,12 +1,4 @@ import * as API from "api/api" -import { - ApiError, - FieldErrors, - getErrorMessage, - hasApiFieldErrors, - isApiError, - mapApiErrorToFieldErrors, -} from "api/errors" import * as TypesGen from "api/typesGenerated" import { assign, createMachine } from "xstate" @@ -15,8 +7,7 @@ export const Language = { } export interface SetupContext { - createFirstUserErrorMessage?: string - createFirstUserFormErrors?: FieldErrors + error?: unknown firstUser?: TypesGen.CreateFirstUserRequest } @@ -52,7 +43,7 @@ export const setupMachine = }, }, creatingFirstUser: { - entry: "clearCreateFirstUserError", + entry: "clearError", invoke: { src: "createFirstUser", id: "createFirstUser", @@ -62,17 +53,10 @@ export const setupMachine = target: "firstUserCreated", }, ], - onError: [ - { - actions: "assignCreateFirstUserFormErrors", - cond: "hasFieldErrors", - target: "idle", - }, - { - actions: "assignCreateFirstUserError", - target: "idle", - }, - ], + onError: { + actions: "assignError", + target: "idle", + }, }, tags: "loading", }, @@ -86,28 +70,16 @@ export const setupMachine = services: { createFirstUser: (_, event) => API.createFirstUser(event.firstUser), }, - guards: { - hasFieldErrors: (_, event) => - isApiError(event.data) && hasApiFieldErrors(event.data), - }, actions: { assignFirstUserData: assign({ firstUser: (_, event) => event.firstUser, }), - assignCreateFirstUserError: assign({ - createFirstUserErrorMessage: (_, event) => - getErrorMessage(event.data, Language.createFirstUserError), + assignError: assign({ + error: (_, event) => event.data, }), - assignCreateFirstUserFormErrors: assign({ - // the guard ensures it is ApiError - createFirstUserFormErrors: (_, event) => - mapApiErrorToFieldErrors((event.data as ApiError).response.data), + clearError: assign({ + error: (_) => undefined, }), - clearCreateFirstUserError: assign((context: SetupContext) => ({ - ...context, - createFirstUserErrorMessage: undefined, - createFirstUserFormErrors: undefined, - })), }, }, ) From 7ea15a3d57a6625a3c3937b7a07d5a9df552a822 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 4 May 2023 14:50:41 +0000 Subject: [PATCH 6/9] Refactor edit group --- .../pages/GroupsPage/SettingsGroupPage.tsx | 4 +-- .../src/xServices/groups/editGroupXService.ts | 36 ++++--------------- 2 files changed, 9 insertions(+), 31 deletions(-) diff --git a/site/src/pages/GroupsPage/SettingsGroupPage.tsx b/site/src/pages/GroupsPage/SettingsGroupPage.tsx index 8abf07eec244f..2849650013d7f 100644 --- a/site/src/pages/GroupsPage/SettingsGroupPage.tsx +++ b/site/src/pages/GroupsPage/SettingsGroupPage.tsx @@ -26,7 +26,7 @@ export const SettingsGroupPage: FC = () => { onUpdate: navigateToGroup, }, }) - const { updateGroupFormErrors, group } = editState.context + const { error, group } = editState.context return ( <> @@ -40,7 +40,7 @@ export const SettingsGroupPage: FC = () => { sendEditEvent({ type: "UPDATE", data }) }} group={group} - formErrors={updateGroupFormErrors} + formErrors={error} isLoading={editState.matches("loading")} isUpdating={editState.matches("updating")} /> diff --git a/site/src/xServices/groups/editGroupXService.ts b/site/src/xServices/groups/editGroupXService.ts index 93c2311825b8f..bdf43eeda32f4 100644 --- a/site/src/xServices/groups/editGroupXService.ts +++ b/site/src/xServices/groups/editGroupXService.ts @@ -1,11 +1,5 @@ import { getGroup, patchGroup } from "api/api" -import { - ApiError, - getErrorMessage, - hasApiFieldErrors, - isApiError, - mapApiErrorToFieldErrors, -} from "api/errors" +import { getErrorMessage } from "api/errors" import { Group } from "api/typesGenerated" import { displayError } from "components/GlobalSnackbar/utils" import { assign, createMachine } from "xstate" @@ -17,7 +11,7 @@ export const editGroupMachine = createMachine( context: {} as { groupId: string group?: Group - updateGroupFormErrors?: unknown + error?: unknown }, services: {} as { loadGroup: { @@ -61,26 +55,15 @@ export const editGroupMachine = createMachine( onDone: { actions: ["onUpdate"], }, - onError: [ - { - target: "idle", - cond: "hasFieldErrors", - actions: ["assignUpdateGroupFormErrors"], - }, - { - target: "idle", - actions: ["displayUpdateGroupError"], - }, - ], + onError: { + target: "idle", + actions: ["assignError"], + }, }, }, }, }, { - guards: { - hasFieldErrors: (_, event) => - isApiError(event.data) && hasApiFieldErrors(event.data), - }, services: { loadGroup: ({ groupId }) => getGroup(groupId), @@ -104,12 +87,7 @@ export const editGroupMachine = createMachine( const message = getErrorMessage(data, "Failed to the group.") displayError(message) }, - displayUpdateGroupError: (_, { data }) => { - const message = getErrorMessage(data, "Failed to update the group.") - displayError(message) - }, - assignUpdateGroupFormErrors: (_, event) => - mapApiErrorToFieldErrors((event.data as ApiError).response.data), + assignError: (_, event) => event.data, }, }, ) From 3fcaabf47558ff28f3eb756f70514fbe68e62209 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 4 May 2023 14:55:32 +0000 Subject: [PATCH 7/9] Refactor create group page --- site/src/pages/GroupsPage/CreateGroupPage.tsx | 4 +- .../xServices/groups/createGroupXService.ts | 40 +++++-------------- .../src/xServices/groups/editGroupXService.ts | 4 +- 3 files changed, 14 insertions(+), 34 deletions(-) diff --git a/site/src/pages/GroupsPage/CreateGroupPage.tsx b/site/src/pages/GroupsPage/CreateGroupPage.tsx index db2cf075bf4a0..78ec70b68b21e 100644 --- a/site/src/pages/GroupsPage/CreateGroupPage.tsx +++ b/site/src/pages/GroupsPage/CreateGroupPage.tsx @@ -20,7 +20,7 @@ export const CreateGroupPage: FC = () => { }, }, }) - const { createGroupFormErrors } = createState.context + const { error } = createState.context return ( <> @@ -34,7 +34,7 @@ export const CreateGroupPage: FC = () => { data, }) }} - formErrors={createGroupFormErrors} + formErrors={error} isLoading={createState.matches("creatingGroup")} /> diff --git a/site/src/xServices/groups/createGroupXService.ts b/site/src/xServices/groups/createGroupXService.ts index 7faf4ee6fb9ad..404c79325d0bb 100644 --- a/site/src/xServices/groups/createGroupXService.ts +++ b/site/src/xServices/groups/createGroupXService.ts @@ -1,14 +1,6 @@ import { createGroup } from "api/api" -import { - ApiError, - getErrorMessage, - hasApiFieldErrors, - isApiError, - mapApiErrorToFieldErrors, -} from "api/errors" import { CreateGroupRequest, Group } from "api/typesGenerated" -import { displayError } from "components/GlobalSnackbar/utils" -import { createMachine } from "xstate" +import { createMachine, assign } from "xstate" export const createGroupMachine = createMachine( { @@ -16,7 +8,7 @@ export const createGroupMachine = createMachine( schema: { context: {} as { organizationId: string - createGroupFormErrors?: unknown + error?: unknown }, services: {} as { createGroup: { @@ -45,37 +37,23 @@ export const createGroupMachine = createMachine( target: "idle", actions: ["onCreate"], }, - onError: [ - { - target: "idle", - cond: "hasFieldErrors", - actions: ["assignCreateGroupFormErrors"], - }, - { - target: "idle", - actions: ["displayCreateGroupError"], - }, - ], + onError: { + target: "idle", + actions: ["assignError"], + }, }, }, }, }, { - guards: { - hasFieldErrors: (_, event) => - isApiError(event.data) && hasApiFieldErrors(event.data), - }, services: { createGroup: ({ organizationId }, { data }) => createGroup(organizationId, data), }, actions: { - displayCreateGroupError: (_, { data }) => { - const message = getErrorMessage(data, "Error on creating the group.") - displayError(message) - }, - assignCreateGroupFormErrors: (_, event) => - mapApiErrorToFieldErrors((event.data as ApiError).response.data), + assignError: assign({ + error: (_, event) => event.data, + }), }, }, ) diff --git a/site/src/xServices/groups/editGroupXService.ts b/site/src/xServices/groups/editGroupXService.ts index bdf43eeda32f4..25104dbb62ca1 100644 --- a/site/src/xServices/groups/editGroupXService.ts +++ b/site/src/xServices/groups/editGroupXService.ts @@ -87,7 +87,9 @@ export const editGroupMachine = createMachine( const message = getErrorMessage(data, "Failed to the group.") displayError(message) }, - assignError: (_, event) => event.data, + assignError: assign({ + error: (_, event) => event.data, + }), }, }, ) From 8288308013339292f50b940b5fc9150035b1cfb9 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 4 May 2023 15:10:42 +0000 Subject: [PATCH 8/9] Fix --- site/src/utils/formUtils.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/site/src/utils/formUtils.ts b/site/src/utils/formUtils.ts index 2a01eb14c0b04..828f6ed103be7 100644 --- a/site/src/utils/formUtils.ts +++ b/site/src/utils/formUtils.ts @@ -1,5 +1,5 @@ import { isApiValidationError, mapApiErrorToFieldErrors } from "api/errors" -import { FormikContextType, FormikErrors } from "formik" +import { FormikContextType, FormikErrors, getIn } from "formik" import { ChangeEvent, ChangeEventHandler, @@ -36,7 +36,7 @@ interface FormHelpers { export const getFormHelpers = (form: FormikContextType, error?: unknown) => ( - fieldName: keyof TFormValues, + fieldName: keyof TFormValues | string, helperText?: ReactNode, // backendFieldName is used when the value in the form is named different from the backend backendFieldName?: string, @@ -46,12 +46,13 @@ export const getFormHelpers = error.response.data, ) as FormikErrors & { [key: string]: string }) : undefined - const touched = Boolean(form.touched[fieldName]) + // Since the fieldName can be a path string like parameters[0].value we need to use getIn + const touched = Boolean(getIn(form.touched, fieldName.toString())) + const formError = getIn(form.errors, fieldName.toString()) // Since the field in the form can be diff from the backend, we need to // check for both when getting the error const apiField = backendFieldName ?? fieldName const apiError = apiValidationErrors?.[apiField.toString()] - const formError = form.errors[fieldName]?.toString() const errorToDisplay = apiError ?? formError return { From da6c6a5cb98550b88e81e4a15b3186f136e65ee3 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 4 May 2023 17:18:15 +0000 Subject: [PATCH 9/9] Fix stories --- .../CreateUserForm/CreateUserForm.stories.tsx | 15 ++++--------- .../pages/UsersPage/UsersPageView.stories.tsx | 21 ++++++++----------- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/site/src/components/CreateUserForm/CreateUserForm.stories.tsx b/site/src/components/CreateUserForm/CreateUserForm.stories.tsx index 378ede761e131..323a839ec8b09 100644 --- a/site/src/components/CreateUserForm/CreateUserForm.stories.tsx +++ b/site/src/components/CreateUserForm/CreateUserForm.stories.tsx @@ -1,6 +1,7 @@ import { action } from "@storybook/addon-actions" import { Story } from "@storybook/react" import { CreateUserForm, CreateUserFormProps } from "./CreateUserForm" +import { mockApiError } from "testHelpers/entities" export default { title: "components/CreateUserForm", @@ -18,22 +19,14 @@ Ready.args = { isLoading: false, } -export const UnknownError = Template.bind({}) -UnknownError.args = { - onCancel: action("cancel"), - onSubmit: action("submit"), - isLoading: false, - error: "Something went wrong", -} - export const FormError = Template.bind({}) FormError.args = { onCancel: action("cancel"), onSubmit: action("submit"), isLoading: false, - formErrors: { - username: "Username taken", - }, + error: mockApiError({ + validations: [{ field: "username", detail: "Username taken" }], + }), } export const Loading = Template.bind({}) diff --git a/site/src/pages/UsersPage/UsersPageView.stories.tsx b/site/src/pages/UsersPage/UsersPageView.stories.tsx index f51a4d4f013bc..6e06ae72a6222 100644 --- a/site/src/pages/UsersPage/UsersPageView.stories.tsx +++ b/site/src/pages/UsersPage/UsersPageView.stories.tsx @@ -4,6 +4,7 @@ import { MockUser, MockUser2, MockAssignableSiteRoles, + mockApiError, } from "testHelpers/entities" import { UsersPageView, UsersPageViewProps } from "./UsersPageView" @@ -42,17 +43,13 @@ EmptyPage.args = { users: [], isNonInitialPage: true } export const Error = Template.bind({}) Error.args = { users: undefined, - error: { - response: { - data: { - message: "Invalid user search query.", - validations: [ - { - field: "status", - detail: `Query param "status" has invalid value: "inactive" is not a valid user status`, - }, - ], + error: mockApiError({ + message: "Invalid user search query.", + validations: [ + { + field: "status", + detail: `Query param "status" has invalid value: "inactive" is not a valid user status`, }, - }, - }, + ], + }), }