diff --git a/app/e2e/connections.spec.ts b/app/e2e/connections.spec.ts index 15c5f980..b0db15d2 100644 --- a/app/e2e/connections.spec.ts +++ b/app/e2e/connections.spec.ts @@ -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 diff --git a/app/src/app/(dashboard)/connections/page.tsx b/app/src/app/(dashboard)/connections/page.tsx index abe67dcb..13511c61 100644 --- a/app/src/app/(dashboard)/connections/page.tsx +++ b/app/src/app/(dashboard)/connections/page.tsx @@ -35,6 +35,7 @@ import { PasswordInput, Alert, AlertDescription, + useToast, } from "@neoboard/components"; import type { ConnectionState } from "@neoboard/components"; import { @@ -42,6 +43,8 @@ import { 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, @@ -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(); @@ -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, @@ -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), @@ -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; + } + } + 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( @@ -473,6 +511,7 @@ export default function ConnectionsPage() { ) : (
@@ -758,6 +797,7 @@ export default function ConnectionsPage() { @@ -774,6 +814,19 @@ export default function ConnectionsPage() { the existing one.

+
+ + ) => + setEditForm((f) => ({ ...f, name: e.target.value })) + } + required + placeholder="My database" + /> +
+
+ ) : ( + + ) + } status={status} statusText={testErrors[c.id]} onClick={ diff --git a/app/src/app/api/connections/[id]/test/__tests__/route.test.ts b/app/src/app/api/connections/[id]/test/__tests__/route.test.ts index c2e48948..3b75bc0e 100644 --- a/app/src/app/api/connections/[id]/test/__tests__/route.test.ts +++ b/app/src/app/api/connections/[id]/test/__tests__/route.test.ts @@ -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 — diff --git a/app/src/app/api/connections/[id]/test/route.ts b/app/src/app/api/connections/[id]/test/route.ts index 7847c67f..4a56938b 100644 --- a/app/src/app/api/connections/[id]/test/route.ts +++ b/app/src/app/api/connections/[id]/test/route.ts @@ -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, @@ -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"); diff --git a/app/src/app/api/connections/test-inline/__tests__/route.test.ts b/app/src/app/api/connections/test-inline/__tests__/route.test.ts index d6cf5d17..34c62f87 100644 --- a/app/src/app/api/connections/test-inline/__tests__/route.test.ts +++ b/app/src/app/api/connections/test-inline/__tests__/route.test.ts @@ -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( @@ -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 () => { diff --git a/app/src/app/api/connections/test-inline/route.ts b/app/src/app/api/connections/test-inline/route.ts index 3c995abe..351b7258 100644 --- a/app/src/app/api/connections/test-inline/route.ts +++ b/app/src/app/api/connections/test-inline/route.ts @@ -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 { @@ -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"); diff --git a/app/src/components/widget-editor/widget-preview-panel.tsx b/app/src/components/widget-editor/widget-preview-panel.tsx index 89f931df..1c368c79 100644 --- a/app/src/components/widget-editor/widget-preview-panel.tsx +++ b/app/src/components/widget-editor/widget-preview-panel.tsx @@ -4,6 +4,7 @@ import type { RefObject } from "react"; import { AlertCircle, Play } from "lucide-react"; import { CardContainer } from "../card-container"; import { ParameterPreview } from "./parameter-preview"; +import { mapPreviewError } from "@/lib/query/preview-error"; import type { StylingConfig } from "@/lib/db/schema"; import type { Transform } from "@/lib/query/data-transforms"; import type { ParamUIType, DateSubType } from "@/stores/widget-editor-store"; @@ -164,34 +165,53 @@ function renderChart(props: {
)} {previewQuery.isError && !previewQuery.data && !initialPreviewData ? ( -
- -

Query failed

-

- {previewQuery.error?.message} + (() => { + // Map blocked-write driver errors to a clear message (#1043). + const writeMsg = mapPreviewError(previewQuery.error?.message); + return ( +

+ +

+ {writeMsg ? "Writes not allowed" : "Query failed"} +

+

+ {writeMsg ?? previewQuery.error?.message} +

+
+ ); + })() + ) : previewQuery.data || initialPreviewData ? ( +
+
+ +
+ {/* The preview query is capped server-side; surface the silent + LIMIT so authors don't mistake it for the full result (#1043). */} +

+ Preview shows up to 25 rows

- ) : previewQuery.data || initialPreviewData ? ( - ) : connectionId && query.trim() && !previewQuery.isError ? (
diff --git a/app/src/lib/connector/__tests__/connection-form-validation.test.ts b/app/src/lib/connector/__tests__/connection-form-validation.test.ts new file mode 100644 index 00000000..2f5eabc0 --- /dev/null +++ b/app/src/lib/connector/__tests__/connection-form-validation.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect } from "vitest"; +import { missingRequiredConnectionFields } from "../connection-form-validation"; + +const FULL = { + name: "DB", + uri: "bolt://localhost:7687", + username: "neo4j", + password: "pw", +}; + +describe("missingRequiredConnectionFields (#1043)", () => { + it("returns no missing fields when all are filled", () => { + expect(missingRequiredConnectionFields(FULL)).toEqual([]); + }); + + it("lists every missing required field at once", () => { + expect( + missingRequiredConnectionFields({ + name: "", + uri: "", + username: "", + password: "", + }), + ).toEqual(["Name", "URI", "Username", "Password"]); + }); + + it("treats whitespace-only values as missing", () => { + expect(missingRequiredConnectionFields({ ...FULL, name: " " })).toEqual([ + "Name", + ]); + }); +}); diff --git a/app/src/lib/connector/__tests__/connection-test-result.test.ts b/app/src/lib/connector/__tests__/connection-test-result.test.ts new file mode 100644 index 00000000..5e6f7a9a --- /dev/null +++ b/app/src/lib/connector/__tests__/connection-test-result.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from "vitest"; +import { + connectionCheckFalseResult, + connectionTestErrorResult, +} from "../connection-test-result"; + +describe("connection-test-result (#1043)", () => { + it("builds an actionable false result with code unknown", () => { + const r = connectionCheckFalseResult(); + expect(r.success).toBe(false); + expect(r.code).toBe("unknown"); + expect(r.error).not.toMatch(/check returned false/i); + expect(r.error).toMatch(/verify the host, port, credentials/i); + }); + + it("classifies a thrown network error", () => { + const r = connectionTestErrorResult(new Error("connect ECONNREFUSED")); + expect(r.success).toBe(false); + expect(r.code).toBe("network"); + expect(r.error).toBeTruthy(); + }); + + it("classifies a thrown auth error", () => { + const r = connectionTestErrorResult( + new Error("password authentication failed for user"), + ); + expect(r.code).toBe("auth_failed"); + }); + + it("falls back for a non-Error throw", () => { + const r = connectionTestErrorResult("boom"); + expect(r.success).toBe(false); + expect(r.code).toBe("unknown"); + }); +}); diff --git a/app/src/lib/connector/__tests__/validate-connection-uri.test.ts b/app/src/lib/connector/__tests__/validate-connection-uri.test.ts new file mode 100644 index 00000000..1bb6d775 --- /dev/null +++ b/app/src/lib/connector/__tests__/validate-connection-uri.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from "vitest"; +import { validateConnectionUri } from "../validate-connection-uri"; + +describe("validateConnectionUri (#1043)", () => { + it("rejects a non-URI string", () => { + expect(validateConnectionUri("not-a-uri", "neo4j")).toMatch(/valid URI/i); + expect(validateConnectionUri("not-a-uri", "postgresql")).toMatch( + /valid URI/i, + ); + }); + + it("rejects an empty URI", () => { + expect(validateConnectionUri(" ", "neo4j")).toMatch(/required/i); + }); + + it("rejects a wrong scheme for the connector type", () => { + expect( + validateConnectionUri("postgresql://localhost:5432", "neo4j"), + ).toMatch(/scheme/i); + expect( + validateConnectionUri("bolt://localhost:7687", "postgresql"), + ).toMatch(/scheme/i); + }); + + it("accepts valid Neo4j schemes", () => { + expect(validateConnectionUri("bolt://localhost:7687", "neo4j")).toBeNull(); + expect(validateConnectionUri("neo4j+s://host.example", "neo4j")).toBeNull(); + }); + + it("accepts valid PostgreSQL schemes", () => { + expect( + validateConnectionUri("postgresql://localhost:5432/db", "postgresql"), + ).toBeNull(); + expect( + validateConnectionUri("postgres://user@host:5432/db", "postgresql"), + ).toBeNull(); + }); + + it("rejects a URI with no host", () => { + expect(validateConnectionUri("bolt://", "neo4j")).toMatch( + /valid URI|host/i, + ); + }); +}); diff --git a/app/src/lib/connector/connection-error-classifier.ts b/app/src/lib/connector/connection-error-classifier.ts index 9baf2772..719292be 100644 --- a/app/src/lib/connector/connection-error-classifier.ts +++ b/app/src/lib/connector/connection-error-classifier.ts @@ -17,6 +17,14 @@ export type ConnectionErrorCode = | "bad_uri" | "unknown"; +/** + * Shown when a connector's check returns false *without* throwing — there's no + * driver message to classify, so the old "Connection check returned false" was + * a dead end. This points at the knobs to check instead (#1043). + */ +export const CONNECTION_CHECK_FALSE_MESSAGE = + "The database rejected the connection check without reporting why. Verify the host, port, credentials, and that the database is running and reachable."; + // Keyword lists are lowercased; the matcher lowercases input once. const BAD_URI_KEYWORDS = [ "invalid uri", diff --git a/app/src/lib/connector/connection-form-validation.ts b/app/src/lib/connector/connection-form-validation.ts new file mode 100644 index 00000000..c3abffe2 --- /dev/null +++ b/app/src/lib/connector/connection-form-validation.ts @@ -0,0 +1,24 @@ +/** + * Required-field check for the connection create form (#1043). + * + * Returns the labels of all missing required fields so the dialog can show + * them together in one styled inline alert, instead of the native browser + * tooltip surfacing them one at a time. + */ +export interface RequiredConnectionFields { + name: string; + uri: string; + username: string; + password: string; +} + +export function missingRequiredConnectionFields( + fields: RequiredConnectionFields, +): string[] { + const missing: string[] = []; + if (!fields.name.trim()) missing.push("Name"); + if (!fields.uri.trim()) missing.push("URI"); + if (!fields.username.trim()) missing.push("Username"); + if (!fields.password.trim()) missing.push("Password"); + return missing; +} diff --git a/app/src/lib/connector/connection-test-result.ts b/app/src/lib/connector/connection-test-result.ts new file mode 100644 index 00000000..2e5c4af5 --- /dev/null +++ b/app/src/lib/connector/connection-test-result.ts @@ -0,0 +1,38 @@ +import { sanitizeErrorMessage } from "@/lib/api/api-utils"; +import { + classifyConnectionError, + CONNECTION_CHECK_FALSE_MESSAGE, + type ConnectionErrorCode, +} from "@/lib/connector/connection-error-classifier"; + +/** + * Shared shape of a connection-test API result, so the `[id]/test` and + * `test-inline` routes build it identically (#1043) — they previously + * duplicated the false/catch handling. + */ +export interface ConnectionTestResult { + success: boolean; + code?: ConnectionErrorCode; + error?: string; +} + +/** A driver check that returned false without throwing — no message to classify. */ +export function connectionCheckFalseResult(): ConnectionTestResult { + return { + success: false, + code: "unknown", + error: CONNECTION_CHECK_FALSE_MESSAGE, + }; +} + +/** A thrown driver error — classify for a targeted hint, then sanitize for display. */ +export function connectionTestErrorResult( + thrown: unknown, +): ConnectionTestResult { + const rawMessage = + thrown instanceof Error ? thrown.message : "Connection test failed"; + // Classify BEFORE sanitization — the classifier needs the raw driver text. + const code = classifyConnectionError(rawMessage); + const error = sanitizeErrorMessage(rawMessage, "Connection test failed"); + return { success: false, code, error }; +} diff --git a/app/src/lib/connector/validate-connection-uri.ts b/app/src/lib/connector/validate-connection-uri.ts new file mode 100644 index 00000000..9bce8dfa --- /dev/null +++ b/app/src/lib/connector/validate-connection-uri.ts @@ -0,0 +1,47 @@ +import type { ConnectorType } from "@/lib/connector/connector-types"; + +/** + * Client-side URI *format* validation for the connection dialog (#1043). + * + * Saving an unreachable connection is intentional, but a malformed URI like + * `not-a-uri` should be caught before save instead of persisting as an + * Error-badge connection. This checks shape only (parseable, expected scheme, + * has a host) — it never attempts a network connection. + * + * Returns null when the URI is well-formed, otherwise an actionable message. + */ +const SCHEMES: Record = { + neo4j: ["bolt:", "bolt+s:", "bolt+ssc:", "neo4j:", "neo4j+s:", "neo4j+ssc:"], + postgresql: ["postgres:", "postgresql:"], +}; + +export function validateConnectionUri( + uri: string, + type: ConnectorType, +): string | null { + const trimmed = uri.trim(); + if (!trimmed) return "URI is required."; + + let parsed: URL; + try { + parsed = new URL(trimmed); + } catch { + return type === "neo4j" + ? "Enter a valid URI, e.g. bolt://localhost:7687 or neo4j+s://host." + : "Enter a valid URI, e.g. postgresql://localhost:5432/db."; + } + + if (!parsed.hostname) { + return "The URI is missing a host."; + } + + const allowed = SCHEMES[type]; + if (allowed && !allowed.includes(parsed.protocol)) { + return `Unexpected scheme "${parsed.protocol.replace( + ":", + "", + )}". Use one of: ${allowed.map((s) => s.replace(":", "")).join(", ")}.`; + } + + return null; +} diff --git a/app/src/lib/query/__tests__/preview-error.test.ts b/app/src/lib/query/__tests__/preview-error.test.ts new file mode 100644 index 00000000..1db5851e --- /dev/null +++ b/app/src/lib/query/__tests__/preview-error.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from "vitest"; +import { + mapPreviewError, + PREVIEW_WRITE_NOT_ALLOWED_MESSAGE, +} from "../preview-error"; + +describe("mapPreviewError (#1043)", () => { + it("maps a PostgreSQL wrapped-write syntax error to the write message", () => { + // DELETE wrapped as SELECT * FROM (DELETE …) AS __preview + expect(mapPreviewError('syntax error at or near "DELETE"')).toBe( + PREVIEW_WRITE_NOT_ALLOWED_MESSAGE, + ); + expect(mapPreviewError('syntax error at or near "UPDATE"')).toBe( + PREVIEW_WRITE_NOT_ALLOWED_MESSAGE, + ); + expect(mapPreviewError('syntax error at or near "INSERT"')).toBe( + PREVIEW_WRITE_NOT_ALLOWED_MESSAGE, + ); + }); + + it("maps a Neo4j read-access-mode write error to the write message", () => { + expect( + mapPreviewError( + "Neo.ClientError.Request.Invalid: Writing in read access mode not allowed.", + ), + ).toBe(PREVIEW_WRITE_NOT_ALLOWED_MESSAGE); + }); + + it("maps a PostgreSQL read-only transaction violation to the write message", () => { + expect( + mapPreviewError("cannot execute DELETE in a read-only transaction"), + ).toBe(PREVIEW_WRITE_NOT_ALLOWED_MESSAGE); + }); + + it("returns null for a genuine (non-write) syntax error so the raw message shows", () => { + expect(mapPreviewError('syntax error at or near "FROMM"')).toBeNull(); + }); + + it("returns null for an unrelated error", () => { + expect(mapPreviewError('column "foo" does not exist')).toBeNull(); + }); + + it("returns null for empty/undefined input", () => { + expect(mapPreviewError(undefined)).toBeNull(); + expect(mapPreviewError("")).toBeNull(); + }); +}); diff --git a/app/src/lib/query/preview-error.ts b/app/src/lib/query/preview-error.ts new file mode 100644 index 00000000..b15dace0 --- /dev/null +++ b/app/src/lib/query/preview-error.ts @@ -0,0 +1,65 @@ +/** + * Map a raw preview-query error into a clear, user-facing message (#1043). + * + * Widget previews run through the read-only query route, and non-Form widget + * queries are wrapped with a preview LIMIT. A write statement therefore fails + * in one of two confusing ways: + * + * - PostgreSQL: `DELETE …` wrapped as `SELECT * FROM (DELETE …) AS __preview` + * reports `syntax error at or near "DELETE"` — driver-speak that hides the + * real cause. + * - Neo4j: `CREATE …` runs in read access mode and reports + * `Writing in read access mode not allowed`. + * + * Both really mean the same thing: you can't write from a widget query. Detect + * those shapes and return a single actionable message; otherwise return null so + * the caller shows the original error. + */ + +const WRITE_KEYWORDS = [ + "insert", + "update", + "delete", + "merge", + "create", + "drop", + "alter", + "truncate", + "set ", + "remove ", +]; + +const READ_ONLY_PHRASES = [ + "writing in read access mode not allowed", + "write operations are not allowed", + "read-only transaction", + "read only transaction", + "cannot execute", +]; + +/** True when a wrapped write produced a "syntax error at or near ". */ +function isWrappedWriteSyntaxError(lower: string): boolean { + const m = /syntax error at or near "([a-z]+)"/.exec(lower); + if (!m) return false; + return WRITE_KEYWORDS.some((k) => k.trim() === m[1]); +} + +export const PREVIEW_WRITE_NOT_ALLOWED_MESSAGE = + "Writes aren't allowed from widget queries. Widget previews run read-only — use a Form widget to write to the database."; + +/** + * Returns the friendly write-not-allowed message when the raw error looks like + * a blocked write attempt, otherwise null. + */ +export function mapPreviewError(rawMessage: string | undefined): string | null { + if (!rawMessage) return null; + const lower = rawMessage.toLowerCase(); + + if (READ_ONLY_PHRASES.some((p) => lower.includes(p))) { + return PREVIEW_WRITE_NOT_ALLOWED_MESSAGE; + } + if (isWrappedWriteSyntaxError(lower)) { + return PREVIEW_WRITE_NOT_ALLOWED_MESSAGE; + } + return null; +} diff --git a/component/src/components/composed/__tests__/connection-card.test.tsx b/component/src/components/composed/__tests__/connection-card.test.tsx index e4010671..1b1a1b7e 100644 --- a/component/src/components/composed/__tests__/connection-card.test.tsx +++ b/component/src/components/composed/__tests__/connection-card.test.tsx @@ -35,6 +35,16 @@ describe("ConnectionCard", () => { expect(container.querySelector("svg")).toBeInTheDocument(); }); + it("renders a custom connector-type icon when provided (#1043)", () => { + render( + } + />, + ); + expect(screen.getByTestId("neo4j-logo")).toBeInTheDocument(); + }); + it("applies active border when active", () => { const { container } = render(); expect(container.firstChild).toHaveClass("border-primary"); @@ -42,7 +52,7 @@ describe("ConnectionCard", () => { it("applies cursor-pointer when onClick is provided", () => { const { container } = render( - + , ); expect(container.firstChild).toHaveClass("cursor-pointer"); }); @@ -56,30 +66,38 @@ describe("ConnectionCard", () => { it("renders dropdown menu when action handlers are provided", () => { render(); - expect(screen.getByRole("button", { name: /connection actions/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /connection actions/i }), + ).toBeInTheDocument(); }); it("does not render dropdown when no action handlers", () => { render(); - expect(screen.queryByRole("button", { name: /connection actions/i })).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /connection actions/i }), + ).not.toBeInTheDocument(); }); it("applies custom className", () => { const { container } = render( - + , ); expect(container.firstChild).toHaveClass("custom-card"); }); it("renders actions dropdown when onDuplicate is provided", () => { render(); - expect(screen.getByRole("button", { name: /connection actions/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /connection actions/i }), + ).toBeInTheDocument(); }); it("renders Duplicate menu item when onDuplicate is provided", async () => { const user = userEvent.setup(); render(); - await user.click(screen.getByRole("button", { name: /connection actions/i })); + await user.click( + screen.getByRole("button", { name: /connection actions/i }), + ); expect(screen.getByText("Duplicate")).toBeInTheDocument(); }); @@ -87,7 +105,9 @@ describe("ConnectionCard", () => { const user = userEvent.setup(); const onDuplicate = vi.fn(); render(); - await user.click(screen.getByRole("button", { name: /connection actions/i })); + await user.click( + screen.getByRole("button", { name: /connection actions/i }), + ); await user.click(screen.getByText("Duplicate")); expect(onDuplicate).toHaveBeenCalledTimes(1); }); @@ -95,7 +115,9 @@ describe("ConnectionCard", () => { it("does not render Duplicate menu item when onDuplicate is not provided", async () => { const user = userEvent.setup(); render(); - await user.click(screen.getByRole("button", { name: /connection actions/i })); + await user.click( + screen.getByRole("button", { name: /connection actions/i }), + ); expect(screen.getByText("Edit")).toBeInTheDocument(); expect(screen.queryByText("Duplicate")).not.toBeInTheDocument(); }); @@ -106,7 +128,7 @@ describe("ConnectionCard", () => { {...defaultProps} status="error" statusText="Connection refused" - /> + />, ); // Error badge is still rendered expect(screen.getByText("Error")).toBeInTheDocument(); diff --git a/component/src/components/composed/connection-card.tsx b/component/src/components/composed/connection-card.tsx index 67f788bb..6584a2ac 100644 --- a/component/src/components/composed/connection-card.tsx +++ b/component/src/components/composed/connection-card.tsx @@ -1,3 +1,4 @@ +import type { ReactNode } from "react"; import { Database, MoreVertical, @@ -24,6 +25,12 @@ import { cn } from "@/lib/utils"; export interface ConnectionCardProps { name: string; host: string; + /** + * Optional connector-type icon (e.g. a Neo4j or PostgreSQL logo). Falls back + * to a generic database glyph so every type is visually distinct (#1043). + * Passed in by the app to keep this library free of app-specific assets. + */ + icon?: ReactNode; database?: string; status: ConnectionState; statusText?: string; @@ -44,6 +51,7 @@ export interface ConnectionCardProps { function ConnectionCard({ name, host, + icon, database, status, statusText, @@ -70,7 +78,7 @@ function ConnectionCard({ >
- + {icon ?? }