feat(service-credential-tester): slice 2 — Test button + gating (shared ServiceTestPanel)
Presentational ServiceTestPanel (props-driven, no internal hooks) wired into both CreateServiceDialog (ServicesPage.tsx) and ServiceConfigEditor (Settings.tsx). Parent owns testResult + saveAnyway state; store-previous pattern resets on input change (avoids setState-in-effect). Create/Save button gated on testPassed || saveAnyway. 7 panel tests (button states, success/failure pills, checkbox toggle). All gates: 158 vitest, build exit 0, lint 0 errors, 362 backend pytest (regression).
This commit is contained in:
@@ -2,6 +2,7 @@ import { del, get, post, put } from "./shared";
|
|||||||
import type {
|
import type {
|
||||||
ServiceInstance,
|
ServiceInstance,
|
||||||
ServiceInstanceInput,
|
ServiceInstanceInput,
|
||||||
|
ServiceTestResult,
|
||||||
ServiceTypeInfo,
|
ServiceTypeInfo,
|
||||||
} from "../types";
|
} from "../types";
|
||||||
|
|
||||||
@@ -36,3 +37,9 @@ export async function deleteServiceInstance(
|
|||||||
): Promise<{ status: string }> {
|
): Promise<{ status: string }> {
|
||||||
return del<{ status: string }>(`/api/services/instances/${serviceId}`);
|
return del<{ status: string }>(`/api/services/instances/${serviceId}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export async function testServiceInstance(
|
||||||
|
input: ServiceInstanceInput,
|
||||||
|
): Promise<ServiceTestResult> {
|
||||||
|
return post<ServiceTestResult>("/api/services/test", input);
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,72 @@
|
|||||||
|
import { Alert, AlertDescription } from "@/components/ui/alert";
|
||||||
|
import { Button } from "@/components/ui/button";
|
||||||
|
import type { ServiceTestResult } from "../types";
|
||||||
|
|
||||||
|
interface Props {
|
||||||
|
/** Current test result (null = not tested yet). Parent clears this when the form input changes. */
|
||||||
|
result: ServiceTestResult | null;
|
||||||
|
/** Whether the test mutation is in-flight. */
|
||||||
|
isPending: boolean;
|
||||||
|
/** Whether the "Save anyway" checkbox is checked. */
|
||||||
|
saveAnyway: boolean;
|
||||||
|
/** Fired when the user clicks "Test credentials". */
|
||||||
|
onTest: () => void;
|
||||||
|
/** Fired when the "Save anyway" checkbox toggles. */
|
||||||
|
onSaveAnywayChange: (checked: boolean) => void;
|
||||||
|
/** Disable the Test button (e.g. no draft yet). */
|
||||||
|
disabled?: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Shared "Test credentials" panel used by both the add-service dialog and the
|
||||||
|
* edit-service panel. Purely presentational — the parent owns the test result
|
||||||
|
* + saveAnyway state and the mutation hook. This avoids setState-in-effect
|
||||||
|
* issues with clearing the result on input change (the parent uses the
|
||||||
|
* React-recommended "store previous prop" pattern instead).
|
||||||
|
*/
|
||||||
|
export function ServiceTestPanel({
|
||||||
|
result,
|
||||||
|
isPending,
|
||||||
|
saveAnyway,
|
||||||
|
onTest,
|
||||||
|
onSaveAnywayChange,
|
||||||
|
disabled,
|
||||||
|
}: Props) {
|
||||||
|
const testPassed = result?.ok === true;
|
||||||
|
|
||||||
|
return (
|
||||||
|
<div className="flex flex-col gap-2">
|
||||||
|
<Button
|
||||||
|
variant="outline"
|
||||||
|
size="sm"
|
||||||
|
onClick={onTest}
|
||||||
|
disabled={isPending || disabled}
|
||||||
|
className="mobile-touch-target"
|
||||||
|
>
|
||||||
|
{isPending ? "Testing…" : "Test credentials"}
|
||||||
|
</Button>
|
||||||
|
{result ? (
|
||||||
|
<Alert variant={result.ok ? "default" : "destructive"}>
|
||||||
|
<AlertDescription>
|
||||||
|
{result.ok
|
||||||
|
? `✓ Connected${result.evidence ? ` — ${result.evidence}` : ""}`
|
||||||
|
: `✗ ${result.detail}`}
|
||||||
|
</AlertDescription>
|
||||||
|
</Alert>
|
||||||
|
) : null}
|
||||||
|
<label className="flex items-center gap-2 text-xs text-muted-foreground">
|
||||||
|
<input
|
||||||
|
type="checkbox"
|
||||||
|
checked={saveAnyway}
|
||||||
|
onChange={(e) => onSaveAnywayChange(e.target.checked)}
|
||||||
|
/>
|
||||||
|
Save anyway (skip test)
|
||||||
|
</label>
|
||||||
|
{!testPassed && !saveAnyway ? (
|
||||||
|
<p className="text-xs text-muted-foreground">
|
||||||
|
Test credentials or check "Save anyway" to enable the save button.
|
||||||
|
</p>
|
||||||
|
) : null}
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -0,0 +1,115 @@
|
|||||||
|
import { describe, it, expect, vi } from "vitest";
|
||||||
|
import { render, screen, fireEvent } from "@testing-library/react";
|
||||||
|
import { ServiceTestPanel } from "../ServiceTestPanel";
|
||||||
|
import type { ServiceTestResult } from "../../types";
|
||||||
|
|
||||||
|
function noop() {}
|
||||||
|
|
||||||
|
describe("ServiceTestPanel", () => {
|
||||||
|
it("renders the Test credentials button", () => {
|
||||||
|
render(
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={null}
|
||||||
|
isPending={false}
|
||||||
|
saveAnyway={false}
|
||||||
|
onTest={noop}
|
||||||
|
onSaveAnywayChange={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(screen.getByText("Test credentials")).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows Testing… and disables button while pending", () => {
|
||||||
|
render(
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={null}
|
||||||
|
isPending={true}
|
||||||
|
saveAnyway={false}
|
||||||
|
onTest={noop}
|
||||||
|
onSaveAnywayChange={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(screen.getByText("Testing…")).toBeTruthy();
|
||||||
|
expect(screen.getByText("Testing…")).toBeDisabled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders green ✓ Connected pill with evidence on success", () => {
|
||||||
|
const result: ServiceTestResult = {
|
||||||
|
ok: true,
|
||||||
|
detail: "ok",
|
||||||
|
evidence: "v4.5.0",
|
||||||
|
};
|
||||||
|
render(
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={result}
|
||||||
|
isPending={false}
|
||||||
|
saveAnyway={false}
|
||||||
|
onTest={noop}
|
||||||
|
onSaveAnywayChange={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(screen.getByText(/✓ Connected — v4.5.0/)).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders red ✗ pill with detail on failure", () => {
|
||||||
|
const result: ServiceTestResult = {
|
||||||
|
ok: false,
|
||||||
|
detail: "Authentication failed",
|
||||||
|
evidence: null,
|
||||||
|
};
|
||||||
|
render(
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={result}
|
||||||
|
isPending={false}
|
||||||
|
saveAnyway={false}
|
||||||
|
onTest={noop}
|
||||||
|
onSaveAnywayChange={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(screen.getByText(/✗ Authentication failed/)).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("fires onTest when Test credentials is clicked", () => {
|
||||||
|
const onTest = vi.fn();
|
||||||
|
render(
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={null}
|
||||||
|
isPending={false}
|
||||||
|
saveAnyway={false}
|
||||||
|
onTest={onTest}
|
||||||
|
onSaveAnywayChange={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
fireEvent.click(screen.getByText("Test credentials"));
|
||||||
|
expect(onTest).toHaveBeenCalledOnce();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("fires onSaveAnywayChange when checkbox is toggled", () => {
|
||||||
|
const onSaveAnywayChange = vi.fn();
|
||||||
|
render(
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={null}
|
||||||
|
isPending={false}
|
||||||
|
saveAnyway={false}
|
||||||
|
onTest={noop}
|
||||||
|
onSaveAnywayChange={onSaveAnywayChange}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
const checkbox = screen.getByRole("checkbox");
|
||||||
|
fireEvent.click(checkbox);
|
||||||
|
expect(onSaveAnywayChange).toHaveBeenCalledWith(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders the Save anyway checkbox", () => {
|
||||||
|
render(
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={null}
|
||||||
|
isPending={false}
|
||||||
|
saveAnyway={false}
|
||||||
|
onTest={noop}
|
||||||
|
onSaveAnywayChange={noop}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(screen.getByRole("checkbox")).toBeTruthy();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -4,6 +4,7 @@ import {
|
|||||||
deleteServiceInstance,
|
deleteServiceInstance,
|
||||||
fetchServiceInstances,
|
fetchServiceInstances,
|
||||||
fetchServiceTypes,
|
fetchServiceTypes,
|
||||||
|
testServiceInstance,
|
||||||
updateServiceInstance,
|
updateServiceInstance,
|
||||||
} from "../api/services";
|
} from "../api/services";
|
||||||
import type { ServiceInstanceInput } from "../types";
|
import type { ServiceInstanceInput } from "../types";
|
||||||
@@ -45,3 +46,9 @@ export function useDeleteServiceInstance() {
|
|||||||
},
|
},
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function useTestServiceInstance() {
|
||||||
|
return useMutation({
|
||||||
|
mutationFn: (input: ServiceInstanceInput) => testServiceInstance(input),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ import {
|
|||||||
useDeleteServiceInstance,
|
useDeleteServiceInstance,
|
||||||
useSaveServiceInstance,
|
useSaveServiceInstance,
|
||||||
useServiceInstances,
|
useServiceInstances,
|
||||||
|
useTestServiceInstance,
|
||||||
} from "../hooks/useServices";
|
} from "../hooks/useServices";
|
||||||
import { useServiceTypes } from "../hooks/useServices";
|
import { useServiceTypes } from "../hooks/useServices";
|
||||||
import {
|
import {
|
||||||
@@ -41,12 +42,14 @@ import type {
|
|||||||
SecretFieldInfo,
|
SecretFieldInfo,
|
||||||
ServiceInstance,
|
ServiceInstance,
|
||||||
ServiceInstanceInput,
|
ServiceInstanceInput,
|
||||||
|
ServiceTestResult,
|
||||||
ServiceTypeInfo,
|
ServiceTypeInfo,
|
||||||
} from "../types";
|
} from "../types";
|
||||||
import { SectionCard } from "../components/SectionCard";
|
import { SectionCard } from "../components/SectionCard";
|
||||||
import { ConfirmDialog } from "../components/ConfirmDialog";
|
import { ConfirmDialog } from "../components/ConfirmDialog";
|
||||||
import { DialogFooter } from "../components/DialogFooter";
|
import { DialogFooter } from "../components/DialogFooter";
|
||||||
import { getServiceBinding } from "../integrations/registry";
|
import { getServiceBinding } from "../integrations/registry";
|
||||||
|
import { ServiceTestPanel } from "../components/ServiceTestPanel";
|
||||||
import { serviceLinkTarget } from "../components/PinnedServiceLink";
|
import { serviceLinkTarget } from "../components/PinnedServiceLink";
|
||||||
import type { NamedDashboardInput } from "../api/dashboards";
|
import type { NamedDashboardInput } from "../api/dashboards";
|
||||||
|
|
||||||
@@ -174,12 +177,48 @@ function CreateServiceDialog({
|
|||||||
}) {
|
}) {
|
||||||
const { data: types = [] } = useServiceTypes();
|
const { data: types = [] } = useServiceTypes();
|
||||||
const saveService = useSaveServiceInstance();
|
const saveService = useSaveServiceInstance();
|
||||||
|
const testService = useTestServiceInstance();
|
||||||
const [draft, setDraft] = useState<CreateDraft | null>(null);
|
const [draft, setDraft] = useState<CreateDraft | null>(null);
|
||||||
const [submitError, setSubmitError] = useState<string | null>(null);
|
const [submitError, setSubmitError] = useState<string | null>(null);
|
||||||
|
const [testResult, setTestResult] = useState<ServiceTestResult | null>(null);
|
||||||
|
const [saveAnyway, setSaveAnyway] = useState(false);
|
||||||
|
|
||||||
|
// Reset test state when draft changes (React "store previous" pattern —
|
||||||
|
// avoids setState-in-effect). Any field edit creates a new draft object.
|
||||||
|
const [prevDraft, setPrevDraft] = useState(draft);
|
||||||
|
if (draft !== prevDraft) {
|
||||||
|
setPrevDraft(draft);
|
||||||
|
setTestResult(null);
|
||||||
|
setSaveAnyway(false);
|
||||||
|
}
|
||||||
|
|
||||||
|
const testPassed = (testResult?.ok ?? false) || saveAnyway;
|
||||||
|
|
||||||
function reset() {
|
function reset() {
|
||||||
setDraft(null);
|
setDraft(null);
|
||||||
setSubmitError(null);
|
setSubmitError(null);
|
||||||
|
setTestResult(null);
|
||||||
|
setSaveAnyway(false);
|
||||||
|
}
|
||||||
|
|
||||||
|
async function handleTest() {
|
||||||
|
if (!draft) return;
|
||||||
|
try {
|
||||||
|
const res = await testService.mutateAsync({
|
||||||
|
service_type: draft.serviceType,
|
||||||
|
name: draft.name.trim(),
|
||||||
|
config: draft.config,
|
||||||
|
secrets: draft.secrets,
|
||||||
|
enabled: draft.enabled,
|
||||||
|
});
|
||||||
|
setTestResult(res);
|
||||||
|
} catch (err) {
|
||||||
|
setTestResult({
|
||||||
|
ok: false,
|
||||||
|
detail: err instanceof Error ? err.message : String(err),
|
||||||
|
evidence: null,
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async function save() {
|
async function save() {
|
||||||
@@ -196,7 +235,7 @@ function CreateServiceDialog({
|
|||||||
try {
|
try {
|
||||||
await saveService.mutateAsync(input);
|
await saveService.mutateAsync(input);
|
||||||
reset();
|
reset();
|
||||||
onClose();
|
onClose();
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
setSubmitError(err instanceof Error ? err.message : String(err));
|
setSubmitError(err instanceof Error ? err.message : String(err));
|
||||||
}
|
}
|
||||||
@@ -265,9 +304,16 @@ function CreateServiceDialog({
|
|||||||
onCheckedChange={(checked) =>
|
onCheckedChange={(checked) =>
|
||||||
setDraft({ ...draft, enabled: checked })
|
setDraft({ ...draft, enabled: checked })
|
||||||
}
|
}
|
||||||
/>
|
/>
|
||||||
<Label htmlFor="service-enabled">Enabled</Label>
|
<Label htmlFor="service-enabled">Enabled</Label>
|
||||||
</div>
|
</div>
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={testResult}
|
||||||
|
isPending={testService.isPending}
|
||||||
|
saveAnyway={saveAnyway}
|
||||||
|
onTest={handleTest}
|
||||||
|
onSaveAnywayChange={setSaveAnyway}
|
||||||
|
/>
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
@@ -281,7 +327,7 @@ function CreateServiceDialog({
|
|||||||
onCancel={reset}
|
onCancel={reset}
|
||||||
onConfirm={save}
|
onConfirm={save}
|
||||||
confirmLabel="Create service"
|
confirmLabel="Create service"
|
||||||
confirmDisabled={!draft.name.trim() || saveService.isPending}
|
confirmDisabled={!draft.name.trim() || saveService.isPending || !testPassed}
|
||||||
/>
|
/>
|
||||||
) : null}
|
) : null}
|
||||||
</DialogContent>
|
</DialogContent>
|
||||||
|
|||||||
@@ -55,10 +55,13 @@ import {
|
|||||||
useSaveServiceInstance,
|
useSaveServiceInstance,
|
||||||
useServiceInstances,
|
useServiceInstances,
|
||||||
useServiceTypes,
|
useServiceTypes,
|
||||||
|
useTestServiceInstance,
|
||||||
} from "../hooks/useServices";
|
} from "../hooks/useServices";
|
||||||
|
import { ServiceTestPanel } from "../components/ServiceTestPanel";
|
||||||
import type {
|
import type {
|
||||||
ServiceInstance,
|
ServiceInstance,
|
||||||
ServiceInstanceInput,
|
ServiceInstanceInput,
|
||||||
|
ServiceTestResult,
|
||||||
ServiceTypeInfo,
|
ServiceTypeInfo,
|
||||||
} from "../types";
|
} from "../types";
|
||||||
|
|
||||||
@@ -1435,6 +1438,7 @@ function ServiceConfigEditor({
|
|||||||
}) {
|
}) {
|
||||||
const saveService = useSaveServiceInstance();
|
const saveService = useSaveServiceInstance();
|
||||||
const deleteService = useDeleteServiceInstance();
|
const deleteService = useDeleteServiceInstance();
|
||||||
|
const testService = useTestServiceInstance();
|
||||||
const [name, setName] = useState(instance.name);
|
const [name, setName] = useState(instance.name);
|
||||||
const [enabled, setEnabled] = useState(instance.enabled);
|
const [enabled, setEnabled] = useState(instance.enabled);
|
||||||
const [draftConfig, setDraftConfig] = useState<Record<string, unknown>>({
|
const [draftConfig, setDraftConfig] = useState<Record<string, unknown>>({
|
||||||
@@ -1442,6 +1446,49 @@ function ServiceConfigEditor({
|
|||||||
});
|
});
|
||||||
const [draftSecrets, setDraftSecrets] = useState<Record<string, string>>({});
|
const [draftSecrets, setDraftSecrets] = useState<Record<string, string>>({});
|
||||||
const [deleteOpen, setDeleteOpen] = useState(false);
|
const [deleteOpen, setDeleteOpen] = useState(false);
|
||||||
|
const [testResult, setTestResult] = useState<ServiceTestResult | null>(null);
|
||||||
|
const [saveAnyway, setSaveAnyway] = useState(false);
|
||||||
|
|
||||||
|
// Build a test input from the current editor state. For the test, include
|
||||||
|
// ALL typed secrets (unfiltered) so the backend can authenticate.
|
||||||
|
function buildTestInput(): ServiceInstanceInput {
|
||||||
|
return {
|
||||||
|
id: instance.id,
|
||||||
|
service_type: instance.service_type,
|
||||||
|
name,
|
||||||
|
config: draftConfig,
|
||||||
|
secrets: draftSecrets,
|
||||||
|
enabled,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
const testInput = buildTestInput();
|
||||||
|
|
||||||
|
// Reset test state when the test input changes (store-previous pattern).
|
||||||
|
const [prevTestInput, setPrevTestInput] = useState(testInput);
|
||||||
|
if (
|
||||||
|
testInput !== prevTestInput &&
|
||||||
|
JSON.stringify(testInput) !== JSON.stringify(prevTestInput)
|
||||||
|
) {
|
||||||
|
setPrevTestInput(testInput);
|
||||||
|
setTestResult(null);
|
||||||
|
setSaveAnyway(false);
|
||||||
|
}
|
||||||
|
|
||||||
|
const testPassed = (testResult?.ok ?? false) || saveAnyway;
|
||||||
|
|
||||||
|
async function handleTest() {
|
||||||
|
try {
|
||||||
|
const res = await testService.mutateAsync(testInput);
|
||||||
|
setTestResult(res);
|
||||||
|
} catch (err) {
|
||||||
|
setTestResult({
|
||||||
|
ok: false,
|
||||||
|
detail: err instanceof Error ? err.message : String(err),
|
||||||
|
evidence: null,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const properties =
|
const properties =
|
||||||
(
|
(
|
||||||
@@ -1560,10 +1607,18 @@ function ServiceConfigEditor({
|
|||||||
</FormField>
|
</FormField>
|
||||||
))}
|
))}
|
||||||
|
|
||||||
|
<ServiceTestPanel
|
||||||
|
result={testResult}
|
||||||
|
isPending={testService.isPending}
|
||||||
|
saveAnyway={saveAnyway}
|
||||||
|
onTest={handleTest}
|
||||||
|
onSaveAnywayChange={setSaveAnyway}
|
||||||
|
/>
|
||||||
|
|
||||||
<div className="flex justify-between">
|
<div className="flex justify-between">
|
||||||
<Button
|
<Button
|
||||||
onClick={handleSave}
|
onClick={handleSave}
|
||||||
disabled={saveService.isPending}
|
disabled={saveService.isPending || !testPassed}
|
||||||
className="mobile-touch-target"
|
className="mobile-touch-target"
|
||||||
>
|
>
|
||||||
Save
|
Save
|
||||||
|
|||||||
@@ -473,6 +473,12 @@ export interface ServiceInstanceInput {
|
|||||||
enabled: boolean;
|
enabled: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export interface ServiceTestResult {
|
||||||
|
ok: boolean;
|
||||||
|
detail: string;
|
||||||
|
evidence: string | null;
|
||||||
|
}
|
||||||
|
|
||||||
export interface BuiltinWidgetKindInfo {
|
export interface BuiltinWidgetKindInfo {
|
||||||
kind: string;
|
kind: string;
|
||||||
name: string;
|
name: string;
|
||||||
|
|||||||
Reference in New Issue
Block a user