Skip to content

Commit 40741fd

Browse files
committed
feat(folders): add the generic resourceType-driven folder engine
Folder create/update/delete/restore/reorder existed once per resource type in intent but only once in code, hardcoded to workflows. Extract a config-driven engine so knowledge bases and tables get folders from the same implementation instead of a fourth copy. - lib/folders/config.ts declares FOLDER_RESOURCES: the child table, its id/folderId/workspace/soft-delete columns, the TS property key backing the soft-delete column, membership scope, audit label, and cascade count key. Workflow-only behavior (archiving through the workflow lifecycle, the last-workflow delete guard, restoring schedules and webhooks) is declared as data on the workflow entry, not branched on. - lib/folders/cascade.ts performs archive/restore under one shared timestamp, in a fixed number of statements regardless of subtree depth. - lib/folders/lifecycle.ts is the single create/update/delete/restore implementation; the workflow perform* functions become thin wrappers so existing callers are untouched. - Widen the folder contract to workflow | knowledge_base | table and thread resourceType through all four routes and the React Query hooks, including the query keys so the three folder trees stop sharing a cache entry. - Move the folder cycle check out of lib/workflows/utils.ts: it was hardcoded to resourceType 'workflow' and could walk out of one resource's tree into another's.
1 parent 591702b commit 40741fd

24 files changed

Lines changed: 1721 additions & 725 deletions

File tree

apps/sim/app/api/folders/[id]/restore/route.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ import { restoreFolderContract } from '@/lib/api/contracts'
55
import { parseRequest } from '@/lib/api/server'
66
import { getSession } from '@/lib/auth'
77
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
8+
import { restoreFolder } from '@/lib/folders/lifecycle'
89
import { captureServerEvent } from '@/lib/posthog/server'
9-
import { performRestoreFolder } from '@/lib/workflows/orchestration/folder-lifecycle'
10+
import { statusForOrchestrationError } from '@/lib/workflows/orchestration/types'
1011
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1112

1213
const logger = createLogger('RestoreFolderAPI')
@@ -23,29 +24,36 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Route
2324
const parsed = await parseRequest(restoreFolderContract, request, context)
2425
if (!parsed.success) return parsed.response
2526
const { id: folderId } = parsed.data.params
26-
const { workspaceId } = parsed.data.body
27+
const { workspaceId, resourceType } = parsed.data.body
2728

2829
const permission = await getUserEntityPermissions(session.user.id, 'workspace', workspaceId)
2930
if (permission !== 'admin' && permission !== 'write') {
3031
return NextResponse.json({ error: 'Insufficient permissions' }, { status: 403 })
3132
}
3233

