Skip to content

Commit

Permalink
dest api: Clean up error API (#3698)
Browse files Browse the repository at this point in the history
* standardize error codes

* reorganize

* cleanup errors on other components

* remove special case for dest type err

* rename/simplify split helper

* update form

* update test

* fix type issue
  • Loading branch information
mastercactapus committed Feb 21, 2024
1 parent 3ccba54 commit e6633a4
Show file tree
Hide file tree
Showing 14 changed files with 235 additions and 163 deletions.
3 changes: 2 additions & 1 deletion graphql2/generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 25 additions & 0 deletions graphql2/graph/errorcodes.graphqls
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@


"""
Known error codes that the server can return.
These values will be returned in the `extensions.code` field of the error response.
"""
enum ErrorCode {

"""
The input value is invalid, the `path` field will contain the exact path to the invalid input.
A separate error will be returned for each invalid field.
"""
INVALID_INPUT_VALUE

"""
The `path` field contains the exact path to the DestinationInput that is invalid.
The `extensions.fieldID` field contains the ID of the input field that is invalid.
A separate error will be returned for each invalid field.
"""
INVALID_DEST_FIELD_VALUE
}
8 changes: 5 additions & 3 deletions graphql2/graphqlapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ func isGQLValidation(gqlErr *gqlerror.Error) bool {
return false
}

_, ok := gqlErr.Extensions["code"].(graphql2.ErrorCode)
if ok {
return true
}

code, ok := gqlErr.Extensions["code"].(string)
if !ok {
return false
Expand All @@ -157,9 +162,6 @@ func isGQLValidation(gqlErr *gqlerror.Error) bool {
case errcode.ValidationFailed, errcode.ParseFailed:
// These are gqlgen validation errors.
return true
case ErrCodeInvalidDestType, ErrCodeInvalidDestValue:
// These are destination validation errors.
return true
}

return false
Expand Down
14 changes: 3 additions & 11 deletions graphql2/graphqlapp/destinationvalidation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ import (
"github.com/vektah/gqlparser/v2/gqlerror"
)

const (
ErrCodeInvalidDestType = "INVALID_DESTINATION_TYPE"
ErrCodeInvalidDestValue = "INVALID_DESTINATION_FIELD_VALUE"
)

func errReason(err error) string {
var fErr validation.FieldError
if errors.As(err, &fErr) {
Expand All @@ -48,16 +43,13 @@ func addDestFieldError(ctx context.Context, parentField, fieldID string, err err
for _, part := range parentParts {
p = append(p, ast.PathName(part))
}
p = append(p,
ast.PathName("values"), // DestinationInput.Values
ast.PathName(fieldID),
)

graphql.AddError(ctx, &gqlerror.Error{
Message: errReason(err),
Path: p,
Extensions: map[string]interface{}{
"code": ErrCodeInvalidDestValue,
"code": graphql2.ErrorCodeInvalidDestFieldValue,
"fieldID": fieldID,
},
})

Expand Down Expand Up @@ -186,7 +178,7 @@ func (a *App) ValidateDestination(ctx context.Context, fieldName string, dest *g
Message: "unsupported destination type",
Path: p,
Extensions: map[string]interface{}{
"code": ErrCodeInvalidDestType,
"code": graphql2.ErrorCodeInvalidInputValue,
},
})

Expand Down
52 changes: 52 additions & 0 deletions graphql2/models_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 10 additions & 2 deletions web/src/app/selection/DestinationField.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,20 @@ export const FieldError: Story = {
disabled: false,
destFieldErrors: [
{
path: 'third-field',
path: ['input', 'dest'],
message: 'This is an error message (third)',
extensions: {
code: 'INVALID_DEST_FIELD_VALUE',
fieldID: 'third-field',
},
},
{
path: 'first-field',
path: ['input', 'dest'],
message: 'This is an error message (first)',
extensions: {
code: 'INVALID_DEST_FIELD_VALUE',
fieldID: 'first-field',
},
},
],
},
Expand Down
10 changes: 3 additions & 7 deletions web/src/app/selection/DestinationField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import DestinationInputDirect from './DestinationInputDirect'
import { useDestinationType } from '../util/RequireConfig'
import DestinationSearchSelect from './DestinationSearchSelect'
import { Grid } from '@mui/material'
import { InputFieldError } from '../util/errutil'
import { DestFieldValueError } from '../util/errtypes'

export type DestinationFieldProps = {
value: FieldValueInput[]
Expand All @@ -13,7 +13,7 @@ export type DestinationFieldProps = {

disabled?: boolean

destFieldErrors?: InputFieldError[]
destFieldErrors?: DestFieldValueError[]
}

function capFirstLetter(s: string): string {
Expand Down Expand Up @@ -46,13 +46,9 @@ export default function DestinationField(
props.onChange(newValues)
}

// getFieldID returns the last segment of the path.
const getFieldID = (err: InputFieldError): string =>
err.path.split('.').pop() || ''

const fieldErrMsg = capFirstLetter(
props.destFieldErrors?.find(
(err) => getFieldID(err) === field.fieldID,
(err) => err.extensions.fieldID === field.fieldID,
)?.message || '',
)

Expand Down
15 changes: 11 additions & 4 deletions web/src/app/users/UserContactMethodFormDest.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,14 @@ export const ErrorSingleField: Story = {
statusUpdates: false,
},
disabled: false,
fieldErrors: [
errors: [
{
message: 'number is too short', // note: the 'n' is lowercase
path: 'dest.values.phone-number',
path: ['input', 'dest'],
extensions: {
code: 'INVALID_DEST_FIELD_VALUE',
fieldID: 'phone-number',
},
},
],
},
Expand Down Expand Up @@ -153,10 +157,13 @@ export const ErrorMultiField: Story = {
statusUpdates: false,
},
disabled: false,
fieldErrors: [
errors: [
{
path: 'name',
path: ['input', 'name'],
message: 'must begin with a letter',
extensions: {
code: 'INVALID_INPUT_VALUE',
},
},
],
},
Expand Down
44 changes: 20 additions & 24 deletions web/src/app/users/UserContactMethodFormDest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,14 @@ import React from 'react'
import { DestinationInput } from '../../schema'
import { FormContainer, FormField } from '../forms'
import { renderMenuItem } from '../selection/DisableableMenuItem'
import { InputFieldError, SimpleError } from '../util/errutil'
import DestinationField from '../selection/DestinationField'
import { useContactMethodTypes } from '../util/RequireConfig'
import {
isInputFieldError,
isDestFieldError,
KnownError,
DestFieldValueError,
} from '../util/errtypes'

export type Value = {
name: string
Expand All @@ -18,19 +23,24 @@ export type Value = {
export type UserContactMethodFormProps = {
value: Value

typeError?: SimpleError
fieldErrors?: Array<InputFieldError>
errors?: Array<KnownError | DestFieldValueError>

disabled?: boolean
edit?: boolean

onChange?: (CMValue: Value) => void
}

export const errorPaths = (prefix = '*'): string[] => [
`${prefix}.name`,
`${prefix}.dest.type`,
`${prefix}.dest`,
]

export default function UserContactMethodFormDest(
props: UserContactMethodFormProps,
): JSX.Element {
const { value, edit = false, ...other } = props
const { value, edit = false, errors = [], ...other } = props

const destinationTypes = useContactMethodTypes()
const currentType = destinationTypes.find((d) => d.type === value.dest.type)
Expand All @@ -47,28 +57,14 @@ export default function UserContactMethodFormDest(
statusUpdateChecked = false
}

const formErrors = []
if (props.typeError) {
formErrors.push({
// the old form code requires this shape to map errors to the correct field
message: props.typeError.message,
field: 'dest.type',
})
}
const nameErr = props.fieldErrors?.find(
(err) => err.path.split('.').pop() === 'name',
)
if (nameErr) {
formErrors.push({
message: nameErr.message,
field: 'name',
})
}

return (
<FormContainer
{...other}
errors={formErrors}
errors={errors?.filter(isInputFieldError).map((e) => ({
// need to convert to FormContainer's error format
message: e.message,
field: e.path[e.path.length - 1].toString(),
}))}
value={value}
mapOnChangeValue={(newValue: Value): Value => {
if (newValue.dest.type === value.dest.type) {
Expand Down Expand Up @@ -119,7 +115,7 @@ export default function UserContactMethodFormDest(
destType={value.dest.type}
component={DestinationField}
disabled={edit}
destFieldErrors={props.fieldErrors}
destFieldErrors={errors.filter(isDestFieldError)}
/>
</Grid>
<Grid item xs={12}>
Expand Down
Loading

0 comments on commit e6633a4

Please sign in to comment.