fix(frontend): Prevent saving empty custom model (All-Hands-AI#6149)

AlexCuadron · Jan 13, 2025 · b89a42c · b89a42c
1 parent 90665c9
commit b89a42c
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 10 deletions.
diff --git a/frontend/__tests__/components/shared/modals/settings/settings-form.test.tsx b/frontend/__tests__/components/shared/modals/settings/settings-form.test.tsx
@@ -1,13 +1,22 @@
 import { screen, fireEvent } from "@testing-library/react";
-import { describe, it, expect, vi } from "vitest";
+import { describe, it, expect, vi, afterEach } from "vitest";
 import { renderWithProviders } from "test-utils";
 import { createRoutesStub } from "react-router";
+import userEvent from "@testing-library/user-event";
 import { DEFAULT_SETTINGS } from "#/services/settings";
 import { SettingsForm } from "#/components/shared/modals/settings/settings-form";
 import OpenHands from "#/api/open-hands";
 
 describe("SettingsForm", () => {
   const getConfigSpy = vi.spyOn(OpenHands, "getConfig");
+  const saveSettingsSpy = vi.spyOn(OpenHands, "saveSettings");
+
+  const onCloseMock = vi.fn();
+
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
   getConfigSpy.mockResolvedValue({
     APP_MODE: "saas",
     GITHUB_CLIENT_ID: "123",
@@ -19,10 +28,10 @@ describe("SettingsForm", () => {
       Component: () => (
         <SettingsForm
           settings={DEFAULT_SETTINGS}
-          models={[]}
-          agents={[]}
-          securityAnalyzers={[]}
-          onClose={() => {}}
+          models={["anthropic/claude-3-5-sonnet-20241022", "model2"]}
+          agents={["CodeActAgent", "agent2"]}
+          securityAnalyzers={["analyzer1", "analyzer2"]}
+          onClose={onCloseMock}
         />
       ),
       path: "/",
@@ -35,11 +44,33 @@ describe("SettingsForm", () => {
   });
 
   it("should show runtime size selector when advanced options are enabled", async () => {
+    const user = userEvent.setup();
     renderWithProviders(<RouterStub />);
-    const advancedSwitch = screen.getByRole("switch", {
-      name: "SETTINGS_FORM$ADVANCED_OPTIONS_LABEL",
-    });
-    fireEvent.click(advancedSwitch);
-    await screen.findByText("SETTINGS_FORM$RUNTIME_SIZE_LABEL");
+
+    const toggleAdvancedMode = screen.getByTestId("advanced-option-switch");
+    await user.click(toggleAdvancedMode);
+
+    await screen.findByTestId("runtime-size");
+  });
+
+  it("should not submit the form if required fields are empty", async () => {
+    const user = userEvent.setup();
+    renderWithProviders(<RouterStub />);
+
+    expect(screen.queryByTestId("custom-model-input")).not.toBeInTheDocument();
+
+    const toggleAdvancedMode = screen.getByTestId("advanced-option-switch");
+    await user.click(toggleAdvancedMode);
+
+    const customModelInput = screen.getByTestId("custom-model-input");
+    expect(customModelInput).toBeInTheDocument();
+
+    await user.clear(customModelInput);
+
+    const saveButton = screen.getByTestId("save-settings-button");
+    await user.click(saveButton);
+
+    expect(saveSettingsSpy).not.toHaveBeenCalled();
+    expect(onCloseMock).not.toHaveBeenCalled();
   });
 });
diff --git a/frontend/src/components/shared/inputs/advanced-option-switch.tsx b/frontend/src/components/shared/inputs/advanced-option-switch.tsx
@@ -18,6 +18,7 @@ export function AdvancedOptionSwitch({
 
   return (
     <Switch
+      data-testid="advanced-option-switch"
       isDisabled={isDisabled}
       name="use-advanced-options"
       defaultSelected={showAdvancedOptions}

diff --git a/frontend/src/components/shared/inputs/custom-model-input.tsx b/frontend/src/components/shared/inputs/custom-model-input.tsx
@@ -22,7 +22,9 @@ export function CustomModelInput({
         {t(I18nKey.SETTINGS_FORM$CUSTOM_MODEL_LABEL)}
       </label>
       <Input
+        data-testid="custom-model-input"
         isDisabled={isDisabled}
+        isRequired
         id="custom-model"
         name="custom-model"
         defaultValue={defaultValue}

diff --git a/frontend/src/components/shared/modals/settings/runtime-size-selector.tsx b/frontend/src/components/shared/modals/settings/runtime-size-selector.tsx
@@ -21,6 +21,7 @@ export function RuntimeSizeSelector({
         {t("SETTINGS_FORM$RUNTIME_SIZE_LABEL")}
       </label>
       <Select
+        data-testid="runtime-size"
         id="runtime-size"
         name="runtime-size"
         defaultSelectedKeys={[String(defaultValue || 1)]}

diff --git a/frontend/src/components/shared/modals/settings/settings-form.tsx b/frontend/src/components/shared/modals/settings/settings-form.tsx
@@ -204,6 +204,7 @@ export function SettingsForm({
         <div className="flex flex-col gap-2">
           <div className="flex gap-2">
             <ModalButton
+              testId="save-settings-button"
               disabled={disabled}
               type="submit"
               text={t(I18nKey.SETTINGS_FORM$SAVE_LABEL)}