From fc527b6cc8435ed74505dcb2d0cc9c6526281534 Mon Sep 17 00:00:00 2001 From: DaKheera47 Date: Thu, 22 Jan 2026 12:46:43 +0000 Subject: [PATCH] checkbox didn't enable the save button, and disabling the toggle wipes the credentials in the backend upon saving --- .../src/client/pages/SettingsPage.test.tsx | 85 +++++++++++++++++++ .../src/client/pages/SettingsPage.tsx | 28 ++++-- .../EnvironmentSettingsSection.test.tsx | 7 +- .../components/EnvironmentSettingsSection.tsx | 30 ++++--- .../settings/components/SettingsInput.tsx | 10 ++- orchestrator/src/shared/settings-schema.ts | 1 + 6 files changed, 134 insertions(+), 27 deletions(-) diff --git a/orchestrator/src/client/pages/SettingsPage.test.tsx b/orchestrator/src/client/pages/SettingsPage.test.tsx index 3189857..ba334f3 100644 --- a/orchestrator/src/client/pages/SettingsPage.test.tsx +++ b/orchestrator/src/client/pages/SettingsPage.test.tsx @@ -195,4 +195,89 @@ describe("SettingsPage", () => { }) ) }) + + it("enables save button when model is changed", async () => { + vi.mocked(api.getSettings).mockResolvedValue(baseSettings) + renderPage() + const saveButton = screen.getByRole("button", { name: /^save$/i }) + expect(saveButton).toBeDisabled() + + const modelTrigger = await screen.findByRole("button", { name: /model/i }) + fireEvent.click(modelTrigger) + const modelInput = screen.getByLabelText(/override model/i) + fireEvent.change(modelInput, { target: { value: "new-model" } }) + expect(saveButton).toBeEnabled() + }) + + it("enables save button when numeric setting is changed", async () => { + vi.mocked(api.getSettings).mockResolvedValue(baseSettings) + renderPage() + const saveButton = screen.getByRole("button", { name: /^save$/i }) + + const visaTrigger = await screen.findByRole("button", { name: /ukvisajobs extractor/i }) + fireEvent.click(visaTrigger) + const maxJobsInput = screen.getByLabelText(/max jobs to fetch/i) + fireEvent.change(maxJobsInput, { target: { value: "100" } }) + expect(saveButton).toBeEnabled() + }) + + it("enables save button when display setting is changed", async () => { + vi.mocked(api.getSettings).mockResolvedValue(baseSettings) + renderPage() + const saveButton = screen.getByRole("button", { name: /^save$/i }) + + const displayTrigger = await screen.findByRole("button", { name: /display settings/i }) + fireEvent.click(displayTrigger) + const sponsorCheckbox = screen.getByLabelText(/show visa sponsor information/i) + fireEvent.click(sponsorCheckbox) + expect(saveButton).toBeEnabled() + }) + + it("enables save button when basic auth toggle is changed", async () => { + vi.mocked(api.getSettings).mockResolvedValue(baseSettings) + renderPage() + const saveButton = screen.getByRole("button", { name: /^save$/i }) + + const envTrigger = await screen.findByRole("button", { name: /environment & accounts/i }) + fireEvent.click(envTrigger) + const authCheckbox = screen.getByLabelText(/enable basic authentication/i) + fireEvent.click(authCheckbox) + expect(saveButton).toBeEnabled() + }) + + it("wipes basic auth credentials when toggle is disabled and saved", async () => { + // Initial state: Basic Auth is active + const activeSettings = { + ...baseSettings, + basicAuthActive: true, + basicAuthUser: "admin", + basicAuthPasswordHint: "pass", + } + vi.mocked(api.getSettings).mockResolvedValue(activeSettings) + vi.mocked(api.updateSettings).mockResolvedValue(baseSettings) + + renderPage() + + const envTrigger = await screen.findByRole("button", { name: /environment & accounts/i }) + fireEvent.click(envTrigger) + + const authCheckbox = screen.getByLabelText(/enable basic authentication/i) + expect(authCheckbox).toBeChecked() + + // Disable it + fireEvent.click(authCheckbox) + expect(authCheckbox).not.toBeChecked() + + const saveButton = screen.getByRole("button", { name: /^save$/i }) + expect(saveButton).toBeEnabled() + fireEvent.click(saveButton) + + await waitFor(() => expect(api.updateSettings).toHaveBeenCalled()) + expect(api.updateSettings).toHaveBeenCalledWith( + expect.objectContaining({ + basicAuthUser: null, + basicAuthPassword: null, + }) + ) + }) }) diff --git a/orchestrator/src/client/pages/SettingsPage.tsx b/orchestrator/src/client/pages/SettingsPage.tsx index 075286a..e995f49 100644 --- a/orchestrator/src/client/pages/SettingsPage.tsx +++ b/orchestrator/src/client/pages/SettingsPage.tsx @@ -49,6 +49,7 @@ const DEFAULT_FORM_VALUES: UpdateSettingsInput = { ukvisajobsEmail: "", ukvisajobsPassword: "", webhookSecret: "", + enableBasicAuth: false, } const NULL_SETTINGS_PAYLOAD: UpdateSettingsInput = { @@ -105,6 +106,7 @@ const mapSettingsToForm = (data: AppSettings): UpdateSettingsInput => ({ ukvisajobsEmail: data.ukvisajobsEmail ?? "", ukvisajobsPassword: "", webhookSecret: "", + enableBasicAuth: data.basicAuthActive, }) const normalizeString = (value: string | null | undefined) => { @@ -284,7 +286,7 @@ export const SettingsPage: React.FC = () => { if (!settings) return try { setIsSaving(true) - + // Prepare payload: nullify if equal to default const resumeProjectsData = data.resumeProjects const resumeProjectsOverride = (resumeProjectsData && defaultResumeProjects && resumeProjectsEqual(resumeProjectsData, defaultResumeProjects)) @@ -301,8 +303,18 @@ export const SettingsPage: React.FC = () => { envPayload.ukvisajobsEmail = normalizeString(data.ukvisajobsEmail) } - if (dirtyFields.basicAuthUser) { - envPayload.basicAuthUser = normalizeString(data.basicAuthUser) + if (data.enableBasicAuth === false) { + envPayload.basicAuthUser = null + envPayload.basicAuthPassword = null + } else { + if (dirtyFields.basicAuthUser) { + envPayload.basicAuthUser = normalizeString(data.basicAuthUser) + } + + if (dirtyFields.basicAuthPassword) { + const value = normalizePrivateInput(data.basicAuthPassword) + if (value !== undefined) envPayload.basicAuthPassword = value + } } if (dirtyFields.openrouterApiKey) { @@ -320,11 +332,6 @@ export const SettingsPage: React.FC = () => { if (value !== undefined) envPayload.ukvisajobsPassword = value } - if (dirtyFields.basicAuthPassword) { - const value = normalizePrivateInput(data.basicAuthPassword) - if (value !== undefined) envPayload.basicAuthPassword = value - } - if (dirtyFields.webhookSecret) { const value = normalizePrivateInput(data.webhookSecret) if (value !== undefined) envPayload.webhookSecret = value @@ -354,6 +361,11 @@ export const SettingsPage: React.FC = () => { ...envPayload, } + // Remove virtual field because the backend doesn't expect it + // this exists only to toggle the UI + // need o track it so that the save button is enabled when it changes + delete payload.enableBasicAuth + const updated = await api.updateSettings(payload) setSettings(updated) reset(mapSettingsToForm(updated)) diff --git a/orchestrator/src/client/pages/settings/components/EnvironmentSettingsSection.test.tsx b/orchestrator/src/client/pages/settings/components/EnvironmentSettingsSection.test.tsx index 8801d2e..3a3b863 100644 --- a/orchestrator/src/client/pages/settings/components/EnvironmentSettingsSection.test.tsx +++ b/orchestrator/src/client/pages/settings/components/EnvironmentSettingsSection.test.tsx @@ -16,6 +16,7 @@ const EnvironmentSettingsHarness = () => { ukvisajobsPassword: "", basicAuthPassword: "", webhookSecret: "", + enableBasicAuth: true, } }) @@ -53,9 +54,9 @@ describe("EnvironmentSettingsSection", () => { expect(screen.getByDisplayValue("resume@example.com")).toBeInTheDocument() expect(screen.getByDisplayValue("visa@example.com")).toBeInTheDocument() - expect(screen.getByText("sk-1********")).toBeInTheDocument() - expect(screen.getByText("pass********")).toBeInTheDocument() - expect(screen.getByText("abcd********")).toBeInTheDocument() + expect(screen.getByText(/sk-1\*{8}/)).toBeInTheDocument() + expect(screen.getByText(/pass\*{8}/)).toBeInTheDocument() + expect(screen.getByText(/abcd\*{8}/)).toBeInTheDocument() expect(screen.getByText("Not set")).toBeInTheDocument() // Basic Auth diff --git a/orchestrator/src/client/pages/settings/components/EnvironmentSettingsSection.tsx b/orchestrator/src/client/pages/settings/components/EnvironmentSettingsSection.tsx index c7511d5..f4bee27 100644 --- a/orchestrator/src/client/pages/settings/components/EnvironmentSettingsSection.tsx +++ b/orchestrator/src/client/pages/settings/components/EnvironmentSettingsSection.tsx @@ -1,5 +1,5 @@ -import React, { useState, useEffect } from "react" -import { useFormContext } from "react-hook-form" +import React from "react" +import { useFormContext, Controller } from "react-hook-form" import { AccordionContent, AccordionItem, AccordionTrigger } from "@/components/ui/accordion" import { Checkbox } from "@/components/ui/checkbox" @@ -21,14 +21,10 @@ export const EnvironmentSettingsSection: React.FC { - const { register, formState: { errors } } = useFormContext() - const { private: privateValues, basicAuthActive } = values + const { register, control, watch, formState: { errors } } = useFormContext() + const { private: privateValues } = values - const [isBasicAuthEnabled, setIsBasicAuthEnabled] = useState(basicAuthActive) - - useEffect(() => { - setIsBasicAuthEnabled(basicAuthActive) - }, [basicAuthActive]) + const isBasicAuthEnabled = watch("enableBasicAuth") return ( @@ -110,11 +106,17 @@ export const EnvironmentSettingsSection: React.FC
Security
- setIsBasicAuthEnabled(checked === true)} - disabled={isLoading || isSaving} + ( + + )} />