Skip to content

fix: error messages from workspaceScheduleXService #3255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jul 28, 2022
2 changes: 1 addition & 1 deletion site/src/api/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export const Language = {
},
}

interface FieldError {
export interface FieldError {
field: string
detail: string
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { action } from "@storybook/addon-actions"
import { Story } from "@storybook/react"
import dayjs from "dayjs"
import advancedFormat from "dayjs/plugin/advancedFormat"
import timezone from "dayjs/plugin/timezone"
import utc from "dayjs/plugin/utc"
import { makeMockApiError } from "testHelpers/entities"
import {
defaultWorkspaceSchedule,
WorkspaceScheduleForm,
Expand All @@ -17,6 +17,14 @@ dayjs.extend(timezone)
export default {
title: "components/WorkspaceScheduleForm",
component: WorkspaceScheduleForm,
argTypes: {
onCancel: {
action: "onCancel",
},
onSubmit: {
action: "onSubmit",
},
},
}

const Template: Story<WorkspaceScheduleFormProps> = (args) => <WorkspaceScheduleForm {...args} />
Expand All @@ -27,8 +35,6 @@ WorkspaceWillNotShutDown.args = {
...defaultWorkspaceSchedule(5),
ttl: 0,
},
onCancel: () => action("onCancel"),
onSubmit: () => action("onSubmit"),
}

export const WorkspaceWillShutdownInAnHour = Template.bind({})
Expand All @@ -37,8 +43,6 @@ WorkspaceWillShutdownInAnHour.args = {
...defaultWorkspaceSchedule(5),
ttl: 1,
},
onCancel: () => action("onCancel"),
onSubmit: () => action("onSubmit"),
}

export const WorkspaceWillShutdownInTwoHours = Template.bind({})
Expand All @@ -47,8 +51,6 @@ WorkspaceWillShutdownInTwoHours.args = {
...defaultWorkspaceSchedule(2),
ttl: 2,
},
onCancel: () => action("onCancel"),
onSubmit: () => action("onSubmit"),
}

export const WorkspaceWillShutdownInADay = Template.bind({})
Expand All @@ -57,8 +59,6 @@ WorkspaceWillShutdownInADay.args = {
...defaultWorkspaceSchedule(2),
ttl: 24,
},
onCancel: () => action("onCancel"),
onSubmit: () => action("onSubmit"),
}

export const WorkspaceWillShutdownInTwoDays = Template.bind({})
Expand All @@ -67,6 +67,13 @@ WorkspaceWillShutdownInTwoDays.args = {
...defaultWorkspaceSchedule(2),
ttl: 48,
},
onCancel: () => action("onCancel"),
onSubmit: () => action("onSubmit"),
}

