Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions apps/sim/app/api/v1/admin/workspaces/[id]/folders/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* @vitest-environment node
*/
import {
createMockRequest,
dbChainMockFns,
flattenMockConditions,
queueTableRows,
resetDbChainMock,
schemaMock,
} from '@sim/testing'
import { beforeEach, describe, expect, it, vi } from 'vitest'

/**
* The route composes `withAdminAuthParams`, so auth is bypassed by making that wrapper a
* passthrough — the assertions here are about query construction, not the auth gate.
*/
vi.mock('@/app/api/v1/admin/middleware', () => ({
withAdminAuthParams: (handler: unknown) => handler,
}))

import { GET } from '@/app/api/v1/admin/workspaces/[id]/folders/route'

const WORKSPACE_ID = 'ws-1'
const routeContext = { params: Promise.resolve({ id: WORKSPACE_ID }) }

function listRequest() {
return createMockRequest(
'GET',
undefined,
{},
`http://localhost:3000/api/v1/admin/workspaces/${WORKSPACE_ID}/folders?limit=50&offset=0`
)
}

describe('admin workspace folders GET', () => {
beforeEach(() => {
vi.clearAllMocks()
resetDbChainMock()
})

/**
* Both the count and the page must exclude soft-deleted folders. Without the filter an operator
* inspecting a workspace sees folders that live in Recently Deleted and an inflated total, and
* this endpoint disagrees with every user-facing folder list — all of which filter `deletedAt`.
*/
it('excludes soft-deleted folders from both the count and the page', async () => {
queueTableRows(schemaMock.workspace, [{ id: WORKSPACE_ID }])
queueTableRows(schemaMock.folder, [{ total: 0 }])
queueTableRows(schemaMock.folder, [])

await GET(listRequest(), routeContext)

// Calls: [0] workspace lookup, then the count and page share one prebuilt condition.
const folderWheres = dbChainMockFns.where.mock.calls.slice(1).map(([where]) => where)
expect(folderWheres.length).toBeGreaterThanOrEqual(2)
for (const where of folderWheres) {
// Asserted on the COLUMN: `resourceType`/`workspaceId` are eq nodes, so a bare
// "some isNull exists" check could pass on an unrelated clause.
expect(
flattenMockConditions(where).some(
(node) => node.type === 'isNull' && node.column === schemaMock.folder.deletedAt
)
).toBe(true)
}
})

it('still scopes to the workspace and to workflow folders', async () => {
queueTableRows(schemaMock.workspace, [{ id: WORKSPACE_ID }])
queueTableRows(schemaMock.folder, [{ total: 0 }])
queueTableRows(schemaMock.folder, [])

await GET(listRequest(), routeContext)

const where = dbChainMockFns.where.mock.calls[1]?.[0]
const nodes = flattenMockConditions(where)
expect(nodes.some((n) => n.type === 'eq' && n.right === WORKSPACE_ID)).toBe(true)
expect(nodes.some((n) => n.type === 'eq' && n.right === 'workflow')).toBe(true)
})
})
25 changes: 15 additions & 10 deletions apps/sim/app/api/v1/admin/workspaces/[id]/folders/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import { db } from '@sim/db'
import { folder as folderTable, workspace } from '@sim/db/schema'
import { createLogger } from '@sim/logger'
import { and, count, eq } from 'drizzle-orm'
import { and, count, eq, isNull } from 'drizzle-orm'
import { adminV1ListWorkspaceFoldersContract } from '@/lib/api/contracts/v1/admin'
import { parseRequest } from '@/lib/api/server'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
Expand Down Expand Up @@ -46,19 +46,24 @@ export const GET = withRouteHandler(
return notFoundResponse('Workspace')
}

/**
* Soft-deleted folders are excluded. Without this the count and the page both include rows
* sitting in Recently Deleted, so an operator inspecting a workspace sees phantom folders
* and an inflated total — and the two disagree with every user-facing folder list, all of
* which filter on `deletedAt`.
*/
const activeWorkflowFolders = and(
eq(folderTable.workspaceId, workspaceId),
eq(folderTable.resourceType, 'workflow'),
isNull(folderTable.deletedAt)
)

