Skip to content

Commit 9509d93

Browse files
committed
fix(tables): resolve reference review findings
1 parent 55f46e4 commit 9509d93

11 files changed

Lines changed: 191 additions & 25 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-event-stream.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,12 +475,16 @@ export function useTableEventStream({
475475
// invalidateTableSchema set — the definition (exact, so rows stay on the
476476
// debounce), the run-state + enrichment sibling queries under detail (a group
477477
// delete/restructure can otherwise leave a stale running badge or enrichment
478-
// panel), the tables list (column/row counts), and the debounced rows.
478+
// panel), the tables list (column/row counts), open reference previews, and the
479+
// debounced rows.
479480
else if (entry.event?.kind === 'schema') {
480481
void queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId), exact: true })
481482
void queryClient.invalidateQueries({ queryKey: tableKeys.activeDispatches(tableId) })
482483
void queryClient.invalidateQueries({ queryKey: tableKeys.enrichmentDetails(tableId) })
483484
void queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
485+
void queryClient.invalidateQueries({
486+
queryKey: tableKeys.referencePreviewsForTable(tableId),
487+
})
484488
scheduleRowsInvalidate()
485489
}
486490
// A collaborator changed the column layout (width/pin/order): refetch the

‎apps/sim/app/workspace/[workspaceId]/tables/hooks/use-workspace-tables-room.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ import { tableKeys } from '@/hooks/queries/utils/table-keys'
88

