Skip to content

Commit 2eecb5e

Browse files
committed
address comments
1 parent e19f7f9 commit 2eecb5e

14 files changed

Lines changed: 395 additions & 165 deletions

File tree

apps/realtime/src/database/operations.ts

Lines changed: 31 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
import { getActiveWorkflowContext } from '@sim/workflow-authz'
1717
import { loadWorkflowFromNormalizedTablesRaw } from '@sim/workflow-persistence/load'
1818
import { mergeSubBlockValues } from '@sim/workflow-persistence/subblocks'
19+
import { isWorkflowBlockProtected } from '@sim/workflow-types/workflow'
1920
import { and, eq, inArray, isNull, or, sql } from 'drizzle-orm'
2021
import { drizzle } from 'drizzle-orm/postgres-js'
2122
import postgres from 'postgres'
@@ -47,26 +48,6 @@ interface DbBlockRef {
4748
data: unknown
4849
}
4950

50-
/**
51-
* Checks if a block is protected (locked or inside a locked ancestor).
52-
* Works with raw DB records.
53-
*/
54-
function isDbBlockProtected(blockId: string, blocksById: Record<string, DbBlockRef>): boolean {
55-
const block = blocksById[blockId]
56-
if (!block) return false
57-
if (block.locked) return true
58-
const visited = new Set<string>()
59-
let parentId = (block.data as Record<string, unknown> | null)?.parentId as string | undefined
60-
while (parentId && !visited.has(parentId)) {
61-
visited.add(parentId)
62-
if (blocksById[parentId]?.locked) return true
63-
parentId = (blocksById[parentId]?.data as Record<string, unknown> | null)?.parentId as
64-
| string
65-
| undefined
66-
}
67-
return false
68-
}
69-
7051
/**
7152
* Finds all descendant block IDs of a container (recursive).
7253
* Works with raw DB block arrays.
@@ -880,7 +861,7 @@ async function handleBlocksOperationTx(
880861
)
881862

882863
// Filter out protected blocks from deletion request
883-
const deletableIds = ids.filter((id) => !isDbBlockProtected(id, blocksById))
864+
const deletableIds = ids.filter((id) => !isWorkflowBlockProtected(id, blocksById))
884865
if (deletableIds.length === 0) {
885866
logger.info('All requested blocks are protected, skipping deletion')
886867
return
@@ -995,14 +976,14 @@ async function handleBlocksOperationTx(
995976
// Collect all blocks to toggle including descendants of containers
996977
for (const id of blockIds) {
997978
const block = blocksById[id]
998-
if (!block || isDbBlockProtected(id, blocksById)) continue
979+
if (!block || isWorkflowBlockProtected(id, blocksById)) continue
999980

1000981
blocksToToggle.add(id)
1001982

1002983
// If it's a loop or parallel, also include all non-locked descendants
1003984
if (block.type === 'loop' || block.type === 'parallel') {
1004985
for (const descId of findDbDescendants(id, allBlocks)) {
1005-
if (!isDbBlockProtected(descId, blocksById)) {
986+
if (!isWorkflowBlockProtected(descId, blocksById)) {
1006987
blocksToToggle.add(descId)
1007988
}
1008989
}
@@ -1057,7 +1038,7 @@ async function handleBlocksOperationTx(
10571038

10581039
// Filter to only toggle handles on unprotected blocks
10591040
const blocksToToggle = blockIds.filter(
1060-
(id) => blocksById[id] && !isDbBlockProtected(id, blocksById)
1041+
(id) => blocksById[id] && !isWorkflowBlockProtected(id, blocksById)
10611042
)
10621043
if (blocksToToggle.length === 0) {
10631044
logger.info('All requested blocks are protected, skipping handles toggle')
@@ -1169,13 +1150,13 @@ async function handleBlocksOperationTx(
11691150
if (!id) continue
11701151

11711152
// Skip protected blocks (locked or inside locked container)
1172-
if (isDbBlockProtected(id, blocksById)) {
1153+
if (isWorkflowBlockProtected(id, blocksById)) {
11731154
logger.info(`Skipping block ${id} parent update - block is protected`)
11741155
continue
11751156
}
11761157

11771158
// Skip if trying to move into a locked container (or any of its ancestors)
1178-
if (parentId && isDbBlockProtected(parentId, blocksById)) {
1159+
if (parentId && isWorkflowBlockProtected(parentId, blocksById)) {
11791160
logger.info(`Skipping block ${id} parent update - target parent ${parentId} is protected`)
11801161
continue
11811162
}
@@ -1299,7 +1280,7 @@ async function handleEdgeOperationTx(tx: any, workflowId: string, operation: str
12991280
}
13001281
}
13011282

1302-
if (isDbBlockProtected(payload.target, blocksById)) {
1283+
if (isWorkflowBlockProtected(payload.target, blocksById)) {
13031284
logger.info(`Skipping edge add - target block is protected`)
13041285
break
13051286
}
@@ -1387,7 +1368,7 @@ async function handleEdgeOperationTx(tx: any, workflowId: string, operation: str
13871368
}
13881369
}
13891370

1390-
if (isDbBlockProtected(edgeToRemove.targetBlockId, blocksById)) {
1371+
if (isWorkflowBlockProtected(edgeToRemove.targetBlockId, blocksById)) {
13911372
logger.info(`Skipping edge remove - target block is protected`)
13921373
break
13931374
}
@@ -1498,7 +1479,7 @@ async function handleEdgesOperationTx(
14981479
}
14991480

15001481
const safeEdgeIds = edgesToRemove
1501-
.filter((e: EdgeToRemove) => !isDbBlockProtected(e.targetBlockId, blocksById))
1482+
.filter((e: EdgeToRemove) => !isWorkflowBlockProtected(e.targetBlockId, blocksById))
15021483
.map((e: EdgeToRemove) => e.id)
15031484

15041485
if (safeEdgeIds.length === 0) {
@@ -1585,7 +1566,7 @@ async function handleEdgesOperationTx(
15851566

15861567
// Filter edges - only add edges where target block is not protected
15871568
const safeEdges = (edges as Array<Record<string, unknown>>).filter(
1588-
(e) => !isDbBlockProtected(e.target as string, blocksById)
1569+
(e) => !isWorkflowBlockProtected(e.target as string, blocksById)
15891570
)
15901571

15911572
if (safeEdges.length === 0) {
@@ -1756,43 +1737,34 @@ async function handleSubblockOperationTx(
17561737
return
17571738
}
17581739

1740+
const allBlocks = await tx
1741+
.select({
1742+
id: workflowBlocks.id,
1743+
subBlocks: workflowBlocks.subBlocks,
1744+
locked: workflowBlocks.locked,
1745+
data: workflowBlocks.data,
1746+
})
1747+
.from(workflowBlocks)
1748+
.where(eq(workflowBlocks.workflowId, workflowId))
1749+
1750+
type SubblockUpdateBlockRecord = (typeof allBlocks)[number]
1751+
const blocksById: Record<string, SubblockUpdateBlockRecord> = Object.fromEntries(
1752+
allBlocks.map((block: SubblockUpdateBlockRecord) => [block.id, block])
1753+
)
1754+
17591755
for (const update of updates) {
17601756
const { blockId, subblockId, value, expectedValue } = update
17611757
if (!blockId || !subblockId) {
17621758
throw new Error('Missing required fields for subblock batch update')
17631759
}
17641760

1765-
const [block] = await tx
1766-
.select({
1767-
subBlocks: workflowBlocks.subBlocks,
1768-
locked: workflowBlocks.locked,
1769-
data: workflowBlocks.data,
1770-
})
1771-
.from(workflowBlocks)
1772-
.where(and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId)))
1773-
.limit(1)
1774-
1761+
const block = blocksById[blockId]
17751762
if (!block) {
17761763
throw new Error(`Block ${blockId} not found`)
17771764
}
17781765

1779-
if (block.locked) {
1780-
throw new Error(`Block ${blockId} is locked`)
1781-
}
1782-
1783-
const parentId = (block.data as Record<string, unknown> | null)?.parentId as
1784-
| string
1785-
| undefined
1786-
if (parentId) {
1787-
const [parentBlock] = await tx
1788-
.select({ locked: workflowBlocks.locked })
1789-
.from(workflowBlocks)
1790-
.where(and(eq(workflowBlocks.id, parentId), eq(workflowBlocks.workflowId, workflowId)))
1791-
.limit(1)
1792-
1793-
if (parentBlock?.locked) {
1794-
throw new Error(`Parent block ${parentId} is locked`)
1795-
}
1766+
if (isWorkflowBlockProtected(blockId, blocksById)) {
1767+
throw new Error(`Block ${blockId} is locked or inside a locked container`)
17961768
}
17971769

17981770
const subBlocks = { ...((block.subBlocks as Record<string, any>) || {}) }
@@ -1813,6 +1785,8 @@ async function handleSubblockOperationTx(
18131785
updatedAt: new Date(),
18141786
})
18151787
.where(and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId)))
1788+
1789+
blocksById[blockId] = { ...block, subBlocks }
18161790
}
18171791

18181792
logger.debug(`Batch updated ${updates.length} subblocks for workflow ${workflowId}`)

apps/realtime/src/handlers/subblocks.ts

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { workflow, workflowBlocks } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { SUBBLOCK_OPERATIONS } from '@sim/realtime-protocol/constants'
55
import { assertWorkflowMutable, WorkflowLockedError } from '@sim/workflow-authz'
6+
import { isWorkflowBlockProtected } from '@sim/workflow-types/workflow'
67
import { and, eq } from 'drizzle-orm'
78
import type { AuthenticatedSocket } from '@/middleware/auth'
89
import { checkRolePermission } from '@/middleware/permissions'
@@ -273,45 +274,33 @@ async function flushSubblockUpdate(
273274
let updateSuccessful = false
274275
let blockLocked = false
275276
await db.transaction(async (tx) => {
276-
const [block] = await tx
277+
const allBlocks = await tx
277278
.select({
279+
id: workflowBlocks.id,
278280
subBlocks: workflowBlocks.subBlocks,
279281
locked: workflowBlocks.locked,
280282
data: workflowBlocks.data,
281283
})
282284
.from(workflowBlocks)
283-
.where(and(eq(workflowBlocks.id, blockId), eq(workflowBlocks.workflowId, workflowId)))
284-
.limit(1)
285+
.where(eq(workflowBlocks.workflowId, workflowId))
285286

287+
type SubblockUpdateBlockRecord = (typeof allBlocks)[number]
288+
const blocksById: Record<string, SubblockUpdateBlockRecord> = Object.fromEntries(
289+
allBlocks.map((block: SubblockUpdateBlockRecord) => [block.id, block])
290+
)
291+
const block = blocksById[blockId]
286292
if (!block) {
287293
return
288294
}
289295

290-
// Check if block is locked directly
291-
if (block.locked) {
292-
logger.info(`Skipping subblock update - block ${blockId} is locked`)
296+
if (isWorkflowBlockProtected(blockId, blocksById)) {
297+
logger.info(
298+
`Skipping subblock update - block ${blockId} is locked or inside a locked container`
299+
)
293300
blockLocked = true
294301
return
295302
}
296303

297-
// Check if block is inside a locked parent container
298-
const parentId = (block.data as Record<string, unknown> | null)?.parentId as
299-
| string
300-
| undefined
301-
if (parentId) {
302-
const [parentBlock] = await tx
303-
.select({ locked: workflowBlocks.locked })
304-
.from(workflowBlocks)
305-
.where(and(eq(workflowBlocks.id, parentId), eq(workflowBlocks.workflowId, workflowId)))
306-
.limit(1)
307-
308-
if (parentBlock?.locked) {
309-
logger.info(`Skipping subblock update - parent ${parentId} is locked`)
310-
blockLocked = true
311-
return
312-
}
313-
}
314-
315304
const subBlocks = (block.subBlocks as any) || {}
316305
if (!subBlocks[subblockId]) {
317306
subBlocks[subblockId] = { id: subblockId, type: 'unknown', value }

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
44
import { ChevronDown, ChevronRight, ChevronUp, X } from 'lucide-react'
55
import { useParams } from 'next/navigation'
66
import { Button, Input } from '@/components/emcn'
7+
import { cn } from '@/lib/core/utils/cn'
78
import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies'
89
import { indexWorkflowSearchMatches } from '@/lib/workflows/search-replace/indexer'
910
import {
@@ -140,10 +141,10 @@ export function WorkflowSearchReplace() {
140141
() =>
141142
getWorkflowSearchBlocks({
142143
blocks: currentWorkflow.blocks,
143-
workflowId,
144144
isSnapshotView: currentWorkflow.isSnapshotView,
145+
subblockValues: workflowSubblockValues,
145146
}),
146-
[currentWorkflow.blocks, currentWorkflow.isSnapshotView, workflowId, workflowSubblockValues]
147+
[currentWorkflow.blocks, currentWorkflow.isSnapshotView, workflowSubblockValues]
147148
)
148149

149150
const matches = useMemo(
@@ -394,8 +395,15 @@ export function WorkflowSearchReplace() {
394395
return
395396
}
396397

397-
const applied =
398-
batchUpdates.length === 0 ? true : collaborativeBatchSetSubblockValues(batchUpdates)
398+
const applied = collaborativeBatchSetSubblockValues(batchUpdates, {
399+
subflowUpdates: plan.subflowUpdates.map((update) => ({
400+
blockId: update.blockId,
401+
blockType: update.blockType,
402+
fieldId: update.fieldId,
403+
before: update.previousValue,
404+
after: update.nextValue,
405+
})),
406+
})
399407
if (!applied) {
400408
addNotification({
401409
level: 'error',
@@ -435,7 +443,7 @@ export function WorkflowSearchReplace() {
435443
: `${activeMatchIndex >= 0 ? activeMatchIndex + 1 : 1} of ${hydratedMatches.length}`
436444
return (
437445
<div
438-
className='fixed z-30 flex flex-col overflow-hidden rounded-lg border border-[var(--border)] bg-[var(--surface-1)] px-2.5 pt-0.5 pb-2'
446+
className='fixed z-[var(--z-dropdown)] flex flex-col overflow-hidden rounded-lg border border-[var(--border)] bg-[var(--surface-1)] px-2.5 pt-0.5 pb-2'
439447
style={{
440448
left: `${actualPosition.x}px`,
441449
top: `${actualPosition.y}px`,
@@ -471,7 +479,7 @@ export function WorkflowSearchReplace() {
471479
onClick={() => setIsReplaceExpanded((expanded) => !expanded)}
472480
>
473481
<ChevronRight
474-
className={`h-4 w-4 transition-transform ${isReplaceExpanded ? 'rotate-90' : ''}`}
482+
className={cn('h-4 w-4 transition-transform', isReplaceExpanded && 'rotate-90')}
475483
/>
476484
</Button>
477485
<Input

apps/sim/hooks/use-collaborative-workflow.ts

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,9 +1537,24 @@ export function useCollaborativeWorkflow() {
15371537
subblockId: string
15381538
value: unknown
15391539
expectedValue?: unknown
1540-
}>
1540+
}>,
1541+
options: {
1542+
subflowUpdates?: Array<{
1543+
blockId: string
1544+
blockType: 'loop' | 'parallel'
1545+
fieldId: string
1546+
before: unknown
1547+
after: unknown
1548+
}>
1549+
} = {}
15411550
) => {
1542-
if (isApplyingRemoteChange.current || updates.length === 0) return false
1551+
const undoSubflowUpdates = options.subflowUpdates ?? []
1552+
if (
1553+
isApplyingRemoteChange.current ||
1554+
(updates.length === 0 && undoSubflowUpdates.length === 0)
1555+
) {
1556+
return false
1557+
}
15431558

15441559
if (isBaselineDiffView) {
15451560
logger.debug('Skipping collaborative batch subblock update while viewing baseline diff')
@@ -1551,32 +1566,35 @@ export function useCollaborativeWorkflow() {
15511566
return false
15521567
}
15531568

1554-
updates.forEach((update) => {
1555-
useSubBlockStore.getState().setValue(update.blockId, update.subblockId, update.value)
1556-
useWorkflowStore
1557-
.getState()
1558-
.syncDynamicHandleSubblockValue(update.blockId, update.subblockId, update.value)
1559-
})
1569+
if (updates.length > 0) {
1570+
updates.forEach((update) => {
1571+
useSubBlockStore.getState().setValue(update.blockId, update.subblockId, update.value)
1572+
useWorkflowStore
1573+
.getState()
1574+
.syncDynamicHandleSubblockValue(update.blockId, update.subblockId, update.value)
1575+
})
15601576

1561-
const operationId = generateId()
1562-
addToQueue({
1563-
id: operationId,
1564-
operation: {
1565-
operation: SUBBLOCK_OPERATIONS.BATCH_UPDATE,
1566-
target: OPERATION_TARGETS.SUBBLOCK,
1567-
payload: { updates },
1568-
},
1569-
workflowId: activeWorkflowId,
1570-
userId: session?.user?.id || 'unknown',
1571-
})
1577+
const operationId = generateId()
1578+
addToQueue({
1579+
id: operationId,
1580+
operation: {
1581+
operation: SUBBLOCK_OPERATIONS.BATCH_UPDATE,
1582+
target: OPERATION_TARGETS.SUBBLOCK,
1583+
payload: { updates },
1584+
},
1585+
workflowId: activeWorkflowId,
1586+
userId: session?.user?.id || 'unknown',
1587+
})
1588+
}
15721589

15731590
undoRedo.recordBatchUpdateSubblocks(
15741591
updates.map((update) => ({
15751592
blockId: update.blockId,
15761593
subBlockId: update.subblockId,
15771594
before: update.expectedValue,
15781595
after: update.value,
1579-
}))
1596+
})),
1597+
undoSubflowUpdates
15801598
)
15811599

15821600
return true

0 commit comments

Comments
 (0)