diff --git a/site/src/components/ErrorSummary/ErrorSummary.tsx b/site/src/components/ErrorSummary/ErrorSummary.tsx index 77bea7b056363..ceca96f7d72df 100644 --- a/site/src/components/ErrorSummary/ErrorSummary.tsx +++ b/site/src/components/ErrorSummary/ErrorSummary.tsx @@ -9,7 +9,7 @@ import { ApiError, getErrorDetail, getErrorMessage } from "api/errors" import { Stack } from "components/Stack/Stack" import { FC, useState } from "react" -const Language = { +export const Language = { retryMessage: "Retry", unknownErrorMessage: "An unknown error has occurred", moreDetails: "More", @@ -91,7 +91,6 @@ interface StyleProps { const useStyles = makeStyles((theme) => ({ root: { background: darken(theme.palette.error.main, 0.6), - margin: `${theme.spacing(2)}px`, padding: `${theme.spacing(2)}px`, borderRadius: theme.shape.borderRadius, gap: 0, diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.stories.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.stories.tsx new file mode 100644 index 0000000000000..2698f3b67fcb6 --- /dev/null +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.stories.tsx @@ -0,0 +1,53 @@ +import { Story } from "@storybook/react" +import { AccountForm, AccountFormProps } from "./SettingsAccountForm" + +export default { + title: "components/SettingsAccountForm", + component: AccountForm, + argTypes: { + onSubmit: { action: "Submit" }, + }, +} + +const Template: Story = (args: AccountFormProps) => + +export const Example = Template.bind({}) +Example.args = { + email: "test-user@org.com", + isLoading: false, + initialValues: { + username: "test-user", + }, + updateProfileError: undefined, + onSubmit: () => { + return Promise.resolve() + }, +} + +export const Loading = Template.bind({}) +Loading.args = { + ...Example.args, + isLoading: true, +} + +export const WithError = Template.bind({}) +WithError.args = { + ...Example.args, + updateProfileError: { + response: { + data: { + message: "Username is invalid", + validations: [ + { + field: "username", + detail: "Username is too long.", + }, + ], + }, + }, + isAxiosError: true, + }, + initialTouched: { + username: true, + }, +} diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx index 6ac89d53a03ce..2fb4c90f73754 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx @@ -1,9 +1,9 @@ -import FormHelperText from "@material-ui/core/FormHelperText" import TextField from "@material-ui/core/TextField" -import { FormikContextType, FormikErrors, useFormik } from "formik" +import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" +import { FormikContextType, FormikTouched, useFormik } from "formik" import { FC } from "react" import * as Yup from "yup" -import { getFormHelpers, nameValidator, onChangeTrimmed } from "../../util/formUtils" +import { getFormHelpersWithError, nameValidator, onChangeTrimmed } from "../../util/formUtils" import { LoadingButton } from "../LoadingButton/LoadingButton" import { Stack } from "../Stack/Stack" @@ -21,15 +21,14 @@ const validationSchema = Yup.object({ username: nameValidator(Language.usernameLabel), }) -export type AccountFormErrors = FormikErrors - export interface AccountFormProps { email: string isLoading: boolean initialValues: AccountFormValues onSubmit: (values: AccountFormValues) => void - formErrors?: AccountFormErrors - error?: string + updateProfileError?: Error | unknown + // initialTouched is only used for testing the error state of the form. + initialTouched?: FormikTouched } export const AccountForm: FC = ({ @@ -37,20 +36,22 @@ export const AccountForm: FC = ({ isLoading, onSubmit, initialValues, - formErrors = {}, - error, + updateProfileError, + initialTouched, }) => { const form: FormikContextType = useFormik({ initialValues, validationSchema, onSubmit, + initialTouched, }) - const getFieldHelpers = getFormHelpers(form, formErrors) + const getFieldHelpers = getFormHelpersWithError(form, updateProfileError) return ( <>
+ {updateProfileError && } = ({ variant="outlined" /> - {error && {error}} -
{isLoading ? "" : Language.updateSettings} diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx new file mode 100644 index 0000000000000..841011405c717 --- /dev/null +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx @@ -0,0 +1,54 @@ +import { Story } from "@storybook/react" +import { SecurityForm, SecurityFormProps } from "./SettingsSecurityForm" + +export default { + title: "components/SettingsSecurityForm", + component: SecurityForm, + argTypes: { + onSubmit: { action: "Submit" }, + }, +} + +const Template: Story = (args: SecurityFormProps) => + +export const Example = Template.bind({}) +Example.args = { + isLoading: false, + initialValues: { + old_password: "", + password: "", + confirm_password: "", + }, + updateSecurityError: undefined, + onSubmit: () => { + return Promise.resolve() + }, +} + +export const Loading = Template.bind({}) +Loading.args = { + ...Example.args, + isLoading: true, +} + +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.", + }, + ], + }, + }, + isAxiosError: true, + }, + initialTouched: { + old_password: true, + }, +} diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx index d10726227f427..a92102012e153 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx @@ -1,9 +1,9 @@ -import FormHelperText from "@material-ui/core/FormHelperText" import TextField from "@material-ui/core/TextField" -import { FormikContextType, FormikErrors, useFormik } from "formik" +import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" +import { FormikContextType, FormikTouched, useFormik } from "formik" import React from "react" import * as Yup from "yup" -import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils" +import { getFormHelpersWithError, onChangeTrimmed } from "../../util/formUtils" import { LoadingButton } from "../LoadingButton/LoadingButton" import { Stack } from "../Stack/Stack" @@ -40,33 +40,35 @@ const validationSchema = Yup.object({ }), }) -export type SecurityFormErrors = FormikErrors export interface SecurityFormProps { isLoading: boolean initialValues: SecurityFormValues onSubmit: (values: SecurityFormValues) => void - formErrors?: SecurityFormErrors - error?: string + updateSecurityError?: Error | unknown + // initialTouched is only used for testing the error state of the form. + initialTouched?: FormikTouched } export const SecurityForm: React.FC = ({ isLoading, onSubmit, initialValues, - formErrors = {}, - error, + updateSecurityError, + initialTouched, }) => { const form: FormikContextType = useFormik({ initialValues, validationSchema, onSubmit, + initialTouched, }) - const getFieldHelpers = getFormHelpers(form, formErrors) + const getFieldHelpers = getFormHelpersWithError(form, updateSecurityError) return ( <> + {updateSecurityError && } = ({ type="password" /> - {error && {error}} -
{isLoading ? "" : Language.updatePassword} diff --git a/site/src/components/SignInForm/SignInForm.stories.tsx b/site/src/components/SignInForm/SignInForm.stories.tsx index 13a834fee7555..7e6b24f79ed2a 100644 --- a/site/src/components/SignInForm/SignInForm.stories.tsx +++ b/site/src/components/SignInForm/SignInForm.stories.tsx @@ -6,7 +6,6 @@ export default { component: SignInForm, argTypes: { isLoading: "boolean", - authErrorMessage: "string", onSubmit: { action: "Submit" }, }, } @@ -16,7 +15,7 @@ const Template: Story = (args: SignInFormProps) => { return Promise.resolve() }, @@ -33,12 +32,31 @@ Loading.args = { } export const WithLoginError = Template.bind({}) -WithLoginError.args = { ...SignedOut.args, authErrorMessage: "Email or password was invalid" } +WithLoginError.args = { + ...SignedOut.args, + authError: { + response: { + data: { + message: "Email or password was invalid", + validations: [ + { + field: "password", + detail: "Password is invalid.", + }, + ], + }, + }, + isAxiosError: true, + }, + initialTouched: { + password: true, + }, +} export const WithAuthMethodsError = Template.bind({}) WithAuthMethodsError.args = { ...SignedOut.args, - methodsErrorMessage: "Failed to fetch auth methods", + methodsError: new Error("Failed to fetch auth methods"), } export const WithGithub = Template.bind({}) diff --git a/site/src/components/SignInForm/SignInForm.tsx b/site/src/components/SignInForm/SignInForm.tsx index a3184572fc0d7..bfa28ca8bd3cb 100644 --- a/site/src/components/SignInForm/SignInForm.tsx +++ b/site/src/components/SignInForm/SignInForm.tsx @@ -1,14 +1,15 @@ import Button from "@material-ui/core/Button" -import FormHelperText from "@material-ui/core/FormHelperText" import Link from "@material-ui/core/Link" import { makeStyles } from "@material-ui/core/styles" import TextField from "@material-ui/core/TextField" import GitHubIcon from "@material-ui/icons/GitHub" -import { FormikContextType, useFormik } from "formik" +import { ErrorSummary } from "components/ErrorSummary/ErrorSummary" +import { Stack } from "components/Stack/Stack" +import { FormikContextType, FormikTouched, useFormik } from "formik" import { FC } from "react" import * as Yup from "yup" import { AuthMethods } from "../../api/typesGenerated" -import { getFormHelpers, onChangeTrimmed } from "../../util/formUtils" +import { getFormHelpersWithError, onChangeTrimmed } from "../../util/formUtils" import { Welcome } from "../Welcome/Welcome" import { LoadingButton } from "./../LoadingButton/LoadingButton" @@ -39,17 +40,6 @@ const validationSchema = Yup.object({ }) const useStyles = makeStyles((theme) => ({ - loginBtnWrapper: { - marginTop: theme.spacing(6), - borderTop: `1px solid ${theme.palette.action.disabled}`, - paddingTop: theme.spacing(3), - }, - loginTextField: { - marginTop: theme.spacing(2), - }, - submitBtn: { - marginTop: theme.spacing(2), - }, buttonIcon: { width: 14, height: 14, @@ -78,19 +68,22 @@ const useStyles = makeStyles((theme) => ({ export interface SignInFormProps { isLoading: boolean redirectTo: string - authErrorMessage?: string - methodsErrorMessage?: string + authError?: Error | unknown + methodsError?: Error | unknown authMethods?: AuthMethods onSubmit: ({ email, password }: { email: string; password: string }) => Promise + // initialTouched is only used for testing the error state of the form. + initialTouched?: FormikTouched } export const SignInForm: FC = ({ authMethods, redirectTo, isLoading, - authErrorMessage, - methodsErrorMessage, + authError, + methodsError, onSubmit, + initialTouched, }) => { const styles = useStyles() @@ -106,43 +99,46 @@ export const SignInForm: FC = ({ // field), or after a submission attempt. validateOnBlur: false, onSubmit, + initialTouched, }) - const getFieldHelpers = getFormHelpers(form) + const getFieldHelpers = getFormHelpersWithError(form, authError) return ( <> - - - {authErrorMessage && {authErrorMessage}} - {methodsErrorMessage && ( - {Language.methodsErrorMessage} - )} -
- - {isLoading ? "" : Language.passwordSignIn} - -
+ + {authError && ( + + )} + {methodsError && ( + + )} + + +
+ + {isLoading ? "" : Language.passwordSignIn} + +
+
{authMethods?.github && ( <> diff --git a/site/src/pages/LoginPage/LoginPage.test.tsx b/site/src/pages/LoginPage/LoginPage.test.tsx index cd6242e9dc994..e1090454542fc 100644 --- a/site/src/pages/LoginPage/LoginPage.test.tsx +++ b/site/src/pages/LoginPage/LoginPage.test.tsx @@ -52,10 +52,11 @@ describe("LoginPage", () => { it("shows an error if fetching auth methods fails", async () => { // Given + const apiErrorMessage = "Unable to fetch methods" server.use( // Make login fail rest.get("/api/v2/users/authmethods", async (req, res, ctx) => { - return res(ctx.status(500), ctx.json({ message: "nope" })) + return res(ctx.status(500), ctx.json({ message: apiErrorMessage })) }), ) @@ -63,7 +64,7 @@ describe("LoginPage", () => { render() // Then - const errorMessage = await screen.findByText(Language.methodsErrorMessage) + const errorMessage = await screen.findByText(apiErrorMessage) expect(errorMessage).toBeDefined() }) diff --git a/site/src/pages/LoginPage/LoginPage.tsx b/site/src/pages/LoginPage/LoginPage.tsx index 02acc7311d9ab..9f6e7a0a93247 100644 --- a/site/src/pages/LoginPage/LoginPage.tsx +++ b/site/src/pages/LoginPage/LoginPage.tsx @@ -3,7 +3,6 @@ import { useActor } from "@xstate/react" import React, { useContext } from "react" import { Helmet } from "react-helmet" import { Navigate, useLocation } from "react-router-dom" -import { isApiError } from "../../api/errors" import { Footer } from "../../components/Footer/Footer" import { SignInForm } from "../../components/SignInForm/SignInForm" import { pageTitle } from "../../util/page" @@ -36,12 +35,6 @@ export const LoginPage: React.FC = () => { const [authState, authSend] = useActor(xServices.authXService) const isLoading = authState.hasTag("loading") const redirectTo = retrieveRedirect(location.search) - const authErrorMessage = isApiError(authState.context.authError) - ? authState.context.authError.response.data.message - : undefined - const getMethodsError = authState.context.getMethodsError - ? (authState.context.getMethodsError as Error).message - : undefined const onSubmit = async ({ email, password }: { email: string; password: string }) => { authSend({ type: "SIGN_IN", email, password }) @@ -61,8 +54,8 @@ export const LoginPage: React.FC = () => { authMethods={authState.context.methods} redirectTo={redirectTo} isLoading={isLoading} - authErrorMessage={authErrorMessage} - methodsErrorMessage={getMethodsError} + authError={authState.context.authError} + methodsError={authState.context.getMethodsError as Error} onSubmit={onSubmit} />
diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx index 0d203dc005f5c..af67869d4237f 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx @@ -1,10 +1,11 @@ import { fireEvent, screen, waitFor } from "@testing-library/react" +import { Language as ErrorSummaryLanguage } from "components/ErrorSummary/ErrorSummary" import * as API from "../../../api/api" import { GlobalSnackbar } from "../../../components/GlobalSnackbar/GlobalSnackbar" import * as AccountForm from "../../../components/SettingsAccountForm/SettingsAccountForm" import { renderWithAuth } from "../../../testHelpers/renderHelpers" import * as AuthXService from "../../../xServices/auth/authXService" -import { AccountPage, Language } from "./AccountPage" +import { AccountPage } from "./AccountPage" const renderPage = () => { return renderWithAuth( @@ -80,7 +81,7 @@ describe("AccountPage", () => { const { user } = renderPage() await fillAndSubmitForm() - const errorMessage = await screen.findByText(Language.unknownError) + const errorMessage = await screen.findByText(ErrorSummaryLanguage.unknownErrorMessage) expect(errorMessage).toBeDefined() expect(API.updateProfile).toBeCalledTimes(1) expect(API.updateProfile).toBeCalledWith(user.id, newData) diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx index 97e2defecb9ac..acc5ef3ddd549 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx @@ -1,25 +1,17 @@ import { useActor } from "@xstate/react" import React, { useContext } from "react" -import { isApiError, mapApiErrorToFieldErrors } from "../../../api/errors" import { Section } from "../../../components/Section/Section" import { AccountForm } from "../../../components/SettingsAccountForm/SettingsAccountForm" import { XServiceContext } from "../../../xServices/StateContext" export const Language = { title: "Account", - unknownError: "Oops, an unknown error occurred.", } export const AccountPage: React.FC = () => { const xServices = useContext(XServiceContext) const [authState, authSend] = useActor(xServices.authXService) const { me, updateProfileError } = authState.context - const hasError = !!updateProfileError - const formErrors = - hasError && isApiError(updateProfileError) - ? mapApiErrorToFieldErrors(updateProfileError.response.data) - : undefined - const hasUnknownError = hasError && !isApiError(updateProfileError) if (!me) { throw new Error("No current user found") @@ -29,8 +21,7 @@ export const AccountPage: React.FC = () => {
{ diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx index ffc1800d205f7..2ca46a3b6c8e2 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx @@ -1,11 +1,12 @@ import { fireEvent, screen, waitFor } from "@testing-library/react" +import { Language as ErrorSummaryLanguage } from "components/ErrorSummary/ErrorSummary" import React from "react" import * as API from "../../../api/api" import { GlobalSnackbar } from "../../../components/GlobalSnackbar/GlobalSnackbar" import * as SecurityForm from "../../../components/SettingsSecurityForm/SettingsSecurityForm" import { renderWithAuth } from "../../../testHelpers/renderHelpers" import * as AuthXService from "../../../xServices/auth/authXService" -import { Language, SecurityPage } from "./SecurityPage" +import { SecurityPage } from "./SecurityPage" const renderPage = () => { return renderWithAuth( @@ -65,8 +66,9 @@ describe("SecurityPage", () => { const { user } = renderPage() await fillAndSubmitForm() - const errorMessage = await screen.findByText("Incorrect password.") + const errorMessage = await screen.findAllByText("Incorrect password.") expect(errorMessage).toBeDefined() + expect(errorMessage).toHaveLength(2) expect(API.updateUserPassword).toBeCalledTimes(1) expect(API.updateUserPassword).toBeCalledWith(user.id, newData) }) @@ -87,8 +89,9 @@ describe("SecurityPage", () => { const { user } = renderPage() await fillAndSubmitForm() - const errorMessage = await screen.findByText("Invalid password.") + const errorMessage = await screen.findAllByText("Invalid password.") expect(errorMessage).toBeDefined() + expect(errorMessage).toHaveLength(2) expect(API.updateUserPassword).toBeCalledTimes(1) expect(API.updateUserPassword).toBeCalledWith(user.id, newData) }) @@ -103,7 +106,7 @@ describe("SecurityPage", () => { const { user } = renderPage() await fillAndSubmitForm() - const errorMessage = await screen.findByText(Language.unknownError) + const errorMessage = await screen.findByText(ErrorSummaryLanguage.unknownErrorMessage) expect(errorMessage).toBeDefined() expect(API.updateUserPassword).toBeCalledTimes(1) expect(API.updateUserPassword).toBeCalledWith(user.id, newData) diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx index 49157467b51c6..ac9e2ddb1937b 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx @@ -1,25 +1,17 @@ import { useActor } from "@xstate/react" import React, { useContext } from "react" -import { isApiError, mapApiErrorToFieldErrors } from "../../../api/errors" import { Section } from "../../../components/Section/Section" import { SecurityForm } from "../../../components/SettingsSecurityForm/SettingsSecurityForm" import { XServiceContext } from "../../../xServices/StateContext" export const Language = { title: "Security", - unknownError: "Oops, an unknown error occurred.", } export const SecurityPage: React.FC = () => { const xServices = useContext(XServiceContext) const [authState, authSend] = useActor(xServices.authXService) const { me, updateSecurityError } = authState.context - const hasError = !!updateSecurityError - const formErrors = - hasError && isApiError(updateSecurityError) - ? mapApiErrorToFieldErrors(updateSecurityError.response.data) - : undefined - const hasUnknownError = hasError && !isApiError(updateSecurityError) if (!me) { throw new Error("No current user found") @@ -28,8 +20,7 @@ export const SecurityPage: React.FC = () => { return (
{ diff --git a/site/src/util/formUtils.ts b/site/src/util/formUtils.ts index 12d4939693d4d..3dc137bf4d969 100644 --- a/site/src/util/formUtils.ts +++ b/site/src/util/formUtils.ts @@ -1,3 +1,4 @@ +import { hasApiFieldErrors, isApiError, mapApiErrorToFieldErrors } from "api/errors" import { FormikContextType, FormikErrors, getIn } from "formik" import { ChangeEvent, ChangeEventHandler, FocusEventHandler, ReactNode } from "react" import * as Yup from "yup" @@ -45,6 +46,17 @@ export const getFormHelpers = } } +export const getFormHelpersWithError = ( + form: FormikContextType, + error?: Error | unknown, +): ((name: keyof T, HelperText?: ReactNode) => FormHelpers) => { + const apiValidationErrors = + isApiError(error) && hasApiFieldErrors(error) + ? (mapApiErrorToFieldErrors(error.response.data) as FormikErrors) + : undefined + return getFormHelpers(form, apiValidationErrors) +} + export const onChangeTrimmed = (form: FormikContextType) => (event: ChangeEvent): void => { diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 66d05dbfc8885..11d2c9c68d7b3 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -1,4 +1,3 @@ -import { AxiosError } from "axios" import { assign, createMachine } from "xstate" import * as API from "../../api/api" import * as TypesGen from "../../api/typesGenerated" @@ -49,8 +48,10 @@ type Permissions = Record export interface AuthContext { getUserError?: Error | unknown + // The getMethods API call does not return an ApiError. + // It can only error out in a generic fashion. getMethodsError?: Error | unknown - authError?: Error | AxiosError | unknown + authError?: Error | unknown updateProfileError?: Error | unknown updateSecurityError?: Error | unknown me?: TypesGen.User @@ -194,12 +195,12 @@ export const authMachine = }, }, signingIn: { + entry: "clearAuthError", invoke: { src: "signIn", id: "signIn", onDone: [ { - actions: "clearAuthError", target: "gettingUser", }, ],