const [countResult, folders] = await Promise.all([
db
.select({ total: count() })
.from(folderTable)
.where(
and(eq(folderTable.workspaceId, workspaceId), eq(folderTable.resourceType, 'workflow'))
),
db.select({ total: count() }).from(folderTable).where(activeWorkflowFolders),
db
.select()
.from(folderTable)
.where(
and(eq(folderTable.workspaceId, workspaceId), eq(folderTable.resourceType, 'workflow'))
)
.where(activeWorkflowFolders)
.orderBy(folderTable.sortOrder, folderTable.name)
.limit(limit)
.offset(offset),
Expand Down
36 changes: 11 additions & 25 deletions apps/sim/lib/folders/cascade.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @vitest-environment node
*/
import { flattenMockConditions, hasMockCondition } from '@sim/testing'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import {
archiveFolderCascade,
Expand Down Expand Up @@ -81,23 +82,6 @@ function makeConfig(overrides: Partial<FolderResourceConfig> = {}): FolderResour
}
}

/** Flattens the nested `and(...)` objects the drizzle operator mocks produce. */
function flattenConditions(condition: unknown): Array<Record<string, unknown>> {
if (!condition || typeof condition !== 'object') return []
const node = condition as Record<string, unknown>
if (node.type === 'and' && Array.isArray(node.conditions)) {
return node.conditions.flatMap(flattenConditions)
}
return [node]
}

function hasCondition(
condition: unknown,
predicate: (node: Record<string, unknown>) => boolean
): boolean {
return flattenConditions(condition).some(predicate)
}

const TIMESTAMP = new Date('2026-01-01T00:00:00.000Z')
const NOW = new Date('2026-02-02T00:00:00.000Z')

Expand Down Expand Up @@ -138,7 +122,7 @@ describe('collectCascadeSubtreeIds', () => {

expect(ids).toEqual(['root', 'child', 'grandchild'])
// Either still active, or carrying this cascade's own stamp — never another snapshot's.
const clause = flattenConditions(selectCalls[0].where).find((node) => node.type === 'or')
const clause = flattenMockConditions(selectCalls[0].where).find((node) => node.type === 'or')
expect(clause).toBeDefined()
const branches = (clause?.conditions ?? []) as Array<Record<string, unknown>>
expect(branches.some((node) => node.type === 'isNull')).toBe(true)
Expand All @@ -150,8 +134,10 @@ describe('collectCascadeSubtreeIds', () => {

await collectCascadeSubtreeIds(tx, 'ws-1', 'knowledge_base', 'root', TIMESTAMP)

expect(hasCondition(selectCalls[0].where, (node) => node.right === 'knowledge_base')).toBe(true)
expect(hasCondition(selectCalls[0].where, (node) => node.right === 'ws-1')).toBe(true)
expect(hasMockCondition(selectCalls[0].where, (node) => node.right === 'knowledge_base')).toBe(
true
)
expect(hasMockCondition(selectCalls[0].where, (node) => node.right === 'ws-1')).toBe(true)
})
})

Expand All @@ -169,7 +155,7 @@ describe('collectArchivedSubtreeIds', () => {
const ids = await collectArchivedSubtreeIds(tx, 'ws-1', 'table', 'root', TIMESTAMP)

expect(ids).toEqual(['root', 'child'])
expect(hasCondition(selectCalls[0].where, (node) => node.right === TIMESTAMP)).toBe(true)
expect(hasMockCondition(selectCalls[0].where, (node) => node.right === TIMESTAMP)).toBe(true)
})

it('terminates on a parent cycle instead of recursing forever', async () => {
Expand Down Expand Up @@ -230,7 +216,7 @@ describe('archiveFolderCascade', () => {
await archiveFolderCascade(tx, makeConfig(), 'ws-1', ['root'], TIMESTAMP)

for (const call of updateCalls) {
expect(hasCondition(call.where, (node) => node.type === 'isNull')).toBe(true)
expect(hasMockCondition(call.where, (node) => node.type === 'isNull')).toBe(true)
}
})

Expand Down Expand Up @@ -299,7 +285,7 @@ describe('restoreFolderCascade', () => {
expect(updateCalls[1].set).toEqual({ archivedAt: null, updatedAt: NOW })
expect(updateCalls[2].table).toBe(DEPENDENT_TABLE)
expect(
hasCondition(updateCalls[2].where, (node) => {
hasMockCondition(updateCalls[2].where, (node) => {
return node.type === 'inArray' && Array.isArray(node.values) && node.values.length === 2
})
).toBe(true)
Expand Down Expand Up @@ -353,7 +339,7 @@ describe('restoreFolderCascade', () => {
)

for (const call of updateCalls) {
expect(hasCondition(call.where, (node) => node.right === TIMESTAMP)).toBe(true)
expect(hasMockCondition(call.where, (node) => node.right === TIMESTAMP)).toBe(true)
}
})
})
Expand All @@ -373,7 +359,7 @@ describe('restoreFolderRows', () => {

expect(folders).toBe(2)
expect(updateCalls).toHaveLength(1)
expect(hasCondition(updateCalls[0].where, (node) => node.right === TIMESTAMP)).toBe(true)
expect(hasMockCondition(updateCalls[0].where, (node) => node.right === TIMESTAMP)).toBe(true)
})
})

Expand Down
38 changes: 38 additions & 0 deletions apps/sim/lib/folders/lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
auditMock,
dbChainMock,
dbChainMockFns,
flattenMockConditions,
queueTableRows,
resetDbChainMock,
schemaMock,
Expand Down Expand Up @@ -230,6 +231,43 @@ describe('createFolder', () => {
expect(dbChainMockFns.values).toHaveBeenCalledWith(expect.objectContaining({ sortOrder: -3 }))
})

it('ignores soft-deleted folders and resources when picking the new sortOrder', async () => {
/**
* `min - 1` means archived rows would ratchet the floor further negative on every delete and
* never recover. Both minima must therefore see only rows a user can still see. Asserted on
* the WHERE clauses because the mock returns whatever is queued regardless of the filter, so
* an assertion on the resulting sortOrder alone would pass without either clause.
*/
setConfig({
resourceType: 'workflow',
countKey: 'workflows',
sortOrderColumn: 'child.sortOrder',
})
queueTableRows(schemaMock.folder, [{ minSortOrder: 0 }])
queueTableRows(CHILD_TABLE, [{ minSortOrder: 0 }])
dbChainMockFns.returning.mockResolvedValueOnce([folderRow({ sortOrder: -1 })])

await createFolder({ ...baseCreate, resourceType: 'workflow' })

const [folderWhere, childWhere] = dbChainMockFns.where.mock.calls
.slice(0, 2)
.map(([where]) => where)

// Assert on the specific COLUMN, not merely that some isNull exists: for a root folder the
// parent condition is itself `isNull(parentId)`, so a presence-only check passes with the
// soft-delete filter deleted. That made the first version of this test vacuous.
expect(
flattenMockConditions(folderWhere).some(
(node) => node.type === 'isNull' && node.column === schemaMock.folder.deletedAt
)
).toBe(true)
expect(
flattenMockConditions(childWhere).some(
(node) => node.type === 'isNull' && node.column === 'child.archivedAt'
)
).toBe(true)
})

it('starts at zero when the folder is the first thing in its location', async () => {
queueTableRows(schemaMock.folder, [{ minSortOrder: null }])
dbChainMockFns.returning.mockResolvedValueOnce([folderRow()])
Expand Down
11 changes: 10 additions & 1 deletion apps/sim/lib/folders/lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,22 @@ export async function nextFolderSortOrder(
? eq(folderTable.parentId, parentId)
: isNull(folderTable.parentId)

/**
* Soft-deleted rows are excluded from both minima. This function returns `min - 1` to put a
* new folder at the top, so counting archived rows lets every delete ratchet the floor further
* negative and never recover — an archived folder at -400 forces the next new folder to -401
* forever. Only rows a user can actually see should influence the ordering. The Files path
* (`workspace-file-folder-manager`) has always filtered this way.
*/
const folderMinPromise = tx
.select({ minSortOrder: min(folderTable.sortOrder) })
.from(folderTable)
.where(
and(
eq(folderTable.workspaceId, workspaceId),
eq(folderTable.resourceType, resourceType),
folderParentCondition
folderParentCondition,
isNull(folderTable.deletedAt)
)
)

Expand All @@ -147,6 +155,7 @@ export async function nextFolderSortOrder(
and(
eq(config.workspaceColumn, workspaceId),
parentId ? eq(config.folderIdColumn, parentId) : isNull(config.folderIdColumn),
isNull(config.deletedColumn),
config.scope
)
)
Expand Down
Loading
Loading