Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions app/e2e/connections.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,63 @@ test.describe("Connections", () => {
await expect(dialog).not.toBeVisible();
});

test("should rename a connection from the edit dialog (#1043)", async ({
page,
}) => {
const name = `Rename Me ${Date.now()}`;
const renamed = `Renamed ${Date.now()}`;
// Create a connection to rename.
await page.getByRole("button", { name: "Add Connection" }).click();
let dialog = page.getByRole("dialog");
await dialog.getByTestId("pick-neo4j").click();
await dialog.locator("#conn-name").fill(name);
await dialog.locator("#conn-uri").fill(TEST_NEO4J_BOLT_URL);
await dialog.locator("#conn-username").fill("neo4j");
await dialog.locator("#conn-password").fill("neoboard123");
await dialog.getByRole("button", { name: "Create" }).click();
await expect(page.getByText(name)).toBeVisible();

// Open its edit dialog and change the Name field. Scope to the smallest
// card div that holds both this connection's name and its actions button.
const card = page
.locator("div")
.filter({ has: page.getByText(name, { exact: true }) })
.filter({ has: page.getByRole("button", { name: "Connection actions" }) })
.last();
await card.getByRole("button", { name: "Connection actions" }).click();
await page.getByRole("menuitem", { name: /Edit/ }).click();
dialog = page.getByRole("dialog");
await expect(dialog.locator("#edit-name")).toHaveValue(name, {
timeout: 5000,
});
await dialog.locator("#edit-name").fill(renamed);
await dialog.getByRole("button", { name: "Save" }).click();

// The new name appears and a confirmation toast fires (#1043). Use exact
// match to avoid the aria-live "Notification …" status span.
await expect(
page.getByText("Connection updated", { exact: true }),
).toBeVisible({ timeout: 10000 });
await expect(page.getByText(renamed)).toBeVisible();
});

test("blocks save of a malformed URI with an inline error (#1043)", async ({
page,
}) => {
await page.getByRole("button", { name: "Add Connection" }).click();
const dialog = page.getByRole("dialog");
await dialog.getByTestId("pick-neo4j").click();
await dialog.locator("#conn-name").fill("Bad URI");
await dialog.locator("#conn-uri").fill("not-a-uri");
await dialog.locator("#conn-username").fill("neo4j");
await dialog.locator("#conn-password").fill("pw");
await dialog.getByRole("button", { name: "Create" }).click();

// Inline error shown; dialog stays open (no Error-badge connection saved).
await expect(dialog.getByText(/valid URI/i)).toBeVisible({ timeout: 5000 });
await expect(dialog).toBeVisible();
});

