History

Test that enable-services steps never auto-chain popups

Extract the Continue handler as continueEnableServiceStep so the popup-open contract is testable: completing a step (success or failure) only records the result, and the next popup opens only on a fresh Continue click. Also cover the mount boundary: the modal renders nothing while closed or without an account, so the body and its OAuth flow unmount on close.

Rhys Sullivan committed 4 months ago

Commit 962df011 parent 289c6402 files · +147 −16Browse files

Unified · Split

2 files
  1. packages/react/src/components/enable-services-modal.test.ts +107 −0
  2. packages/react/src/components/enable-services-modal.tsx +40 −16
packages/react/src/components/enable-services-modal.test.ts+107−0
@@ -14,9 +14,13 @@ import { · 13 unmodified lines
1414 activeEnableServiceStep,
1515 applyEnableServiceStepResult,
1616 buildEnableServiceOAuthStartPayload,
17+ continueEnableServiceStep,
1718 createEnableServicesQueue,
19+ EnableServicesModal,
1820 retryEnableServiceStep,
1921 type EnableServiceIntegration,
22+ type EnableServiceQueue,
23+ type EnableServiceStepStatus,
2024 } from "./enable-services-modal";
2125 import type { OAuthStartPayload } from "../plugins/oauth-sign-in";
2226
@@ -159,3 +163,106 @@ describe("buildEnableServiceOAuthStartPayload", () => { · 136 unmodified lines
159163 ).toBeNull();
160164 });
161165 });
166+
167+describe("continueEnableServiceStep", () => {
168+ /** Drives the queue exactly like EnableServicesModalBody: `continueStep`
169+ * mirrors handleContinue (Continue click), and step completion flows back
170+ * through markActive, so the opener-call count is observable per click. */
171+ const harness = () => {
172+ let queue: EnableServiceQueue<EnableServiceIntegration> = createEnableServicesQueue([
173+ gmail,
174+ calendar,
175+ ]);
176+ const started: {
177+ readonly payload: OAuthStartPayload;
178+ readonly onSuccess: () => void;
179+ readonly onError: () => void;
180+ }[] = [];
181+ const markActive = (status: EnableServiceStepStatus) => {
182+ const active = activeEnableServiceStep(queue);
183+ if (active) queue = applyEnableServiceStepResult(queue, active.integration.slug, status);
184+ };
185+ const continueStep = () => {
186+ const active = activeEnableServiceStep(queue);
187+ if (!active) return;
188+ continueEnableServiceStep({
189+ account: account(),
190+ integration: active.integration,
191+ organizationId: "org_123",
192+ start: (input) => started.push(input),
193+ markActive,
194+ });
195+ };
196+ return {
197+ started,
198+ continueStep,
199+ queue: () => queue,
200+ };
201+ };
202+
203+ it("opens exactly one popup per Continue click and never auto-chains on success", () => {
204+ const { started, continueStep, queue } = harness();
205+
206+ continueStep();
207+ expect(started).toHaveLength(1);
208+ expect(started[0]?.payload.integration).toBe(gmail.slug);
209+
210+ // The popup completes: the step is marked done and the queue advances,
211+ // but no second popup opens without a fresh Continue click.
212+ started[0]!.onSuccess();
213+ expect(queue().steps[0]?.status).toBe("done");
214+ expect(activeEnableServiceStep(queue())?.integration.slug).toBe(calendar.slug);
215+ expect(started).toHaveLength(1);
216+
217+ continueStep();
218+ expect(started).toHaveLength(2);
219+ expect(started[1]?.payload.integration).toBe(calendar.slug);
220+ });
221+
222+ it("does not auto-retry or auto-advance when a step fails", () => {
223+ const { started, continueStep, queue } = harness();
224+
225+ continueStep();
226+ started[0]!.onError();
227+ expect(queue().steps[0]?.status).toBe("failed");
228+ expect(activeEnableServiceStep(queue())?.integration.slug).toBe(gmail.slug);
229+ expect(started).toHaveLength(1);
230+ });
231+
232+ it("marks the step failed without opening a popup when no payload can be built", () => {
233+ const started: OAuthStartPayload[] = [];
234+ const statuses: EnableServiceStepStatus[] = [];
235+ continueEnableServiceStep({
236+ account: account({ label: "Personal Google" }),
237+ integration: calendar,
238+ organizationId: "org_123",
239+ start: (input) => started.push(input.payload),
240+ markActive: (status) => statuses.push(status),
241+ });
242+ expect(started).toHaveLength(0);
243+ expect(statuses).toEqual(["failed"]);
244+ });
245+});
246+
247+describe("EnableServicesModal mount boundary", () => {
248+ // The wrapper renders nothing while closed, so the body (and with it the
249+ // useOAuthPopupFlow instance owning the in-flight popup) unmounts on close;
250+ // the hook's unmount effect cancels the session. The wrapper has no hooks,
251+ // so calling it as a plain function is safe here.
252+ const wrapperProps = {
253+ integrations: [gmail, calendar],
254+ onOpenChange: () => {},
255+ };
256+
257+ it("renders nothing when closed, unmounting the body and its OAuth flow", () => {
258+ expect(EnableServicesModal({ ...wrapperProps, open: false, account: account() })).toBeNull();
259+ });
260+
261+ it("renders nothing without an account", () => {
262+ expect(EnableServicesModal({ ...wrapperProps, open: true, account: null })).toBeNull();
263+ });
264+
265+ it("mounts the body only while open with an account", () => {
266+ expect(EnableServicesModal({ ...wrapperProps, open: true, account: account() })).not.toBeNull();
267+ });
268+});
packages/react/src/components/enable-services-modal.tsx+40−16
@@ -141,6 +141,43 @@ export const buildEnableServiceOAuthStartPayload = (input: { · 140 unmodified lines
141141 };
142142 };
143143
144+/** Run one queue step: build the payload and open exactly one OAuth popup.
145+ * The completion callbacks only record the step result; advancing to the next
146+ * service always requires a fresh Continue click, so a finishing popup can
147+ * never auto-open the next one. */
148+export const continueEnableServiceStep = (input: {
149+ readonly account: ProviderAccount<Connection, EnableServiceIntegration>;
150+ readonly integration: EnableServiceIntegration;
151+ readonly organizationId: string | null;
152+ readonly start: (input: {
153+ readonly payload: OAuthStartPayload;
154+ readonly onSuccess: () => void;
155+ readonly onError: () => void;
156+ }) => void;
157+ readonly markActive: (status: EnableServiceStepStatus) => void;
158+}): void => {
159+ const payload = buildEnableServiceOAuthStartPayload({
160+ account: input.account,
161+ integration: input.integration,
162+ organizationId: input.organizationId,
163+ });
164+ if (!payload) {
165+ input.markActive("failed");
166+ toast.error("This service cannot reuse the selected OAuth app");
167+ return;
168+ }
169+ input.start({
170+ payload,
171+ onSuccess: () => {
172+ input.markActive("done");
173+ toast.success(`${input.integration.name} connected`);
174+ },
175+ onError: () => {
176+ input.markActive("failed");
177+ },
178+ });
179+};
180+
144181 const serviceKey = (integration: EnableServiceIntegration): string => String(integration.slug);
145182
146183 export function EnableServicesModal(props: {
@@ -203,25 +240,12 @@ function EnableServicesModalBody(props: { · 56 unmodified lines
203240
204241 const handleContinue = () => {
205242 if (!active) return;
206− const payload = buildEnableServiceOAuthStartPayload({
243+ continueEnableServiceStep({
207244 account: props.account,
208245 integration: active.integration,
209246 organizationId,
210− });
211− if (!payload) {
212− markActive("failed");
213− toast.error("This service cannot reuse the selected OAuth app");
214− return;
215− }
216− void oauthPopup.start({
217− payload,
218− onSuccess: () => {
219− markActive("done");
220− toast.success(`${active.integration.name} connected`);
221− },
222− onError: () => {
223− markActive("failed");
224− },
247+ start: (input) => void oauthPopup.start(input),
248+ markActive,
225249 });
226250 };
227251