Skip to content

Commit 829f3b8

Browse files
committed
fix(folders): scope the reorder route's cycle-detection graph correctly
The workspaceFolders query backing the ancestor-walk cycle check fetched every folder in the workspace regardless of resourceType or deletedAt. Folder ids are globally unique and the folder_parent_resource_type_match trigger prevents cross-resourceType parent links in valid data, but the unscoped query still let unrelated trees and soft-deleted nodes pollute the in-memory parentId map the walk traverses, risking a false "circular reference" rejection. Scoped to the batch's resourceType and active rows only, matching the same filters collectFolderSubtreeIds already applies for its own tree walk elsewhere in this file.
1 parent 80c40d7 commit 829f3b8

2 files changed

Lines changed: 37 additions & 1 deletion

File tree

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,36 @@ describe('PUT /api/folders/reorder', () => {
7777
expect(data).toMatchObject({ success: true, updated: 1 })
7878
})
7979

80+
it('scopes the cycle-detection graph to the batch resourceType and excludes soft-deleted folders', async () => {
81+
// Regression test: the workspaceFolders query used to build the cycle-check
82+
// graph previously fetched every folder in the workspace regardless of
83+
// resourceType or deletedAt -- unrelated trees and deleted nodes could
84+
// pollute the ancestor walk and produce a false "circular reference" error.
85+
mockWhere
86+
.mockReturnValueOnce([
87+
{ id: 'folder-1', workspaceId: 'workspace-123', resourceType: 'workflow' },
88+
])
89+
.mockReturnValueOnce([{ id: 'folder-1', parentId: null }])
90+
.mockReturnValueOnce([{ id: 'folder-1', workspaceId: 'workspace-123' }])
91+
92+
const req = createMockRequest('PUT', {
93+
workspaceId: 'workspace-123',
94+
updates: [{ id: 'folder-1', sortOrder: 2, parentId: null }],
95+
})
96+
97+
await PUT(req)
98+
99+
const workspaceFoldersCondition = mockWhere.mock.calls[1][0]
100+
expect(workspaceFoldersCondition).toMatchObject({
101+
type: 'and',
102+
conditions: expect.arrayContaining([
103+
{ type: 'eq', left: expect.anything(), right: 'workspace-123' },
104+
{ type: 'eq', left: expect.anything(), right: 'workflow' },
105+
{ type: 'isNull', column: expect.anything() },
106+
]),
107+
})
108+
})
109+
80110
it('rejects a parentId that belongs to another workspace, failing the whole batch', async () => {
81111
// Parent-id validity is checked once, inside `performReorderFolders`
82112
// (via `assertFolderParentValid`) — the route no longer re-implements

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,13 @@ export const PUT = withRouteHandler(async (req: NextRequest) => {
8989
const workspaceFolders = await db
9090
.select({ id: folder.id, parentId: folder.parentId })
9191
.from(folder)
92-
.where(eq(folder.workspaceId, workspaceId))
92+
.where(
93+
and(
94+
eq(folder.workspaceId, workspaceId),
95+
eq(folder.resourceType, resourceType),
96+
isNull(folder.deletedAt)
97+
)
98+
)
9399

94100
const parentById = new Map<string, string | null>()
95101
for (const folderRow of workspaceFolders) {

0 commit comments

Comments
 (0)