33-
const result = await performRestoreFolder({
34+
const result = await restoreFolder({
35+
resourceType,
3436
folderId,
3537
workspaceId,
3638
userId: session.user.id,
3739
})
3840

3941
if (!result.success) {
40-
return NextResponse.json({ error: result.error }, { status: 400 })
42+
return NextResponse.json(
43+
{ error: result.error },
44+
{ status: statusForOrchestrationError(result.errorCode) }
45+
)
4146
}
4247

43-
logger.info(`Restored folder ${folderId}`, { restoredItems: result.restoredItems })
48+
logger.info(`Restored folder ${folderId}`, {
49+
resourceType,
50+
restoredItems: result.restoredItems,
51+
})
4452

4553
captureServerEvent(
4654
session.user.id,
4755
'folder_restored',
48-
{ folder_id: folderId, workspace_id: workspaceId },
56+
{ folder_id: folderId, workspace_id: workspaceId, resource_type: resourceType },
4957
{ groups: { workspace: workspaceId } }
5058
)
5159

apps/sim/app/api/folders/[id]/route.test.ts

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,14 @@ import {
88
authMockFns,
99
createMockRequest,
1010
dbChainMockFns,
11+
foldersLifecycleMock,
12+
foldersLifecycleMockFns,
1113
type MockUser,
1214
permissionsMock,
1315
permissionsMockFns,
1416
queueTableRows,
1517
resetDbChainMock,
1618
schemaMock,
17-
workflowsOrchestrationMock,
18-
workflowsOrchestrationMockFns,
19-
workflowsUtilsMock,
20-
workflowsUtilsMockFns,
2119
} from '@sim/testing'
2220
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
2321

@@ -36,8 +34,11 @@ const { mockLogger } = vi.hoisted(() => {
3634
}
3735
})
3836

39-
const mockPerformDeleteFolder = workflowsOrchestrationMockFns.mockPerformDeleteFolder
40-
const mockPerformUpdateFolder = workflowsOrchestrationMockFns.mockPerformUpdateFolder
37+
const mockDeleteFolder = foldersLifecycleMockFns.mockDeleteFolder
38+
const mockUpdateFolder = foldersLifecycleMockFns.mockUpdateFolder
39+
40+
/** Parent ids the mocked engine treats as closing a cycle for the folder under test. */
41+
const cyclicParentIds = new Set<string>()
4142

4243
const mockGetUserEntityPermissions = permissionsMockFns.mockGetUserEntityPermissions
4344

@@ -48,8 +49,7 @@ vi.mock('@sim/logger', () => ({
4849
getRequestContext: () => undefined,
4950
}))
5051
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
51-
vi.mock('@/lib/workflows/orchestration', () => workflowsOrchestrationMock)
52-
vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock)
52+
vi.mock('@/lib/folders/lifecycle', () => foldersLifecycleMock)
5353

5454
import { DELETE, PUT } from '@/app/api/folders/[id]/route'
5555