99
/**
1010
* Keeps the tables browser live: joins the workspace-tables room so a `workspace-tables-changed`
11-
* broadcast (fanned out by the table + table-folder mutation services) invalidates the tables list
12-
* AND the table folders so every viewer refetches without waiting for staleness. A created/renamed/
11+
* broadcast (fanned out by the table + table-folder mutation services) invalidates table lists,
12+
* names, reference previews, AND table folders so every viewer refetches without waiting for
13+
* staleness. A created/renamed/
1314
* moved/deleted/restored table changes the list result (including folder placement); a folder
1415
* create/rename/delete/restore changes the folder tree — the page renders both, so both are
1516
* invalidated. Thin binding over {@link useWorkspaceInvalidationRoom}.
@@ -18,6 +19,8 @@ export function useWorkspaceTablesRoom(workspaceId: string): void {
1819
const queryClient = useQueryClient()
1920
useWorkspaceInvalidationRoom(workspaceId, ROOM_TYPES.WORKSPACE_TABLES, () => {
2021
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
22+
queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() })
23+
queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviews() })
2124
queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') })
2225
})
2326
}

‎apps/sim/hooks/queries/folders.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,11 @@ function invalidateCascadedResourceLists(
128128
case 'workflow':
129129
return invalidateWorkflowLists(queryClient, workspaceId, ['active', 'archived'])
130130
case 'table':
131-
return queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
131+
return Promise.all([
132+
queryClient.invalidateQueries({ queryKey: tableKeys.lists() }),
133+
queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() }),
134+
queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviews() }),
135+
]).then(() => undefined)
132136
case 'knowledge_base':
133137
return queryClient.invalidateQueries({ queryKey: knowledgeKeys.lists() })
134138
/**

‎apps/sim/hooks/queries/tables.test.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ describe('useTableNames', () => {
127127
enabled: true,
128128
queryKey: tableKeys.names(WORKSPACE_ID, [TABLE_ID, 'tbl-2']),
129129
})
130+
expect(options.queryKey.slice(0, tableKeys.namesRoot().length)).toEqual(tableKeys.namesRoot())
131+
expect(options.queryKey.slice(0, tableKeys.lists().length)).not.toEqual(tableKeys.lists())
130132
expect(requestJson).toHaveBeenCalledWith(listTableNamesContract, {
131133
body: { workspaceId: WORKSPACE_ID, tableIds: [TABLE_ID, 'tbl-2'] },
132134
signal,
@@ -580,7 +582,7 @@ describe('useDeleteColumn optimistic update', () => {
580582
expect(getCache(ROWS_KEY)).toEqual(originalRows)
581583
})
582584

583-
it('invalidates schema, rows, and lists in onSettled', () => {
585+
it('invalidates schema, rows, lists, and mounted reference previews in onSettled', () => {
584586
const hook = useDeleteColumn({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID })
585587
hook.onSettled?.(undefined, null, 'age', undefined)
586588

@@ -590,6 +592,7 @@ describe('useDeleteColumn optimistic update', () => {
590592
tableKeys.detail(TABLE_ID),
591593
tableKeys.rowsRoot(TABLE_ID),
592594
tableKeys.lists(),
595+
tableKeys.referencePreviewsForTable(TABLE_ID),
593596
])
594597
)
595598
})
@@ -648,6 +651,15 @@ describe('useUpdateColumn optimistic update', () => {
648651
)
649652
expect(detail?.schema.columns[0]).toMatchObject({ id: 'age', name: 'years' })
650653
})
654+
655+
it('invalidates mounted previews when the referenced table schema changes', () => {
656+
const hook = useUpdateColumn({ workspaceId: WORKSPACE_ID, tableId: TABLE_ID })
657+
hook.onSettled?.(undefined, null, { columnName: 'age', updates: { name: 'years' } }, undefined)
658+
659+
expect(queryClient.invalidateQueries).toHaveBeenCalledWith({
660+
queryKey: tableKeys.referencePreviewsForTable(TABLE_ID),
661+
})
662+
})
651663
})
652664

653665
describe('useRestoreTable cache invalidation', () => {
@@ -674,16 +686,18 @@ describe('useRestoreTable cache invalidation', () => {
674686
})
675687
})
676688

677-
it('invalidates lists, table detail, and row data for the restored table', () => {
689+
it('invalidates names, previews, lists, table detail, and row data for the restored table', () => {
678690
const hook = useRestoreTable()
679691
hook.onSettled?.(undefined, null, TABLE_ID, undefined)
680692

681693
const calls = queryClient.invalidateQueries.mock.calls.map((c) => c[0]?.queryKey)
682694
expect(calls).toEqual(
683695
expect.arrayContaining([
684696
tableKeys.lists(),
697+
tableKeys.namesRoot(),
685698
tableKeys.detail(TABLE_ID),
686699
tableKeys.rowsRoot(TABLE_ID),
700+
tableKeys.referencePreviewsForTable(TABLE_ID),
687701
])
688702
)
689703
})

‎apps/sim/hooks/queries/tables.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,17 @@ function invalidateReferencePreviews(
280280
})
281281
}
282282

283+
function invalidateTableNames(queryClient: ReturnType<typeof useQueryClient>) {
284+
queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() })
285+
}
286+
287+
function invalidateReferenceTablePreviews(
288+
queryClient: ReturnType<typeof useQueryClient>,
289+
tableId: string
290+
) {
291+
queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviewsForTable(tableId) })
292+
}
293+
283294
/**
284295
* Invalidate only the row-count surfaces — the table detail and the tables
285296
* list, both of which carry the unfiltered `rowCount`. Deliberately leaves
@@ -304,6 +315,7 @@ function invalidateTableSchema(queryClient: ReturnType<typeof useQueryClient>, t
304315
queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) })
305316
queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) })
306317
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
318+
invalidateReferenceTablePreviews(queryClient, tableId)
307319
}
308320

309321
/**
@@ -321,6 +333,7 @@ function invalidateTableSchemaOnly(
321333
) {
322334
queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) })
323335
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
336+
invalidateReferenceTablePreviews(queryClient, tableId)
324337
}
325338

326339
/**
@@ -730,6 +743,7 @@ export function useCreateTable(workspaceId: string) {
730743
},
731744
onSettled: () => {
732745
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
746+
invalidateTableNames(queryClient)
733747
},
734748
})
735749
}
@@ -779,6 +793,8 @@ export function useRenameTable(workspaceId: string) {
779793
onSettled: (_data, _error, variables) => {
780794
queryClient.invalidateQueries({ queryKey: tableKeys.detail(variables.tableId) })
781795
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
796+
invalidateTableNames(queryClient)
797+
invalidateReferenceTablePreviews(queryClient, variables.tableId)
782798
},
783799
})
784800
}
@@ -889,6 +905,8 @@ export function useDeleteTable(workspaceId: string) {
889905
},
890906
onSettled: (_data, _error, tableId) => {
891907
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
908+
invalidateTableNames(queryClient)
909+
invalidateReferenceTablePreviews(queryClient, tableId)
892910
queryClient.removeQueries({ queryKey: tableKeys.detail(tableId) })
893911
queryClient.removeQueries({ queryKey: tableKeys.rowsRoot(tableId) })
894912
},
@@ -1954,8 +1972,12 @@ export function useRestoreTable() {
19541972
onSettled: (_data, _error, tableId) => {
19551973
return Promise.all([
19561974
queryClient.invalidateQueries({ queryKey: tableKeys.lists() }),
1975+
queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() }),
19571976
queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) }),
19581977
queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) }),
1978+
queryClient.invalidateQueries({
1979+
queryKey: tableKeys.referencePreviewsForTable(tableId),
1980+
}),
19591981
])
19601982
},
19611983
})
@@ -2073,6 +2095,7 @@ export function useImportCsv() {
20732095
},
20742096
onSettled: () => {
20752097
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
2098+
invalidateTableNames(queryClient)
20762099
},
20772100
})
20782101
}
@@ -2117,6 +2140,7 @@ export function useImportFileAsTable() {
21172140
},
21182141
onSettled: () => {
21192142
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
2143+
invalidateTableNames(queryClient)
21202144
},
21212145
})
21222146
}
@@ -2784,6 +2808,8 @@ export function useBulkDeleteTables(workspaceId: string) {
27842808
},
27852809
onSettled: (_data, _error, { tableIds = [] }) => {
27862810
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
2811+
invalidateTableNames(queryClient)
2812+
queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviews() })
27872813
queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') })
27882814
for (const tableId of tableIds) {
27892815
queryClient.removeQueries({ queryKey: tableKeys.detail(tableId) })

‎apps/sim/hooks/queries/utils/table-keys.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,19 @@ export const tableKeys = {
2020
lists: () => [...tableKeys.all, 'list'] as const,
2121
list: (workspaceId?: string, scope: TableQueryScope = 'active') =>
2222
[...tableKeys.lists(), workspaceId ?? '', scope] as const,
23+
namesRoot: () => [...tableKeys.all, 'names'] as const,
2324
names: (workspaceId: string | undefined, tableIds: readonly string[]) =>
24-
[...tableKeys.lists(), 'names', workspaceId ?? '', tableIds] as const,
25+
[...tableKeys.namesRoot(), workspaceId ?? '', tableIds] as const,
2526
details: () => [...tableKeys.all, 'detail'] as const,
2627
detail: (tableId: string) => [...tableKeys.details(), tableId] as const,
2728
exportJobs: (workspaceId?: string) =>
2829
[...tableKeys.all, 'export-jobs', workspaceId ?? ''] as const,
2930
rowsRoot: (tableId: string) => [...tableKeys.detail(tableId), 'rows'] as const,
3031
referencePreviews: () => [...tableKeys.all, 'reference-preview'] as const,
32+
referencePreviewsForTable: (tableId: string) =>
33+
[...tableKeys.referencePreviews(), tableId] as const,
3134
referencePreview: (tableId: string, rowId: string, sourceRowId = '', sourceColumnKey = '') =>
32-
[...tableKeys.referencePreviews(), tableId, rowId, sourceRowId, sourceColumnKey] as const,
35+
[...tableKeys.referencePreviewsForTable(tableId), rowId, sourceRowId, sourceColumnKey] as const,
3336
/**
3437
* Prefix covering only the paged row lists. `rowsRoot` is a shared parent — `find`
3538
* hangs off it holding a different shape — so anything walking the cache for row

‎apps/sim/lib/folders/cascade.test.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ import { FolderCollectionLimitExceededError } from '@/lib/folders/errors'
2323
import { folderResourceSupportsLocking } from '@/lib/folders/resource-traits'
2424
import { folderMutationStatus } from '@/lib/folders/status'
2525

26+
const tableServiceMocks = vi.hoisted(() => ({
27+
deleteTables: vi.fn(),
28+
}))
29+
30+
vi.mock('@/lib/table/service', () => ({
31+
deleteTables: tableServiceMocks.deleteTables,
32+
}))
33+
2634
interface SelectCall {
2735
where: unknown
2836
}
@@ -545,6 +553,41 @@ describe('knowledge_base and table folder resources', () => {
545553
expect(knowledgeConfig.sortOrderColumn).toBeUndefined()
546554
expect(tableConfig.sortOrderColumn).toBeUndefined()
547555
})
556+
557+
it('archives table children as one restorable cohort', async () => {
558+
resetDbChainMock()
559+
queueTableRows(schemaMock.userTableDefinitions, [
560+
{ id: 'tbl_accounts' },
561+
{ id: 'tbl_contacts' },
562+
])
563+
tableServiceMocks.deleteTables.mockResolvedValueOnce({
564+
archived: [
565+
{ id: 'tbl_accounts', name: 'Accounts', workspaceId: 'ws-1' },
566+
{ id: 'tbl_contacts', name: 'Contacts', workspaceId: 'ws-1' },
567+
],
568+
failed: [],
569+
notFound: [],
570+
})
571+
572+
await expect(
573+
tableConfig.archiveChildren?.({
574+
workspaceId: 'ws-1',
575+
folderIds: ['folder-1'],
576+
timestamp: TIMESTAMP,
577+
})
578+
).resolves.toBe(2)
579+
580+
expect(tableServiceMocks.deleteTables).toHaveBeenCalledWith(
581+
['tbl_accounts', 'tbl_contacts'],
582+
'folder-cascade-folder-1',
583+
{
584+
expectedWorkspaceId: 'ws-1',
585+
archivedAt: TIMESTAMP,
586+
skipNotify: true,
587+
archiveAsCohort: true,
588+
}
589+
)
590+
})
548591
})
549592

550593
describe('table folder deletion guard', () => {

‎apps/sim/lib/folders/cascade.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ function childFilter(config: FolderResourceConfig, workspaceId: string, folderId
9999
* also reviving siblings that were deleted independently.
100100
*
101101
* Folders are stamped BEFORE their children, which is what makes a failed cascade
102-
* recoverable. `archiveChildren` hooks walk resources one at a time through their canonical
103-
* delete, so a mid-loop failure can leave some children archived and some not. With the
102+
* recoverable. `archiveChildren` hooks may walk resources through their canonical deletes, so
103+
* a mid-cascade failure can leave some children archived and some not. With the
104104
* folder already stamped, `deleteFolder` reuses that same `deletedAt` on the retry and the
105105
* stragglers join the original snapshot. Stamping children first would leave the folder
106106
* active, so a retry would mint a fresh timestamp and the partially-archived children could

‎apps/sim/lib/folders/config.ts‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
import { eq, type SQL } from 'drizzle-orm'
1212
import type { PgColumn, PgTable } from 'drizzle-orm/pg-core'
1313
import type { FolderResourceType } from '@/lib/api/contracts/folders'
14+
import { OrchestrationError } from '@/lib/core/orchestration/types'
1415
import {
1516
FOLDER_RESOURCE_LABELS,
1617
FOLDER_RESOURCE_SUPPORTS_LOCKING,
@@ -310,18 +311,21 @@ async function restoreKnowledgeBaseChildren(context: CascadeChildrenContext): Pr
310311
* encounter a partial cascade.
311312
*/
312313
async function archiveTableChildren(context: CascadeChildrenContext): Promise<number> {
313-
const { deleteTable } = await import('@/lib/table/service')
314+
const { deleteTables } = await import('@/lib/table/service')
314315
const ids = await selectChildIds(FOLDER_RESOURCES.table, context, 'active')
315-
316-
for (const id of ids) {
317-
await deleteTable(id, `folder-cascade-${context.folderIds[0]}`, {
318-
archivedAt: context.timestamp,
319-
// deleteFolder fires one folder-level live-list notify for the whole subtree.
320-
skipNotify: true,
321-
})
316+
const result = await deleteTables(ids, `folder-cascade-${context.folderIds[0]}`, {
317+
expectedWorkspaceId: context.workspaceId,
318+
archivedAt: context.timestamp,
319+
// deleteFolder fires one folder-level live-list notify for the whole subtree.
320+
skipNotify: true,
321+
archiveAsCohort: true,
322+
})
323+
if (result.terminalError) throw result.terminalError
324+
if (result.failed[0]) {
325+
throw new OrchestrationError('conflict', result.failed[0].reason)
322326
}
323327

324-
return ids.length
328+
return result.archived.length
325329
}
326330

327331
/**

‎apps/sim/lib/table/service.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,35 @@ describe('deleteTables reference guard', () => {
746746
expect(dbChainMockFns.update).toHaveBeenCalledOnce()
747747
})
748748

749+
it('archives a reference cycle atomically when it will be restored as one cohort', async () => {
750+
queueTableRows(schemaMock.userTableDefinitions, [
751+
batchTable('tbl_accounts', 'Accounts', ['tbl_contacts']),
752+
batchTable('tbl_contacts', 'Contacts', ['tbl_accounts']),
753+
])
754+
dbChainMockFns.returning.mockResolvedValueOnce([
755+
{ id: 'tbl_accounts', name: 'Accounts', workspaceId: WORKSPACE_ID },
756+
{ id: 'tbl_contacts', name: 'Contacts', workspaceId: WORKSPACE_ID },
757+
])
758+
759+
await expect(
760+
deleteTables(['tbl_accounts', 'tbl_contacts'], 'folder-cascade-folder-1', {
761+
expectedWorkspaceId: WORKSPACE_ID,
762+
skipNotify: true,
763+
archiveAsCohort: true,
764+
})
765+
).resolves.toEqual({
766+
archived: [
767+
{ id: 'tbl_accounts', name: 'Accounts', workspaceId: WORKSPACE_ID },
768+
{ id: 'tbl_contacts', name: 'Contacts', workspaceId: WORKSPACE_ID },
769+
],
770+
failed: [],
771+
notFound: [],
772+
})
773+
774+
expect(dbChainMockFns.update).toHaveBeenCalledOnce()
775+
expect(dbChainMockFns.transaction).toHaveBeenCalledOnce()
776+
})
777+
749778
it('allows a self-referencing table to archive', async () => {
750779
queueTableRows(schemaMock.userTableDefinitions, [
751780
batchTable('tbl_categories', 'Categories', ['tbl_categories']),

0 commit comments

Comments
 (0)