Skip to content

Commit 9006b21

Browse files
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
1 parent f5601cd commit 9006b21

File tree

2 files changed

+60
-2
lines changed

2 files changed

+60
-2
lines changed

site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersForm.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ export const WorkspaceParametersForm: FC<WorkspaceParameterFormProps> = ({
154154
<FormFooter
155155
onCancel={onCancel}
156156
isLoading={isSubmitting}
157-
submitDisabled={disabled}
157+
submitLabel="Submit and restart"
158+
submitDisabled={disabled || !form.dirty}
158159
/>
159160
</HorizontalForm>
160161
</>

site/src/pages/WorkspaceSettingsPage/WorkspaceParametersPage/WorkspaceParametersPage.test.tsx

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ test("Submit the workspace settings page successfully", async () => {
6161
);
6262
await user.clear(parameter2);
6363
await user.type(parameter2, "1");
64-
await user.click(within(form).getByRole("button", { name: "Submit" }));
64+
await user.click(
65+
within(form).getByRole("button", { name: "Submit and restart" }),
66+
);
6567
// Assert that the API calls were made with the correct data
6668
await waitFor(() => {
6769
expect(postWorkspaceBuildSpy).toHaveBeenCalledWith(MockWorkspace.id, {
@@ -73,3 +75,58 @@ test("Submit the workspace settings page successfully", async () => {
7375
});
7476
});
7577
});
78+
79+
test("Submit button is only enabled when changes are made", async () => {
80+
// Mock the API calls that loads data
81+
jest
82+
.spyOn(API, "getWorkspaceByOwnerAndName")
83+
.mockResolvedValueOnce(MockWorkspace);
84+
jest.spyOn(API, "getTemplateVersionRichParameters").mockResolvedValueOnce([
85+
MockTemplateVersionParameter1,
86+
MockTemplateVersionParameter2,
87+
// Immutable parameters
88+
MockTemplateVersionParameter4,
89+
]);
90+
jest.spyOn(API, "getWorkspaceBuildParameters").mockResolvedValueOnce([
91+
MockWorkspaceBuildParameter1,
92+
MockWorkspaceBuildParameter2,
93+
// Immutable value
94+
MockWorkspaceBuildParameter4,
95+
]);
96+
// Setup event and rendering
97+
const user = userEvent.setup();
98+
renderWithWorkspaceSettingsLayout(<WorkspaceParametersPage />, {
99+
route: "/@test-user/test-workspace/settings",
100+
path: "/:username/:workspace/settings",
101+
// Need this because after submit the user is redirected
102+
extraRoutes: [{ path: "/:username/:workspace", element: <div /> }],
103+
});
104+
await waitForLoaderToBeRemoved();
105+
106+
const submitButton: HTMLButtonElement = screen.getByRole("button", {
107+
name: "Submit and restart",
108+
});
109+
110+
const form = screen.getByTestId("form");
111+
const parameter1 = within(form).getByLabelText(
112+
MockWorkspaceBuildParameter1.name,
113+
{ exact: false },
114+
);
115+
116+
// There are no changes, the button should be disabled.
117+
expect(submitButton.disabled).toBeTruthy();
118+
119+
// Make changes to the form
120+
await user.clear(parameter1);
121+
await user.type(parameter1, "new-value");
122+
123+
// There are now changes, the button should be enabled.
124+
expect(submitButton.disabled).toBeFalsy();
125+
126+
// Change form value back to default
127+
await user.clear(parameter1);
128+
await user.type(parameter1, MockWorkspaceBuildParameter1.value);
129+
130+
// There are now no changes, the button should be disabled.
131+
expect(submitButton.disabled).toBeTruthy();
132+
});

0 commit comments

Comments
 (0)