From 9006b217580923b609202b7f94f7ab564f30f231 Mon Sep 17 00:00:00 2001
From: Danielle Maywood <danielle@themaywoods.com>
Date: Fri, 13 Sep 2024 16:19:17 +0100
Subject: [PATCH] fix: only allow submitting form if changes have been made
 (#14602)

* fix: only allow submitting form if dirty

* test: add test for submit button behaviour

* fix: apply 'make fmt'

* chore: rename 'Submit' to 'Submit and restart'

* test: fix tests
---
 .../WorkspaceParametersForm.tsx               |  3 +-
 .../WorkspaceParametersPage.test.tsx          | 59 ++++++++++++++++++-
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx
index 3963e1ec80..db0667ea8e 100644
--- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx
+++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx
@@ -154,7 +154,8 @@ export const WorkspaceParametersForm: FC<WorkspaceParameterFormProps> = ({
 				<FormFooter
 					onCancel={onCancel}
 					isLoading={isSubmitting}
-					submitDisabled={disabled}
+					submitLabel="Submit and restart"
+					submitDisabled={disabled || !form.dirty}
 				/>
 			</HorizontalForm>
 		</>
diff --git a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.test.tsx b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.test.tsx
index 763fe12fde..4c97a40e33 100644
--- a/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.test.tsx
+++ b/site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.test.tsx
@@ -61,7 +61,9 @@ test("Submit the workspace settings page successfully", async () => {
 	);
 	await user.clear(parameter2);
 	await user.type(parameter2, "1");
-	await user.click(within(form).getByRole("button", { name: "Submit" }));
+	await user.click(
+		within(form).getByRole("button", { name: "Submit and restart" }),
+	);
 	// Assert that the API calls were made with the correct data
 	await waitFor(() => {
 		expect(postWorkspaceBuildSpy).toHaveBeenCalledWith(MockWorkspace.id, {
@@ -73,3 +75,58 @@ test("Submit the workspace settings page successfully", async () => {
 		});
 	});
 });
+
+test("Submit button is only enabled when changes are made", async () => {
+	// Mock the API calls that loads data
+	jest
+		.spyOn(API, "getWorkspaceByOwnerAndName")
+		.mockResolvedValueOnce(MockWorkspace);
+	jest.spyOn(API, "getTemplateVersionRichParameters").mockResolvedValueOnce([
+		MockTemplateVersionParameter1,
+		MockTemplateVersionParameter2,
+		// Immutable parameters
+		MockTemplateVersionParameter4,
+	]);
+	jest.spyOn(API, "getWorkspaceBuildParameters").mockResolvedValueOnce([
+		MockWorkspaceBuildParameter1,
+		MockWorkspaceBuildParameter2,
+		// Immutable value
+		MockWorkspaceBuildParameter4,
+	]);
+	// Setup event and rendering
+	const user = userEvent.setup();
+	renderWithWorkspaceSettingsLayout(<WorkspaceParametersPage />, {
+		route: "/@test-user/test-workspace/settings",
+		path: "/:username/:workspace/settings",
+		// Need this because after submit the user is redirected
+		extraRoutes: [{ path: "/:username/:workspace", element: <div /> }],
+	});
+	await waitForLoaderToBeRemoved();
+
+	const submitButton: HTMLButtonElement = screen.getByRole("button", {
+		name: "Submit and restart",
+	});
+
+	const form = screen.getByTestId("form");
+	const parameter1 = within(form).getByLabelText(
+		MockWorkspaceBuildParameter1.name,
+		{ exact: false },
+	);
+
+	// There are no changes, the button should be disabled.
+	expect(submitButton.disabled).toBeTruthy();
+
+	// Make changes to the form
+	await user.clear(parameter1);
+	await user.type(parameter1, "new-value");
+
+	// There are now changes, the button should be enabled.
+	expect(submitButton.disabled).toBeFalsy();
+
+	// Change form value back to default
+	await user.clear(parameter1);
+	await user.type(parameter1, MockWorkspaceBuildParameter1.value);
+
+	// There are now no changes, the button should be disabled.
+	expect(submitButton.disabled).toBeTruthy();
+});