From 9a8b8dadeb6da976ea94654c1bcb7a59a1c1e291 Mon Sep 17 00:00:00 2001 From: Oliver Walters Date: Sat, 27 Jun 2026 04:48:47 +0000 Subject: [PATCH] Don't use idx to identify rows --- .../components/forms/fields/TableField.tsx | 120 ++++++++++-- src/frontend/src/forms/StockForms.tsx | 179 ++++++++++++------ 2 files changed, 222 insertions(+), 77 deletions(-) diff --git a/src/frontend/src/components/forms/fields/TableField.tsx b/src/frontend/src/components/forms/fields/TableField.tsx index ed0debe4db..a1ed084fa7 100644 --- a/src/frontend/src/components/forms/fields/TableField.tsx +++ b/src/frontend/src/components/forms/fields/TableField.tsx @@ -2,7 +2,7 @@ import { t } from '@lingui/core/macro'; import { Trans } from '@lingui/react/macro'; import { Alert, Container, Group, Stack, Table, Text } from '@mantine/core'; import { IconExclamationCircle } from '@tabler/icons-react'; -import { type ReactNode, useCallback, useEffect, useMemo } from 'react'; +import { type ReactNode, useCallback, useEffect, useMemo, useRef } from 'react'; import type { FieldValues, UseControllerReturn } from 'react-hook-form'; import { AddItemButton } from '@lib/components/AddItemButton'; @@ -14,15 +14,25 @@ import { StandaloneField } from '../StandaloneField'; export interface TableFieldRowProps { item: any; idx: number; + rowId?: string | number; rowErrors: any; control: UseControllerReturn; - changeFn: (idx: number, key: string, value: any) => void; - removeFn: (idx: number) => void; + changeFn: (idx: number | string, key: string, value: any) => void; + removeFn: (idx: number | string) => void; +} + +function getRowIdentifier(item: any, idx: number): string | number { + if (item && typeof item === 'object') { + return item.pk ?? item.item ?? item.id ?? item.uuid ?? idx; + } + + return item ?? idx; } function TableFieldRow({ item, idx, + rowId, errors, definition, control, @@ -31,11 +41,12 @@ function TableFieldRow({ }: Readonly<{ item: any; idx: number; + rowId?: string | number; errors: any; definition: ApiFormFieldType; control: UseControllerReturn; changeFn: (idx: number, key: string, value: any) => void; - removeFn: (idx: number) => void; + removeFn: (idx: number | string) => void; }>) { // Table fields require render function if (!definition.modelRenderer) { @@ -53,6 +64,7 @@ function TableFieldRow({ return definition.modelRenderer({ item: item, idx: idx, + rowId: rowId, rowErrors: errors, control: control, changeFn: changeFn, @@ -98,23 +110,84 @@ export function TableField({ } = control; const { value } = field; - const onRowFieldChange = useCallback( - (idx: number, key: string, value: any) => { - const val = field.value; - val[idx][key] = value; + const valueRef = useRef(value); + const rowIndexByIdRef = useRef(new Map()); - field.onChange(val); + useEffect(() => { + valueRef.current = value; + + const nextRowIndexById = new Map(); + + value?.forEach((item: any, idx: number) => { + nextRowIndexById.set(getRowIdentifier(item, idx), idx); + }); + + rowIndexByIdRef.current = nextRowIndexById; + }, [value]); + + const resolveRowIndex = useCallback((identifier: number | string) => { + const mappedIndex = rowIndexByIdRef.current.get(identifier); + + if (mappedIndex !== undefined) { + return mappedIndex; + } + + if (typeof identifier === 'number' && identifier >= 0) { + return identifier; + } + + return undefined; + }, []); + + const onRowFieldChange = useCallback( + (identifier: number | string, key: string, rowValue: any) => { + const idx = resolveRowIndex(identifier); + + if (idx === undefined) { + return; + } + + const currentValue = valueRef.current; + + if (!Array.isArray(currentValue) || currentValue[idx] === undefined) { + return; + } + + const nextValue = [...currentValue]; + const currentRow = nextValue[idx]; + + if (currentRow && typeof currentRow === 'object') { + nextValue[idx] = { + ...currentRow, + [key]: rowValue + }; + } + + field.onChange(nextValue); }, - [field.value, field.onChange] + [field.onChange, resolveRowIndex] ); const removeRow = useCallback( - (idx: number) => { - const val = field.value; - val.splice(idx, 1); - field.onChange(val); + (identifier: number | string) => { + const idx = resolveRowIndex(identifier); + + if (idx === undefined) { + return; + } + + const currentValue = valueRef.current; + + if (!Array.isArray(currentValue)) { + return; + } + + const nextValue = [...currentValue]; + nextValue.splice(idx, 1); + + field.onChange(nextValue); }, - [field.value, field.onChange] + [field.onChange, resolveRowIndex] ); // Extract errors associated with the current row @@ -152,11 +225,14 @@ export function TableField({ {(value?.length ?? 0) > 0 ? ( value.map((item: any, idx: number) => { + const rowId = getRowIdentifier(item, idx); + return ( @@ -230,10 +304,16 @@ export function TableFieldExtraRow({ emptyValue?: any; onValueChange: (value: any) => void; }) { + const hasMounted = useRef(false); + // Callback whenever the visibility of the sub-field changes + // Skip the initial mount — the value was never set, nothing to reset useEffect(() => { + if (!hasMounted.current) { + hasMounted.current = true; + return; + } if (!visible) { - // If the sub-field is hidden, reset the value to the "empty" value onValueChange(emptyValue); } }, [visible]); diff --git a/src/frontend/src/forms/StockForms.tsx b/src/frontend/src/forms/StockForms.tsx index a0de3608b2..0d979c93d4 100644 --- a/src/frontend/src/forms/StockForms.tsx +++ b/src/frontend/src/forms/StockForms.tsx @@ -38,6 +38,7 @@ import dayjs from 'dayjs'; import { type JSX, Suspense, + memo, useCallback, useEffect, useMemo, @@ -567,6 +568,50 @@ type StockRow = { removeFn: TableFieldRefreshFn; }; +function ReturnStockMoveButton({ + record, + quantity, + onRemove, + returnStock +}: { + record: any; + quantity: StockItemQuantity; + onRemove: () => void; + returnStock: boolean; +}) { + const form = useFormContext(); + + return ( + + moveToDefault( + record, + quantity, + onRemove, + returnStock + ? { + title: t`Confirm Stock Return`, + onConfirm: (location: number) => { + form.setValue('location', location, { + shouldDirty: true, + shouldValidate: true + }); + } + } + : undefined + ) + } + icon={} + tooltip={t`Move to default location`} + tooltipAlignment='top' + disabled={ + !record.part_detail?.default_location && + !record.part_detail?.category_default_location + } + /> + ); +} + function StockOperationsRow({ props, transfer = false, @@ -588,7 +633,7 @@ function StockOperationsRow({ returnStock?: boolean; record?: any; }) { - const form = useFormContext(); + const rowId = props.rowId ?? props.idx; const statusOptions: ApiFormFieldChoice[] = useMemo(() => { return ( @@ -607,23 +652,26 @@ function StockOperationsRow({ const [status, setStatus] = useState(undefined); - const removeAndRefresh = () => { - props.removeFn(props.idx); - }; + const removeAndRefresh = useCallback(() => { + props.removeFn(rowId); + }, [props.removeFn, rowId]); - const callChangeFn = (idx: number, key: string, value: any) => { - setTimeout(() => props.changeFn(idx, key, value), 0); - }; + const callChangeFn = useCallback( + (identifier: number | string, key: string, value: any) => { + props.changeFn(identifier, key, value); + }, + [props.changeFn] + ); const [packagingOpen, packagingHandlers] = useDisclosure(false, { onOpen: () => { if (transfer) { - callChangeFn(props.idx, 'packaging', record?.packaging || undefined); + callChangeFn(rowId, 'packaging', record?.packaging || undefined); } }, onClose: () => { if (transfer) { - callChangeFn(props.idx, 'packaging', undefined); + callChangeFn(rowId, 'packaging', undefined); } } }); @@ -631,11 +679,11 @@ function StockOperationsRow({ const [statusOpen, statusHandlers] = useDisclosure(false, { onOpen: () => { setStatus(record?.status_custom_key || record?.status || undefined); - props.changeFn(props.idx, 'status', record?.status || undefined); + props.changeFn(rowId, 'status', record?.status || undefined); }, onClose: () => { setStatus(undefined); - callChangeFn(props.idx, 'status', undefined); + callChangeFn(rowId, 'status', undefined); } }); @@ -696,7 +744,7 @@ function StockOperationsRow({ value: quantity, onValueChange: (value: any) => { setQuantity(value); - props.changeFn(props.idx, 'quantity', value); + props.changeFn(rowId, 'quantity', value); } }} error={props.rowErrors?.quantity?.message} @@ -705,35 +753,28 @@ function StockOperationsRow({ )} - {transfer && ( - - moveToDefault( - record, - props.item.quantity, - removeAndRefresh, - returnStock - ? { - title: t`Confirm Stock Return`, - onConfirm: (location: number) => { - form.setValue('location', location, { - shouldDirty: true, - shouldValidate: true - }); - } - } - : undefined - ) - } - icon={} - tooltip={t`Move to default location`} - tooltipAlignment='top' - disabled={ - !record.part_detail?.default_location && - !record.part_detail?.category_default_location - } - /> - )} + {transfer && + (returnStock ? ( + + ) : ( + + moveToDefault(record, props.item.quantity, removeAndRefresh) + } + icon={} + tooltip={t`Move to default location`} + tooltipAlignment='top' + disabled={ + !record.part_detail?.default_location && + !record.part_detail?.category_default_location + } + /> + ))} {changeStatus && ( } tooltip={t`Merge into existing stock`} - onClick={() => - callChangeFn(props.idx, 'merge', !props.item?.merge) - } + onClick={() => callChangeFn(rowId, 'merge', !props.item?.merge)} variant={props.item?.merge ? 'filled' : 'transparent'} /> )} - props.removeFn(props.idx)} /> + props.removeFn(rowId)} /> @@ -772,7 +811,7 @@ function StockOperationsRow({ visible={statusOpen} onValueChange={(value: any) => { setStatus(value); - props.changeFn(props.idx, 'status', value || undefined); + props.changeFn(rowId, 'status', value || undefined); }} fieldName='status' fieldDefinition={{ @@ -788,7 +827,7 @@ function StockOperationsRow({ { - props.changeFn(props.idx, 'packaging', value || undefined); + props.changeFn(rowId, 'packaging', value || undefined); }} fieldName='packaging' fieldDefinition={{ @@ -802,6 +841,27 @@ function StockOperationsRow({ ); } +const MemoizedStockOperationsRow = memo( + StockOperationsRow, + (previousProps, nextProps) => { + return ( + previousProps.props.rowId === nextProps.props.rowId && + previousProps.props.item === nextProps.props.item && + previousProps.props.rowErrors === nextProps.props.rowErrors && + previousProps.props.changeFn === nextProps.props.changeFn && + previousProps.props.removeFn === nextProps.props.removeFn && + previousProps.record === nextProps.record && + previousProps.transfer === nextProps.transfer && + previousProps.changeStatus === nextProps.changeStatus && + previousProps.add === nextProps.add && + previousProps.setMax === nextProps.setMax && + previousProps.merge === nextProps.merge && + previousProps.transferMerge === nextProps.transferMerge && + previousProps.returnStock === nextProps.returnStock + ); + } +); + type StockItemQuantity = number | '' | undefined; type StockAdjustmentItem = { @@ -850,7 +910,7 @@ function stockTransferFields( const record = records[row.item.pk]; return ( - { return ( - { return ( - { return ( - { return ( - stockCountFields(items), + [] + ); + return useStockOperationModal({ ...props, - fieldGenerator: stockCountFields, + fieldGenerator: fieldGenerator, endpoint: ApiEndpoints.stock_count, title: t`Count Stock`, successMessage: t`Stock counted`,