Skip to content

Commit 3ad97f0

Browse files
committed
fix(folders): gate the orphan fallback on a resolved folder index, not a loading flag
The URL heal was fixed to use `isSuccess && !isPlaceholderData`, but the list filters that decide "this resource's folderId does not resolve, so show it at the root" were left on `isLoading` — the same footgun, one layer down. `isLoading` is false for a disabled query (no workspaceId), false for an errored one, and — because `useFolders` sets `keepPreviousData` — false while the previous workspace's folders are still on screen during a switch. In all three the index is empty or belongs to another workspace, so every foldered knowledge base and table was treated as an orphan and dragged to the root, or filtered out of a deep-linked folder entirely. A transient fetch failure was enough to make a folder look empty. `useFolderNavigation` now exposes `foldersResolved` instead of `isLoading`, so the heal and both list filters share one signal and the footgun is no longer reachable from the hook's surface.
1 parent 7f0d935 commit 3ad97f0

3 files changed

Lines changed: 34 additions & 27 deletions

File tree

apps/sim/app/workspace/[workspaceId]/components/folders/use-folder-navigation.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,16 @@ export interface FolderNavigation {
2929
/** Every active folder in this resource's tree, as returned by the folders API. */
3030
folders: WorkflowFolder[]
3131
folderById: Map<string, WorkflowFolder>
32-
isLoading: boolean
32+
/**
33+
* Whether `folders`/`folderById` can be trusted to be the COMPLETE set for this workspace.
34+
*
35+
* Deliberately exposed instead of `isLoading`, which is a footgun here: it is false for a
36+
* disabled query (no `workspaceId`), false for an errored one, and — because `useFolders`
37+
* sets `keepPreviousData` — false while the previous workspace's folders are still on screen
38+
* during a switch. A caller deciding "this resource's `folderId` does not resolve, so treat it
39+
* as an orphan" off `isLoading` would dump every foldered row at the root in all three.
40+
*/
41+
foldersResolved: boolean
3342
}
3443

