Skip to content

Commit 22b3569

Browse files
authored
fix(files): order file folders in SQL like every other folder list (#6599)
The list use case defaulted to a name sort whose defaults were written for the new v2 contract, so the internal route — whose contract exposes no sort params — could no longer reach the repository's sortOrder ASC, createdAt ASC ordering. Surfaces that render the payload positionally (the @-mention Folders group, the add-resource search results, Copilot's list_file_folders) silently flipped from newest-first to alphabetical, and the Files browser's SSR prefetch hydrated a different order than its own refetch. Push the sort down to the query like the workflow, knowledge, and table folder lists already do, reusing FOLDER_SORTS. Omitting sortBy keeps the position ordering; v2 always sends one from its contract defaults. A name sort now also uses the database collation and the shared createdAt tiebreak instead of a JS comparator over UTF-16 code units.
1 parent 933eea5 commit 22b3569

5 files changed

Lines changed: 64 additions & 18 deletions

File tree

apps/sim/lib/api/contracts/v2/shared.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ import { FolderPathError, parseFolderPath, requireNonRootFolderPath } from '@/li
7272
*
7373
* Every one of these is pushed into SQL, except on `GET /skills` (which merges
7474
* the static builtin registry into the DB rows, then re-filters and re-sorts the
75-
* merged array) and `GET /files/folders` (which applies `parentPath`, `search`,
76-
* and the sort in JS). Both read a full result set to produce a page; neither is
77-
* a pattern to copy.
75+
* merged array) and `GET /files/folders` (which applies `parentPath` and `search`
76+
* in JS; its sort is pushed into SQL like every other folder list). Both read a
77+
* full result set to produce a page; neither is a pattern to copy.
7878
*
7979
* ## Which lists are paged
8080
*

