Skip to content

Commit 79e6451

Browse files
j15zclaude
andcommitted
fix(tables): refuse to delete a table's last saved view
The sibling check and the delete share the views advisory lock, so racing deletes cannot drop a live table to zero views and regress it to the legacy "All"-only state. Views of a hard-deleted table are removed by the FK cascade, which this guard never sees. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2a379fa commit 79e6451

4 files changed

Lines changed: 58 additions & 16 deletions

File tree

apps/sim/app/api/table/[tableId]/views/[viewId]/route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ export const DELETE = withRouteHandler(
9999

100100
return NextResponse.json({ success: true, data: { deleted: true } })
101101
} catch (error) {
102+
if (error instanceof TableViewValidationError) {
103+
return NextResponse.json({ error: error.message }, { status: 400 })
104+
}
102105
logger.error(`[${requestId}] Error deleting table view:`, error)
103106
return NextResponse.json({ error: 'Failed to delete view' }, { status: 500 })
104107
}

apps/sim/lib/table/application/views.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,13 @@ export const deleteTableViewUseCase = defineAuthorizedTableUseCase({
180180
context.workspaceId
181181
)
182182
if (!existing) throw new OrchestrationError('not_found', 'View not found')
183-
const deleted = await deleteTableView(input.viewId, context.table.id, context.workspaceId)
184-
if (!deleted) throw new OrchestrationError('not_found', 'View not found')
185-
return { viewId: input.viewId, viewName: existing.name, table: context.table }
183+
try {
184+
const deleted = await deleteTableView(input.viewId, context.table.id, context.workspaceId)
185+
if (!deleted) throw new OrchestrationError('not_found', 'View not found')
186+
return { viewId: input.viewId, viewName: existing.name, table: context.table }
187+
} catch (error) {
188+
rethrowViewError(error)
189+
}
186190
},
187191
projectAudit({ result }) {
188192
return {

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ describe('table-view mutations signal collaborators', () => {
227227
})
228228

229229
it('deleteTableView signals when a row was actually deleted', async () => {
230+
queueTableRows(tableViews, [{ id: 'view-1' }, { id: 'view-2' }]) // the in-lock sibling check
230231
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'view-1' }])
231232

232233
const deleted = await deleteTableView('view-1', 'table-1')
@@ -237,11 +238,22 @@ describe('table-view mutations signal collaborators', () => {
237238
})
238239

239240
it('deleteTableView does NOT signal when nothing was deleted', async () => {
240-
dbChainMockFns.returning.mockResolvedValueOnce([])
241+
queueTableRows(tableViews, [{ id: 'view-1' }])
241242

242243
const deleted = await deleteTableView('missing', 'table-1')
243244

244245
expect(deleted).toBe(false)
246+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
247+
expect(mockSignalTableViewsChanged).not.toHaveBeenCalled()
248+
})
249+
250+
it('deleteTableView refuses to delete the last remaining view', async () => {
251+
queueTableRows(tableViews, [{ id: 'view-1' }])
252+
253+
await expect(deleteTableView('view-1', 'table-1')).rejects.toThrow(
254+
'A table must keep at least one saved view'
255+
)
256+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
245257
expect(mockSignalTableViewsChanged).not.toHaveBeenCalled()
246258
})
247259
})

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

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -621,27 +621,50 @@ export async function updateTableView(data: UpdateTableViewData): Promise<TableV
621621
return toTableView(outcome.row, data.columns)
622622
}
623623

624-
/** Deleting the default simply leaves the table on "All". */
624+
/**
625+
* Deleting the default while siblings remain simply leaves the table on "All".
626+
* The last remaining view is not deletable: a table that has views keeps at
627+
* least one, so it can never regress to the legacy zero-view state. Views of a
628+
* hard-deleted table are removed by the FK cascade, never through this path.
629+
* The sibling check and the delete share the views lock, so two racing deletes
630+
* cannot both observe a sibling and drop the table to zero.
631+
*/
625632
export async function deleteTableView(
626633
viewId: string,
627634
tableId: string,
628635
workspaceId?: string
629636
): Promise<boolean> {
630-
const deleted = await db
631-
.delete(tableViews)
632-
.where(
633-
and(
634-
eq(tableViews.id, viewId),
635-
eq(tableViews.tableId, tableId),
636-
workspaceId ? eq(tableViews.workspaceId, workspaceId) : undefined
637+
const deleted = await withTableViewsLock(tableId, async (trx) => {
638+
const siblings = await trx
639+
.select({ id: tableViews.id })
640+
.from(tableViews)
641+
.where(
642+
and(
643+
eq(tableViews.tableId, tableId),
644+
workspaceId ? eq(tableViews.workspaceId, workspaceId) : undefined
645+
)
637646
)
638-
)
639-
.returning({ id: tableViews.id })
647+
if (!siblings.some((view) => view.id === viewId)) return false
648+
if (siblings.length === 1) {
649+
throw new TableViewValidationError('A table must keep at least one saved view')
650+
}
651+
const result = await trx
652+
.delete(tableViews)
653+
.where(
654+
and(
655+
eq(tableViews.id, viewId),
656+
eq(tableViews.tableId, tableId),
657+
workspaceId ? eq(tableViews.workspaceId, workspaceId) : undefined
658+
)
659+
)
660+
.returning({ id: tableViews.id })
661+
return result.length > 0
662+
})
640663

641-
if (deleted.length > 0) {
664+
if (deleted) {
642665
logger.info('Deleted table view', { tableId, viewId })
643666
// Only signal a real deletion — a missing view (nothing deleted) changed nothing.
644667
signalTableViewsChanged(tableId)
645668
}
646-
return deleted.length > 0
669+
return deleted
647670
}

0 commit comments

Comments
 (0)