From 8237b5b429f03c0f7800a9c46557686aaad89d10 Mon Sep 17 00:00:00 2001 From: Alec M Date: Thu, 19 Sep 2024 13:32:09 -0400 Subject: [PATCH 1/2] CRDCDH-1614 Misc. CLI Config File dialog updates --- .../DataSubmissions/DataUpload.test.tsx | 71 +++++++++++++++++- src/components/DataSubmissions/DataUpload.tsx | 2 +- .../UploaderConfigDialog/index.test.tsx | 74 ++++++++++++++++++- src/components/UploaderConfigDialog/index.tsx | 55 ++++++++++---- 4 files changed, 180 insertions(+), 22 deletions(-) diff --git a/src/components/DataSubmissions/DataUpload.test.tsx b/src/components/DataSubmissions/DataUpload.test.tsx index 9d41bb1e..6cf4325f 100644 --- a/src/components/DataSubmissions/DataUpload.test.tsx +++ b/src/components/DataSubmissions/DataUpload.test.tsx @@ -150,7 +150,9 @@ describe("Basic Functionality", () => { // Open the dialog userEvent.click(getByTestId("uploader-cli-config-button")); - // Skip filling the fields and click the download button + userEvent.type(getByTestId("uploader-config-dialog-input-data-folder"), "test-folder"); + userEvent.type(getByTestId("uploader-config-dialog-input-manifest"), "test-manifest"); + userEvent.click(getByText("Download")); await waitFor(() => { @@ -185,7 +187,9 @@ describe("Basic Functionality", () => { // Open the dialog userEvent.click(getByTestId("uploader-cli-config-button")); - // Skip filling the fields and click the download button + userEvent.type(getByTestId("uploader-config-dialog-input-data-folder"), "test-folder"); + userEvent.type(getByTestId("uploader-config-dialog-input-manifest"), "test-manifest"); + userEvent.click(getByText("Download")); await waitFor(() => { @@ -197,6 +201,61 @@ describe("Basic Functionality", () => { ); }); }); + + it("should hide the CLI Configuration dialog when onClose is called", async () => { + const { getByTestId, findAllByRole, queryByRole } = render( + + + + ); + + // Open the dialog + userEvent.click(getByTestId("uploader-cli-config-button")); + + const dialog = await findAllByRole("presentation"); + + expect(dialog[1]).toBeVisible(); + + // Close the dialog + userEvent.click(dialog[1]); + + await waitFor(() => { + expect(queryByRole("dialog")).not.toBeInTheDocument(); + }); + }); + + it("should hide the Uploader CLI dialog when onClose is called", async () => { + const { getByTestId, findAllByRole, queryByRole } = render( + + + + ); + + // Open the dialog + userEvent.click(getByTestId("uploader-cli-download-button")); + + const dialog = await findAllByRole("presentation"); + + expect(dialog[1]).toBeVisible(); + + // Close the dialog + userEvent.click(dialog[1]); + + await waitFor(() => { + expect(queryByRole("dialog")).not.toBeInTheDocument(); + }); + }); }); describe("Implementation Requirements", () => { @@ -334,7 +393,9 @@ describe("Implementation Requirements", () => { userEvent.click(getByTestId("uploader-cli-config-button")); }); - // Skip filling the fields and click the download button + userEvent.type(getByTestId("uploader-config-dialog-input-data-folder"), "test-folder"); + userEvent.type(getByTestId("uploader-config-dialog-input-manifest"), "test-manifest"); + // eslint-disable-next-line testing-library/no-unnecessary-act -- RHF is throwing an error without act await act(async () => { userEvent.click(getByText("Download")); @@ -380,7 +441,9 @@ describe("Implementation Requirements", () => { // Open the dialog userEvent.click(getByTestId("uploader-cli-config-button")); - // Skip filling the fields and click the download button + userEvent.type(getByTestId("uploader-config-dialog-input-data-folder"), "test-folder"); + userEvent.type(getByTestId("uploader-config-dialog-input-manifest"), "test-manifest"); + userEvent.click(getByText("Download")); await waitFor(() => { diff --git a/src/components/DataSubmissions/DataUpload.tsx b/src/components/DataSubmissions/DataUpload.tsx index 04730c96..aa02460f 100644 --- a/src/components/DataSubmissions/DataUpload.tsx +++ b/src/components/DataSubmissions/DataUpload.tsx @@ -102,7 +102,7 @@ export const DataUpload: FC = ({ submission }: Props) => { }; const Actions: ReactElement = useMemo(() => { - if (submission?.dataType === "Metadata Only") { + if (submission?.dataType !== "Metadata and Data Files") { return null; } diff --git a/src/components/UploaderConfigDialog/index.test.tsx b/src/components/UploaderConfigDialog/index.test.tsx index e339157d..067efc49 100644 --- a/src/components/UploaderConfigDialog/index.test.tsx +++ b/src/components/UploaderConfigDialog/index.test.tsx @@ -98,7 +98,7 @@ describe("Basic Functionality", () => { }); }); - it("should call the onDownload function when the 'Download' button is clicked", async () => { + it("should call the onDownload function when the 'Download' button is clicked with a valid form", async () => { const { getByTestId } = render( @@ -107,6 +107,9 @@ describe("Basic Functionality", () => { expect(mockDownload).not.toHaveBeenCalled(); + userEvent.type(getByTestId("uploader-config-dialog-input-data-folder"), "test-folder"); + userEvent.type(getByTestId("uploader-config-dialog-input-manifest"), "test-manifest"); + userEvent.click(getByTestId("uploader-config-dialog-download-button")); await waitFor(() => { @@ -155,3 +158,72 @@ describe("Basic Functionality", () => { }); }); }); + +describe("Implementation Requirements", () => { + const mockDownload = jest.fn(); + const mockOnClose = jest.fn(); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it("should not submit the form if any of the inputs are invalid", async () => { + const { getByTestId, queryAllByText } = render( + + + + ); + + userEvent.click(getByTestId("uploader-config-dialog-download-button")); + + await waitFor(() => { + expect(queryAllByText(/This field is required/)).toHaveLength(2); + }); + + expect(mockDownload).not.toHaveBeenCalled(); + }); + + it("should have a tooltip on the Data Files Folder input", async () => { + const { getByTestId, findByRole } = render( + + + + ); + + userEvent.hover(getByTestId("data-folder-input-tooltip")); + + const tooltip = await findByRole("tooltip"); + expect(tooltip).toBeInTheDocument(); + expect(tooltip).toHaveTextContent( + "Enter the full path for the Data Files folder on your local machine or S3 bucket" + ); + + userEvent.unhover(getByTestId("data-folder-input-tooltip")); + + await waitFor(() => { + expect(tooltip).not.toBeVisible(); + }); + }); + + it("should have a tooltip on the Manifest File input", async () => { + const { getByTestId, findByRole } = render( + + + + ); + + userEvent.hover(getByTestId("manifest-input-tooltip")); + + const tooltip = await findByRole("tooltip"); + expect(tooltip).toBeInTheDocument(); + expect(tooltip).toHaveTextContent( + "Enter the full path for the File Manifest on your local machine or S3 bucket" + ); + + userEvent.unhover(getByTestId("manifest-input-tooltip")); + + await waitFor(() => { + expect(tooltip).not.toBeVisible(); + }); + }); +}); diff --git a/src/components/UploaderConfigDialog/index.tsx b/src/components/UploaderConfigDialog/index.tsx index 2244a71b..fd54e45b 100644 --- a/src/components/UploaderConfigDialog/index.tsx +++ b/src/components/UploaderConfigDialog/index.tsx @@ -11,9 +11,13 @@ import { } from "@mui/material"; import { LoadingButton } from "@mui/lab"; import { useForm } from "react-hook-form"; +import { isEqual } from "lodash"; import { ReactComponent as CloseIconSvg } from "../../assets/icons/close_icon.svg"; import StyledOutlinedInput from "../StyledFormComponents/StyledOutlinedInput"; import StyledLabel from "../StyledFormComponents/StyledLabel"; +import StyledAsterisk from "../StyledFormComponents/StyledAsterisk"; +import Tooltip from "../Tooltip"; +import StyledHelperText from "../StyledFormComponents/StyledHelperText"; const StyledDialog = styled(Dialog)({ "& .MuiDialog-paper": { @@ -65,7 +69,7 @@ const StyledCloseDialogButton = styled(IconButton)({ const StyledForm = styled("form")({ display: "flex", flexDirection: "column", - gap: "28px", + gap: "8px", margin: "0 auto", marginTop: "28px", maxWidth: "484px", @@ -117,13 +121,14 @@ type Props = { * @returns {React.FC} */ const UploaderConfigDialog: FC = ({ onClose, onDownload, open, ...rest }) => { - const { reset, register, getValues } = useForm(); + const { reset, register, handleSubmit, formState } = useForm(); + const { errors } = formState; const [downloading, setDownloading] = useState(false); - const handleDownload = async () => { + const onSubmit = async (data: InputForm) => { setDownloading(true); - await onDownload(getValues()); + await onDownload(data); setDownloading(false); }; @@ -151,33 +156,50 @@ const UploaderConfigDialog: FC = ({ onClose, onDownload, open, ...rest }) data-testid="uploader-config-dialog-header" variant="h1" > - Download -
- Configuration File + Download Configuration File - Please provide the full pathway to the data files on your local system and to the file - manifest. + Please provide the full path to the data files and to the file manifest. - + - Local Path of Data Files Folder + + Full Path to Data Files Folder + + + + {errors?.dataFolder?.message} - Local Path of Manifest File + + Full Path to Manifest File + + + + {errors?.manifest?.message} @@ -192,7 +214,8 @@ const UploaderConfigDialog: FC = ({ onClose, onDownload, open, ...rest }) Download @@ -202,4 +225,4 @@ const UploaderConfigDialog: FC = ({ onClose, onDownload, open, ...rest }) ); }; -export default UploaderConfigDialog; +export default React.memo(UploaderConfigDialog, isEqual); From 3e386a87d55325f9724a3e9f96b73a406abbc0db Mon Sep 17 00:00:00 2001 From: Alec M Date: Mon, 23 Sep 2024 09:23:14 -0400 Subject: [PATCH 2/2] fix: Whitespace-only input should not be accepted --- .../UploaderConfigDialog/index.test.tsx | 76 +++++++++++++++++++ src/components/UploaderConfigDialog/index.tsx | 22 ++++-- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/src/components/UploaderConfigDialog/index.test.tsx b/src/components/UploaderConfigDialog/index.test.tsx index 067efc49..667640ea 100644 --- a/src/components/UploaderConfigDialog/index.test.tsx +++ b/src/components/UploaderConfigDialog/index.test.tsx @@ -157,6 +157,32 @@ describe("Basic Functionality", () => { expect(manifestInput).toHaveValue(""); }); }); + + it("should trim whitespace from the input fields before submitting", async () => { + const { getByTestId } = render( + + + + ); + + userEvent.type( + getByTestId("uploader-config-dialog-input-data-folder"), + "C:/Users/me/my-data-folder " + ); + userEvent.type( + getByTestId("uploader-config-dialog-input-manifest"), + "C:/Users/me/my-manifest.tsv " + ); + + userEvent.click(getByTestId("uploader-config-dialog-download-button")); + + await waitFor(() => { + expect(mockDownload).toHaveBeenCalledWith({ + dataFolder: "C:/Users/me/my-data-folder", + manifest: "C:/Users/me/my-manifest.tsv", + }); + }); + }); }); describe("Implementation Requirements", () => { @@ -183,6 +209,56 @@ describe("Implementation Requirements", () => { expect(mockDownload).not.toHaveBeenCalled(); }); + it("should not accept whitespace-only input for the Data Files Folder", async () => { + const { getByTestId } = render( + + + + ); + + userEvent.type(getByTestId("uploader-config-dialog-input-data-folder"), " ".repeat(10)); + userEvent.type( + getByTestId("uploader-config-dialog-input-manifest"), + "C:/someUser/someFolder/someManifest.tsv" + ); + + userEvent.click(getByTestId("uploader-config-dialog-download-button")); + + await waitFor(() => { + expect(getByTestId("uploader-config-dialog-error-data-folder")).toBeVisible(); + expect(getByTestId("uploader-config-dialog-error-data-folder")).toHaveTextContent( + /This field is required/ + ); + }); + + expect(mockDownload).not.toHaveBeenCalled(); + }); + + it("should not accept whitespace-only input for the Manifest File", async () => { + const { getByTestId } = render( + + + + ); + + userEvent.type( + getByTestId("uploader-config-dialog-input-data-folder"), + "C:/someUser/someFolder/someDataFolder" + ); + userEvent.type(getByTestId("uploader-config-dialog-input-manifest"), " ".repeat(10)); + + userEvent.click(getByTestId("uploader-config-dialog-download-button")); + + await waitFor(() => { + expect(getByTestId("uploader-config-dialog-error-manifest")).toBeVisible(); + expect(getByTestId("uploader-config-dialog-error-manifest")).toHaveTextContent( + /This field is required/ + ); + }); + + expect(mockDownload).not.toHaveBeenCalled(); + }); + it("should have a tooltip on the Data Files Folder input", async () => { const { getByTestId, findByRole } = render( diff --git a/src/components/UploaderConfigDialog/index.tsx b/src/components/UploaderConfigDialog/index.tsx index fd54e45b..7345351b 100644 --- a/src/components/UploaderConfigDialog/index.tsx +++ b/src/components/UploaderConfigDialog/index.tsx @@ -168,38 +168,48 @@ const UploaderConfigDialog: FC = ({ onClose, onDownload, open, ...rest }) Full Path to Data Files Folder v?.trim(), + })} placeholder="/Users/me/my-data-files-folder" data-testid="uploader-config-dialog-input-data-folder" inputProps={{ "aria-labelledby": "data-folder-input-label" }} /> - {errors?.dataFolder?.message} + + {errors?.dataFolder?.message} + Full Path to Manifest File v?.trim(), + })} placeholder="/Users/me/my-metadata-folder/my-file-manifest.tsv" data-testid="uploader-config-dialog-input-manifest" inputProps={{ "aria-labelledby": "manifest-input-label" }} /> - {errors?.manifest?.message} + + {errors?.manifest?.message} +