Skip to content

Commit 92848f2

Browse files
committed
address comments
1 parent 90dc147 commit 92848f2

11 files changed

Lines changed: 402 additions & 51 deletions

File tree

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { parseRequest } from '@/lib/api/server'
1111
import { getSession } from '@/lib/auth'
1212
import { generateRequestId } from '@/lib/core/utils/request'
1313
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
14+
import type { DbOrTx } from '@/lib/db/types'
1415
import { duplicateWorkflow } from '@/lib/workflows/persistence/duplicate'
1516
import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
1617

@@ -216,10 +217,8 @@ export const POST = withRouteHandler(
216217
}
217218
)
218219

219-
type FolderDuplicateTransaction = Parameters<Parameters<typeof db.transaction>[0]>[0]
220-
221220
async function assertTargetParentFolderMutable(
222-
tx: FolderDuplicateTransaction,
221+
tx: DbOrTx,
223222
parentId: string | null,
224223
targetWorkspaceId: string,
225224
sourceFolderId: string
@@ -256,7 +255,7 @@ async function assertTargetParentFolderMutable(
256255
}
257256

258257
async function deduplicateFolderName(
259-
tx: FolderDuplicateTransaction,
258+
tx: DbOrTx,
260259
workspaceId: string,
261260
parentId: string | null,
262261
requestedName: string
@@ -287,7 +286,7 @@ async function deduplicateFolderName(
287286
}
288287

289288
async function duplicateFolderStructure(
290-
tx: FolderDuplicateTransaction,
289+
tx: DbOrTx,
291290
sourceFolderId: string,
292291
newParentFolderId: string,
293292
sourceWorkspaceId: string,
@@ -339,7 +338,7 @@ async function duplicateFolderStructure(
339338
}
340339

341340
async function duplicateWorkflowsInFolderTree(
342-
tx: FolderDuplicateTransaction,
341+
tx: DbOrTx,
343342
sourceWorkspaceId: string,
344343
targetWorkspaceId: string,
345344
folderMapping: Map<string, string>,

apps/sim/app/api/workflows/[id]/deploy/route.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ export const GET = withRouteHandler(
3939
if (error) {
4040
return createErrorResponse(error.message, error.status)
4141
}
42-
await assertWorkflowMutable(id)
4342

4443
if (!workflowData.isDeployed) {
4544
logger.info(`[${requestId}] Workflow is not deployed: ${id}`)

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ import { PreviewWorkflow } from '@/app/workspace/[workspaceId]/w/components/prev
4949
import { getBlock } from '@/blocks/registry'
5050
import type { SubBlockType } from '@/blocks/types'
5151
import { useFolderMap } from '@/hooks/queries/folders'
52-
import { isFolderOrAncestorLocked } from '@/hooks/queries/utils/folder-locks'
52+
import { isFolderOrAncestorLocked } from '@/hooks/queries/utils/folder-tree'
5353
import { useWorkflowMap, useWorkflowState } from '@/hooks/queries/workflows'
5454
import { useCollaborativeWorkflow } from '@/hooks/use-collaborative-workflow'
5555
import { usePanelEditorStore } from '@/stores/panel'

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ import {
7373
useCopilotChats,
7474
} from '@/hooks/queries/copilot-chats'
7575
import { useFolderMap } from '@/hooks/queries/folders'
76-
import { isFolderOrAncestorLocked } from '@/hooks/queries/utils/folder-locks'
76+
import { isFolderOrAncestorLocked } from '@/hooks/queries/utils/folder-tree'
7777
import { useDuplicateWorkflowMutation, useWorkflowMap } from '@/hooks/queries/workflows'
7878
import { useCollaborativeWorkflow } from '@/hooks/use-collaborative-workflow'
7979
import { usePermissionConfig } from '@/hooks/use-permission-config'

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,10 @@ import { isAnnotationOnlyBlock } from '@/executor/constants'
8080
import { useWorkspaceEnvironment } from '@/hooks/queries/environment'
8181
import { useFolderMap } from '@/hooks/queries/folders'
8282
import { useAutoConnect, useSnapToGridSize } from '@/hooks/queries/general-settings'
83-
import { isFolderOrAncestorLocked } from '@/hooks/queries/utils/folder-locks'
83+
import {
84+
findLockedAncestorFolder,
85+
isFolderOrAncestorLocked,
86+
} from '@/hooks/queries/utils/folder-tree'
8487
import { useUpdateWorkflow, useWorkflowMap } from '@/hooks/queries/workflows'
8588
import { useCanvasViewport } from '@/hooks/use-canvas-viewport'
8689
import { useCollaborativeWorkflow } from '@/hooks/use-collaborative-workflow'
@@ -1240,35 +1243,64 @@ const WorkflowContent = React.memo(
12401243
}
12411244
}, [activeWorkflowId, clearLockNotification])
12421245

1246+
/**
1247+
* Locate the folder ancestor that supplies the inherited lock so the
1248+
* notification can name it. Null when the lock is row/block-level instead.
1249+
*/
1250+
const inheritedLockFolderName = useMemo(() => {
1251+
if (!workflowFolderLocked) return null
1252+
return findLockedAncestorFolder(workflowMetadata?.folderId, folders)?.name ?? null
1253+
}, [workflowFolderLocked, workflowMetadata?.folderId, folders])
1254+
12431255
const prevCanAdminRef = useRef(effectivePermissions.canAdmin)
1256+
const prevLockSignatureRef = useRef<string | null>(null)
12441257
useEffect(() => {
12451258
if (!isWorkflowReady) return
12461259

12471260
const canAdminChanged = prevCanAdminRef.current !== effectivePermissions.canAdmin
12481261
prevCanAdminRef.current = effectivePermissions.canAdmin
12491262

1250-
// Clear stale notification when admin status changes so it recreates with correct message
1251-
if (canAdminChanged) {
1263+
const lockSignature = workflowReadOnly
1264+
? `${workflowRowLocked ? 'row' : workflowFolderLocked ? `folder:${inheritedLockFolderName ?? ''}` : 'blocks'}`
1265+
: null
1266+
const lockSignatureChanged = prevLockSignatureRef.current !== lockSignature
1267+
prevLockSignatureRef.current = lockSignature
1268+
1269+
if (canAdminChanged || lockSignatureChanged) {
12521270
clearLockNotification()
12531271
}
12541272

12551273
if (workflowReadOnly) {
12561274
if (lockNotificationIdRef.current) return
12571275

12581276
const isAdmin = effectivePermissions.canAdmin
1277+
const isFolderInherited = workflowFolderLocked && !workflowRowLocked
1278+
const message = isFolderInherited
1279+
? inheritedLockFolderName
1280+
? `This workflow is locked by folder "${inheritedLockFolderName}"`
1281+
: 'This workflow is locked by a parent folder'
1282+
: isAdmin
1283+
? 'This workflow is locked'
1284+
: 'This workflow is locked. Ask an admin to unlock it.'
1285+
1286+
const showInlineUnlock = isAdmin && !isFolderInherited
1287+
12591288
lockNotificationIdRef.current = addNotification({
12601289
level: 'info',
1261-
message: isAdmin
1262-
? 'This workflow is locked'
1263-
: 'This workflow is locked. Ask an admin to unlock it.',
1290+
message,
12641291
workflowId: activeWorkflowId || undefined,
1265-
...(isAdmin ? { action: { type: 'unlock-workflow' as const, message: '' } } : {}),
1292+
...(showInlineUnlock
1293+
? { action: { type: 'unlock-workflow' as const, message: '' } }
1294+
: {}),
12661295
})
12671296
} else {
12681297
clearLockNotification()
12691298
}
12701299
}, [
12711300
workflowReadOnly,
1301+
workflowRowLocked,
1302+
workflowFolderLocked,
1303+
inheritedLockFolderName,
12721304
isWorkflowReady,
12731305
effectivePermissions.canAdmin,
12741306
addNotification,
@@ -1291,6 +1323,8 @@ const WorkflowContent = React.memo(
12911323
return
12921324
}
12931325

1326+
if (workflowFolderLocked) return
1327+
12941328
const currentBlocks = useWorkflowStore.getState().blocks
12951329
const ids = getWorkflowLockToggleIds(currentBlocks, false)
12961330
if (ids.length > 0) collaborativeBatchToggleLocked(ids)
@@ -1302,6 +1336,7 @@ const WorkflowContent = React.memo(
13021336
activeWorkflowId,
13031337
collaborativeBatchToggleLocked,
13041338
updateWorkflowMutation,
1339+
workflowFolderLocked,
13051340
workflowRowLocked,
13061341
workspaceId,
13071342
])

apps/sim/hooks/queries/utils/folder-locks.ts

Lines changed: 0 additions & 25 deletions
This file was deleted.
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import {
6+
findLockedAncestorFolder,
7+
getFolderPath,
8+
isFolderOrAncestorLocked,
9+
} from '@/hooks/queries/utils/folder-tree'
10+
import type { WorkflowFolder } from '@/stores/folders/types'
11+
12+
function makeFolder(overrides: Partial<WorkflowFolder> & { id: string }): WorkflowFolder {
13+
return {
14+
id: overrides.id,
15+
name: overrides.name ?? overrides.id,
16+
userId: 'user-1',
17+
workspaceId: 'ws-1',
18+
parentId: overrides.parentId ?? null,
19+
color: '#000000',
20+
isExpanded: false,
21+
locked: overrides.locked ?? false,
22+
sortOrder: 0,
23+
createdAt: new Date(0),
24+
updatedAt: new Date(0),
25+
archivedAt: null,
26+
}
27+
}
28+
29+
describe('isFolderOrAncestorLocked', () => {
30+
it('returns false for root', () => {
31+
expect(isFolderOrAncestorLocked(null, {})).toBe(false)
32+
expect(isFolderOrAncestorLocked(undefined, {})).toBe(false)
33+
})
34+
35+
it('returns true when the folder itself is locked', () => {
36+
const folders = { f1: makeFolder({ id: 'f1', locked: true }) }
37+
expect(isFolderOrAncestorLocked('f1', folders)).toBe(true)
38+
})
39+
40+
it('returns true when an ancestor is locked', () => {
41+
const folders = {
42+
f1: makeFolder({ id: 'f1', locked: true }),
43+
f2: makeFolder({ id: 'f2', parentId: 'f1' }),
44+
f3: makeFolder({ id: 'f3', parentId: 'f2' }),
45+
}
46+
expect(isFolderOrAncestorLocked('f3', folders)).toBe(true)
47+
})
48+
49+
it('returns false when nothing in the chain is locked', () => {
50+
const folders = {
51+
f1: makeFolder({ id: 'f1' }),
52+
f2: makeFolder({ id: 'f2', parentId: 'f1' }),
53+
}
54+
expect(isFolderOrAncestorLocked('f2', folders)).toBe(false)
55+
})
56+
57+
it('short-circuits on cycles instead of looping forever', () => {
58+
const folders = {
59+
f1: makeFolder({ id: 'f1', parentId: 'f2' }),
60+
f2: makeFolder({ id: 'f2', parentId: 'f1' }),
61+
}
62+
expect(isFolderOrAncestorLocked('f1', folders)).toBe(false)
63+
})
64+
})
65+
66+
describe('getFolderPath', () => {
67+
it('returns null for root or unknown folders', () => {
68+
expect(getFolderPath(null, {})).toBeNull()
69+
expect(getFolderPath(undefined, {})).toBeNull()
70+
expect(getFolderPath('missing', {})).toBeNull()
71+
})
72+
73+
it('returns the leaf folder name when at top level', () => {
74+
const folders = { f1: makeFolder({ id: 'f1', name: 'Engineering' }) }
75+
expect(getFolderPath('f1', folders)).toBe('Engineering')
76+
})
77+
78+
it('joins ancestor names from root to leaf', () => {
79+
const folders = {
80+
eng: makeFolder({ id: 'eng', name: 'Engineering' }),
81+
be: makeFolder({ id: 'be', name: 'Backend', parentId: 'eng' }),
82+
api: makeFolder({ id: 'api', name: 'API', parentId: 'be' }),
83+
}
84+
expect(getFolderPath('api', folders)).toBe('Engineering / Backend / API')
85+
})
86+
87+
it('respects custom separators', () => {
88+
const folders = {
89+
eng: makeFolder({ id: 'eng', name: 'Engineering' }),
90+
be: makeFolder({ id: 'be', name: 'Backend', parentId: 'eng' }),
91+
}
92+
expect(getFolderPath('be', folders, ' > ')).toBe('Engineering > Backend')
93+
})
94+
95+
it('returns the partial path resolved before a missing ancestor', () => {
96+
const folders = {
97+
be: makeFolder({ id: 'be', name: 'Backend', parentId: 'missing' }),
98+
}
99+
expect(getFolderPath('be', folders)).toBe('Backend')
100+
})
101+
102+
it('short-circuits on cycles instead of looping forever', () => {
103+
const folders = {
104+
f1: makeFolder({ id: 'f1', name: 'A', parentId: 'f2' }),
105+
f2: makeFolder({ id: 'f2', name: 'B', parentId: 'f1' }),
106+
}
107+
expect(getFolderPath('f1', folders)).toBe('B / A')
108+
})
109+
})
110+
111+
describe('findLockedAncestorFolder', () => {
112+
it('returns null for root or unknown folders', () => {
113+
expect(findLockedAncestorFolder(null, {})).toBeNull()
114+
expect(findLockedAncestorFolder(undefined, {})).toBeNull()
115+
expect(findLockedAncestorFolder('missing', {})).toBeNull()
116+
})
117+
118+
it('returns the folder itself when it is the locked one', () => {
119+
const folders = { f1: makeFolder({ id: 'f1', name: 'Engineering', locked: true }) }
120+
expect(findLockedAncestorFolder('f1', folders)?.name).toBe('Engineering')
121+
})
122+
123+
it('returns the closest locked ancestor, not the root', () => {
124+
const folders = {
125+
root: makeFolder({ id: 'root', name: 'Root', locked: true }),
126+
mid: makeFolder({ id: 'mid', name: 'Mid', parentId: 'root', locked: true }),
127+
leaf: makeFolder({ id: 'leaf', name: 'Leaf', parentId: 'mid' }),
128+
}
129+
expect(findLockedAncestorFolder('leaf', folders)?.id).toBe('mid')
130+
})
131+
132+
it('returns null when no folder in the chain is locked', () => {
133+
const folders = {
134+
f1: makeFolder({ id: 'f1', name: 'A' }),
135+
f2: makeFolder({ id: 'f2', name: 'B', parentId: 'f1' }),
136+
}
137+
expect(findLockedAncestorFolder('f2', folders)).toBeNull()
138+
})
139+
140+
it('short-circuits on cycles instead of looping forever', () => {
141+
const folders = {
142+
f1: makeFolder({ id: 'f1', name: 'A', parentId: 'f2' }),
143+
f2: makeFolder({ id: 'f2', name: 'B', parentId: 'f1' }),
144+
}
145+
expect(findLockedAncestorFolder('f1', folders)).toBeNull()
146+
})
147+
})

0 commit comments

Comments
 (0)