From 1bfc4609b4f299c8a3d7ba3831ac1d90de69dd7f Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Tue, 2 Apr 2024 08:04:37 +0000 Subject: [PATCH 01/29] usage statues of moving items #456 --- src/systems/systemDetails.component.tsx | 2 +- src/systems/systemItemsDialog.component.tsx | 280 ++++++++++-- .../systemItemsTable.component.test.tsx | 2 +- src/systems/systemItemsTable.component.tsx | 420 ++++++++++++++++-- 4 files changed, 626 insertions(+), 78 deletions(-) diff --git a/src/systems/systemDetails.component.tsx b/src/systems/systemDetails.component.tsx index e60173237..f33711d33 100644 --- a/src/systems/systemDetails.component.tsx +++ b/src/systems/systemDetails.component.tsx @@ -180,7 +180,7 @@ function SystemDetails(props: SystemDetailsProps) { - + )} diff --git a/src/systems/systemItemsDialog.component.tsx b/src/systems/systemItemsDialog.component.tsx index fe97e23b6..960ea2899 100644 --- a/src/systems/systemItemsDialog.component.tsx +++ b/src/systems/systemItemsDialog.component.tsx @@ -1,19 +1,25 @@ import { + Box, Button, Dialog, DialogActions, DialogContent, DialogTitle, Grid, + Step, + StepLabel, + Stepper, + Typography, } from '@mui/material'; import { MRT_RowSelectionState } from 'material-react-table'; import React from 'react'; import { useMoveItemsToSystem } from '../api/items'; import { useSystem, useSystems, useSystemsBreadcrumbs } from '../api/systems'; -import { Item } from '../app.types'; +import { Item, UsageStatusType } from '../app.types'; import handleTransferState from '../handleTransferState'; import Breadcrumbs from '../view/breadcrumbs.component'; import { SystemsTableView } from './systemsTableView.component'; +import { SystemItemsTable } from './systemItemsTable.component'; export interface SystemItemsDialogProps { open: boolean; @@ -23,6 +29,17 @@ export interface SystemItemsDialogProps { parentSystemId: string | null; } +export interface UsageStatuesType { + item_id: string; + catalogue_item_id: string; + usageStatus: UsageStatusType | ''; +} + +export interface UsageStatuesErrorType + extends Omit { + error: boolean; +} + const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { const { open, onClose, selectedItems, onChangeSelectedItems } = props; @@ -31,10 +48,47 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { const [parentSystemId, setParentSystemId] = React.useState( props.parentSystemId ); + const [usageStatues, setUsageStatues] = React.useState( + [] + ); + + const [usageStatuesErrors, setUsageStatuesErrors] = React.useState< + UsageStatuesErrorType[] + >([]); + + const [aggregatedCellUsageStatus, setAggregatedCellUsageStatus] = + React.useState[]>([]); + + const [placeIntoSystemError, setPlaceIntoSystemError] = React.useState(false); + React.useEffect(() => { + if (open) { + const initialUsageStatues: UsageStatuesType[] = selectedItems.map( + (item) => ({ + item_id: item.id, + catalogue_item_id: item.catalogue_item_id, + usageStatus: '', + }) + ); + + const initialUsageStatuesErrors: UsageStatuesErrorType[] = + selectedItems.map((item) => ({ + item_id: item.id, + catalogue_item_id: item.catalogue_item_id, + error: false, + })); + setUsageStatues(initialUsageStatues); + setUsageStatuesErrors(initialUsageStatuesErrors); + } + }, [open, selectedItems]); + React.useEffect(() => { setParentSystemId(props.parentSystemId); }, [props.parentSystemId]); + React.useEffect(() => { + setPlaceIntoSystemError(false); + }, [parentSystemId]); + const { data: parentSystemBreadcrumbs } = useSystemsBreadcrumbs(parentSystemId); @@ -48,11 +102,53 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { const { mutateAsync: moveItemsToSystem, isPending: isMovePending } = useMoveItemsToSystem(); + const errorUsageStatuesItemId = usageStatuesErrors + .map((status) => { + if (status.error) { + return status.item_id; + } + return null; + }) + .filter((errorItemId) => errorItemId !== null); + + const validateUsageStatus = React.useCallback(() => { + const errorItemId = usageStatues + .map((status) => { + if (status.usageStatus === '') { + return status.item_id; + } + return null; + }) + .filter((errorItemId) => errorItemId !== null); + + setUsageStatuesErrors((prevErrors) => + prevErrors.map((error) => { + const index = errorItemId.indexOf(error.item_id); + if (index !== -1) { + return { ...error, error: true }; // Set error status to true if item_id exists in errorItemId + } + return error; // Return unchanged error object if item_id doesn't exist in errorItemId + }) + ); + return errorItemId.length !== 0; + }, [usageStatues]); + const handleClose = React.useCallback(() => { + setActiveStep(0); onClose(); }, [onClose]); + const hasSystemErrors = + props.parentSystemId === parentSystemId || + parentSystemId === null || + !(!targetSystemLoading && targetSystem !== undefined); + const handleMoveTo = React.useCallback(() => { + const hasUsageStatusErrors = validateUsageStatus(); + if (hasSystemErrors || hasUsageStatusErrors) { + hasSystemErrors && setPlaceIntoSystemError(hasSystemErrors); + return; + } // Ensure finished loading and not moving to root // (where we don't need to load anything as the name is known) if (!targetSystemLoading && targetSystem !== undefined) { @@ -69,15 +165,99 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { } }, [ handleClose, + hasSystemErrors, moveItemsToSystem, onChangeSelectedItems, selectedItems, targetSystem, targetSystemLoading, + validateUsageStatus, ]); + // Stepper + const STEPS = ['Place into a system', 'Set usage statues']; + const [activeStep, setActiveStep] = React.useState(0); + + const handleNext = React.useCallback( + (step: number) => { + switch (step) { + case 0: { + setPlaceIntoSystemError(hasSystemErrors); + return ( + !hasSystemErrors && + setActiveStep((prevActiveStep) => prevActiveStep + 1) + ); + } + default: + setActiveStep((prevActiveStep) => prevActiveStep + 1); + } + }, + [hasSystemErrors] + ); + + const handleBack = () => { + setActiveStep((prevActiveStep) => prevActiveStep - 1); + }; + + const isStepFailed = React.useCallback( + (step: number) => { + switch (step) { + case 0: { + return placeIntoSystemError; + } + case 1: + return errorUsageStatuesItemId.length !== 0; + } + }, + [errorUsageStatuesItemId.length, placeIntoSystemError] + ); + + const renderStepContent = (step: number) => { + switch (step) { + case 0: + return ( + + + { + setParentSystemId(null); + }} + navigateHomeAriaLabel={'navigate to systems home'} + /> + + + + + + ); + case 1: + return ( + + ); + } + }; + return ( - + @@ -87,45 +267,75 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { : '1 item'}{' '} to a different system - - { - setParentSystemId(null); - }} - navigateHomeAriaLabel={'navigate to systems home'} - /> - - + + {STEPS.map((label, index) => { + const labelProps: { + optional?: React.ReactNode; + error?: boolean; + } = {}; + + if (isStepFailed(index)) { + labelProps.optional = ( + + {index === 1 && 'Please select a usage status for all items'} + {index === 0 && + 'Move items from current location or root to another directory'} + + ); + labelProps.error = true; + } + + return ( + + setActiveStep(index)}> + {label} + + + ); + })} + + + {renderStepContent(activeStep)} - - + + + {activeStep === STEPS.length - 1 ? ( + + ) : ( + + )} ); diff --git a/src/systems/systemItemsTable.component.test.tsx b/src/systems/systemItemsTable.component.test.tsx index bc217a76f..0ab46e731 100644 --- a/src/systems/systemItemsTable.component.test.tsx +++ b/src/systems/systemItemsTable.component.test.tsx @@ -21,7 +21,7 @@ describe('SystemItemsTable', () => { }; beforeEach(() => { - props = { system: mockSystem }; + props = { system: mockSystem, type: 'normal' }; user = userEvent.setup(); diff --git a/src/systems/systemItemsTable.component.tsx b/src/systems/systemItemsTable.component.tsx index 3c416a748..4553506e9 100644 --- a/src/systems/systemItemsTable.component.tsx +++ b/src/systems/systemItemsTable.component.tsx @@ -1,6 +1,17 @@ import ClearIcon from '@mui/icons-material/Clear'; import DriveFileMoveOutlinedIcon from '@mui/icons-material/DriveFileMoveOutlined'; -import { Box, Button, Link as MuiLink, Typography } from '@mui/material'; +import ErrorIcon from '@mui/icons-material/Error'; +import { + Box, + Button, + FormControl, + FormHelperText, + InputLabel, + MenuItem, + Link as MuiLink, + Select, + Typography, +} from '@mui/material'; import { MRT_ColumnDef, MRT_ColumnFiltersState, @@ -15,7 +26,10 @@ import { useCatalogueItemIds } from '../api/catalogueItems'; import { useItems } from '../api/items'; import { CatalogueItem, Item, System, UsageStatusType } from '../app.types'; import ItemsDetailsPanel from '../items/itemsDetailsPanel.component'; -import SystemItemsDialog from './systemItemsDialog.component'; +import SystemItemsDialog, { + UsageStatuesErrorType, + UsageStatuesType, +} from './systemItemsDialog.component'; import { formatDateTimeStrings } from '../utils'; const MoveItemsButton = (props: { @@ -55,11 +69,35 @@ interface TableRowData { } export interface SystemItemsTableProps { - system: System; + system?: System; + type: 'normal' | 'usageStatus'; + moveToSelectedItems?: Item[]; + usageStatues?: UsageStatuesType[]; + onChangeUsageStatues?: React.Dispatch< + React.SetStateAction + >; + usageStatuesErrors?: UsageStatuesErrorType[]; + onChangeUsageStatuesErrors?: React.Dispatch< + React.SetStateAction + >; + aggregatedCellUsageStatus?: Omit[]; + onChangeAggregatedCellUsageStatus?: React.Dispatch< + React.SetStateAction[]> + >; } export function SystemItemsTable(props: SystemItemsTableProps) { - const { system } = props; + const { + system, + type, + moveToSelectedItems, + usageStatues, + onChangeUsageStatues, + usageStatuesErrors, + onChangeUsageStatuesErrors, + aggregatedCellUsageStatus, + onChangeAggregatedCellUsageStatus, + } = props; // States const [tableRows, setTableRows] = React.useState([]); @@ -69,20 +107,33 @@ export function SystemItemsTable(props: SystemItemsTableProps) { // Data const { data: itemsData, isLoading: isLoadingItems } = useItems( - system.id, + system?.id, undefined ); // Obtain the selected system data, not just the selection state const selectedRowIds = Object.keys(rowSelection); const selectedItems = - itemsData?.filter((item) => selectedRowIds.includes(item.id)) ?? []; + type === 'normal' + ? itemsData?.filter((item) => selectedRowIds.includes(item.id)) ?? [] + : moveToSelectedItems?.filter((item) => + selectedRowIds.includes(item.id) + ) ?? []; // Fetch catalogue items for each item to display in the table - const catalogueItemIdSet = new Set( - itemsData?.map((item) => item.catalogue_item_id) ?? [] + const catalogueItemIdSet = React.useMemo( + () => + type === 'normal' + ? new Set( + itemsData?.map((item) => item.catalogue_item_id) ?? [] + ) + : new Set( + moveToSelectedItems?.map((item) => item.catalogue_item_id) ?? [] + ), + [itemsData, moveToSelectedItems, type] ); let isLoading = isLoadingItems; + const catalogueItemList: (CatalogueItem | undefined)[] = useCatalogueItemIds( Array.from(catalogueItemIdSet.values()) ).map((query) => { @@ -107,6 +158,19 @@ export function SystemItemsTable(props: SystemItemsTableProps) { }) as TableRowData ) ); + } else if (moveToSelectedItems) { + setTableRows( + moveToSelectedItems.map( + (itemData) => + ({ + item: itemData, + catalogueItem: catalogueItemList?.find( + (catalogueItem) => + catalogueItem?.id === itemData.catalogue_item_id + ), + }) as TableRowData + ) + ); } // Purposefully leave out catalogueItemList - this will never be the same due // to the reference changing so instead am relying on isLoading to have changed to @@ -116,39 +180,111 @@ export function SystemItemsTable(props: SystemItemsTableProps) { // eslint-disable-next-line react-hooks/exhaustive-deps }, [isLoading, itemsData]); + const status = (usageStatus: UsageStatusType | undefined | '') => { + if (typeof usageStatus !== 'number') return ''; + const status = Object.values(UsageStatusType).find( + (value) => + UsageStatusType[value as keyof typeof UsageStatusType] === usageStatus + ); + return status || ''; + }; + + const isFirstRun = React.useRef(true); + React.useEffect(() => { + if ( + isFirstRun.current && + onChangeAggregatedCellUsageStatus && + aggregatedCellUsageStatus?.length === 0 + ) { + isFirstRun.current = false; // Set isFirstRun to false after the first run + const initialUsageStatues: Omit[] = + Array.from(catalogueItemIdSet).map((catalogue_item_id) => ({ + catalogue_item_id: catalogue_item_id, + usageStatus: '', // Setting usageStatus to an empty string by default + })); + + onChangeAggregatedCellUsageStatus(initialUsageStatues); // Using onChangeAggregatedCellUsageStatus to update state + } + }, [ + aggregatedCellUsageStatus, + catalogueItemIdSet, + onChangeAggregatedCellUsageStatus, + ]); + console.log(aggregatedCellUsageStatus); const columns = React.useMemo[]>(() => { return [ { header: 'Catalogue Item', accessorFn: (row) => row.catalogueItem?.name, id: 'catalogueItem.name', - Cell: ({ renderedCellValue, row }) => ( - - {renderedCellValue} - - ), + Cell: + type === 'normal' + ? ({ renderedCellValue, row }) => ( + + {renderedCellValue} + + ) + : undefined, size: 250, + GroupedCell: ({ row, table }) => { + const { grouping } = table.getState(); + const error = usageStatuesErrors + ? usageStatuesErrors.filter( + (status) => + status.catalogue_item_id === row.original.catalogueItem?.id && + status.error === true + ).length !== 0 + : false; + console.log(error); + + return ( + + {error && } + {type === 'normal' ? ( + + {row.getValue(grouping[grouping.length - 1])} + + ) : ( + + {row.getValue(grouping[grouping.length - 1])} + + )}{' '} + + {`(${row.subRows?.length})`} + + + ); + }, }, { header: 'Serial Number', accessorFn: (row) => row.item.serial_number ?? 'No serial number', id: 'item.serial_number', size: 250, - Cell: ({ row }) => ( - - {row.original.item.serial_number ?? 'No serial number'} - - ), + Cell: + type === 'normal' + ? ({ row }) => ( + + {row.original.item.serial_number ?? 'No serial number'} + + ) + : undefined, enableGrouping: false, }, { @@ -209,9 +345,201 @@ export function SystemItemsTable(props: SystemItemsTableProps) { id: 'item.usage_status', size: 200, filterVariant: 'select', + AggregatedCell: + type === 'usageStatus' + ? ({ row }) => { + return ( + + + {`Usage statues`} + + + + ); + } + : undefined, + Cell: + type === 'usageStatus' + ? ({ row }) => { + const error = usageStatuesErrors?.find( + (status) => status.item_id === row.original.item.id + )?.error; + return ( + + status.item_id === row.original.item.id + )?.error + } + > + Usage status + + + {error && ( + + Please select a usage status + + )} + + ); + } + : undefined, }, ]; - }, []); + }, [ + aggregatedCellUsageStatus, + onChangeAggregatedCellUsageStatus, + onChangeUsageStatues, + onChangeUsageStatuesErrors, + type, + usageStatues, + usageStatuesErrors, + ]); const [columnFilters, setColumnFilters] = React.useState([]); @@ -219,20 +547,27 @@ export function SystemItemsTable(props: SystemItemsTableProps) { const noResultsText = 'No items found'; const table = useMaterialReactTable({ // Data - columns: columns, + columns: + type === 'normal' + ? columns + : [ + { ...columns[0], size: 200 }, + { ...columns[1], size: 200 }, + { ...columns[6], size: 200 }, + ], data: tableRows, // Features - enableColumnOrdering: true, + enableColumnOrdering: type === 'normal' ? true : false, enableFacetedValues: true, - enableColumnResizing: true, + enableColumnResizing: type === 'normal' ? true : false, enableStickyHeader: true, enableDensityToggle: false, - enableHiding: true, + enableHiding: type === 'normal' ? true : false, enableTopToolbar: true, enableRowVirtualization: false, - enableFullScreenToggle: true, + enableFullScreenToggle: type === 'normal' ? true : false, enableColumnVirtualization: false, - enableRowSelection: true, + enableRowSelection: type === 'normal' ? true : false, enableGrouping: true, enablePagination: true, // Other settings @@ -316,15 +651,18 @@ export function SystemItemsTable(props: SystemItemsTableProps) { > Clear Filters - + {system && type === 'normal' && ( + + )} ), renderDetailPanel: ({ row }) => - row.original.catalogueItem !== undefined ? ( + type === 'usageStatus' ? undefined : row.original.catalogueItem !== + undefined ? ( Date: Tue, 2 Apr 2024 08:16:58 +0000 Subject: [PATCH 02/29] clear aggregatedCell status if a nested status is changed --- src/systems/systemItemsTable.component.tsx | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/systems/systemItemsTable.component.tsx b/src/systems/systemItemsTable.component.tsx index 4553506e9..334251f1f 100644 --- a/src/systems/systemItemsTable.component.tsx +++ b/src/systems/systemItemsTable.component.tsx @@ -210,7 +210,7 @@ export function SystemItemsTable(props: SystemItemsTableProps) { catalogueItemIdSet, onChangeAggregatedCellUsageStatus, ]); - console.log(aggregatedCellUsageStatus); + const columns = React.useMemo[]>(() => { return [ { @@ -241,7 +241,6 @@ export function SystemItemsTable(props: SystemItemsTableProps) { status.error === true ).length !== 0 : false; - console.log(error); return ( @@ -260,7 +259,8 @@ export function SystemItemsTable(props: SystemItemsTableProps) { {row.getValue(grouping[grouping.length - 1])} - )}{' '} + )} + {`(${row.subRows?.length})`} @@ -511,6 +511,21 @@ export function SystemItemsTable(props: SystemItemsTableProps) { } ); } + + if (onChangeAggregatedCellUsageStatus) { + onChangeAggregatedCellUsageStatus((prev) => { + const itemIndex = prev.findIndex( + (status: Omit) => + status.catalogue_item_id === + row.original.catalogueItem?.id + ); + const updatedUsageStatues = [...prev]; + + updatedUsageStatues[itemIndex].usageStatus = ''; + + return updatedUsageStatues; + }); + } }} error={error} label="Usage status" From 7c1743aa4ee5829922dc1e2618420cc4ad55e60f Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Tue, 2 Apr 2024 08:26:07 +0000 Subject: [PATCH 03/29] remove useRef --- src/systems/systemItemsTable.component.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/systems/systemItemsTable.component.tsx b/src/systems/systemItemsTable.component.tsx index 334251f1f..0897a0a4c 100644 --- a/src/systems/systemItemsTable.component.tsx +++ b/src/systems/systemItemsTable.component.tsx @@ -189,14 +189,11 @@ export function SystemItemsTable(props: SystemItemsTableProps) { return status || ''; }; - const isFirstRun = React.useRef(true); React.useEffect(() => { if ( - isFirstRun.current && onChangeAggregatedCellUsageStatus && aggregatedCellUsageStatus?.length === 0 ) { - isFirstRun.current = false; // Set isFirstRun to false after the first run const initialUsageStatues: Omit[] = Array.from(catalogueItemIdSet).map((catalogue_item_id) => ({ catalogue_item_id: catalogue_item_id, From 2488f8d4de129b0b28cf76d23f158e5df7ecddbd Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Tue, 2 Apr 2024 08:47:48 +0000 Subject: [PATCH 04/29] update the api hook to include usage status --- src/api/items.tsx | 3 +++ src/app.types.tsx | 5 +++++ src/systems/systemItemsDialog.component.tsx | 20 +++++++++++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/api/items.tsx b/src/api/items.tsx index 4da46deb0..8591b20a6 100644 --- a/src/api/items.tsx +++ b/src/api/items.tsx @@ -159,6 +159,9 @@ export const useMoveItemsToSystem = (): UseMutationResult< return editItem({ id: item.id, system_id: moveItemsToSystem.targetSystem?.id || '', + usage_status: moveItemsToSystem.usageStatues.find( + (status) => status.item_id === item.id + )?.usage_status, }) .then((result: Item) => { const targetSystemName = diff --git a/src/app.types.tsx b/src/app.types.tsx index 59adb395c..12e8c9ffa 100644 --- a/src/app.types.tsx +++ b/src/app.types.tsx @@ -268,7 +268,12 @@ export interface EditItem extends Partial { id: string; } +export interface MoveItemsToSystemUsageStatus { + item_id: string; + usage_status: UsageStatusType; +} export interface MoveItemsToSystem { + usageStatues: MoveItemsToSystemUsageStatus[]; selectedItems: Item[]; targetSystem: System; } diff --git a/src/systems/systemItemsDialog.component.tsx b/src/systems/systemItemsDialog.component.tsx index 960ea2899..31573378a 100644 --- a/src/systems/systemItemsDialog.component.tsx +++ b/src/systems/systemItemsDialog.component.tsx @@ -15,7 +15,11 @@ import { MRT_RowSelectionState } from 'material-react-table'; import React from 'react'; import { useMoveItemsToSystem } from '../api/items'; import { useSystem, useSystems, useSystemsBreadcrumbs } from '../api/systems'; -import { Item, UsageStatusType } from '../app.types'; +import { + Item, + MoveItemsToSystemUsageStatus, + UsageStatusType, +} from '../app.types'; import handleTransferState from '../handleTransferState'; import Breadcrumbs from '../view/breadcrumbs.component'; import { SystemsTableView } from './systemsTableView.component'; @@ -40,6 +44,17 @@ export interface UsageStatuesErrorType error: boolean; } +const moveItemsToSystemUsageStatues = ( + list: UsageStatuesType[] +): MoveItemsToSystemUsageStatus[] => { + return list + .filter((item) => item.usageStatus !== '') // Exclude items with empty usageStatus + .map((item) => ({ + item_id: item.item_id, + usage_status: item.usageStatus as UsageStatusType, // Safer now, but still a type assertion. + })); +}; + const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { const { open, onClose, selectedItems, onChangeSelectedItems } = props; @@ -149,10 +164,12 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { hasSystemErrors && setPlaceIntoSystemError(hasSystemErrors); return; } + // Ensure finished loading and not moving to root // (where we don't need to load anything as the name is known) if (!targetSystemLoading && targetSystem !== undefined) { moveItemsToSystem({ + usageStatues: moveItemsToSystemUsageStatues(usageStatues), selectedItems: selectedItems, // Only reason for targetSystem to be undefined here is if not loading at all // which happens when at root @@ -171,6 +188,7 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { selectedItems, targetSystem, targetSystemLoading, + usageStatues, validateUsageStatus, ]); From 4641d429bea846fd0b06374a93aaa902f0bbdae0 Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Tue, 2 Apr 2024 08:52:49 +0000 Subject: [PATCH 05/29] clear usestates onclose --- src/systems/systemItemsDialog.component.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/systems/systemItemsDialog.component.tsx b/src/systems/systemItemsDialog.component.tsx index 31573378a..eda22823e 100644 --- a/src/systems/systemItemsDialog.component.tsx +++ b/src/systems/systemItemsDialog.component.tsx @@ -149,6 +149,10 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { }, [usageStatues]); const handleClose = React.useCallback(() => { + setAggregatedCellUsageStatus([]); + setUsageStatues([]); + setUsageStatuesErrors([]); + setPlaceIntoSystemError(false); setActiveStep(0); onClose(); }, [onClose]); @@ -275,7 +279,7 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { }; return ( - + From 6fbd13e7d42c7b667e94ef4e8fa7e9e0bb121491 Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Tue, 2 Apr 2024 09:10:10 +0000 Subject: [PATCH 06/29] move to item unit test --- src/api/items.test.tsx | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/api/items.test.tsx b/src/api/items.test.tsx index 82215a2cd..4a908c0cf 100644 --- a/src/api/items.test.tsx +++ b/src/api/items.test.tsx @@ -18,6 +18,7 @@ import { EditItem, Item, MoveItemsToSystem, + MoveItemsToSystemUsageStatus, System, } from '../app.types'; import SystemsJSON from '../mocks/Systems.json'; @@ -206,6 +207,11 @@ describe('catalogue items api functions', () => { getItemById('G463gOIA'), ]; + const mockUsageStatues: MoveItemsToSystemUsageStatus[] = [ + { item_id: 'KvT2Ox7n', usage_status: 0 }, + { item_id: 'G463gOIA', usage_status: 0 }, + ]; + let moveItemsToSystem: MoveItemsToSystem; // Use patch spy for testing since response is not actual data in this case @@ -215,6 +221,7 @@ describe('catalogue items api functions', () => { beforeEach(() => { moveItemsToSystem = { // Prevent test interference if modifying the selected items + usageStatues: JSON.parse(JSON.stringify(mockUsageStatues)), selectedItems: JSON.parse(JSON.stringify(mockItems)), targetSystem: SystemsJSON[0] as System, }; @@ -241,6 +248,9 @@ describe('catalogue items api functions', () => { moveItemsToSystem.selectedItems.map((item) => expect(axiosPatchSpy).toHaveBeenCalledWith(`/v1/items/${item.id}`, { system_id: moveItemsToSystem.targetSystem.id, + usage_status: moveItemsToSystem.usageStatues.find( + (status) => status.item_id === item.id + )?.usage_status, }) ); expect(result.current.data).toEqual( @@ -258,6 +268,10 @@ describe('catalogue items api functions', () => { name: 'New system name', id: 'new_system_id', }; + moveItemsToSystem.usageStatues = [ + ...moveItemsToSystem.usageStatues, + { item_id: 'Error 409', usage_status: 2 }, + ]; // Fail just the 1st system moveItemsToSystem.selectedItems[0].id = 'Error 409'; @@ -276,6 +290,9 @@ describe('catalogue items api functions', () => { moveItemsToSystem.selectedItems.map((item) => expect(axiosPatchSpy).toHaveBeenCalledWith(`/v1/items/${item.id}`, { system_id: 'new_system_id', + usage_status: moveItemsToSystem.usageStatues.find( + (status) => status.item_id === item.id + )?.usage_status, }) ); expect(result.current.data).toEqual( From 92d567b78328a638d154047b0b7b23965ca48133 Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Wed, 3 Apr 2024 08:45:25 +0000 Subject: [PATCH 07/29] unit tests for systemitemstable --- .../systemItemsTable.component.test.tsx.snap | 1167 ++++++++++++++++- .../systemItemsTable.component.test.tsx | 564 ++++++-- src/systems/systemItemsTable.component.tsx | 240 ++-- 3 files changed, 1698 insertions(+), 273 deletions(-) diff --git a/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap b/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap index f135940bb..812c169f5 100644 --- a/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap +++ b/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap @@ -1,6 +1,6 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`SystemItemsTable > renders correctly 1`] = ` +exports[`SystemItemsTable > SystemItemsTable (normal) > renders correctly 1`] = `
renders correctly 1`] = ` class="MuiTableCell-root MuiTableCell-body MuiTableCell-alignLeft MuiTableCell-sizeMedium css-1qa3y6f-MuiTableCell-root" data-index="0" > - - Turbomolecular Pumps 42 - - (1) + + Turbomolecular Pumps 42 + + (1) +
renders correctly 1`] = ` class="MuiTableCell-root MuiTableCell-body MuiTableCell-alignLeft MuiTableCell-sizeMedium css-1qa3y6f-MuiTableCell-root" data-index="0" > - - Turbomolecular Pumps 43 - - (2) + + Turbomolecular Pumps 43 + + (2) + renders correctly 1`] = `
`; + +exports[`SystemItemsTable > SystemItemsTable (usageStatus) > renders correctly 1`] = ` + +
+
+ +
+
+ +
+
+
+
+
+
+
+ + +
+ + + +
+ +
+
+
+
+
+
+ +
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + +
+
+ +
+ +
+
+
+
+
+
+
+ +
+ + + +
+
+
+
+
+
+
+
+ +
+
+
+
+ +
+ +
+
+
+
+
+
+
+ +
+ + + +
+
+
+
+
+
+
+
+ +
+ + +
+
+
+
+
+
+
+ + + +
+ + + +
+
+
+
+
+
+
+
+
+ Cameras 1 +
+ (2) +
+
+ + + + + +
+ +
+ + + + +
+
+
+
+
+ Cameras 6 +
+ (2) +
+
+ + + + + +
+ +
+ + + + +
+
+
+
+
+
+
+
+ +
+
+
+
+ +
+
+
+ +
+ + + +
+
+ +
+
+
+
+
+
+`; diff --git a/src/systems/systemItemsTable.component.test.tsx b/src/systems/systemItemsTable.component.test.tsx index 0ab46e731..6d2a979ac 100644 --- a/src/systems/systemItemsTable.component.test.tsx +++ b/src/systems/systemItemsTable.component.test.tsx @@ -1,7 +1,8 @@ -import { screen, waitFor } from '@testing-library/react'; +import { screen, waitFor, within } from '@testing-library/react'; import userEvent, { UserEvent } from '@testing-library/user-event'; -import { System } from '../app.types'; +import { Item, System } from '../app.types'; import SystemsJSON from '../mocks/Systems.json'; +import ItemJSON from '../mocks/Items.json'; import { renderComponentWithRouterProvider } from '../testUtils'; import { SystemItemsTable, @@ -39,166 +40,449 @@ describe('SystemItemsTable', () => { vi.clearAllMocks(); }); - it('renders correctly', async () => { - const view = createView(); - - // Name (obtained from catalouge category item) - await waitFor( - () => { - expect( - screen.getByRole('cell', { - name: `Turbomolecular Pumps 42 (1)`, - }) - ).toBeInTheDocument(); - }, - { timeout: 4000 } - ); - - // Ensure no loading bars visible - await waitFor(() => - expect(screen.queryByRole('progressbar')).not.toBeInTheDocument() - ); - - // Expand a group so all columns are rendered to improve test coverage - // (expanding all causes an infinite loop due to an issue with details panels) - await user.click(screen.getAllByRole('button', { name: 'Expand' })[0]); - //also unhide created column - await user.click( - await screen.findByRole('button', { name: 'Show/Hide columns' }) - ); - await user.click(screen.getByText('Created')); - - // Rest in a snapshot - expect(view.asFragment()).toMatchSnapshot(); - }); + describe('SystemItemsTable (normal)', () => { + afterEach(() => { + vi.clearAllMocks(); + }); - it('renders correctly when there are no items to display', async () => { - props.system = { ...props.system, id: 'invalid' }; + it('renders correctly', async () => { + const view = createView(); + + // Name (obtained from catalouge category item) + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Turbomolecular Pumps 42 (1)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); + + // Ensure no loading bars visible + await waitFor(() => + expect(screen.queryByRole('progressbar')).not.toBeInTheDocument() + ); + + // Expand a group so all columns are rendered to improve test coverage + // (expanding all causes an infinite loop due to an issue with details panels) + await user.click(screen.getAllByRole('button', { name: 'Expand' })[0]); + //also unhide created column + await user.click( + await screen.findByRole('button', { name: 'Show/Hide columns' }) + ); + await user.click(screen.getByText('Created')); + + // Rest in a snapshot + expect(view.asFragment()).toMatchSnapshot(); + }); - createView(); + it('renders correctly when there are no items to display', async () => { + props.system = { ...props.system, id: 'invalid' }; - expect(screen.getByText('No items found')).toBeInTheDocument(); - }); + createView(); - it('can set a table filter and clear them again', async () => { - createView(); - - // Name (obtained from catalouge category item) - await waitFor( - () => { - expect( - screen.getByRole('cell', { - name: `Turbomolecular Pumps 42 (1)`, - }) - ).toBeInTheDocument(); - }, - { timeout: 4000 } - ); - - const clearFiltersButton = screen.getByRole('button', { - name: 'Clear Filters', + expect(screen.getByText('No items found')).toBeInTheDocument(); }); - expect(clearFiltersButton).toBeDisabled(); - - await user.type(screen.getByLabelText('Filter by Catalogue Item'), '43'); - - await waitFor( - () => { - expect( - screen.queryByRole('cell', { - name: `Turbomolecular Pumps 42 (1)`, - }) - ).not.toBeInTheDocument(); - }, - { timeout: 4000 } - ); - - await user.click(clearFiltersButton); - - await waitFor( - () => { - expect( - screen.getByRole('cell', { - name: `Turbomolecular Pumps 42 (1)`, - }) - ).toBeInTheDocument(); - }, - { timeout: 4000 } - ); - }); - it('can select and deselect items', async () => { - createView(); - - // Name (obtained from catalouge category item) - await waitFor( - () => { - expect( - screen.getByRole('cell', { - name: `Turbomolecular Pumps 42 (1)`, - }) - ).toBeInTheDocument(); - }, - { timeout: 4000 } - ); - - expect(screen.getByRole('button', { name: 'Move to' })).toBeDisabled(); - - const checkboxes = screen.getAllByRole('checkbox', { - name: 'Toggle select row', + it('can set a table filter and clear them again', async () => { + createView(); + + // Name (obtained from catalouge category item) + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Turbomolecular Pumps 42 (1)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); + + const clearFiltersButton = screen.getByRole('button', { + name: 'Clear Filters', + }); + expect(clearFiltersButton).toBeDisabled(); + + await user.type(screen.getByLabelText('Filter by Catalogue Item'), '43'); + + await waitFor( + () => { + expect( + screen.queryByRole('cell', { + name: `Turbomolecular Pumps 42 (1)`, + }) + ).not.toBeInTheDocument(); + }, + { timeout: 4000 } + ); + + await user.click(clearFiltersButton); + + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Turbomolecular Pumps 42 (1)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); }); - await user.click(checkboxes[0]); - await user.click(checkboxes[1]); + it('can select and deselect items', async () => { + createView(); + + // Name (obtained from catalouge category item) + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Turbomolecular Pumps 42 (1)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); + + expect(screen.getByRole('button', { name: 'Move to' })).toBeDisabled(); + + const checkboxes = screen.getAllByRole('checkbox', { + name: 'Toggle select row', + }); + + await user.click(checkboxes[0]); + await user.click(checkboxes[1]); + + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Move to' })).toBeEnabled(); + }); + + await user.click(checkboxes[0]); + await user.click(checkboxes[1]); - await waitFor(() => { - expect(screen.getByRole('button', { name: 'Move to' })).toBeEnabled(); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Move to' })).toBeDisabled(); + }); }); - await user.click(checkboxes[0]); - await user.click(checkboxes[1]); + it('can open and close the move items dialog', async () => { + createView(); + + // Name (obtained from catalouge category item) + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Turbomolecular Pumps 42 (1)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); - await waitFor(() => { expect(screen.getByRole('button', { name: 'Move to' })).toBeDisabled(); + + const checkboxes = screen.getAllByRole('checkbox', { + name: 'Toggle select row', + }); + + await user.click(checkboxes[0]); + + const moveToButton = screen.getByRole('button', { name: 'Move to' }); + await waitFor(() => { + expect(moveToButton).toBeEnabled(); + }); + + await user.click(moveToButton); + + expect(screen.getByRole('dialog')).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + await waitFor(() => { + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); }); }); - it('can open and close the move items dialog', async () => { - createView(); - - // Name (obtained from catalouge category item) - await waitFor( - () => { - expect( - screen.getByRole('cell', { - name: `Turbomolecular Pumps 42 (1)`, - }) - ).toBeInTheDocument(); - }, - { timeout: 4000 } - ); - - expect(screen.getByRole('button', { name: 'Move to' })).toBeDisabled(); - - const checkboxes = screen.getAllByRole('checkbox', { - name: 'Toggle select row', + describe('SystemItemsTable (usageStatus)', () => { + const onChangeUsageStatues = vi.fn(); + const onChangeUsageStatuesErrors = vi.fn(); + const onChangeAggregatedCellUsageStatus = vi.fn(); + const moveToSelectedItems: Item[] = [ + ItemJSON[0], + ItemJSON[1], + ItemJSON[22], + ItemJSON[23], + ]; + + beforeEach(() => { + props = { + system: undefined, + type: 'usageStatus', + onChangeAggregatedCellUsageStatus, + onChangeUsageStatues, + onChangeUsageStatuesErrors, + aggregatedCellUsageStatus: [ + { catalogue_item_id: '1', usageStatus: '' }, + { catalogue_item_id: '25', usageStatus: '' }, + ], + usageStatues: [ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: '' }, + { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: '' }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: '' }, + { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: '' }, + ], + usageStatuesErrors: [ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', error: false }, + { item_id: 'G463gOIA', catalogue_item_id: '1', error: false }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', error: false }, + { item_id: 'QQen23yW', catalogue_item_id: '25', error: false }, + ], + moveToSelectedItems: moveToSelectedItems, + }; }); - await user.click(checkboxes[0]); - - const moveToButton = screen.getByRole('button', { name: 'Move to' }); - await waitFor(() => { - expect(moveToButton).toBeEnabled(); + const selectUsageStatus = async (values: { + index: number; + usageStatus: string; + }) => { + await user.click(screen.getAllByRole('combobox')[values.index]); + + const dropdown = await screen.findByRole('listbox'); + + await user.click( + within(dropdown).getByRole('option', { name: values.usageStatus }) + ); + }; + + const modifyUsageStatus = async (values: { + cameras1?: string; + cameras6?: string; + cameras1Item1?: string; + cameras1Item2?: string; + cameras6Item1?: string; + cameras6Item2?: string; + }) => { + if ( + values.cameras1Item1 || + values.cameras1Item2 || + values.cameras6Item1 || + values.cameras6Item2 + ) { + await user.click(screen.getAllByLabelText('Expand all')[1]); + values.cameras1Item1 && + (await selectUsageStatus({ + index: 2, + usageStatus: values.cameras1Item1, + })); + + values.cameras1Item2 && + (await selectUsageStatus({ + index: 3, + usageStatus: values.cameras1Item2, + })); + + values.cameras6Item1 && + (await selectUsageStatus({ + index: 5, + usageStatus: values.cameras6Item1, + })); + + values.cameras6Item2 && + (await selectUsageStatus({ + index: 6, + usageStatus: values.cameras6Item2, + })); + + await user.click(await screen.findByLabelText('Collapse all')); + } + values.cameras1 && + (await selectUsageStatus({ index: 1, usageStatus: values.cameras1 })); + + values.cameras6 && + (await selectUsageStatus({ index: 2, usageStatus: values.cameras6 })); + }; + + it('renders correctly', async () => { + const view = createView(); + + // Name (obtained from catalogue category item) + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Cameras 1 (2)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); + + // Ensure no loading bars visible + await waitFor(() => + expect(screen.queryByRole('progressbar')).not.toBeInTheDocument() + ); + + // Rest in a snapshot + expect(view.asFragment()).toMatchSnapshot(); }); - await user.click(moveToButton); - - expect(screen.getByRole('dialog')).toBeInTheDocument(); + it('sets the usages status using the aggregate cell (sets all items of a catalogue item to the selected usage status)', async () => { + createView(); + + // Name (obtained from catalogue category item) + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Cameras 1 (2)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); + + // Ensure no loading bars visible + await waitFor(() => + expect(screen.queryByRole('progressbar')).not.toBeInTheDocument() + ); + + await modifyUsageStatus({ cameras1: 'Used' }); + + // Change usages status for cameras 1 items + expect(onChangeUsageStatues).toHaveBeenCalledWith([ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: '' }, + { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: '' }, + ]); + expect(onChangeAggregatedCellUsageStatus).toHaveBeenCalledWith([ + { catalogue_item_id: '1', usageStatus: 2 }, + { catalogue_item_id: '25', usageStatus: '' }, + ]); + + await modifyUsageStatus({ cameras6: 'Used' }); + + // Change usages status for cameras 6 items + expect(onChangeUsageStatues).toHaveBeenCalledWith([ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: 2 }, + { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: 2 }, + ]); + expect(onChangeAggregatedCellUsageStatus).toHaveBeenCalledWith([ + { catalogue_item_id: '1', usageStatus: 2 }, + { catalogue_item_id: '25', usageStatus: 2 }, + ]); + }); - await user.click(screen.getByRole('button', { name: 'Cancel' })); + it('sets the usages status one by one', async () => { + createView(); + + // Name (obtained from catalogue category item) + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Cameras 1 (2)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); + + // Ensure no loading bars visible + await waitFor(() => + expect(screen.queryByRole('progressbar')).not.toBeInTheDocument() + ); + + await modifyUsageStatus({ cameras1Item1: 'Used' }); + + expect(onChangeUsageStatues).toHaveBeenCalledWith([ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: '' }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: '' }, + { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: '' }, + ]); + + await modifyUsageStatus({ cameras1Item2: 'Used' }); + + expect(onChangeUsageStatues).toHaveBeenCalledWith([ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: '' }, + { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: '' }, + ]); + + await modifyUsageStatus({ cameras6Item1: 'Used' }); + + expect(onChangeUsageStatues).toHaveBeenCalledWith([ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: 2 }, + { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: '' }, + ]); + + await modifyUsageStatus({ cameras6Item2: 'Used' }); + + expect(onChangeUsageStatues).toHaveBeenCalledWith([ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: 2 }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: 2 }, + { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: 2 }, + ]); + }); - await waitFor(() => { - expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + it('displays errors message correctly', async () => { + props.usageStatuesErrors = [ + { item_id: 'KvT2Ox7n', catalogue_item_id: '1', error: true }, + { item_id: 'G463gOIA', catalogue_item_id: '1', error: true }, + { item_id: '7Lrj9KVu', catalogue_item_id: '25', error: true }, + { item_id: 'QQen23yW', catalogue_item_id: '25', error: true }, + ]; + + createView(); + + // Name (obtained from catalogue category item) + await waitFor( + () => { + expect( + screen.getByRole('cell', { + name: `Cameras 1 (2)`, + }) + ).toBeInTheDocument(); + }, + { timeout: 4000 } + ); + + // Ensure no loading bars visible + await waitFor(() => + expect(screen.queryByRole('progressbar')).not.toBeInTheDocument() + ); + + const errorIcon = screen.getAllByTestId('ErrorIcon'); + + expect(errorIcon.length).toEqual(2); + + await user.click(screen.getAllByLabelText('Expand all')[1]); + const helperTexts = screen.getAllByText('Please select a usage status'); + expect(helperTexts.length).toEqual(4); + + // await modifyUsageStatus({ cameras1: 'Used' }); + // await modifyUsageStatus({ cameras6Item1: 'Used', cameras6Item2: 'Used' }); + + // await waitFor(() => { + // expect(screen.queryByTestId('ErrorIcon')).not.toBeInTheDocument(); + // }); + + // expect( + // screen.queryByTestId('Please select a usage status') + // ).not.toBeInTheDocument(); }); }); }); diff --git a/src/systems/systemItemsTable.component.tsx b/src/systems/systemItemsTable.component.tsx index 0897a0a4c..f578dae9a 100644 --- a/src/systems/systemItemsTable.component.tsx +++ b/src/systems/systemItemsTable.component.tsx @@ -73,17 +73,15 @@ export interface SystemItemsTableProps { type: 'normal' | 'usageStatus'; moveToSelectedItems?: Item[]; usageStatues?: UsageStatuesType[]; - onChangeUsageStatues?: React.Dispatch< - React.SetStateAction - >; + onChangeUsageStatues?: (usageStatues: UsageStatuesType[]) => void; usageStatuesErrors?: UsageStatuesErrorType[]; - onChangeUsageStatuesErrors?: React.Dispatch< - React.SetStateAction - >; + onChangeUsageStatuesErrors?: ( + usageStatuesErrors: UsageStatuesErrorType[] + ) => void; aggregatedCellUsageStatus?: Omit[]; - onChangeAggregatedCellUsageStatus?: React.Dispatch< - React.SetStateAction[]> - >; + onChangeAggregatedCellUsageStatus?: ( + aggregatedCellUsageStatus?: Omit[] + ) => void; } export function SystemItemsTable(props: SystemItemsTableProps) { @@ -104,7 +102,6 @@ export function SystemItemsTable(props: SystemItemsTableProps) { const [rowSelection, setRowSelection] = React.useState( {} ); - // Data const { data: itemsData, isLoading: isLoadingItems } = useItems( system?.id, @@ -132,7 +129,7 @@ export function SystemItemsTable(props: SystemItemsTableProps) { ), [itemsData, moveToSelectedItems, type] ); - let isLoading = isLoadingItems; + let isLoading = type === 'normal' ? isLoadingItems : false; const catalogueItemList: (CatalogueItem | undefined)[] = useCatalogueItemIds( Array.from(catalogueItemIdSet.values()) @@ -158,7 +155,7 @@ export function SystemItemsTable(props: SystemItemsTableProps) { }) as TableRowData ) ); - } else if (moveToSelectedItems) { + } else if (!isLoading && moveToSelectedItems) { setTableRows( moveToSelectedItems.map( (itemData) => @@ -192,7 +189,8 @@ export function SystemItemsTable(props: SystemItemsTableProps) { React.useEffect(() => { if ( onChangeAggregatedCellUsageStatus && - aggregatedCellUsageStatus?.length === 0 + aggregatedCellUsageStatus && + aggregatedCellUsageStatus.length === 0 ) { const initialUsageStatues: Omit[] = Array.from(catalogueItemIdSet).map((catalogue_item_id) => ({ @@ -240,7 +238,13 @@ export function SystemItemsTable(props: SystemItemsTableProps) { : false; return ( - + {error && } {type === 'normal' ? ( {row.getValue(grouping[grouping.length - 1])} ) : ( - + {row.getValue(grouping[grouping.length - 1])} )} - - {`(${row.subRows?.length})`} - + {`(${row.subRows?.length})`} ); }, @@ -348,12 +350,12 @@ export function SystemItemsTable(props: SystemItemsTableProps) { return ( - {`Usage statues`} + Usage statues New In Use @@ -447,7 +456,7 @@ export function SystemItemsTable(props: SystemItemsTableProps) { Cell: type === 'usageStatus' ? ({ row }) => { - const error = usageStatuesErrors?.find( + const error = usageStatusesErrors?.find( (status) => status.item_id === row.original.item.id )?.error; return ( @@ -456,7 +465,7 @@ export function SystemItemsTable(props: SystemItemsTableProps) { required={true} id={`usage-status-${row.original.item.id}`} error={ - usageStatuesErrors?.find( + usageStatusesErrors?.find( (status) => status.item_id === row.original.item.id )?.error } @@ -469,36 +478,39 @@ export function SystemItemsTable(props: SystemItemsTableProps) { size="small" value={ status( - usageStatues?.find( + usageStatuses?.find( (status) => status.item_id === row.original.item.id )?.usageStatus ) ?? '' } onChange={(event) => { - if (onChangeUsageStatues && usageStatues) { - const itemIndex = usageStatues.findIndex( + if (onChangeUsageStatuses && usageStatuses) { + const itemIndex = usageStatuses.findIndex( (status: UsageStatusesType) => status.item_id === row.original.item.id ); - const updatedUsageStatues = [...usageStatues]; + const updatedUsageStatuses = [...usageStatuses]; - updatedUsageStatues[itemIndex].usageStatus = + updatedUsageStatuses[itemIndex].usageStatus = UsageStatusType[ event.target.value as keyof typeof UsageStatusType ]; - onChangeUsageStatues(updatedUsageStatues); + onChangeUsageStatuses(updatedUsageStatuses); } - if (onChangeUsageStatuesErrors && usageStatuesErrors) { - const itemIndex = usageStatuesErrors.findIndex( - (status: UsageStatuesErrorType) => + if ( + onChangeUsageStatusesErrors && + usageStatusesErrors + ) { + const itemIndex = usageStatusesErrors.findIndex( + (status: UsageStatusesErrorType) => status.item_id === row.original.item.id ); - const updatedUsageStatues = [...usageStatuesErrors]; + const updatedUsageStatuses = [...usageStatusesErrors]; - updatedUsageStatues[itemIndex].error = false; + updatedUsageStatuses[itemIndex].error = false; - onChangeUsageStatuesErrors(updatedUsageStatues); + onChangeUsageStatusesErrors(updatedUsageStatuses); } if ( @@ -510,14 +522,14 @@ export function SystemItemsTable(props: SystemItemsTableProps) { status.catalogue_item_id === row.original.catalogueItem?.id ); - const updatedUsageStatues = [ + const updatedUsageStatuses = [ ...aggregatedCellUsageStatus, ]; - updatedUsageStatues[itemIndex].usageStatus = ''; + updatedUsageStatuses[itemIndex].usageStatus = ''; onChangeAggregatedCellUsageStatus( - updatedUsageStatues + updatedUsageStatuses ); } }} @@ -543,11 +555,11 @@ export function SystemItemsTable(props: SystemItemsTableProps) { }, [ aggregatedCellUsageStatus, onChangeAggregatedCellUsageStatus, - onChangeUsageStatues, - onChangeUsageStatuesErrors, + onChangeUsageStatuses, + onChangeUsageStatusesErrors, type, - usageStatues, - usageStatuesErrors, + usageStatuses, + usageStatusesErrors, ]); const [columnFilters, setColumnFilters] = From 6a44a65aa39441b9dbfdea096cbc4bab0dcb134d Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Mon, 8 Apr 2024 08:12:56 +0000 Subject: [PATCH 24/29] update dependency array --- src/systems/systemItemsTable.component.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/systems/systemItemsTable.component.tsx b/src/systems/systemItemsTable.component.tsx index 3bf856f5c..b30ee6436 100644 --- a/src/systems/systemItemsTable.component.tsx +++ b/src/systems/systemItemsTable.component.tsx @@ -173,9 +173,8 @@ export function SystemItemsTable(props: SystemItemsTableProps) { // to the reference changing so instead am relying on isLoading to have changed to // false and then back to true again for any refetches that occurr - only // alternative I can see right now requires backend changes - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isLoading, itemsData]); + }, [isLoading, itemsData, moveToSelectedItems]); const status = (usageStatus: UsageStatusType | undefined | '') => { if (typeof usageStatus !== 'number') return ''; From eb15fa83c5c46a1da1abd72c4d2f81716d205df9 Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Mon, 8 Apr 2024 08:24:18 +0000 Subject: [PATCH 25/29] remove useEffect --- src/systems/systemItemsDialog.component.tsx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/systems/systemItemsDialog.component.tsx b/src/systems/systemItemsDialog.component.tsx index b36dd0a44..447c10593 100644 --- a/src/systems/systemItemsDialog.component.tsx +++ b/src/systems/systemItemsDialog.component.tsx @@ -100,9 +100,10 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { setParentSystemId(props.parentSystemId); }, [props.parentSystemId]); - React.useEffect(() => { + const changeParentSystemId = (newParentSystemId: string | null) => { + setParentSystemId(newParentSystemId); setPlaceIntoSystemError(false); - }, [parentSystemId]); + }; const { data: parentSystemBreadcrumbs } = useSystemsBreadcrumbs(parentSystemId); @@ -242,9 +243,9 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { { - setParentSystemId(null); + changeParentSystemId(null); }} navigateHomeAriaLabel={'navigate to systems home'} /> @@ -253,7 +254,7 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { Date: Mon, 8 Apr 2024 09:58:04 +0100 Subject: [PATCH 26/29] fix flaky e2e tests. the check has been moved to the Finish button --- src/systems/systemItemsDialog.component.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/systems/systemItemsDialog.component.tsx b/src/systems/systemItemsDialog.component.tsx index 447c10593..0d80736f5 100644 --- a/src/systems/systemItemsDialog.component.tsx +++ b/src/systems/systemItemsDialog.component.tsx @@ -161,9 +161,7 @@ const SystemItemsDialog = React.memo((props: SystemItemsDialogProps) => { const hasSystemErrors = // Disable when not moving anywhere different // or when attempting to move to root i.e. no system - props.parentSystemId === parentSystemId || - parentSystemId === null || - !(!targetSystemLoading && targetSystem !== undefined); + props.parentSystemId === parentSystemId || parentSystemId === null; const handleMoveTo = React.useCallback(() => { const hasUsageStatusErrors = validateUsageStatus(); From 52d8c75c3d97be3b9dfd8744947d19e32aa73bfb Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Tue, 9 Apr 2024 08:14:48 +0000 Subject: [PATCH 27/29] Change notification to use the serial number instead of database id --- src/api/items.test.tsx | 7 ++++--- src/api/items.tsx | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/api/items.test.tsx b/src/api/items.test.tsx index adc1ecdd8..882257b59 100644 --- a/src/api/items.test.tsx +++ b/src/api/items.test.tsx @@ -256,7 +256,7 @@ describe('catalogue items api functions', () => { expect(result.current.data).toEqual( moveItemsToSystem.selectedItems.map((item) => ({ message: `Successfully moved to Giant laser`, - name: item.id, + name: item.serial_number, state: 'success', })) ); @@ -275,6 +275,7 @@ describe('catalogue items api functions', () => { // Fail just the 1st system moveItemsToSystem.selectedItems[0].id = 'Error 409'; + moveItemsToSystem.selectedItems[0].serial_number = null; const { result } = renderHook(() => useMoveItemsToSystem(), { wrapper: hooksWrapperWithProviders(), @@ -301,12 +302,12 @@ describe('catalogue items api functions', () => { index === 0 ? { message: 'The specified system ID does not exist', - name: item.id, + name: item.serial_number ?? 'No serial number', state: 'error', } : { message: 'Successfully moved to New system name', - name: item.id, + name: item.serial_number, state: 'success', } ) diff --git a/src/api/items.tsx b/src/api/items.tsx index c7c529a39..b2a1dc907 100644 --- a/src/api/items.tsx +++ b/src/api/items.tsx @@ -168,7 +168,7 @@ export const useMoveItemsToSystem = (): UseMutationResult< moveItemsToSystem.targetSystem?.name || 'Root'; transferStates.push({ // Not technically a name, but will be displayed as ID: Message - name: item.id, + name: item.serial_number ?? 'No serial number', message: `Successfully moved to ${targetSystemName}`, state: 'success', }); @@ -180,7 +180,7 @@ export const useMoveItemsToSystem = (): UseMutationResult< const response = error.response?.data as ErrorParsing; transferStates.push({ - name: item.id, + name: item.serial_number ?? 'No serial number', message: response.detail, state: 'error', }); From 0c8522e7de1d800f3384a4dae9798fa3ec6ab3ae Mon Sep 17 00:00:00 2001 From: Joshua Kitenge Date: Tue, 9 Apr 2024 12:19:48 +0000 Subject: [PATCH 28/29] address review comments --- .../systemItemsTable.component.test.tsx.snap | 4 +- .../systemItemsTable.component.test.tsx | 96 +++++++++---------- src/systems/systemItemsTable.component.tsx | 4 +- 3 files changed, 50 insertions(+), 54 deletions(-) diff --git a/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap b/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap index e3cb10125..b7c3d92aa 100644 --- a/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap +++ b/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap @@ -234,7 +234,7 @@ exports[`SystemItemsTable > SystemItemsTable (normal) > renders correctly 1`] =
SystemItemsTable (usageStatus) > renders correctly 1
{ values.cameras6Item1 || values.cameras6Item2 ) { - await user.click(screen.getAllByLabelText('Expand all')[1]); - values.cameras1Item1 && - (await selectUsageStatus({ - index: 2, + await user.click(screen.getByTestId('CancelIcon')); + + if (values.cameras1Item1) { + await selectUsageStatus({ + index: 1, usageStatus: values.cameras1Item1, - })); + }); + } - values.cameras1Item2 && - (await selectUsageStatus({ - index: 3, + if (values.cameras1Item2) { + await selectUsageStatus({ + index: 2, usageStatus: values.cameras1Item2, - })); + }); + } - values.cameras6Item1 && - (await selectUsageStatus({ - index: 5, + if (values.cameras6Item1) { + await selectUsageStatus({ + index: 3, usageStatus: values.cameras6Item1, - })); + }); + } - values.cameras6Item2 && - (await selectUsageStatus({ - index: 6, + if (values.cameras6Item2) { + await selectUsageStatus({ + index: 4, usageStatus: values.cameras6Item2, - })); - - await user.click(await screen.findByLabelText('Collapse all')); + }); + } } values.cameras1 && (await selectUsageStatus({ index: 1, usageStatus: values.cameras1 })); @@ -475,34 +478,12 @@ describe('SystemItemsTable', () => { expect(screen.queryByRole('progressbar')).not.toBeInTheDocument() ); - await modifyUsageStatus({ cameras1Item1: 'Used' }); - - expect(onChangeUsageStatuses).toHaveBeenCalledWith([ - { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, - { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: '' }, - { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: '' }, - { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: '' }, - ]); - - await modifyUsageStatus({ cameras1Item2: 'Used' }); - - expect(onChangeUsageStatuses).toHaveBeenCalledWith([ - { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, - { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: 2 }, - { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: '' }, - { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: '' }, - ]); - - await modifyUsageStatus({ cameras6Item1: 'Used' }); - - expect(onChangeUsageStatuses).toHaveBeenCalledWith([ - { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, - { item_id: 'G463gOIA', catalogue_item_id: '1', usageStatus: 2 }, - { item_id: '7Lrj9KVu', catalogue_item_id: '25', usageStatus: 2 }, - { item_id: 'QQen23yW', catalogue_item_id: '25', usageStatus: '' }, - ]); - - await modifyUsageStatus({ cameras6Item2: 'Used' }); + await modifyUsageStatus({ + cameras1Item1: 'Used', + cameras1Item2: 'Used', + cameras6Item1: 'Used', + cameras6Item2: 'Used', + }); expect(onChangeUsageStatuses).toHaveBeenCalledWith([ { item_id: 'KvT2Ox7n', catalogue_item_id: '1', usageStatus: 2 }, @@ -543,9 +524,24 @@ describe('SystemItemsTable', () => { expect(errorIcon.length).toEqual(2); - await user.click(screen.getAllByLabelText('Expand all')[1]); - const helperTexts = screen.getAllByText('Please select a usage status'); - expect(helperTexts.length).toEqual(4); + await user.click(screen.getAllByRole('button', { name: 'Expand' })[0]); + + expect( + screen.getAllByText('Please select a usage status').length + ).toEqual(2); + await user.click(screen.getAllByLabelText('Collapse')[1]); + + await waitFor(() => { + expect( + screen.queryByText('Please select a usage status') + ).not.toBeInTheDocument(); + }); + + await user.click(screen.getAllByRole('button', { name: 'Expand' })[1]); + + expect( + screen.getAllByText('Please select a usage status').length + ).toEqual(2); }); }); }); diff --git a/src/systems/systemItemsTable.component.tsx b/src/systems/systemItemsTable.component.tsx index b30ee6436..c19e42b7e 100644 --- a/src/systems/systemItemsTable.component.tsx +++ b/src/systems/systemItemsTable.component.tsx @@ -632,6 +632,7 @@ export function SystemItemsTable(props: SystemItemsTableProps) { sx: { minHeight: '360.4px', height: table.getState().isFullScreen ? '100%' : undefined, + maxHeight: type === 'usageStatus' ? '670px' : undefined, }, }), muiSearchTextFieldProps: { @@ -681,8 +682,7 @@ export function SystemItemsTable(props: SystemItemsTableProps) { ), renderDetailPanel: ({ row }) => - type === 'usageStatus' ? undefined : row.original.catalogueItem !== - undefined ? ( + row.original.catalogueItem !== undefined ? ( Date: Tue, 9 Apr 2024 12:49:59 +0000 Subject: [PATCH 29/29] fix unit tests --- .../systemItemsTable.component.test.tsx.snap | 18 ++--- .../systemItemsTable.component.test.tsx | 69 +++++++++---------- 2 files changed, 43 insertions(+), 44 deletions(-) diff --git a/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap b/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap index b7c3d92aa..153424671 100644 --- a/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap +++ b/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap @@ -2358,7 +2358,7 @@ exports[`SystemItemsTable > SystemItemsTable (usageStatus) > renders correctly 1 aria-invalid="false" autocomplete="new-password" class="MuiInputBase-input MuiOutlinedInput-input MuiInputBase-inputSizeSmall MuiInputBase-inputAdornedStart MuiInputBase-inputAdornedEnd css-12yjm75-MuiInputBase-input-MuiOutlinedInput-input" - id=":rae:" + id=":rbh:" placeholder="Search" type="text" value="" @@ -2550,7 +2550,7 @@ exports[`SystemItemsTable > SystemItemsTable (usageStatus) > renders correctly 1 aria-label="Filter by Catalogue Item" autocomplete="new-password" class="MuiInputBase-input MuiInput-input MuiInputBase-inputAdornedEnd css-929hxt-MuiInputBase-input-MuiInput-input" - id=":raj:" + id=":rbm:" placeholder="Filter by Catalogue Item" title="Filter by Catalogue Item" type="text" @@ -2741,7 +2741,7 @@ exports[`SystemItemsTable > SystemItemsTable (usageStatus) > renders correctly 1 aria-label="Filter by Serial Number" autocomplete="new-password" class="MuiInputBase-input MuiInput-input MuiInputBase-inputAdornedEnd css-929hxt-MuiInputBase-input-MuiInput-input" - id=":rao:" + id=":rbr:" placeholder="Filter by Serial Number" title="Filter by Serial Number" type="text" @@ -2895,13 +2895,13 @@ exports[`SystemItemsTable > SystemItemsTable (usageStatus) > renders correctly 1 class="MuiInputBase-root MuiInput-root MuiInput-underline MuiInputBase-colorPrimary MuiInputBase-fullWidth MuiInputBase-formControl MuiInputBase-adornedEnd css-953pxc-MuiInputBase-root-MuiInput-root" >