From 2cf959cb8d1fd31f8a58e5989b20671614eefb55 Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@gmail.com>
Date: Sun, 25 Aug 2024 11:04:18 +1000
Subject: [PATCH] Forms refactor (#7981)

* Refactor "receive stock" table

- Display errors
- Fix infinite rendering loop
- Correctly set values to undefined on close

* Refactor stock operations table

* Fix for "change stock status" form

* Fix default values

* Unit test fix
---
 .../components/forms/fields/TableField.tsx    |   4 +
 src/frontend/src/forms/BuildForms.tsx         |   2 +
 src/frontend/src/forms/PurchaseOrderForms.tsx | 137 +++++++--------
 src/frontend/src/forms/StockForms.tsx         | 160 +++++++++---------
 4 files changed, 157 insertions(+), 146 deletions(-)

diff --git a/src/frontend/src/components/forms/fields/TableField.tsx b/src/frontend/src/components/forms/fields/TableField.tsx
index 7fb6e3fa39..333ed40cac 100644
--- a/src/frontend/src/components/forms/fields/TableField.tsx
+++ b/src/frontend/src/components/forms/fields/TableField.tsx
@@ -34,6 +34,7 @@ export function TableField({
   const onRowFieldChange = (idx: number, key: string, value: any) => {
     const val = field.value;
     val[idx][key] = value;
+
     field.onChange(val);
   };
 
@@ -114,11 +115,13 @@ export function TableFieldExtraRow({
   fieldDefinition,
   defaultValue,
   emptyValue,
+  error,
   onValueChange
 }: {
   visible: boolean;
   fieldDefinition: ApiFormFieldType;
   defaultValue?: any;
+  error?: string;
   emptyValue?: any;
   onValueChange: (value: any) => void;
 }) {
@@ -151,6 +154,7 @@ export function TableFieldExtraRow({
             <StandaloneField
               fieldDefinition={field}
               defaultValue={defaultValue}
+              error={error}
             />
           </Group>
         </Table.Td>
diff --git a/src/frontend/src/forms/BuildForms.tsx b/src/frontend/src/forms/BuildForms.tsx
index fd86e6b4b8..b5b69e718d 100644
--- a/src/frontend/src/forms/BuildForms.tsx
+++ b/src/frontend/src/forms/BuildForms.tsx
@@ -554,12 +554,14 @@ function BuildAllocateLineRow({
         </Table.Td>
         <Table.Td>
           <StandaloneField
+            fieldName="stock_item"
             fieldDefinition={stockField}
             error={props.rowErrors?.stock_item?.message}
           />
         </Table.Td>
         <Table.Td>
           <StandaloneField
+            fieldName="quantity"
             fieldDefinition={quantityField}
             error={props.rowErrors?.quantity?.message}
           />
diff --git a/src/frontend/src/forms/PurchaseOrderForms.tsx b/src/frontend/src/forms/PurchaseOrderForms.tsx
index a6579a91b9..1d0b4c3019 100644
--- a/src/frontend/src/forms/PurchaseOrderForms.tsx
+++ b/src/frontend/src/forms/PurchaseOrderForms.tsx
@@ -5,7 +5,6 @@ import {
   FocusTrap,
   Group,
   Modal,
-  NumberInput,
   Table,
   TextInput
 } from '@mantine/core';
@@ -34,7 +33,10 @@ import {
   ApiFormAdjustFilterType,
   ApiFormFieldSet
 } from '../components/forms/fields/ApiFormField';
-import { TableFieldExtraRow } from '../components/forms/fields/TableField';
+import {
+  TableFieldExtraRow,
+  TableFieldRowProps
+} from '../components/forms/fields/TableField';
 import { Thumbnail } from '../components/images/Thumbnail';
 import { ProgressBar } from '../components/items/ProgressBar';
 import { StylishText } from '../components/items/StylishText';
@@ -192,67 +194,53 @@ export function usePurchaseOrderFields(): ApiFormFieldSet {
  * Render a table row for a single TableField entry
  */
 function LineItemFormRow({
-  input,
+  props,
   record,
   statuses
 }: {
-  input: any;
+  props: TableFieldRowProps;
   record: any;
   statuses: any;
 }) {
   // Barcode Modal state
-  const [opened, { open, close }] = useDisclosure(false);
+  const [opened, { open, close }] = useDisclosure(false, {
+    onClose: () => props.changeFn(props.idx, 'barcode', undefined)
+  });
 
-  // Location value
-  const [location, setLocation] = useState(
-    input.item.location ??
-      record.part_detail.default_location ??
-      record.part_detail.category_default_location
-  );
-  const [locationOpen, locationHandlers] = useDisclosure(
-    location ? true : false,
-    {
-      onClose: () => input.changeFn(input.idx, 'location', null),
-      onOpen: () => input.changeFn(input.idx, 'location', location)
-    }
-  );
-
-  // Change form value when state is altered
-  useEffect(() => {
-    input.changeFn(input.idx, 'location', location);
-  }, [location]);
+  const [locationOpen, locationHandlers] = useDisclosure(false, {
+    onClose: () => props.changeFn(props.idx, 'location', undefined)
+  });
 
+  // Batch code generator
   const batchCodeGenerator = useBatchCodeGenerator((value: any) => {
-    if (!batchCode) {
-      setBatchCode(value);
+    if (value) {
+      props.changeFn(props.idx, 'batch_code', value);
     }
   });
 
+  // Serial numbebr generator
   const serialNumberGenerator = useSerialNumberGenerator((value: any) => {
-    if (!serials) {
-      setSerials(value);
+    if (value) {
+      props.changeFn(props.idx, 'serial_numbers', value);
     }
   });
 
   const [packagingOpen, packagingHandlers] = useDisclosure(false, {
     onClose: () => {
-      input.changeFn(input.idx, 'packaging', undefined);
+      props.changeFn(props.idx, 'packaging', undefined);
     }
   });
 
   const [noteOpen, noteHandlers] = useDisclosure(false, {
     onClose: () => {
-      input.changeFn(input.idx, 'note', undefined);
+      props.changeFn(props.idx, 'note', undefined);
     }
   });
 
-  // State for serializing
-  const [batchCode, setBatchCode] = useState<string>('');
-  const [serials, setSerials] = useState<string>('');
   const [batchOpen, batchHandlers] = useDisclosure(false, {
     onClose: () => {
-      input.changeFn(input.idx, 'batch_code', undefined);
-      input.changeFn(input.idx, 'serial_numbers', '');
+      props.changeFn(props.idx, 'batch_code', undefined);
+      props.changeFn(props.idx, 'serial_numbers', undefined);
     },
     onOpen: () => {
       // Generate a new batch code
@@ -263,23 +251,23 @@ function LineItemFormRow({
       // Generate new serial numbers
       serialNumberGenerator.update({
         part: record?.supplier_part_detail?.part,
-        quantity: input.item.quantity
+        quantity: props.item.quantity
       });
     }
   });
 
   // Status value
   const [statusOpen, statusHandlers] = useDisclosure(false, {
-    onClose: () => input.changeFn(input.idx, 'status', 10)
+    onClose: () => props.changeFn(props.idx, 'status', undefined)
   });
 
   // Barcode value
   const [barcodeInput, setBarcodeInput] = useState<any>('');
-  const [barcode, setBarcode] = useState(null);
+  const [barcode, setBarcode] = useState<String | undefined>(undefined);
 
   // Change form value when state is altered
   useEffect(() => {
-    input.changeFn(input.idx, 'barcode', barcode);
+    props.changeFn(props.idx, 'barcode', barcode);
   }, [barcode]);
 
   // Update location field description on state change
@@ -371,13 +359,16 @@ function LineItemFormRow({
             progressLabel
           />
         </Table.Td>
-        <Table.Td style={{ width: '1%', whiteSpace: 'nowrap' }}>
-          <NumberInput
-            value={input.item.quantity}
-            style={{ width: '100px' }}
-            max={input.item.quantity}
-            min={0}
-            onChange={(value) => input.changeFn(input.idx, 'quantity', value)}
+        <Table.Td style={{ whiteSpace: 'nowrap' }}>
+          <StandaloneField
+            fieldName="quantity"
+            fieldDefinition={{
+              field_type: 'number',
+              value: props.item.quantity,
+              onValueChange: (value) =>
+                props.changeFn(props.idx, 'quantity', value)
+            }}
+            error={props.rowErrors?.quantity?.message}
           />
         </Table.Td>
         <Table.Td style={{ width: '1%', whiteSpace: 'nowrap' }}>
@@ -404,6 +395,7 @@ function LineItemFormRow({
               size="sm"
               icon={<InvenTreeIcon icon="packaging" />}
               tooltip={t`Adjust Packaging`}
+              tooltipAlignment="top"
               onClick={() => packagingHandlers.toggle()}
               variant={packagingOpen ? 'filled' : 'transparent'}
             />
@@ -428,7 +420,7 @@ function LineItemFormRow({
                 tooltipAlignment="top"
                 variant="filled"
                 color="red"
-                onClick={() => setBarcode(null)}
+                onClick={() => setBarcode(undefined)}
               />
             ) : (
               <ActionButton
@@ -439,7 +431,7 @@ function LineItemFormRow({
                 onClick={() => open()}
               />
             )}
-            <RemoveRowButton onClick={() => input.removeFn(input.idx)} />
+            <RemoveRowButton onClick={() => props.removeFn(props.idx)} />
           </Flex>
         </Table.Td>
       </Table.Tr>
@@ -459,7 +451,7 @@ function LineItemFormRow({
                     structural: false
                   },
                   onValueChange: (value) => {
-                    setLocation(value);
+                    props.changeFn(props.idx, 'location', value);
                   },
                   description: locationDescription,
                   value: location,
@@ -480,7 +472,9 @@ function LineItemFormRow({
                     icon={<InvenTreeIcon icon="default_location" />}
                     tooltip={t`Store at default location`}
                     onClick={() =>
-                      setLocation(
+                      props.changeFn(
+                        props.idx,
+                        'location',
                         record.part_detail.default_location ??
                           record.part_detail.category_default_location
                       )
@@ -492,7 +486,9 @@ function LineItemFormRow({
                   <ActionButton
                     icon={<InvenTreeIcon icon="destination" />}
                     tooltip={t`Store at line item destination `}
-                    onClick={() => setLocation(record.destination)}
+                    onClick={() =>
+                      props.changeFn(props.idx, 'location', record.destination)
+                    }
                     tooltipAlignment="top"
                   />
                 )}
@@ -502,7 +498,13 @@ function LineItemFormRow({
                     <ActionButton
                       icon={<InvenTreeIcon icon="repeat_destination" />}
                       tooltip={t`Store with already received stock`}
-                      onClick={() => setLocation(record.destination_detail.pk)}
+                      onClick={() =>
+                        props.changeFn(
+                          props.idx,
+                          'location',
+                          record.destination_detail.pk
+                        )
+                      }
                       tooltipAlignment="top"
                     />
                   )}
@@ -513,51 +515,56 @@ function LineItemFormRow({
       )}
       <TableFieldExtraRow
         visible={batchOpen}
-        onValueChange={(value) => input.changeFn(input.idx, 'batch', value)}
+        onValueChange={(value) => props.changeFn(props.idx, 'batch', value)}
         fieldDefinition={{
           field_type: 'string',
           label: t`Batch Code`,
-          value: batchCode
+          value: props.item.batch_code
         }}
+        error={props.rowErrors?.batch_code?.message}
       />
       <TableFieldExtraRow
         visible={batchOpen && record.trackable}
         onValueChange={(value) =>
-          input.changeFn(input.idx, 'serial_numbers', value)
+          props.changeFn(props.idx, 'serial_numbers', value)
         }
         fieldDefinition={{
           field_type: 'string',
           label: t`Serial numbers`,
-          value: serials
+          value: props.item.serial_numbers
         }}
+        error={props.rowErrors?.serial_numbers?.message}
       />
       <TableFieldExtraRow
         visible={packagingOpen}
-        onValueChange={(value) => input.changeFn(input.idx, 'packaging', value)}
+        onValueChange={(value) => props.changeFn(props.idx, 'packaging', value)}
         fieldDefinition={{
           field_type: 'string',
           label: t`Packaging`
         }}
         defaultValue={record?.supplier_part_detail?.packaging}
+        error={props.rowErrors?.packaging?.message}
       />
       <TableFieldExtraRow
         visible={statusOpen}
         defaultValue={10}
-        onValueChange={(value) => input.changeFn(input.idx, 'status', value)}
+        onValueChange={(value) => props.changeFn(props.idx, 'status', value)}
         fieldDefinition={{
           field_type: 'choice',
           api_url: apiUrl(ApiEndpoints.stock_status),
           choices: statuses,
           label: t`Status`
         }}
+        error={props.rowErrors?.status?.message}
       />
       <TableFieldExtraRow
         visible={noteOpen}
-        onValueChange={(value) => input.changeFn(input.idx, 'note', value)}
+        onValueChange={(value) => props.changeFn(props.idx, 'note', value)}
         fieldDefinition={{
           field_type: 'string',
           label: t`Note`
         }}
+        error={props.rowErrors?.note?.message}
       />
     </>
   );
@@ -619,12 +626,12 @@ export function useReceiveLineItems(props: LineItemsForm) {
           barcode: null
         };
       }),
-      modelRenderer: (instance) => {
-        const record = records[instance.item.line_item];
+      modelRenderer: (row: TableFieldRowProps) => {
+        const record = records[row.item.line_item];
 
         return (
           <LineItemFormRow
-            input={instance}
+            props={row}
             record={record}
             statuses={data}
             key={record.pk}
@@ -640,18 +647,14 @@ export function useReceiveLineItems(props: LineItemsForm) {
     }
   };
 
-  const url = apiUrl(ApiEndpoints.purchase_order_receive, null, {
-    id: props.orderPk
-  });
-
   return useCreateApiFormModal({
     ...props.formProps,
-    url: url,
+    url: apiUrl(ApiEndpoints.purchase_order_receive, props.orderPk),
     title: t`Receive Line Items`,
     fields: fields,
     initialData: {
       location: null
     },
-    size: 'xl'
+    size: '80%'
   });
 }
diff --git a/src/frontend/src/forms/StockForms.tsx b/src/frontend/src/forms/StockForms.tsx
index 18de04eb91..772d704d98 100644
--- a/src/frontend/src/forms/StockForms.tsx
+++ b/src/frontend/src/forms/StockForms.tsx
@@ -1,18 +1,22 @@
 import { t } from '@lingui/macro';
-import { Flex, Group, NumberInput, Skeleton, Table, Text } from '@mantine/core';
+import { Flex, Group, Skeleton, Table, Text } from '@mantine/core';
 import { useDisclosure } from '@mantine/hooks';
 import { modals } from '@mantine/modals';
 import { useQuery, useSuspenseQuery } from '@tanstack/react-query';
-import { Suspense, useCallback, useMemo, useState } from 'react';
+import { Suspense, useMemo, useState } from 'react';
 
 import { api } from '../App';
 import { ActionButton } from '../components/buttons/ActionButton';
 import RemoveRowButton from '../components/buttons/RemoveRowButton';
+import { StandaloneField } from '../components/forms/StandaloneField';
 import {
   ApiFormAdjustFilterType,
   ApiFormFieldSet
 } from '../components/forms/fields/ApiFormField';
-import { TableFieldExtraRow } from '../components/forms/fields/TableField';
+import {
+  TableFieldExtraRow,
+  TableFieldRowProps
+} from '../components/forms/fields/TableField';
 import { Thumbnail } from '../components/images/Thumbnail';
 import { StylishText } from '../components/items/StylishText';
 import { StatusRenderer } from '../components/render/StatusRenderer';
@@ -139,7 +143,9 @@ export function useStockFields({
         value: batchCode,
         onValueChange: (value) => setBatchCode(value)
       },
-      status_custom_key: {},
+      status_custom_key: {
+        label: t`Stock Status`
+      },
       expiry_date: {
         // TODO: icon
       },
@@ -295,47 +301,37 @@ type StockRow = {
 };
 
 function StockOperationsRow({
-  input,
+  props,
   transfer = false,
   add = false,
   setMax = false,
   merge = false,
   record
 }: {
-  input: StockRow;
+  props: TableFieldRowProps;
   transfer?: boolean;
   add?: boolean;
   setMax?: boolean;
   merge?: boolean;
   record?: any;
 }) {
-  const item = input.item;
-
-  const [value, setValue] = useState<StockItemQuantity>(
-    add ? 0 : item.quantity ?? 0
-  );
-
-  const onChange = useCallback(
-    (value: any) => {
-      setValue(value);
-      input.changeFn(input.idx, 'quantity', value);
-    },
-    [item]
+  const [quantity, setQuantity] = useState<StockItemQuantity>(
+    add ? 0 : props.item?.quantity ?? 0
   );
 
   const removeAndRefresh = () => {
-    input.removeFn(input.idx);
+    props.removeFn(props.idx);
   };
 
   const [packagingOpen, packagingHandlers] = useDisclosure(false, {
     onOpen: () => {
       if (transfer) {
-        input.changeFn(input.idx, 'packaging', record?.packaging || undefined);
+        props.changeFn(props.idx, 'packaging', record?.packaging || undefined);
       }
     },
     onClose: () => {
       if (transfer) {
-        input.changeFn(input.idx, 'packaging', undefined);
+        props.changeFn(props.idx, 'packaging', undefined);
       }
     }
   });
@@ -371,25 +367,24 @@ function StockOperationsRow({
           {record.location ? record.location_detail?.pathstring : '-'}
         </Table.Td>
         <Table.Td>
-          <Flex align="center" gap="xs">
-            <Group justify="space-between">
-              <Text>{stockString}</Text>
-              <StatusRenderer
-                status={record.status}
-                type={ModelType.stockitem}
-              />
-            </Group>
-          </Flex>
+          <Group grow justify="space-between" wrap="nowrap">
+            <Text>{stockString}</Text>
+            <StatusRenderer status={record.status} type={ModelType.stockitem} />
+          </Group>
         </Table.Td>
         {!merge && (
           <Table.Td>
-            <NumberInput
-              value={value}
-              onChange={onChange}
-              disabled={!!record.serial && record.quantity == 1}
-              max={setMax ? record.quantity : undefined}
-              min={0}
-              style={{ maxWidth: '100px' }}
+            <StandaloneField
+              fieldName="quantity"
+              fieldDefinition={{
+                field_type: 'number',
+                value: quantity,
+                onValueChange: (value: any) => {
+                  setQuantity(value);
+                  props.changeFn(props.idx, 'quantity', value);
+                }
+              }}
+              error={props.rowErrors?.quantity?.message}
             />
           </Table.Td>
         )}
@@ -397,7 +392,9 @@ function StockOperationsRow({
           <Flex gap="3px">
             {transfer && (
               <ActionButton
-                onClick={() => moveToDefault(record, value, removeAndRefresh)}
+                onClick={() =>
+                  moveToDefault(record, props.item.quantity, removeAndRefresh)
+                }
                 icon={<InvenTreeIcon icon="default_location" />}
                 tooltip={t`Move to default location`}
                 tooltipAlignment="top"
@@ -416,7 +413,7 @@ function StockOperationsRow({
                 variant={packagingOpen ? 'filled' : 'transparent'}
               />
             )}
-            <RemoveRowButton onClick={() => input.removeFn(input.idx)} />
+            <RemoveRowButton onClick={() => props.removeFn(props.idx)} />
           </Flex>
         </Table.Td>
       </Table.Tr>
@@ -424,7 +421,7 @@ function StockOperationsRow({
         <TableFieldExtraRow
           visible={transfer && packagingOpen}
           onValueChange={(value: any) => {
-            input.changeFn(input.idx, 'packaging', value || undefined);
+            props.changeFn(props.idx, 'packaging', value || undefined);
           }}
           fieldDefinition={{
             field_type: 'string',
@@ -452,9 +449,9 @@ function mapAdjustmentItems(items: any[]) {
     return {
       pk: elem.pk,
       quantity: elem.quantity,
-      batch: elem.batch,
-      status: elem.status,
-      packaging: elem.packaging,
+      batch: elem.batch || undefined,
+      status: elem.status || undefined,
+      packaging: elem.packaging || undefined,
       obj: elem
     };
   });
@@ -473,14 +470,16 @@ function stockTransferFields(items: any[]): ApiFormFieldSet {
     items: {
       field_type: 'table',
       value: mapAdjustmentItems(items),
-      modelRenderer: (val) => {
+      modelRenderer: (row: TableFieldRowProps) => {
+        const record = records[row.item.pk];
+
         return (
           <StockOperationsRow
-            input={val}
+            props={row}
             transfer
             setMax
-            key={val.item.pk}
-            record={records[val.item.pk]}
+            key={record.pk}
+            record={record}
           />
         );
       },
@@ -508,13 +507,16 @@ function stockRemoveFields(items: any[]): ApiFormFieldSet {
     items: {
       field_type: 'table',
       value: mapAdjustmentItems(items),
-      modelRenderer: (val) => {
+      modelRenderer: (row: TableFieldRowProps) => {
+        const record = records[row.item.pk];
+
         return (
           <StockOperationsRow
-            input={val}
+            props={row}
             setMax
-            key={val.item.pk}
-            record={records[val.item.pk]}
+            add
+            key={record.pk}
+            record={record}
           />
         );
       },
@@ -537,14 +539,11 @@ function stockAddFields(items: any[]): ApiFormFieldSet {
     items: {
       field_type: 'table',
       value: mapAdjustmentItems(items),
-      modelRenderer: (val) => {
+      modelRenderer: (row: TableFieldRowProps) => {
+        const record = records[row.item.pk];
+
         return (
-          <StockOperationsRow
-            input={val}
-            add
-            key={val.item.pk}
-            record={records[val.item.pk]}
-          />
+          <StockOperationsRow props={row} add key={record.pk} record={record} />
         );
       },
       headers: [t`Part`, t`Location`, t`In Stock`, t`Add`, t`Actions`]
@@ -566,12 +565,12 @@ function stockCountFields(items: any[]): ApiFormFieldSet {
     items: {
       field_type: 'table',
       value: mapAdjustmentItems(items),
-      modelRenderer: (val) => {
+      modelRenderer: (row: TableFieldRowProps) => {
         return (
           <StockOperationsRow
-            input={val}
-            key={val.item.pk}
-            record={records[val.item.pk]}
+            props={row}
+            key={row.item.pk}
+            record={records[row.item.pk]}
           />
         );
       },
@@ -596,19 +595,19 @@ function stockChangeStatusFields(items: any[]): ApiFormFieldSet {
       value: items.map((elem) => {
         return elem.pk;
       }),
-      modelRenderer: (val) => {
+      modelRenderer: (row: TableFieldRowProps) => {
         return (
           <StockOperationsRow
-            input={val}
-            key={val.item}
+            props={row}
+            key={row.item}
             merge
-            record={records[val.item]}
+            record={records[row.item]}
           />
         );
       },
       headers: [t`Part`, t`Location`, t`In Stock`, t`Actions`]
     },
-    status_custom_key: {},
+    status: {},
     note: {}
   };
 
@@ -631,13 +630,13 @@ function stockMergeFields(items: any[]): ApiFormFieldSet {
           obj: elem
         };
       }),
-      modelRenderer: (val) => {
+      modelRenderer: (row: TableFieldRowProps) => {
         return (
           <StockOperationsRow
-            input={val}
-            key={val.item.item}
+            props={row}
+            key={row.item.item}
             merge
-            record={records[val.item.item]}
+            record={records[row.item.item]}
           />
         );
       },
@@ -673,13 +672,13 @@ function stockAssignFields(items: any[]): ApiFormFieldSet {
           obj: elem
         };
       }),
-      modelRenderer: (val) => {
+      modelRenderer: (row: TableFieldRowProps) => {
         return (
           <StockOperationsRow
-            input={val}
-            key={val.item.item}
+            props={row}
+            key={row.item.item}
             merge
-            record={records[val.item.item]}
+            record={records[row.item.item]}
           />
         );
       },
@@ -709,13 +708,15 @@ function stockDeleteFields(items: any[]): ApiFormFieldSet {
       value: items.map((elem) => {
         return elem.pk;
       }),
-      modelRenderer: (val) => {
+      modelRenderer: (row: TableFieldRowProps) => {
+        const record = records[row.item];
+
         return (
           <StockOperationsRow
-            input={val}
-            key={val.item}
+            props={row}
+            key={record.pk}
             merge
-            record={records[val.item]}
+            record={record}
           />
         );
       },
@@ -803,6 +804,7 @@ function stockOperationModal({
     url: endpoint,
     fields: fields,
     title: title,
+    size: '80%',
     onFormSuccess: () => refresh()
   });
 }