From 99bcf3da9db66514fef53e9a7eca84a8b6acd542 Mon Sep 17 00:00:00 2001 From: Philipinho <16838612+Philipinho@users.noreply.github.com> Date: Sun, 9 Aug 2026 15:18:43 +0100 Subject: [PATCH] fix(integration): resolve unfurls per viewer at render time --- .../common/editor-paste-handler.tsx | 1 - .../integration-link-view.tsx | 16 ++- .../integration-mention-view.tsx | 11 +- .../integration-link/use-unfurl.test.tsx | 118 ++++++++++++++++++ .../components/integration-link/use-unfurl.ts | 77 ++++++------ 5 files changed, 174 insertions(+), 49 deletions(-) create mode 100644 apps/client/src/features/editor/components/integration-link/use-unfurl.test.tsx diff --git a/apps/client/src/features/editor/components/common/editor-paste-handler.tsx b/apps/client/src/features/editor/components/common/editor-paste-handler.tsx index 658bad9d7..f9b360df6 100644 --- a/apps/client/src/features/editor/components/common/editor-paste-handler.tsx +++ b/apps/client/src/features/editor/components/common/editor-paste-handler.tsx @@ -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 diff --git a/apps/client/src/features/editor/components/integration-link/integration-link-view.tsx b/apps/client/src/features/editor/components/integration-link/integration-link-view.tsx index 36281284d..170d3d9b6 100644 --- a/apps/client/src/features/editor/components/integration-link/integration-link-view.tsx +++ b/apps/client/src/features/editor/components/integration-link/integration-link-view.tsx @@ -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 ( @@ -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 ( @@ -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)) { diff --git a/apps/client/src/features/editor/components/integration-link/integration-mention-view.tsx b/apps/client/src/features/editor/components/integration-link/integration-mention-view.tsx index 3c28912bb..078bf4c33 100644 --- a/apps/client/src/features/editor/components/integration-link/integration-mention-view.tsx +++ b/apps/client/src/features/editor/components/integration-link/integration-mention-view.tsx @@ -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)} diff --git a/apps/client/src/features/editor/components/integration-link/use-unfurl.test.tsx b/apps/client/src/features/editor/components/integration-link/use-unfurl.test.tsx new file mode 100644 index 000000000..bcd234e50 --- /dev/null +++ b/apps/client/src/features/editor/components/integration-link/use-unfurl.test.tsx @@ -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 }) => ( + + {children} + + ); +} + +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")); + }); +}); diff --git a/apps/client/src/features/editor/components/integration-link/use-unfurl.ts b/apps/client/src/features/editor/components/integration-link/use-unfurl.ts index 4db0bfc8f..ee218e4a1 100644 --- a/apps/client/src/features/editor/components/integration-link/use-unfurl.ts +++ b/apps/client/src/features/editor/components/integration-link/use-unfurl.ts @@ -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) => void, -) { - const [needsConnection, setNeedsConnection] = - useState(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 }; }