Skip to content

Commit 1e07a7b

Browse files
committed
fix(tables): resolve reference review findings
1 parent fa3fff6 commit 1e07a7b

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
@@ -286,6 +286,17 @@ function invalidateReferencePreviews(
286286
})
287287
}
288288

289+
function invalidateTableNames(queryClient: ReturnType<typeof useQueryClient>) {
290+
queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() })
291+
}
292+
293+
function invalidateReferenceTablePreviews(
294+
queryClient: ReturnType<typeof useQueryClient>,
295+
tableId: string
296+
) {
297+
queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviewsForTable(tableId) })
298+
}
299+
289300
/**
290301
* Invalidate only the row-count surfaces — the table detail and the tables
291302
* list, both of which carry the unfiltered `rowCount`. Deliberately leaves
@@ -310,6 +321,7 @@ function invalidateTableSchema(queryClient: ReturnType<typeof useQueryClient>, t
310321
queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) })
311322
queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) })
312323
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
324+
invalidateReferenceTablePreviews(queryClient, tableId)
313325
}
314326

315327
/**
@@ -327,6 +339,7 @@ function invalidateTableSchemaOnly(
327339
) {
328340
queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) })
329341
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
342+
invalidateReferenceTablePreviews(queryClient, tableId)
330343
}
331344

332345
/**
@@ -736,6 +749,7 @@ export function useCreateTable(workspaceId: string) {
736749
},
737750
onSettled: () => {
738751
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
752+
invalidateTableNames(queryClient)
739753
},
740754
})
741755
}
@@ -785,6 +799,8 @@ export function useRenameTable(workspaceId: string) {
785799
onSettled: (_data, _error, variables) => {
786800
queryClient.invalidateQueries({ queryKey: tableKeys.detail(variables.tableId) })
787801
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
802+
invalidateTableNames(queryClient)
803+
invalidateReferenceTablePreviews(queryClient, variables.tableId)
788804
},
789805
})
790806
}
@@ -895,6 +911,8 @@ export function useDeleteTable(workspaceId: string) {
895911
},
896912
onSettled: (_data, _error, tableId) => {
897913
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
914+
invalidateTableNames(queryClient)
915+
invalidateReferenceTablePreviews(queryClient, tableId)
898916
queryClient.removeQueries({ queryKey: tableKeys.detail(tableId) })
899917
queryClient.removeQueries({ queryKey: tableKeys.rowsRoot(tableId) })
900918
},
@@ -1986,8 +2004,12 @@ export function useRestoreTable() {
19862004
onSettled: (_data, _error, tableId) => {
19872005
return Promise.all([
19882006
queryClient.invalidateQueries({ queryKey: tableKeys.lists() }),
2007+
queryClient.invalidateQueries({ queryKey: tableKeys.namesRoot() }),
19892008
queryClient.invalidateQueries({ queryKey: tableKeys.detail(tableId) }),
19902009
queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) }),
2010+
queryClient.invalidateQueries({
2011+
queryKey: tableKeys.referencePreviewsForTable(tableId),
2012+
}),
19912013
])
19922014
},
19932015
})
@@ -2105,6 +2127,7 @@ export function useImportCsv() {
21052127
},
21062128
onSettled: () => {
21072129
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
2130+
invalidateTableNames(queryClient)
21082131
},
21092132
})
21102133
}
@@ -2149,6 +2172,7 @@ export function useImportFileAsTable() {
21492172
},
21502173
onSettled: () => {
21512174
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
2175+
invalidateTableNames(queryClient)
21522176
},
21532177
})
21542178
}
@@ -2816,6 +2840,8 @@ export function useBulkDeleteTables(workspaceId: string) {
28162840
},
28172841
onSettled: (_data, _error, { tableIds = [] }) => {
28182842
queryClient.invalidateQueries({ queryKey: tableKeys.lists() })
2843+
invalidateTableNames(queryClient)
2844+
queryClient.invalidateQueries({ queryKey: tableKeys.referencePreviews() })
28192845
queryClient.invalidateQueries({ queryKey: folderKeys.resource('table') })
28202846
for (const tableId of tableIds) {
28212847
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
}
@@ -543,6 +551,41 @@ describe('knowledge_base and table folder resources', () => {
543551
expect(knowledgeConfig.sortOrderColumn).toBeUndefined()
544552
expect(tableConfig.sortOrderColumn).toBeUndefined()
545553
})
554+
555+
it('archives table children as one restorable cohort', async () => {
556+
resetDbChainMock()
557+
queueTableRows(schemaMock.userTableDefinitions, [
558+
{ id: 'tbl_accounts' },
559+
{ id: 'tbl_contacts' },
560+
])
561+
tableServiceMocks.deleteTables.mockResolvedValueOnce({
562+
archived: [
563+
{ id: 'tbl_accounts', name: 'Accounts', workspaceId: 'ws-1' },
564+
{ id: 'tbl_contacts', name: 'Contacts', workspaceId: 'ws-1' },
565+
],
566+
failed: [],
567+
notFound: [],
568+
})
569+
570+
await expect(
571+
tableConfig.archiveChildren?.({
572+
workspaceId: 'ws-1',
573+
folderIds: ['folder-1'],
574+
timestamp: TIMESTAMP,
575+
})
576+
).resolves.toBe(2)
577+
578+
expect(tableServiceMocks.deleteTables).toHaveBeenCalledWith(
579+
['tbl_accounts', 'tbl_contacts'],
580+
'folder-cascade-folder-1',
581+
{
582+
expectedWorkspaceId: 'ws-1',
583+
archivedAt: TIMESTAMP,
584+
skipNotify: true,
585+
archiveAsCohort: true,
586+
}
587+
)
588+
})
546589
})
547590

548591
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,
@@ -342,18 +343,21 @@ async function restoreKnowledgeBaseChildren(context: CascadeChildrenContext): Pr
342343
* encounter a partial cascade.
343344
*/
344345
async function archiveTableChildren(context: CascadeChildrenContext): Promise<number> {
345-
const { deleteTable } = await import('@/lib/table/service')
346+
const { deleteTables } = await import('@/lib/table/service')
346347
const ids = await selectChildIds(FOLDER_RESOURCES.table, context, 'active')
347-
348-
for (const id of ids) {
349-
await deleteTable(id, `folder-cascade-${context.folderIds[0]}`, {
350-
archivedAt: context.timestamp,
351-
// deleteFolder fires one folder-level live-list notify for the whole subtree.
352-
skipNotify: true,
353-
})
348+
const result = await deleteTables(ids, `folder-cascade-${context.folderIds[0]}`, {
349+
expectedWorkspaceId: context.workspaceId,
350+
archivedAt: context.timestamp,
351+
// deleteFolder fires one folder-level live-list notify for the whole subtree.
352+
skipNotify: true,
353+
archiveAsCohort: true,
354+
})
355+
if (result.terminalError) throw result.terminalError
356+
if (result.failed[0]) {
357+
throw new OrchestrationError('conflict', result.failed[0].reason)
354358
}
355359

356-
return ids.length
360+
return result.archived.length
357361
}
358362

359363
/**

‎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)