Skip to content

Commit edfb2a8

Browse files
committed
fix(tables): restore virtual table read-only behavior
1 parent b3588ad commit edfb2a8

10 files changed

Lines changed: 342 additions & 48 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type {
2020
TableViewConfig,
2121
WorkflowGroup,
2222
} from '@/lib/table'
23+
import { canMutateTable, canRenameTable } from '@/lib/table/capabilities'
2324
import { getColumnId } from '@/lib/table/column-keys'
2425
import { TABLE_LIMITS } from '@/lib/table/constants'
2526
import {
@@ -1001,7 +1002,7 @@ export function Table({
10011002

10021003
const handleStartTableRename = useCallback(() => {
10031004
const data = tableDataRef.current
1004-
if (data) tableHeaderRename.startRename(tableId, data.name)
1005+
if (data && canRenameTable(data)) tableHeaderRename.startRename(tableId, data.name)
10051006
}, [tableHeaderRename.startRename, tableId])
10061007

10071008
const handleAddColumnOfType = (type: ColumnDefinition['type']) => {
@@ -1088,11 +1089,15 @@ export function Table({
10881089
}
10891090
: undefined,
10901091
dropdownItems: [
1091-
{
1092-
label: 'Rename',
1093-
icon: Pencil,
1094-
onClick: handleStartTableRename,
1095-
},
1092+
...(canRenameTable(tableData)
1093+
? [
1094+
{
1095+
label: 'Rename',
1096+
icon: Pencil,
1097+
onClick: handleStartTableRename,
1098+
},
1099+
]
1100+
: []),
10961101
// Reachable with the flag off when something is locked, so an
10971102
// admin can always clear locks (the route allows clearing).
10981103
...(!tableData.isVirtual &&
@@ -1106,12 +1111,16 @@ export function Table({
11061111
},
11071112
]
11081113
: []),
1109-
{
1110-
label: 'Delete',
1111-
icon: Trash,
1112-
onClick: onRequestDeleteTable,
1113-
disabled: userPermissions.canEdit !== true || tableData.locks.deleteLocked,
1114-
},
1114+
...(canMutateTable(tableData)
1115+
? [
1116+
{
1117+
label: 'Delete',
1118+
icon: Trash,
1119+
onClick: onRequestDeleteTable,
1120+
disabled: userPermissions.canEdit !== true || tableData.locks.deleteLocked,
1121+
},
1122+
]
1123+
: []),
11151124
],
11161125
}
11171126
: { label: '…', terminal: true },

apps/sim/app/workspace/[workspaceId]/tables/tables.tsx

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { generateId } from '@sim/utils/id'
1010
import { useParams, useRouter } from 'next/navigation'
1111
import { useQueryStates } from 'nuqs'
1212
import type { TableDefinition } from '@/lib/table'
13+
import { canMutateTable, canRenameTable } from '@/lib/table/capabilities'
1314
import { CSV_ASYNC_IMPORT_THRESHOLD_BYTES, generateUniqueTableName } from '@/lib/table/constants'
1415
import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state'
1516
import type {
@@ -1119,11 +1120,17 @@ export function Tables() {
11191120
onCopyId={() => {
11201121
if (activeTable) navigator.clipboard.writeText(activeTable.id)
11211122
}}
1122-
onDelete={() => setIsDeleteDialogOpen(true)}
1123-
onRename={() => {
1124-
if (activeTable) listRename.startRename(activeTable.id, activeTable.name)
1125-
}}
1126-
onImportCsv={() => setIsImportDialogOpen(true)}
1123+
onDelete={
1124+
activeTable && canMutateTable(activeTable) ? () => setIsDeleteDialogOpen(true) : undefined
1125+
}
1126+
onRename={
1127+
activeTable && canRenameTable(activeTable)
1128+
? () => listRename.startRename(activeTable.id, activeTable.name)
1129+
: undefined
1130+
}
1131+
onImportCsv={
1132+
activeTable && canMutateTable(activeTable) ? () => setIsImportDialogOpen(true) : undefined
1133+
}
11271134
onExportCsv={async () => {
11281135
if (!activeTable) return
11291136
try {
@@ -1135,8 +1142,10 @@ export function Tables() {
11351142
}}
11361143
onTogglePin={handleTogglePin}
11371144
pinned={activeTable ? pinnedTableIds.has(activeTable.id) : false}
1138-
onMove={canEdit ? handleMoveTable : undefined}
1139-
moveOptions={canEdit ? tableMoveOptions : undefined}
1145+
onMove={canEdit && activeTable && canMutateTable(activeTable) ? handleMoveTable : undefined}
1146+
moveOptions={
1147+
canEdit && activeTable && canMutateTable(activeTable) ? tableMoveOptions : undefined
1148+
}
11401149
disableDelete={!canEdit}
11411150
disableRename={!canEdit}
11421151
disableImport={!canEdit}

apps/sim/lib/table/__tests__/service-filter-threading.test.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,21 @@ import { decodeCursor } from '@/lib/table/rows/cursor'
1515
import { buildFilterClause, buildSortClause } from '@/lib/table/sql'
1616
import type { ColumnDefinition, TableDefinition } from '@/lib/table/types'
1717

18-
const { mockQueryVirtualTableRows } = vi.hoisted(() => ({
18+
const { mockFindVirtualTableRowMatches, mockQueryVirtualTableRows } = vi.hoisted(() => ({
19+
mockFindVirtualTableRowMatches: vi.fn(),
1920
mockQueryVirtualTableRows: vi.fn(),
2021
}))
2122

2223
vi.mock('@/lib/virtual-tables/service.server', () => ({
24+
findVirtualTableRowMatches: mockFindVirtualTableRowMatches,
2325
queryVirtualTableRows: mockQueryVirtualTableRows,
2426
}))
2527

2628
vi.mock('@/lib/table/sql', () => ({
2729
buildFilterClause: vi.fn(() => sql`true`),
2830
buildSortClause: vi.fn(() => sql`true`),
2931
buildPredicateClause: vi.fn(() => sql`true`),
32+
escapeLikePattern: vi.fn((value: string) => value),
3033
TableQueryValidationError: class TableQueryValidationError extends Error {},
3134
}))
3235

@@ -53,7 +56,12 @@ vi.mock('@/lib/table/validation', () => ({
5356
checkBatchUniqueConstraintsDb: vi.fn(async () => ({ valid: true, errors: [] })),
5457
}))
5558

56-
import { deleteRowsByFilter, queryRows, updateRowsByFilter } from '@/lib/table/rows/service'
59+
import {
60+
deleteRowsByFilter,
61+
findRowMatches,
62+
queryRows,
63+
updateRowsByFilter,
64+
} from '@/lib/table/rows/service'
5765

5866
const COLUMNS: ColumnDefinition[] = [
5967
{ name: 'name', type: 'string' },
@@ -175,6 +183,31 @@ describe('queryRows storage dispatch', () => {
175183
})
176184
})
177185

186+
describe('findRowMatches storage dispatch', () => {
187+
beforeEach(() => {
188+
vi.clearAllMocks()
189+
resetDbChainMock()
190+
})
191+
192+
it('searches only virtual storage for a virtual table', async () => {
193+
const virtualResult = {
194+
matches: [{ ordinal: 0, rowId: 'memory-1', column: 'transcript' }],
195+
truncated: false,
196+
}
197+
mockFindVirtualTableRowMatches.mockResolvedValueOnce(virtualResult)
198+
199+
await expect(
200+
findRowMatches({ ...TABLE, isVirtual: true }, { q: 'hello' }, 'req-1')
201+
).resolves.toBe(virtualResult)
202+
203+
expect(mockFindVirtualTableRowMatches).toHaveBeenCalledWith(
204+
expect.objectContaining({ id: TABLE.id, isVirtual: true }),
205+
{ q: 'hello' }
206+
)
207+
expect(dbChainMockFns.transaction).not.toHaveBeenCalled()
208+
})
209+
})
210+
178211
describe('bulk update/delete limited-subset ordering', () => {
179212
beforeEach(() => {
180213
vi.clearAllMocks()
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { canMutateTable, canRenameTable } from '@/lib/table/capabilities'
3+
4+
describe('canMutateTable', () => {
5+
it('prevents persisted-table actions for virtual tables', () => {
6+
expect(canMutateTable({ isVirtual: true })).toBe(false)
7+
})
8+
9+
it('allows persisted-table actions for stored tables', () => {
10+
expect(canMutateTable({ isVirtual: false })).toBe(true)
11+
expect(canMutateTable({})).toBe(true)
12+
})
13+
})
14+
15+
describe('canRenameTable', () => {
16+
it('prevents renaming virtual tables such as Memory', () => {
17+
expect(canRenameTable({ isVirtual: true })).toBe(false)
18+
})
19+
20+
it('allows renaming persisted tables', () => {
21+
expect(canRenameTable({ isVirtual: false })).toBe(true)
22+
expect(canRenameTable({})).toBe(true)
23+
})
24+
})

apps/sim/lib/table/capabilities.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import type { TableDefinition } from '@/lib/table/types'
2+
3+
/** Returns whether a table supports persisted mutation and management actions. */
4+
export function canMutateTable(table: Pick<TableDefinition, 'isVirtual'>): boolean {
5+
return table.isVirtual !== true
6+
}
7+
8+
/** Returns whether a table supports changing its persisted display name. */
9+
export function canRenameTable(table: Pick<TableDefinition, 'isVirtual'>): boolean {
10+
return canMutateTable(table)
11+
}

apps/sim/lib/table/rows/service.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,10 @@ import {
102102
validateRowSize,
103103
} from '@/lib/table/validation'
104104
import { cancelWorkflowGroupRuns, runWorkflowColumn } from '@/lib/table/workflow-columns'
105-
import { queryVirtualTableRows } from '@/lib/virtual-tables/service.server'
105+
import {
106+
findVirtualTableRowMatches,
107+
queryVirtualTableRows,
108+
} from '@/lib/virtual-tables/service.server'
106109

107110
const logger = createLogger('TableRowsService')
108111

@@ -866,6 +869,8 @@ export async function findRowMatches(
866869
options: { q: string; filter?: Filter; sort?: Sort },
867870
requestId: string
868871
): Promise<{ matches: FindRowMatch[]; truncated: boolean }> {
872+
if (table.isVirtual) return findVirtualTableRowMatches(table, options)
873+
869874
const tableName = USER_TABLE_ROWS_SQL_NAME
870875
const columns = table.schema.columns
871876
// Row data is keyed by stable column id, so scan/return JSONB keys as ids.

apps/sim/lib/virtual-tables/memory-virtual-table.server.test.ts

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,23 @@ import {
1111
import { sql as drizzleSql } from 'drizzle-orm'
1212
import { beforeEach, describe, expect, it, vi } from 'vitest'
1313

14-
const { mockBuildFilterClause, mockBuildPredicateClause, mockBuildSortClause } = vi.hoisted(() => ({
14+
const {
15+
mockBuildFilterClause,
16+
mockBuildPredicateClause,
17+
mockBuildSortClause,
18+
mockEscapeLikePattern,
19+
} = vi.hoisted(() => ({
1520
mockBuildFilterClause: vi.fn(() => ({ type: 'filter' })),
1621
mockBuildPredicateClause: vi.fn(() => ({ type: 'predicate' })),
1722
mockBuildSortClause: vi.fn(() => ({ type: 'sort' })),
23+
mockEscapeLikePattern: vi.fn((value: string) => value),
1824
}))
1925

2026
vi.mock('@/lib/table/sql', () => ({
2127
buildFilterClause: mockBuildFilterClause,
2228
buildPredicateClause: mockBuildPredicateClause,
2329
buildSortClause: mockBuildSortClause,
30+
escapeLikePattern: mockEscapeLikePattern,
2431
}))
2532

2633
vi.mock('drizzle-orm', () => {
@@ -47,6 +54,7 @@ vi.mock('drizzle-orm', () => {
4754
})
4855

4956
import {
57+
findMemoryTableRowMatches,
5058
getMemoryTableDefinition,
5159
queryMemoryTableRows,
5260
} from '@/lib/virtual-tables/memory-virtual-table.server'
@@ -258,6 +266,60 @@ describe('Memory virtual table', () => {
258266
)
259267
})
260268

269+
it('finds matching cells in one storage query and preserves filtered sort ordinals', async () => {
270+
dbChainMockFns.execute.mockResolvedValueOnce([
271+
{ ordinal: '4', id: 'memory-1', column_name: 'transcript' },
272+
{ ordinal: 7, id: 'memory-2', column_name: 'conversation_id' },
273+
])
274+
275+
await expect(
276+
findMemoryTableRowMatches({
277+
workspaceId: 'workspace-1',
278+
q: '50%_off',
279+
filter: { conversation_id: { $contains: 'customer' } },
280+
sort: { updated_at: 'asc' },
281+
})
282+
).resolves.toEqual({
283+
matches: [
284+
{ ordinal: 4, rowId: 'memory-1', column: 'transcript' },
285+
{ ordinal: 7, rowId: 'memory-2', column: 'conversation_id' },
286+
],
287+
truncated: false,
288+
})
289+
290+
expect(dbChainMockFns.execute).toHaveBeenCalledTimes(1)
291+
expect(mockEscapeLikePattern).toHaveBeenCalledWith('50%_off')
292+
expect(mockBuildFilterClause).toHaveBeenCalledWith(
293+
{ conversation_id: { $contains: 'customer' } },
294+
'memory_rows',
295+
expect.any(Array)
296+
)
297+
expect(mockBuildSortClause).toHaveBeenCalledWith(
298+
{ updated_at: 'asc' },
299+
'memory_rows',
300+
expect.any(Array)
301+
)
302+
const findSqlCall = vi
303+
.mocked(drizzleSql)
304+
.mock.calls.find(([strings]) => Array.from(strings).join('').includes('jsonb_each_text'))
305+
expect(findSqlCall).toBeDefined()
306+
})
307+
308+
it('caps storage matches and reports truncation', async () => {
309+
dbChainMockFns.execute.mockResolvedValueOnce(
310+
Array.from({ length: 1001 }, (_, index) => ({
311+
ordinal: index,
312+
id: `memory-${index}`,
313+
column_name: 'transcript',
314+
}))
315+
)
316+
317+
const result = await findMemoryTableRowMatches({ workspaceId: 'workspace-1', q: 'hello' })
318+
319+
expect(result).toMatchObject({ truncated: true })
320+
expect(result.matches).toHaveLength(1000)
321+
})
322+
261323
it('returns an empty page', async () => {
262324
queueTableRows(schemaMock.memory, [])
263325

0 commit comments

Comments
 (0)