Skip to content

Commit 87ed366

Browse files
waleedlatif1waleedlatif1
committed
refactor(workflows): one owner for new-workflow sort order
The same ~35-line query — parent condition for workflows and folders, two parallel min(sortOrder) reads, fold to a min, subtract one, fall back to 0 — existed three times: lib/workflows/utils.ts inline in createWorkflowRecord lib/workflows/orchestration/... as a file-private nextWorkflowSortOrder lib/workflows/persistence/duplicate inline, inside the duplicate transaction The first two are character-identical modulo the table alias. The third had drifted: it omits isNull(workflow.archivedAt), which the other two apply, so a folder whose lowest-sortOrder workflow is soft-deleted positioned a *duplicate* differently from a *create*. The folder-side query agrees in all three, which marks it as a copy-paste slip rather than intent. Promotes the helper to lib/workflows/sort-order.ts, taking an optional DbOrTx so the duplicate path can keep reading inside its transaction. Its own module rather than utils.ts because duplicate.test.ts and workflow-lifecycle.test.ts both mock '@/lib/workflows/utils' wholesale — from a separate module the real query still runs under those suites, so their existing sort-order assertions keep their meaning and needed no edits. Note the archived-row behavior itself is not unit-testable here: the shared dbChainMock does not evaluate WHERE predicates. The guarantee is structural — one query builder instead of three means the predicate can no longer drift.
1 parent 167fcb4 commit 87ed366

4 files changed

Lines changed: 68 additions & 118 deletions

File tree

‎apps/sim/lib/workflows/orchestration/workflow-lifecycle.ts‎

Lines changed: 2 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ import { createLogger } from '@sim/logger'
55
import { isFolderInWorkspace } from '@sim/platform-authz/workflow'
66
import { getPostgresConstraintName, getPostgresErrorCode, toError } from '@sim/utils/errors'
77
import { generateId } from '@sim/utils/id'
8-
import { and, eq, isNull, min, ne } from 'drizzle-orm'
8+
import { and, eq, isNull, ne } from 'drizzle-orm'
99
import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types'
1010
import { generateRequestId } from '@/lib/core/utils/request'
1111
import type { DbOrTx } from '@/lib/db/types'
1212
import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults'
1313
import { archiveWorkflow, restoreWorkflow } from '@/lib/workflows/lifecycle'
1414
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
15+
import { nextWorkflowSortOrder } from '@/lib/workflows/sort-order'
1516
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
1617

1718
const logger = createLogger('WorkflowLifecycle')
@@ -126,51 +127,6 @@ export interface PerformRestoreWorkflowResult {
126127
workflow?: Awaited<ReturnType<typeof restoreWorkflow>>['workflow']
127128
}
128129