test("should delete a connection with confirmation", async ({ page }) => {
const name = `To Delete ${Date.now()}`;
// Create one first
Expand Down
70 changes: 65 additions & 5 deletions app/src/app/(dashboard)/connections/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,16 @@ import {
PasswordInput,
Alert,
AlertDescription,
useToast,
} from "@neoboard/components";
import type { ConnectionState } from "@neoboard/components";
import {
type ConnectorType,
CONNECTOR_LABELS,
} from "@/lib/connector/connector-types";
import { hintForConnectionErrorCode } from "@/lib/connector/connection-error-classifier";
import { validateConnectionUri } from "@/lib/connector/validate-connection-uri";
import { missingRequiredConnectionFields } from "@/lib/connector/connection-form-validation";
import {
parseOptionalInt,
mapConfigToEditForm,
Expand Down Expand Up @@ -69,6 +72,7 @@ const DEFAULT_FORM = {

export default function ConnectionsPage() {
const { data: session } = useSession();
const { toast } = useToast();
const isAdmin = session?.user?.role === "admin";
const { data: connections, isLoading } = useConnections();
const createConnection = useCreateConnection();
Expand Down Expand Up @@ -244,6 +248,20 @@ export default function ConnectionsPage() {
async function handleCreate(e: React.FormEvent) {
e.preventDefault();
setCreateError(null);
// Report all missing required fields at once via a styled inline alert
// (form is noValidate) instead of one native browser tooltip at a time
// (#1043).
const missing = missingRequiredConnectionFields(form);
if (missing.length > 0) {
setCreateError(`Please fill in: ${missing.join(", ")}.`);
return;
}
// Validate URI format client-side before save (#1043).
const uriError = validateConnectionUri(form.uri, form.type);
if (uriError) {
setCreateError(uriError);
return;
}
try {
const newConn = await createConnection.mutateAsync({
name: form.name,
Expand Down Expand Up @@ -370,12 +388,16 @@ export default function ConnectionsPage() {
function buildEditConfig() {
// Only include credential fields when the user has explicitly filled them in.
// Omitting them (undefined) tells the server to keep the existing stored values
// rather than overwriting them with blank strings.
// rather than overwriting them with blank strings. Gate on the *trimmed*
// value so whitespace-only input (possible now the form is noValidate)
// doesn't clobber stored credentials (#1043).
const uri = editForm.uri.trim();
const username = editForm.username.trim();
return {
...(editForm.uri ? { uri: editForm.uri } : {}),
...(editForm.username ? { username: editForm.username } : {}),
...(editForm.password ? { password: editForm.password } : {}),
database: editForm.database || undefined,
...(uri ? { uri } : {}),
...(username ? { username } : {}),
...(editForm.password.trim() ? { password: editForm.password } : {}),
database: editForm.database.trim() || undefined,
connectionTimeout: parseOptionalInt(editForm.connectionTimeout),
queryTimeout: parseOptionalInt(editForm.queryTimeout),
maxPoolSize: parseOptionalInt(editForm.maxPoolSize),
Expand All @@ -394,12 +416,28 @@ export default function ConnectionsPage() {
if (!editTarget) return;
setEditError(null);

if (!editForm.name.trim()) {
setEditError("Name is required.");
return;
}
// Validate URI *format* before save when the user changed it (blank keeps
// the existing one). Catches malformed URIs client-side (#1043).
if (editForm.uri.trim()) {
const uriError = validateConnectionUri(editForm.uri, editTarget.type);
if (uriError) {
setEditError(uriError);
return;
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

try {
await updateConnection.mutateAsync({
id: editTarget.id,
name: editForm.name.trim(),
config: buildEditConfig(),
});
setEditTarget(null);
toast({ title: "Connection updated" });
handleTest(editTarget.id);
} catch (error) {
setEditError(
Expand Down Expand Up @@ -473,6 +511,7 @@ export default function ConnectionsPage() {
) : (
<form
onSubmit={handleCreate}
noValidate
className="flex min-h-0 flex-col overflow-hidden"
>
<DialogHeader>
Expand Down Expand Up @@ -758,6 +797,7 @@ export default function ConnectionsPage() {
<DialogContent className="flex flex-col overflow-hidden">
<form
onSubmit={handleEdit}
noValidate
className="flex min-h-0 flex-col overflow-hidden"
>
<DialogHeader>
Expand All @@ -774,6 +814,19 @@ export default function ConnectionsPage() {
the existing one.
</p>

<div className="space-y-2">
<Label htmlFor="edit-name">Name</Label>
<Input
id="edit-name"
value={editForm.name}
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
setEditForm((f) => ({ ...f, name: e.target.value }))
}
required
placeholder="My database"
/>
</div>

<div className="space-y-2">
<Label htmlFor="edit-uri">URI</Label>
<Input
Expand Down Expand Up @@ -1164,6 +1217,13 @@ export default function ConnectionsPage() {
<ConnectionCard
name={c.name}
host={c.type}
icon={
c.type === "neo4j" ? (
<Neo4jLogo className="h-5 w-5" />
) : (
<PostgreSQLLogo className="h-5 w-5" />
)
}
status={status}
statusText={testErrors[c.id]}
onClick={
Expand Down
44 changes: 44 additions & 0 deletions app/src/app/api/connections/[id]/test/__tests__/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,50 @@ describe("POST /api/connections/[id]/test", () => {
const body = await res.json();
expect(body.data.success).toBe(false);
expect(body.data.error).toBe("Connection refused");
// The error is classified so the UI can show a targeted hint (#1043).
expect(body.data.code).toBe("network");
});

it("classifies an auth failure thrown by testConnection (#1043)", async () => {
mockRequireSession.mockResolvedValue(SESSION);
mockDb.select.mockReturnValue(
makeSelectChain([
{ id: "c1", userId: "user-1", type: "neo4j", configEncrypted: "enc" },
]),
);
mockDecryptJson.mockReturnValue({ uri: "bolt://h", username: "u" });
mockTestConnection.mockRejectedValue(
new Error("The client is unauthorized due to authentication failure."),
);

const res = await POST({} as Request, makeParams("c1"));
const body = await res.json();
expect(body.data.success).toBe(false);
expect(body.data.code).toBe("auth_failed");
});

it("returns an actionable message + code when testConnection returns false (#1043)", async () => {
mockRequireSession.mockResolvedValue(SESSION);
mockDb.select.mockReturnValue(
makeSelectChain([
{
id: "c1",
userId: "user-1",
type: "postgresql",
configEncrypted: "enc",
},
]),
);
mockDecryptJson.mockReturnValue({ uri: "pg://h", username: "u" });
mockTestConnection.mockResolvedValue(false);

const res = await POST({} as Request, makeParams("c1"));
const body = await res.json();
expect(body.data.success).toBe(false);
expect(body.data.code).toBe("unknown");
// No longer the dead-end "Connection check returned false".
expect(body.data.error).not.toMatch(/check returned false/i);
expect(body.data.error).toMatch(/verify the host, port, credentials/i);
});

// Lost/rotated ENCRYPTION_KEY is a documented operational failure mode —
Expand Down
27 changes: 10 additions & 17 deletions app/src/app/api/connections/[id]/test/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import { decryptJson } from "@/lib/crypto/crypto";
import { testConnection } from "@/lib/query/query-executor";
import type { ConnectionCredentials, DbType } from "@/lib/query/query-executor";
import { apiSuccess } from "@/lib/api/api-response";
import { notFound, handleRouteError } from "@/lib/api/api-utils";
import {
notFound,
handleRouteError,
sanitizeErrorMessage,
} from "@/lib/api/api-utils";
connectionCheckFalseResult,
connectionTestErrorResult,
} from "@/lib/connector/connection-test-result";

export async function POST(
_request: Request,
Expand Down Expand Up @@ -54,20 +54,13 @@ export async function POST(
connection.type as DbType,
credentials,
);
return apiSuccess({
success,
...(!success ? { error: "Connection check returned false" } : {}),
});
} catch (testError) {
const rawMessage =
testError instanceof Error
? testError.message
: "Connection test failed";
const message = sanitizeErrorMessage(
rawMessage,
"Connection test failed",
// A false result (no throw) gets an actionable fallback; a thrown error
// is classified for a targeted hint. Both via the shared helper (#1043).
return apiSuccess(
success ? { success: true } : connectionCheckFalseResult(),
);
return apiSuccess({ success: false, error: message });
} catch (testError) {
return apiSuccess(connectionTestErrorResult(testError));
}
} catch (error) {
return handleRouteError(error, "Connection test failed");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe("POST /api/connections/test-inline", () => {
);
});

it("returns success:false when testConnection returns false", async () => {
it("returns success:false with actionable message + code when testConnection returns false (#1043)", async () => {
mockRequireSession.mockResolvedValue(SESSION);
mockTestConnection.mockResolvedValue(false);
const res = await POST(
Expand All @@ -196,6 +196,9 @@ describe("POST /api/connections/test-inline", () => {
expect(res.status).toBe(200);
const body = await res.json();
expect(body.data.success).toBe(false);
expect(body.data.code).toBe("unknown");
expect(body.data.error).not.toMatch(/check returned false/i);
expect(body.data.error).toMatch(/verify the host, port, credentials/i);
});

it("returns success:false with fallback message for non-Error throws", async () => {
Expand Down
31 changes: 10 additions & 21 deletions app/src/app/api/connections/test-inline/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import { testConnection } from "@/lib/query/query-executor";
import type { DbType } from "@/lib/query/query-executor";
import { testInlineSchema } from "@/lib/shared/schemas";
import { apiSuccess } from "@/lib/api/api-response";
import { handleRouteError, validateBody } from "@/lib/api/api-utils";
import {
handleRouteError,
validateBody,
sanitizeErrorMessage,
} from "@/lib/api/api-utils";
import { classifyConnectionError } from "@/lib/connector/connection-error-classifier";
connectionCheckFalseResult,
connectionTestErrorResult,
} from "@/lib/connector/connection-test-result";

export async function POST(request: Request) {
try {
Expand Down Expand Up @@ -38,23 +37,13 @@ export async function POST(request: Request) {
statementTimeout: config.statementTimeout,
sslRejectUnauthorized: config.sslRejectUnauthorized,
});
return apiSuccess({
success,
...(!success ? { error: "Connection check returned false" } : {}),
});
} catch (testError) {
const rawMessage =
testError instanceof Error
? testError.message
: "Connection test failed";
// Classify BEFORE sanitization — the classifier needs the raw driver
// text to bucket reliably; the sanitizer strips that detail for display.
const code = classifyConnectionError(rawMessage);
const message = sanitizeErrorMessage(
rawMessage,
"Connection test failed",
// Shared helper builds the false/thrown result identically to the
// [id]/test route (#1043).
return apiSuccess(
success ? { success: true } : connectionCheckFalseResult(),
);
return apiSuccess({ success: false, code, error: message });
} catch (testError) {
return apiSuccess(connectionTestErrorResult(testError));
}
} catch (error) {
return handleRouteError(error, "Connection test failed");
Expand Down
Loading
Loading