From e4d35735b11e6396ce203e8daef0b791d3565fe7 Mon Sep 17 00:00:00 2001 From: Waleed Date: Sat, 28 Mar 2026 12:04:26 -0700 Subject: [PATCH] fix(knowledge): give users choice to keep or delete documents when removing connector (#3825) * fix(knowledge): give users choice to keep or delete documents when removing connector * refactor(knowledge): clean up connector delete and extract shared extension validator - Extract `isAlphanumericExtension` helper to deduplicate regex across parser-extension.ts and validation.ts - Extract `closeDeleteModal` callback to eliminate 4x scattered state resets - Add archivedAt/deletedAt filters to UPDATE query in keep-docs delete path - Parallelize storage file cleanup and tag definition cleanup with Promise.all - Deduplicate URL construction in delete connector hook Co-Authored-By: Claude Opus 4.6 * refactor(knowledge): remove duplicate extension list from parser-extension Use SUPPORTED_DOCUMENT_EXTENSIONS and isSupportedExtension from validation.ts instead of maintaining a separate identical list. Co-Authored-By: Claude Opus 4.6 * fix(db): change document.connectorId FK from cascade to set null The cascade behavior meant deleting a connector would always delete its documents, contradicting the "keep documents" option. With set null, the database automatically nullifies connectorId when a connector is removed, and we only need explicit deletion when the user opts in. Co-Authored-By: Claude Opus 4.6 * chore(db): add migration metadata for connectorId FK change Co-Authored-By: Claude Opus 4.6 * fix(knowledge): fix connector delete test and use URL-safe searchParams Use `new URL(request.url).searchParams` instead of `request.nextUrl.searchParams` for compatibility with test mocks. Add missing `connectorType` to test fixture. Co-Authored-By: Claude Opus 4.6 * spacing --------- Co-authored-by: Claude Opus 4.6 --- .../connectors/[connectorId]/route.test.ts | 2 +- .../[id]/connectors/[connectorId]/route.ts | 42 +- .../add-connector-modal.tsx | 10 +- .../connectors-section/connectors-section.tsx | 43 +- .../edit-connector-modal.tsx | 4 +- apps/sim/hooks/queries/kb/connectors.ts | 5 +- .../knowledge/documents/parser-extension.ts | 37 +- apps/sim/lib/uploads/utils/validation.ts | 13 +- .../db/migrations/0182_luxuriant_quasar.sql | 3 + .../db/migrations/meta/0182_snapshot.json | 14430 ++++++++++++++++ packages/db/migrations/meta/_journal.json | 7 + packages/db/schema.ts | 2 +- 12 files changed, 14532 insertions(+), 66 deletions(-) create mode 100644 packages/db/migrations/0182_luxuriant_quasar.sql create mode 100644 packages/db/migrations/meta/0182_snapshot.json diff --git a/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.test.ts b/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.test.ts index dd2f4fb198..0ed9a96160 100644 --- a/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.test.ts +++ b/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.test.ts @@ -237,7 +237,7 @@ describe('Knowledge Connector By ID API Route', () => { .mockReturnValueOnce(mockDbChain) .mockResolvedValueOnce([{ id: 'doc-1', fileUrl: '/api/uploads/test.txt' }]) .mockReturnValueOnce(mockDbChain) - mockDbChain.limit.mockResolvedValueOnce([{ id: 'conn-456' }]) + mockDbChain.limit.mockResolvedValueOnce([{ id: 'conn-456', connectorType: 'jira' }]) mockDbChain.returning.mockResolvedValueOnce([{ id: 'conn-456' }]) const req = createMockRequest('DELETE') diff --git a/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts b/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts index 0d8212c98f..d1b7e1054d 100644 --- a/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts +++ b/apps/sim/app/api/knowledge/[id]/connectors/[connectorId]/route.ts @@ -292,7 +292,10 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { return NextResponse.json({ error: 'Connector not found' }, { status: 404 }) } - const connectorDocuments = await db.transaction(async (tx) => { + const { searchParams } = new URL(request.url) + const deleteDocuments = searchParams.get('deleteDocuments') === 'true' + + const { deletedDocs, docCount } = await db.transaction(async (tx) => { await tx.execute(sql`SELECT 1 FROM knowledge_connector WHERE id = ${connectorId} FOR UPDATE`) const docs = await tx @@ -306,10 +309,12 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { ) ) - const documentIds = docs.map((doc) => doc.id) - if (documentIds.length > 0) { - await tx.delete(embedding).where(inArray(embedding.documentId, documentIds)) - await tx.delete(document).where(inArray(document.id, documentIds)) + if (deleteDocuments) { + const documentIds = docs.map((doc) => doc.id) + if (documentIds.length > 0) { + await tx.delete(embedding).where(inArray(embedding.documentId, documentIds)) + await tx.delete(document).where(inArray(document.id, documentIds)) + } } const deletedConnectors = await tx @@ -328,16 +333,23 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { throw new Error('Connector not found') } - return docs + return { deletedDocs: deleteDocuments ? docs : [], docCount: docs.length } }) - await deleteDocumentStorageFiles(connectorDocuments, requestId) + if (deleteDocuments) { + await Promise.all([ + deletedDocs.length > 0 + ? deleteDocumentStorageFiles(deletedDocs, requestId) + : Promise.resolve(), + cleanupUnusedTagDefinitions(knowledgeBaseId, requestId).catch((error) => { + logger.warn(`[${requestId}] Failed to cleanup tag definitions`, error) + }), + ]) + } - await cleanupUnusedTagDefinitions(knowledgeBaseId, requestId).catch((error) => { - logger.warn(`[${requestId}] Failed to cleanup tag definitions`, error) - }) - - logger.info(`[${requestId}] Hard-deleted connector ${connectorId} and its documents`) + logger.info( + `[${requestId}] Deleted connector ${connectorId}${deleteDocuments ? ` and ${docCount} documents` : `, kept ${docCount} documents`}` + ) recordAudit({ workspaceId: writeCheck.knowledgeBase.workspaceId, @@ -349,7 +361,11 @@ export async function DELETE(request: NextRequest, { params }: RouteParams) { resourceId: connectorId, resourceName: existingConnector[0].connectorType, description: `Deleted connector from knowledge base "${writeCheck.knowledgeBase.name}"`, - metadata: { knowledgeBaseId, documentsDeleted: connectorDocuments.length }, + metadata: { + knowledgeBaseId, + documentsDeleted: deleteDocuments ? docCount : 0, + documentsKept: deleteDocuments ? 0 : docCount, + }, request, }) diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx index e7d9603da8..5144ce5d8a 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/add-connector-modal/add-connector-modal.tsx @@ -373,7 +373,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo
{/* Auth: API key input or OAuth credential selection */} {isApiKeyMode ? ( -
+
) : ( -
+
{credentialsLoading ? (
@@ -442,7 +442,7 @@ export function AddConnectorModal({ open, onOpenChange, knowledgeBaseId }: AddCo canonicalId && (canonicalGroups.get(canonicalId)?.length ?? 0) === 2 return ( -
+