Skip to content

Commit

Permalink
Add unique key to transporter array elements (#718)
Browse files Browse the repository at this point in the history
* in reference to issue #620 #620, created a unique key for each Transporter by combining the EPAID and array index. I also found a bug where the tooltips in the Transporter list displayed the zero-index value, i.e. 'view transporter 0 details' for transporter 1. I corrected the tests to reflect this change in tooltip labels. Also added a test to ensure that only one accordion expands when the same Transporter appears in the manifest and 'Details' is clicked.

* add uuid  in transporter section and transporter schema

* increment patch version

---------

Co-authored-by: David Graham <dpgraham4401@gmail.com>
  • Loading branch information
sheckathorne and dpgraham4401 authored May 30, 2024
1 parent d10a81a commit ca64499
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 14 deletions.
4 changes: 2 additions & 2 deletions client/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion client/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "haztrak",
"version": "0.7.1",
"version": "0.7.2",
"private": true,
"type": "module",
"scripts": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function TransporterRowActions({
swapTransporter(index, index - 1);
},
disabled: isFirst,
label: `move transporter ${index} up`,
label: `move transporter ${index + 1} up`,
},
{
text: 'Move Down',
Expand All @@ -68,7 +68,7 @@ export function TransporterRowActions({
swapTransporter(index, index + 1);
},
disabled: isLast,
label: `move transporter ${index} down`,
label: `move transporter ${index + 1} down`,
},
{
text: 'Remove',
Expand All @@ -77,7 +77,7 @@ export function TransporterRowActions({
removeTransporter(index);
},
disabled: false,
label: `remove transporter ${index}`,
label: `remove transporter ${index + 1}`,
},
{
text: open ? 'Close' : 'Details',
Expand All @@ -86,7 +86,7 @@ export function TransporterRowActions({
decoratedOnClick(event);
},
disabled: false,
label: `View transporter ${index} details`,
label: `View transporter ${index + 1} details`,
},
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,16 @@ import { useReadOnly } from 'hooks/manifest';
import { useHandlerSearchConfig } from 'hooks/manifest/useOpenHandlerSearch/useHandlerSearchConfig';
import { Alert } from 'react-bootstrap';
import { useFieldArray, useFormContext } from 'react-hook-form';
import { v4 as uuidv4 } from 'uuid';

interface TransporterSectionProps {
setupSign: () => void;
}

interface TransporterWithKey extends Transporter {
key: string;
}

export function TransporterSection({ setupSign }: TransporterSectionProps) {
const [, setSearchConfigs] = useHandlerSearchConfig();
const [readOnly] = useReadOnly();
Expand All @@ -20,7 +25,14 @@ export function TransporterSection({ setupSign }: TransporterSectionProps) {
control: manifestForm.control,
name: 'transporters',
});
const transporters: Array<Transporter> = manifestForm.getValues('transporters');
const transporters = transporterForm.fields;

transporters.forEach((transporter, index) => {
if (!transporter.clientKey) {
// @ts-ignore
manifestForm.setValue(`transporters[${index}].clientKey`, uuidv4());
}
});

return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('TransporterTable', () => {
});
for (let i = 1; i < TRAN_ARRAY.length; i++) {
await userEvent.click(actionDropdowns[i]);
expect(screen.getByTitle(`move transporter ${i} up`)).not.toBeDisabled();
expect(screen.getByTitle(`move transporter ${i + 1} up`)).not.toBeDisabled();
}
});
test('All but last move-down buttons are not disabled', async () => {
Expand All @@ -113,7 +113,33 @@ describe('TransporterTable', () => {
});
for (let i = 0; i < TRAN_ARRAY.length; i++) {
await userEvent.click(actionDropdowns[i]);
expect(screen.getByTitle(`move transporter ${i} down`)).not.toBeDisabled();
expect(screen.getByTitle(`move transporter ${i + 1} down`)).not.toBeDisabled();
}
});
});
test('Expanding transporter opens one accordion only', async () => {
const emptyArrayFieldMethods = {}; // empty method placeholders to fulfill required prop
TRAN_ARRAY.push({
...createMockTransporter({ epaSiteId: HANDLER_ID_1, name: HANDLER_NAME_1, order: 3 }),
});

renderWithProviders(
<TransporterTable
setupSign={mockSetupSign}
// @ts-ignore
arrayFieldMethods={emptyArrayFieldMethods}
transporters={TRAN_ARRAY}
/>,
{ preloadedState: { manifest: { readOnly: false } } }
);
const actionDropdown = await screen.findByRole('button', {
name: /transporter 1 actions/,
});
await userEvent.click(actionDropdown);
const detailButton = await screen.findByTitle('View transporter 1 details');
await userEvent.click(detailButton);

const accordion = document.querySelectorAll('.accordion-collapse.collapse.show');
expect(accordion).toHaveLength(1);
expect(accordion[0]).toBeInTheDocument();
});
10 changes: 6 additions & 4 deletions client/src/components/Manifest/Transporter/TransporterTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useReadOnly } from 'hooks/manifest';
import React, { useState } from 'react';
import { Accordion, Button, Card, Col, Row, Table, useAccordionButton } from 'react-bootstrap';
import { UseFieldArrayReturn } from 'react-hook-form';
import { v4 as uuidv4 } from 'uuid';
import { TransporterRowActions } from './TransporterRowActions';

interface TransporterTableProps {
Expand Down Expand Up @@ -52,8 +53,9 @@ function TransporterTable({ transporters, arrayFieldMethods, setupSign }: Transp
<>
<Accordion ref={parent}>
{transporters.map((transporter, index) => {
const transporterKey: string = transporter.clientKey || uuidv4();
return (
<Card key={transporter.epaSiteId} className="py-2 ps-4 pe-2 my-2">
<Card key={transporterKey} className="py-2 ps-4 pe-2 my-2">
<Row className="d-flex justify-content-around">
<Col xs={8} className="d-flex align-items-center">
<h5 className="mb-0 me-3">{transporter.order} </h5>
Expand All @@ -76,19 +78,19 @@ function TransporterTable({ transporters, arrayFieldMethods, setupSign }: Transp
</Col>
<Col xs={2} className="d-flex justify-content-end align-items-center">
{readOnly ? (
<CustomToggle eventKey={transporter.epaSiteId}></CustomToggle>
<CustomToggle eventKey={transporterKey}></CustomToggle>
) : (
<TransporterRowActions
removeTransporter={arrayFieldMethods.remove}
swapTransporter={arrayFieldMethods.swap}
index={index}
length={transporters?.length}
eventKey={transporter.epaSiteId}
eventKey={transporterKey}
/>
)}
</Col>
</Row>
<Accordion.Collapse eventKey={transporter.epaSiteId}>
<Accordion.Collapse eventKey={transporterKey}>
<Card.Body>
<Table responsive>
<thead>
Expand Down
1 change: 1 addition & 0 deletions client/src/components/Manifest/manifestSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export const handlerSchema = rcraSite.extend({
export type Handler = z.infer<typeof handlerSchema>;

export const transporterSchema = handlerSchema.extend({
clientKey: z.string().optional(),
order: z.number(),
manifest: z.number().optional(),
});
Expand Down
6 changes: 6 additions & 0 deletions client/src/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,9 @@ Object.defineProperty(window, 'matchMedia', {
dispatchEvent: vi.fn(),
})),
});

// Mocking the useAutoAnimate hook which causes error in the test environment
// https://github.com/formkit/auto-animate/issues/149#issuecomment-1782772600
vi.mock('@formkit/auto-animate/react', () => ({
useAutoAnimate: () => [null],
}));

0 comments on commit ca64499

Please sign in to comment.