From 55f46e471833253265fbbffd6b53392ae6746331 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Wed, 2 Sep 2026 10:31:35 -0700 Subject: [PATCH 1/6] feat(tables): preview referenced rows inline --- apps/sim/app/api/table/names/route.test.ts | 71 +++ apps/sim/app/api/table/names/route.ts | 22 + .../table-grid/cells/cell-content.tsx | 13 +- .../table-grid/cells/cell-render.test.ts | 4 + .../table-grid/cells/cell-render.test.tsx | 204 +++++++ .../table-grid/cells/cell-render.tsx | 56 +- .../components/table-grid/data-row.tsx | 56 +- .../table-grid/reference-row-preview.test.tsx | 425 +++++++++++++++ .../table-grid/reference-row-preview.tsx | 248 +++++++++ .../components/table-grid/table-grid.tsx | 230 +++++--- .../[tableId]/components/table-grid/types.ts | 8 + .../components/table-grid/utils.test.ts | 38 ++ .../[tableId]/components/table-grid/utils.ts | 31 +- .../lib/copy/copy-resources.test.ts | 414 +++++++++++++++ .../lib/copy/copy-resources.ts | 208 +++++++- .../lib/promote/copy-unmapped.test.ts | 8 +- .../lib/promote/copy-unmapped.ts | 1 + .../lib/remap/remap-table-groups.ts | 11 + apps/sim/hooks/queries/tables.test.ts | 314 ++++++++++- apps/sim/hooks/queries/tables.ts | 132 ++++- apps/sim/hooks/queries/utils/table-keys.ts | 5 + apps/sim/lib/api/contracts/tables.ts | 32 ++ apps/sim/lib/api/contracts/workspace-fork.ts | 4 +- apps/sim/lib/folders/bulk.test.ts | 1 + apps/sim/lib/folders/bulk.ts | 18 +- apps/sim/lib/folders/cascade.test.ts | 53 +- apps/sim/lib/folders/config.ts | 45 +- .../sim/lib/table/application/batch-policy.ts | 10 +- apps/sim/lib/table/application/bulk.test.ts | 159 +++++- apps/sim/lib/table/application/bulk.ts | 107 +++- apps/sim/lib/table/application/tables.test.ts | 15 + apps/sim/lib/table/application/tables.ts | 15 + apps/sim/lib/table/column-types/reference.ts | 8 + .../column-types/registry.server.test.ts | 214 +++++++- .../lib/table/column-types/registry.server.ts | 157 +++++- .../lib/table/column-types/types.server.ts | 8 + apps/sim/lib/table/column-types/types.ts | 7 + apps/sim/lib/table/service.test.ts | 499 +++++++++++++++++- apps/sim/lib/table/service.ts | 370 +++++++++++-- 39 files changed, 4002 insertions(+), 219 deletions(-) create mode 100644 apps/sim/app/api/table/names/route.test.ts create mode 100644 apps/sim/app/api/table/names/route.ts create mode 100644 apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.tsx create mode 100644 apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview.test.tsx create mode 100644 apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview.tsx diff --git a/apps/sim/app/api/table/names/route.test.ts b/apps/sim/app/api/table/names/route.test.ts new file mode 100644 index 00000000000..0348bcf9389 --- /dev/null +++ b/apps/sim/app/api/table/names/route.test.ts @@ -0,0 +1,71 @@ +/** + * @vitest-environment node + */ + +import { createMockRequest } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + getSession: vi.fn(), + listNames: vi.fn(), +})) + +vi.mock('@/lib/auth', () => ({ getSession: mocks.getSession })) +vi.mock('@/lib/table/application/operations', () => ({ + tableOperations: { list: { id: 'tables.list' } }, +})) +vi.mock('@/lib/table/application/tables', () => ({ + listTableNamesUseCase: { operation: { id: 'tables.list' }, execute: mocks.listNames }, +})) + +import { POST } from '@/app/api/table/names/route' + +function request(body?: unknown) { + return createMockRequest('POST', body, {}, 'http://localhost/api/table/names') +} + +describe('POST /api/table/names', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.getSession.mockResolvedValue({ + user: { id: 'user-1' }, + session: { id: 'session-1' }, + }) + mocks.listNames.mockResolvedValue({ + tables: [{ id: 'table-1', name: 'Accounts' }], + }) + }) + + it('returns the lightweight table-name projection', async () => { + const response = await POST( + request({ workspaceId: 'workspace-1', tableIds: ['table-1', 'table-2'] }), + {} + ) + + expect(response.status).toBe(200) + expect(mocks.listNames.mock.calls[0][0]).toMatchObject({ + principal: { kind: 'session', userId: 'user-1' }, + input: { workspaceId: 'workspace-1', tableIds: ['table-1', 'table-2'] }, + }) + expect(await response.json()).toEqual({ + success: true, + data: { tables: [{ id: 'table-1', name: 'Accounts' }] }, + }) + }) + + it('authenticates before validating the body', async () => { + mocks.getSession.mockResolvedValue(null) + + const response = await POST(request(), {}) + + expect(response.status).toBe(401) + expect(mocks.listNames).not.toHaveBeenCalled() + }) + + it('rejects an empty table ID list', async () => { + const response = await POST(request({ workspaceId: 'workspace-1', tableIds: [] }), {}) + + expect(response.status).toBe(400) + expect(mocks.listNames).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/table/names/route.ts b/apps/sim/app/api/table/names/route.ts new file mode 100644 index 00000000000..349c92dfea1 --- /dev/null +++ b/apps/sim/app/api/table/names/route.ts @@ -0,0 +1,22 @@ +import { listTableNamesContract } from '@/lib/api/contracts/tables' +import { + defineInternalJsonRoute, + internalOrchestrationErrorPolicy, + internalRateLimits, + internalSessionAuth, +} from '@/lib/api/server/routes' +import { tableOperations } from '@/lib/table/application/operations' +import { listTableNamesUseCase } from '@/lib/table/application/tables' + +export const POST = defineInternalJsonRoute({ + contract: listTableNamesContract, + operation: tableOperations.list, + auth: internalSessionAuth, + rateLimit: internalRateLimits.none({ + reason: 'Preserve existing internal table list behavior', + }), + errorPolicy: internalOrchestrationErrorPolicy, + mapInput: ({ body }) => body, + useCase: listTableNamesUseCase, + present: ({ tables }) => ({ success: true as const, data: { tables } }), +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-content.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-content.tsx index bdb533d9773..6ba14c2cb4f 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-content.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-content.tsx @@ -1,10 +1,14 @@ 'use client' import type { RowExecutionMetadata } from '@/lib/table' +import { + CellRender, + type ReferenceCellAction, + resolveCellRender, +} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render' import type { TimezoneState } from '@/hooks/queries/general-settings' import type { SaveReason } from '../../../types' import type { DisplayColumn } from '../types' -import { CellRender, resolveCellRender } from './cell-render' import { InlineEditor } from './inline-editors' interface CellContentProps { @@ -16,6 +20,7 @@ interface CellContentProps { workspaceId: string timeZone: string timezoneStatus: TimezoneState['status'] + referenceColumnsEnabled: boolean isEditing: boolean initialCharacter?: string | null onSave: (value: unknown, reason: SaveReason) => void @@ -28,6 +33,7 @@ interface CellContentProps { waitingOnLabels?: string[] /** Column is an enrichment output — a completed-but-empty cell renders "Not found". */ isEnrichmentOutput?: boolean + referenceAction?: ReferenceCellAction } /** @@ -43,12 +49,14 @@ export function CellContent({ workspaceId, timeZone, timezoneStatus, + referenceColumnsEnabled, isEditing, initialCharacter, onSave, onCancel, waitingOnLabels, isEnrichmentOutput, + referenceAction, }: CellContentProps) { const kind = resolveCellRender({ value, @@ -59,6 +67,7 @@ export function CellContent({ currentWorkspaceId: workspaceId, timeZone, timezoneStatus, + referenceColumnsEnabled, }) return ( @@ -74,7 +83,7 @@ export function CellContent({ /> )} - + ) } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.ts index ab0789ff2d4..02017ddfafb 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.ts @@ -30,6 +30,7 @@ describe('resolveCellRender', () => { exec: undefined, column: column('ttl'), waitingOnLabels: undefined, + referenceColumnsEnabled: false, timeZone: 'America/New_York', }) ).toEqual({ kind: 'date', text: '2023-11-14T17:13:20-05:00' }) @@ -42,6 +43,7 @@ describe('resolveCellRender', () => { exec: undefined, column: column('ttl'), waitingOnLabels: undefined, + referenceColumnsEnabled: false, timeZone: 'America/Los_Angeles', timezoneStatus: 'invalid', }) @@ -55,6 +57,7 @@ describe('resolveCellRender', () => { exec: undefined, column: column('ttl'), waitingOnLabels: undefined, + referenceColumnsEnabled: false, timeZone: 'America/Los_Angeles', timezoneStatus: 'loading', }) @@ -68,6 +71,7 @@ describe('resolveCellRender', () => { exec: undefined, column: column('date'), waitingOnLabels: undefined, + referenceColumnsEnabled: false, timeZone: 'America/Los_Angeles', timezoneStatus: 'error', }) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.tsx new file mode 100644 index 00000000000..1a27df72f2b --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.test.tsx @@ -0,0 +1,204 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { DisplayColumn } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/types' + +vi.mock('@sim/emcn', () => ({ + Badge: ({ children }: { children: React.ReactNode }) => {children}, + Button: ({ + children, + size, + variant, + ...props + }: React.ButtonHTMLAttributes & { + size?: string + variant?: string + }) => ( + + ), + Checkbox: () => null, + ChipTag: ({ + children, + variant, + ...props + }: React.HTMLAttributes & { variant?: string }) => ( + + {children} + + ), + cn: (...values: Array) => values.filter(Boolean).join(' '), + Tooltip: { + Root: ({ children }: { children: React.ReactNode }) => children, + Trigger: ({ children }: { children: React.ReactNode }) => children, + Content: ({ children }: { children: React.ReactNode }) => children, + }, +})) + +vi.mock('@/app/workspace/[workspaceId]/logs/utils', () => ({ + StatusBadge: () => null, +})) + +vi.mock( + '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/sim-resource-cell', + () => ({ SimResourceCell: () => null }) +) + +vi.mock('@/app/workspace/[workspaceId]/tables/[tableId]/components/select-field', () => ({ + resolveSelectOptions: () => [], + SelectPill: () => null, +})) + +import { + CellRender, + resolveCellRender, +} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render' + +const REFERENCE_COLUMN: DisplayColumn = { + id: 'col-account', + key: 'col-account', + name: 'Account', + type: 'reference', + referenceTableId: 'table-accounts', + referenceTableName: 'Accounts', + groupSize: 1, + groupStartColIndex: 0, + headerLabel: 'Account', + isGroupStart: true, +} + +let container: HTMLDivElement +let root: Root + +beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + act(() => { + root = createRoot(container) + }) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() +}) + +describe('reference cell rendering', () => { + it('resolves a stored row ID to a chip labeled with the referenced table name', () => { + expect( + resolveCellRender({ + value: 'row-account-1', + exec: undefined, + column: REFERENCE_COLUMN, + waitingOnLabels: undefined, + referenceColumnsEnabled: true, + }) + ).toEqual({ kind: 'reference-chip', label: 'Accounts' }) + }) + + it('keeps an empty reference cell empty', () => { + expect( + resolveCellRender({ + value: '', + exec: undefined, + column: REFERENCE_COLUMN, + waitingOnLabels: undefined, + referenceColumnsEnabled: true, + }) + ).toEqual({ kind: 'empty' }) + }) + + it('uses a neutral label while the referenced table name is unavailable', () => { + expect( + resolveCellRender({ + value: 'row-account-1', + exec: undefined, + column: { ...REFERENCE_COLUMN, referenceTableName: undefined }, + waitingOnLabels: undefined, + referenceColumnsEnabled: true, + }) + ).toEqual({ kind: 'reference-chip', label: 'Referenced table' }) + }) + + it('renders the stored row ID as plain text when the feature is disabled', () => { + expect( + resolveCellRender({ + value: 'row-account-1', + exec: undefined, + column: REFERENCE_COLUMN, + waitingOnLabels: undefined, + referenceColumnsEnabled: false, + }) + ).toEqual({ kind: 'text', text: 'row-account-1' }) + }) + + it('opens the referenced row from the chip without exposing its stored row ID', () => { + const onReferenceClick = vi.fn() + + act(() => { + root.render( + + ) + }) + + const chip = container.querySelector('button') + expect(chip?.textContent).toBe('Accounts') + expect(chip?.dataset.variant).toBe('ghost') + expect(chip?.dataset.size).toBe('sm') + expect(chip).toHaveProperty('dataset.referenceCellTrigger', '') + expect(chip?.className).toContain('max-w-full') + expect(chip?.className).toContain('p-0') + expect(chip?.querySelector('svg')).toBeNull() + const tag = chip?.querySelector('[data-chip-tag-variant="field"]') + expect(tag?.textContent).toBe('Accounts') + expect(tag?.className).toContain('min-w-0') + expect(tag?.className).toContain('max-w-full') + + act(() => chip?.click()) + + expect(onReferenceClick).toHaveBeenCalledOnce() + expect(container.textContent).not.toContain('row-account-1') + }) + + it('keeps a chip double-click from reaching the reference cell', () => { + const onCellDoubleClick = vi.fn() + const onReferenceClick = vi.fn() + + act(() => { + root.render( +
+ +
+ ) + }) + + act(() => { + const chip = container.querySelector('button') + chip?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 1 })) + chip?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 2 })) + chip?.dispatchEvent(new MouseEvent('dblclick', { bubbles: true, detail: 2 })) + }) + + expect(onReferenceClick).toHaveBeenCalledOnce() + expect(onCellDoubleClick).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx index 22971e399d0..9973c2ef47d 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/cell-render.tsx @@ -2,7 +2,7 @@ import type React from 'react' import { useEffect, useRef, useState } from 'react' -import { Badge, Checkbox, cn, Tooltip } from '@sim/emcn' +import { Badge, Button, Checkbox, ChipTag, cn, Tooltip } from '@sim/emcn' import { parse } from 'tldts' import { faviconUrl } from '@/lib/core/utils/favicon' import type { RowExecutionMetadata, SelectOption } from '@/lib/table' @@ -29,6 +29,7 @@ export type CellRenderKind = // Plain typed cells | { kind: 'boolean'; checked: boolean } | { kind: 'select'; options: SelectOption[] } + | { kind: 'reference-chip'; label: string } | { kind: 'json'; text: string } | { kind: 'date'; text: string; raw?: boolean } | { kind: 'url'; text: string; href: string; domain: string } @@ -58,6 +59,7 @@ interface ResolveCellRenderInput { timeZone?: string /** Invalid or unavailable preferences render time-based values without conversion. */ timezoneStatus?: TimezoneState['status'] + referenceColumnsEnabled: boolean } export function resolveCellRender({ @@ -69,6 +71,7 @@ export function resolveCellRender({ currentWorkspaceId, timeZone, timezoneStatus, + referenceColumnsEnabled, }: ResolveCellRenderInput): CellRenderKind { const isNull = value === null || value === undefined const isEmpty = isNull || value === '' @@ -135,6 +138,16 @@ export function resolveCellRender({ if (column.type === 'select') { return { kind: 'select', options: resolveSelectOptions(column, value) } } + const typeDefinition = columnTypeOf(column) + if (referenceColumnsEnabled && typeDefinition.referencePreview) { + const rowId = typeDefinition.referencePreview.getRowId(value) + return rowId + ? { + kind: 'reference-chip', + label: column.referenceTableName ?? 'Referenced table', + } + : { kind: 'empty' } + } if (isNull) return { kind: 'empty' } // Formatted here rather than in a render branch because the symbol and // fraction digits come from the COLUMN's currency, which the render switch @@ -264,9 +277,19 @@ function extractSimResourceInfo( interface CellRenderProps { kind: CellRenderKind isEditing: boolean + referenceAction?: ReferenceCellAction +} + +export interface ReferenceCellAction { + expanded: boolean + onClick: () => void } -export function CellRender({ kind, isEditing }: CellRenderProps): React.ReactElement | null { +export function CellRender({ + kind, + isEditing, + referenceAction, +}: CellRenderProps): React.ReactElement | null { const valueText = kind.kind === 'value' ? kind.text : null const revealedValueText = useTypewriter(valueText) @@ -388,6 +411,35 @@ export function CellRender({ kind, isEditing }: CellRenderProps): React.ReactEle ) + case 'reference-chip': { + const chip = ( + + {kind.label} + + ) + if (!referenceAction) return chip + return ( + + ) + } + case 'json': return ( + expandedReference: ReferencePreviewTarget | null + onReferenceClick: (target: ReferencePreviewTarget) => void } function cellRangeRowChanged( @@ -121,6 +132,7 @@ function dataRowPropsAreEqual(prev: DataRowProps, next: DataRowProps): boolean { prev.workspaceId !== next.workspaceId || prev.timeZone !== next.timeZone || prev.timezoneStatus !== next.timezoneStatus || + prev.referenceColumnsEnabled !== next.referenceColumnsEnabled || prev.rowIndex !== next.rowIndex || prev.isFirstRow !== next.isFirstRow || prev.editingColumnName !== next.editingColumnName || @@ -145,7 +157,9 @@ function dataRowPropsAreEqual(prev: DataRowProps, next: DataRowProps): boolean { prev.activeDispatches !== next.activeDispatches || prev.pinnedOffsets !== next.pinnedOffsets || prev.lastPinnedColKey !== next.lastPinnedColKey || - prev.findMatchColumns !== next.findMatchColumns + prev.findMatchColumns !== next.findMatchColumns || + prev.expandedReference !== next.expandedReference || + prev.onReferenceClick !== next.onReferenceClick ) { return false } @@ -170,6 +184,7 @@ export const DataRow = React.memo(function DataRow({ workspaceId, timeZone, timezoneStatus, + referenceColumnsEnabled, rowIndex, isFirstRow, editingColumnName, @@ -197,6 +212,8 @@ export const DataRow = React.memo(function DataRow({ pinnedOffsets, lastPinnedColKey, findMatchColumns, + expandedReference, + onReferenceClick, }: DataRowProps) { const sel = normalizedSelection /** @@ -310,6 +327,24 @@ export const DataRow = React.memo(function DataRow({ {columns.map((column, colIndex) => { + const value = + pendingCellValue && column.key in pendingCellValue + ? pendingCellValue[column.key] + : row.data[column.key] + const referencePreview = referenceColumnsEnabled + ? columnTypeOf(column).referencePreview + : undefined + const referenceRowId = referencePreview?.getRowId(value) ?? null + const referenceTableId = referencePreview?.getTableId(column) + const referenceTarget = + referenceTableId && referenceRowId + ? { + sourceRowId: row.id, + sourceColumnKey: column.key, + referenceTableId, + referenceRowId, + } + : null const inRange = sel !== null && rowIndex >= sel.startRow && @@ -407,11 +442,8 @@ export const DataRow = React.memo(function DataRow({ workspaceId={workspaceId} timeZone={timeZone} timezoneStatus={timezoneStatus} - value={ - pendingCellValue && column.key in pendingCellValue - ? pendingCellValue[column.key] - : row.data[column.key] - } + referenceColumnsEnabled={referenceColumnsEnabled} + value={value} exec={resolveCellExec( row, column.workflowGroupId @@ -435,6 +467,14 @@ export const DataRow = React.memo(function DataRow({ 'enrichment' : false } + referenceAction={ + referenceTarget + ? { + expanded: isSameReferencePreviewTarget(expandedReference, referenceTarget), + onClick: () => onReferenceClick(referenceTarget), + } + : undefined + } /> diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview.test.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview.test.tsx new file mode 100644 index 00000000000..d644476759c --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview.test.tsx @@ -0,0 +1,425 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createTableColumn, createTableDefinition, createTableRow } from '@sim/testing' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { previewQuery } = vi.hoisted(() => ({ + previewQuery: { + data: undefined as ReturnType | null | undefined, + }, +})) + +vi.mock('@/lib/table/column-types', () => ({ + columnTypeById: () => ({ icon: () => null }), + columnTypeOf: (column: { type: string; referenceTableId?: string }) => ({ + referencePreview: + column.type === 'reference' + ? { + getTableId: () => column.referenceTableId, + } + : undefined, + }), +})) + +vi.mock('@sim/emcn/icons', () => ({ + Loader: ({ animate }: { animate?: boolean }) => ( + + ), + SquareArrowUpRight: () => , +})) + +vi.mock('@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells', () => ({ + CellContent: ({ column, value }: { column: { referenceTableName?: string }; value: unknown }) => ( + {String(value)} + ), +})) + +vi.mock( + '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/column-type-icon', + () => ({ ColumnTypeIcon: () => null }) +) + +import { + REFERENCE_ROW_PREVIEW_HEIGHT, + ReferenceRowPreview, +} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview' + +let container: HTMLDivElement +let root: Root +let previewTable: ReturnType | undefined +let previewStatus: 'loading' | 'error' | 'ready' +const REFERENCE_TABLE_NAMES = new Map([ + ['table-accounts', 'Accounts'], + ['table-owners', 'Owners'], +]) + +beforeEach(() => { + globalThis.IS_REACT_ACT_ENVIRONMENT = true + const columns = [ + createTableColumn({ id: 'col-name', name: 'Name', type: 'string' }), + createTableColumn({ id: 'col-tier', name: 'Tier', type: 'string' }), + ] + previewTable = createTableDefinition({ + id: 'table-accounts', + name: 'Accounts', + columns, + }) + previewQuery.data = createTableRow({ + id: 'row-account-1', + data: { 'col-name': 'Acme', 'col-tier': 'Enterprise' }, + }) + previewStatus = 'ready' + container = document.createElement('div') + document.body.appendChild(container) + act(() => { + root = createRoot(container) + }) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() + vi.restoreAllMocks() + vi.unstubAllGlobals() +}) + +function renderPreview() { + if (previewStatus === 'ready' && !previewTable) { + throw new Error('Ready preview fixture requires a table') + } + const previewState = + previewStatus === 'ready' + ? ({ status: 'ready', table: previewTable, row: previewQuery.data ?? null } as const) + : ({ status: previewStatus } as const) + const preview = ( + + + + +
+ ) + + act(() => { + root.render(
{preview}
) + }) +} + +function horizontalRect(left: number, right: number): DOMRect { + return { + bottom: 0, + height: 0, + left, + right, + top: 0, + width: right - left, + x: left, + y: 0, + toJSON: () => ({}), + } +} + +describe('ReferenceRowPreview', () => { + it('shows only a loading state until the referenced schema and row are ready', () => { + previewStatus = 'loading' + previewTable = undefined + previewQuery.data = undefined + + renderPreview() + + expect(container.querySelector('[data-testid="reference-preview-loader"]')).not.toBeNull() + expect(container.querySelector('[role="table"]')).toBeNull() + expect(container.textContent).not.toContain('Table unavailable') + }) + + it('shows the referenced table schema and the matching row inline', () => { + renderPreview() + + expect(container.textContent).toContain('Accounts') + expect(container.textContent).toContain('Name') + expect(container.textContent).toContain('Tier') + expect(container.textContent).toContain('Acme') + expect(container.textContent).toContain('Enterprise') + expect(container.textContent).not.toContain('Open in sub view') + const goToTableLink = container.querySelector('a[aria-label="Go to table"]') + expect(goToTableLink?.getAttribute('href')).toBe('/workspace/workspace-1/tables/table-accounts') + expect(goToTableLink?.getAttribute('title')).toBe('Go to table') + expect(goToTableLink).toHaveProperty('dataset.referenceCellTrigger', '') + expect(goToTableLink?.className).toContain('size-[20px]') + expect(goToTableLink?.className).toContain('hover-hover:bg-[var(--surface-active)]') + expect(goToTableLink?.parentElement?.className).toContain('h-9') + expect(goToTableLink?.parentElement?.className).toContain('gap-1.5') + expect(goToTableLink?.previousElementSibling?.textContent).toBe('Accounts') + expect(goToTableLink?.previousElementSibling?.className).not.toContain('font-medium') + expect(goToTableLink?.textContent).toBe('') + expect( + goToTableLink?.querySelector('[data-testid="square-arrow-up-right-icon"]') + ).not.toBeNull() + const previewShell = container.querySelector('tbody > tr > td > div > div') + expect(previewShell?.lastElementChild?.className).toContain('h-9') + expect(previewShell?.lastElementChild?.querySelector('a')).toBeNull() + const previewCell = container.querySelector('tbody > tr > td') + expect(previewCell?.className).toContain('overflow-clip') + expect(previewCell?.className).toContain('border-r') + expect(container.querySelector('td > div')?.className).toContain('sticky left-0') + expect(container.querySelector('td > div')?.className).toContain('w-0') + expect(container.querySelector('td > div')?.className).toContain( + `h-[${REFERENCE_ROW_PREVIEW_HEIGHT}px]` + ) + const subtable = container.querySelector('[role="table"]') + expect(subtable?.className).toContain('w-full') + expect(subtable?.className).toContain('h-full') + expect(subtable?.className).not.toContain('cursor-default') + expect(subtable?.className).not.toContain('select-none') + expect(subtable?.className).toContain('grid-rows-2') + expect(subtable?.querySelectorAll('[role="row"]')).toHaveLength(2) + expect(subtable?.querySelectorAll('[role="columnheader"]')).toHaveLength(2) + expect(subtable?.querySelectorAll('[role="cell"]')).toHaveLength(2) + const dataValueWrappers = subtable?.querySelectorAll('[role="cell"] > div') ?? [] + expect( + Array.from(dataValueWrappers).every( + (node) => + node.classList.contains('w-full') && + node.classList.contains('min-w-0') && + node.classList.contains('overflow-clip') + ) + ).toBe(true) + const subtableViewport = container.querySelector('.overscroll-x-contain') + expect(subtableViewport?.className).toContain('overflow-x-auto') + expect(subtableViewport?.className).toContain('overflow-y-hidden') + expect(subtableViewport?.className).toContain('border-y') + expect(container.innerHTML).not.toContain('rounded-md') + }) + + it('passes referenced table names to reference cells in the preview', () => { + const referenceColumn = createTableColumn({ + id: 'col-owner', + name: 'Owner', + }) + Object.assign(referenceColumn, { + type: 'reference', + referenceTableId: 'table-owners', + }) + previewTable = createTableDefinition({ + id: 'table-accounts', + name: 'Accounts', + columns: [referenceColumn], + }) + previewQuery.data = createTableRow({ + id: 'row-account-1', + data: { 'col-owner': 'row-owner-1' }, + }) + + renderPreview() + + const referenceValue = container.querySelector('[data-reference-table-name="Owners"]') + expect(referenceValue?.textContent).toBe('row-owner-1') + }) + + it('scrolls horizontally when wheel input starts on cell text', () => { + renderPreview() + + const subtableViewport = container.querySelector('.overscroll-x-contain') + const cellText = Array.from(container.querySelectorAll('[role="cell"] span')).find( + (element) => element.textContent === 'Acme' + ) + if (!subtableViewport || !cellText) throw new Error('Expected the referenced row preview') + + const wheelEvent = new WheelEvent('wheel', { + bubbles: true, + cancelable: true, + deltaX: 80, + }) + act(() => { + cellText.dispatchEvent(wheelEvent) + }) + + expect(subtableViewport.scrollLeft).toBe(80) + expect(wheelEvent.defaultPrevented).toBe(true) + + const verticalWheelEvent = new WheelEvent('wheel', { + bubbles: true, + cancelable: true, + deltaX: 10, + deltaY: 80, + }) + act(() => { + cellText.dispatchEvent(verticalWheelEvent) + }) + + expect(subtableViewport.scrollLeft).toBe(80) + expect(verticalWheelEvent.defaultPrevented).toBe(false) + }) + + it('sizes the inner scroller to the visible portion of the preview cell', () => { + let previewCellRight = 1_500 + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function () { + if (this.matches('[data-table-scroll]')) return horizontalRect(100, 920) + if (this.matches('tbody > tr > td')) return horizontalRect(-500, previewCellRight) + return horizontalRect(0, 0) + }) + vi.spyOn(Element.prototype, 'clientWidth', 'get').mockImplementation(function () { + return this.matches('[data-table-scroll]') ? 800 : 0 + }) + renderPreview() + + const previewShell = container.querySelector('tbody > tr > td > div > div') + expect(previewShell?.style.getPropertyValue('--reference-preview-width')).toBe('800px') + + previewCellRight = 780 + const scrollRoot = container.querySelector('[data-table-scroll]') + if (!scrollRoot) throw new Error('Expected the table scroll root to be rendered') + scrollRoot.scrollLeft = 120 + act(() => { + scrollRoot.dispatchEvent(new Event('scroll')) + }) + + expect(previewShell?.style.getPropertyValue('--reference-preview-width')).toBe('680px') + }) + + it('updates on resize and releases its observer and scroll listener', () => { + let previewCellRight = 1_500 + let resizeCallback: ResizeObserverCallback | null = null + let resizeObserver: ResizeObserver | null = null + const observe = vi.fn() + const disconnect = vi.fn() + + class MockResizeObserver implements ResizeObserver { + constructor(callback: ResizeObserverCallback) { + resizeCallback = callback + resizeObserver = this + } + + observe(target: Element, options?: ResizeObserverOptions) { + observe(target, options) + } + + unobserve() {} + + disconnect() { + disconnect() + } + } + + vi.stubGlobal('ResizeObserver', MockResizeObserver) + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function () { + if (this.matches('[data-table-scroll]')) return horizontalRect(100, 900) + if (this.matches('tbody > tr > td')) return horizontalRect(-500, previewCellRight) + return horizontalRect(0, 0) + }) + vi.spyOn(Element.prototype, 'clientWidth', 'get').mockImplementation(function () { + return this.matches('[data-table-scroll]') ? 800 : 0 + }) + const registeredListeners: Array<{ + target: EventTarget + type: string + listener: EventListenerOrEventListenerObject | null + }> = [] + const removedListeners: typeof registeredListeners = [] + const originalAddEventListener = EventTarget.prototype.addEventListener + const originalRemoveEventListener = EventTarget.prototype.removeEventListener + vi.spyOn(EventTarget.prototype, 'addEventListener').mockImplementation( + function (type, listener, options) { + registeredListeners.push({ target: this, type, listener }) + originalAddEventListener.call(this, type, listener, options) + } + ) + vi.spyOn(EventTarget.prototype, 'removeEventListener').mockImplementation( + function (type, listener, options) { + removedListeners.push({ target: this, type, listener }) + originalRemoveEventListener.call(this, type, listener, options) + } + ) + + renderPreview() + + const previewShell = container.querySelector('tbody > tr > td > div > div') + const scrollRoot = container.querySelector('[data-table-scroll]') + const previewCell = container.querySelector('tbody > tr > td') + const previewViewport = container.querySelector('.overscroll-x-contain') + if (!scrollRoot) throw new Error('Expected the table scroll root to be rendered') + if (!previewCell) throw new Error('Expected the preview cell to be rendered') + if (!previewViewport) throw new Error('Expected the preview viewport to be rendered') + const scrollListener = registeredListeners.find( + ({ target, type }) => target === scrollRoot && type === 'scroll' + )?.listener + const wheelListener = registeredListeners.find( + ({ target, type }) => target === previewViewport && type === 'wheel' + )?.listener + if (!scrollListener) throw new Error('Expected the scroll listener to be registered') + if (!wheelListener) throw new Error('Expected the wheel listener to be registered') + expect(observe).toHaveBeenCalledTimes(2) + expect(observe.mock.calls.some(([target]) => target === scrollRoot)).toBe(true) + expect(observe.mock.calls.some(([target]) => target === previewCell)).toBe(true) + + previewCellRight = 780 + if (!resizeCallback || !resizeObserver) { + throw new Error('Expected the resize observer to be initialized') + } + act(() => resizeCallback([], resizeObserver)) + + expect(previewShell?.style.getPropertyValue('--reference-preview-width')).toBe('680px') + + act(() => root.render(null)) + + expect(disconnect).toHaveBeenCalledOnce() + expect(removedListeners).toContainEqual({ + target: scrollRoot, + type: 'scroll', + listener: scrollListener, + }) + expect(removedListeners).toContainEqual({ + target: previewViewport, + type: 'wheel', + listener: wheelListener, + }) + }) + + it('shows no match when the stored row ID does not resolve', () => { + previewQuery.data = null + + renderPreview() + + expect(container.textContent).toContain('No matching row') + }) + + it('keeps non-404 failures distinct from missing rows', () => { + previewStatus = 'error' + + renderPreview() + + expect(container.textContent).toContain("Couldn't load reference") + expect(container.textContent).not.toContain('No matching row') + expect(container.querySelector('[data-testid="reference-preview-loader"]')).toBeNull() + }) + + it('shows an empty-schema state when the referenced table has no columns', () => { + if (!previewTable) throw new Error('Expected the referenced table fixture') + previewTable.schema.columns = [] + + renderPreview() + + expect(container.textContent).toContain('This table has no columns') + }) + + it('preserves a missing row for an empty schema', () => { + if (!previewTable) throw new Error('Expected the referenced table fixture') + previewTable.schema.columns = [] + previewQuery.data = null + + renderPreview() + expect(container.textContent).toContain('No matching row') + expect(container.textContent).not.toContain('This table has no columns') + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview.tsx new file mode 100644 index 00000000000..e267967fec9 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview.tsx @@ -0,0 +1,248 @@ +'use client' + +import { memo, type ReactNode, useLayoutEffect, useMemo, useRef } from 'react' +import { buttonVariants } from '@sim/emcn' +import { Loader, SquareArrowUpRight } from '@sim/emcn/icons' +import { noop } from '@sim/utils/helpers' +import Link from 'next/link' +import type { GetTableRowResponse } from '@/lib/api/contracts/tables' +import type { TableDefinition } from '@/lib/table' +import { columnTypeById } from '@/lib/table/column-types' +import { CellContent } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells' +import { ColumnTypeIcon } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/column-type-icon' +import { expandToDisplayColumns } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils' +import type { TimezoneState } from '@/hooks/queries/general-settings' + +/** + * Must match the sticky anchor's `h-[144px]` class below because the row + * virtualizer reserves this exact height. The zero-width anchor stays sticky + * across the full table width without JavaScript-driven positioning. + */ +export const REFERENCE_ROW_PREVIEW_HEIGHT = 144 + +const ReferenceIcon = columnTypeById('reference').icon + +interface ReferenceRowPreviewBaseProps { + workspaceId: string + timeZone: string + timezoneStatus: TimezoneState['status'] + referenceColumnsEnabled: boolean + referenceTableId: string + referenceTableNames: ReadonlyMap + colSpan: number +} + +type ReferenceRowPreviewProps = ReferenceRowPreviewBaseProps & + ( + | { status: 'loading' | 'error' } + | { + status: 'ready' + table: TableDefinition + row: GetTableRowResponse['data']['row'] | null + } + ) + +export const ReferenceRowPreview = memo(function ReferenceRowPreview( + props: ReferenceRowPreviewProps +) { + const { + workspaceId, + timeZone, + timezoneStatus, + referenceColumnsEnabled, + referenceTableId, + status, + referenceTableNames, + colSpan, + } = props + const table = status === 'ready' ? props.table : undefined + const row = status === 'ready' ? props.row : undefined + const previewCellRef = useRef(null) + const previewShellRef = useRef(null) + const previewViewportRef = useRef(null) + const columns = useMemo( + () => expandToDisplayColumns(table?.schema.columns ?? [], [], referenceTableNames), + [table?.schema.columns, referenceTableNames] + ) + + useLayoutEffect(() => { + const previewCell = previewCellRef.current + const previewShell = previewShellRef.current + const scrollRoot = previewCell?.closest('[data-table-scroll]') + if (!previewCell || !previewShell || !scrollRoot) return + + let previousWidth: number | null = null + let previousScrollLeft = scrollRoot.scrollLeft + + const updateWidth = () => { + const cellBounds = previewCell.getBoundingClientRect() + const viewportBounds = scrollRoot.getBoundingClientRect() + const viewportLeft = viewportBounds.left + scrollRoot.clientLeft + const viewportRight = viewportLeft + scrollRoot.clientWidth + const visibleLeft = Math.max(cellBounds.left, viewportLeft) + const visibleRight = Math.min(cellBounds.right, viewportRight) + const width = Math.max(0, visibleRight - visibleLeft) + if (width === previousWidth) return + previousWidth = width + previewShell.style.setProperty('--reference-preview-width', `${width}px`) + } + + const handleScroll = () => { + if (scrollRoot.scrollLeft === previousScrollLeft) return + previousScrollLeft = scrollRoot.scrollLeft + updateWidth() + } + + updateWidth() + scrollRoot.addEventListener('scroll', handleScroll, { passive: true }) + + const resizeObserver = + typeof ResizeObserver === 'undefined' ? null : new ResizeObserver(updateWidth) + resizeObserver?.observe(scrollRoot) + resizeObserver?.observe(previewCell) + + return () => { + scrollRoot.removeEventListener('scroll', handleScroll) + resizeObserver?.disconnect() + } + }, []) + + useLayoutEffect(() => { + const previewViewport = previewViewportRef.current + if (!previewViewport) return + + const handleWheel = (event: WheelEvent) => { + if (Math.abs(event.deltaX) <= Math.abs(event.deltaY)) return + event.preventDefault() + previewViewport.scrollLeft += event.deltaX + } + + previewViewport.addEventListener('wheel', handleWheel, { passive: false }) + return () => previewViewport.removeEventListener('wheel', handleWheel) + }, [status]) + + let content: ReactNode + if (columns.length === 0 && !row) { + content = ( +
+ No matching row +
+ ) + } else if (columns.length === 0) { + content = ( +
+ This table has no columns +
+ ) + } else { + content = ( +
+
+ {columns.map((column) => ( +
+ + + {column.name} + +
+ ))} +
+
+
+ {!row ? ( +
+ No matching row +
+ ) : ( + <> + {columns.map((column) => ( +
+
+ +
+
+ ))} +
+ + )} +
+
+ ) + } + + return ( + + +
+
+ {status === 'loading' ? ( +
+ +
+ ) : status === 'error' ? ( +
+ Couldn't load reference +
+ ) : ( + <> +
+ + {table?.name} + + + +
+ +
+ {content} +
+ +
+ + )} +
+
+ + + ) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index 82457145a8b..7a224ee8af6 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -1,7 +1,7 @@ 'use client' import type React from 'react' -import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' +import { Fragment, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { cn, toast, useToast } from '@sim/emcn' import { Loader, TableX } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' @@ -31,6 +31,14 @@ import { cellValueFilterConditions } from '@/lib/table/query-builder/cell-filter import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' import { FindBar } from '@/app/workspace/[workspaceId]/components' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' +import { + REFERENCE_ROW_PREVIEW_HEIGHT, + ReferenceRowPreview, +} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/reference-row-preview' +import type { + DisplayColumn, + ReferencePreviewTarget, +} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/types' import { getTimezoneEditBlockedMessage } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/timezone-editing' import type { RemoteTableSelection } from '@/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-room' import type { BlockedTableAction } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' @@ -43,6 +51,8 @@ import { useDeleteColumn, useDeleteWorkflowGroup, useFindTableRows, + useReferenceRowPreview, + useTableNames, useTableRunState, useUpdateColumn, useUpdateTableMetadata, @@ -68,7 +78,6 @@ import { ColumnHeaderMenu, WorkflowGroupMetaCell } from './headers' import { RemoteSelectionOverlay } from './remote-selection-overlay' import { exceedsTablePasteRowLimit, parseBoundedTsv } from './table-paste' import { AddRowButton, SelectAllCheckbox, TableColGroup } from './table-primitives' -import type { DisplayColumn } from './types' import { buildHeaderGroups, buildTableSelectionContext, @@ -84,6 +93,7 @@ import { expandToDisplayColumns, horizontalEdgeScrollVelocity, isCellInSelection, + isSameReferencePreviewTarget, moveCell, ROW_SELECTION_ALL, ROW_SELECTION_NONE, @@ -600,6 +610,24 @@ export function TableGrid({ // (and one the server rejects outright). filter: effectiveFilter, } = useTable({ workspaceId, tableId, queryOptions }) + const referencedTableIds = useMemo( + () => + referenceColumnsEnabled + ? columns.flatMap((column) => { + const referenceTableId = columnTypeOf(column).referencePreview?.getTableId(column) + return referenceTableId ? [referenceTableId] : [] + }) + : [], + [columns, referenceColumnsEnabled] + ) + const { data: referencedTables } = useTableNames(workspaceId, referencedTableIds) + const referenceTableNames = useMemo(() => { + const names = new Map() + for (const table of referencedTables ?? []) { + names.set(table.id, table.name) + } + return names + }, [referencedTables]) /** Sort is single-column, so only the first spec entry can be active. */ const activeSort = queryOptions.sort?.[0] @@ -658,19 +686,7 @@ export function TableGrid({ */ const [headerHeight, setHeaderHeight] = useState(0) const [rowHeight, setRowHeight] = useState(ROW_HEIGHT_ESTIMATE) - - const rowVirtualizer = useVirtualizer({ - count: rows.length, - getScrollElement: () => scrollRef.current, - estimateSize: () => rowHeight, - overscan: 12, - scrollMargin: headerHeight, - getItemKey: (index) => rows[index]?.id ?? index, - }) - - useEffect(() => { - rowVirtualizer.measure() - }, [rowHeight, rowVirtualizer]) + const [expandedReference, setExpandedReference] = useState(null) useLayoutEffect(() => { const el = theadRef.current @@ -904,8 +920,61 @@ export function TableGrid({ const hidden = new Set(hiddenColumns) ordered = ordered.filter((col) => !hidden.has(getColumnId(col))) } - return expandToDisplayColumns(ordered, tableWorkflowGroups) - }, [columns, columnOrder, hiddenColumns, tableWorkflowGroups]) + return expandToDisplayColumns(ordered, tableWorkflowGroups, referenceTableNames) + }, [columns, columnOrder, hiddenColumns, tableWorkflowGroups, referenceTableNames]) + + const activeReferenceTarget = useMemo(() => { + if (!referenceColumnsEnabled || !expandedReference) return null + const sourceRow = rows.find((row) => row.id === expandedReference.sourceRowId) + const sourceColumn = displayColumns.find( + (column) => column.key === expandedReference.sourceColumnKey + ) + const referencePreview = sourceColumn ? columnTypeOf(sourceColumn).referencePreview : undefined + if (!sourceRow || !sourceColumn || !referencePreview) return null + return referencePreview.getRowId(sourceRow.data[expandedReference.sourceColumnKey]) === + expandedReference.referenceRowId && + referencePreview.getTableId(sourceColumn) === expandedReference.referenceTableId + ? expandedReference + : null + }, [displayColumns, rows, expandedReference, referenceColumnsEnabled]) + const referencePreviewQuery = useReferenceRowPreview({ + workspaceId, + tableId: activeReferenceTarget?.referenceTableId, + rowId: activeReferenceTarget?.referenceRowId, + sourceRowId: activeReferenceTarget?.sourceRowId, + sourceColumnKey: activeReferenceTarget?.sourceColumnKey, + }) + const previewReferenceTableNames = useMemo(() => { + const names = new Map(referenceTableNames) + for (const table of referencePreviewQuery.data?.referenceTables ?? []) { + names.set(table.id, table.name) + } + return names + }, [referenceTableNames, referencePreviewQuery.data?.referenceTables]) + const referencePreviewState = referencePreviewQuery.isError + ? ({ status: 'error' } as const) + : referencePreviewQuery.isFetching || !referencePreviewQuery.data + ? ({ status: 'loading' } as const) + : ({ + status: 'ready', + table: referencePreviewQuery.data.table, + row: referencePreviewQuery.data.row, + } as const) + const expandedSourceRowId = activeReferenceTarget?.sourceRowId ?? null + + const rowVirtualizer = useVirtualizer({ + count: rows.length, + getScrollElement: () => scrollRef.current, + estimateSize: (index) => + rowHeight + (rows[index]?.id === expandedSourceRowId ? REFERENCE_ROW_PREVIEW_HEIGHT : 0), + overscan: 12, + scrollMargin: headerHeight, + getItemKey: (index) => rows[index]?.id ?? index, + }) + + useEffect(() => { + rowVirtualizer.measure() + }, [rowHeight, expandedSourceRowId, rowVirtualizer]) /** Column id → its rendered index (matches the cells' `data-col`), for placing overlays. * Only built when collaborators are present (the overlay it feeds is gated on that too), @@ -2756,6 +2825,14 @@ export function TableGrid({ [] ) + const handleReferenceClick = useCallback((target: ReferencePreviewTarget) => { + setEditingCell(null) + setInitialCharacter(null) + setExpandedReference((current) => + isSameReferencePreviewTarget(current, target) ? null : target + ) + }, []) + const handleCellDoubleClick = useCallback( (rowId: string, columnName: string, columnKey: string) => { const column = columnsRef.current.find((c) => c.key === columnKey) @@ -2872,8 +2949,15 @@ export function TableGrid({ if (!el) return const handleKeyDown = (e: KeyboardEvent) => { - const tag = (e.target as HTMLElement).tagName - if (tag === 'INPUT' || tag === 'TEXTAREA' || tag === 'SELECT') return + const target = e.target as HTMLElement + const tag = target.tagName + if ( + tag === 'INPUT' || + tag === 'TEXTAREA' || + tag === 'SELECT' || + target.closest('[data-reference-cell-trigger]') + ) + return if ((e.metaKey || e.ctrlKey) && (e.key === 'z' || e.key === 'y')) { e.preventDefault() @@ -4707,7 +4791,7 @@ export function TableGrid({ ref={scrollRef} tabIndex={-1} className={cn( - 'min-h-0 flex-1 overflow-auto overscroll-none outline-none', + 'min-h-0 flex-1 overflow-auto overscroll-none outline-none [container-type:inline-size]', resizingColumn && 'select-none' )} data-table-scroll @@ -4973,50 +5057,70 @@ export function TableGrid({ const index = virtualRow.index const row = rows[index] if (!row) return null + const rowReferenceTarget = + activeReferenceTarget?.sourceRowId === row.id + ? activeReferenceTarget + : null return ( - 0 ? pinnedOffsets : undefined} - lastPinnedColKey={lastPinnedColKey} - findMatchColumns={findMatchColumnsByRowId.get(row.id)} - /> + + 0 ? pinnedOffsets : undefined} + lastPinnedColKey={lastPinnedColKey} + findMatchColumns={findMatchColumnsByRowId.get(row.id)} + expandedReference={rowReferenceTarget} + onReferenceClick={handleReferenceClick} + /> + {rowReferenceTarget ? ( + + ) : null} + ) })} {paddingBottom > 0 && ( diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/types.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/types.ts index af5cceea88c..5e793ab8ee0 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/types.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/types.ts @@ -22,6 +22,7 @@ export interface ColumnSourceInfo { export interface DisplayColumn extends ColumnDefinition { /** Stable per-visual-column identifier (= column.name). */ key: string + referenceTableName?: string /** Block id producing this column's value (workflow-output columns only). */ outputBlockId?: string /** Pluck path the workflow ran for this column. */ @@ -35,3 +36,10 @@ export interface DisplayColumn extends ColumnDefinition { /** True when this is the leftmost sibling of its group (or non-grouped). */ isGroupStart: boolean } + +export interface ReferencePreviewTarget { + sourceRowId: string + sourceColumnKey: string + referenceTableId: string + referenceRowId: string +} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts index 80534939ca4..e4fc58c8bf6 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.test.ts @@ -13,7 +13,9 @@ import { canWriteRowsWithChip, chipRowCount, drainTargetForChip, + expandToDisplayColumns, horizontalEdgeScrollVelocity, + isSameReferencePreviewTarget, selectedColumnIds, } from './utils' @@ -26,6 +28,42 @@ function columns(count: number): DisplayColumn[] { const rowIds = (count: number) => Array.from({ length: count }, (_, i) => `r${i}`) +describe('expandToDisplayColumns', () => { + it('attaches the referenced table name to reference display columns', () => { + const [column] = expandToDisplayColumns( + [ + { + id: 'account-column', + name: 'Account', + type: 'reference', + referenceTableId: 'accounts-table', + }, + ], + [], + new Map([['accounts-table', 'Accounts']]) + ) + + expect(column).toMatchObject({ referenceTableName: 'Accounts' }) + }) +}) + +describe('isSameReferencePreviewTarget', () => { + const target = { + sourceRowId: 'source-row', + sourceColumnKey: 'account-column', + referenceTableId: 'accounts-table', + referenceRowId: 'account-row', + } + + it('matches only the same source cell and referenced row', () => { + expect(isSameReferencePreviewTarget(target, target)).toBe(true) + expect(isSameReferencePreviewTarget(null, target)).toBe(false) + for (const key of Object.keys(target) as Array) { + expect(isSameReferencePreviewTarget({ ...target, [key]: 'different' }, target)).toBe(false) + } + }) +}) + describe('horizontalEdgeScrollVelocity', () => { const getVelocity = (pointerX: number) => horizontalEdgeScrollVelocity({ diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts index 4f3e9282d17..1198a7d95f5 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts @@ -12,11 +12,15 @@ import type { WorkflowGroup, } from '@/lib/table' import { getColumnId } from '@/lib/table/column-keys' +import { columnTypeOf } from '@/lib/table/column-types' import { TABLE_LIMITS } from '@/lib/table/constants' import { areGroupDepsSatisfied, areOutputsFilled } from '@/lib/table/deps' +import type { + DisplayColumn, + ReferencePreviewTarget, +} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/types' import type { ChatContext } from '@/stores/panel' import type { DeletedRowSnapshot } from '@/stores/table/types' -import type { DisplayColumn } from './types' /** * `all` means "every row matching the active filter" — including rows not yet loaded by the @@ -31,6 +35,18 @@ export type RowSelection = export const ROW_SELECTION_NONE: RowSelection = { kind: 'none' } export const ROW_SELECTION_ALL: RowSelection = { kind: 'all' } +export function isSameReferencePreviewTarget( + left: ReferencePreviewTarget | null, + right: ReferencePreviewTarget +): boolean { + return ( + left?.sourceRowId === right.sourceRowId && + left.sourceColumnKey === right.sourceColumnKey && + left.referenceTableId === right.referenceTableId && + left.referenceRowId === right.referenceRowId + ) +} + interface HorizontalEdgeScrollVelocityInput { pointerX: number visibleLeft: number @@ -157,6 +173,14 @@ export type HeaderGroup = workflowId: string } +function resolveReferenceTableName( + column: ColumnDefinition, + referenceTableNames: ReadonlyMap | undefined +): string | undefined { + const tableId = columnTypeOf(column).referencePreview?.getTableId(column) + return tableId ? referenceTableNames?.get(tableId) : undefined +} + /** * Flat schema → one DisplayColumn per ColumnDefinition. Pre-pass computes * `groupSize` and `groupStartColIndex` for every consecutive run of columns @@ -165,7 +189,8 @@ export type HeaderGroup = */ export function expandToDisplayColumns( columns: ColumnDefinition[], - workflowGroups: WorkflowGroup[] + workflowGroups: WorkflowGroup[], + referenceTableNames?: ReadonlyMap ): DisplayColumn[] { const out: DisplayColumn[] = [] const groupById = new Map(workflowGroups.map((g) => [g.id, g])) @@ -194,6 +219,7 @@ export function expandToDisplayColumns( out.push({ ...child, key: getColumnId(child), + referenceTableName: resolveReferenceTableName(child, referenceTableNames), outputBlockId: output?.blockId, outputPath: output?.path, groupSize: size, @@ -207,6 +233,7 @@ export function expandToDisplayColumns( out.push({ ...column, key: getColumnId(column), + referenceTableName: resolveReferenceTableName(column, referenceTableNames), groupSize: 1, groupStartColIndex: out.length, headerLabel: column.name, diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts index b6fcb8a963c..aa71c79ac3b 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts @@ -13,6 +13,7 @@ import { storageServiceMockFns, } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { MAX_FORK_RESOURCE_IDS_PER_TYPE } from '@/lib/api/contracts/workspace-fork' import { hashDurableSecretProvenanceValue } from '@/lib/execution/durable-secret-provenance' import { bindKnowledgeDocumentFieldSecretProvenance, @@ -175,6 +176,61 @@ describe('copyForkResourceContent', () => { expect(inserted[0]).toEqual(expect.objectContaining({ secretProvenanceVersion: null })) }) + it('rewrites reference cells to the copied referenced-row identity', async () => { + const updatedAt = new Date('2026-08-05T00:00:00.000Z') + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + row: { + id: 'row-order-1', + tableId: 'src-orders', + workspaceId: 'src-ws', + data: { 'col-account': 'row-account-1' }, + secretProvenanceVersion: null, + updatedAt, + }, + provenance: null, + provenanceIsCurrent: false, + }, + ]) + .mockResolvedValueOnce([ + { + row: { + id: 'row-account-1', + tableId: 'src-accounts', + workspaceId: 'src-ws', + data: { 'col-name': 'Acme' }, + secretProvenanceVersion: null, + updatedAt, + }, + provenance: null, + provenanceIsCurrent: false, + }, + ]) + + const result = await copyForkResourceContent({ + contentPlan: basePlan({ + tables: [ + { + sourceId: 'src-orders', + childId: 'child-orders', + dependsOnChildIds: ['child-accounts'], + referenceColumnTargetTableIds: { 'col-account': 'child-accounts' }, + }, + { sourceId: 'src-accounts', childId: 'child-accounts' }, + ], + }), + requestId: 'test', + }) + + expect(result.failed).toBe(0) + const copiedOrderRows = dbChainMockFns.values.mock.calls[0][0] as Array<{ + data: Record + }> + const copiedAccountRows = dbChainMockFns.values.mock.calls[1][0] as Array<{ id: string }> + expect(copiedOrderRows[0].data['col-account']).toBe(copiedAccountRows[0].id) + }) + it('turns stale tracked table provenance into unknown instead of laundering it', async () => { const rowUpdatedAt = new Date('2026-08-05T00:00:00.000Z') dbChainMockFns.limit.mockResolvedValueOnce([ @@ -260,6 +316,51 @@ describe('copyForkResourceContent', () => { ]) }) + it('fails copied tables whose referenced-table dependency failed to copy', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + row: { + id: 'row-order-1', + tableId: 'src-orders', + workspaceId: 'src-ws', + data: { 'col-account': 'row-account-1' }, + secretProvenanceVersion: null, + updatedAt: new Date('2026-08-05T00:00:00.000Z'), + }, + provenance: null, + provenanceIsCurrent: false, + }, + ]) + .mockRejectedValueOnce(new Error('copy failed')) + + const result = await copyForkResourceContent({ + contentPlan: basePlan({ + tables: [ + { + sourceId: 'src-orders', + childId: 'child-orders', + dependsOnChildIds: ['child-accounts'], + }, + { sourceId: 'src-accounts', childId: 'child-accounts' }, + ], + }), + requestId: 'test', + }) + + expect(result).toEqual({ + copied: 0, + failed: 2, + failures: [ + { kind: 'table', childId: 'child-accounts' }, + { kind: 'table', childId: 'child-orders' }, + ], + }) + expect(dbChainMockFns.values).toHaveBeenCalledWith([ + expect.objectContaining({ tableId: 'child-orders' }), + ]) + }) + it('#1 binds a copied KB document blob to the CHILD workspace + initiating user', async () => { dbChainMockFns.limit .mockResolvedValueOnce([sourceDoc]) @@ -1212,6 +1313,319 @@ describe('copyForkResourceContent', () => { }) describe('copyForkResourceContainers table views', () => { + it('rejects a mapped referenced table when row mappings are unavailable', async () => { + const now = new Date('2026-08-19T00:00:00.000Z') + const selectedDefinition = { + id: 'table-orders', + workspaceId: 'src-ws', + folderId: null, + name: 'Orders', + description: null, + schema: { + columns: [ + { + id: 'col-account', + name: 'Account', + type: 'reference', + referenceTableId: 'table-accounts', + }, + ], + }, + metadata: {}, + maxRows: 10000, + rowCount: 1, + rowsVersion: 1, + schemaLocked: false, + insertLocked: false, + updateLocked: false, + deleteLocked: false, + archivedAt: null, + createdBy: 'source-user', + createdAt: now, + updatedAt: now, + } + const insert = vi.fn() + const tx = { + select: () => ({ + from: () => ({ where: () => Promise.resolve([selectedDefinition]) }), + }), + insert, + } + + await expect( + copyForkResourceContainers({ + tx: tx as unknown as DbOrTx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + now, + selection: { + customTools: [], + skills: [], + mcpServers: [], + workflowMcpServers: [], + tables: ['table-orders'], + knowledgeBases: [], + }, + workflowIdMap: new Map(), + resolveMappedTableReference: (sourceTableId) => + sourceTableId === 'table-accounts' ? 'target-accounts' : null, + documentMappingContext: { edgeChildWorkspaceId: 'child-ws', sourceIsParent: true }, + }) + ).rejects.toThrow( + 'Referenced table table-accounts is mapped to target-accounts, but referenced row mappings are unavailable' + ) + expect(insert).not.toHaveBeenCalled() + }) + + it('rejects an unavailable referenced-table dependency before inserting copies', async () => { + const now = new Date('2026-08-19T00:00:00.000Z') + const selectedDefinition = { + id: 'table-orders', + workspaceId: 'src-ws', + folderId: null, + name: 'Orders', + description: null, + schema: { + columns: [ + { + id: 'col-account', + name: 'Account', + type: 'reference', + referenceTableId: 'table-accounts', + }, + ], + }, + metadata: {}, + maxRows: 10000, + rowCount: 1, + rowsVersion: 1, + schemaLocked: false, + insertLocked: false, + updateLocked: false, + deleteLocked: false, + archivedAt: null, + createdBy: 'source-user', + createdAt: now, + updatedAt: now, + } + const insert = vi.fn() + let definitionRead = 0 + const tx = { + select: () => ({ + from: () => ({ + where: () => Promise.resolve(definitionRead++ === 0 ? [selectedDefinition] : []), + }), + }), + insert, + } + + await expect( + copyForkResourceContainers({ + tx: tx as unknown as DbOrTx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + now, + selection: { + customTools: [], + skills: [], + mcpServers: [], + workflowMcpServers: [], + tables: ['table-orders'], + knowledgeBases: [], + }, + workflowIdMap: new Map(), + documentMappingContext: { edgeChildWorkspaceId: 'child-ws', sourceIsParent: true }, + }) + ).rejects.toThrow('Referenced table table-accounts is unavailable for copy') + expect(insert).not.toHaveBeenCalled() + }) + + it('bounds the expanded referenced-table dependency set', async () => { + const tx = { select: vi.fn(), insert: vi.fn() } + + await expect( + copyForkResourceContainers({ + tx: tx as unknown as DbOrTx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + now: new Date('2026-08-19T00:00:00.000Z'), + selection: { + customTools: [], + skills: [], + mcpServers: [], + workflowMcpServers: [], + tables: Array.from( + { length: MAX_FORK_RESOURCE_IDS_PER_TYPE + 1 }, + (_, index) => `table-${index}` + ), + knowledgeBases: [], + }, + workflowIdMap: new Map(), + documentMappingContext: { edgeChildWorkspaceId: 'child-ws', sourceIsParent: true }, + }) + ).rejects.toThrow( + `Cannot copy more than ${MAX_FORK_RESOURCE_IDS_PER_TYPE} tables including referenced dependencies` + ) + expect(tx.select).not.toHaveBeenCalled() + }) + + it('copies referenced tables transitively and remaps reference columns to their child ids', async () => { + const now = new Date('2026-08-19T00:00:00.000Z') + const definitions = [ + { + id: 'table-orders', + workspaceId: 'src-ws', + folderId: null, + name: 'Orders', + description: null, + schema: { + columns: [ + { + id: 'col-account', + name: 'Account', + type: 'reference', + referenceTableId: 'table-accounts', + }, + ], + }, + metadata: {}, + maxRows: 10000, + rowCount: 1, + rowsVersion: 1, + schemaLocked: false, + insertLocked: false, + updateLocked: false, + deleteLocked: false, + archivedAt: null, + createdBy: 'source-user', + createdAt: now, + updatedAt: now, + }, + { + id: 'table-accounts', + workspaceId: 'src-ws', + folderId: null, + name: 'Accounts', + description: null, + schema: { + columns: [ + { + id: 'col-company', + name: 'Company', + type: 'reference', + referenceTableId: 'table-companies', + }, + ], + }, + metadata: {}, + maxRows: 10000, + rowCount: 1, + rowsVersion: 1, + schemaLocked: false, + insertLocked: false, + updateLocked: false, + deleteLocked: false, + archivedAt: null, + createdBy: 'source-user', + createdAt: now, + updatedAt: now, + }, + { + id: 'table-companies', + workspaceId: 'src-ws', + folderId: null, + name: 'Companies', + description: null, + schema: { columns: [{ id: 'col-name', name: 'Name', type: 'string' }] }, + metadata: {}, + maxRows: 10000, + rowCount: 1, + rowsVersion: 1, + schemaLocked: false, + insertLocked: false, + updateLocked: false, + deleteLocked: false, + archivedAt: null, + createdBy: 'source-user', + createdAt: now, + updatedAt: now, + }, + ] + const inserted = new Map>>() + let definitionRead = 0 + const tx = { + select: () => ({ + from: (table: unknown) => ({ + where: () => { + if (table === tableViews) return Promise.resolve([]) + if (table !== userTableDefinitions) return Promise.resolve([]) + const rows = [definitions[definitionRead]].filter(Boolean) + definitionRead += 1 + return Promise.resolve(rows) + }, + }), + }), + insert: (table: unknown) => ({ + values: (values: Array>) => { + inserted.set(table, values) + return Promise.resolve() + }, + }), + } + + const result = await copyForkResourceContainers({ + tx: tx as unknown as DbOrTx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + now, + selection: { + customTools: [], + skills: [], + mcpServers: [], + workflowMcpServers: [], + tables: ['table-orders'], + knowledgeBases: [], + }, + workflowIdMap: new Map(), + documentMappingContext: { edgeChildWorkspaceId: 'child-ws', sourceIsParent: true }, + }) + + const tableMap = result.idMap.get('table') + const childOrdersId = tableMap?.get('table-orders') + const childAccountsId = tableMap?.get('table-accounts') + const childCompaniesId = tableMap?.get('table-companies') + expect(tableMap?.size).toBe(3) + expect(result.names.tables).toEqual(['Orders', 'Accounts', 'Companies']) + expect(result.contentPlan.tables).toEqual([ + { + sourceId: 'table-orders', + childId: childOrdersId, + dependsOnChildIds: [childAccountsId], + referenceColumnTargetTableIds: { 'col-account': childAccountsId }, + }, + { + sourceId: 'table-accounts', + childId: childAccountsId, + dependsOnChildIds: [childCompaniesId], + referenceColumnTargetTableIds: { 'col-company': childCompaniesId }, + }, + { sourceId: 'table-companies', childId: childCompaniesId }, + ]) + + const copiedDefinitions = inserted.get(userTableDefinitions) + expect(copiedDefinitions).toHaveLength(3) + expect( + copiedDefinitions?.find((definition) => definition.id === childOrdersId)?.schema + ).toMatchObject({ columns: [{ referenceTableId: childAccountsId }] }) + expect( + copiedDefinitions?.find((definition) => definition.id === childAccountsId)?.schema + ).toMatchObject({ columns: [{ referenceTableId: childCompaniesId }] }) + }) + it('copies saved views and seeds a default for a legacy table', async () => { const now = new Date('2026-08-19T00:00:00.000Z') const definitions = [ diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts index bbb32d2cdff..b6498aac46d 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts @@ -35,6 +35,7 @@ import { type SQL, sql, } from 'drizzle-orm' +import { MAX_FORK_RESOURCE_IDS_PER_TYPE } from '@/lib/api/contracts/workspace-fork' import { decrementStorageUsageForBillingContextInTx, incrementStorageUsageForBillingContextInTx, @@ -56,6 +57,8 @@ import { rebindKnowledgeDocumentSecretProvenance, replaceKnowledgeDocumentSecretProvenanceInTx, } from '@/lib/knowledge/secret-provenance' +import { getColumnId } from '@/lib/table/column-keys' +import { collectColumnReferencedTableIds } from '@/lib/table/column-types/registry.server' import { DEFAULT_TABLE_VIEW_NAME } from '@/lib/table/constants' import { nKeysBetween } from '@/lib/table/order-key' import { @@ -91,7 +94,10 @@ import { type ForkReferenceResolver, rewriteEnvRefsInText, } from '@/ee/workspace-forking/lib/remap/remap-references' -import { remapForkTableWorkflowGroups } from '@/ee/workspace-forking/lib/remap/remap-table-groups' +import { + remapForkTableReferences, + remapForkTableWorkflowGroups, +} from '@/ee/workspace-forking/lib/remap/remap-table-groups' const logger = createLogger('WorkspaceForkCopyResources') @@ -217,6 +223,11 @@ export interface CopyResourcesParams { * plan resolver); omitted by fork-create, which preserves env names verbatim (no rewrite). */ resolveEnvName?: (key: string) => string | null | undefined + /** + * Detect whether a referenced source table already maps to a target during promote. Row-level + * mappings do not exist yet, so the copy fails instead of inventing target row identities. + */ + resolveMappedTableReference?: (sourceTableId: string) => string | null | undefined /** * Resolve a source block id to its target block id for copied tables' workflow-group * `outputs[].blockId`. Promote passes the SAME persisted-pair resolver its workflow writes @@ -238,6 +249,13 @@ export interface ForkContentPlanEntry { childId: string } +export interface ForkContentTableEntry extends ForkContentPlanEntry { + /** Copied tables this table's reference columns require to remain available. */ + dependsOnChildIds?: string[] + /** Stable column id to copied target-table id, used to derive copied referenced-row ids. */ + referenceColumnTargetTableIds?: Record +} + /** * A KB to copy post-commit, plus the source-document -> child-document id map for the * documents that were pre-created as placeholders in the transaction (referenced by copied @@ -289,7 +307,7 @@ export interface ForkContentPlan { childWorkspaceId: string /** Initiating user, recorded as the owner of copied KB-document blob bindings in the child. */ userId: string - tables: ForkContentPlanEntry[] + tables: ForkContentTableEntry[] knowledgeBases: ForkContentKbEntry[] skills: ForkContentSkillEntry[] /** Documents copied into an already-existing target KB (sync-only; empty at fork create). */ @@ -360,6 +378,100 @@ function setId(idMap: Map>, type: ForkReso */ type SkillSkeletonInsert = Omit & { content: SQL } +/** Derives the copied row identity without retaining an unbounded source-row map in memory. */ +function deriveCopiedTableRowId(childTableId: string, sourceRowId: string): string { + return `row_${sha256Hex(`table-row:${childTableId}:${sourceRowId}`).slice(0, 32)}` +} + +/** Rewrites reference cells through the same deterministic identity used by copied target rows. */ +function remapCopiedReferenceCells( + data: unknown, + referenceColumnTargetTableEntries: ReadonlyArray | undefined +): unknown { + if (!referenceColumnTargetTableEntries || !isRecordLike(data)) return data + let remapped: Record | undefined + for (const [columnId, childTableId] of referenceColumnTargetTableEntries) { + const sourceRowId = data[columnId] + if (typeof sourceRowId !== 'string' || sourceRowId.length === 0) continue + remapped ??= { ...data } + remapped[columnId] = deriveCopiedTableRowId(childTableId, sourceRowId) + } + return remapped ?? data +} + +/** + * Loads the selected tables plus the transitive closure of tables named by their reference + * columns. Each layer is workspace-scoped and active-only; an unavailable dependency fails the + * copy instead of persisting a source-workspace table id into the child schema. + */ +async function loadTableDefinitionsWithDependencies( + tx: DbOrTx, + sourceWorkspaceId: string, + selectedTableIds: readonly string[], + resolveMappedTableReference?: (sourceTableId: string) => string | null | undefined +): Promise> { + const orderedIds = [...new Set(selectedTableIds)] + if (orderedIds.length > MAX_FORK_RESOURCE_IDS_PER_TYPE) { + throw new Error( + `Cannot copy more than ${MAX_FORK_RESOURCE_IDS_PER_TYPE} tables including referenced dependencies` + ) + } + const scheduledIds = new Set(orderedIds) + const dependencyIds = new Set() + const definitionsById = new Map() + let pendingIds = [...orderedIds] + + while (pendingIds.length > 0) { + const batchIds = pendingIds + pendingIds = [] + const rows = await tx + .select() + .from(userTableDefinitions) + .where( + and( + inArray(userTableDefinitions.id, batchIds), + eq(userTableDefinitions.workspaceId, sourceWorkspaceId), + isNull(userTableDefinitions.archivedAt) + ) + ) + + for (const row of rows) { + definitionsById.set(row.id, row) + const referencedIds = collectColumnReferencedTableIds((row.schema as TableSchema).columns) + for (const referencedId of referencedIds) { + dependencyIds.add(referencedId) + if (scheduledIds.has(referencedId)) continue + const mappedTableId = resolveMappedTableReference?.(referencedId) + if (mappedTableId) { + throw new Error( + `Referenced table ${referencedId} is mapped to ${mappedTableId}, but referenced row mappings are unavailable` + ) + } + if (scheduledIds.size >= MAX_FORK_RESOURCE_IDS_PER_TYPE) { + throw new Error( + `Cannot copy more than ${MAX_FORK_RESOURCE_IDS_PER_TYPE} tables including referenced dependencies` + ) + } + scheduledIds.add(referencedId) + orderedIds.push(referencedId) + pendingIds.push(referencedId) + } + } + + const missingDependencyId = batchIds.find( + (id) => dependencyIds.has(id) && !definitionsById.has(id) + ) + if (missingDependencyId) { + throw new Error(`Referenced table ${missingDependencyId} is unavailable for copy`) + } + } + + return orderedIds.flatMap((id) => { + const definition = definitionsById.get(id) + return definition ? [definition] : [] + }) +} + /** * Copy the selected resources' **container rows** into the child workspace inside * the fork transaction: custom tools, skills, and MCP server configs (each a @@ -628,16 +740,12 @@ export async function copyForkResourceContainers( } if (selection.tables.length > 0) { - const definitions = await tx - .select() - .from(userTableDefinitions) - .where( - and( - inArray(userTableDefinitions.id, selection.tables), - eq(userTableDefinitions.workspaceId, sourceWorkspaceId), - isNull(userTableDefinitions.archivedAt) - ) - ) + const definitions = await loadTableDefinitionsWithDependencies( + tx, + sourceWorkspaceId, + selection.tables, + params.resolveMappedTableReference + ) const sourceViews = definitions.length > 0 ? await tx @@ -672,12 +780,22 @@ export async function copyForkResourceContainers( const inserts: (typeof userTableDefinitions.$inferInsert)[] = [] const viewInserts: (typeof tableViews.$inferInsert)[] = [] + const tableIdMap = new Map( + definitions.map((definition) => [definition.id, generateId()] as const) + ) + for (const [sourceTableId, childTableId] of tableIdMap) { + record('table', sourceTableId, childTableId) + } for (const definition of definitions) { - const childTableId = generateId() - const remappedSchema = remapForkTableWorkflowGroups( - definition.schema as TableSchema, - workflowIdMap, - params.resolveBlockId + const childTableId = tableIdMap.get(definition.id) + if (!childTableId) throw new Error(`Missing copied table identity for ${definition.id}`) + const remappedSchema = remapForkTableReferences( + remapForkTableWorkflowGroups( + definition.schema as TableSchema, + workflowIdMap, + params.resolveBlockId + ), + tableIdMap ) inserts.push({ ...definition, @@ -734,8 +852,27 @@ export async function copyForkResourceContainers( updatedAt: now, }) } - record('table', definition.id, childTableId) - contentPlan.tables.push({ sourceId: definition.id, childId: childTableId }) + const dependsOnChildIds = collectColumnReferencedTableIds( + (definition.schema as TableSchema).columns + ).flatMap((sourceId) => { + const dependencyId = tableIdMap.get(sourceId) + return dependencyId && dependencyId !== childTableId ? [dependencyId] : [] + }) + const referenceColumnTargetTableIds = Object.fromEntries( + (definition.schema as TableSchema).columns.flatMap((column) => { + const [sourceTargetId] = collectColumnReferencedTableIds([column]) + const childTargetId = sourceTargetId ? tableIdMap.get(sourceTargetId) : undefined + return childTargetId ? [[getColumnId(column), childTargetId]] : [] + }) + ) + contentPlan.tables.push({ + sourceId: definition.id, + childId: childTableId, + ...(dependsOnChildIds.length > 0 ? { dependsOnChildIds } : {}), + ...(Object.keys(referenceColumnTargetTableIds).length > 0 + ? { referenceColumnTargetTableIds } + : {}), + }) names.tables.push(definition.name) } if (inserts.length > 0) await tx.insert(userTableDefinitions).values(inserts) @@ -1197,6 +1334,9 @@ export async function copyForkResourceContent(params: { try { let copied = 0 let afterId: string | null = null + const referenceColumnTargetTableEntries = table.referenceColumnTargetTableIds + ? Object.entries(table.referenceColumnTargetTableIds) + : undefined // `order_key` is nullable, and spreading `...row` would inherit NULLs into a // brand-new tableId that the one-shot backfill script-migration never revisits // (it snapshots the pending set up front) — leaving rows the keyset pager has to @@ -1248,14 +1388,17 @@ export async function copyForkResourceContent(params: { return { row: { ...row, - id: generateId(), + id: deriveCopiedTableRowId(table.childId, row.id), tableId: table.childId, workspaceId: childWorkspaceId, orderKey: row.orderKey ?? mintedKeys[mintedIdx++] ?? null, secretProvenanceVersion: classification.mode === 'legacy' ? null : TABLE_ROW_SECRET_PROVENANCE_VERSION, // Repoint resource-chip URLs in cell data at the child copies (no-op when no maps). - data: contentRefMaps ? remapTableRowResourceUrls(row.data, contentRefMaps) : row.data, + data: remapCopiedReferenceCells( + contentRefMaps ? remapTableRowResourceUrls(row.data, contentRefMaps) : row.data, + referenceColumnTargetTableEntries + ), }, provenance: classification.mode === 'tracked' ? classification : undefined, } @@ -1298,6 +1441,29 @@ export async function copyForkResourceContent(params: { } } + const failedTableIds = new Set( + failures.flatMap((failure) => (failure.kind === 'table' ? [failure.childId] : [])) + ) + let foundFailedDependent = true + while (foundFailedDependent) { + foundFailedDependent = false + for (const table of contentPlan.tables) { + if (failedTableIds.has(table.childId)) continue + if (!table.dependsOnChildIds?.some((dependencyId) => failedTableIds.has(dependencyId))) { + continue + } + failedTableIds.add(table.childId) + failures.push({ kind: 'table', childId: table.childId }) + copiedResources -= 1 + failedResources += 1 + foundFailedDependent = true + logger.warn(`[${requestId}] Failed copied table because a referenced table copy failed`, { + sourceTableId: table.sourceId, + childTableId: table.childId, + }) + } + } + for (const kb of contentPlan.knowledgeBases) { try { await logSkippedConnectorDocuments(kb) diff --git a/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.test.ts b/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.test.ts index c177ceee97d..b9f8d6b27fe 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.test.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.test.ts @@ -272,6 +272,9 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { }) it('threads push orientation through the shared container and mapping boundaries', async () => { + const resolver = vi.fn((kind: ForkRemapKind, sourceId: string) => + kind === 'table' && sourceId === 'mapped-table' ? 'target-table' : null + ) await copyPromoteUnmappedResources({ tx, edge, @@ -290,7 +293,7 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { }, workflowIdMap: new Map(), folderIdMap: new Map(), - resolver: () => null, + resolver, resolveBlockId, referencedDocumentIds: [], }) @@ -303,6 +306,9 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { }, }) ) + const containerParams = mockCopyForkResourceContainers.mock.calls.at(-1)?.[0] + expect(containerParams?.resolveMappedTableReference('mapped-table')).toBe('target-table') + expect(resolver).toHaveBeenCalledWith('table', 'mapped-table') expect(mockPersistCopiedResourceMappings).toHaveBeenCalledWith( expect.objectContaining({ edgeChildWorkspaceId: 'edge-child', diff --git a/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.ts b/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.ts index 19269023429..02e546e917e 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/copy-unmapped.ts @@ -232,6 +232,7 @@ export async function copyPromoteUnmappedResources(params: { // A sync can rename env vars, so a copied custom tool's `code` must have its `{{ENV}}` refs // rewritten through the same plan resolver that remaps subblock-value env refs. resolveEnvName: (key) => resolver('env-var', key), + resolveMappedTableReference: (sourceTableId) => resolver('table', sourceTableId), resolveBlockId, documentMappingContext: { edgeChildWorkspaceId: edge.childWorkspaceId, diff --git a/apps/sim/ee/workspace-forking/lib/remap/remap-table-groups.ts b/apps/sim/ee/workspace-forking/lib/remap/remap-table-groups.ts index 592f0565cc7..e0887f32d85 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-table-groups.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-table-groups.ts @@ -1,3 +1,4 @@ +import { remapColumnReferencedTableIds } from '@/lib/table/column-types/registry.server' import type { TableSchema } from '@/lib/table/types' import { deriveForkBlockId, @@ -60,3 +61,13 @@ export function remapForkTableWorkflowGroups( return { ...schema, columns, workflowGroups: remappedGroups } } + +export function remapForkTableReferences( + schema: TableSchema, + tableIdMap: ReadonlyMap +): TableSchema { + const columns = remapColumnReferencedTableIds(schema.columns, tableIdMap) + return columns.some((column, index) => column !== schema.columns[index]) + ? { ...schema, columns } + : schema +} diff --git a/apps/sim/hooks/queries/tables.test.ts b/apps/sim/hooks/queries/tables.test.ts index 0f81a28108f..b6bf8c08b51 100644 --- a/apps/sim/hooks/queries/tables.test.ts +++ b/apps/sim/hooks/queries/tables.test.ts @@ -1,6 +1,8 @@ /** * @vitest-environment node */ + +import { useQuery } from '@tanstack/react-query' import { beforeEach, describe, expect, it, vi } from 'vitest' const { queryClient, cacheStore } = vi.hoisted(() => { @@ -25,6 +27,7 @@ const { queryClient, cacheStore } = vi.hoisted(() => { .filter(([k]) => k.startsWith(prefix)) .map(([k, v]) => [JSON.parse(k), v]) }), + fetchQuery: vi.fn(), removeQueries: vi.fn(), }, } @@ -57,13 +60,26 @@ vi.mock('@sim/emcn', () => ({ toast: { error: vi.fn(), success: vi.fn() }, })) -import type { TableViewWire } from '@/lib/api/contracts/tables' +import { isApiClientError } from '@/lib/api/client/errors' +import { requestJson } from '@/lib/api/client/request' +import { + getTableRowContract, + listTableNamesContract, + type TableViewWire, +} from '@/lib/api/contracts/tables' import { + TABLE_DETAIL_STALE_TIME, tableRowsInfiniteOptions, tableRowsParamsKey, + useBatchUpdateTableRows, useDeleteColumn, + useDeleteTableRow, + useDeleteTableRows, + useReferenceRowPreview, useRestoreTable, + useTableNames, useUpdateColumn, + useUpdateTableRow, useUpdateTableView, } from '@/hooks/queries/tables' import { tableKeys } from '@/hooks/queries/utils/table-keys' @@ -91,6 +107,302 @@ beforeEach(() => { vi.clearAllMocks() }) +describe('useTableNames', () => { + it('loads only the requested table names once with a canonical cache key', async () => { + vi.mocked(requestJson).mockResolvedValueOnce({ + success: true, + data: { tables: [{ id: TABLE_ID, name: 'Accounts' }] }, + }) + + useTableNames(WORKSPACE_ID, ['tbl-2', TABLE_ID, 'tbl-2']) + + const options = vi.mocked(useQuery).mock.calls.at(-1)?.[0] as { + enabled: boolean + queryKey: readonly unknown[] + queryFn: (context: { signal: AbortSignal }) => Promise + } + const signal = new AbortController().signal + await expect(options.queryFn({ signal })).resolves.toEqual([{ id: TABLE_ID, name: 'Accounts' }]) + expect(options).toMatchObject({ + enabled: true, + queryKey: tableKeys.names(WORKSPACE_ID, [TABLE_ID, 'tbl-2']), + }) + expect(requestJson).toHaveBeenCalledWith(listTableNamesContract, { + body: { workspaceId: WORKSPACE_ID, tableIds: [TABLE_ID, 'tbl-2'] }, + signal, + }) + }) + + it('does not fetch when there are no referenced tables', () => { + useTableNames(WORKSPACE_ID, []) + + const options = vi.mocked(useQuery).mock.calls.at(-1)?.[0] as { enabled: boolean } + expect(options.enabled).toBe(false) + }) +}) + +describe('useReferenceRowPreview', () => { + function getQueryOptions() { + return vi.mocked(useQuery).mock.calls.at(-1)?.[0] as { + enabled: boolean + gcTime: number + queryKey: readonly unknown[] + refetchOnMount: 'always' + refetchOnReconnect: boolean + refetchOnWindowFocus: boolean + staleTime: number + queryFn: (context: { signal: AbortSignal }) => Promise + } + } + + it('isolates each opening and fetches only the referenced row', async () => { + const row = { id: 'row-1', data: { name: 'Acme' } } + const table = { id: TABLE_ID, name: 'Accounts', schema: { columns: [] } } + const signal = new AbortController().signal + queryClient.fetchQuery.mockResolvedValueOnce(table) + vi.mocked(requestJson).mockResolvedValueOnce({ data: { row } }) + + useReferenceRowPreview({ + workspaceId: WORKSPACE_ID, + tableId: TABLE_ID, + rowId: row.id, + sourceRowId: 'source-row-1', + sourceColumnKey: 'account', + }) + + const options = getQueryOptions() + expect(options).toMatchObject({ + enabled: true, + gcTime: 0, + queryKey: tableKeys.referencePreview(TABLE_ID, row.id, 'source-row-1', 'account'), + refetchOnMount: 'always', + refetchOnReconnect: false, + refetchOnWindowFocus: false, + staleTime: Number.POSITIVE_INFINITY, + }) + await expect(options.queryFn({ signal })).resolves.toEqual({ + table, + row, + referenceTables: [], + }) + expect(options).not.toHaveProperty('placeholderData') + expect(queryClient.fetchQuery).toHaveBeenCalledWith( + expect.objectContaining({ + queryKey: tableKeys.detail(TABLE_ID), + staleTime: TABLE_DETAIL_STALE_TIME, + }) + ) + expect(requestJson).toHaveBeenCalledOnce() + expect(requestJson).toHaveBeenCalledWith(getTableRowContract, { + params: { tableId: TABLE_ID, rowId: row.id }, + query: { workspaceId: WORKSPACE_ID }, + signal, + }) + }) + + it('loads nested reference table names in one request before resolving the preview', async () => { + const row = { id: 'row-1', data: { owner: 'owner-row-1' } } + const table = { + id: TABLE_ID, + name: 'Accounts', + schema: { + columns: [ + { + id: 'owner-1', + name: 'Owner', + type: 'reference', + referenceTableId: 'tbl-owners', + }, + { + id: 'owner-2', + name: 'Backup owner', + type: 'reference', + referenceTableId: 'tbl-owners', + }, + ], + }, + } + const referenceTables = [{ id: 'tbl-owners', name: 'Owners' }] + const signal = new AbortController().signal + queryClient.fetchQuery.mockResolvedValueOnce(table) + vi.mocked(requestJson) + .mockResolvedValueOnce({ data: { row } }) + .mockResolvedValueOnce({ success: true, data: { tables: referenceTables } }) + + useReferenceRowPreview({ + workspaceId: WORKSPACE_ID, + tableId: TABLE_ID, + rowId: row.id, + sourceRowId: 'source-row-1', + sourceColumnKey: 'account', + }) + + await expect(getQueryOptions().queryFn({ signal })).resolves.toEqual({ + table, + row, + referenceTables, + }) + expect(requestJson).toHaveBeenNthCalledWith(2, listTableNamesContract, { + body: { workspaceId: WORKSPACE_ID, tableIds: ['tbl-owners'] }, + signal, + }) + }) + + it('does not fetch until every referenced-row identity is available', () => { + useReferenceRowPreview({ + workspaceId: WORKSPACE_ID, + tableId: TABLE_ID, + rowId: undefined, + }) + + expect(getQueryOptions().enabled).toBe(false) + }) + + it('returns a null row when the referenced row no longer exists', async () => { + const table = { id: TABLE_ID, name: 'Accounts', schema: { columns: [] } } + queryClient.fetchQuery.mockResolvedValueOnce(table) + vi.mocked(requestJson).mockRejectedValueOnce({ status: 404 }) + vi.mocked(isApiClientError).mockReturnValueOnce(true) + + useReferenceRowPreview({ + workspaceId: WORKSPACE_ID, + tableId: TABLE_ID, + rowId: 'missing-row', + sourceRowId: 'source-row-1', + sourceColumnKey: 'account', + }) + + await expect( + getQueryOptions().queryFn({ signal: new AbortController().signal }) + ).resolves.toEqual({ + table, + row: null, + referenceTables: [], + }) + }) + + it('propagates non-not-found row errors', async () => { + const error = new Error('Failed to load row') + queryClient.fetchQuery.mockResolvedValueOnce({ + id: TABLE_ID, + name: 'Accounts', + schema: { columns: [] }, + }) + vi.mocked(requestJson).mockRejectedValueOnce(error) + + useReferenceRowPreview({ + workspaceId: WORKSPACE_ID, + tableId: TABLE_ID, + rowId: 'row-1', + sourceRowId: 'source-row-1', + sourceColumnKey: 'account', + }) + + await expect(getQueryOptions().queryFn({ signal: new AbortController().signal })).rejects.toBe( + error + ) + }) + + it('uses the source cell to identify each preview opening', () => { + useReferenceRowPreview({ + workspaceId: WORKSPACE_ID, + tableId: TABLE_ID, + rowId: 'row-1', + sourceRowId: 'source-row-1', + sourceColumnKey: 'account', + }) + const firstOpening = getQueryOptions().queryKey + + useReferenceRowPreview({ + workspaceId: WORKSPACE_ID, + tableId: TABLE_ID, + rowId: 'row-1', + sourceRowId: 'source-row-2', + sourceColumnKey: 'account', + }) + + expect(getQueryOptions().queryKey).not.toEqual(firstOpening) + }) +}) + +describe('useBatchUpdateTableRows', () => { + it('invalidates matching reference previews after a batch write settles', () => { + const hook = useBatchUpdateTableRows({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID }) + const updates = [ + { rowId: 'row-1', data: { name: 'Acme' } }, + { rowId: 'row-2', data: { name: 'Globex' } }, + ] + + hook.onSettled?.(undefined, null, { updates }, undefined) + + expect(queryClient.invalidateQueries).toHaveBeenCalledOnce() + const options = queryClient.invalidateQueries.mock.calls[0]?.[0] + expect(options?.queryKey).toEqual(tableKeys.referencePreviews()) + expect( + options?.predicate({ + queryKey: tableKeys.referencePreview(TABLE_ID, 'row-1', 'source-row', 'account'), + }) + ).toBe(true) + expect( + options?.predicate({ + queryKey: tableKeys.referencePreview(TABLE_ID, 'row-3', 'source-row', 'account'), + }) + ).toBe(false) + expect( + options?.predicate({ + queryKey: tableKeys.referencePreview('other-table', 'row-1', 'source-row', 'account'), + }) + ).toBe(false) + }) +}) + +describe('reference preview invalidation', () => { + function expectPreviewInvalidation(rowIds: string[]) { + const call = queryClient.invalidateQueries.mock.calls.find( + ([options]) => + JSON.stringify(options?.queryKey) === JSON.stringify(tableKeys.referencePreviews()) + ) + expect(call).toBeDefined() + const options = call?.[0] + for (const rowId of rowIds) { + expect( + options?.predicate({ + queryKey: tableKeys.referencePreview(TABLE_ID, rowId, 'source-row', 'account'), + }) + ).toBe(true) + } + expect( + options?.predicate({ + queryKey: tableKeys.referencePreview(TABLE_ID, 'untouched-row', 'source-row', 'account'), + }) + ).toBe(false) + } + + it('invalidates a referenced row after an update settles', () => { + const hook = useUpdateTableRow({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID }) + + hook.onSettled?.(undefined, null, { rowId: 'row-1', data: { name: 'Acme' } }, undefined) + + expectPreviewInvalidation(['row-1']) + }) + + it('invalidates a referenced row after a delete settles', () => { + const hook = useDeleteTableRow({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID }) + + hook.onSettled?.(undefined, null, 'row-1', undefined) + + expectPreviewInvalidation(['row-1']) + }) + + it('invalidates every referenced row after a bulk delete settles', () => { + const hook = useDeleteTableRows({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID }) + + hook.onSettled?.(undefined, null, ['row-1', 'row-2'], undefined) + + expectPreviewInvalidation(['row-1', 'row-2']) + }) +}) + describe('useUpdateTableView autosave ordering', () => { it('serializes config and layout patches for the same table', () => { const hook = useUpdateTableView({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID }) diff --git a/apps/sim/hooks/queries/tables.ts b/apps/sim/hooks/queries/tables.ts index 22b4359b9f8..48ca9183559 100644 --- a/apps/sim/hooks/queries/tables.ts +++ b/apps/sim/hooks/queries/tables.ts @@ -59,11 +59,14 @@ import { deleteTableViewContract, deleteWorkflowGroupContract, findTableRowsContract, + type GetTableRowResponse, getEnrichmentDetailContract, getTableContract, + getTableRowContract, type InsertTableRowBodyInput, listActiveDispatchesContract, listTableJobsContract, + listTableNamesContract, listTableRowsContract, listTablesContract, listTableViewsContract, @@ -108,6 +111,7 @@ import type { WorkflowGroupOutput, } from '@/lib/table' import { getColumnId } from '@/lib/table/column-keys' +import { columnTypeOf } from '@/lib/table/column-types' import { TABLE_LIMITS } from '@/lib/table/constants' import { areGroupDepsSatisfied, @@ -144,6 +148,8 @@ export const TABLE_FIND_STALE_TIME = 30 * 1000 export const TABLE_FIND_GC_TIME = 60 * 1000 export const TABLE_ROWS_STALE_TIME = 30 * 1000 export const TABLE_EXPORT_JOBS_STALE_TIME = 5 * 1000 +const TABLE_REFERENCE_PREVIEW_STALE_TIME = Number.POSITIVE_INFINITY +const TABLE_REFERENCE_PREVIEW_GC_TIME = 0 type TableRowsParams = Omit & TableIdParamsInput & { @@ -184,6 +190,22 @@ async function fetchTable( return response.data.table } +function normalizeTableIds(tableIds: readonly string[]) { + return [...new Set(tableIds)].sort() +} + +async function fetchTableNames( + workspaceId: string, + tableIds: readonly string[], + signal?: AbortSignal +) { + const response = await requestJson(listTableNamesContract, { + body: { workspaceId, tableIds: normalizeTableIds(tableIds) }, + signal, + }) + return response.data.tables +} + async function fetchTableRows({ workspaceId, tableId, @@ -217,12 +239,47 @@ async function fetchTableRows({ return { rows, totalCount, nextCursor } } +async function fetchTableRow( + workspaceId: string, + tableId: string, + rowId: string, + signal?: AbortSignal +): Promise { + try { + const response = await requestJson(getTableRowContract, { + params: { tableId, rowId }, + query: { workspaceId }, + signal, + }) + return response.data.row + } catch (error) { + if (isApiClientError(error) && error.status === 404) return null + throw error + } +} + function invalidateRowCount(queryClient: ReturnType, tableId: string) { queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) }) queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) }) queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) } +function invalidateReferencePreviews( + queryClient: ReturnType, + tableId: string, + rowIds: ReadonlySet +) { + const previewsRoot = tableKeys.referencePreviews() + queryClient.invalidateQueries({ + queryKey: previewsRoot, + predicate: (query) => { + const targetTableId = query.queryKey[previewsRoot.length] + const targetRowId = query.queryKey[previewsRoot.length + 1] + return targetTableId === tableId && typeof targetRowId === 'string' && rowIds.has(targetRowId) + }, + }) +} + /** * Invalidate only the row-count surfaces — the table detail and the tables * list, both of which carry the unfiltered `rowCount`. Deliberately leaves @@ -301,6 +358,22 @@ export function useTablesList( }) } +export function useTableNames( + workspaceId: string | undefined, + referencedTableIds: readonly string[] +) { + const tableIds = normalizeTableIds(referencedTableIds) + return useQuery({ + queryKey: tableKeys.names(workspaceId, tableIds), + queryFn: async ({ signal }) => { + if (!workspaceId) throw new Error('Workspace ID required') + return fetchTableNames(workspaceId, tableIds, signal) + }, + enabled: Boolean(workspaceId && tableIds.length > 0), + staleTime: TABLE_LIST_STALE_TIME, + }) +} + /** * Fetch a single table by id. */ @@ -314,6 +387,52 @@ export function useTable(workspaceId: string | undefined, tableId: string | unde }) } +interface ReferenceRowPreviewParams { + workspaceId: string | undefined + tableId: string | undefined + rowId: string | undefined + sourceRowId?: string + sourceColumnKey?: string +} + +/** Loads a referenced table and row together for an expanded source cell. */ +export function useReferenceRowPreview({ + workspaceId, + tableId, + rowId, + sourceRowId, + sourceColumnKey, +}: ReferenceRowPreviewParams) { + const queryClient = useQueryClient() + // rq-lint-allow: tableId is globally unique; workspaceId is only an authz scope on the fetch and cannot collide across workspaces + return useQuery({ + queryKey: tableKeys.referencePreview(tableId ?? '', rowId ?? '', sourceRowId, sourceColumnKey), + queryFn: async ({ signal }) => { + const [table, row] = await Promise.all([ + queryClient.fetchQuery( + getTableDetailQueryOptions(workspaceId as string, tableId as string) + ), + fetchTableRow(workspaceId as string, tableId as string, rowId as string, signal), + ]) + const referenceTableIds = table.schema.columns.flatMap((column) => { + const referenceTableId = columnTypeOf(column).referencePreview?.getTableId(column) + return referenceTableId ? [referenceTableId] : [] + }) + const referenceTables = + referenceTableIds.length === 0 + ? [] + : await fetchTableNames(workspaceId as string, referenceTableIds, signal) + return { table, row, referenceTables } + }, + enabled: Boolean(workspaceId && tableId && rowId && sourceRowId && sourceColumnKey), + staleTime: TABLE_REFERENCE_PREVIEW_STALE_TIME, + gcTime: TABLE_REFERENCE_PREVIEW_GC_TIME, + refetchOnMount: 'always', + refetchOnWindowFocus: false, + refetchOnReconnect: false, + }) +} + /** * Shared table-detail query options so non-component callers (e.g. selector * providers) can `ensureQueryData` the same cache entry `useTable` populates. @@ -1154,6 +1273,9 @@ export function useUpdateTableRow({ workspaceId, tableId }: RowMutationContext) if (isValidationError(error)) return toast.error(error.message, { duration: 5000 }) }, + onSettled: (_data, _error, { rowId }) => { + invalidateReferencePreviews(queryClient, tableId, new Set([rowId])) + }, }) } @@ -1228,6 +1350,10 @@ export function useBatchUpdateTableRows({ workspaceId, tableId }: RowMutationCon if (isValidationError(error)) return toast.error(error.message, { duration: 5000 }) }, + onSettled: (_data, _error, { updates }) => { + const rowIds = new Set(updates.map(({ rowId }) => rowId)) + invalidateReferencePreviews(queryClient, tableId, rowIds) + }, }) } @@ -1249,8 +1375,9 @@ export function useDeleteTableRow({ workspaceId, tableId }: RowMutationContext) if (isValidationError(error)) return toast.error(error.message, { duration: 5000 }) }, - onSettled: () => { + onSettled: (_data, _error, rowId) => { invalidateRowCount(queryClient, tableId) + invalidateReferencePreviews(queryClient, tableId, new Set([rowId])) }, }) } @@ -1298,8 +1425,9 @@ export function useDeleteTableRows({ workspaceId, tableId }: RowMutationContext) if (isValidationError(error)) return toast.error(error.message, { duration: 5000 }) }, - onSettled: () => { + onSettled: (_data, _error, rowIds) => { invalidateRowCount(queryClient, tableId) + invalidateReferencePreviews(queryClient, tableId, new Set(rowIds)) }, }) } diff --git a/apps/sim/hooks/queries/utils/table-keys.ts b/apps/sim/hooks/queries/utils/table-keys.ts index 5ccf7f34457..46805598c2f 100644 --- a/apps/sim/hooks/queries/utils/table-keys.ts +++ b/apps/sim/hooks/queries/utils/table-keys.ts @@ -20,11 +20,16 @@ export const tableKeys = { lists: () => [...tableKeys.all, 'list'] as const, list: (workspaceId?: string, scope: TableQueryScope = 'active') => [...tableKeys.lists(), workspaceId ?? '', scope] as const, + names: (workspaceId: string | undefined, tableIds: readonly string[]) => + [...tableKeys.lists(), 'names', workspaceId ?? '', tableIds] as const, details: () => [...tableKeys.all, 'detail'] as const, detail: (tableId: string) => [...tableKeys.details(), tableId] as const, exportJobs: (workspaceId?: string) => [...tableKeys.all, 'export-jobs', workspaceId ?? ''] as const, rowsRoot: (tableId: string) => [...tableKeys.detail(tableId), 'rows'] as const, + referencePreviews: () => [...tableKeys.all, 'reference-preview'] as const, + referencePreview: (tableId: string, rowId: string, sourceRowId = '', sourceColumnKey = '') => + [...tableKeys.referencePreviews(), tableId, rowId, sourceRowId, sourceColumnKey] as const, /** * Prefix covering only the paged row lists. `rowsRoot` is a shared parent — `find` * hangs off it holding a different shape — so anything walking the cache for row diff --git a/apps/sim/lib/api/contracts/tables.ts b/apps/sim/lib/api/contracts/tables.ts index 4c95b3d6520..b8bc31762a1 100644 --- a/apps/sim/lib/api/contracts/tables.ts +++ b/apps/sim/lib/api/contracts/tables.ts @@ -936,6 +936,36 @@ export const listTablesContract = defineRouteContract({ }) export type ListTablesResponse = ContractJsonResponse +export const listTableNamesBodySchema = z.object({ + workspaceId: workspaceIdSchema, + tableIds: z + .array(referenceTableIdSchema) + .min(1, 'At least one table ID is required') + .max( + TABLE_LIMITS.MAX_COLUMNS_PER_TABLE, + `Cannot request more than ${TABLE_LIMITS.MAX_COLUMNS_PER_TABLE} table names` + ), +}) +export type ListTableNamesBodyInput = z.input + +export const listTableNamesContract = defineRouteContract({ + method: 'POST', + path: '/api/table/names', + body: listTableNamesBodySchema, + response: { + mode: 'json', + schema: successResponseSchema( + z.object({ + tables: z.array( + z.object({ + id: z.string().min(1).max(MAX_ID_LENGTH), + name: tableNameSchema, + }) + ), + }) + ), + }, +}) export const createTableContract = defineRouteContract({ method: 'POST', path: '/api/table', @@ -1467,6 +1497,8 @@ export const getTableRowContract = defineRouteContract({ }, }) +export type GetTableRowResponse = ContractJsonResponse + export const updateTableRowContract = defineRouteContract({ method: 'PATCH', path: '/api/table/[tableId]/rows/[rowId]', diff --git a/apps/sim/lib/api/contracts/workspace-fork.ts b/apps/sim/lib/api/contracts/workspace-fork.ts index 49c7bdd25ca..6a10b7585b9 100644 --- a/apps/sim/lib/api/contracts/workspace-fork.ts +++ b/apps/sim/lib/api/contracts/workspace-fork.ts @@ -133,7 +133,9 @@ export type ForkLineageNodeApi = z.output export type ForkLineageChildApi = z.output export type GetForkLineageResponse = z.output -const forkResourceIdList = z.array(nonEmptyIdSchema).max(2000).optional() +export const MAX_FORK_RESOURCE_IDS_PER_TYPE = 2_000 + +const forkResourceIdList = z.array(nonEmptyIdSchema).max(MAX_FORK_RESOURCE_IDS_PER_TYPE).optional() export const forkResourceSelectionSchema = z.object({ files: forkResourceIdList, diff --git a/apps/sim/lib/folders/bulk.test.ts b/apps/sim/lib/folders/bulk.test.ts index 08bb9092973..4fc6c927e7d 100644 --- a/apps/sim/lib/folders/bulk.test.ts +++ b/apps/sim/lib/folders/bulk.test.ts @@ -46,6 +46,7 @@ describe('planFolderSelection', () => { expect(result.selected).toEqual([{ id: 'a', name: 'A' }]) expect(result.contained).toEqual([]) expect([...result.covered].sort()).toEqual(['a', 'a1', 'a1x']) + expect([...(result.coveredBySelected.get('a') ?? [])].sort()).toEqual(['a', 'a1', 'a1x']) }) it('reports an explicitly selected descendant as contained, not as a second selection', async () => { diff --git a/apps/sim/lib/folders/bulk.ts b/apps/sim/lib/folders/bulk.ts index 2e32c343132..3e09829cb5f 100644 --- a/apps/sim/lib/folders/bulk.ts +++ b/apps/sim/lib/folders/bulk.ts @@ -35,6 +35,8 @@ export interface FolderSelectionPlan { * acted on a second time. */ covered: Set + /** The covered subtree for each top-level selected folder, used for per-folder preflight. */ + coveredBySelected: Map> } /** @@ -52,7 +54,13 @@ export async function planFolderSelection( folderIds: readonly string[] ): Promise { if (folderIds.length === 0) { - return { selected: [], notFound: [], contained: [], covered: new Set() } + return { + selected: [], + notFound: [], + contained: [], + covered: new Set(), + coveredBySelected: new Map(), + } } const rows = await listActiveFolderRows(workspaceId, resourceType, { @@ -64,6 +72,7 @@ export async function planFolderSelection( const notFound: string[] = [] const contained: BulkFolderAffected[] = [] const covered = new Set() + const coveredBySelected = new Map>() const requested = new Set() for (const folderId of folderIds) { @@ -111,8 +120,9 @@ export async function planFolderSelection( } if (covered.has(folderId)) continue selected.push(entry) - covered.add(folderId) - for (const descendantId of descendantsOf.get(folderId) ?? []) covered.add(descendantId) + const selectedCoverage = new Set([folderId, ...(descendantsOf.get(folderId) ?? [])]) + coveredBySelected.set(folderId, selectedCoverage) + for (const coveredId of selectedCoverage) covered.add(coveredId) } /** @@ -126,7 +136,7 @@ export async function planFolderSelection( for (const descendantId of descendantsOf.get(folder.id) ?? []) covered.add(descendantId) } - return { selected, notFound, contained, covered } + return { selected, notFound, contained, covered, coveredBySelected } } /** diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts index 5c4af123168..7fd73951b7a 100644 --- a/apps/sim/lib/folders/cascade.test.ts +++ b/apps/sim/lib/folders/cascade.test.ts @@ -1,7 +1,13 @@ /** * @vitest-environment node */ -import { flattenMockConditions, hasMockCondition } from '@sim/testing' +import { + flattenMockConditions, + hasMockCondition, + queueTableRows, + resetDbChainMock, + schemaMock, +} from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' import { archiveFolderCascade, @@ -540,3 +546,48 @@ describe('knowledge_base and table folder resources', () => { expect(tableConfig.sortOrderColumn).toBeUndefined() }) }) + +describe('table folder deletion guard', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) + + it('refuses the whole folder before a referenced table can be archived', async () => { + queueTableRows(schemaMock.userTableDefinitions, []) + queueTableRows(schemaMock.userTableDefinitions, [ + { + id: 'tbl_customers', + name: 'Customers', + folderId: 'folder-child', + schema: { columns: [{ id: 'name', name: 'Name', type: 'string' }] }, + }, + { + id: 'tbl_orders', + name: 'Orders', + folderId: null, + schema: { + columns: [ + { + id: 'customer', + name: 'Customer', + type: 'reference', + referenceTableId: 'tbl_customers', + }, + ], + }, + }, + ]) + + await expect( + FOLDER_RESOURCES.table.guardDelete?.({ + workspaceId: 'ws-1', + folderIds: ['folder-root', 'folder-child'], + }) + ).resolves.toEqual({ + error: + 'Cannot delete table "Customers" because it is referenced by table "Orders". Remove the reference column first.', + errorCode: 'conflict', + }) + }) +}) diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts index cca02a55313..1ed4a4ff349 100644 --- a/apps/sim/lib/folders/config.ts +++ b/apps/sim/lib/folders/config.ts @@ -305,8 +305,9 @@ async function restoreKnowledgeBaseChildren(context: CascadeChildrenContext): Pr /** * Archives the tables in a folder subtree through the canonical table delete, so the - * `deleteLocked` guard in its WHERE clause still applies. {@link guardLockedTables} has - * already refused the whole folder if any table is locked, so this should not encounter one. + * `deleteLocked` and inbound-reference guards still apply. {@link guardTableDeletion} has + * already refused the whole folder if any table cannot be deleted, so this should not + * encounter a partial cascade. */ async function archiveTableChildren(context: CascadeChildrenContext): Promise { const { deleteTable } = await import('@/lib/table/service') @@ -332,11 +333,13 @@ async function restoreTableChildren(context: CascadeChildrenContext): Promise { - const [{ db }, { and, eq: eqOp, inArray, isNull }] = await Promise.all([ + const [ + { db }, + { and, eq: eqOp, inArray, isNull }, + { findActiveTableReferenceBlockers, tableReferenceBlockerMessage }, + ] = await Promise.all([ import('@sim/db'), import('drizzle-orm'), + import('@/lib/table/column-types/registry.server'), ]) const locked = await db @@ -376,12 +383,22 @@ async function guardLockedTables({ ) ) - if (locked.length === 0) return null + if (locked.length > 0) { + const names = locked.map((row) => row.name).join(', ') + return { + error: `Cannot delete folder: ${locked.length === 1 ? 'table' : 'tables'} ${names} ${locked.length === 1 ? 'is' : 'are'} delete-locked`, + errorCode: 'locked', + } + } + + const [blocker] = await findActiveTableReferenceBlockers(db, workspaceId, { + folderIds: new Set(folderIds), + }) + if (!blocker) return null - const names = locked.map((row) => row.name).join(', ') return { - error: `Cannot delete folder: ${locked.length === 1 ? 'table' : 'tables'} ${names} ${locked.length === 1 ? 'is' : 'are'} delete-locked`, - errorCode: 'locked', + error: tableReferenceBlockerMessage(blocker.targetTableName, [blocker.referencingTableName]), + errorCode: 'conflict', } } @@ -515,7 +532,7 @@ export const FOLDER_RESOURCES: Record >, archiveChildren: archiveTableChildren, restoreChildren: restoreTableChildren, - guardDelete: guardLockedTables, + guardDelete: guardTableDeletion, }, } diff --git a/apps/sim/lib/table/application/batch-policy.ts b/apps/sim/lib/table/application/batch-policy.ts index 5c5e22c2d45..fb012db7a87 100644 --- a/apps/sim/lib/table/application/batch-policy.ts +++ b/apps/sim/lib/table/application/batch-policy.ts @@ -7,11 +7,11 @@ import { import { MAX_TABLE_BATCH_ITEMS } from '@/lib/table/constants' /** - * Bulk table operations run one authorized single-table mutation per item and - * report a per-item outcome, matching the knowledge domain's - * `sequential_best_effort` bulk policy. There is no single-statement archive or - * re-parent primitive that could make the batch atomic: archiving a table - * cascades, and each item is authorized against its own canonical row. + * Bulk table operations authorize each item and report a per-item outcome, + * matching the knowledge domain's `sequential_best_effort` bulk policy. Moves + * remain individual mutations. Deletes validate the explicit selection as a + * group, then use per-table savepoints so recoverable statement failures retain + * a dependency-safe prefix. An outer transaction failure rolls that phase back. */ export const BULK_MOVE_TABLES_COST_POLICY = { maxItems: MAX_TABLE_BATCH_ITEMS, diff --git a/apps/sim/lib/table/application/bulk.test.ts b/apps/sim/lib/table/application/bulk.test.ts index 0726111a1ba..539cdc54fab 100644 --- a/apps/sim/lib/table/application/bulk.test.ts +++ b/apps/sim/lib/table/application/bulk.test.ts @@ -8,7 +8,7 @@ const mocks = vi.hoisted(() => ({ audit: vi.fn(), bulkDeleteFolders: vi.fn(), bulkMoveFolders: vi.fn(), - deleteTable: vi.fn(), + deleteTables: vi.fn(), findActiveFolder: vi.fn(), moveTableToFolder: vi.fn(), planFolderSelection: vi.fn(), @@ -17,6 +17,7 @@ const mocks = vi.hoisted(() => ({ resolveWorkspaceContext: vi.fn(), signal: vi.fn(), notifyTables: vi.fn(), + findReferenceBlockers: vi.fn(), resolveFolderPathFromIndex: vi.fn(), resolveTableFolderPath: vi.fn(), })) @@ -68,7 +69,7 @@ vi.mock('@/lib/table/application/folder-paths', () => ({ resolveTableFolderPath: mocks.resolveTableFolderPath, })) vi.mock('@/lib/table', () => ({ - deleteTable: mocks.deleteTable, + deleteTables: mocks.deleteTables, moveTableToFolder: mocks.moveTableToFolder, })) vi.mock('@/lib/table/application/context', () => ({ @@ -76,6 +77,11 @@ vi.mock('@/lib/table/application/context', () => ({ resolveTableWorkspaceContext: mocks.resolveWorkspaceContext, })) vi.mock('@/lib/table/events', () => ({ signalTableSchemaChanged: mocks.signal })) +vi.mock('@/lib/table/column-types/registry.server', () => ({ + findActiveTableReferenceBlockers: mocks.findReferenceBlockers, + tableReferenceBlockerMessage: (target: string, blockers: string[]) => + `Cannot delete table "${target}" because it is referenced by table "${blockers[0]}". Remove the reference column first.`, +})) import { OrchestrationError } from '@/lib/core/orchestration/types' import { bulkDeleteTables, bulkMoveTables } from '@/lib/table/application/bulk' @@ -98,6 +104,14 @@ function tableContext(id: string, folderId: string | null = null) { } } +function successfulTableDeleteBatch(tableIds: readonly string[]) { + return { + archived: tableIds.map((id) => ({ id, name: 'Archived', workspaceId: 'workspace-1' })), + failed: [], + notFound: [], + } +} + /** * The active folder tree a path-keyed batch resolves against. `undefined` for * anything absent, mirroring `resolveFolderPathFromIndex`; `/` is the workspace @@ -118,12 +132,11 @@ describe('table bulk application use cases', () => { mocks.resolveWorkspaceContext.mockResolvedValue(workspaceContext) mocks.resolvePermission.mockResolvedValue('write') mocks.planFolderSelection.mockResolvedValue(emptyPlan) + mocks.findReferenceBlockers.mockResolvedValue([]) mocks.findActiveFolder.mockResolvedValue({ id: 'folder-1' }) mocks.resolveTableContext.mockImplementation(async (tableId: string) => tableContext(tableId)) mocks.moveTableToFolder.mockResolvedValue({ name: 'Moved' }) - mocks.deleteTable.mockResolvedValue({ - archived: { name: 'Archived', workspaceId: 'workspace-1' }, - }) + mocks.deleteTables.mockImplementation(successfulTableDeleteBatch) mocks.bulkMoveFolders.mockResolvedValue({ succeeded: [], failed: [] }) mocks.bulkDeleteFolders.mockResolvedValue({ succeeded: [], @@ -151,7 +164,7 @@ describe('table bulk application use cases', () => { ).rejects.toMatchObject({ code: 'validation' }) expect(mocks.resolveWorkspaceContext).not.toHaveBeenCalled() - expect(mocks.deleteTable).not.toHaveBeenCalled() + expect(mocks.deleteTables).not.toHaveBeenCalled() }) it('bounds tables and folders against one combined cap', async () => { @@ -235,16 +248,23 @@ describe('table bulk application use cases', () => { }) expect(result.skipped).toEqual([{ kind: 'table', id: 'table-1', name: 'Table table-1' }]) - expect(mocks.deleteTable).not.toHaveBeenCalled() + expect(mocks.deleteTables).toHaveBeenCalledWith([], 'request-1', expect.anything()) expect(mocks.audit).not.toHaveBeenCalledWith( expect.objectContaining({ action: 'table.deleted' }) ) }) it('reports a locked table as a per-item failure without stranding the rest', async () => { - mocks.deleteTable.mockImplementation(async (tableId: string) => { - if (tableId === 'table-locked') throw new TableLockedError('delete') - return { archived: { name: 'Archived', workspaceId: 'workspace-1' } } + mocks.deleteTables.mockResolvedValueOnce({ + archived: [{ id: 'table-2', name: 'Archived', workspaceId: 'workspace-1' }], + failed: [ + { + id: 'table-locked', + name: 'Table table-locked', + reason: new TableLockedError('delete').message, + }, + ], + notFound: [], }) const result = await bulkDeleteTables.execute({ @@ -262,6 +282,70 @@ describe('table bulk application use cases', () => { expect(result.deleted).toEqual([{ kind: 'table', id: 'table-2', name: 'Archived' }]) }) + it('passes the complete authorized table selection to grouped deletion', async () => { + const result = await bulkDeleteTables.execute({ + principal, + input: { + assertedWorkspaceId: 'workspace-1', + tableIds: ['orders', 'customers'], + folderKeying: 'ids' as const, + folders: [], + }, + }) + + expect(result.deleted).toEqual([ + { kind: 'table', id: 'orders', name: 'Archived' }, + { kind: 'table', id: 'customers', name: 'Archived' }, + ]) + expect(result.failed).toEqual([]) + expect(mocks.deleteTables).toHaveBeenCalledExactlyOnceWith( + ['orders', 'customers'], + 'request-1', + expect.anything() + ) + expect(mocks.findReferenceBlockers).not.toHaveBeenCalled() + }) + + it('blocks a selected folder when it contains a referenced table', async () => { + mocks.planFolderSelection.mockResolvedValueOnce({ + selected: [{ id: 'folder-1', name: 'Sales' }], + notFound: [], + contained: [], + covered: new Set(['folder-1', 'folder-child']), + coveredBySelected: new Map([['folder-1', new Set(['folder-1', 'folder-child'])]]), + }) + mocks.findReferenceBlockers.mockResolvedValueOnce([ + { + targetTableId: 'customers', + targetTableName: 'Customers', + targetFolderId: 'folder-child', + referencingTableName: 'Orders', + }, + ]) + + const result = await bulkDeleteTables.execute({ + principal, + input: { + assertedWorkspaceId: 'workspace-1', + tableIds: [], + folderKeying: 'ids' as const, + folders: ['folder-1'], + }, + }) + + expect(result.deleted).toEqual([]) + expect(result.failed).toEqual([ + { + kind: 'folder', + id: 'folder-1', + name: 'Sales', + reason: + 'Cannot delete table "Customers" because it is referenced by table "Orders". Remove the reference column first.', + }, + ]) + expect(mocks.bulkDeleteFolders).not.toHaveBeenCalled() + }) + it('conceals an inaccessible table as not-found rather than naming it', async () => { mocks.resolveTableContext.mockRejectedValueOnce( new OrchestrationError('not_found', 'Table not found') @@ -445,9 +529,9 @@ describe('table bulk application use cases', () => { }) it('still notifies for the prefix a batch committed before it failed', async () => { - mocks.deleteTable.mockImplementation(async (tableId: string) => { - if (tableId === 'table-2') throw new Error('connection reset') - return { archived: { name: 'Archived', workspaceId: 'workspace-1' } } + mocks.deleteTables.mockResolvedValueOnce({ + ...successfulTableDeleteBatch(['table-1']), + terminalError: new Error('archive statement failed'), }) await expect( @@ -460,7 +544,7 @@ describe('table bulk application use cases', () => { folders: [], }, }) - ).rejects.toThrow('connection reset') + ).rejects.toThrow('archive statement failed') expect(mocks.notifyTables).toHaveBeenCalledExactlyOnceWith('workspace-1') }) @@ -483,10 +567,10 @@ describe('table bulk application use cases', () => { expect(mocks.notifyTables).not.toHaveBeenCalled() }) - it('records audit for the committed prefix before rethrowing an infrastructure failure', async () => { - mocks.deleteTable.mockImplementation(async (tableId: string) => { - if (tableId === 'table-2') throw new Error('connection reset') - return { archived: { name: 'Archived', workspaceId: 'workspace-1' } } + it('records audit for the committed prefix before rethrowing an archive statement failure', async () => { + mocks.deleteTables.mockResolvedValueOnce({ + ...successfulTableDeleteBatch(['table-1']), + terminalError: new Error('archive statement failed'), }) await expect( @@ -499,11 +583,43 @@ describe('table bulk application use cases', () => { folders: [], }, }) - ).rejects.toThrow('connection reset') + ).rejects.toThrow('archive statement failed') + + expect(mocks.audit).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ action: 'table.deleted', resourceId: 'table-1' }) + ) + expect(mocks.bulkDeleteFolders).not.toHaveBeenCalled() + }) + + it('preserves an explicit-table prefix when a selected folder is still pending', async () => { + mocks.planFolderSelection.mockResolvedValue({ + selected: [{ id: 'folder-1', name: 'Sales' }], + notFound: [], + contained: [], + covered: new Set(['folder-1']), + }) + mocks.deleteTables.mockResolvedValueOnce({ + ...successfulTableDeleteBatch(['table-1']), + terminalError: new Error('archive statement failed'), + }) + + await expect( + bulkDeleteTables.execute({ + principal, + input: { + assertedWorkspaceId: 'workspace-1', + tableIds: ['table-1', 'table-2'], + folderKeying: 'ids' as const, + folders: ['folder-1'], + }, + }) + ).rejects.toThrow('archive statement failed') expect(mocks.audit).toHaveBeenCalledExactlyOnceWith( expect.objectContaining({ action: 'table.deleted', resourceId: 'table-1' }) ) + expect(mocks.notifyTables).toHaveBeenCalledExactlyOnceWith('workspace-1') + expect(mocks.findReferenceBlockers).not.toHaveBeenCalled() expect(mocks.bulkDeleteFolders).not.toHaveBeenCalled() }) }) @@ -520,12 +636,11 @@ describe('path-keyed bulk table selections', () => { mocks.resolveWorkspaceContext.mockResolvedValue(workspaceContext) mocks.resolvePermission.mockResolvedValue('write') mocks.planFolderSelection.mockResolvedValue(emptyPlan) + mocks.findReferenceBlockers.mockResolvedValue([]) mocks.findActiveFolder.mockResolvedValue({ id: 'folder-1' }) mocks.resolveTableContext.mockImplementation(async (tableId: string) => tableContext(tableId)) mocks.moveTableToFolder.mockResolvedValue({ name: 'Moved' }) - mocks.deleteTable.mockResolvedValue({ - archived: { name: 'Archived', workspaceId: 'workspace-1' }, - }) + mocks.deleteTables.mockImplementation(successfulTableDeleteBatch) mocks.bulkMoveFolders.mockResolvedValue({ succeeded: [], failed: [] }) mocks.bulkDeleteFolders.mockResolvedValue({ succeeded: [], diff --git a/apps/sim/lib/table/application/bulk.ts b/apps/sim/lib/table/application/bulk.ts index 9fd587b4732..594f2b366ce 100644 --- a/apps/sim/lib/table/application/bulk.ts +++ b/apps/sim/lib/table/application/bulk.ts @@ -1,5 +1,6 @@ import { AuditAction, AuditResourceType } from '@sim/audit' import { resolvePrincipalAttribution } from '@sim/auth/principal' +import { db } from '@sim/db' import { createLogger } from '@sim/logger' import { type BulkItemDisposition, classifyBulkItemError } from '@/lib/core/application/bulk-items' import { OrchestrationError } from '@/lib/core/orchestration/types' @@ -13,7 +14,7 @@ import { import { ROOT_FOLDER_PATH } from '@/lib/folders/paths' import { findActiveFolder, resolveFolderPathFromIndex } from '@/lib/folders/queries' import { notifyWorkspaceTablesChanged } from '@/lib/realtime/notify' -import { deleteTable, moveTableToFolder } from '@/lib/table' +import { deleteTables, moveTableToFolder } from '@/lib/table' import { authorizeTableOperation } from '@/lib/table/application/authorization' import { defineAuthorizedTableUseCase } from '@/lib/table/application/authorized-table-use-case' import { @@ -32,6 +33,10 @@ import { } from '@/lib/table/application/context' import { resolveTableFolderPath } from '@/lib/table/application/folder-paths' import { tableOperations } from '@/lib/table/application/operations' +import { + findActiveTableReferenceBlockers, + tableReferenceBlockerMessage, +} from '@/lib/table/column-types/registry.server' import { signalTableSchemaChanged } from '@/lib/table/events' import { TableLockedError } from '@/lib/table/mutation-locks' @@ -502,38 +507,94 @@ export const bulkDeleteTables = defineAuthorizedTableUseCase({ foldFolderPlan(plan, outcome) try { - const terminalError = await runTableItems( + const authorizedTables: BulkTableItem[] = [] + const resolutionError = await runTableItems( context.tableIds, context, plan.covered, (canonical) => authorizeTableOperation(principal, tableOperations.bulkDelete, canonical), - async (canonical) => { - const { archived } = await deleteTable(canonical.table.id, generateRequestId(), { - expectedWorkspaceId: context.workspaceId, - skipNotify: true, - }) - if (!archived) throw new OrchestrationError('not_found', 'Table not found') - return archived.name - }, - deleted, + async (canonical) => canonical.table.name, + authorizedTables, outcome ) + const tableResult = await deleteTables( + authorizedTables.map((table) => table.id), + generateRequestId(), + { + expectedWorkspaceId: context.workspaceId, + skipNotify: true, + } + ) + for (const table of tableResult.archived) { + deleted.push({ kind: 'table', id: table.id, name: table.name }) + } + for (const table of tableResult.failed) { + outcome.failed.push({ kind: 'table', id: table.id, name: table.name, reason: table.reason }) + } + for (const tableId of tableResult.notFound) { + outcome.notFound.push({ kind: 'table', id: tableId }) + } + + let terminalError = tableResult.terminalError ?? resolutionError + let folderReferenceBlockers: Awaited> = [] + if (terminalError === undefined && plan.selected.length > 0) { + try { + folderReferenceBlockers = await findActiveTableReferenceBlockers( + db, + context.workspaceId, + { folderIds: plan.covered } + ) + } catch (error) { + terminalError = error + } + } + const blockedFolderIds = + folderReferenceBlockers.length === 0 + ? new Map() + : new Map( + plan.selected.flatMap((folder) => { + const coveredFolderIds = plan.coveredBySelected.get(folder.id) + if (!coveredFolderIds) { + throw new Error(`Missing folder coverage for ${folder.id}`) + } + const blocker = folderReferenceBlockers.find( + (candidate) => + candidate.targetFolderId !== null && + coveredFolderIds.has(candidate.targetFolderId) + ) + return blocker ? [[folder.id, blocker] as const] : [] + }) + ) const deletedItems = { tables: deleted.length, folders: 0 } if (terminalError === undefined && plan.selected.length > 0) { - const folders = await bulkDeleteFolders({ - workspaceId: context.workspaceId, - resourceType: TABLE_FOLDER_RESOURCE_TYPE, - userId: resolvePrincipalAttribution(principal, { - workspaceBillingOwnerUserId: context.billedAccountUserId, - }).attributedUserId, - folders: plan.selected, - countKey: 'tables', + const deletableFolders = plan.selected.filter((folder) => { + const blocker = blockedFolderIds.get(folder.id) + if (!blocker) return true + outcome.failed.push({ + kind: 'folder', + ...folder, + reason: tableReferenceBlockerMessage(blocker.targetTableName, [ + blocker.referencingTableName, + ]), + }) + return false }) - for (const folder of folders.succeeded) deleted.push({ kind: 'folder', ...folder }) - for (const folder of folders.failed) outcome.failed.push({ kind: 'folder', ...folder }) - deletedItems.folders = folders.folderCount - deletedItems.tables += folders.resourceCount + if (deletableFolders.length > 0) { + const folders = await bulkDeleteFolders({ + workspaceId: context.workspaceId, + resourceType: TABLE_FOLDER_RESOURCE_TYPE, + userId: resolvePrincipalAttribution(principal, { + workspaceBillingOwnerUserId: context.billedAccountUserId, + }).attributedUserId, + folders: deletableFolders, + countKey: 'tables', + }) + for (const folder of folders.succeeded) deleted.push({ kind: 'folder', ...folder }) + for (const folder of folders.failed) outcome.failed.push({ kind: 'folder', ...folder }) + deletedItems.folders = folders.folderCount + deletedItems.tables += folders.resourceCount + } } logger.info('Bulk archived tables and folders', { diff --git a/apps/sim/lib/table/application/tables.test.ts b/apps/sim/lib/table/application/tables.test.ts index 405b1c73056..b7463592d63 100644 --- a/apps/sim/lib/table/application/tables.test.ts +++ b/apps/sim/lib/table/application/tables.test.ts @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ getTableById: vi.fn(), getLimits: vi.fn(), listDefinitions: vi.fn(), + listNames: vi.fn(), loadFolderIndex: vi.fn(), queryTables: vi.fn(), resolveArchivedContext: vi.fn(), @@ -49,6 +50,7 @@ vi.mock('@/lib/table', () => ({ deleteTable: vi.fn(), getTableById: mocks.getTableById, getWorkspaceTableLimits: mocks.getLimits, + listActiveTableNames: mocks.listNames, listTables: mocks.listDefinitions, moveTableToFolder: vi.fn(), queryTables: mocks.queryTables, @@ -82,6 +84,7 @@ vi.mock('@/lib/table/events', () => ({ signalTableSchemaChanged: mocks.signal }) import { listTableDefinitionsUseCase, + listTableNamesUseCase, listTablesUseCase, readTableDefinitionUseCase, readTableDetailsUseCase, @@ -220,6 +223,7 @@ describe('internal table compatibility reads', () => { table: active, }) mocks.listDefinitions.mockResolvedValue([active]) + mocks.listNames.mockResolvedValue([{ id: active.id, name: active.name }]) mocks.getLimits.mockResolvedValue({ maxRowsPerTable: 2500 }) }) @@ -234,6 +238,17 @@ describe('internal table compatibility reads', () => { expect(mocks.loadFolderIndex).not.toHaveBeenCalled() }) + it('lists only active table names for lightweight display lookups', async () => { + const result = await listTableNamesUseCase.execute({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE.workspaceId, tableIds: ['table-1', 'table-2'] }, + }) + + expect(mocks.listNames).toHaveBeenCalledWith(WORKSPACE.workspaceId, ['table-1', 'table-2']) + expect(result.tables).toEqual([{ id: active.id, name: active.name }]) + expect(mocks.listDefinitions).not.toHaveBeenCalled() + }) + it('reads schema-only metadata without loading folders or plan limits', async () => { const result = await readTableDefinitionUseCase.execute({ principal: PRINCIPAL, diff --git a/apps/sim/lib/table/application/tables.ts b/apps/sim/lib/table/application/tables.ts index 52974c43c4a..ee9886a89ad 100644 --- a/apps/sim/lib/table/application/tables.ts +++ b/apps/sim/lib/table/application/tables.ts @@ -15,6 +15,7 @@ import { deleteTable, getTableById, getWorkspaceTableLimits, + listActiveTableNames, listTables as listTableDefinitions, moveTableToFolder, queryTables, @@ -111,6 +112,20 @@ export const listTableDefinitionsUseCase = defineAuthorizedTableUseCase({ }, }) +export interface ListTableNamesInput { + workspaceId: string + tableIds: string[] +} + +export const listTableNamesUseCase = defineAuthorizedTableUseCase({ + operation: tableOperations.list, + resolveContext: ({ input }: { input: ListTableNamesInput }) => + resolveTableWorkspaceContext(input.workspaceId), + async execute({ input, context }) { + return { tables: await listActiveTableNames(context.workspaceId, input.tableIds) } + }, +}) + export interface CreateTableInput { workspaceId: string name: string diff --git a/apps/sim/lib/table/column-types/reference.ts b/apps/sim/lib/table/column-types/reference.ts index 7138c7fe866..932efbd0daf 100644 --- a/apps/sim/lib/table/column-types/reference.ts +++ b/apps/sim/lib/table/column-types/reference.ts @@ -15,6 +15,14 @@ export const referenceColumnType: ColumnTypeDefinition = { workflowInputType: 'string', editor: 'text', expandable: false, + referencePreview: { + getTableId(column) { + return column.referenceTableId + }, + getRowId(value) { + return typeof value === 'string' && value.length > 0 ? value : null + }, + }, coerce: stringColumnType.coerce, diff --git a/apps/sim/lib/table/column-types/registry.server.test.ts b/apps/sim/lib/table/column-types/registry.server.test.ts index 14f64905dd9..fda48cc14ca 100644 --- a/apps/sim/lib/table/column-types/registry.server.test.ts +++ b/apps/sim/lib/table/column-types/registry.server.test.ts @@ -4,17 +4,24 @@ import { hasMockCondition, schemaMock } from '@sim/testing' import { describe, expect, it, vi } from 'vitest' -import { assertColumnReferencesInWorkspace } from '@/lib/table/column-types/registry.server' +import type { DbOrTx } from '@/lib/db/types' +import { + assertColumnReferencesInWorkspace, + findActiveTableReferenceBlockers, + tableReferenceBlockerMessage, +} from '@/lib/table/column-types/registry.server' import type { DbTransaction } from '@/lib/table/planner' function transactionWithTargets(targetIds: string[]) { - const where = vi.fn().mockResolvedValue(targetIds.map((id) => ({ id }))) + const lock = vi.fn().mockResolvedValue(targetIds.map((id) => ({ id }))) + const where = vi.fn(() => ({ for: lock })) const from = vi.fn(() => ({ where })) const select = vi.fn(() => ({ from })) return { trx: { select } as unknown as DbTransaction, select, where, + lock, } } @@ -30,7 +37,7 @@ describe('assertColumnReferencesInWorkspace', () => { }) it('accepts active Reference targets returned for the workspace', async () => { - const { trx, select, where } = transactionWithTargets(['tbl_accounts', 'tbl_companies']) + const { trx, select, where, lock } = transactionWithTargets(['tbl_accounts', 'tbl_companies']) await assertColumnReferencesInWorkspace(trx, 'ws_1', [ { @@ -68,6 +75,7 @@ describe('assertColumnReferencesInWorkspace', () => { node.values.length === 2 ) ).toBe(true) + expect(lock).toHaveBeenCalledWith('key share') expect( hasMockCondition( condition, @@ -77,6 +85,53 @@ describe('assertColumnReferencesInWorkspace', () => { ).toBe(true) }) + it('admits archived targets that are part of the same restore cohort', async () => { + const { trx, where } = transactionWithTargets(['tbl_accounts', 'tbl_companies']) + + await assertColumnReferencesInWorkspace( + trx, + 'ws_1', + [ + { + id: 'col_account', + name: 'Account', + type: 'reference', + referenceTableId: 'tbl_accounts', + }, + { + id: 'col_company', + name: 'Company', + type: 'reference', + referenceTableId: 'tbl_companies', + }, + ], + { allowedArchivedTableIds: new Set(['tbl_companies']) } + ) + + const condition = where.mock.calls[0][0] + expect( + hasMockCondition( + condition, + (node) => + node.type === 'or' && + Array.isArray(node.conditions) && + node.conditions.some( + (nested) => + typeof nested === 'object' && + nested !== null && + 'type' in nested && + nested.type === 'inArray' && + 'column' in nested && + nested.column === schemaMock.userTableDefinitions.id && + 'values' in nested && + Array.isArray(nested.values) && + nested.values.length === 1 && + nested.values[0] === 'tbl_companies' + ) + ) + ).toBe(true) + }) + it('conceals missing, archived, and cross-workspace targets as not found', async () => { const { trx } = transactionWithTargets(['tbl_accounts']) @@ -101,3 +156,156 @@ describe('assertColumnReferencesInWorkspace', () => { }) }) }) + +describe('findActiveTableReferenceBlockers', () => { + const activeTables = [ + { + id: 'tbl_customers', + name: 'Customers', + folderId: 'folder_sales', + schema: { columns: [{ id: 'name', name: 'Name', type: 'string' }] }, + }, + { + id: 'tbl_orders', + name: 'Orders', + folderId: null, + schema: { + columns: [ + { + id: 'customer', + name: 'Customer', + type: 'reference', + referenceTableId: 'tbl_customers', + }, + ], + }, + }, + ] + + function executorWithTables(tables = activeTables) { + const where = vi.fn().mockResolvedValue(tables) + const from = vi.fn(() => ({ where })) + return { select: vi.fn(() => ({ from })) } as unknown as DbOrTx + } + + it('names the referring table for a selected target table', async () => { + await expect( + findActiveTableReferenceBlockers(executorWithTables(), 'ws_1', { + tableIds: ['tbl_customers'], + }) + ).resolves.toEqual([ + { + targetTableId: 'tbl_customers', + targetTableName: 'Customers', + targetFolderId: 'folder_sales', + referencingTableName: 'Orders', + }, + ]) + }) + + it('finds referenced targets anywhere in a selected folder subtree', async () => { + const blockers = await findActiveTableReferenceBlockers(executorWithTables(), 'ws_1', { + folderIds: new Set(['folder_sales']), + }) + + expect(blockers).toHaveLength(1) + expect(blockers[0]?.targetTableName).toBe('Customers') + }) + + it('allows references between tables in the same deletion selection', async () => { + await expect( + findActiveTableReferenceBlockers(executorWithTables(), 'ws_1', { + tableIds: ['tbl_customers', 'tbl_orders'], + }) + ).resolves.toEqual([]) + }) + + it('does not inspect untraversed selected target schemas', async () => { + const selectedTarget = { + id: 'tbl_customers', + name: 'Customers', + folderId: 'folder_sales', + get schema() { + throw new Error('selected target schema should not be inspected') + }, + } + + await expect( + findActiveTableReferenceBlockers(executorWithTables([selectedTarget]), 'ws_1', { + tableIds: ['tbl_customers'], + }) + ).resolves.toEqual([]) + }) + + it('keeps transitively referenced targets when another selected table cannot be deleted', async () => { + const tables = [ + ...activeTables, + { + id: 'tbl_invoices', + name: 'Invoices', + folderId: null, + schema: { + columns: [ + { + id: 'order', + name: 'Order', + type: 'reference', + referenceTableId: 'tbl_orders', + }, + ], + }, + }, + ] + + await expect( + findActiveTableReferenceBlockers(executorWithTables(tables), 'ws_1', { + tableIds: ['tbl_customers', 'tbl_orders'], + }) + ).resolves.toEqual([ + { + targetTableId: 'tbl_customers', + targetTableName: 'Customers', + targetFolderId: 'folder_sales', + referencingTableName: 'Orders', + }, + { + targetTableId: 'tbl_orders', + targetTableName: 'Orders', + targetFolderId: null, + referencingTableName: 'Invoices', + }, + ]) + }) + + it('allows a self-referencing table to be deleted', async () => { + const selfReferencingTable = { + id: 'tbl_categories', + name: 'Categories', + folderId: null, + schema: { + columns: [ + { + id: 'parent', + name: 'Parent', + type: 'reference', + referenceTableId: 'tbl_categories', + }, + ], + }, + } + + await expect( + findActiveTableReferenceBlockers(executorWithTables([selfReferencingTable]), 'ws_1', { + tableIds: ['tbl_categories'], + }) + ).resolves.toEqual([]) + }) +}) + +describe('tableReferenceBlockerMessage', () => { + it('shows the target and every table preventing deletion', () => { + expect(tableReferenceBlockerMessage('Customers', ['Orders', 'Invoices'])).toBe( + 'Cannot delete table "Customers" because it is referenced by tables "Invoices", "Orders". Remove the reference columns first.' + ) + }) +}) diff --git a/apps/sim/lib/table/column-types/registry.server.ts b/apps/sim/lib/table/column-types/registry.server.ts index afc0c2f4a05..2f2a14a4b2d 100644 --- a/apps/sim/lib/table/column-types/registry.server.ts +++ b/apps/sim/lib/table/column-types/registry.server.ts @@ -12,8 +12,10 @@ */ import { userTableDefinitions, userTableRows } from '@sim/db/schema' -import { and, eq, inArray, isNull, sql } from 'drizzle-orm' +import { formatQuotedNameList } from '@sim/utils/string' +import { and, eq, inArray, isNull, or, sql } from 'drizzle-orm' import { OrchestrationError } from '@/lib/core/orchestration/types' +import type { DbOrTx } from '@/lib/db/types' import { COLUMN_TYPE_REGISTRY } from '@/lib/table/column-types/registry' import type { ColumnType } from '@/lib/table/column-types/types' import type { @@ -22,7 +24,7 @@ import type { } from '@/lib/table/column-types/types.server' import type { DbTransaction } from '@/lib/table/planner' import { updateTableRowsWithDerivedSecretProvenance } from '@/lib/table/rows/secret-provenance' -import type { ColumnDefinition, JsonValue, SelectOption } from '@/lib/table/types' +import type { ColumnDefinition, JsonValue, SelectOption, TableSchema } from '@/lib/table/types' /** * Rewrites a column's cells from stored option **ids** to option **names**, for @@ -295,9 +297,136 @@ export const COLUMN_TYPE_SERVER_REGISTRY: Record typeof column.referenceTableId === 'string' ? [column.referenceTableId] : [], + remapReferencedTableIds: (column, tableIdMap) => { + const referenceTableId = column.referenceTableId + if (typeof referenceTableId !== 'string') return column + const remappedTableId = tableIdMap.get(referenceTableId) + return remappedTableId && remappedTableId !== referenceTableId + ? { ...column, referenceTableId: remappedTableId } + : column + }, }, } +export function collectColumnReferencedTableIds(columns: readonly ColumnDefinition[]): string[] { + return [ + ...new Set( + columns.flatMap( + (column) => COLUMN_TYPE_SERVER_REGISTRY[column.type].referencedTableIds?.(column) ?? [] + ) + ), + ] +} + +export interface ActiveTableReferenceBlocker { + targetTableId: string + targetTableName: string + targetFolderId: string | null + referencingTableName: string +} + +export function tableReferenceBlockerMessage( + targetTableName: string, + referencingTableNames: readonly string[] +): string { + const uniqueNames = [...new Set(referencingTableNames)].sort() + const blockerLabel = uniqueNames.length === 1 ? 'table' : 'tables' + const columnLabel = uniqueNames.length === 1 ? 'column' : 'columns' + return `Cannot delete table "${targetTableName}" because it is referenced by ${blockerLabel} ${formatQuotedNameList(uniqueNames, uniqueNames.length)}. Remove the reference ${columnLabel} first.` +} + +/** + * Finds active tables that point at any active table in the requested deletion selection. + * + * Reading the active definitions once avoids issuing one JSONB search per table in a deletion + * selection. Reference ownership still comes from the server column-type registry; this function + * does not duplicate knowledge of the `reference` column shape. + */ +export async function findActiveTableReferenceBlockers( + executor: DbOrTx, + workspaceId: string, + selection: { tableIds?: readonly string[]; folderIds?: ReadonlySet } +): Promise { + const selectedTableIds = new Set(selection.tableIds) + const selectedFolderIds = selection.folderIds ?? new Set() + if (selectedTableIds.size === 0 && selectedFolderIds.size === 0) return [] + + const activeTables = await executor + .select({ + id: userTableDefinitions.id, + name: userTableDefinitions.name, + folderId: userTableDefinitions.folderId, + schema: userTableDefinitions.schema, + }) + .from(userTableDefinitions) + .where( + and( + eq(userTableDefinitions.workspaceId, workspaceId), + isNull(userTableDefinitions.archivedAt) + ) + ) + + const selectedTargets = new Map( + activeTables + .filter( + (table) => + selectedTableIds.has(table.id) || + (table.folderId !== null && selectedFolderIds.has(table.folderId)) + ) + .map((table) => [table.id, table]) + ) + if (selectedTargets.size === 0) return [] + + const remainingTargetIds = new Set(selectedTargets.keys()) + let referencingTables = activeTables.filter((table) => !remainingTargetIds.has(table.id)) + const blockers: ActiveTableReferenceBlocker[] = [] + + while (referencingTables.length > 0 && remainingTargetIds.size > 0) { + const round: ActiveTableReferenceBlocker[] = [] + const newlyBlockedTargetIds = new Set() + for (const referencingTable of referencingTables) { + for (const referencedTableId of collectColumnReferencedTableIds( + (referencingTable.schema as TableSchema).columns + )) { + if (!remainingTargetIds.has(referencedTableId)) continue + const target = selectedTargets.get(referencedTableId) + if (!target) continue + round.push({ + targetTableId: target.id, + targetTableName: target.name, + targetFolderId: target.folderId, + referencingTableName: referencingTable.name, + }) + newlyBlockedTargetIds.add(target.id) + } + } + if (round.length === 0) break + blockers.push(...round) + for (const targetId of newlyBlockedTargetIds) remainingTargetIds.delete(targetId) + referencingTables = [...newlyBlockedTargetIds].flatMap((targetId) => { + const table = selectedTargets.get(targetId) + return table ? [table] : [] + }) + } + + return blockers.sort( + (left, right) => + left.targetTableName.localeCompare(right.targetTableName) || + left.referencingTableName.localeCompare(right.referencingTableName) + ) +} + +export function remapColumnReferencedTableIds( + columns: readonly ColumnDefinition[], + tableIdMap: ReadonlyMap +): ColumnDefinition[] { + return columns.map( + (column) => + COLUMN_TYPE_SERVER_REGISTRY[column.type].remapReferencedTableIds?.(column, tableIdMap) ?? + column + ) +} + /** * Validates every table ID referenced by column metadata in one query. * @@ -307,16 +436,21 @@ export const COLUMN_TYPE_SERVER_REGISTRY: Record } ): Promise { - const referencedTableIds = [ - ...new Set( - columns.flatMap( - (column) => COLUMN_TYPE_SERVER_REGISTRY[column.type].referencedTableIds?.(column) ?? [] - ) - ), - ] + const referencedTableIds = collectColumnReferencedTableIds(columns) if (referencedTableIds.length === 0) return + const allowedArchivedTableIds = referencedTableIds.filter((id) => + options?.allowedArchivedTableIds?.has(id) + ) + const availableTarget = + allowedArchivedTableIds.length > 0 + ? or( + isNull(userTableDefinitions.archivedAt), + inArray(userTableDefinitions.id, allowedArchivedTableIds) + ) + : isNull(userTableDefinitions.archivedAt) const targets = await trx .select({ id: userTableDefinitions.id }) @@ -325,9 +459,10 @@ export async function assertColumnReferencesInWorkspace( and( eq(userTableDefinitions.workspaceId, workspaceId), inArray(userTableDefinitions.id, referencedTableIds), - isNull(userTableDefinitions.archivedAt) + availableTarget ) ) + .for('key share') const foundIds = new Set(targets.map((target) => target.id)) const missingId = referencedTableIds.find((id) => !foundIds.has(id)) if (missingId) { diff --git a/apps/sim/lib/table/column-types/types.server.ts b/apps/sim/lib/table/column-types/types.server.ts index b569c73a0ea..6bc42ad2377 100644 --- a/apps/sim/lib/table/column-types/types.server.ts +++ b/apps/sim/lib/table/column-types/types.server.ts @@ -37,6 +37,14 @@ export interface ColumnTypeServerDefinition { * a schema is persisted. Omitted by types that do not reference tables. */ readonly referencedTableIds?: (column: ColumnDefinition) => readonly string[] + /** + * Rewrites this column's table references through a source-to-target identity map. + * Omitted by types that do not reference tables. + */ + readonly remapReferencedTableIds?: ( + column: ColumnDefinition, + tableIdMap: ReadonlyMap + ) => ColumnDefinition /** * Rewrites cells into this type's canonical storage shape when a column is * converted **to** it. Omitted when the stored bytes are already correct. diff --git a/apps/sim/lib/table/column-types/types.ts b/apps/sim/lib/table/column-types/types.ts index 807b0c3e6ce..addbd31e53a 100644 --- a/apps/sim/lib/table/column-types/types.ts +++ b/apps/sim/lib/table/column-types/types.ts @@ -75,6 +75,11 @@ export type TypeSpecificColumnKey = (typeof TYPE_SPECIFIC_COLUMN_KEYS)[number] /** Result of coercing a raw value toward a column's declared type. */ export type CoerceResult = { ok: true; value: JsonValue } | { ok: false } +export interface ColumnReferencePreviewDefinition { + getTableId(column: ColumnDefinition): string | undefined + getRowId(value: unknown): string | null +} + export interface ColumnTypeDefinition { readonly id: ColumnType @@ -141,6 +146,8 @@ export interface ColumnTypeDefinition { * bounded, structured value. */ readonly expandable: boolean + /** Optional inline referenced-row presentation owned by this column type. */ + readonly referencePreview?: ColumnReferencePreviewDefinition /** `inputMode` for the text editor, when the type wants a specific keypad. */ readonly inputMode?: 'decimal' /** diff --git a/apps/sim/lib/table/service.test.ts b/apps/sim/lib/table/service.test.ts index 0250581ab55..be4d8e349e7 100644 --- a/apps/sim/lib/table/service.test.ts +++ b/apps/sim/lib/table/service.test.ts @@ -10,16 +10,37 @@ import { } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { DbOrTx } from '@/lib/db/types' +import { MAX_TABLE_BATCH_ITEMS } from '@/lib/table/constants' import type { TableSchema } from '@/lib/table/types' const mocks = vi.hoisted(() => ({ assertColumnReferencesInWorkspace: vi.fn(), + collectColumnReferencedTableIds: vi.fn( + (columns: readonly { type: string; referenceTableId?: unknown }[]) => [ + ...new Set( + columns.flatMap((column) => + column.type === 'reference' && typeof column.referenceTableId === 'string' + ? [column.referenceTableId] + : [] + ) + ), + ] + ), + findActiveTableReferenceBlockers: vi.fn(), assertTableReferenceColumnsEnabled: vi.fn(), + getWorkspaceWithOwner: vi.fn(), + tableReferenceBlockerMessage: vi.fn( + (target: string, blockers: string[]) => + `Cannot delete table "${target}" because it is referenced by table "${blockers[0]}". Remove the reference column first.` + ), assertTableRowTtlEnabled: vi.fn(), })) vi.mock('@/lib/table/column-types/registry.server', () => ({ assertColumnReferencesInWorkspace: mocks.assertColumnReferencesInWorkspace, + collectColumnReferencedTableIds: mocks.collectColumnReferencedTableIds, + findActiveTableReferenceBlockers: mocks.findActiveTableReferenceBlockers, + tableReferenceBlockerMessage: mocks.tableReferenceBlockerMessage, })) vi.mock('@/lib/table/reference-columns/availability', () => ({ @@ -35,14 +56,55 @@ vi.mock('@/lib/table/billing', () => ({ notifyTableRowUsage: vi.fn(), })) +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + getWorkspaceWithOwner: mocks.getWorkspaceWithOwner, +})) + vi.mock('@/lib/table/ttl-availability', () => ({ assertTableRowTtlEnabled: mocks.assertTableRowTtlEnabled, })) -import { createTable, getTableById } from '@/lib/table/service' +import { + createTable, + deleteTable, + deleteTables, + getTableById, + listActiveTableNames, + restoreTable, +} from '@/lib/table/service' const WORKSPACE_ID = '6fc7631d-88cd-46f8-9f0a-d4764daef7f8' +describe('listActiveTableNames', () => { + beforeEach(() => resetDbChainMock()) + + it('returns the name projection without loading table schemas', async () => { + queueTableRows(schemaMock.userTableDefinitions, [{ id: 'table-1', name: 'Accounts' }]) + + await expect(listActiveTableNames(WORKSPACE_ID, ['table-1', 'table-2'])).resolves.toEqual([ + { id: 'table-1', name: 'Accounts' }, + ]) + expect(dbChainMockFns.select).toHaveBeenCalledWith({ + id: schemaMock.userTableDefinitions.id, + name: schemaMock.userTableDefinitions.name, + }) + expect( + hasMockCondition( + dbChainMockFns.where.mock.calls[0][0], + (node) => + node.type === 'inArray' && + node.column === schemaMock.userTableDefinitions.id && + JSON.stringify(node.values) === JSON.stringify(['table-1', 'table-2']) + ) + ).toBe(true) + }) + + it('skips the database when no table IDs are requested', async () => { + await expect(listActiveTableNames(WORKSPACE_ID, [])).resolves.toEqual([]) + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) +}) + /** A column produced by a workflow group, and the group that declares it. */ function groupedSchema(overrides: { columnGroupId: string; groupId: string }): TableSchema { return { @@ -372,3 +434,438 @@ describe('getTableById job derivation', () => { expect(dbChainMockFns.select).not.toHaveBeenCalled() }) }) + +describe('restoreTable reference validation', () => { + const referenceSchema = { + columns: [ + { + id: 'col_account', + name: 'account', + type: 'reference', + referenceTableId: 'tbl_accounts', + }, + ], + } as TableSchema + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mocks.assertColumnReferencesInWorkspace.mockResolvedValue(undefined) + mocks.getWorkspaceWithOwner.mockResolvedValue({ id: WORKSPACE_ID, archivedAt: null }) + }) + + it('validates targets under the row lock and admits the restore cohort', async () => { + const archived = definitionRow({ + archivedAt: new Date('2026-01-03T00:00:00Z'), + schema: referenceSchema, + }) + queueTableRows(schemaMock.userTableDefinitions, [archived]) + queueTableRows(schemaMock.userTableDefinitions, [archived]) + queueTableRows(schemaMock.userTableDefinitions, []) + const restoringTableIds = new Set(['tbl_accounts']) + + await restoreTable(TABLE_ID, 'request-1', { restoringTableIds }) + + expect(mocks.assertColumnReferencesInWorkspace).toHaveBeenCalledWith( + expect.anything(), + WORKSPACE_ID, + referenceSchema.columns, + { allowedArchivedTableIds: new Set(['tbl_accounts', TABLE_ID]) } + ) + expect(dbChainMockFns.update).toHaveBeenCalledWith(schemaMock.userTableDefinitions) + }) + + it('leaves the table archived when a reference target is unavailable', async () => { + const archived = definitionRow({ + archivedAt: new Date('2026-01-03T00:00:00Z'), + schema: referenceSchema, + }) + queueTableRows(schemaMock.userTableDefinitions, [archived]) + queueTableRows(schemaMock.userTableDefinitions, [archived]) + mocks.assertColumnReferencesInWorkspace.mockRejectedValueOnce({ code: 'not_found' }) + + await expect(restoreTable(TABLE_ID, 'request-1')).rejects.toMatchObject({ code: 'not_found' }) + + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) +}) + +describe('deleteTable reference guard', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mocks.findActiveTableReferenceBlockers.mockResolvedValue([]) + }) + + const activeTable = { + name: 'Customers', + archivedAt: null, + deleteLocked: false, + workspaceId: WORKSPACE_ID, + } + + it('archives an unreferenced table inside the guarded transaction', async () => { + queueTableRows(schemaMock.userTableDefinitions, [activeTable]) + dbChainMockFns.returning.mockResolvedValueOnce([ + { name: 'Customers', workspaceId: WORKSPACE_ID }, + ]) + + await expect(deleteTable('tbl_customers', 'request-1')).resolves.toEqual({ + archived: { name: 'Customers', workspaceId: WORKSPACE_ID }, + }) + + expect(dbChainMockFns.for).toHaveBeenCalledWith('update') + expect(mocks.findActiveTableReferenceBlockers).toHaveBeenCalledWith( + expect.anything(), + WORKSPACE_ID, + { tableIds: ['tbl_customers'] } + ) + expect(dbChainMockFns.update).toHaveBeenCalledOnce() + }) + + it('blocks deletion and names the table holding the reference', async () => { + queueTableRows(schemaMock.userTableDefinitions, [activeTable]) + mocks.findActiveTableReferenceBlockers.mockResolvedValueOnce([ + { + targetTableId: 'tbl_customers', + targetTableName: 'Customers', + targetFolderId: null, + referencingTableName: 'Orders', + }, + ]) + + await expect(deleteTable('tbl_customers', 'request-1')).rejects.toEqual( + expect.objectContaining({ + code: 'conflict', + message: + 'Cannot delete table "Customers" because it is referenced by table "Orders". Remove the reference column first.', + }) + ) + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) + + it('keeps the existing delete-lock verdict ahead of the reference check', async () => { + queueTableRows(schemaMock.userTableDefinitions, [{ ...activeTable, deleteLocked: true }]) + + await expect(deleteTable('tbl_customers', 'request-1')).rejects.toMatchObject({ + name: 'TableLockedError', + lock: 'delete', + }) + expect(mocks.findActiveTableReferenceBlockers).not.toHaveBeenCalled() + }) +}) + +function batchTable( + id: string, + name: string, + referencedTableIds: readonly string[] = [] +): { + id: string + name: string + schema: TableSchema + archivedAt: null + deleteLocked: false + workspaceId: string +} { + return { + id, + name, + schema: { + columns: referencedTableIds.map((referenceTableId, index) => ({ + id: `reference-${index}`, + name: `Reference ${index}`, + type: 'reference' as const, + referenceTableId, + })), + }, + archivedAt: null, + deleteLocked: false, + workspaceId: WORKSPACE_ID, + } +} + +describe('deleteTables reference guard', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mocks.findActiveTableReferenceBlockers.mockResolvedValue([]) + }) + + it('returns immediately for an empty selection', async () => { + await expect( + deleteTables([], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + ).resolves.toEqual({ archived: [], failed: [], notFound: [] }) + + expect(dbChainMockFns.transaction).not.toHaveBeenCalled() + expect(mocks.findActiveTableReferenceBlockers).not.toHaveBeenCalled() + }) + + it('acquires every schema advisory lock in id order before selecting rows for update', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_customers', 'Customers'), + batchTable('tbl_orders', 'Orders'), + ]) + dbChainMockFns.returning + .mockResolvedValueOnce([{ id: 'tbl_orders', name: 'Orders', workspaceId: WORKSPACE_ID }]) + .mockResolvedValueOnce([ + { id: 'tbl_customers', name: 'Customers', workspaceId: WORKSPACE_ID }, + ]) + + await deleteTables(['tbl_orders', 'tbl_customers'], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + + const advisoryLockCallIndex = dbChainMockFns.execute.mock.calls.findIndex(([query]) => + ((query as { strings?: readonly string[] }).strings ?? []).some((part) => + part.includes('pg_advisory_xact_lock') + ) + ) + expect(advisoryLockCallIndex).toBeGreaterThanOrEqual(0) + const advisoryLockQuery = dbChainMockFns.execute.mock.calls[advisoryLockCallIndex][0] as { + strings?: readonly string[] + values?: unknown[] + } + expect(advisoryLockQuery.strings?.join('')).toContain('FROM unnest(') + expect(advisoryLockQuery.values).toContainEqual(['tbl_customers', 'tbl_orders']) + expect(dbChainMockFns.execute.mock.invocationCallOrder[advisoryLockCallIndex]).toBeLessThan( + dbChainMockFns.select.mock.invocationCallOrder[0] + ) + }) + + it('checks the complete deletion selection once before archiving each table', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_customers', 'Customers'), + batchTable('tbl_orders', 'Orders'), + ]) + dbChainMockFns.returning + .mockResolvedValueOnce([ + { id: 'tbl_customers', name: 'Customers', workspaceId: WORKSPACE_ID }, + ]) + .mockResolvedValueOnce([{ id: 'tbl_orders', name: 'Orders', workspaceId: WORKSPACE_ID }]) + + await expect( + deleteTables(['tbl_customers', 'tbl_orders'], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + ).resolves.toMatchObject({ + archived: [ + { id: 'tbl_customers', name: 'Customers', workspaceId: WORKSPACE_ID }, + { id: 'tbl_orders', name: 'Orders', workspaceId: WORKSPACE_ID }, + ], + failed: [], + notFound: [], + }) + + expect(mocks.findActiveTableReferenceBlockers).toHaveBeenCalledOnce() + expect(mocks.findActiveTableReferenceBlockers).toHaveBeenCalledWith( + expect.anything(), + WORKSPACE_ID, + { tableIds: ['tbl_customers', 'tbl_orders'] } + ) + expect(dbChainMockFns.update).toHaveBeenCalledTimes(2) + }) + + it('archives unreferenced tables while reporting referenced targets', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_customers', 'Customers'), + batchTable('tbl_orders', 'Orders'), + ]) + mocks.findActiveTableReferenceBlockers.mockResolvedValueOnce([ + { + targetTableId: 'tbl_customers', + targetTableName: 'Customers', + targetFolderId: null, + referencingTableName: 'Invoices', + }, + ]) + dbChainMockFns.returning.mockResolvedValueOnce([ + { id: 'tbl_orders', name: 'Orders', workspaceId: WORKSPACE_ID }, + ]) + + await expect( + deleteTables(['tbl_customers', 'tbl_orders'], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + ).resolves.toEqual({ + archived: [{ id: 'tbl_orders', name: 'Orders', workspaceId: WORKSPACE_ID }], + failed: [ + { + id: 'tbl_customers', + name: 'Customers', + reason: + 'Cannot delete table "Customers" because it is referenced by table "Invoices". Remove the reference column first.', + }, + ], + notFound: [], + }) + + expect(mocks.findActiveTableReferenceBlockers).toHaveBeenCalledOnce() + expect(dbChainMockFns.update).toHaveBeenCalledOnce() + }) + + it('rejects a multi-table reference cycle while allowing unrelated tables to archive', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_accounts', 'Accounts', ['tbl_contacts']), + batchTable('tbl_contacts', 'Contacts', ['tbl_accounts']), + batchTable('tbl_notes', 'Notes'), + ]) + dbChainMockFns.returning.mockResolvedValueOnce([ + { id: 'tbl_notes', name: 'Notes', workspaceId: WORKSPACE_ID }, + ]) + + await expect( + deleteTables(['tbl_accounts', 'tbl_contacts', 'tbl_notes'], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + ).resolves.toEqual({ + archived: [{ id: 'tbl_notes', name: 'Notes', workspaceId: WORKSPACE_ID }], + failed: [ + { + id: 'tbl_accounts', + name: 'Accounts', + reason: + 'Cannot delete table "Accounts" because the selected tables contain a reference cycle that cannot be restored safely. Remove a reference column first.', + }, + { + id: 'tbl_contacts', + name: 'Contacts', + reason: + 'Cannot delete table "Contacts" because the selected tables contain a reference cycle that cannot be restored safely. Remove a reference column first.', + }, + ], + notFound: [], + }) + + expect(dbChainMockFns.update).toHaveBeenCalledOnce() + }) + + it('allows a self-referencing table to archive', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_categories', 'Categories', ['tbl_categories']), + ]) + dbChainMockFns.returning.mockResolvedValueOnce([ + { id: 'tbl_categories', name: 'Categories', workspaceId: WORKSPACE_ID }, + ]) + + await expect( + deleteTables(['tbl_categories'], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + ).resolves.toEqual({ + archived: [{ id: 'tbl_categories', name: 'Categories', workspaceId: WORKSPACE_ID }], + failed: [], + notFound: [], + }) + }) + + it('archives a selected referrer before its target when request order is reversed', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_referrer', 'Referrer', ['tbl_target']), + batchTable('tbl_target', 'Target'), + ]) + const statementFailure = new Error('statement timeout') + dbChainMockFns.returning + .mockResolvedValueOnce([{ id: 'tbl_referrer', name: 'Referrer', workspaceId: WORKSPACE_ID }]) + .mockRejectedValueOnce(statementFailure) + + await expect( + deleteTables(['tbl_target', 'tbl_referrer'], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + ).resolves.toEqual({ + archived: [{ id: 'tbl_referrer', name: 'Referrer', workspaceId: WORKSPACE_ID }], + failed: [], + notFound: [], + terminalError: statementFailure, + }) + + expect( + hasMockCondition( + dbChainMockFns.where.mock.calls[1][0], + (node) => + node.type === 'eq' && + node.left === schemaMock.userTableDefinitions.id && + node.right === 'tbl_referrer' + ) + ).toBe(true) + expect( + hasMockCondition( + dbChainMockFns.where.mock.calls[2][0], + (node) => + node.type === 'eq' && + node.left === schemaMock.userTableDefinitions.id && + node.right === 'tbl_target' + ) + ).toBe(true) + }) + + it('stops before a target when its referrer archive returns no row', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_referrer', 'Referrer', ['tbl_target']), + batchTable('tbl_target', 'Target'), + ]) + dbChainMockFns.returning.mockResolvedValueOnce([]) + + const result = await deleteTables(['tbl_target', 'tbl_referrer'], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + + expect(result).toMatchObject({ + archived: [], + failed: [], + notFound: ['tbl_referrer'], + terminalError: { code: 'internal' }, + }) + expect(dbChainMockFns.update).toHaveBeenCalledOnce() + }) + + it('returns the committed prefix when an archive statement can roll back to its savepoint', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_customers', 'Customers'), + batchTable('tbl_orders', 'Orders'), + ]) + const statementFailure = new Error('statement timeout') + dbChainMockFns.returning + .mockResolvedValueOnce([ + { id: 'tbl_customers', name: 'Customers', workspaceId: WORKSPACE_ID }, + ]) + .mockRejectedValueOnce(statementFailure) + + await expect( + deleteTables(['tbl_customers', 'tbl_orders'], 'request-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + }) + ).resolves.toEqual({ + archived: [{ id: 'tbl_customers', name: 'Customers', workspaceId: WORKSPACE_ID }], + failed: [], + notFound: [], + terminalError: statementFailure, + }) + + expect(mocks.findActiveTableReferenceBlockers).toHaveBeenCalledOnce() + expect(dbChainMockFns.update).toHaveBeenCalledTimes(2) + }) + + it('rejects an oversized selection before opening a transaction', async () => { + await expect( + deleteTables( + Array.from({ length: MAX_TABLE_BATCH_ITEMS + 1 }, (_, index) => `table-${index}`), + 'request-1', + { expectedWorkspaceId: WORKSPACE_ID } + ) + ).rejects.toMatchObject({ code: 'validation' }) + + expect(dbChainMockFns.transaction).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index 2a1669b8453..1b4d10194a7 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -13,7 +13,18 @@ import { tableJobs, tableViews, userTableDefinitions, userTableRows } from '@sim import { createLogger } from '@sim/logger' import { getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { and, type Column, count, eq, isNotNull, isNull, type SQL, sql } from 'drizzle-orm' +import { + and, + asc, + type Column, + count, + eq, + inArray, + isNotNull, + isNull, + type SQL, + sql, +} from 'drizzle-orm' import type { V2TableSortBy } from '@/lib/api/contracts/v2/tables' import type { ListSortOrder } from '@/lib/api/list-query' import { @@ -36,10 +47,17 @@ import { resolveRestoredFolderId } from '@/lib/folders/queries' import { notifyWorkspaceTablesChanged } from '@/lib/realtime/notify' import { assertRowCapacity, notifyTableRowUsage } from '@/lib/table/billing' import { generateColumnId, getColumnId, withGeneratedColumnIds } from '@/lib/table/column-keys' -import { assertColumnReferencesInWorkspace } from '@/lib/table/column-types/registry.server' +import { + type ActiveTableReferenceBlocker, + assertColumnReferencesInWorkspace, + collectColumnReferencedTableIds, + findActiveTableReferenceBlockers, + tableReferenceBlockerMessage, +} from '@/lib/table/column-types/registry.server' import { COLUMN_TYPES, DEFAULT_TABLE_VIEW_NAME, + MAX_TABLE_BATCH_ITEMS, NAME_PATTERN, TABLE_LIMITS, } from '@/lib/table/constants' @@ -360,6 +378,25 @@ export async function listTables( return hydrateTableRows(tables) } +/** Lists active table IDs and names without materializing their schemas. */ +export async function listActiveTableNames( + workspaceId: string, + tableIds: readonly string[] +): Promise>> { + if (tableIds.length === 0) return [] + + return db + .select({ id: userTableDefinitions.id, name: userTableDefinitions.name }) + .from(userTableDefinitions) + .where( + and( + eq(userTableDefinitions.workspaceId, workspaceId), + inArray(userTableDefinitions.id, [...tableIds]), + isNull(userTableDefinitions.archivedAt) + ) + ) +} + /** Loads at most two active exact-name matches so callers can fail on corrupt ambiguity. */ export async function findActiveTablesByExactName( workspaceId: string, @@ -1155,32 +1192,11 @@ export async function deleteTable( options?: { archivedAt?: Date; skipNotify?: boolean; expectedWorkspaceId?: string } ): Promise<{ archived: { name: string; workspaceId: string | null } | null }> { const now = options?.archivedAt ?? new Date() - // Archiving destroys access to every row, so it is gated on the delete lock. - // The guard is inline in the WHERE (atomic — no separate read, no TOCTOU); - // a zero-row result is then disambiguated below (locked vs already-archived). - const result = await db - .update(userTableDefinitions) - .set({ archivedAt: now, updatedAt: now }) - .where( - and( - eq(userTableDefinitions.id, tableId), - options?.expectedWorkspaceId - ? eq(userTableDefinitions.workspaceId, options.expectedWorkspaceId) - : undefined, - isNull(userTableDefinitions.archivedAt), - eq(userTableDefinitions.deleteLocked, false) - ) - ) - .returning({ - createdBy: userTableDefinitions.createdBy, - workspaceId: userTableDefinitions.workspaceId, - name: userTableDefinitions.name, - }) - - const deleted = result[0] - if (!deleted) { - const [existing] = await db + const deleted = await db.transaction(async (trx) => { + await setTableTxTimeouts(trx) + const [existing] = await trx .select({ + name: userTableDefinitions.name, archivedAt: userTableDefinitions.archivedAt, deleteLocked: userTableDefinitions.deleteLocked, workspaceId: userTableDefinitions.workspaceId, @@ -1194,8 +1210,11 @@ export async function deleteTable( : undefined ) ) + .for('update') .limit(1) - if (existing && !existing.archivedAt && existing.deleteLocked) { + + if (!existing || existing.archivedAt) return null + if (existing.deleteLocked) { logger.warn('Table mutation blocked by lock', { tableId, workspaceId: existing.workspaceId, @@ -1203,8 +1222,39 @@ export async function deleteTable( }) throw new TableLockedError('delete') } - // Otherwise the table is missing or already archived — a silent no-op, as before. - } + + const blockers = existing.workspaceId + ? await findActiveTableReferenceBlockers(trx, existing.workspaceId, { + tableIds: [tableId], + }) + : [] + if (blockers.length > 0) { + throw new OrchestrationError( + 'conflict', + tableReferenceBlockerMessage( + existing.name, + blockers.map((blocker) => blocker.referencingTableName) + ) + ) + } + + const [archived] = await trx + .update(userTableDefinitions) + .set({ archivedAt: now, updatedAt: now }) + .where( + and( + eq(userTableDefinitions.id, tableId), + isNull(userTableDefinitions.archivedAt), + eq(userTableDefinitions.deleteLocked, false) + ) + ) + .returning({ + workspaceId: userTableDefinitions.workspaceId, + name: userTableDefinitions.name, + }) + return archived ?? null + }) + logger.info(`[${requestId}] Archived table ${tableId}`) // Live tables list: only on a genuine archive (a no-op/already-archived delete changes nothing). // Skipped under a folder cascade — deleteFolder fires one folder-level notify for the whole subtree, @@ -1217,6 +1267,240 @@ export async function deleteTable( return { archived: deleted ? { name: deleted.name, workspaceId: deleted.workspaceId } : null } } +export interface DeleteTablesResult { + archived: { id: string; name: string; workspaceId: string }[] + failed: { id: string; name: string; reason: string }[] + notFound: string[] + terminalError?: unknown +} + +interface ReferenceSafeArchivePlan { + ordered: T[] + blockedByCycle: T[] +} + +/** + * Orders selected tables so every referrer is archived before its target. + * + * Self-references are safe because the one row changes state atomically. Any + * rows left after the topological walk belong to a multi-table cycle or are + * targets reachable from one; none can be archived while a cycle member stays + * active. + */ +function planReferenceSafeArchiveOrder( + candidates: readonly T[], + externallyBlockedIds: ReadonlySet +): ReferenceSafeArchivePlan { + const deletable = candidates.filter((table) => !externallyBlockedIds.has(table.id)) + const deletableById = new Map(deletable.map((table) => [table.id, table])) + const targetsByReferrer = new Map() + const inboundReferenceCount = new Map(deletable.map((table) => [table.id, 0])) + + for (const table of deletable) { + const targets = collectColumnReferencedTableIds((table.schema as TableSchema).columns).filter( + (targetId) => targetId !== table.id && deletableById.has(targetId) + ) + targetsByReferrer.set(table.id, targets) + for (const targetId of targets) { + inboundReferenceCount.set(targetId, (inboundReferenceCount.get(targetId) ?? 0) + 1) + } + } + + const ready = deletable.filter((table) => inboundReferenceCount.get(table.id) === 0) + const ordered: T[] = [] + const orderedIds = new Set() + for (let index = 0; index < ready.length; index++) { + const table = ready[index] + ordered.push(table) + orderedIds.add(table.id) + for (const targetId of targetsByReferrer.get(table.id) ?? []) { + const remaining = (inboundReferenceCount.get(targetId) ?? 0) - 1 + inboundReferenceCount.set(targetId, remaining) + if (remaining === 0) { + const target = deletableById.get(targetId) + if (target) ready.push(target) + } + } + } + + return { + ordered, + blockedByCycle: deletable.filter((table) => !orderedIds.has(table.id)), + } +} + +function tableReferenceCycleMessage(tableName: string): string { + return `Cannot delete table "${tableName}" because the selected tables contain a reference cycle that cannot be restored safely. Remove a reference column first.` +} + +/** + * Archives a bounded table selection after one reference-graph scan. + * + * All explicit targets are locked before the scan. Reference-column writes take a conflicting + * key-share lock on their target, so the scan and every archive in this transaction observe one + * stable reference graph. Per-table savepoints preserve the bulk API's committed-prefix behavior + * for statement failures that can roll back to a savepoint. A connection loss or other failure + * that invalidates the outer transaction rolls back the entire table phase. + */ +export async function deleteTables( + tableIds: readonly string[], + requestId: string, + options: { + expectedWorkspaceId: string + archivedAt?: Date + skipNotify?: boolean + } +): Promise { + const requestedIds = [...new Set(tableIds)] + if (requestedIds.length > MAX_TABLE_BATCH_ITEMS) { + throw new OrchestrationError( + 'validation', + `Cannot delete more than ${MAX_TABLE_BATCH_ITEMS} tables at once` + ) + } + if (requestedIds.length === 0) { + return { archived: [], failed: [], notFound: [] } + } + const now = options.archivedAt ?? new Date() + const result = await db.transaction(async (trx): Promise => { + await setTableTxTimeouts(trx) + const advisoryLockIds = [...requestedIds].sort() + await trx.execute(sql` + SELECT pg_advisory_xact_lock(hashtextextended('user_table_schema:' || requested_id, 0)) + FROM unnest(${advisoryLockIds}::text[]) AS requested(requested_id) + ORDER BY requested_id + `) + const existing = await trx + .select({ + id: userTableDefinitions.id, + name: userTableDefinitions.name, + schema: userTableDefinitions.schema, + archivedAt: userTableDefinitions.archivedAt, + deleteLocked: userTableDefinitions.deleteLocked, + workspaceId: userTableDefinitions.workspaceId, + }) + .from(userTableDefinitions) + .where( + and( + eq(userTableDefinitions.workspaceId, options.expectedWorkspaceId), + inArray(userTableDefinitions.id, requestedIds) + ) + ) + .orderBy(asc(userTableDefinitions.id)) + .for('update') + + const existingById = new Map(existing.map((table) => [table.id, table])) + const notFound: string[] = [] + const failed: DeleteTablesResult['failed'] = [] + const candidates: typeof existing = [] + for (const tableId of requestedIds) { + const table = existingById.get(tableId) + if (!table || table.archivedAt) { + notFound.push(tableId) + continue + } + if (table.deleteLocked) { + failed.push({ + id: table.id, + name: table.name, + reason: new TableLockedError('delete').message, + }) + continue + } + candidates.push(table) + } + + const referenceBlockers = await findActiveTableReferenceBlockers( + trx, + options.expectedWorkspaceId, + { + tableIds: candidates.map((table) => table.id), + } + ) + const blockersByTargetId = new Map() + for (const blocker of referenceBlockers) { + const targetBlockers = blockersByTargetId.get(blocker.targetTableId) ?? [] + targetBlockers.push(blocker) + blockersByTargetId.set(blocker.targetTableId, targetBlockers) + } + + const externallyBlockedIds = new Set(blockersByTargetId.keys()) + const archivePlan = planReferenceSafeArchiveOrder(candidates, externallyBlockedIds) + const cycleBlockedIds = new Set(archivePlan.blockedByCycle.map((table) => table.id)) + for (const table of candidates) { + const blockers = blockersByTargetId.get(table.id) + if (blockers && blockers.length > 0) { + failed.push({ + id: table.id, + name: table.name, + reason: tableReferenceBlockerMessage( + table.name, + blockers.map((blocker) => blocker.referencingTableName) + ), + }) + continue + } + if (cycleBlockedIds.has(table.id)) { + failed.push({ + id: table.id, + name: table.name, + reason: tableReferenceCycleMessage(table.name), + }) + } + } + + const archived: DeleteTablesResult['archived'] = [] + let terminalError: unknown + for (const table of archivePlan.ordered) { + try { + const [updated] = await trx.transaction((savepoint) => + savepoint + .update(userTableDefinitions) + .set({ archivedAt: now, updatedAt: now }) + .where( + and( + eq(userTableDefinitions.id, table.id), + eq(userTableDefinitions.workspaceId, options.expectedWorkspaceId), + isNull(userTableDefinitions.archivedAt), + eq(userTableDefinitions.deleteLocked, false) + ) + ) + .returning({ + id: userTableDefinitions.id, + workspaceId: userTableDefinitions.workspaceId, + name: userTableDefinitions.name, + }) + ) + if (!updated) { + notFound.push(table.id) + terminalError = new OrchestrationError( + 'internal', + `Table "${table.id}" could not be archived after its deletion lock was acquired` + ) + break + } + archived.push(updated) + } catch (error) { + terminalError = error + break + } + } + + return { + archived, + failed, + notFound, + ...(terminalError !== undefined ? { terminalError } : {}), + } + }) + + logger.info(`[${requestId}] Archived ${result.archived.length} tables in batch`) + if (result.archived.length > 0 && !options.skipNotify) { + await notifyWorkspaceTablesChanged(options.expectedWorkspaceId) + } + return result +} + /** * Restores an archived table. * @@ -1229,7 +1513,11 @@ export async function deleteTable( export async function restoreTable( tableId: string, requestId: string, - options?: { restoringFolderIds?: ReadonlySet; skipNotify?: boolean } + options?: { + restoringFolderIds?: ReadonlySet + restoringTableIds?: ReadonlySet + skipNotify?: boolean + } ): Promise { const table = await getTableById(tableId, { includeArchived: true }) if (!table) { @@ -1274,7 +1562,25 @@ export async function restoreTable( try { await db.transaction(async (tx) => { await setTableTxTimeouts(tx) - await tx.execute(sql`SELECT 1 FROM user_table_definitions WHERE id = ${tableId} FOR UPDATE`) + const [currentTable] = await tx + .select({ + workspaceId: userTableDefinitions.workspaceId, + schema: userTableDefinitions.schema, + }) + .from(userTableDefinitions) + .where(eq(userTableDefinitions.id, tableId)) + .for('update') + .limit(1) + if (!currentTable) throw new OrchestrationError('not_found', 'Table not found') + + const allowedArchivedTableIds = new Set(options?.restoringTableIds) + allowedArchivedTableIds.add(tableId) + await assertColumnReferencesInWorkspace( + tx, + currentTable.workspaceId, + (currentTable.schema as TableSchema).columns, + { allowedArchivedTableIds } + ) attemptedRestoreName = await generateRestoreName(table.name, async (candidate) => { const [match] = await tx From 9509d93befb78b04ad1d6b7571bdc2acc56b9fdc Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Thu, 3 Sep 2026 15:48:03 -0700 Subject: [PATCH 2/6] fix(tables): resolve reference review findings --- .../[tableId]/hooks/use-table-event-stream.ts | 6 ++- .../tables/hooks/use-workspace-tables-room.ts | 7 ++- apps/sim/hooks/queries/folders.ts | 6 ++- apps/sim/hooks/queries/tables.test.ts | 18 ++++++- apps/sim/hooks/queries/tables.ts | 26 ++++++++++ apps/sim/hooks/queries/utils/table-keys.ts | 7 ++- apps/sim/lib/folders/cascade.test.ts | 43 +++++++++++++++++ apps/sim/lib/folders/cascade.ts | 4 +- apps/sim/lib/folders/config.ts | 22 +++++---- apps/sim/lib/table/service.test.ts | 29 +++++++++++ apps/sim/lib/table/service.ts | 48 ++++++++++++++++--- 11 files changed, 191 insertions(+), 25 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-event-stream.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-event-stream.ts index c36f87792e8..b9766ed9c3a 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-event-stream.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-event-stream.ts @@ -475,12 +475,16 @@ export function useTableEventStream({ // invalidateTableSchema set — the definition (exact, so rows stay on the // debounce), the run-state + enrichment sibling queries under detail (a group // delete/restructure can otherwise leave a stale running badge or enrichment - // panel), the tables list (column/row counts), and the debounced rows. + // panel), the tables list (column/row counts), open reference previews, and the + // debounced rows. else if (entry.event?.kind === 'schema') { void queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId), exact: true }) void queryClient.invalidateQueries({ queryKey: tableKeys.activeDispatches(tableId) }) void queryClient.invalidateQueries({ queryKey: tableKeys.enrichmentDetails(tableId) }) void queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + void queryClient.invalidateQueries({ + queryKey: tableKeys.referencePreviewsForTable(tableId), + }) scheduleRowsInvalidate() } // A collaborator changed the column layout (width/pin/order): refetch the diff --git a/apps/sim/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room.ts b/apps/sim/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room.ts index 3387ede2b5f..bf1cffac2c0 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room.ts @@ -8,8 +8,9 @@ import { tableKeys } from '@/hooks/queries/utils/table-keys' /** * Keeps the tables browser live: joins the workspace-tables room so a `workspace-tables-changed` - * broadcast (fanned out by the table + table-folder mutation services) invalidates the tables list - * AND the table folders so every viewer refetches without waiting for staleness. A created/renamed/ + * broadcast (fanned out by the table + table-folder mutation services) invalidates table lists, + * names, reference previews, AND table folders so every viewer refetches without waiting for + * staleness. A created/renamed/ * moved/deleted/restored table changes the list result (including folder placement); a folder * create/rename/delete/restore changes the folder tree — the page renders both, so both are * invalidated. Thin binding over {@link useWorkspaceInvalidationRoom}. @@ -18,6 +19,8 @@ export function useWorkspaceTablesRoom(workspaceId: string): void { const queryClient = useQueryClient() useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_TABLES, () => { queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() }) + queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviews() }) queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') }) }) } diff --git a/apps/sim/hooks/queries/folders.ts b/apps/sim/hooks/queries/folders.ts index 8afbef3ed52..1c6396a6c70 100644 --- a/apps/sim/hooks/queries/folders.ts +++ b/apps/sim/hooks/queries/folders.ts @@ -128,7 +128,11 @@ function invalidateCascadedResourceLists( case 'workflow': return invalidateWorkflowLists(queryClient, workspaceId, ['active', 'archived']) case 'table': - return queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + return Promise.all([ + queryClient.invalidateQueries({ queryKey: tableKeys.lists() }), + queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() }), + queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviews() }), + ]).then(() => undefined) case 'knowledge_base': return queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() }) /** diff --git a/apps/sim/hooks/queries/tables.test.ts b/apps/sim/hooks/queries/tables.test.ts index b6bf8c08b51..086db942d31 100644 --- a/apps/sim/hooks/queries/tables.test.ts +++ b/apps/sim/hooks/queries/tables.test.ts @@ -127,6 +127,8 @@ describe('useTableNames', () => { enabled: true, queryKey: tableKeys.names(WORKSPACE_ID, [TABLE_ID, 'tbl-2']), }) + expect(options.queryKey.slice(0, tableKeys.namesRoot().length)).toEqual(tableKeys.namesRoot()) + expect(options.queryKey.slice(0, tableKeys.lists().length)).not.toEqual(tableKeys.lists()) expect(requestJson).toHaveBeenCalledWith(listTableNamesContract, { body: { workspaceId: WORKSPACE_ID, tableIds: [TABLE_ID, 'tbl-2'] }, signal, @@ -580,7 +582,7 @@ describe('useDeleteColumn optimistic update', () => { expect(getCache(ROWS_KEY)).toEqual(originalRows) }) - it('invalidates schema, rows, and lists in onSettled', () => { + it('invalidates schema, rows, lists, and mounted reference previews in onSettled', () => { const hook = useDeleteColumn({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID }) hook.onSettled?.(undefined, null, 'age', undefined) @@ -590,6 +592,7 @@ describe('useDeleteColumn optimistic update', () => { tableKeys.detail(TABLE_ID), tableKeys.rowsRoot(TABLE_ID), tableKeys.lists(), + tableKeys.referencePreviewsForTable(TABLE_ID), ]) ) }) @@ -648,6 +651,15 @@ describe('useUpdateColumn optimistic update', () => { ) expect(detail?.schema.columns[0]).toMatchObject({ id: 'age', name: 'years' }) }) + + it('invalidates mounted previews when the referenced table schema changes', () => { + const hook = useUpdateColumn({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID }) + hook.onSettled?.(undefined, null, { columnName: 'age', updates: { name: 'years' } }, undefined) + + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ + queryKey: tableKeys.referencePreviewsForTable(TABLE_ID), + }) + }) }) describe('useRestoreTable cache invalidation', () => { @@ -674,7 +686,7 @@ describe('useRestoreTable cache invalidation', () => { }) }) - it('invalidates lists, table detail, and row data for the restored table', () => { + it('invalidates names, previews, lists, table detail, and row data for the restored table', () => { const hook = useRestoreTable() hook.onSettled?.(undefined, null, TABLE_ID, undefined) @@ -682,8 +694,10 @@ describe('useRestoreTable cache invalidation', () => { expect(calls).toEqual( expect.arrayContaining([ tableKeys.lists(), + tableKeys.namesRoot(), tableKeys.detail(TABLE_ID), tableKeys.rowsRoot(TABLE_ID), + tableKeys.referencePreviewsForTable(TABLE_ID), ]) ) }) diff --git a/apps/sim/hooks/queries/tables.ts b/apps/sim/hooks/queries/tables.ts index 48ca9183559..323f7cfd190 100644 --- a/apps/sim/hooks/queries/tables.ts +++ b/apps/sim/hooks/queries/tables.ts @@ -280,6 +280,17 @@ function invalidateReferencePreviews( }) } +function invalidateTableNames(queryClient: ReturnType) { + queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() }) +} + +function invalidateReferenceTablePreviews( + queryClient: ReturnType, + tableId: string +) { + queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviewsForTable(tableId) }) +} + /** * Invalidate only the row-count surfaces — the table detail and the tables * list, both of which carry the unfiltered `rowCount`. Deliberately leaves @@ -304,6 +315,7 @@ function invalidateTableSchema(queryClient: ReturnType, t queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) }) queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) }) queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + invalidateReferenceTablePreviews(queryClient, tableId) } /** @@ -321,6 +333,7 @@ function invalidateTableSchemaOnly( ) { queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) }) queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + invalidateReferenceTablePreviews(queryClient, tableId) } /** @@ -730,6 +743,7 @@ export function useCreateTable(workspaceId: string) { }, onSettled: () => { queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + invalidateTableNames(queryClient) }, }) } @@ -779,6 +793,8 @@ export function useRenameTable(workspaceId: string) { onSettled: (_data, _error, variables) => { queryClient.invalidateQueries({ queryKey: tableKeys.detail(variables.tableId) }) queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + invalidateTableNames(queryClient) + invalidateReferenceTablePreviews(queryClient, variables.tableId) }, }) } @@ -889,6 +905,8 @@ export function useDeleteTable(workspaceId: string) { }, onSettled: (_data, _error, tableId) => { queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + invalidateTableNames(queryClient) + invalidateReferenceTablePreviews(queryClient, tableId) queryClient.removeQueries({ queryKey: tableKeys.detail(tableId) }) queryClient.removeQueries({ queryKey: tableKeys.rowsRoot(tableId) }) }, @@ -1954,8 +1972,12 @@ export function useRestoreTable() { onSettled: (_data, _error, tableId) => { return Promise.all([ queryClient.invalidateQueries({ queryKey: tableKeys.lists() }), + queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() }), queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) }), queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) }), + queryClient.invalidateQueries({ + queryKey: tableKeys.referencePreviewsForTable(tableId), + }), ]) }, }) @@ -2073,6 +2095,7 @@ export function useImportCsv() { }, onSettled: () => { queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + invalidateTableNames(queryClient) }, }) } @@ -2117,6 +2140,7 @@ export function useImportFileAsTable() { }, onSettled: () => { queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + invalidateTableNames(queryClient) }, }) } @@ -2784,6 +2808,8 @@ export function useBulkDeleteTables(workspaceId: string) { }, onSettled: (_data, _error, { tableIds = [] }) => { queryClient.invalidateQueries({ queryKey: tableKeys.lists() }) + invalidateTableNames(queryClient) + queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviews() }) queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') }) for (const tableId of tableIds) { queryClient.removeQueries({ queryKey: tableKeys.detail(tableId) }) diff --git a/apps/sim/hooks/queries/utils/table-keys.ts b/apps/sim/hooks/queries/utils/table-keys.ts index 46805598c2f..e6ab9635dc7 100644 --- a/apps/sim/hooks/queries/utils/table-keys.ts +++ b/apps/sim/hooks/queries/utils/table-keys.ts @@ -20,16 +20,19 @@ export const tableKeys = { lists: () => [...tableKeys.all, 'list'] as const, list: (workspaceId?: string, scope: TableQueryScope = 'active') => [...tableKeys.lists(), workspaceId ?? '', scope] as const, + namesRoot: () => [...tableKeys.all, 'names'] as const, names: (workspaceId: string | undefined, tableIds: readonly string[]) => - [...tableKeys.lists(), 'names', workspaceId ?? '', tableIds] as const, + [...tableKeys.namesRoot(), workspaceId ?? '', tableIds] as const, details: () => [...tableKeys.all, 'detail'] as const, detail: (tableId: string) => [...tableKeys.details(), tableId] as const, exportJobs: (workspaceId?: string) => [...tableKeys.all, 'export-jobs', workspaceId ?? ''] as const, rowsRoot: (tableId: string) => [...tableKeys.detail(tableId), 'rows'] as const, referencePreviews: () => [...tableKeys.all, 'reference-preview'] as const, + referencePreviewsForTable: (tableId: string) => + [...tableKeys.referencePreviews(), tableId] as const, referencePreview: (tableId: string, rowId: string, sourceRowId = '', sourceColumnKey = '') => - [...tableKeys.referencePreviews(), tableId, rowId, sourceRowId, sourceColumnKey] as const, + [...tableKeys.referencePreviewsForTable(tableId), rowId, sourceRowId, sourceColumnKey] as const, /** * Prefix covering only the paged row lists. `rowsRoot` is a shared parent — `find` * hangs off it holding a different shape — so anything walking the cache for row diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts index 7fd73951b7a..948a3e5896e 100644 --- a/apps/sim/lib/folders/cascade.test.ts +++ b/apps/sim/lib/folders/cascade.test.ts @@ -23,6 +23,14 @@ import { FolderCollectionLimitExceededError } from '@/lib/folders/errors' import { folderResourceSupportsLocking } from '@/lib/folders/resource-traits' import { folderMutationStatus } from '@/lib/folders/status' +const tableServiceMocks = vi.hoisted(() => ({ + deleteTables: vi.fn(), +})) + +vi.mock('@/lib/table/service', () => ({ + deleteTables: tableServiceMocks.deleteTables, +})) + interface SelectCall { where: unknown } @@ -545,6 +553,41 @@ describe('knowledge_base and table folder resources', () => { expect(knowledgeConfig.sortOrderColumn).toBeUndefined() expect(tableConfig.sortOrderColumn).toBeUndefined() }) + + it('archives table children as one restorable cohort', async () => { + resetDbChainMock() + queueTableRows(schemaMock.userTableDefinitions, [ + { id: 'tbl_accounts' }, + { id: 'tbl_contacts' }, + ]) + tableServiceMocks.deleteTables.mockResolvedValueOnce({ + archived: [ + { id: 'tbl_accounts', name: 'Accounts', workspaceId: 'ws-1' }, + { id: 'tbl_contacts', name: 'Contacts', workspaceId: 'ws-1' }, + ], + failed: [], + notFound: [], + }) + + await expect( + tableConfig.archiveChildren?.({ + workspaceId: 'ws-1', + folderIds: ['folder-1'], + timestamp: TIMESTAMP, + }) + ).resolves.toBe(2) + + expect(tableServiceMocks.deleteTables).toHaveBeenCalledWith( + ['tbl_accounts', 'tbl_contacts'], + 'folder-cascade-folder-1', + { + expectedWorkspaceId: 'ws-1', + archivedAt: TIMESTAMP, + skipNotify: true, + archiveAsCohort: true, + } + ) + }) }) describe('table folder deletion guard', () => { diff --git a/apps/sim/lib/folders/cascade.ts b/apps/sim/lib/folders/cascade.ts index 08db6cab1b3..37299516edd 100644 --- a/apps/sim/lib/folders/cascade.ts +++ b/apps/sim/lib/folders/cascade.ts @@ -99,8 +99,8 @@ function childFilter(config: FolderResourceConfig, workspaceId: string, folderId * also reviving siblings that were deleted independently. * * Folders are stamped BEFORE their children, which is what makes a failed cascade - * recoverable. `archiveChildren` hooks walk resources one at a time through their canonical - * delete, so a mid-loop failure can leave some children archived and some not. With the + * recoverable. `archiveChildren` hooks may walk resources through their canonical deletes, so + * a mid-cascade failure can leave some children archived and some not. With the * folder already stamped, `deleteFolder` reuses that same `deletedAt` on the retry and the * stragglers join the original snapshot. Stamping children first would leave the folder * active, so a retry would mint a fresh timestamp and the partially-archived children could diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts index 1ed4a4ff349..99efe0f03ec 100644 --- a/apps/sim/lib/folders/config.ts +++ b/apps/sim/lib/folders/config.ts @@ -11,6 +11,7 @@ import { import { eq, type SQL } from 'drizzle-orm' import type { PgColumn, PgTable } from 'drizzle-orm/pg-core' import type { FolderResourceType } from '@/lib/api/contracts/folders' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { FOLDER_RESOURCE_LABELS, FOLDER_RESOURCE_SUPPORTS_LOCKING, @@ -310,18 +311,21 @@ async function restoreKnowledgeBaseChildren(context: CascadeChildrenContext): Pr * encounter a partial cascade. */ async function archiveTableChildren(context: CascadeChildrenContext): Promise { - const { deleteTable } = await import('@/lib/table/service') + const { deleteTables } = await import('@/lib/table/service') const ids = await selectChildIds(FOLDER_RESOURCES.table, context, 'active') - - for (const id of ids) { - await deleteTable(id, `folder-cascade-${context.folderIds[0]}`, { - archivedAt: context.timestamp, - // deleteFolder fires one folder-level live-list notify for the whole subtree. - skipNotify: true, - }) + const result = await deleteTables(ids, `folder-cascade-${context.folderIds[0]}`, { + expectedWorkspaceId: context.workspaceId, + archivedAt: context.timestamp, + // deleteFolder fires one folder-level live-list notify for the whole subtree. + skipNotify: true, + archiveAsCohort: true, + }) + if (result.terminalError) throw result.terminalError + if (result.failed[0]) { + throw new OrchestrationError('conflict', result.failed[0].reason) } - return ids.length + return result.archived.length } /** diff --git a/apps/sim/lib/table/service.test.ts b/apps/sim/lib/table/service.test.ts index be4d8e349e7..88feb38f021 100644 --- a/apps/sim/lib/table/service.test.ts +++ b/apps/sim/lib/table/service.test.ts @@ -746,6 +746,35 @@ describe('deleteTables reference guard', () => { expect(dbChainMockFns.update).toHaveBeenCalledOnce() }) + it('archives a reference cycle atomically when it will be restored as one cohort', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + batchTable('tbl_accounts', 'Accounts', ['tbl_contacts']), + batchTable('tbl_contacts', 'Contacts', ['tbl_accounts']), + ]) + dbChainMockFns.returning.mockResolvedValueOnce([ + { id: 'tbl_accounts', name: 'Accounts', workspaceId: WORKSPACE_ID }, + { id: 'tbl_contacts', name: 'Contacts', workspaceId: WORKSPACE_ID }, + ]) + + await expect( + deleteTables(['tbl_accounts', 'tbl_contacts'], 'folder-cascade-folder-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + archiveAsCohort: true, + }) + ).resolves.toEqual({ + archived: [ + { id: 'tbl_accounts', name: 'Accounts', workspaceId: WORKSPACE_ID }, + { id: 'tbl_contacts', name: 'Contacts', workspaceId: WORKSPACE_ID }, + ], + failed: [], + notFound: [], + }) + + expect(dbChainMockFns.update).toHaveBeenCalledOnce() + expect(dbChainMockFns.transaction).toHaveBeenCalledOnce() + }) + it('allows a self-referencing table to archive', async () => { queueTableRows(schemaMock.userTableDefinitions, [ batchTable('tbl_categories', 'Categories', ['tbl_categories']), diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index 1b4d10194a7..e55cdf1c96a 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -1334,13 +1334,14 @@ function tableReferenceCycleMessage(tableName: string): string { } /** - * Archives a bounded table selection after one reference-graph scan. + * Archives a table selection after one reference-graph scan. * * All explicit targets are locked before the scan. Reference-column writes take a conflicting * key-share lock on their target, so the scan and every archive in this transaction observe one - * stable reference graph. Per-table savepoints preserve the bulk API's committed-prefix behavior - * for statement failures that can roll back to a savepoint. A connection loss or other failure - * that invalidates the outer transaction rolls back the entire table phase. + * stable reference graph. Folder cascades archive one restorable cohort atomically; explicit + * selections use per-table savepoints to preserve the bulk API's committed-prefix behavior for + * statement failures that can roll back to a savepoint. A connection loss or other failure that + * invalidates the outer transaction rolls back the entire table phase. */ export async function deleteTables( tableIds: readonly string[], @@ -1349,10 +1350,15 @@ export async function deleteTables( expectedWorkspaceId: string archivedAt?: Date skipNotify?: boolean + /** + * Archives every eligible table in one statement and allows reference cycles. Use only when + * the caller will restore the same tables as one cohort, such as a table-folder cascade. + */ + archiveAsCohort?: boolean } ): Promise { const requestedIds = [...new Set(tableIds)] - if (requestedIds.length > MAX_TABLE_BATCH_ITEMS) { + if (!options.archiveAsCohort && requestedIds.length > MAX_TABLE_BATCH_ITEMS) { throw new OrchestrationError( 'validation', `Cannot delete more than ${MAX_TABLE_BATCH_ITEMS} tables at once` @@ -1426,7 +1432,9 @@ export async function deleteTables( const externallyBlockedIds = new Set(blockersByTargetId.keys()) const archivePlan = planReferenceSafeArchiveOrder(candidates, externallyBlockedIds) - const cycleBlockedIds = new Set(archivePlan.blockedByCycle.map((table) => table.id)) + const cycleBlockedIds = new Set( + options.archiveAsCohort ? [] : archivePlan.blockedByCycle.map((table) => table.id) + ) for (const table of candidates) { const blockers = blockersByTargetId.get(table.id) if (blockers && blockers.length > 0) { @@ -1450,6 +1458,34 @@ export async function deleteTables( } const archived: DeleteTablesResult['archived'] = [] + if (options.archiveAsCohort) { + const cohort = [...archivePlan.ordered, ...archivePlan.blockedByCycle] + if (cohort.length > 0) { + archived.push( + ...(await trx + .update(userTableDefinitions) + .set({ archivedAt: now, updatedAt: now }) + .where( + and( + inArray( + userTableDefinitions.id, + cohort.map((table) => table.id) + ), + eq(userTableDefinitions.workspaceId, options.expectedWorkspaceId), + isNull(userTableDefinitions.archivedAt), + eq(userTableDefinitions.deleteLocked, false) + ) + ) + .returning({ + id: userTableDefinitions.id, + workspaceId: userTableDefinitions.workspaceId, + name: userTableDefinitions.name, + })) + ) + } + return { archived, failed, notFound } + } + let terminalError: unknown for (const table of archivePlan.ordered) { try { From f4c8a117bd7525b9c42b6aed1048e6450ae10a3a Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:06:56 -0700 Subject: [PATCH 3/6] fix(tables): harden reference deletion coordination --- .../[workspaceId]/tables/[tableId]/table.tsx | 2 + apps/sim/lib/folders/bulk.ts | 8 ++- apps/sim/lib/folders/cascade.test.ts | 40 +++++++++++++ apps/sim/lib/folders/config.ts | 11 +++- apps/sim/lib/folders/orchestration.test.ts | 30 ++++++++++ apps/sim/lib/folders/orchestration.ts | 25 ++++++-- apps/sim/lib/table/application/bulk.test.ts | 57 +++++++++++++++++++ apps/sim/lib/table/application/bulk.ts | 41 +++++++++++++ apps/sim/lib/table/service.test.ts | 41 +++++++++++++ apps/sim/lib/table/service.ts | 14 ++++- 10 files changed, 262 insertions(+), 7 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx index 3f90d87d679..11301274339 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx @@ -51,6 +51,7 @@ import { } from '@/app/workspace/[workspaceId]/tables/[tableId]/view-state' import { ImportCsvDialog } from '@/app/workspace/[workspaceId]/tables/components/import-csv-dialog' import { ImportProgressMenu } from '@/app/workspace/[workspaceId]/tables/components/import-progress-menu' +import { useWorkspaceTablesRoom } from '@/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room' import { useLogByExecutionId } from '@/hooks/queries/logs' import { downloadExportResult, @@ -197,6 +198,7 @@ export function Table({ const tableId = propTableId || (params.tableId as string) const hostContext = useOptionalWorkspaceHostContext() const referenceColumnsEnabled = hostContext?.features?.referenceColumns ?? false + useWorkspaceTablesRoom(workspaceId) const posthog = usePostHog() const tableRowTtlEnabled = useFeatureFlag('table-row-ttl') diff --git a/apps/sim/lib/folders/bulk.ts b/apps/sim/lib/folders/bulk.ts index 3e09829cb5f..299d4ebfe45 100644 --- a/apps/sim/lib/folders/bulk.ts +++ b/apps/sim/lib/folders/bulk.ts @@ -251,6 +251,8 @@ export async function bulkDeleteFolders(params: { userId: string folders: readonly BulkFolderAffected[] countKey: 'tables' | 'knowledgeBases' + /** Reuse a reference scan completed for this batch while retaining each folder's lock guard. */ + referenceCheckCompleted?: boolean }): Promise { const succeeded: BulkFolderAffected[] = [] const failed: BulkFolderFailure[] = [] @@ -267,7 +269,11 @@ export async function bulkDeleteFolders(params: { userId: params.userId, folderName: folder.name, }, - { projectAudit: false, notify: false } + { + projectAudit: false, + notify: false, + referenceCheckCompleted: params.referenceCheckCompleted, + } ) if (result.success) { succeeded.push(folder) diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts index 948a3e5896e..0ccc436d77a 100644 --- a/apps/sim/lib/folders/cascade.test.ts +++ b/apps/sim/lib/folders/cascade.test.ts @@ -2,6 +2,7 @@ * @vitest-environment node */ import { + dbChainMockFns, flattenMockConditions, hasMockCondition, queueTableRows, @@ -588,6 +589,31 @@ describe('knowledge_base and table folder resources', () => { } ) }) + + it('preserves a concurrent table lock as a locked folder failure', async () => { + resetDbChainMock() + queueTableRows(schemaMock.userTableDefinitions, [{ id: 'tbl_accounts' }]) + tableServiceMocks.deleteTables.mockResolvedValueOnce({ + archived: [], + failed: [ + { + id: 'tbl_accounts', + name: 'Accounts', + reason: 'Table deletion is locked', + code: 'locked', + }, + ], + notFound: [], + }) + + await expect( + tableConfig.archiveChildren?.({ + workspaceId: 'ws-1', + folderIds: ['folder-1'], + timestamp: TIMESTAMP, + }) + ).rejects.toMatchObject({ code: 'locked', message: 'Table deletion is locked' }) + }) }) describe('table folder deletion guard', () => { @@ -633,4 +659,18 @@ describe('table folder deletion guard', () => { errorCode: 'conflict', }) }) + + it('reuses a completed bulk reference check while still checking table locks', async () => { + queueTableRows(schemaMock.userTableDefinitions, []) + + await expect( + FOLDER_RESOURCES.table.guardDelete?.({ + workspaceId: 'ws-1', + folderIds: ['folder-root'], + referenceCheckCompleted: true, + }) + ).resolves.toBeNull() + + expect(dbChainMockFns.select).toHaveBeenCalledOnce() + }) }) diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts index 99efe0f03ec..8e1e8d9918a 100644 --- a/apps/sim/lib/folders/config.ts +++ b/apps/sim/lib/folders/config.ts @@ -147,6 +147,8 @@ export interface FolderResourceConfig { guardDelete?: (context: { workspaceId: string folderIds: string[] + /** The caller already checked active references for this exact folder selection. */ + referenceCheckCompleted?: boolean }) => Promise } @@ -322,7 +324,10 @@ async function archiveTableChildren(context: CascadeChildrenContext): Promise { const [ { db }, @@ -395,6 +402,8 @@ async function guardTableDeletion({ } } + if (referenceCheckCompleted) return null + const [blocker] = await findActiveTableReferenceBlockers(db, workspaceId, { folderIds: new Set(folderIds), }) diff --git a/apps/sim/lib/folders/orchestration.test.ts b/apps/sim/lib/folders/orchestration.test.ts index 204f26a34c1..ecaafa74f2d 100644 --- a/apps/sim/lib/folders/orchestration.test.ts +++ b/apps/sim/lib/folders/orchestration.test.ts @@ -11,6 +11,7 @@ import { schemaMock, } from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { OrchestrationError } from '@/lib/core/orchestration/types' import { FolderCollectionFullError, FolderCollectionLimitExceededError } from '@/lib/folders/errors' import { folderMutationStatus } from '@/lib/folders/status' @@ -770,6 +771,21 @@ describe('deleteFolder', () => { expect(mockArchiveFolderCascade).not.toHaveBeenCalled() }) + it('returns a classified failure when a table lock appears after the guard', async () => { + queueTableRows(schemaMock.folder, [{ deletedAt: null }]) + mockArchiveFolderCascade.mockRejectedValueOnce( + new OrchestrationError('locked', 'Table deletion is locked') + ) + + const result = await deleteFolder(baseDelete) + + expect(result).toEqual({ + success: false, + error: 'Table deletion is locked', + errorCode: 'locked', + }) + }) + it('hands the guard the whole resolved subtree, not just the root', async () => { setConfig({ guardDelete: mockGuardDelete }) mockGuardDelete.mockResolvedValueOnce(null) @@ -784,6 +800,20 @@ describe('deleteFolder', () => { }) }) + it('passes a completed reference preflight through to the table guard', async () => { + setConfig({ guardDelete: mockGuardDelete }) + mockGuardDelete.mockResolvedValueOnce(null) + queueTableRows(schemaMock.folder, [{ deletedAt: null }]) + + await deleteFolder(baseDelete, { referenceCheckCompleted: true }) + + expect(mockGuardDelete).toHaveBeenCalledWith({ + workspaceId: 'ws-1', + folderIds: ['folder-1'], + referenceCheckCompleted: true, + }) + }) + it('reuses an already-archived folder’s timestamp so a retry rejoins the same snapshot', async () => { // A fresh stamp would be unrecoverable: the folder row keeps its original deletedAt, so // anything archived under the new stamp would never match on restore. diff --git a/apps/sim/lib/folders/orchestration.ts b/apps/sim/lib/folders/orchestration.ts index 345f262c3ea..17136ef5015 100644 --- a/apps/sim/lib/folders/orchestration.ts +++ b/apps/sim/lib/folders/orchestration.ts @@ -792,8 +792,11 @@ export async function deleteFolder( * * `notify: false` for a caller deleting several folders in one gesture that * sends a single batch notification of its own — see {@link bulkDeleteFolders}. + * `referenceCheckCompleted: true` reuses that batch's reference scan; the + * table guard still checks deletion locks, and the table service rechecks + * references under its own transaction locks before archiving the cohort. */ - options?: { projectAudit?: boolean; notify?: boolean } + options?: { projectAudit?: boolean; notify?: boolean; referenceCheckCompleted?: boolean } ): Promise { const existing = await withFolderTreeLock(params.workspaceId, params.resourceType, async (tx) => { const [row] = await tx @@ -817,13 +820,14 @@ export async function deleteFolder( return deleteFolderWithoutTreeLock(params, existing.deletedAt, { projectAudit: options?.projectAudit ?? true, notify: options?.notify ?? true, + referenceCheckCompleted: options?.referenceCheckCompleted, }) } async function deleteFolderWithoutTreeLock( params: DeleteFolderParams, deletedAt: Date | null, - options: { projectAudit: boolean; notify: boolean } + options: { projectAudit: boolean; notify: boolean; referenceCheckCompleted?: boolean } ): Promise { const { resourceType, folderId, workspaceId, userId, folderName, folderPath } = params const config = folderResourceConfig(resourceType) @@ -844,12 +848,25 @@ async function deleteFolderWithoutTreeLock( params.maxFolderRows ) - const rejection = await config.guardDelete?.({ workspaceId, folderIds }) + const rejection = await config.guardDelete?.({ + workspaceId, + folderIds, + ...(options.referenceCheckCompleted ? { referenceCheckCompleted: true } : {}), + }) if (rejection) { return { success: false, error: rejection.error, errorCode: rejection.errorCode } } - const counts = await archiveFolderCascade(db, config, workspaceId, folderIds, timestamp) + let counts: Awaited> + try { + counts = await archiveFolderCascade(db, config, workspaceId, folderIds, timestamp) + } catch (error) { + const classified = asOrchestrationError(error) + if (classified?.code === 'locked' || classified?.code === 'conflict') { + return { success: false, error: classified.message, errorCode: classified.code } + } + throw error + } logger.info('Deleted folder and all contents', { folderId, resourceType, counts }) diff --git a/apps/sim/lib/table/application/bulk.test.ts b/apps/sim/lib/table/application/bulk.test.ts index 539cdc54fab..f909e2e9cef 100644 --- a/apps/sim/lib/table/application/bulk.test.ts +++ b/apps/sim/lib/table/application/bulk.test.ts @@ -221,6 +221,63 @@ describe('table bulk application use cases', () => { ) }) + it('retries an explicit target after its selected-folder referrer is archived', async () => { + mocks.planFolderSelection.mockResolvedValue({ + selected: [{ id: 'folder-1', name: 'Reports' }], + notFound: [], + contained: [], + covered: new Set(['folder-1']), + coveredBySelected: new Map([['folder-1', new Set(['folder-1'])]]), + }) + mocks.deleteTables + .mockResolvedValueOnce({ + archived: [], + failed: [ + { + id: 'table-target', + name: 'Target', + reason: 'Target is referenced by Referrer', + code: 'reference', + }, + ], + notFound: [], + }) + .mockResolvedValueOnce(successfulTableDeleteBatch(['table-target'])) + mocks.bulkDeleteFolders.mockResolvedValue({ + succeeded: [{ id: 'folder-1', name: 'Reports' }], + failed: [], + folderCount: 1, + resourceCount: 1, + }) + + const result = await bulkDeleteTables.execute({ + principal, + input: { + assertedWorkspaceId: 'workspace-1', + tableIds: ['table-target'], + folderKeying: 'ids' as const, + folders: ['folder-1'], + }, + }) + + expect(result.deleted).toEqual([ + { kind: 'folder', id: 'folder-1', name: 'Reports' }, + { kind: 'table', id: 'table-target', name: 'Archived' }, + ]) + expect(result.failed).toEqual([]) + expect(result.deletedItems).toEqual({ tables: 2, folders: 1 }) + expect(mocks.deleteTables).toHaveBeenCalledTimes(2) + expect(mocks.deleteTables).toHaveBeenLastCalledWith( + ['table-target'], + 'request-1', + expect.anything() + ) + expect(mocks.findReferenceBlockers).toHaveBeenCalledOnce() + expect(mocks.bulkDeleteFolders).toHaveBeenCalledWith( + expect.objectContaining({ referenceCheckCompleted: true }) + ) + }) + /** * The whole point of taking both id lists in one request: a table that is * also inside a selected folder must be archived exactly once, under the diff --git a/apps/sim/lib/table/application/bulk.ts b/apps/sim/lib/table/application/bulk.ts index 594f2b366ce..b4e05050a23 100644 --- a/apps/sim/lib/table/application/bulk.ts +++ b/apps/sim/lib/table/application/bulk.ts @@ -529,7 +529,9 @@ export const bulkDeleteTables = defineAuthorizedTableUseCase({ for (const table of tableResult.archived) { deleted.push({ kind: 'table', id: table.id, name: table.name }) } + const referenceFailures = tableResult.failed.filter((table) => table.code === 'reference') for (const table of tableResult.failed) { + if (table.code === 'reference') continue outcome.failed.push({ kind: 'table', id: table.id, name: table.name, reason: table.reason }) } for (const tableId of tableResult.notFound) { @@ -567,6 +569,7 @@ export const bulkDeleteTables = defineAuthorizedTableUseCase({ }) ) const deletedItems = { tables: deleted.length, folders: 0 } + let deletedFolder = false if (terminalError === undefined && plan.selected.length > 0) { const deletableFolders = plan.selected.filter((folder) => { const blocker = blockedFolderIds.get(folder.id) @@ -589,11 +592,49 @@ export const bulkDeleteTables = defineAuthorizedTableUseCase({ }).attributedUserId, folders: deletableFolders, countKey: 'tables', + referenceCheckCompleted: true, }) for (const folder of folders.succeeded) deleted.push({ kind: 'folder', ...folder }) for (const folder of folders.failed) outcome.failed.push({ kind: 'folder', ...folder }) deletedItems.folders = folders.folderCount deletedItems.tables += folders.resourceCount + deletedFolder = folders.succeeded.length > 0 + } + } + + if (terminalError === undefined && deletedFolder && referenceFailures.length > 0) { + const retryResult = await deleteTables( + referenceFailures.map((table) => table.id), + generateRequestId(), + { + expectedWorkspaceId: context.workspaceId, + skipNotify: true, + } + ) + for (const table of retryResult.archived) { + deleted.push({ kind: 'table', id: table.id, name: table.name }) + deletedItems.tables += 1 + } + for (const table of retryResult.failed) { + outcome.failed.push({ + kind: 'table', + id: table.id, + name: table.name, + reason: table.reason, + }) + } + for (const tableId of retryResult.notFound) { + outcome.notFound.push({ kind: 'table', id: tableId }) + } + terminalError = retryResult.terminalError + } else { + for (const table of referenceFailures) { + outcome.failed.push({ + kind: 'table', + id: table.id, + name: table.name, + reason: table.reason, + }) } } diff --git a/apps/sim/lib/table/service.test.ts b/apps/sim/lib/table/service.test.ts index 88feb38f021..eb8eddd6b4c 100644 --- a/apps/sim/lib/table/service.test.ts +++ b/apps/sim/lib/table/service.test.ts @@ -11,6 +11,7 @@ import { import { beforeEach, describe, expect, it, vi } from 'vitest' import type { DbOrTx } from '@/lib/db/types' import { MAX_TABLE_BATCH_ITEMS } from '@/lib/table/constants' +import { TableLockedError } from '@/lib/table/mutation-locks' import type { TableSchema } from '@/lib/table/types' const mocks = vi.hoisted(() => ({ @@ -472,6 +473,15 @@ describe('restoreTable reference validation', () => { referenceSchema.columns, { allowedArchivedTableIds: new Set(['tbl_accounts', TABLE_ID]) } ) + const advisoryLockCallIndex = dbChainMockFns.execute.mock.calls.findIndex(([query]) => + ((query as { strings?: readonly string[] }).strings ?? []).some((part) => + part.includes('pg_advisory_xact_lock') + ) + ) + expect(advisoryLockCallIndex).toBeGreaterThanOrEqual(0) + expect(dbChainMockFns.execute.mock.invocationCallOrder[advisoryLockCallIndex]).toBeLessThan( + dbChainMockFns.select.mock.invocationCallOrder[1] + ) expect(dbChainMockFns.update).toHaveBeenCalledWith(schemaMock.userTableDefinitions) }) @@ -700,6 +710,7 @@ describe('deleteTables reference guard', () => { name: 'Customers', reason: 'Cannot delete table "Customers" because it is referenced by table "Invoices". Remove the reference column first.', + code: 'reference', }, ], notFound: [], @@ -732,12 +743,14 @@ describe('deleteTables reference guard', () => { name: 'Accounts', reason: 'Cannot delete table "Accounts" because the selected tables contain a reference cycle that cannot be restored safely. Remove a reference column first.', + code: 'reference_cycle', }, { id: 'tbl_contacts', name: 'Contacts', reason: 'Cannot delete table "Contacts" because the selected tables contain a reference cycle that cannot be restored safely. Remove a reference column first.', + code: 'reference_cycle', }, ], notFound: [], @@ -775,6 +788,34 @@ describe('deleteTables reference guard', () => { expect(dbChainMockFns.transaction).toHaveBeenCalledOnce() }) + it('archives none of a restore cohort when one table becomes delete-locked', async () => { + queueTableRows(schemaMock.userTableDefinitions, [ + { ...batchTable('tbl_accounts', 'Accounts'), deleteLocked: true }, + batchTable('tbl_contacts', 'Contacts'), + ]) + + await expect( + deleteTables(['tbl_accounts', 'tbl_contacts'], 'folder-cascade-folder-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + archiveAsCohort: true, + }) + ).resolves.toEqual({ + archived: [], + failed: [ + { + id: 'tbl_accounts', + name: 'Accounts', + reason: new TableLockedError('delete').message, + code: 'locked', + }, + ], + notFound: [], + }) + + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) + it('allows a self-referencing table to archive', async () => { queueTableRows(schemaMock.userTableDefinitions, [ batchTable('tbl_categories', 'Categories', ['tbl_categories']), diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index e55cdf1c96a..61248a42443 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -1269,7 +1269,12 @@ export async function deleteTable( export interface DeleteTablesResult { archived: { id: string; name: string; workspaceId: string }[] - failed: { id: string; name: string; reason: string }[] + failed: { + id: string + name: string + reason: string + code: 'locked' | 'reference' | 'reference_cycle' + }[] notFound: string[] terminalError?: unknown } @@ -1410,6 +1415,7 @@ export async function deleteTables( id: table.id, name: table.name, reason: new TableLockedError('delete').message, + code: 'locked', }) continue } @@ -1445,6 +1451,7 @@ export async function deleteTables( table.name, blockers.map((blocker) => blocker.referencingTableName) ), + code: 'reference', }) continue } @@ -1453,12 +1460,14 @@ export async function deleteTables( id: table.id, name: table.name, reason: tableReferenceCycleMessage(table.name), + code: 'reference_cycle', }) } } const archived: DeleteTablesResult['archived'] = [] if (options.archiveAsCohort) { + if (failed.length > 0) return { archived, failed, notFound } const cohort = [...archivePlan.ordered, ...archivePlan.blockedByCycle] if (cohort.length > 0) { archived.push( @@ -1598,6 +1607,9 @@ export async function restoreTable( try { await db.transaction(async (tx) => { await setTableTxTimeouts(tx) + await tx.execute( + sql`SELECT pg_advisory_xact_lock(hashtextextended(${`user_table_schema:${tableId}`}, 0))` + ) const [currentTable] = await tx .select({ workspaceId: userTableDefinitions.workspaceId, From c6b5faf4c7937d23965c60c3c0f50eef7fe1937a Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:19:16 -0700 Subject: [PATCH 4/6] fix(tables): make folder cascades retry-safe --- apps/sim/lib/folders/bulk.test.ts | 91 +++++++++++++++++-- apps/sim/lib/folders/bulk.ts | 73 ++++++++++----- apps/sim/lib/folders/orchestration.test.ts | 9 ++ apps/sim/lib/folders/orchestration.ts | 1 + .../table/reference-columns/availability.ts | 2 +- apps/sim/lib/table/service.test.ts | 17 +++- apps/sim/lib/table/service.ts | 1 + 7 files changed, 164 insertions(+), 30 deletions(-) diff --git a/apps/sim/lib/folders/bulk.test.ts b/apps/sim/lib/folders/bulk.test.ts index 4fc6c927e7d..9d76bad247d 100644 --- a/apps/sim/lib/folders/bulk.test.ts +++ b/apps/sim/lib/folders/bulk.test.ts @@ -3,24 +3,28 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockListActiveFolderRows } = vi.hoisted(() => ({ - mockListActiveFolderRows: vi.fn(), -})) +const { mockDeleteFolder, mockListActiveFolderRows, mockNotifyFolderResourceChanged } = vi.hoisted( + () => ({ + mockDeleteFolder: vi.fn(), + mockListActiveFolderRows: vi.fn(), + mockNotifyFolderResourceChanged: vi.fn(), + }) +) vi.mock('@/lib/folders/queries', () => ({ listActiveFolderRows: mockListActiveFolderRows, })) vi.mock('@/lib/folders/orchestration', () => ({ - deleteFolder: vi.fn(), + deleteFolder: mockDeleteFolder, updateFolder: vi.fn(), })) vi.mock('@/lib/realtime/notify', () => ({ - notifyFolderResourceChanged: vi.fn(), + notifyFolderResourceChanged: mockNotifyFolderResourceChanged, })) -import { planFolderSelection } from '@/lib/folders/bulk' +import { bulkDeleteFolders, planFolderSelection } from '@/lib/folders/bulk' /** * `a` holds `a1`, which holds `a1x`. `b` is a sibling with nothing inside it, so a plan can @@ -94,3 +98,78 @@ describe('planFolderSelection', () => { expect(result.selected).toEqual([{ id: 'b', name: 'B' }]) }) }) + +describe('bulkDeleteFolders', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + const deleteFolders = (folders: Array<{ id: string; name: string }>) => + bulkDeleteFolders({ + workspaceId: 'ws-1', + resourceType: 'table', + userId: 'user-1', + folders, + countKey: 'tables', + referenceCheckCompleted: true, + }) + + it('retries a target folder after a later referrer folder succeeds', async () => { + mockDeleteFolder + .mockResolvedValueOnce({ + success: false, + error: 'Target is referenced by Referrer', + errorCode: 'conflict', + }) + .mockResolvedValueOnce({ + success: true, + deletedItems: { folders: 1, tables: 1 }, + }) + .mockResolvedValueOnce({ + success: true, + deletedItems: { folders: 1, tables: 1 }, + }) + + const result = await deleteFolders([ + { id: 'target-folder', name: 'Target' }, + { id: 'referrer-folder', name: 'Referrer' }, + ]) + + expect(result).toEqual({ + succeeded: [ + { id: 'referrer-folder', name: 'Referrer' }, + { id: 'target-folder', name: 'Target' }, + ], + failed: [], + folderCount: 2, + resourceCount: 2, + }) + expect(mockDeleteFolder.mock.calls.map(([params]) => params.folderId)).toEqual([ + 'target-folder', + 'referrer-folder', + 'target-folder', + ]) + expect(mockNotifyFolderResourceChanged).toHaveBeenCalledOnce() + }) + + it('stops when a reference cycle makes no progress', async () => { + mockDeleteFolder.mockResolvedValue({ + success: false, + error: 'Referenced by another selected folder', + errorCode: 'conflict', + }) + + const result = await deleteFolders([ + { id: 'folder-a', name: 'A' }, + { id: 'folder-b', name: 'B' }, + ]) + + expect(result.succeeded).toEqual([]) + expect(result.failed).toEqual([ + { id: 'folder-a', name: 'A', reason: 'Referenced by another selected folder' }, + { id: 'folder-b', name: 'B', reason: 'Referenced by another selected folder' }, + ]) + expect(mockDeleteFolder).toHaveBeenCalledTimes(2) + expect(mockNotifyFolderResourceChanged).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/folders/bulk.ts b/apps/sim/lib/folders/bulk.ts index 299d4ebfe45..0169f892b57 100644 --- a/apps/sim/lib/folders/bulk.ts +++ b/apps/sim/lib/folders/bulk.ts @@ -244,6 +244,10 @@ export async function bulkMoveFolders(params: { * notification replaces a per-folder storm of identical invalidations, and it * fires from a `finally` so a batch cut short by an internal fault still * announces the folders it did archive. + * + * A reference conflict is retried only after another folder succeeds. This + * resolves cross-folder dependency chains without relying on request order, + * while a cycle makes no progress and terminates after one wave. */ export async function bulkDeleteFolders(params: { workspaceId: string @@ -260,30 +264,55 @@ export async function bulkDeleteFolders(params: { let resourceCount = 0 try { - for (const folder of params.folders) { - const result = await deleteFolder( - { - resourceType: params.resourceType, - folderId: folder.id, - workspaceId: params.workspaceId, - userId: params.userId, - folderName: folder.name, - }, - { - projectAudit: false, - notify: false, - referenceCheckCompleted: params.referenceCheckCompleted, + let pending = [...params.folders] + while (pending.length > 0) { + const retryable: BulkFolderAffected[] = [] + const retryReasons = new Map() + let completedInWave = 0 + + for (const folder of pending) { + const result = await deleteFolder( + { + resourceType: params.resourceType, + folderId: folder.id, + workspaceId: params.workspaceId, + userId: params.userId, + folderName: folder.name, + }, + { + projectAudit: false, + notify: false, + referenceCheckCompleted: params.referenceCheckCompleted, + } + ) + if (result.success) { + succeeded.push(folder) + folderCount += result.deletedItems?.folders ?? 0 + resourceCount += result.deletedItems?.[params.countKey] ?? 0 + completedInWave += 1 + continue } - ) - if (result.success) { - succeeded.push(folder) - folderCount += result.deletedItems?.folders ?? 0 - resourceCount += result.deletedItems?.[params.countKey] ?? 0 - continue + if (result.errorCode === 'internal') { + throw new Error(result.error ?? 'Failed to delete folder') + } + if (result.errorCode === 'conflict') { + retryable.push(folder) + retryReasons.set(folder.id, result.error ?? 'Failed to delete folder') + continue + } + failed.push({ ...folder, reason: result.error ?? 'Failed to delete folder' }) + } + + if (completedInWave === 0) { + for (const folder of retryable) { + failed.push({ + ...folder, + reason: retryReasons.get(folder.id) ?? 'Failed to delete folder', + }) + } + break } - if (result.errorCode === 'internal') - throw new Error(result.error ?? 'Failed to delete folder') - failed.push({ ...folder, reason: result.error ?? 'Failed to delete folder' }) + pending = retryable } } finally { if (succeeded.length > 0) { diff --git a/apps/sim/lib/folders/orchestration.test.ts b/apps/sim/lib/folders/orchestration.test.ts index ecaafa74f2d..ea6fc5575af 100644 --- a/apps/sim/lib/folders/orchestration.test.ts +++ b/apps/sim/lib/folders/orchestration.test.ts @@ -784,6 +784,15 @@ describe('deleteFolder', () => { error: 'Table deletion is locked', errorCode: 'locked', }) + const archiveTimestamp = mockArchiveFolderCascade.mock.calls[0][4] + expect(mockRestoreFolderRows).toHaveBeenCalledWith( + dbChainMock.db, + resourceConfig.current, + 'ws-1', + ['folder-1'], + archiveTimestamp, + expect.any(Date) + ) }) it('hands the guard the whole resolved subtree, not just the root', async () => { diff --git a/apps/sim/lib/folders/orchestration.ts b/apps/sim/lib/folders/orchestration.ts index 17136ef5015..00388090721 100644 --- a/apps/sim/lib/folders/orchestration.ts +++ b/apps/sim/lib/folders/orchestration.ts @@ -863,6 +863,7 @@ async function deleteFolderWithoutTreeLock( } catch (error) { const classified = asOrchestrationError(error) if (classified?.code === 'locked' || classified?.code === 'conflict') { + await restoreFolderRows(db, config, workspaceId, folderIds, timestamp, new Date()) return { success: false, error: classified.message, errorCode: classified.code } } throw error diff --git a/apps/sim/lib/table/reference-columns/availability.ts b/apps/sim/lib/table/reference-columns/availability.ts index d50c9a5327b..c7b18a659b1 100644 --- a/apps/sim/lib/table/reference-columns/availability.ts +++ b/apps/sim/lib/table/reference-columns/availability.ts @@ -9,7 +9,7 @@ export function areTableReferenceColumnsEnabled(): Promise { return isFeatureEnabled('table-reference-columns') } -/** Rejects mutations that introduce or reconfigure a Reference column. */ +/** Rejects operations that expose or mutate Reference-column behavior. */ export async function assertTableReferenceColumnsEnabled(): Promise { if (!(await areTableReferenceColumnsEnabled())) { throw new OrchestrationError('forbidden', TABLE_REFERENCE_COLUMNS_DISABLED_MESSAGE) diff --git a/apps/sim/lib/table/service.test.ts b/apps/sim/lib/table/service.test.ts index eb8eddd6b4c..8ccedcf14e5 100644 --- a/apps/sim/lib/table/service.test.ts +++ b/apps/sim/lib/table/service.test.ts @@ -77,7 +77,11 @@ import { const WORKSPACE_ID = '6fc7631d-88cd-46f8-9f0a-d4764daef7f8' describe('listActiveTableNames', () => { - beforeEach(() => resetDbChainMock()) + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mocks.assertTableReferenceColumnsEnabled.mockResolvedValue(undefined) + }) it('returns the name projection without loading table schemas', async () => { queueTableRows(schemaMock.userTableDefinitions, [{ id: 'table-1', name: 'Accounts' }]) @@ -102,6 +106,17 @@ describe('listActiveTableNames', () => { it('skips the database when no table IDs are requested', async () => { await expect(listActiveTableNames(WORKSPACE_ID, [])).resolves.toEqual([]) + expect(mocks.assertTableReferenceColumnsEnabled).toHaveBeenCalledOnce() + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + + it('rejects name lookups before querying when reference columns are disabled', async () => { + mocks.assertTableReferenceColumnsEnabled.mockRejectedValueOnce({ code: 'forbidden' }) + + await expect(listActiveTableNames(WORKSPACE_ID, ['table-1'])).rejects.toMatchObject({ + code: 'forbidden', + }) + expect(dbChainMockFns.select).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index 61248a42443..ebcc52f43f3 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -383,6 +383,7 @@ export async function listActiveTableNames( workspaceId: string, tableIds: readonly string[] ): Promise>> { + await assertTableReferenceColumnsEnabled() if (tableIds.length === 0) return [] return db From c913829db5a5e6f52c6dd7eba7297ba1904ca25d Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:34:22 -0700 Subject: [PATCH 5/6] fix(tables): roll back partial folder restores --- apps/sim/lib/folders/cascade.test.ts | 38 +++++++++++++++++++++++ apps/sim/lib/folders/config.ts | 45 +++++++++++++++++++++------- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts index 0ccc436d77a..423245a042e 100644 --- a/apps/sim/lib/folders/cascade.test.ts +++ b/apps/sim/lib/folders/cascade.test.ts @@ -26,10 +26,12 @@ import { folderMutationStatus } from '@/lib/folders/status' const tableServiceMocks = vi.hoisted(() => ({ deleteTables: vi.fn(), + restoreTable: vi.fn(), })) vi.mock('@/lib/table/service', () => ({ deleteTables: tableServiceMocks.deleteTables, + restoreTable: tableServiceMocks.restoreTable, })) interface SelectCall { @@ -614,6 +616,42 @@ describe('knowledge_base and table folder resources', () => { }) ).rejects.toMatchObject({ code: 'locked', message: 'Table deletion is locked' }) }) + + it('re-archives a successful restore prefix when a later cohort table fails', async () => { + resetDbChainMock() + queueTableRows(schemaMock.userTableDefinitions, [ + { id: 'tbl_accounts' }, + { id: 'tbl_contacts' }, + ]) + const restoreError = new Error('restore failed') + tableServiceMocks.restoreTable + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(restoreError) + tableServiceMocks.deleteTables.mockResolvedValueOnce({ + archived: [{ id: 'tbl_accounts', name: 'Accounts', workspaceId: 'ws-1' }], + failed: [], + notFound: [], + }) + + await expect( + tableConfig.restoreChildren?.({ + workspaceId: 'ws-1', + folderIds: ['folder-1'], + timestamp: TIMESTAMP, + }) + ).rejects.toBe(restoreError) + + expect(tableServiceMocks.deleteTables).toHaveBeenCalledWith( + ['tbl_accounts'], + 'folder-restore-rollback-folder-1', + { + expectedWorkspaceId: 'ws-1', + archivedAt: TIMESTAMP, + skipNotify: true, + archiveAsCohort: true, + } + ) + }) }) describe('table folder deletion guard', () => { diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts index 8e1e8d9918a..dc80b5dbbd8 100644 --- a/apps/sim/lib/folders/config.ts +++ b/apps/sim/lib/folders/config.ts @@ -336,21 +336,46 @@ async function archiveTableChildren(context: CascadeChildrenContext): Promise { - const { restoreTable } = await import('@/lib/table/service') + const { deleteTables, restoreTable } = await import('@/lib/table/service') const ids = await selectChildIds(FOLDER_RESOURCES.table, context, 'archived') const restoringFolderIds = new Set(context.folderIds) const restoringTableIds = new Set(ids) - - for (const id of ids) { - // restoreFolder fires one folder-level live-list notify for the whole subtree. - await restoreTable(id, `folder-cascade-${context.folderIds[0]}`, { - restoringFolderIds, - restoringTableIds, - skipNotify: true, - }) + const restoredIds: string[] = [] + + try { + for (const id of ids) { + // restoreFolder fires one folder-level live-list notify for the whole subtree. + await restoreTable(id, `folder-cascade-${context.folderIds[0]}`, { + restoringFolderIds, + restoringTableIds, + skipNotify: true, + }) + restoredIds.push(id) + } + } catch (error) { + const rollback = await deleteTables( + restoredIds, + `folder-restore-rollback-${context.folderIds[0]}`, + { + expectedWorkspaceId: context.workspaceId, + archivedAt: context.timestamp, + skipNotify: true, + archiveAsCohort: true, + } + ) + if (rollback.terminalError) throw rollback.terminalError + if ( + rollback.failed.length > 0 || + rollback.notFound.length > 0 || + rollback.archived.length !== restoredIds.length + ) { + throw new OrchestrationError('internal', 'Failed to roll back the table restore cohort') + } + throw error } return ids.length From 803b9a8100915af9f414b53160a10b262ad2cce4 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Thu, 3 Sep 2026 16:43:03 -0700 Subject: [PATCH 6/6] fix(tables): reject incomplete folder cohorts --- apps/sim/lib/folders/cascade.test.ts | 24 ++++++++++++++++++++++++ apps/sim/lib/folders/config.ts | 6 ++++++ apps/sim/lib/table/service.test.ts | 18 ++++++++++++++++++ apps/sim/lib/table/service.ts | 2 +- 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/apps/sim/lib/folders/cascade.test.ts b/apps/sim/lib/folders/cascade.test.ts index 423245a042e..10d61ab3fbe 100644 --- a/apps/sim/lib/folders/cascade.test.ts +++ b/apps/sim/lib/folders/cascade.test.ts @@ -617,6 +617,30 @@ describe('knowledge_base and table folder resources', () => { ).rejects.toMatchObject({ code: 'locked', message: 'Table deletion is locked' }) }) + it('rejects the cohort when a table changes after child selection', async () => { + resetDbChainMock() + queueTableRows(schemaMock.userTableDefinitions, [ + { id: 'tbl_accounts' }, + { id: 'tbl_contacts' }, + ]) + tableServiceMocks.deleteTables.mockResolvedValueOnce({ + archived: [], + failed: [], + notFound: ['tbl_contacts'], + }) + + await expect( + tableConfig.archiveChildren?.({ + workspaceId: 'ws-1', + folderIds: ['folder-1'], + timestamp: TIMESTAMP, + }) + ).rejects.toMatchObject({ + code: 'conflict', + message: 'One or more tables changed while their folder was being deleted', + }) + }) + it('re-archives a successful restore prefix when a later cohort table fails', async () => { resetDbChainMock() queueTableRows(schemaMock.userTableDefinitions, [ diff --git a/apps/sim/lib/folders/config.ts b/apps/sim/lib/folders/config.ts index dc80b5dbbd8..e2375973050 100644 --- a/apps/sim/lib/folders/config.ts +++ b/apps/sim/lib/folders/config.ts @@ -329,6 +329,12 @@ async function archiveTableChildren(context: CascadeChildrenContext): Promise 0 || result.archived.length !== ids.length) { + throw new OrchestrationError( + 'conflict', + 'One or more tables changed while their folder was being deleted' + ) + } return result.archived.length } diff --git a/apps/sim/lib/table/service.test.ts b/apps/sim/lib/table/service.test.ts index 8ccedcf14e5..589f79ec6e6 100644 --- a/apps/sim/lib/table/service.test.ts +++ b/apps/sim/lib/table/service.test.ts @@ -831,6 +831,24 @@ describe('deleteTables reference guard', () => { expect(dbChainMockFns.update).not.toHaveBeenCalled() }) + it('archives none of a restore cohort when one selected table is no longer active', async () => { + queueTableRows(schemaMock.userTableDefinitions, [batchTable('tbl_accounts', 'Accounts')]) + + await expect( + deleteTables(['tbl_accounts', 'tbl_contacts'], 'folder-cascade-folder-1', { + expectedWorkspaceId: WORKSPACE_ID, + skipNotify: true, + archiveAsCohort: true, + }) + ).resolves.toEqual({ + archived: [], + failed: [], + notFound: ['tbl_contacts'], + }) + + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) + it('allows a self-referencing table to archive', async () => { queueTableRows(schemaMock.userTableDefinitions, [ batchTable('tbl_categories', 'Categories', ['tbl_categories']), diff --git a/apps/sim/lib/table/service.ts b/apps/sim/lib/table/service.ts index ebcc52f43f3..fcad050d9cd 100644 --- a/apps/sim/lib/table/service.ts +++ b/apps/sim/lib/table/service.ts @@ -1468,7 +1468,7 @@ export async function deleteTables( const archived: DeleteTablesResult['archived'] = [] if (options.archiveAsCohort) { - if (failed.length > 0) return { archived, failed, notFound } + if (failed.length > 0 || notFound.length > 0) return { archived, failed, notFound } const cohort = [...archivePlan.ordered, ...archivePlan.blockedByCycle] if (cohort.length > 0) { archived.push(