apps/sim/lib/folders/queries.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ export async function resolveRestoredFolderId(
149149
* enum by `satisfies`. Each ends in `createdAt` so folders sharing a name or a
150150
* `sortOrder` still come back in a stable order.
151151
*/
152-
const FOLDER_SORTS = {
152+
export const FOLDER_SORTS = {
153153
position: [folder.sortOrder, folder.createdAt],
154154
name: [folder.name, folder.createdAt],
155155
createdAt: [folder.createdAt],

apps/sim/lib/uploads/contexts/workspace/workspace-file-folder-manager.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import { folder as folderTable, workspaceFiles, workspace as workspaceTable } fr
33
import { createLogger } from '@sim/logger'
44
import { getErrorMessage, getPostgresErrorCode } from '@sim/utils/errors'
55
import { generateId } from '@sim/utils/id'
6-
import { and, asc, eq, inArray, isNull, min, sql } from 'drizzle-orm'
6+
import { and, eq, inArray, isNull, min, sql } from 'drizzle-orm'
7+
import { type ListSortOrder, listOrderBy } from '@/lib/api/list-query'
78
import { OrchestrationError } from '@/lib/core/orchestration/types'
89
import type { DbOrTx } from '@/lib/db/types'
910
import { acquireFolderMutationLock } from '@/lib/folders/locks'
@@ -17,6 +18,7 @@ import {
1718
parseFolderPath,
1819
requireNonRootFolderPath,
1920
} from '@/lib/folders/paths'
21+
import { FOLDER_SORTS, type FolderSortBy } from '@/lib/folders/queries'
2022
import { collectDescendantFolderIds } from '@/lib/folders/subtree'
2123
import { encodeWorkspaceFileFolderDisplaySegment } from '@/lib/workspace-files/folder-display-path'
2224
import { MAX_WORKSPACE_FILE_BULK_AFFECTED_ITEMS } from '@/lib/workspace-files/limits'
@@ -383,11 +385,21 @@ export async function findWorkspaceFileFolderIdByPath(
383385
return parentId
384386
}
385387

388+
/**
389+
* Lists a workspace's file folders, ordered in the database like every other folder
390+
* list so a name sort uses the same collation and the same `createdAt` tiebreak.
391+
* Defaults to `position` — `sortOrder ASC, createdAt ASC` — which honours a user's
392+
* manual ordering and is what surfaces reading the payload positionally expect.
393+
*/
386394
export async function listWorkspaceFileFolders(
387395
workspaceId: string,
388-
options?: { scope?: WorkspaceFileFolderScope }
396+
options?: {
397+
scope?: WorkspaceFileFolderScope
398+
sortBy?: FolderSortBy
399+
sortOrder?: ListSortOrder
400+
}
389401
): Promise<WorkspaceFileFolderRecord[]> {
390-
const { scope = 'active' } = options ?? {}
402+
const { scope = 'active', sortBy = 'position', sortOrder = 'asc' } = options ?? {}
391403
const rows = await db
392404
.select()
393405
.from(folderTable)
@@ -406,7 +418,7 @@ export async function listWorkspaceFileFolders(
406418
isNull(folderTable.deletedAt)
407419
)
408420
)
409-
.orderBy(asc(folderTable.sortOrder), asc(folderTable.createdAt))
421+
.orderBy(...listOrderBy(FOLDER_SORTS[sortBy], sortOrder))
410422

411423
const paths = buildWorkspaceFileFolderPathMap(rows)
412424
return rows.map((row) => mapFolder(row, paths))

apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,38 @@ describe('workspace file folder operations', () => {
121121
expect(mockNotify).toHaveBeenCalledOnce()
122122
})
123123

124+
it.each([
125+
['leaves the sort unset so the repository keeps its position ordering', {}, undefined],
126+
['delegates an explicit sort', { sortBy: 'name', sortOrder: 'desc' } as const, 'name'],
127+
])('%s', async (_label, sortInput, expectedSortBy) => {
128+
mockList.mockResolvedValue([folder])
129+
130+
await listWorkspaceFileFoldersOperation.execute({
131+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
132+
input: { workspaceId: 'ws-1', ...sortInput },
133+
})
134+
135+
expect(mockList).toHaveBeenCalledWith(
136+
'ws-1',
137+
expect.objectContaining({ sortBy: expectedSortBy })
138+
)
139+
})
140+
141+
it('preserves the order the repository returned rather than re-sorting in memory', async () => {
142+
mockList.mockResolvedValue([
143+
{ ...folder, id: 'newest', name: 'zeta' },
144+
{ ...folder, id: 'middle', name: 'Alpha' },
145+
{ ...folder, id: 'oldest', name: 'beta' },
146+
])
147+
148+
const result = await listWorkspaceFileFoldersOperation.execute({
149+
principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
150+
input: { workspaceId: 'ws-1' },
151+
})
152+
153+
expect(result.folders.map((item) => item.id)).toEqual(['newest', 'middle', 'oldest'])
154+
})
155+
124156
it('matches a canonical encoded parent path against decoded stored folder paths', async () => {
125157
mockList.mockResolvedValue([
126158
{ ...folder, id: 'child-1', name: 'Q1', path: 'Reports & Plans/Q1' },

apps/sim/lib/workspace-files/application/workspace-file-folders.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { AuditAction, AuditResourceType } from '@sim/audit'
22
import { resolvePrincipalAttribution } from '@sim/auth/principal'
33
import { createLogger } from '@sim/logger'
4+
import type { ListSortOrder } from '@/lib/api/list-query'
45
import { OrchestrationError } from '@/lib/core/orchestration/types'
56
import { parseFolderPath } from '@/lib/folders/paths'
7+
import type { FolderSortBy } from '@/lib/folders/queries'
68
import { notifyWorkspaceFilesChanged } from '@/lib/realtime/notify'
79
import {
810
assertWorkspaceFileItemsBelongToWorkspace,
@@ -30,8 +32,14 @@ export interface ListWorkspaceFileFoldersInput {
3032
scope?: 'active' | 'archived' | 'all'
3133
parentPath?: string
3234
search?: string
33-
sortBy?: 'name' | 'createdAt' | 'updatedAt'
34-
sortOrder?: 'asc' | 'desc'
35+
/**
36+
* Only v2 sends a sort; the internal route, Copilot, and the VFS do not, and some of
37+
* their consumers render the payload in the order it arrives. So this stays optional
38+
* and undefined means "leave the repository's `position` ordering alone" — a default
39+
* applied here would silently reorder those surfaces.
40+
*/
41+
sortBy?: Exclude<FolderSortBy, 'position'>
42+
sortOrder?: ListSortOrder
3543
}
3644

3745
export interface ListWorkspaceFileFoldersResult {
@@ -115,6 +123,8 @@ async function executeListWorkspaceFileFolders(args: {
115123
}): Promise<ListWorkspaceFileFoldersResult> {
116124
let folders = await listWorkspaceFileFolders(args.context.workspaceId, {
117125
scope: args.input.scope,
126+
sortBy: args.input.sortBy,
127+
sortOrder: args.input.sortOrder,
118128
})
119129
if (args.input.parentPath !== undefined) {
120130
const parentSegments = parseFolderPath(args.input.parentPath)
@@ -128,14 +138,6 @@ async function executeListWorkspaceFileFolders(args: {
128138
const search = args.input.search.toLowerCase()
129139
folders = folders.filter((folder) => folder.name.toLowerCase().includes(search))
130140
}
131-
const sortBy = args.input.sortBy ?? 'name'
132-
const sortOrder = args.input.sortOrder ?? 'asc'
133-
folders.sort((left, right) => {
134-
const leftValue = left[sortBy]
135-
const rightValue = right[sortBy]
136-
const comparison = leftValue < rightValue ? -1 : leftValue > rightValue ? 1 : 0
137-
return sortOrder === 'asc' ? comparison : -comparison
138-
})
139141
return { folders }
140142
}
141143

0 commit comments

Comments
 (0)