USEPA / haztrak

An open-source web app that illustrates how waste management software can interface with RCRAInfo to track hazardous waste electronically
https://usepa.github.io/haztrak/
MIT License
45 stars 19 forks source link

Add unique key to transporter array elements #718

Closed sheckathorne closed 1 month ago

sheckathorne commented 1 month ago

Description

Created a unique key for each Transporter by combining the EPAID and array index. This allows the user to apply all actions in the transporter drop-down menu without side-effects.

I found a bug where the tool-tips 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 tool-tip labels since the selectors are based on the index value.

Added a test to ensure that only one accordion expands when the same Transporter appears in the manifest and 'Details' is clicked.

Issue ticket number and link

Closes issue #620 [https://github.com/USEPA/haztrak/issues/620]

Checklist

dpgraham4401 commented 1 month ago

@sheckathorne Thanks for the PR, I should have time to look at it this week.

dpgraham4401 commented 1 month ago

@sheckathorne Thanks for getting the ball rolling on this and submitting the PR. I've added a clientKey field to the transporter schema, the field is checked and, if undefined, a uuid is generated and used as a key for the transporter instance.

This was the only way I could think to fix the original bug while playing nice with react and not relying on the order/index of the transporter instance.

image

sheckathorne commented 1 month ago

After this commit I'm no longer able to add transporters to a manifest unless I change:

// TransporterSection.tsx 
 const transporters = transporterForm.fields;

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

to

// TransporterSection.tsx 
  const transporters: Array<Transporter> = manifestForm.getValues('transporters');
  transporters.forEach(transporter => {
    transporter.clientKey = uuidv4();
  })

however this re-introduces the problem of React re-rendering the entire table every time the transporters array is modified.

dpgraham4401 commented 1 month ago

Ah geez, here's the quick fix.


export function TransporterSection({ setupSign }: TransporterSectionProps) {
  const [, setSearchConfigs] = useHandlerSearchConfig();
  const [readOnly] = useReadOnly();
  const manifestForm = useFormContext<Manifest>();
  const { errors } = manifestForm.formState;
  const transporterForm = useFieldArray<Manifest, 'transporters'>({
    control: manifestForm.control,
    name: 'transporters',
  });
  const transporters = manifestForm.watch('transporters');

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

The difference is the change from const transporters = transporterForm.fields; to const transporters = manifestForm.watch('transporters');

@sheckathorne Do you want to submit a PR for this?

I've started looking at the react-hook-form useFieldArray documentation more, turns out the fields value returned from that hook has a unique id appended to each object intended to be used for this very thing (Doh!). I couldn't get the "Controlled Field Array" example (bottom of the page) example to rerender when a new transporter is appended, but that appears to be what we're going for as a long term fix.

sheckathorne commented 1 month ago

Sure, I'll take a look at it today