History

Confirm before removing a connection

Rhys Sullivan committed 3 months ago

Commit a37e4641 parent fc1e5892 files · +176 −1Browse files

Unified · Split

2 files
  1. e2e/scenarios/connection-remove-confirm.test.ts +121 −0
  2. packages/react/src/components/accounts-section.tsx +55 −1
e2e/scenarios/connection-remove-confirm.test.ts+121−0
@@ -0,0 +1,121 @@
1+// Cross-target (browser): removing a connection asks for confirmation first.
2+// Remove is destructive and irreversible (credentials are immutable, so a
3+// removed connection cannot be restored — only re-created), so the row's
4+// Remove menu item must open a confirm dialog rather than firing the
5+// mutation directly. Cancel keeps the connection; confirming removes it for
6+// real (asserted through the API, not just the list).
7+import { randomBytes } from "node:crypto";
8+
9+import { Effect } from "effect";
10+import { expect } from "@effect/vitest";
11+import { composePluginApi } from "@executor-js/api/server";
12+import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api";
13+import { AuthTemplateSlug, ConnectionName, IntegrationSlug } from "@executor-js/sdk/shared";
14+
15+import { scenario } from "../src/scenario";
16+import { Api, Browser, Target } from "../src/services";
17+
18+const api = composePluginApi([openApiHttpPlugin()] as const);
19+
20+/** Minimal apiKey-authenticated spec — a connection can bind to it; the single
21+ * operation is never invoked here. */
22+const pingSpec = JSON.stringify({
23+ openapi: "3.0.3",
24+ info: { title: "Ping API", version: "1.0.0" },
25+ paths: {
26+ "/ping": {
27+ get: { operationId: "ping", summary: "Ping", responses: { "200": { description: "pong" } } },
28+ },
29+ },
30+});
31+
32+scenario(
33+ "Connections (UI) · Remove asks for confirmation; cancel keeps, confirm removes",
34+ {},
35+ Effect.gen(function* () {
36+ const target = yield* Target;
37+ const browser = yield* Browser;
38+ const { client: makeClient } = yield* Api;
39+ const identity = yield* target.newIdentity();
40+ const client = yield* makeClient(api, identity);
41+
42+ const slug = IntegrationSlug.make(`rm-confirm-${randomBytes(4).toString("hex")}`);
43+ const name = ConnectionName.make("main");
44+
45+ yield* Effect.ensuring(
46+ Effect.gen(function* () {
47+ yield* client.openapi.addSpec({
48+ payload: {
49+ spec: { kind: "blob", value: pingSpec },
50+ slug,
51+ baseUrl: "http://127.0.0.1:59999", // never contacted
52+ authenticationTemplate: [
53+ {
54+ slug: "apiKey",
55+ type: "apiKey",
56+ headers: { authorization: ["Bearer ", { type: "variable", name: "token" }] },
57+ },
58+ ],
59+ },
60+ });
61+ yield* client.connections.create({
62+ payload: {
63+ owner: "org",
64+ name,
65+ integration: slug,
66+ template: AuthTemplateSlug.make("apiKey"),
67+ value: `key_${randomBytes(8).toString("hex")}`,
68+ },
69+ });
70+
71+ yield* browser.session(identity, async ({ page, step }) => {
72+ const connections = page.locator("section").filter({
73+ has: page.getByRole("heading", { level: 3, name: "Connections" }),
74+ });
75+ const row = connections.getByText("main", { exact: true });
76+ const menuTrigger = connections.locator('button[aria-haspopup="menu"]');
77+ const confirm = page.getByRole("alertdialog");
78+
79+ await step("Open the integration's connections", async () => {
80+ await page.goto(`/integrations/${slug}`, { waitUntil: "networkidle" });
81+ await row.waitFor();
82+ });
83+
84+ await step("Remove asks for confirmation instead of firing", async () => {
85+ await menuTrigger.click();
86+ await page.getByRole("menuitem", { name: "Remove" }).click();
87+ await confirm.getByText("Remove main?").waitFor();
88+ });
89+
90+ await step("Cancel keeps the connection", async () => {
91+ await confirm.getByRole("button", { name: "Cancel" }).click();
92+ await confirm.waitFor({ state: "detached" });
93+ await row.waitFor();
94+ });
95+
96+ await step("Confirming actually removes it", async () => {
97+ await menuTrigger.click();
98+ await page.getByRole("menuitem", { name: "Remove" }).click();
99+ await confirm.getByRole("button", { name: "Remove connection" }).click();
100+ await confirm.waitFor({ state: "detached" });
101+ await row.waitFor({ state: "detached" });
102+ });
103+ });
104+
105+ // Cancel left the removal un-fired and confirm fired it for real: the
106+ // connection is gone from the API, not merely from the rendered list.
107+ const remaining = yield* client.connections.list({ query: { integration: slug } });
108+ expect(
109+ remaining.map((connection) => String(connection.name)),
110+ "the removed connection is gone from the API",
111+ ).not.toContain(String(name));
112+ }),
113+ Effect.gen(function* () {
114+ yield* client.connections
115+ .remove({ params: { owner: "org", integration: slug, name } })
116+ .pipe(Effect.ignore);
117+ yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore);
118+ }),
119+ );
120+ }),
121+);
packages/react/src/components/accounts-section.tsx+55−1
@@ -30,6 +30,16 @@ import { useOAuthPopupFlow } from "../plugins/oauth-sign-in"; · 29 unmodified lines
3030 import { AddAccountModal } from "./add-account-modal";
3131 import { ConnectionEditSheet } from "./metadata-edit-sheet";
3232 import type { CreateCustomMethod } from "./add-custom-method-modal";
33+import {
34+ AlertDialog,
35+ AlertDialogAction,
36+ AlertDialogCancel,
37+ AlertDialogContent,
38+ AlertDialogDescription,
39+ AlertDialogFooter,
40+ AlertDialogHeader,
41+ AlertDialogTitle,
42+} from "./alert-dialog";
3343 import { Badge } from "./badge";
3444 import { Button } from "./button";
3545 import {
@@ -60,6 +70,16 @@ import { · 24 unmodified lines
6070
6171 const OWNERS: readonly Owner[] = ["org", "user"];
6272
73+// The confirm dialog names the connection the way the row does — stored
74+// identity label first, connection name otherwise. (No live probe identity
75+// here; that state lives inside the row.)
76+const connectionDisplayLabel = (connection: Connection | null): string => {
77+ if (connection === null) return "connection";
78+ return connection.identityLabel && connection.identityLabel.length > 0
79+ ? connection.identityLabel
80+ : String(connection.name);
81+};
82+
6383 function AccountRow(props: {
6484 readonly connection: Connection;
6585 /** The integration declares scopes this connection was not granted — it must
@@ -212,6 +232,10 @@ function OwnerAccounts(props: { · 146 unmodified lines
212232 }) {
213233 const { integration, owner } = props;
214234 const connections = useAtomValue(connectionsForIntegrationAtom({ integration, owner }));
235+ // Removal confirms in a dialog. State lives here (not in the row) because the
236+ // Remove menu item closes its dropdown on click, which would unmount a dialog
237+ // nested inside it — so the row only nominates the connection to remove.
238+ const [removingConnection, setRemovingConnection] = useState<Connection | null>(null);
215239 const doRemove = useAtomSet(removeConnectionOptimistic(owner), {
216240 mode: "promiseExit",
217241 });
@@ -324,6 +348,7 @@ function OwnerAccounts(props: { · 106 unmodified lines
324348 };
325349
326350 const handleRemove = async (connection: Connection) => {
351+ setRemovingConnection(null);
327352 const exit = await doRemove({
328353 params: {
329354 owner: connection.owner,
@@ -354,10 +379,39 @@ function OwnerAccounts(props: { · 24 unmodified lines
354379 showOwnerLabel={props.showOwnerLabels}
355380 onEdit={() => props.onEdit(connection)}
356381 onReconnect={() => void handleReconnect(connection)}
357− onRemove={() => void handleRemove(connection)}
382+ onRemove={() => setRemovingConnection(connection)}
358383 />
359384 ))}
360385 </CardStackContent>
386+ <AlertDialog
387+ open={removingConnection !== null}
388+ onOpenChange={(open: boolean) => {
389+ if (!open) setRemovingConnection(null);
390+ }}
391+ >
392+ <AlertDialogContent size="sm">
393+ <AlertDialogHeader>
394+ <AlertDialogTitle>
395+ Remove {connectionDisplayLabel(removingConnection)}?
396+ </AlertDialogTitle>
397+ <AlertDialogDescription>
398+ Tools using this connection lose access. This cannot be undone; reconnecting later
399+ creates a new connection.
400+ </AlertDialogDescription>
401+ </AlertDialogHeader>
402+ <AlertDialogFooter>
403+ <AlertDialogCancel>Cancel</AlertDialogCancel>
404+ <AlertDialogAction
405+ variant="destructive"
406+ onClick={() => {
407+ if (removingConnection !== null) void handleRemove(removingConnection);
408+ }}
409+ >
410+ Remove connection
411+ </AlertDialogAction>
412+ </AlertDialogFooter>
413+ </AlertDialogContent>
414+ </AlertDialog>
361415 </CardStack>
362416 );
363417 }