129-
async function nextWorkflowSortOrder(
130-
workspaceId: string,
131-
folderId: string | null | undefined
132-
): Promise<number> {
133-
const workflowParentCondition = folderId
134-
? eq(workflow.folderId, folderId)
135-
: isNull(workflow.folderId)
136-
const folderParentCondition = folderId
137-
? eq(folderTable.parentId, folderId)
138-
: isNull(folderTable.parentId)
139-
140-
const [[workflowMinResult], [folderMinResult]] = await Promise.all([
141-
db
142-
.select({ minOrder: min(workflow.sortOrder) })
143-
.from(workflow)
144-
.where(
145-
and(
146-
eq(workflow.workspaceId, workspaceId),
147-
workflowParentCondition,
148-
isNull(workflow.archivedAt)
149-
)
150-
),
151-
db
152-
.select({ minOrder: min(folderTable.sortOrder) })
153-
.from(folderTable)
154-
.where(
155-
and(
156-
eq(folderTable.workspaceId, workspaceId),
157-
eq(folderTable.resourceType, 'workflow'),
158-
folderParentCondition
159-
)
160-
),
161-
])
162-
163-
const minSortOrder = [workflowMinResult?.minOrder, folderMinResult?.minOrder].reduce<
164-
number | null
165-
>((currentMin, candidate) => {
166-
if (candidate == null) return currentMin
167-
if (currentMin == null) return candidate
168-
return Math.min(currentMin, candidate)
169-
}, null)
170-
171-
return minSortOrder != null ? minSortOrder - 1 : 0
172-
}
173-
174130
async function workflowNameExistsInFolder(params: {
175131
workspaceId: string
176132
name: string

‎apps/sim/lib/workflows/persistence/duplicate.ts‎

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
normalizeWorkflowEdgeSourceHandle,
1818
normalizeWorkflowEdgeTargetHandle,
1919
} from '@sim/workflow-types/workflow'
20-
import { and, eq, isNull, min } from 'drizzle-orm'
20+
import { and, eq } from 'drizzle-orm'
2121
import type { DbOrTx } from '@/lib/db/types'
2222
import { remapConditionEdgeHandle } from '@/lib/workflows/condition-ids'
2323
import {
@@ -27,6 +27,7 @@ import {
2727
type SubBlockRecord,
2828
sanitizeSubBlocksForDuplicate,
2929
} from '@/lib/workflows/persistence/remap-internal-ids'
30+
import { nextWorkflowSortOrder } from '@/lib/workflows/sort-order'
3031
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
3132
import type { Variable } from '@/stores/variables/types'
3233
import type { LoopConfig, ParallelConfig } from '@/stores/workflows/workflow/types'
@@ -183,37 +184,7 @@ export async function duplicateWorkflow(
183184
const targetFolderId = folderId !== undefined ? folderId : source.folderId
184185
await assertTargetFolderMutable(tx, targetFolderId, targetWorkspaceId)
185186

186-
const workflowParentCondition = targetFolderId
187-
? eq(workflow.folderId, targetFolderId)
188-
: isNull(workflow.folderId)
189-
const folderParentCondition = targetFolderId
190-
? eq(folderTable.parentId, targetFolderId)
191-
: isNull(folderTable.parentId)
192-
193-
const [[workflowMinResult], [folderMinResult]] = await Promise.all([
194-
tx
195-
.select({ minOrder: min(workflow.sortOrder) })
196-
.from(workflow)
197-
.where(and(eq(workflow.workspaceId, targetWorkspaceId), workflowParentCondition)),
198-
tx
199-
.select({ minOrder: min(folderTable.sortOrder) })
200-
.from(folderTable)
201-
.where(
202-
and(
203-
eq(folderTable.workspaceId, targetWorkspaceId),
204-
eq(folderTable.resourceType, 'workflow'),
205-
folderParentCondition
206-
)
207-
),
208-
])
209-
const minSortOrder = [workflowMinResult?.minOrder, folderMinResult?.minOrder].reduce<
210-
number | null
211-
>((currentMin, candidate) => {
212-
if (candidate == null) return currentMin
213-
if (currentMin == null) return candidate
214-
return Math.min(currentMin, candidate)
215-
}, null)
216-
const sortOrder = minSortOrder != null ? minSortOrder - 1 : 0
187+
const sortOrder = await nextWorkflowSortOrder(targetWorkspaceId, targetFolderId, tx)
217188

218189
// Mapping from old variable IDs to new variable IDs (populated during variable duplication)
219190
const varIdMapping = new Map<string, string>()

‎apps/sim/lib/workflows/sort-order.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { db } from '@sim/db'
2+
import { folder as folderTable, workflow as workflowTable } from '@sim/db/schema'
3+
import { and, eq, isNull, min } from 'drizzle-orm'
4+
import type { DbOrTx } from '@/lib/db/types'
5+
6+
/**
7+
* Sort order placing a new workflow above everything already in its folder.
8+
*
9+
* Workflows and folders share one ordering, so both minimums are consulted.
10+
* Archived workflows are excluded: a soft-deleted row must not hold a slot that
11+
* pushes new siblings further up each time one is created.
12+
*
13+
* Pass `tx` when the caller is inside a transaction, so the read sees that
14+
* transaction's uncommitted rows rather than the pre-transaction snapshot.
15+
*/
16+
export async function nextWorkflowSortOrder(
17+
workspaceId: string,
18+
folderId: string | null | undefined,
19+
tx: DbOrTx = db
20+
): Promise<number> {
21+
const workflowParentCondition = folderId
22+
? eq(workflowTable.folderId, folderId)
23+
: isNull(workflowTable.folderId)
24+
const folderParentCondition = folderId
25+
? eq(folderTable.parentId, folderId)
26+
: isNull(folderTable.parentId)
27+
28+
const [[workflowMinResult], [folderMinResult]] = await Promise.all([
29+
tx
30+
.select({ minOrder: min(workflowTable.sortOrder) })
31+
.from(workflowTable)
32+
.where(
33+
and(
34+
eq(workflowTable.workspaceId, workspaceId),
35+
workflowParentCondition,
36+
isNull(workflowTable.archivedAt)
37+
)
38+
),
39+
tx
40+
.select({ minOrder: min(folderTable.sortOrder) })
41+
.from(folderTable)
42+
.where(
43+
and(
44+
eq(folderTable.workspaceId, workspaceId),
45+
eq(folderTable.resourceType, 'workflow'),
46+
folderParentCondition
47+
)
48+
),
49+
])
50+
51+
const minSortOrder = [workflowMinResult?.minOrder, folderMinResult?.minOrder].reduce<
52+
number | null
53+
>((currentMin, candidate) => {
54+
if (candidate == null) return currentMin
55+
if (currentMin == null) return candidate
56+
return Math.min(currentMin, candidate)
57+
}, null)
58+
59+
return minSortOrder != null ? minSortOrder - 1 : 0
60+
}

‎apps/sim/lib/workflows/utils.ts‎

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@ import { folder as folderTable, workflow as workflowTable } from '@sim/db/schema
33
import { createLogger } from '@sim/logger'
44
import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/workflow'
55
import { generateId } from '@sim/utils/id'
6-
import { and, asc, eq, inArray, isNull, min, sql } from 'drizzle-orm'
6+
import { and, asc, eq, inArray, isNull, sql } from 'drizzle-orm'
77
import { NextResponse } from 'next/server'
88
import { getSession } from '@/lib/auth'
99
import { materializeInlineExecutionValue } from '@/lib/execution/payloads/inline-materialization.server'
1010
import type { ExecutionMaterializationContext } from '@/lib/execution/payloads/materialization.server'
1111
import { buildDefaultWorkflowArtifacts } from '@/lib/workflows/defaults'
1212
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
13+
import { nextWorkflowSortOrder } from '@/lib/workflows/sort-order'
1314
import { listAccessibleWorkspaceRowsForUser } from '@/lib/workspaces/utils'
1415
import type { ExecutionResult } from '@/executor/types'
1516

@@ -396,45 +397,7 @@ export async function createWorkflowRecord(params: CreateWorkflowInput) {
396397
)
397398
}
398399

399-
const workflowParentCondition = folderId
400-
? eq(workflowTable.folderId, folderId)
401-
: isNull(workflowTable.folderId)
402-
const folderParentCondition = folderId
403-
? eq(folderTable.parentId, folderId)
404-
: isNull(folderTable.parentId)
405-
406-
const [[workflowMinResult], [folderMinResult]] = await Promise.all([
407-
db
408-
.select({ minOrder: min(workflowTable.sortOrder) })
409-
.from(workflowTable)
410-
.where(
411-
and(
412-
eq(workflowTable.workspaceId, workspaceId),
413-
workflowParentCondition,
414-
isNull(workflowTable.archivedAt)
415-
)
416-
),
417-
db
418-
.select({ minOrder: min(folderTable.sortOrder) })
419-
.from(folderTable)
420-
.where(
421-
and(
422-
eq(folderTable.workspaceId, workspaceId),
423-
eq(folderTable.resourceType, 'workflow'),
424-
folderParentCondition
425-
)
426-
),
427-
])
428-
429-
const minSortOrder = [workflowMinResult?.minOrder, folderMinResult?.minOrder].reduce<
430-
number | null
431-
>((currentMin, candidate) => {
432-
if (candidate == null) return currentMin
433-
if (currentMin == null) return candidate
434-
return Math.min(currentMin, candidate)
435-
}, null)
436-
437-
const sortOrder = minSortOrder != null ? minSortOrder - 1 : 0
400+
const sortOrder = await nextWorkflowSortOrder(workspaceId, folderId)
438401

439402
await db.insert(workflowTable).values({
440403
id: workflowId,

0 commit comments

Comments
 (0)