service: fix label error messaging for create and edit (#4259)

* fix(ServiceEditDialog): streamline loading state and default value handling * fix(ServiceEditDialog): enhance error handling and improve label filtering * fix(ServiceCreateDialog): improve error handling and validation for required labels
target · Feb 11, 2025 · d45f234 · d45f234
1 parent f49337f
commit d45f234
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 50 deletions.
diff --git a/web/src/app/services/ServiceCreateDialog.tsx b/web/src/app/services/ServiceCreateDialog.tsx
@@ -1,10 +1,11 @@
 import React, { useState } from 'react'
 import { gql, useMutation, CombinedError } from 'urql'
-import { fieldErrors, nonFieldErrors } from '../util/errutil'
 import FormDialog from '../dialogs/FormDialog'
 import ServiceForm, { Value } from './ServiceForm'
 import { Redirect } from 'wouter'
 import { Label } from '../../schema'
+import { useErrorConsumer } from '../util/ErrorConsumer'
+import { useConfigValue } from '../util/RequireConfig'
 
 interface InputVar {
   name: string
@@ -77,18 +78,24 @@ export default function ServiceCreateDialog(props: {
   const [createKeyStatus, commit] = useMutation(createMutation)
 
   const { data, error } = createKeyStatus
+  const errs = useErrorConsumer(error)
+  const [reqLabels] = useConfigValue('Services.RequiredLabels') as [string[]]
+
   if (data && data.createService) {
+    errs.done()
     return <Redirect to={`/services/${data.createService.id}`} />
   }
-
-  const fieldErrs = fieldErrors(error).filter(
-    (e) => !e.field.startsWith('newEscalationPolicy.'),
-  )
+  const labelErr = { key: '', msg: '' }
+  reqLabels.some((key, i) => {
+    labelErr.key = key
+    labelErr.msg = errs.getErrorByField('labels[' + i + '].Value') || ''
+    return !!labelErr.msg
+  })
 
   return (
     <FormDialog
       title='Create New Service'
-      errors={nonFieldErrors(error)}
+      errors={errs.remainingLegacyCallback()}
       onClose={props.onClose}
       onSubmit={() => {
         let n = 1
@@ -121,7 +128,11 @@ export default function ServiceCreateDialog(props: {
       }}
       form={
         <ServiceForm
-          errors={fieldErrs}
+          nameError={errs.getErrorByField('Name')}
+          descError={errs.getErrorByField('Description')}
+          epError={errs.getErrorByField('EscalationPolicyID')}
+          labelErrorKey={labelErr.key}
+          labelErrorMsg={labelErr.msg}
           value={value}
           onChange={(val: Value) => setValue(val)}
         />

diff --git a/web/src/app/services/ServiceEditDialog.tsx b/web/src/app/services/ServiceEditDialog.tsx
@@ -1,12 +1,11 @@
 import React, { useState } from 'react'
 import { gql, useQuery, useMutation } from 'urql'
 
-import { fieldErrors, nonFieldErrors } from '../util/errutil'
-
 import FormDialog from '../dialogs/FormDialog'
 import ServiceForm from './ServiceForm'
-import Spinner from '../loading/components/Spinner'
 import { Label } from '../../schema'
+import { useErrorConsumer } from '../util/ErrorConsumer'
+import { useConfigValue } from '../util/RequireConfig'
 
 interface Value {
   name: string
@@ -47,38 +46,32 @@ export default function ServiceEditDialog(props: {
   serviceID: string
   onClose: () => void
 }): JSX.Element {
-  const [value, setValue] = useState<Value | null>(null)
-  const [{ data, fetching: dataFetching, error: dataError }] = useQuery({
+  const [{ data, error: dataError }] = useQuery({
     query,
     variables: { id: props.serviceID },
   })
-
-  const [saveStatus, save] = useMutation(mutation)
-  const [saveLabelStatus, saveLabel] = useMutation(setLabel)
-
-  if (dataFetching && !data) {
-    return <Spinner />
-  }
-
+  const [req] = useConfigValue('Services.RequiredLabels') as [string[]]
   const defaultValue = {
     name: data?.service?.name,
     description: data?.service?.description,
     escalationPolicyID: data?.service?.ep?.id,
-    labels: data?.service?.labels || [],
+    labels: (data?.service?.labels || []).filter((l: Label) =>
+      req.includes(l.key),
+    ),
   }
+  const [value, setValue] = useState<Value>(defaultValue)
 
-  const fieldErrs = fieldErrors(saveStatus.error).concat(
-    fieldErrors(saveLabelStatus.error),
-  )
+  const [saveStatus, save] = useMutation(mutation)
+  const [saveLabelStatus, saveLabel] = useMutation(setLabel)
+
+  const errs = useErrorConsumer(saveStatus.error).append(saveLabelStatus.error)
+  console.log()
 
   return (
     <FormDialog
       title='Edit Service'
-      loading={saveStatus.fetching || (!data && dataFetching)}
-      errors={nonFieldErrors(saveStatus.error).concat(
-        nonFieldErrors(dataError),
-        nonFieldErrors(saveLabelStatus.error),
-      )}
+      loading={saveStatus.fetching}
+      errors={errs.remainingLegacyCallback()}
       onClose={props.onClose}
       onSubmit={async () => {
         const saveRes = await save(
@@ -115,11 +108,13 @@ export default function ServiceEditDialog(props: {
       form={
         <ServiceForm
           epRequired
-          errors={fieldErrs}
-          disabled={Boolean(
-            saveStatus.fetching || (!data && dataFetching) || dataError,
-          )}
-          value={value || defaultValue}
+          nameError={errs.getErrorByField('Name')}
+          descError={errs.getErrorByField('Description')}
+          epError={errs.getErrorByField('EscalationPolicyID')}
+          labelErrorKey={saveLabelStatus.operation?.variables?.input?.key}
+          labelErrorMsg={errs.getErrorByField('Value')}
+          disabled={Boolean(saveStatus.fetching || !data || dataError)}
+          value={value}
           onChange={(value) => setValue(value)}
         />
       }

diff --git a/web/src/app/services/ServiceForm.tsx b/web/src/app/services/ServiceForm.tsx
@@ -3,7 +3,6 @@ import Grid from '@mui/material/Grid'
 import TextField from '@mui/material/TextField'
 import { EscalationPolicySelect } from '../selection/EscalationPolicySelect'
 import { FormContainer, FormField } from '../forms'
-import { FieldError } from '../util/errutil'
 import { useConfigValue } from '../util/RequireConfig'
 import { Label } from '../../schema'
 import { InputAdornment } from '@mui/material'
@@ -20,7 +19,12 @@ export interface Value {
 interface ServiceFormProps {
   value: Value
 
-  errors: FieldError[]
+  nameError?: string
+  descError?: string
+  epError?: string
+
+  labelErrorKey?: string
+  labelErrorMsg?: string
 
   onChange: (val: Value) => void
 
@@ -30,18 +34,28 @@ interface ServiceFormProps {
 }
 
 export default function ServiceForm(props: ServiceFormProps): JSX.Element {
-  const { epRequired, errors, ...containerProps } = props
+  const {
+    epRequired,
+    nameError,
+    descError,
+    epError,
+    labelErrorKey,
+    labelErrorMsg,
+    ...containerProps
+  } = props
 
-  const formErrs = errors.map((e) => {
-    if (e.field !== 'value') {
-      // label value
-      return e
-    }
-    return {
-      ...e,
-      field: 'labels',
-    }
-  })
+  const formErrs = [
+    { field: 'name', message: nameError },
+    { field: 'description', message: descError },
+    {
+      field: 'escalationPolicyID',
+      message: epError,
+    },
+    {
+      field: 'label_' + labelErrorKey,
+      message: labelErrorMsg,
+    },
+  ].filter((e) => e.message)
 
   const [reqLabels] = useConfigValue('Services.RequiredLabels') as [string[]]
 
@@ -92,6 +106,7 @@ export default function ServiceForm(props: ServiceFormProps): JSX.Element {
                 required={!epRequired} // optional when editing
                 component={TextField}
                 fieldName='labels'
+                errorName={'label_' + labelName}
                 label={
                   reqLabels.length === 1
                     ? 'Service Label'

diff --git a/web/src/app/util/ErrorConsumer.ts b/web/src/app/util/ErrorConsumer.ts
@@ -13,7 +13,11 @@ type ErrorStore = {
 /** ErrorConsumer is a utility class for consuming and handling errors from a CombinedError. */
 export class ErrorConsumer {
   constructor(e?: CombinedError | null | undefined) {
-    if (!e) return
+    this.append(e)
+  }
+
+  append(e?: CombinedError | null | undefined): ErrorConsumer {
+    if (!e) return this
 
     if (e.networkError) {
       this.store.errors.add({
@@ -74,7 +78,7 @@ export class ErrorConsumer {
       })
     }
 
-    this.hadErrors = this.hasErrors()
+    this._hadErrors = this._hadErrors || this.hasErrors()
 
     // Use FinalizationRegistry if available, this will allow us to raise any errors that are forgotten.
     if (
@@ -89,13 +93,18 @@ export class ErrorConsumer {
 
       r.register(this, { e: this.store }, this)
     }
+
+    return this
   }
 
   private isDone: boolean = false
   private store: ErrorStore = { errors: new Set() }
 
   /** Whether there were any errors in the original error. */
-  public readonly hadErrors: boolean = false
+  private _hadErrors: boolean = false
+  public get hadErrors(): boolean {
+    return this._hadErrors
+  }
 
   private doneCheck(): void {
     if (!this.isDone) return