From afad866d1dd11ae24b14a49228036affd174a513 Mon Sep 17 00:00:00 2001 From: Oliver Date: Sun, 21 Jul 2024 14:27:18 +1000 Subject: [PATCH] [PUI] form error fix (#7689) * Make initial data query wait until options query is complete * Fix form error issues - Form fields were being re-constructed * Update playwright tests - check for form error message * Prevent reconstruction of form fields * Hide form elements until OPTIONS request is complete * Fix for - "value" must be stringified! * Handle undefined choice values * Add "batch code" to stock detail page * Fix for initial focus * Allow form field definition to change externally * Force override of fetched data * Update playwright tests * Add backup value * Cleanup initialdataquery * Unit test updates * Test updates * Tweak API Form * Adjust playwright test --- src/frontend/package.json | 2 +- src/frontend/src/components/forms/ApiForm.tsx | 168 +++++++++--------- .../components/forms/fields/ChoiceField.tsx | 17 +- .../src/components/forms/fields/TextField.tsx | 3 + src/frontend/src/forms/PartForms.tsx | 1 + src/frontend/src/pages/stock/StockDetail.tsx | 6 + .../src/tables/part/PartCategoryTable.tsx | 1 + .../src/tables/stock/StockLocationTable.tsx | 1 + src/frontend/tests/baseFixtures.ts | 2 + src/frontend/tests/pages/pui_part.spec.ts | 2 +- src/frontend/tests/pages/pui_scan.spec.ts | 1 - src/frontend/tests/pui_general.spec.ts | 12 ++ src/frontend/tests/pui_settings.spec.ts | 39 +++- src/frontend/tests/pui_tables.spec.ts | 2 - src/frontend/yarn.lock | 8 +- 15 files changed, 167 insertions(+), 98 deletions(-) diff --git a/src/frontend/package.json b/src/frontend/package.json index 6abad83d32..54dad53cc2 100644 --- a/src/frontend/package.json +++ b/src/frontend/package.json @@ -52,7 +52,7 @@ "dayjs": "^1.11.10", "embla-carousel-react": "^8.1.6", "html5-qrcode": "^2.3.8", - "mantine-datatable": "^7.11.1", + "mantine-datatable": "^7.11.2", "react": "^18.3.1", "react-dom": "^18.3.1", "react-grid-layout": "^1.4.4", diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx index d7585d69f8..797862c0f3 100644 --- a/src/frontend/src/components/forms/ApiForm.tsx +++ b/src/frontend/src/components/forms/ApiForm.tsx @@ -249,6 +249,31 @@ export function ApiForm({ [props.url, props.pk, props.pathParams] ); + // Define function to process API response + const processFields = (fields: ApiFormFieldSet, data: NestedDict) => { + const res: NestedDict = {}; + + for (const [k, field] of Object.entries(fields)) { + const dataValue = data[k]; + + if ( + field.field_type === 'nested object' && + field.children && + typeof dataValue === 'object' + ) { + res[k] = processFields(field.children, dataValue); + } else { + res[k] = dataValue; + + if (field.onValueChange) { + field.onValueChange(dataValue, data); + } + } + } + + return res; + }; + // Query manager for retrieving initial data from the server const initialDataQuery = useQuery({ enabled: false, @@ -261,79 +286,51 @@ export function ApiForm({ props.pathParams ], queryFn: async () => { - try { - // Await API call - let response = await api.get(url); + return await api + .get(url) + .then((response: any) => { + // Process API response + const fetchedData: any = processFields(fields, response.data); - // Define function to process API response - const processFields = (fields: ApiFormFieldSet, data: NestedDict) => { - const res: NestedDict = {}; - - // TODO: replace with .map() - for (const [k, field] of Object.entries(fields)) { - const dataValue = data[k]; - - if ( - field.field_type === 'nested object' && - field.children && - typeof dataValue === 'object' - ) { - res[k] = processFields(field.children, dataValue); - } else { - res[k] = dataValue; - - if (field.onValueChange) { - field.onValueChange(dataValue, data); - } - } - } - - return res; - }; - - // Process API response - const initialData: any = processFields(fields, response.data); - - // Update form values, but only for the fields specified for this form - form.reset(initialData); - - // Update the field references, too - Object.keys(fields).forEach((fieldName) => { - if (fieldName in initialData) { - let field = fields[fieldName] ?? {}; - fields[fieldName] = { - ...field, - value: initialData[fieldName] - }; - } + // Update form values, but only for the fields specified for this form + form.reset(fetchedData); + return fetchedData; + }) + .catch(() => { + return {}; }); - - return response; - } catch (error) { - console.error('ERR: Error fetching initial data:', error); - // Re-throw error to allow react-query to handle error - return {}; - } } }); useEffect(() => { - let _fields = props.fields ?? {}; + let _fields: any = props.fields || {}; + let _initialData: any = props.initialData || {}; + let _fetchedData: any = initialDataQuery.data || {}; - // Ensure default values override initial field spec for (const k of Object.keys(_fields)) { + // Ensure default values override initial field spec if (defaultValues[k]) { _fields[k].value = defaultValues[k]; } + + // Ensure initial data overrides default values + if (_initialData && _initialData[k]) { + _fields[k].value = _initialData[k]; + } + + // Ensure fetched data overrides also + if (_fetchedData && _fetchedData[k]) { + _fields[k].value = _fetchedData[k]; + } } setFields(_fields); - }, [props.fields, defaultValues, initialDataQuery.data]); + }, [props.fields, props.initialData, defaultValues, initialDataQuery.data]); // Fetch initial data on form load useEffect(() => { // Fetch initial data if the fetchInitialData property is set - if (props.fetchInitialData) { + if (!optionsLoading && props.fetchInitialData) { queryClient.removeQueries({ queryKey: [ 'form-initial-data', @@ -346,22 +343,16 @@ export function ApiForm({ }); initialDataQuery.refetch(); } - }, [props.fetchInitialData]); + }, [props.fetchInitialData, optionsLoading]); - const isLoading = useMemo( + const isLoading: boolean = useMemo( () => isFormLoading || initialDataQuery.isFetching || optionsLoading || isSubmitting || !fields, - [ - isFormLoading, - initialDataQuery.isFetching, - isSubmitting, - fields, - optionsLoading - ] + [isFormLoading, initialDataQuery, isSubmitting, fields, optionsLoading] ); const [initialFocus, setInitialFocus] = useState(''); @@ -381,7 +372,7 @@ export function ApiForm({ }); } - if (isLoading || initialFocus == focusField) { + if (isLoading) { return; } @@ -533,6 +524,14 @@ export function ApiForm({ props.onFormError?.(); }, [props.onFormError]); + if (optionsLoading || initialDataQuery.isFetching) { + return ( + + + + ); + } + return ( @@ -546,13 +545,15 @@ export function ApiForm({ {/* Form Fields */} {(!isValid || nonFieldErrors.length > 0) && ( - - {nonFieldErrors.length > 0 && ( + + {nonFieldErrors.length > 0 ? ( {nonFieldErrors.map((message) => ( {message} ))} + ) : ( + {t`Errors exist for one or more form fields`} )} )} @@ -570,23 +571,22 @@ export function ApiForm({ )} - {!isLoading && ( - - - {!optionsLoading && - Object.entries(fields).map(([fieldName, field]) => ( - - ))} - - - )} + + + {Object.entries(fields).map(([fieldName, field]) => { + return ( + + ); + })} + + {props.postFormContent} diff --git a/src/frontend/src/components/forms/fields/ChoiceField.tsx b/src/frontend/src/components/forms/fields/ChoiceField.tsx index 7407edf4b0..10e860aedd 100644 --- a/src/frontend/src/components/forms/fields/ChoiceField.tsx +++ b/src/frontend/src/components/forms/fields/ChoiceField.tsx @@ -1,6 +1,6 @@ import { Select } from '@mantine/core'; import { useId } from '@mantine/hooks'; -import { useCallback, useMemo } from 'react'; +import { useCallback, useEffect, useMemo } from 'react'; import { FieldValues, UseControllerReturn } from 'react-hook-form'; import { ApiFormFieldType } from './ApiFormField'; @@ -10,7 +10,8 @@ import { ApiFormFieldType } from './ApiFormField'; */ export function ChoiceField({ controller, - definition + definition, + fieldName }: { controller: UseControllerReturn; definition: ApiFormFieldType; @@ -23,6 +24,8 @@ export function ChoiceField({ fieldState: { error } } = controller; + const { value } = field; + // Build a set of choices for the field const choices: any[] = useMemo(() => { let choices = definition.choices ?? []; @@ -48,6 +51,14 @@ export function ChoiceField({ [field.onChange, definition] ); + const choiceValue = useMemo(() => { + if (!value) { + return ''; + } else { + return value.toString(); + } + }, [value]); + return (