Skip to content

Commit

Permalink
fix: remove reset logic which cleared all slots (#2353)
Browse files Browse the repository at this point in the history
* fix: remove reset logic which cleared all slots

- Encountered a bug where if a file went from having one label (tag) to no labels then a validation error would appear and all tags would be lost
- Discovered that in these cases resetAllSlots was being called causing this behaviour
- Removed the reset all logic which resolved the bug

* refactor: remove variable definition

- The `updatedFileList` was being defined but the value of that defintion was never being used
- Removed the definition and added repetition but hopefully leaving more readable code.

* refactor: use cloneDeep instead of merge

- Found that merge with a single argument was shallow copying
- This lead to us directly updating the fileList
- Changed to `cloneDeep` as there was already precedence within the file
- As per #1811 structuredClone not yet supported in our stack
  • Loading branch information
Mike-Heneghan authored Oct 31, 2023
1 parent a9db181 commit 723a0e7
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 15 deletions.
19 changes: 8 additions & 11 deletions editor.planx.uk/src/@planx/components/FileUploadAndLabel/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import Select, { SelectChangeEvent, SelectProps } from "@mui/material/Select";
import { styled } from "@mui/material/styles";
import Typography from "@mui/material/Typography";
import capitalize from "lodash/capitalize";
import merge from "lodash/merge";
import React, { useEffect, useState } from "react";
import { usePrevious } from "react-use";
import ErrorWrapper from "ui/ErrorWrapper";
Expand All @@ -28,7 +27,6 @@ import {
FileList,
getTagsForSlot,
removeSlots,
resetAllSlots,
} from "./model";
import { fileLabelSchema } from "./schema";

Expand Down Expand Up @@ -172,23 +170,22 @@ const SelectMultiple = (props: SelectMultipleProps) => {
previousTags: string[] | undefined,
tags: string[],
) => {
let updatedFileList: FileList = merge(fileList);
const updatedTags = tags.filter((tag) => !previousTags?.includes(tag));
const removedTags = previousTags?.filter((tag) => !tags?.includes(tag));

if (updatedTags.length > 0) {
updatedFileList = addOrAppendSlots(updatedTags, uploadedFile, fileList);
const updatedFileList = addOrAppendSlots(
updatedTags,
uploadedFile,
fileList,
);
setFileList(updatedFileList);
}

if (removedTags && removedTags.length > 0) {
updatedFileList = removeSlots(removedTags, uploadedFile, fileList);
const updatedFileList = removeSlots(removedTags, uploadedFile, fileList);
setFileList(updatedFileList);
}

if (tags.length === 0 && previousTags) {
updatedFileList = resetAllSlots(fileList);
}

setFileList(updatedFileList);
};

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import cloneDeep from "lodash/cloneDeep";
import merge from "lodash/merge";
import sortBy from "lodash/sortBy";
import uniqBy from "lodash/uniqBy";
import { Store } from "pages/FlowEditor/lib/store";
Expand Down Expand Up @@ -331,7 +330,7 @@ export const addOrAppendSlots = (
uploadedFile: FileUploadSlot,
fileList: FileList,
): FileList => {
const updatedFileList: FileList = merge(fileList);
const updatedFileList: FileList = cloneDeep(fileList);
const categories = Object.keys(updatedFileList) as Array<
keyof typeof updatedFileList
>;
Expand Down Expand Up @@ -368,7 +367,7 @@ export const removeSlots = (
uploadedFile: FileUploadSlot,
fileList: FileList,
): FileList => {
const updatedFileList: FileList = merge(fileList);
const updatedFileList: FileList = cloneDeep(fileList);
const categories = Object.keys(updatedFileList) as Array<
keyof typeof updatedFileList
>;
Expand Down Expand Up @@ -396,7 +395,7 @@ export const removeSlots = (
};

export const resetAllSlots = (fileList: FileList): FileList => {
const updatedFileList: FileList = merge(fileList);
const updatedFileList: FileList = cloneDeep(fileList);
const categories = Object.keys(updatedFileList) as Array<
keyof typeof updatedFileList
>;
Expand Down

0 comments on commit 723a0e7

Please sign in to comment.