3544
const EMPTY_FOLDERS: WorkflowFolder[] = []
@@ -54,7 +63,6 @@ export function useFolderNavigation({
5463

5564
const {
5665
data: folders = EMPTY_FOLDERS,
57-
isLoading,
5866
isSuccess,
5967
isPlaceholderData,
6068
} = useFolders(workspaceId, { resourceType })
@@ -67,7 +75,7 @@ export function useFolderNavigation({
6775
* folders during a workspace switch. In all three the list is empty or stale, and healing
6876
* off it would throw away a perfectly good folder.
6977
*/
70-
const foldersAreAuthoritative = isSuccess && !isPlaceholderData
78+
const foldersResolved = isSuccess && !isPlaceholderData
7179

7280
const setCurrentFolderId = useCallback(
7381
(folderId: string | null) => {
@@ -92,18 +100,19 @@ export function useFolderNavigation({
92100
* and upload actions keep targeting that id, so a new resource is filed somewhere nothing
93101
* can reach.
94102
*
95-
* Waits for `isLoading` so an empty index mid-fetch never evicts a perfectly good id.
103+
* Gated on {@link FolderNavigation.foldersResolved} so an empty or stale index never evicts a
104+
* perfectly good id.
96105
*/
97106
useEffect(() => {
98-
if (!foldersAreAuthoritative || !currentFolderId || folderById.has(currentFolderId)) return
107+
if (!foldersResolved || !currentFolderId || folderById.has(currentFolderId)) return
99108
/**
100109
* `history: 'replace'`, overriding the `push` these params default to. Opening a folder is
101110
* a navigation and belongs in the back stack; correcting a URL that never pointed anywhere
102111
* is not. Pushing here strands the user: Back returns to the dead `?folderId=`, which heals
103112
* and pushes again, so Back never escapes the page.
104113
*/
105114
void setFolderParams({ folderId: null }, { history: 'replace' })
106-
}, [foldersAreAuthoritative, currentFolderId, folderById, setFolderParams])
115+
}, [foldersResolved, currentFolderId, folderById, setFolderParams])
107116

108117
const breadcrumbs = useMemo(() => {
109118
if (!currentFolderId) return EMPTY_FOLDERS
@@ -135,6 +144,6 @@ export function useFolderNavigation({
135144
breadcrumbs,
136145
folders,
137146
folderById,
138-
isLoading,
147+
foldersResolved,
139148
}
140149
}

apps/sim/app/workspace/[workspaceId]/knowledge/knowledge.tsx

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -193,17 +193,11 @@ export function Knowledge() {
193193
const { mutateAsync: updateKnowledgeBaseMutation } = useUpdateKnowledgeBase(workspaceId)
194194
const { mutateAsync: deleteKnowledgeBaseMutation } = useDeleteKnowledgeBase(workspaceId)
195195

196-
const {
197-
currentFolderId,
198-
setCurrentFolderId,
199-
breadcrumbs,
200-
folders,
201-
folderById,
202-
isLoading: foldersLoading,
203-
} = useFolderNavigation({
204-
resourceType: FOLDER_RESOURCE_TYPE,
205-
workspaceId,
206-
})
196+
const { currentFolderId, setCurrentFolderId, breadcrumbs, folders, folderById, foldersResolved } =
197+
useFolderNavigation({
198+
resourceType: FOLDER_RESOURCE_TYPE,
199+
workspaceId,
200+
})
207201

208202
const createFolder = useCreateFolder()
209203
const updateFolder = useUpdateFolder()
@@ -432,13 +426,15 @@ export function Knowledge() {
432426
* A `folderId` that no longer names an active folder — a base restored on its own out of
433427
* Recently Deleted while its folder stayed archived, or a cascade that failed partway —
434428
* would otherwise match no level at all and leave the base unreachable from every view.
435-
* Fall it back to the root instead. Skipped while the folder list is still loading, when
436-
* an empty index would transiently drag every base to the root.
429+
* Fall it back to the root instead — but only once `foldersResolved` says the index is the
430+
* complete set for THIS workspace. Gating on a loading flag instead would treat an errored
431+
* fetch, a disabled query, or the previous workspace's cached folders as "no such folder"
432+
* and drag every foldered base to the root.
437433
*/
438434
let result = filterKnowledgeBases(knowledgeBases, debouncedSearchQuery).filter((kb) => {
439435
const folderId = kb.folderId ?? null
440436
const effectiveFolderId =
441-
foldersLoading || !folderId || folderById.has(folderId) ? folderId : null
437+
!foldersResolved || !folderId || folderById.has(folderId) ? folderId : null
442438
return effectiveFolderId === currentFolderId
443439
})
444440

@@ -507,7 +503,7 @@ export function Knowledge() {
507503
knowledgeBases,
508504
currentFolderId,
509505
folderById,
510-
foldersLoading,
506+
foldersResolved,
511507
debouncedSearchQuery,
512508
connectorFilter,
513509
contentFilter,

apps/sim/app/workspace/[workspaceId]/tables/tables.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export function Tables() {
127127
breadcrumbs: folderChain,
128128
folders,
129129
folderById,
130-
isLoading: foldersLoading,
130+
foldersResolved,
131131
} = useFolderNavigation({
132132
resourceType: 'table',
133133
workspaceId,
@@ -302,13 +302,15 @@ export function Tables() {
302302
* A `folderId` that no longer names an active folder — restored on its own out
303303
* of Recently Deleted while its folder stayed archived — would otherwise match
304304
* no level at all and leave the table unreachable from every view. Fall it back
305-
* to the root instead. Skipped while the folder list is still loading, when an
306-
* empty index would transiently drag every table to the root.
305+
* to the root instead — but only once `foldersResolved` says the index is the complete
306+
* set for THIS workspace. Gating on a loading flag instead would treat an errored fetch,
307+
* a disabled query, or the previous workspace's cached folders as "no such folder" and
308+
* drag every foldered table to the root.
307309
*/
308310
let result = tables.filter((t) => {
309311
const folderId = t.folderId ?? null
310312
const effectiveFolderId =
311-
foldersLoading || !folderId || folderById.has(folderId) ? folderId : null
313+
!foldersResolved || !folderId || folderById.has(folderId) ? folderId : null
312314
return effectiveFolderId === currentFolderId
313315
})
314316
if (query) result = result.filter((t) => t.name.toLowerCase().includes(query))
@@ -361,7 +363,7 @@ export function Tables() {
361363
tables,
362364
currentFolderId,
363365
folderById,
364-
foldersLoading,
366+
foldersResolved,
365367
debouncedSearchTerm,
366368
rowCountFilter,
367369
ownerFilter,

0 commit comments

Comments
 (0)