@@ -101,25 +101,19 @@ describe('Individual Folder API Route', () => {
101101
resetDbChainMock()
102102

103103
mockGetUserEntityPermissions.mockResolvedValue('admin')
104-
mockPerformDeleteFolder.mockResolvedValue({
104+
mockDeleteFolder.mockResolvedValue({
105105
success: true,
106106
deletedItems: { folders: 1, workflows: 0 },
107107
})
108-
mockPerformUpdateFolder.mockImplementation(async (params) => {
108+
mockUpdateFolder.mockImplementation(async (params) => {
109109
if (params.parentId && params.parentId === params.folderId) {
110110
return {
111111
success: false,
112112
error: 'Folder cannot be its own parent',
113113
errorCode: 'validation',
114114
}
115115
}
116-
if (
117-
params.parentId &&
118-
(await workflowsUtilsMockFns.mockCheckForCircularReference(
119-
params.folderId,
120-
params.parentId
121-
))
122-
) {
116+
if (params.parentId && cyclicParentIds.has(params.parentId)) {
123117
return {
124118
success: false,
125119
error: 'Cannot create circular folder reference',
@@ -140,7 +134,7 @@ describe('Individual Folder API Route', () => {
140134
},
141135
}
142136
})
143-
workflowsUtilsMockFns.mockCheckForCircularReference.mockResolvedValue(false)
137+
cyclicParentIds.clear()
144138
})
145139

146140
describe('PUT /api/folders/[id]', () => {
@@ -368,7 +362,7 @@ describe('Individual Folder API Route', () => {
368362
workspaceId: 'workspace-123',
369363
})
370364

371-
workflowsUtilsMockFns.mockCheckForCircularReference.mockResolvedValue(true)
365+
cyclicParentIds.add('folder-1')
372366

373367
const req = createMockRequest('PUT', {
374368
name: 'Updated Folder 3',
@@ -382,9 +376,8 @@ describe('Individual Folder API Route', () => {
382376

383377
const data = await response.json()
384378
expect(data).toHaveProperty('error', 'Cannot create circular folder reference')
385-
expect(workflowsUtilsMockFns.mockCheckForCircularReference).toHaveBeenCalledWith(
386-
'folder-3',
387-
'folder-1'
379+
expect(mockUpdateFolder).toHaveBeenCalledWith(
380+
expect.objectContaining({ folderId: 'folder-3', parentId: 'folder-1' })
388381
)
389382
})
390383
})
@@ -405,7 +398,8 @@ describe('Individual Folder API Route', () => {
405398
const data = await response.json()
406399
expect(data).toHaveProperty('success', true)
407400
expect(data).toHaveProperty('deletedItems')
408-
expect(mockPerformDeleteFolder).toHaveBeenCalledWith({
401+
expect(mockDeleteFolder).toHaveBeenCalledWith({
402+
resourceType: 'workflow',
409403
folderId: 'folder-1',
410404
workspaceId: 'workspace-123',
411405
userId: TEST_USER.id,
@@ -458,7 +452,7 @@ describe('Individual Folder API Route', () => {
458452

459453
const data = await response.json()
460454
expect(data).toHaveProperty('success', true)
461-
expect(mockPerformDeleteFolder).toHaveBeenCalled()
455+
expect(mockDeleteFolder).toHaveBeenCalled()
462456
})
463457

464458
it('should allow folder deletion for admin permissions', async () => {
@@ -476,7 +470,7 @@ describe('Individual Folder API Route', () => {
476470

477471
const data = await response.json()
478472
expect(data).toHaveProperty('success', true)
479-
expect(mockPerformDeleteFolder).toHaveBeenCalled()
473+
expect(mockDeleteFolder).toHaveBeenCalled()
480474
})
481475

482476
it('should handle database errors during deletion', async () => {

apps/sim/app/api/folders/[id]/route.ts

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ import { createLogger } from '@sim/logger'
44
import { assertFolderMutable, FolderLockedError } from '@sim/platform-authz/workflow'
55
import { and, eq } from 'drizzle-orm'
66
import { type NextRequest, NextResponse } from 'next/server'
7-
import { updateFolderContract } from '@/lib/api/contracts'
7+
import { deleteFolderContract, updateFolderContract } from '@/lib/api/contracts'
88
import { parseRequest } from '@/lib/api/server'
99
import { getSession } from '@/lib/auth'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
11+
import { deleteFolder, updateFolder } from '@/lib/folders/lifecycle'
1112
import { toFolderApi } from '@/lib/folders/queries'
1213
import { captureServerEvent } from '@/lib/posthog/server'
13-
import { performDeleteFolder, performUpdateFolder } from '@/lib/workflows/orchestration'
1414
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1515

1616
/** Maps an orchestration errorCode to its HTTP status; mirrors the POST /api/folders route. */
@@ -47,13 +47,13 @@ export const PUT = withRouteHandler(
4747
if (!parsed.success) return parsed.response
4848

4949
const { id } = parsed.data.params
50+
const { resourceType } = parsed.data.query
5051
const { name, locked, parentId, sortOrder } = parsed.data.body
5152

52-
// Verify the folder exists
5353
const existingFolder = await db
5454
.select()
5555
.from(folderTable)
56-
.where(and(eq(folderTable.id, id), eq(folderTable.resourceType, 'workflow')))
56+
.where(and(eq(folderTable.id, id), eq(folderTable.resourceType, resourceType)))
5757
.then((rows) => rows[0])
5858

5959
if (!existingFolder) {
@@ -81,15 +81,19 @@ export const PUT = withRouteHandler(
8181
)
8282
}
8383

84-
const hasNonLockUpdate = Object.keys(parsed.data.body).some((key) => key !== 'locked')
85-
if (hasNonLockUpdate) {
86-
await assertFolderMutable(id)
87-
}
88-
if (parentId !== undefined) {
89-
await assertFolderMutable(parentId)
84+
// Folder locking is a workflow-only feature; other resource types leave `locked` false.
85+
if (resourceType === 'workflow') {
86+
const hasNonLockUpdate = Object.keys(parsed.data.body).some((key) => key !== 'locked')
87+
if (hasNonLockUpdate) {
88+
await assertFolderMutable(id)
89+
}
90+
if (parentId !== undefined) {
91+
await assertFolderMutable(parentId)
92+
}
9093
}
9194

92-
const result = await performUpdateFolder({
95+
const result = await updateFolder({
96+
resourceType,
9397
folderId: id,
9498
workspaceId: existingFolder.workspaceId,
9599
userId: session.user.id,
@@ -120,20 +124,22 @@ export const PUT = withRouteHandler(
120124

121125
// DELETE - Delete a folder and all its contents
122126
export const DELETE = withRouteHandler(
123-
async (request: NextRequest, { params }: { params: Promise<{ id: string }> }) => {
127+
async (request: NextRequest, context: { params: Promise<{ id: string }> }) => {
124128
try {
125129
const session = await getSession()
126130
if (!session?.user?.id) {
127131
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
128132
}
129133

130-
const { id } = await params
134+
const parsed = await parseRequest(deleteFolderContract, request, context)
135+
if (!parsed.success) return parsed.response
136+
const { id } = parsed.data.params
137+
const { resourceType } = parsed.data.query
131138

132-
// Verify the folder exists
133139
const existingFolder = await db
134140
.select()
135141
.from(folderTable)
136-
.where(and(eq(folderTable.id, id), eq(folderTable.resourceType, 'workflow')))
142+
.where(and(eq(folderTable.id, id), eq(folderTable.resourceType, resourceType)))
137143
.then((rows) => rows[0])
138144

139145
if (!existingFolder) {
@@ -153,9 +159,12 @@ export const DELETE = withRouteHandler(
153159
)
154160
}
155161

156-
await assertFolderMutable(id)
162+
if (resourceType === 'workflow') {
163+
await assertFolderMutable(id)
164+
}
157165

158-
const result = await performDeleteFolder({
166+
const result = await deleteFolder({
167+
resourceType,
159168
folderId: id,
160169
workspaceId: existingFolder.workspaceId,
161170
userId: session.user.id,
@@ -171,7 +180,7 @@ export const DELETE = withRouteHandler(
171180
captureServerEvent(
172181
session.user.id,
173182
'folder_deleted',
174-
{ workspace_id: existingFolder.workspaceId },
183+
{ workspace_id: existingFolder.workspaceId, resource_type: resourceType },
175184
{ groups: { workspace: existingFolder.workspaceId } }
176185
)
177186

apps/sim/app/api/folders/reorder/route.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,33 @@ describe('PUT /api/folders/reorder', () => {
6666
expect(data).toMatchObject({ success: true, updated: 1 })
6767
})
6868

69+
it('maps a sibling-name collision from a reparent to a 409', async () => {
70+
mockWhere
71+
.mockReturnValueOnce([{ id: 'folder-1', workspaceId: 'workspace-123' }])
72+
.mockReturnValueOnce([{ id: 'parent-1', workspaceId: 'workspace-123', archivedAt: null }])
73+
.mockReturnValueOnce([
74+
{ id: 'folder-1', parentId: null },
75+
{ id: 'parent-1', parentId: null },
76+
])
77+
78+
const uniqueViolation = Object.assign(new Error('duplicate key value'), { code: '23505' })
79+
mockDb.transaction.mockImplementationOnce(async () => {
80+
throw uniqueViolation
81+
})
82+
83+
const req = createMockRequest('PUT', {
84+
workspaceId: 'workspace-123',
85+
resourceType: 'knowledge_base',
86+
updates: [{ id: 'folder-1', sortOrder: 0, parentId: 'parent-1' }],
87+
})
88+
89+
const response = await PUT(req)
90+
91+
expect(response.status).toBe(409)
92+
const data = await response.json()
93+
expect(data.error).toBe('A folder with this name already exists in this location')
94+
})
95+
6996
it('rejects a parentId that belongs to another workspace', async () => {
7097
mockWhere
7198
.mockReturnValueOnce([{ id: 'folder-1', workspaceId: 'workspace-123' }])

0 commit comments

Comments
 (0)