Nov 22, 2024 · Nov 18, 2024 · Nov 22, 2024 · Nov 18, 2024 · Nov 22, 2024
diff --git a/site/src/pages/CreateTemplatePage/CreateTemplatePage.test.tsx b/site/src/pages/CreateTemplatePage/CreateTemplatePage.test.tsx
 import { screen, waitFor, within } from "@testing-library/react";
 import {fireEvent,screen, waitFor, within } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { API } from "api/api";
 import {
 MockTemplateVersionVariable1,
 MockTemplateVersionVariable2,
 MockTemplateVersionVariable3,
 mockApiError,
 } from "testHelpers/entities";
 import { renderWithAuth } from "testHelpers/renderHelpers";
 import CreateTemplatePage from "./CreateTemplatePage";
 );
 });
 });

 test("The page displays an error if the upload fails", async () => {
 const errMsg = "Unsupported content type header";
 jest.spyOn(API, "uploadFile").mockRejectedValueOnce(
 mockApiError({
 message: errMsg,
 }),
 );
 await renderPage(new URLSearchParams());

 const file = new File([""], "invalidfile.tar");
 const dropZone = screen.getByTestId("drop-zone");

 fireEvent.drop(dropZone, {
 dataTransfer: { files: [file] },
 });

 await waitFor(() => expect(API.uploadFile).toHaveBeenCalledTimes(1));

 // Error toast should be displayed
 expect(await screen.findByText(errMsg)).toBeInTheDocument();

 // The upload box should have reset itself
 expect(
 await screen.findByText(/The template has to be a .tar or .zip file/),
 ).toBeInTheDocument();
 });
diff --git a/site/src/pages/CreateTemplatePage/UploadTemplateView.tsx b/site/src/pages/CreateTemplatePage/UploadTemplateView.tsx
 import { getErrorMessage } from "api/errors";
 import { uploadFile } from "api/queries/files";
 import {
 JobError,
 templateVersionLogs,
 templateVersionVariables,
 } from "api/queries/templates";
 import { displayError } from "components/GlobalSnackbar/utils";
 import { useDashboard } from "modules/dashboard/useDashboard";
 import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility";
 import type { FC } from "react";
 jobError={isJobError ? error.job.error : undefined}
 logs={templateVersionLogsQuery.data}
 upload={{
 onUpload: uploadFileMutation.mutateAsync,
 onUpload: async (file: File) => {
 try {
 await uploadFileMutation.mutateAsync(file);
 } catch (error) {
 displayError(getErrorMessage(error, "Failed to upload file"));
 uploadFileMutation.reset();
 }
 },
 isUploading: uploadFileMutation.isLoading,
 onRemove: uploadFileMutation.reset,
 file: uploadFileMutation.variables,
Original file line number	Diff line number	Diff line change
		@@ -1,4 +1,4 @@
		import { screen, waitFor, within } from "@testing-library/react";
		import {fireEvent,screen, waitFor, within } from "@testing-library/react";
		import userEvent from "@testing-library/user-event";
		import { API } from "api/api";
		import {
Expand All		@@ -8,6 +8,7 @@ import {
		MockTemplateVersionVariable1,
		MockTemplateVersionVariable2,
		MockTemplateVersionVariable3,
		mockApiError,
		} from "testHelpers/entities";
		import { renderWithAuth } from "testHelpers/renderHelpers";
		import CreateTemplatePage from "./CreateTemplatePage";
Expand DownExpand Up		@@ -150,3 +151,30 @@ test("Create template from duplicating a template", async () => {
		);
		});
		});

		test("The page displays an error if the upload fails", async () => {
		const errMsg = "Unsupported content type header";
		jest.spyOn(API, "uploadFile").mockRejectedValueOnce(
		mockApiError({
		message: errMsg,
		}),
		);
		await renderPage(new URLSearchParams());

		const file = new File([""], "invalidfile.tar");
		const dropZone = screen.getByTestId("drop-zone");

		fireEvent.drop(dropZone, {
		dataTransfer: { files: [file] },
		});

		await waitFor(() => expect(API.uploadFile).toHaveBeenCalledTimes(1));

		// Error toast should be displayed
		expect(await screen.findByText(errMsg)).toBeInTheDocument();

		// The upload box should have reset itself
		expect(
		await screen.findByText(/The template has to be a .tar or .zip file/),
		).toBeInTheDocument();
		});
Original file line number	Diff line number	Diff line change
		@@ -1,9 +1,11 @@
		import { getErrorMessage } from "api/errors";
		import { uploadFile } from "api/queries/files";
		import {
		JobError,
		templateVersionLogs,
		templateVersionVariables,
		} from "api/queries/templates";
		import { displayError } from "components/GlobalSnackbar/utils";
		import { useDashboard } from "modules/dashboard/useDashboard";
		import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility";
		import type { FC } from "react";
Expand DownExpand Up		@@ -51,7 +53,14 @@ export const UploadTemplateView: FC<CreateTemplatePageViewProps> = ({
		jobError={isJobError ? error.job.error : undefined}
		logs={templateVersionLogsQuery.data}
		upload={{
		onUpload: uploadFileMutation.mutateAsync,
		onUpload: async (file: File) => {
		try {
		await uploadFileMutation.mutateAsync(file);
		} catch (error) {
Copy link MemberAuthor ethanndicksonNov 18, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. I'm not familiar with this query library, but what I found is that even though you can set`onError`, an exception returned from`mutateAsync` isn't caught unless you explicitly catch it. So, If I displayed the error as part of`onError` in`useMutation`, I'd still need to wrap`mutateAsync` in a try block, with an empty catch, which seems pointless. Copy link Collaborator BrunoQuaresmaNov 22, 2024 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. You did it correctly. We usually use`mutateAsync` to keep the error handling close to where the mutation is called.
		displayError(getErrorMessage(error, "Failed to upload file"));
		uploadFileMutation.reset();
		}
		},
		isUploading: uploadFileMutation.isLoading,
		onRemove: uploadFileMutation.reset,
		file: uploadFileMutation.variables,
Expand Down