fix(integration): resolve unfurls per viewer at render time

2026-08-19 22:51:37 +10:00 · 2026-08-09 15:18:43 +01:00
parent 16f7139e6f
commit 99bcf3da9d
5 changed files with 174 additions and 49 deletions
@@ -59,7 +59,6 @@ export const handlePaste = (
      .setIntegrationLink({
        url: pastedUrl,
        provider: integrationMatch.provider,
-        status: "pending",
      })
      // Anchor the "Paste as" menu to the inserted node, in the SAME
      // transaction: BubbleMenu ignores meta-only transactions (it only
@@ -216,13 +216,16 @@ function JiraIssueCard({
 }

 function IntegrationLinkView(props: any) {
-  const { node, updateAttributes, editor } = props;
-  const { url, provider, unfurlData, status } = node.attrs;
+  const { node } = props;
+  const { url, provider } = node.attrs;
  const { t } = useTranslation();

-  const { needsConnection } = useUnfurl(url, status, updateAttributes);
+  const unfurl = useUnfurl(url);
  const [connecting, setConnecting] = useState(false);

+  const needsConnection =
+    unfurl.state === "needsConnection" ? unfurl.needsConnection : null;
+
  const handleConnect = useCallback(
    async (event: React.MouseEvent) => {
      event.preventDefault();
@@ -287,7 +290,7 @@ function IntegrationLinkView(props: any) {
    );
  }

-  if (status === "pending") {
+  if (unfurl.state === "loading") {
    return (
      <NodeViewWrapper data-drag-handle="">
        <Card className={classes.card} withBorder padding="sm" radius="sm">
@@ -303,7 +306,8 @@ function IntegrationLinkView(props: any) {
    );
  }

-  if (status === "error" || !unfurlData) {
+  if (unfurl.state !== "loaded") {
+    // anonymous or error: a plain link card, no third-party data
    return (
      <NodeViewWrapper data-drag-handle="">
        <Card className={classes.card} withBorder padding="sm" radius="sm">
@@ -315,6 +319,8 @@ function IntegrationLinkView(props: any) {
    );
  }

+  const unfurlData = unfurl.data;
+
  // metadata.ts marks legacy message unfurls stored before metadata.type existed.
  const slackMeta = provider === "slack" ? unfurlData.metadata : null;
  if (slackMeta?.type === "message" || (slackMeta && !slackMeta.type && slackMeta.ts)) {
@@ -17,13 +17,12 @@ function shortUrl(url: string): string {
 }

 function IntegrationMentionView(props: any) {
-  const { node, updateAttributes } = props;
-  const { url, provider, unfurlData, status } = node.attrs;
+  const { node } = props;
+  const { url, provider } = node.attrs;
  const { t } = useTranslation();

-  useUnfurl(url, status, updateAttributes);
-
-  const data = unfurlData;
+  const unfurl = useUnfurl(url);
+  const data = unfurl.state === "loaded" ? unfurl.data : null;
  const meta = data?.metadata ?? {};
  const isSlackMessage =
    provider === "slack" && (meta.type === "message" || (!meta.type && meta.ts));
@@ -49,7 +48,7 @@ function IntegrationMentionView(props: any) {

  let content;
  if (!data) {
-    // pending / error / needs-connection: a compact link chip
+    // anonymous / loading / error / needs-connection: a compact link chip
    content = (
      <>
        {getIntegrationIcon(provider, 14)}
@@ -0,0 +1,118 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { renderHook, waitFor } from "@testing-library/react";
+import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import { createStore, Provider } from "jotai";
+import { ReactNode } from "react";
+import { useUnfurl } from "./use-unfurl";
+import { currentUserAtom } from "@/features/user/atoms/current-user-atom";
+import { unfurlUrl } from "@/features/integration/services/integration-service";
+import {
+  UnfurlNeedsConnection,
+  UnfurlResult,
+} from "@/features/integration/types/integration.types";
+
+vi.mock("@/features/integration/services/integration-service", () => ({
+  unfurlUrl: vi.fn(),
+}));
+
+const mockedUnfurlUrl = vi.mocked(unfurlUrl);
+
+const ISSUE_URL = "https://github.com/acme/repo/issues/42";
+
+const loadedResult: UnfurlResult = {
+  title: "Fix race condition in file watcher",
+  url: ISSUE_URL,
+  provider: "github",
+  status: "open",
+};
+
+const needsConnectionResult: UnfurlNeedsConnection = {
+  needsConnection: true,
+  integrationId: "int-1",
+  integrationType: "github",
+  integrationName: "GitHub",
+  title: "GitHub link",
+  description: "github.com/acme/repo/issues/42",
+};
+
+function createWrapper(loggedIn: boolean) {
+  const store = createStore();
+  if (loggedIn) {
+    store.set(currentUserAtom, { user: { id: "user-1" } } as any);
+  }
+  const queryClient = new QueryClient({
+    defaultOptions: { queries: { retry: false } },
+  });
+  return ({ children }: { children: ReactNode }) => (
+    <Provider store={store}>
+      <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
+    </Provider>
+  );
+}
+
+describe("useUnfurl", () => {
+  beforeEach(() => {
+    localStorage.clear();
+    vi.resetAllMocks();
+  });
+
+  it("never fetches for anonymous viewers and reports anonymous", () => {
+    const { result } = renderHook(() => useUnfurl(ISSUE_URL), {
+      wrapper: createWrapper(false),
+    });
+
+    expect(result.current.state).toBe("anonymous");
+    expect(mockedUnfurlUrl).not.toHaveBeenCalled();
+  });
+
+  it("starts loading then exposes the unfurl result", async () => {
+    mockedUnfurlUrl.mockResolvedValue(loadedResult);
+
+    const { result } = renderHook(() => useUnfurl(ISSUE_URL), {
+      wrapper: createWrapper(true),
+    });
+
+    expect(result.current.state).toBe("loading");
+    await waitFor(() => expect(result.current.state).toBe("loaded"));
+    expect(
+      result.current.state === "loaded" && result.current.data,
+    ).toEqual(loadedResult);
+    expect(mockedUnfurlUrl).toHaveBeenCalledWith({ url: ISSUE_URL });
+  });
+
+  it("maps a needsConnection response without treating it as an error", async () => {
+    mockedUnfurlUrl.mockResolvedValue(needsConnectionResult);
+
+    const { result } = renderHook(() => useUnfurl(ISSUE_URL), {
+      wrapper: createWrapper(true),
+    });
+
+    await waitFor(() =>
+      expect(result.current.state).toBe("needsConnection"),
+    );
+    expect(
+      result.current.state === "needsConnection" &&
+        result.current.needsConnection,
+    ).toEqual(needsConnectionResult);
+  });
+
+  it("maps a null result (no matching provider) to error", async () => {
+    mockedUnfurlUrl.mockResolvedValue(null);
+
+    const { result } = renderHook(() => useUnfurl(ISSUE_URL), {
+      wrapper: createWrapper(true),
+    });
+
+    await waitFor(() => expect(result.current.state).toBe("error"));
+  });
+
+  it("maps a rejected request to error", async () => {
+    mockedUnfurlUrl.mockRejectedValue(new Error("boom"));
+
+    const { result } = renderHook(() => useUnfurl(ISSUE_URL), {
+      wrapper: createWrapper(true),
+    });
+
+    await waitFor(() => expect(result.current.state).toBe("error"));
+  });
+});
@@ -1,44 +1,47 @@
-import { useCallback, useEffect, useState } from "react";
+import { useQuery } from "@tanstack/react-query";
+import { useAtomValue } from "jotai";
+import { currentUserAtom } from "@/features/user/atoms/current-user-atom";
 import { unfurlUrl } from "@/features/integration/services/integration-service";
-import { UnfurlNeedsConnection } from "@/features/integration/types/integration.types";
+import {
+  UnfurlNeedsConnection,
+  UnfurlResult,
+} from "@/features/integration/types/integration.types";

-// Fetches the unfurl for a node still in "pending" and writes the result into
-// its attrs. A needs-connection response stays local, never in the attrs: the
-// doc keeps status "pending" so a viewer who IS connected still unfurls and
-// materializes the card for everyone.
-export function useUnfurl(
-  url: string,
-  status: string,
-  updateAttributes: (attrs: Record<string, any>) => void,
-) {
-  const [needsConnection, setNeedsConnection] =
-    useState<UnfurlNeedsConnection | null>(null);
+const UNFURL_STALE_TIME = 5 * 60 * 1000; // mirrors the server-side Redis TTL

-  const doUnfurl = useCallback(async () => {
-    if (status !== "pending" || !url) return;
+export type UnfurlState =
+  | { state: "anonymous" }
+  | { state: "loading" }
+  | { state: "error" }
+  | { state: "needsConnection"; needsConnection: UnfurlNeedsConnection }
+  | { state: "loaded"; data: UnfurlResult };

-    try {
-      const result = await unfurlUrl({ url });
-      if (result && "needsConnection" in result) {
-        setNeedsConnection(result);
-      } else if (result) {
-        updateAttributes({
-          unfurlData: result,
-          status: "loaded",
-        });
-      } else {
-        updateAttributes({ status: "error" });
-      }
-    } catch {
-      updateAttributes({ status: "error" });
-    }
-  }, [url, status, updateAttributes]);
+// Resolves the unfurl per viewer at render time. Nothing is written back into
+// the document, so third-party permissions are enforced on every view:
+// unconnected viewers get needsConnection and anonymous viewers never fetch.
+export function useUnfurl(url: string): UnfurlState {
+  const currentUser = useAtomValue(currentUserAtom);
+  const isAuthenticated = Boolean(currentUser?.user);

-  useEffect(() => {
-    if (status === "pending") {
-      doUnfurl();
-    }
-  }, [status, doUnfurl]);
+  const query = useQuery({
+    queryKey: ["unfurl", url],
+    queryFn: () => unfurlUrl({ url }),
+    enabled: isAuthenticated && Boolean(url),
+    staleTime: UNFURL_STALE_TIME,
+    retry: false,
+  });

-  return { needsConnection };
+  if (!isAuthenticated || !url) {
+    return { state: "anonymous" };
+  }
+  if (query.isPending) {
+    return { state: "loading" };
+  }
+  if (query.isError || !query.data) {
+    return { state: "error" };
+  }
+  if ("needsConnection" in query.data) {
+    return { state: "needsConnection", needsConnection: query.data };
+  }
+  return { state: "loaded", data: query.data };
 }