From 5edfa628cd0b132e3a7be3a0b2cda002d8b344b9 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 23 Jun 2026 01:21:37 +0000 Subject: [PATCH] Harden external object sync edge cases --- server/src/services/external-objects.ts | 104 ++++++++++--------- ui/src/components/IssuesList.tsx | 7 +- ui/src/hooks/useIssueExternalObjects.test.ts | 48 --------- ui/src/lib/external-objects.test.ts | 49 ++++++++- 4 files changed, 108 insertions(+), 100 deletions(-) delete mode 100644 ui/src/hooks/useIssueExternalObjects.test.ts diff --git a/server/src/services/external-objects.ts b/server/src/services/external-objects.ts index ad7204275ce..db34f3b476b 100644 --- a/server/src/services/external-objects.ts +++ b/server/src/services/external-objects.ts @@ -515,58 +515,64 @@ export function externalObjectService( } async function syncComment(commentId: string, dbOrTx: any = db) { - if (!(await isEnabled())) return; - const comment = await dbOrTx - .select({ - id: issueComments.id, - companyId: issueComments.companyId, - issueId: issueComments.issueId, - body: issueComments.body, - }) - .from(issueComments) - .where(eq(issueComments.id, commentId)) - .then((rows: Array<{ id: string; companyId: string; issueId: string; body: string }>) => rows[0] ?? null); - if (!comment) throw notFound("Issue comment not found"); - await replaceSourceMentions({ - companyId: comment.companyId, - sourceIssueId: comment.issueId, - sourceKind: "comment", - sourceRecordId: comment.id, - documentKey: null, - propertyKey: null, - text: comment.body, - }, dbOrTx); + const runSync = async (tx: any) => { + if (!(await isEnabled())) return; + const comment = await tx + .select({ + id: issueComments.id, + companyId: issueComments.companyId, + issueId: issueComments.issueId, + body: issueComments.body, + }) + .from(issueComments) + .where(eq(issueComments.id, commentId)) + .then((rows: Array<{ id: string; companyId: string; issueId: string; body: string }>) => rows[0] ?? null); + if (!comment) throw notFound("Issue comment not found"); + await replaceSourceMentions({ + companyId: comment.companyId, + sourceIssueId: comment.issueId, + sourceKind: "comment", + sourceRecordId: comment.id, + documentKey: null, + propertyKey: null, + text: comment.body, + }, tx); + }; + return dbOrTx === db ? db.transaction(runSync) : runSync(dbOrTx); } async function syncDocument(documentId: string, dbOrTx: any = db) { - if (!(await isEnabled())) return; - const document = await dbOrTx - .select({ - documentId: documents.id, - companyId: documents.companyId, - issueId: issueDocuments.issueId, - key: issueDocuments.key, - body: documents.latestBody, - }) - .from(issueDocuments) - .innerJoin(documents, eq(issueDocuments.documentId, documents.id)) - .where(eq(documents.id, documentId)) - .then((rows: Array<{ documentId: string; companyId: string; issueId: string; key: string; body: string }>) => rows[0] ?? null); - if (!document) { - await dbOrTx - .delete(externalObjectMentions) - .where(and(eq(externalObjectMentions.sourceKind, "document"), eq(externalObjectMentions.sourceRecordId, documentId))); - return; - } - await replaceSourceMentions({ - companyId: document.companyId, - sourceIssueId: document.issueId, - sourceKind: "document", - sourceRecordId: document.documentId, - documentKey: document.key, - propertyKey: null, - text: document.body, - }, dbOrTx); + const runSync = async (tx: any) => { + if (!(await isEnabled())) return; + const document = await tx + .select({ + documentId: documents.id, + companyId: documents.companyId, + issueId: issueDocuments.issueId, + key: issueDocuments.key, + body: documents.latestBody, + }) + .from(issueDocuments) + .innerJoin(documents, eq(issueDocuments.documentId, documents.id)) + .where(eq(documents.id, documentId)) + .then((rows: Array<{ documentId: string; companyId: string; issueId: string; key: string; body: string }>) => rows[0] ?? null); + if (!document) { + await tx + .delete(externalObjectMentions) + .where(and(eq(externalObjectMentions.sourceKind, "document"), eq(externalObjectMentions.sourceRecordId, documentId))); + return; + } + await replaceSourceMentions({ + companyId: document.companyId, + sourceIssueId: document.issueId, + sourceKind: "document", + sourceRecordId: document.documentId, + documentKey: document.key, + propertyKey: null, + text: document.body, + }, tx); + }; + return dbOrTx === db ? db.transaction(runSync) : runSync(dbOrTx); } async function safeSync(label: string, fn: () => Promise) { diff --git a/ui/src/components/IssuesList.tsx b/ui/src/components/IssuesList.tsx index bded52262d8..81d4af908ed 100644 --- a/ui/src/components/IssuesList.tsx +++ b/ui/src/components/IssuesList.tsx @@ -1006,6 +1006,9 @@ export function IssuesList({ externalObjectSummaryByIssueId, externalObjectSummariesReady: externalObjectSummariesReady && !externalObjectSummariesLoading, }), [externalObjectSummariesLoading, externalObjectSummariesReady, externalObjectSummaryByIssueId, issueFilterWorkspaceContext]); + const externalObjectFilterLoading = hasExternalObjectStatusFilters + && externalObjectSummariesLoading + && !externalObjectSummariesReady; const filtered = useMemo(() => { const filteredByControls = applyIssueFilters( @@ -1597,7 +1600,7 @@ export function IssuesList({ - {isLoading && } + {(isLoading || externalObjectFilterLoading) && } {error &&

{error.message}

} {!searchWithinLoadedIssues && normalizedIssueSearch.length > 0 && searchedIssues.length === ISSUE_SEARCH_RESULT_LIMIT && (

@@ -1609,7 +1612,7 @@ export function IssuesList({ Some board columns are showing up to {ISSUE_BOARD_COLUMN_RESULT_LIMIT} tasks. Refine filters or search to reveal the rest.

)} - {!isLoading && filtered.length === 0 && viewState.viewMode === "list" && ( + {!isLoading && !externalObjectFilterLoading && filtered.length === 0 && viewState.viewMode === "list" && ( ({ - externalObjectsApi: { - getIssueSummaries: vi.fn(), - }, -})); - -const emptySummary: ExternalObjectSummary = { - total: 0, - byStatusCategory: {}, - byLiveness: {}, - highestSeverity: "neutral", - objects: [], -}; - -describe("fetchIssueExternalObjectSummariesInBatches", () => { - afterEach(() => { - vi.mocked(externalObjectsApi.getIssueSummaries).mockReset(); - }); - - it("chunks bulk summary requests below the server validation cap and merges results", async () => { - const issueIds = Array.from( - { length: EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE * 2 + 3 }, - (_entry, index) => `issue-${index}`, - ); - vi.mocked(externalObjectsApi.getIssueSummaries).mockImplementation(async (_companyId, ids) => ({ - summaries: Object.fromEntries(ids.map((id) => [id, { ...emptySummary, total: id.endsWith("-0") ? 1 : 0 }])), - })); - - const result = await fetchIssueExternalObjectSummariesInBatches("company-1", issueIds); - - expect(externalObjectsApi.getIssueSummaries).toHaveBeenCalledTimes(3); - expect(vi.mocked(externalObjectsApi.getIssueSummaries).mock.calls.map((call) => call[1].length)).toEqual([ - EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE, - EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE, - 3, - ]); - expect(Object.keys(result.summaries)).toHaveLength(issueIds.length); - expect(result.summaries["issue-0"]?.total).toBe(1); - }); -}); diff --git a/ui/src/lib/external-objects.test.ts b/ui/src/lib/external-objects.test.ts index 7b78f1d1695..c06a480ea45 100644 --- a/ui/src/lib/external-objects.test.ts +++ b/ui/src/lib/external-objects.test.ts @@ -1,4 +1,10 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { ExternalObjectSummary } from "@paperclipai/shared"; +import { externalObjectsApi } from "../api/externalObjects"; +import { + EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE, + fetchIssueExternalObjectSummariesInBatches, +} from "../hooks/useIssueExternalObjects"; import { dominantExternalObjectTone, externalObjectCategoryLabel, @@ -14,6 +20,20 @@ import { } from "./external-objects"; import { normalizeExternalObjectHref } from "./external-object-href"; +vi.mock("../api/externalObjects", () => ({ + externalObjectsApi: { + getIssueSummaries: vi.fn(), + }, +})); + +const emptySummary: ExternalObjectSummary = { + total: 0, + byStatusCategory: {}, + byLiveness: {}, + highestSeverity: "neutral", + objects: [], +}; + describe("normalizeExternalObjectHref", () => { it("lowercases the host (case preserving the path) and drops query/fragment", () => { expect( @@ -35,6 +55,33 @@ describe("normalizeExternalObjectHref", () => { }); }); +describe("fetchIssueExternalObjectSummariesInBatches", () => { + afterEach(() => { + vi.mocked(externalObjectsApi.getIssueSummaries).mockReset(); + }); + + it("chunks bulk summary requests below the server validation cap and merges results", async () => { + const issueIds = Array.from( + { length: EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE * 2 + 3 }, + (_entry, index) => `issue-${index}`, + ); + vi.mocked(externalObjectsApi.getIssueSummaries).mockImplementation(async (_companyId, ids) => ({ + summaries: Object.fromEntries(ids.map((id) => [id, { ...emptySummary, total: id.endsWith("-0") ? 1 : 0 }])), + })); + + const result = await fetchIssueExternalObjectSummariesInBatches("company-1", issueIds); + + expect(externalObjectsApi.getIssueSummaries).toHaveBeenCalledTimes(3); + expect(vi.mocked(externalObjectsApi.getIssueSummaries).mock.calls.map((call) => call[1].length)).toEqual([ + EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE, + EXTERNAL_OBJECT_SUMMARY_BATCH_SIZE, + 3, + ]); + expect(Object.keys(result.summaries)).toHaveLength(issueIds.length); + expect(result.summaries["issue-0"]?.total).toBe(1); + }); +}); + describe("external-objects helpers", () => { it("labels categories with copy from the UX spec", () => { expect(externalObjectCategoryLabel("auth_required")).toBe("Authorization required");