export const WithError = Template.bind({})
WithError.args = {
initialTouched: { ttl: true },
submitScheduleError: makeMockApiError({
message: "Something went wrong.",
validations: [{ field: "ttl_ms", detail: "Invalid time until shutdown." }],
}),
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,17 @@ import FormLabel from "@material-ui/core/FormLabel"
import MenuItem from "@material-ui/core/MenuItem"
import makeStyles from "@material-ui/core/styles/makeStyles"
import TextField from "@material-ui/core/TextField"
import { ErrorSummary } from "components/ErrorSummary/ErrorSummary"
import dayjs from "dayjs"
import advancedFormat from "dayjs/plugin/advancedFormat"
import duration from "dayjs/plugin/duration"
import relativeTime from "dayjs/plugin/relativeTime"
import timezone from "dayjs/plugin/timezone"
import utc from "dayjs/plugin/utc"
import { useFormik } from "formik"
import { FormikTouched, useFormik } from "formik"
import { FC } from "react"
import * as Yup from "yup"
import { FieldErrors } from "../../api/errors"
import { getFormHelpers } from "../../util/formUtils"
import { getFormHelpersWithError } from "../../util/formUtils"
import { FormFooter } from "../FormFooter/FormFooter"
import { FullPageForm } from "../FullPageForm/FullPageForm"
import { Stack } from "../Stack/Stack"
Expand Down Expand Up @@ -54,11 +54,13 @@ export const Language = {
}

export interface WorkspaceScheduleFormProps {
fieldErrors?: FieldErrors
submitScheduleError?: Error | unknown
initialValues?: WorkspaceScheduleFormValues
isLoading: boolean
onCancel: () => void
onSubmit: (values: WorkspaceScheduleFormValues) => void
// for storybook
initialTouched?: FormikTouched<WorkspaceScheduleFormValues>
}

export interface WorkspaceScheduleFormValues {
Expand Down Expand Up @@ -178,20 +180,25 @@ export const defaultWorkspaceSchedule = (
})

export const WorkspaceScheduleForm: FC<WorkspaceScheduleFormProps> = ({
fieldErrors,
submitScheduleError,
initialValues = defaultWorkspaceSchedule(),
isLoading,
onCancel,
onSubmit,
initialTouched,
}) => {
const styles = useStyles()

const form = useFormik<WorkspaceScheduleFormValues>({
initialValues,
onSubmit,
validationSchema,
initialTouched,
})
const formHelpers = getFormHelpers<WorkspaceScheduleFormValues>(form, fieldErrors)
const formHelpers = getFormHelpersWithError<WorkspaceScheduleFormValues>(
form,
submitScheduleError,
)

const checkboxes: Array<{ value: boolean; name: string; label: string }> = [
{ value: form.values.sunday, name: "sunday", label: Language.daySundayLabel },
Expand All @@ -207,6 +214,7 @@ export const WorkspaceScheduleForm: FC<WorkspaceScheduleFormProps> = ({
<FullPageForm onCancel={onCancel} title="Workspace schedule">
<form onSubmit={form.handleSubmit} className={styles.form}>
<Stack>
{submitScheduleError && <ErrorSummary error={submitScheduleError} />}
<TextField
{...formHelpers("startTime", Language.startTimeHelperText)}
disabled={isLoading}
Expand Down Expand Up @@ -262,7 +270,7 @@ export const WorkspaceScheduleForm: FC<WorkspaceScheduleFormProps> = ({
</FormControl>

<TextField
{...formHelpers("ttl", ttlShutdownAt(form.values.ttl))}
{...formHelpers("ttl", ttlShutdownAt(form.values.ttl), "ttl_ms")}
disabled={isLoading}
inputProps={{ min: 0, step: 1 }}
label={Language.ttlLabel}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ dayjs.extend(timezone)

const Language = {
forbiddenError: "You don't have permissions to update the schedule for this workspace.",
getWorkspaceError: "Failed to fetch workspace.",
checkPermissionsError: "Failed to fetch permissions.",
}

export const formValuesToAutoStartRequest = (
Expand Down Expand Up @@ -156,7 +158,7 @@ export const WorkspaceSchedulePage: React.FC = () => {
userId: me?.id,
},
})
const { checkPermissionsError, formErrors, getWorkspaceError, permissions, workspace } =
const { checkPermissionsError, submitScheduleError, getWorkspaceError, permissions, workspace } =
scheduleState.context

// Get workspace on mount and whenever the args for getting a workspace change.
Expand All @@ -183,6 +185,9 @@ export const WorkspaceSchedulePage: React.FC = () => {
return (
<ErrorSummary
error={getWorkspaceError || checkPermissionsError}
defaultMessage={
getWorkspaceError ? Language.getWorkspaceError : Language.checkPermissionsError
}
retry={() => scheduleSend({ type: "GET_WORKSPACE", username, workspaceName })}
/>
)
Expand All @@ -195,7 +200,7 @@ export const WorkspaceSchedulePage: React.FC = () => {
if (scheduleState.matches("presentForm") || scheduleState.matches("submittingSchedule")) {
return (
<WorkspaceScheduleForm
fieldErrors={formErrors}
submitScheduleError={submitScheduleError}
initialValues={workspaceToInitialValues(workspace, dayjs.tz.guess())}
isLoading={scheduleState.tags.has("loading")}
onCancel={() => {
Expand Down
21 changes: 21 additions & 0 deletions site/src/testHelpers/entities.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FieldError } from "api/errors"
import * as Types from "../api/types"
import * as TypesGen from "../api/typesGenerated"

Expand Down Expand Up @@ -584,3 +585,23 @@ export const MockWorkspaceBuildLogs: TypesGen.ProvisionerJobLog[] = [
export const MockCancellationMessage = {
message: "Job successfully canceled",
}

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
export const makeMockApiError = ({
message,
detail,
validations,
}: {
message?: string
detail?: string
validations?: FieldError[]
}) => ({
response: {
data: {
message: message ?? "Something went wrong.",
detail: detail ?? undefined,
validations: validations ?? undefined,
},
},
isAxiosError: true,
})
14 changes: 8 additions & 6 deletions site/src/util/formUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,21 @@ interface FormHelpers {
helperText?: ReactNode
}

// backendErrorName can be used if the backend names a field differently than the frontend does
export const getFormHelpers =
<T>(form: FormikContextType<T>, formErrors?: FormikErrors<T>) =>
(name: keyof T, HelperText: ReactNode = ""): FormHelpers => {
<T>(form: FormikContextType<T>, apiValidationErrors?: FormikErrors<T>) =>
(name: keyof T, HelperText: ReactNode = "", backendErrorName?: string): FormHelpers => {
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(formErrors, name)
const validationError = getIn(form.errors, name)
const error = apiError ?? validationError
const apiError = getIn(apiValidationErrors, apiErrorName)
const frontendError = getIn(form.errors, name)
const error = apiError ?? frontendError
return {
...form.getFieldProps(name),
id: name,
Expand All @@ -49,7 +51,7 @@ export const getFormHelpers =
export const getFormHelpersWithError = <T>(
form: FormikContextType<T>,
error?: Error | unknown,
): ((name: keyof T, HelperText?: ReactNode) => FormHelpers) => {
): ((name: keyof T, HelperText?: ReactNode, errorName?: string) => FormHelpers) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Instead of ‘errorName’, we can call the argument field ‘fieldName’ or ‘backendFieldName’.

const apiValidationErrors =
isApiError(error) && hasApiFieldErrors(error)
? (mapApiErrorToFieldErrors(error.response.data) as FormikErrors<T>)
Expand Down
21 changes: 6 additions & 15 deletions site/src/xServices/workspaceSchedule/workspaceScheduleXService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,16 @@
*/
import { assign, createMachine } from "xstate"
import * as API from "../../api/api"
import { ApiError, FieldErrors, mapApiErrorToFieldErrors } from "../../api/errors"
import * as TypesGen from "../../api/typesGenerated"
import { displayError, displaySuccess } from "../../components/GlobalSnackbar/utils"
import { displaySuccess } from "../../components/GlobalSnackbar/utils"

export const Language = {
errorSubmissionFailed: "Failed to update schedule",
errorWorkspaceFetch: "Failed to fetch workspace",
successMessage: "Successfully updated workspace schedule.",
}

type Permissions = Record<keyof ReturnType<typeof permissionsToCheck>, boolean>

export interface WorkspaceScheduleContext {
formErrors?: FieldErrors
getWorkspaceError?: Error | unknown
/**
* Each workspace has their own schedule (start and ttl). For this reason, we
Expand All @@ -29,6 +25,7 @@ export interface WorkspaceScheduleContext {
userId?: string
permissions?: Permissions
checkPermissionsError?: Error | unknown
submitScheduleError?: Error | unknown
}

export const checks = {
Expand Down Expand Up @@ -86,7 +83,7 @@ export const workspaceSchedule = createMachine(
},
onError: {
target: "error",
actions: ["assignGetWorkspaceError", "displayWorkspaceError"],
actions: ["assignGetWorkspaceError"],
},
},
tags: "loading",
Expand Down Expand Up @@ -125,7 +122,7 @@ export const workspaceSchedule = createMachine(
},
onError: {
target: "presentForm",
actions: ["assignSubmissionError", "displaySubmissionError"],
actions: ["assignSubmissionError"],
},
},
tags: "loading",
Expand All @@ -145,7 +142,7 @@ export const workspaceSchedule = createMachine(
{
actions: {
assignSubmissionError: assign({
formErrors: (_, event) => mapApiErrorToFieldErrors((event.data as ApiError).response.data),
submitScheduleError: (_, event) => event.data,
}),
assignWorkspace: assign({
workspace: (_, event) => event.data,
Expand All @@ -170,12 +167,6 @@ export const workspaceSchedule = createMachine(
clearGetWorkspaceError: (context) => {
assign({ ...context, getWorkspaceError: undefined })
},
displayWorkspaceError: () => {
displayError(Language.errorWorkspaceFetch)
},
displaySubmissionError: () => {
displayError(Language.errorSubmissionFailed)
},
displaySuccess: () => {
displaySuccess(Language.successMessage)
},
Expand All @@ -197,7 +188,7 @@ export const workspaceSchedule = createMachine(
submitSchedule: async (context, event) => {
if (!context.workspace?.id) {
// This state is theoretically impossible, but helps TS
throw new Error("failed to load workspace")
throw new Error("Failed to load workspace.")
}

// REMARK: These calls are purposefully synchronous because if one
Expand Down