Skip to content

Commit 6183919

Browse files
committed
fix: make shelter numbering strict-mode safe
Shelter numbers could jump in Strict Mode because numbering was done in a component effect. Move number assignment to add/copy actions in the shelters form utilities. Now each new shelter gets its number only when the user adds or copies, so numbering stays predictable.
1 parent bf10ec0 commit 6183919

3 files changed

Lines changed: 34 additions & 21 deletions

File tree

ui/src/components/stop-registry/stops/stop-details/shelters/ShelterFormFields.tsx

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { t } from 'i18next';
2-
import { FC, useEffect } from 'react';
2+
import { FC } from 'react';
33
import { useFormContext } from 'react-hook-form';
44
import {
55
StopRegistryShelterCondition,
@@ -50,23 +50,8 @@ export const ShelterFormFields: FC<ShelterFormFieldsProps> = ({
5050
onRemove,
5151
onCopy,
5252
}) => {
53-
const { register, watch, setValue } = useFormContext<SheltersFormState>();
53+
const { register, watch } = useFormContext<SheltersFormState>();
5454
const toBeDeleted = watch(`shelters.${index}.toBeDeleted`);
55-
const allShelters = watch('shelters');
56-
const currentShelterNumber = watch(`shelters.${index}.shelterNumber`);
57-
58-
// Suggest a shelter number based on previous numbers
59-
useEffect(() => {
60-
if (currentShelterNumber === null) {
61-
const nextNumber =
62-
Math.max(
63-
0,
64-
...allShelters.map((shelter) => Number(shelter.shelterNumber)),
65-
) + 1;
66-
67-
setValue(`shelters.${index}.shelterNumber`, nextNumber);
68-
}
69-
}, [allShelters, index, currentShelterNumber, setValue]);
7055

7156
return (
7257
<Column className="gap-4">

ui/src/components/stop-registry/stops/stop-details/shelters/SheltersForm.spec.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,12 @@ describe('useSheltersFormUtils', () => {
7676
result.current.addNewShelter();
7777
});
7878

79-
expect(appendMock).toHaveBeenCalledWith(mapShelterDataToFormState({}));
79+
expect(appendMock).toHaveBeenCalledWith(
80+
expect.objectContaining({
81+
...mapShelterDataToFormState({}),
82+
shelterNumber: 2,
83+
}),
84+
);
8085
expect(mockOnShelterCountChanged).toHaveBeenCalledWith(1);
8186
});
8287

@@ -103,6 +108,7 @@ describe('useSheltersFormUtils', () => {
103108
mapShelterDataToFormState({
104109
...testShelter,
105110
id: null,
111+
shelterNumber: 2,
106112
}),
107113
),
108114
);

ui/src/components/stop-registry/stops/stop-details/shelters/useSheltersForm.tsx

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,16 @@ type UtilProps = {
88
readonly onShelterCountChanged: (newShelterCount: number) => void;
99
};
1010

11+
const getNextShelterNumber = (
12+
shelters: SheltersFormState['shelters'],
13+
): number => {
14+
const usedNumbers = shelters
15+
.map((shelter) => shelter.shelterNumber)
16+
.filter((num): num is number => num !== null && !Number.isNaN(num));
17+
18+
return Math.max(0, ...usedNumbers) + 1;
19+
};
20+
1121
export const useSheltersFormUtils = (props: UtilProps) => {
1222
const { methods, onShelterCountChanged } = props;
1323
const { control, setValue, getValues, handleSubmit } = methods;
@@ -29,15 +39,27 @@ export const useSheltersFormUtils = (props: UtilProps) => {
2939
}, [getValues, onShelterCountChanged]);
3040

3141
const addNewShelter = useCallback(() => {
32-
append(mapShelterDataToFormState({}));
42+
const currentShelters = getValues('shelters');
43+
const nextShelterNumber = getNextShelterNumber(currentShelters);
44+
45+
append({
46+
...mapShelterDataToFormState({}),
47+
shelterNumber: nextShelterNumber,
48+
});
3349
updateShelterCount();
34-
}, [append, updateShelterCount]);
50+
}, [append, getValues, updateShelterCount]);
3551

3652
const copyToNewShelter = useCallback(
3753
(shelterIndex: number) => {
3854
const currentShelters = getValues('shelters'); // Get the latest state
3955
const shelterToCopy = currentShelters?.at(shelterIndex);
40-
const newShelter = shelterToCopy ? { ...shelterToCopy, id: null } : null;
56+
const newShelter = shelterToCopy
57+
? {
58+
...shelterToCopy,
59+
id: null,
60+
shelterNumber: getNextShelterNumber(currentShelters),
61+
}
62+
: null;
4163

4264
if (newShelter) {
4365
append(

0 commit comments

Comments
 (0)