Skip to content

fix(connectors): clear query on type switch, pre-fill edit dialog #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alfredo1996 wants to merge 23 commits into from
Closed

fix(connectors): clear query on type switch, pre-fill edit dialog #345

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b76923c
fix: auto-run preview in widget creation mode when connection and que…
alfredorubin96 Apr 2, 2026
75d28c5
fix: show warning in query editor when no connector is selected (#314)
alfredorubin96 Apr 2, 2026
739fd36
fix: prevent dashboard list layout shift on scroll (#317)
alfredorubin96 Apr 2, 2026
a770020
fix: show "No connection configured" instead of misleading "Waiting f…
alfredorubin96 Apr 2, 2026
5734e15
fix: prevent graph chart infinite loading loop on fullscreen expand (…
alfredorubin96 Apr 2, 2026
4744a43
fix: make connector error click E2E test less brittle on CI
alfredorubin96 Apr 2, 2026
b84cd21
chore: restore CLAUDE.md, agents, skills, hooks, settings + add Playw…
alfredorubin96 Apr 2, 2026
ebd6c97
chore: streamline agent pipeline — remove redundant agents/skills, up…
alfredorubin96 Apr 2, 2026
482d98f
fix: harden query editor panel tests per CodeRabbit review
alfredorubin96 Apr 2, 2026
99b190a
test: add coverage for graph chart fullscreen fix to meet SonarCloud …
alfredorubin96 Apr 2, 2026
7935997
Merge branch 'fix/313-graph-chart-fullscreen-loop' into release/1.0-i…
alfredorubin96 Apr 2, 2026
8dbd6de
Merge branch 'fix/314-query-editor-no-connector-warning' into release…
alfredorubin96 Apr 2, 2026
160aa71
Merge branch 'fix/315-widget-preview-blank-on-creation' into release/…
alfredorubin96 Apr 2, 2026
4c9148c
chore: resolve merge conflict — split card-container tests
alfredorubin96 Apr 2, 2026
cd16f2d
Merge branch 'fix/317-dashboard-scroll-layout-shift' into release/1.0…
alfredorubin96 Apr 2, 2026
72f99da
fix: replace fullscreen loading text with spinner to prevent bleed-th…
alfredorubin96 Apr 2, 2026
da2e03f
feat: add user simulation agents for UX friction reporting
alfredorubin96 Apr 2, 2026
e246b87
docs: add UX friction report from automated user simulations
alfredorubin96 Apr 2, 2026
489bcc4
fix(connectors): clear query on type switch, pre-fill edit dialog (#3…
alfredorubin96 Apr 2, 2026
8610931
fix: update E2E tests for settings tab navigation and error card locator
alfredorubin96 Apr 3, 2026
e1c43b8
Merge branch 'release/1.0' into fix/issue-325-326-connectors
alfredorubin96 Apr 3, 2026
004351a
test: add coverage for connector fixes to meet SonarCloud gate (#345)
alfredorubin96 Apr 3, 2026
5e25871
test: add final coverage for connector fixes (#345)
alfredorubin96 Apr 3, 2026
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
33 changes: 29 additions & 4 deletions app/src/app/(dashboard)/connections/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,16 +283,40 @@ export default function ConnectionsPage() {
setShowCreate(true);
}

function openEditDialog(conn: {
async function openEditDialog(conn: {
id: string;
name: string;
type: ConnectorType;
}) {
setEditTarget(conn);
// Reset the edit form — advanced fields start empty (user fills what they want to change)
setEditForm({ ...DEFAULT_FORM, type: conn.type, name: conn.name });
setEditError(null);
setShowEditAdvanced(true);

// Fetch existing config (sans password) and pre-fill the form
try {
const res = await fetch(`/api/connections/${conn.id}`);
const body = await res.json();
const config = body?.data?.config;
if (config) {
setEditForm((prev) => ({
...prev,
uri: config.uri ?? "",
username: config.username ?? "",
database: config.database ?? "",
connectionTimeout: config.connectionTimeout?.toString() ?? "",
queryTimeout: config.queryTimeout?.toString() ?? "",
maxPoolSize: config.maxPoolSize?.toString() ?? "",
connectionAcquisitionTimeout:
config.connectionAcquisitionTimeout?.toString() ?? "",
idleTimeout: config.idleTimeout?.toString() ?? "",
statementTimeout: config.statementTimeout?.toString() ?? "",
sslRejectUnauthorized: config.sslRejectUnauthorized,
}));
}
} catch {
Comment on lines +289 to +300

@coderabbitai coderabbitai Bot Apr 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Ignore stale edit-prefill responses.

This fetch always applies its result to editForm, even if the dialog was closed, another connection was opened, or the user already started typing. A slow response for connection A can overwrite connection B's form and then save A's config into B. Guard the update with the active conn.id or cancel the previous request.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/src/app/`(dashboard)/connections/page.tsx around lines 296 - 317, The
fetch that prefills the form can race and overwrite the wrong editor; guard or
cancel it by tying the response to the active connection ID or using an
AbortController. Specifically, when calling fetch(`/api/connections/${conn.id}`)
ensure you only call setEditForm if the current active conn.id still matches
(e.g., compare to a ref/prop holding the latest active connection ID) or abort
any previous request before starting a new one (use AbortController and pass its
signal to fetch, abort on dialog close or conn change). Update the logic around
setEditForm, the fetch call, and any dialog open/close handlers so stale
responses cannot mutate the editForm state.

// Non-critical — form still works with empty fields
}
}

function buildEditConfig() {
Expand Down Expand Up @@ -655,7 +679,8 @@ export default function ConnectionsPage() {
</DialogHeader>
<div className="space-y-4 py-4">
<p className="text-sm text-muted-foreground">
Re-enter your credentials to update advanced settings.
Update your connection settings. Leave password blank to keep
the existing one.
</p>

<div className="space-y-2">
Expand Down Expand Up @@ -695,7 +720,7 @@ export default function ConnectionsPage() {
onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
setEditForm((f) => ({ ...f, password: e.target.value }))
}
required
placeholder="Leave blank to keep existing"
/>
</div>
</div>
Expand Down
198 changes: 174 additions & 24 deletions app/src/app/api/connections/[id]/__tests__/route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,33 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { makeSelectChain, makeUpdateChain, makeDeleteChain } from "@/__tests__/helpers/drizzle-mocks";
import {
makeSelectChain,
makeUpdateChain,
makeDeleteChain,
} from "@/__tests__/helpers/drizzle-mocks";
import { makeRequest, makeParams } from "@/__tests__/helpers/request-helpers";
import { nextResponseMockFactory } from "@/__tests__/helpers/next-mocks";

// ---------------------------------------------------------------------------
// Mocks
// ---------------------------------------------------------------------------

const mockRequireSession = vi.fn<
() => Promise<{ userId: string; role: string; canWrite: boolean; tenantId: string }>
>();
const mockRequireSession =
vi.fn<
() => Promise<{
userId: string;
role: string;
canWrite: boolean;
tenantId: string;
}>
>();
const mockEncryptJson = vi.fn((v: unknown) => `enc:${JSON.stringify(v)}`);
const mockDecryptJson = vi.fn(() => ({
uri: "bolt://localhost:7687",
username: "neo4j",
password: "secret",
database: "neo4j",
connectionTimeout: 5000,
}));
const mockPrefetchSchema = vi.fn();

const mockDb = {
Expand All @@ -32,21 +49,39 @@ class ForbiddenError extends Error {

vi.mock("@/lib/auth/session", () => ({ requireSession: mockRequireSession }));
vi.mock("@/lib/db", () => ({ db: mockDb }));
vi.mock("@/lib/crypto", () => ({ encryptJson: mockEncryptJson, decryptJson: vi.fn() }));
vi.mock("@/lib/schema-prefetch", () => ({ prefetchSchema: mockPrefetchSchema }));
vi.mock("@/lib/crypto", () => ({
encryptJson: mockEncryptJson,
decryptJson: mockDecryptJson,
}));
vi.mock("@/lib/schema-prefetch", () => ({
prefetchSchema: mockPrefetchSchema,
}));
vi.mock("next/server", () => nextResponseMockFactory());
vi.mock("@/lib/auth/errors", () => ({ UnauthorizedError, ForbiddenError }));

const SESSION = { userId: "user-1", role: "creator", canWrite: true, tenantId: "t1" };
const ADMIN_SESSION = { userId: "admin-1", role: "admin", canWrite: true, tenantId: "t1" };
const SESSION = {
userId: "user-1",
role: "creator",
canWrite: true,
tenantId: "t1",
};
const ADMIN_SESSION = {
userId: "admin-1",
role: "admin",
canWrite: true,
tenantId: "t1",
};

// ---------------------------------------------------------------------------
// GET /api/connections/[id]
// ---------------------------------------------------------------------------

describe("GET /api/connections/[id]", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let GET: (req: Request, ctx: { params: Promise<{ id: string }> }) => Promise<any>;
let GET: (
req: Request,
ctx: { params: Promise<{ id: string }> },
) => Promise<any>;

beforeEach(async () => {
vi.resetModules();
Expand All @@ -63,7 +98,13 @@ describe("GET /api/connections/[id]", () => {

it("returns connection metadata in envelope (owner)", async () => {
mockRequireSession.mockResolvedValue(SESSION);
const conn = { id: "c1", name: "My DB", type: "postgresql", createdAt: new Date(), updatedAt: new Date() };
const conn = {
id: "c1",
name: "My DB",
type: "postgresql",
createdAt: new Date(),
updatedAt: new Date(),
};
mockDb.select.mockReturnValue(makeSelectChain([conn]));

const res = await GET(makeRequest({}), makeParams("c1"));
Expand All @@ -75,7 +116,13 @@ describe("GET /api/connections/[id]", () => {

it("admin can view any connection in tenant", async () => {
mockRequireSession.mockResolvedValue(ADMIN_SESSION);
const conn = { id: "c1", name: "Other DB", type: "neo4j", createdAt: new Date(), updatedAt: new Date() };
const conn = {
id: "c1",
name: "Other DB",
type: "neo4j",
createdAt: new Date(),
updatedAt: new Date(),
};
// First select (owner check) returns empty
mockDb.select.mockReturnValueOnce(makeSelectChain([]));
// Second select (admin fallback) returns the connection
Expand All @@ -99,13 +146,41 @@ describe("GET /api/connections/[id]", () => {

it("does not expose configEncrypted", async () => {
mockRequireSession.mockResolvedValue(SESSION);
const conn = { id: "c1", name: "DB", type: "neo4j", createdAt: new Date(), updatedAt: new Date() };
const conn = {
id: "c1",
name: "DB",
type: "neo4j",
createdAt: new Date(),
updatedAt: new Date(),
};
mockDb.select.mockReturnValue(makeSelectChain([conn]));

const res = await GET(makeRequest({}), makeParams("c1"));
const body = await res.json();
expect(body.data.configEncrypted).toBeUndefined();
});

it("returns decrypted config without password", async () => {
mockRequireSession.mockResolvedValue(SESSION);
const conn = {
id: "c1",
name: "DB",
type: "neo4j",
configEncrypted: "enc:data",
createdAt: new Date(),
updatedAt: new Date(),
};
mockDb.select.mockReturnValue(makeSelectChain([conn]));

const res = await GET(makeRequest({}), makeParams("c1"));
const body = await res.json();
expect(body.data.config).toBeDefined();
expect(body.data.config.uri).toBe("bolt://localhost:7687");
expect(body.data.config.username).toBe("neo4j");
expect(body.data.config.database).toBe("neo4j");
expect(body.data.config.connectionTimeout).toBe(5000);
expect(body.data.config.password).toBeUndefined();
});
});

// ---------------------------------------------------------------------------
Expand All @@ -114,7 +189,10 @@ describe("GET /api/connections/[id]", () => {

describe("PATCH /api/connections/[id]", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let PATCH: (req: Request, ctx: { params: Promise<{ id: string }> }) => Promise<any>;
let PATCH: (
req: Request,
ctx: { params: Promise<{ id: string }> },
) => Promise<any>;

beforeEach(async () => {
vi.resetModules();
Expand All @@ -125,23 +203,37 @@ describe("PATCH /api/connections/[id]", () => {

it("returns 401 when unauthenticated", async () => {
mockRequireSession.mockRejectedValue(new UnauthorizedError());
const res = await PATCH(makeRequest({ name: "New name" }), makeParams("c1"));
const res = await PATCH(
makeRequest({ name: "New name" }),
makeParams("c1"),
);
expect(res.status).toBe(401);
});

it("returns 404 when connection not owned", async () => {
mockRequireSession.mockResolvedValue(SESSION);
mockDb.update.mockReturnValue(makeUpdateChain([]));
const res = await PATCH(makeRequest({ name: "New name" }), makeParams("c1"));
const res = await PATCH(
makeRequest({ name: "New name" }),
makeParams("c1"),
);
expect(res.status).toBe(404);
});

it("updates name and returns envelope", async () => {
mockRequireSession.mockResolvedValue(SESSION);
const updated = { id: "c1", name: "New name", type: "neo4j", updatedAt: new Date() };
const updated = {
id: "c1",
name: "New name",
type: "neo4j",
updatedAt: new Date(),
};
mockDb.update.mockReturnValue(makeUpdateChain([updated]));

const res = await PATCH(makeRequest({ name: "New name" }), makeParams("c1"));
const res = await PATCH(
makeRequest({ name: "New name" }),
makeParams("c1"),
);
expect(res.status).toBe(200);
const body = await res.json();
expect(body.data).toEqual(updated);
Expand All @@ -150,15 +242,70 @@ describe("PATCH /api/connections/[id]", () => {

it("re-encrypts config and triggers prefetch", async () => {
mockRequireSession.mockResolvedValue(SESSION);
const updated = { id: "c1", name: "Neo4j", type: "neo4j", updatedAt: new Date() };
const updated = {
id: "c1",
name: "Neo4j",
type: "neo4j",
updatedAt: new Date(),
};
mockDb.update.mockReturnValue(makeUpdateChain([updated]));

await PATCH(makeRequest({
config: { uri: "bolt://new-host", username: "neo4j", password: "newpass" },
}), makeParams("c1"));
await PATCH(
makeRequest({
config: {
uri: "bolt://new-host",
username: "neo4j",
password: "newpass",
},
}),
makeParams("c1"),
);

expect(mockEncryptJson).toHaveBeenCalledWith({
uri: "bolt://new-host",
username: "neo4j",
password: "newpass",
});
expect(mockPrefetchSchema).toHaveBeenCalledWith("neo4j", {
uri: "bolt://new-host",
username: "neo4j",
password: "newpass",
});
});

expect(mockEncryptJson).toHaveBeenCalledWith({ uri: "bolt://new-host", username: "neo4j", password: "newpass" });
expect(mockPrefetchSchema).toHaveBeenCalledWith("neo4j", { uri: "bolt://new-host", username: "neo4j", password: "newpass" });
it("allows config without password (merges with existing)", async () => {
mockRequireSession.mockResolvedValue(SESSION);
// First select to fetch existing encrypted config
const existing = {
id: "c1",
configEncrypted: "enc:existing",
type: "neo4j",
};
mockDb.select.mockReturnValue(makeSelectChain([existing]));
const updated = {
id: "c1",
name: "Neo4j",
type: "neo4j",
updatedAt: new Date(),
};
mockDb.update.mockReturnValue(makeUpdateChain([updated]));

const res = await PATCH(
makeRequest({
config: { uri: "bolt://new-host", username: "neo4j", database: "mydb" },
}),
makeParams("c1"),
);

expect(res.status).toBe(200);
// Should merge existing password into new config
expect(mockEncryptJson).toHaveBeenCalledWith(
expect.objectContaining({
uri: "bolt://new-host",
username: "neo4j",
password: "secret",
}),
);
});
});

Expand All @@ -168,7 +315,10 @@ describe("PATCH /api/connections/[id]", () => {

describe("DELETE /api/connections/[id]", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let DELETE: (req: Request, ctx: { params: Promise<{ id: string }> }) => Promise<any>;
let DELETE: (
req: Request,
ctx: { params: Promise<{ id: string }> },
) => Promise<any>;

beforeEach(async () => {
vi.resetModules();
Expand Down
Loading
Loading