Skip to content

Commit e5750ea

Browse files
fix bugs
1 parent 69ac23d commit e5750ea

3 files changed

Lines changed: 162 additions & 49 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table/table.tsx

Lines changed: 91 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ import {
7474
useUpdateColumn,
7575
useUpdateTableMetadata,
7676
useUpdateTableRow,
77+
useUpdateWorkflowGroup,
7778
} from '@/hooks/queries/tables'
7879
import { useDeploymentInfo, useDeployWorkflow } from '@/hooks/queries/deployments'
7980
import { useLogByExecutionId } from '@/hooks/queries/logs'
@@ -432,6 +433,7 @@ export function Table({
432433
const cancelRunsMutation = useCancelTableRuns({ workspaceId, tableId })
433434
const runGroupMutation = useRunGroup({ workspaceId, tableId })
434435
const deleteWorkflowGroupMutation = useDeleteWorkflowGroup({ workspaceId, tableId })
436+
const updateWorkflowGroupMutation = useUpdateWorkflowGroup({ workspaceId, tableId })
435437

436438
const handleRunGroup = useCallback(
437439
(groupId: string, workflowId: string, mode: 'all' | 'incomplete' = 'all') => {
@@ -659,6 +661,7 @@ export function Table({
659661

660662
const columnsRef = useRef(displayColumns)
661663
const schemaColumnsRef = useRef(columns)
664+
const workflowGroupsRef = useRef(tableWorkflowGroups)
662665
const rowsRef = useRef(rows)
663666
const selectionAnchorRef = useRef(selectionAnchor)
664667
const selectionFocusRef = useRef(selectionFocus)
@@ -668,6 +671,7 @@ export function Table({
668671

669672
columnsRef.current = displayColumns
670673
schemaColumnsRef.current = columns
674+
workflowGroupsRef.current = tableWorkflowGroups
671675
rowsRef.current = rows
672676
selectionAnchorRef.current = selectionAnchor
673677
selectionFocusRef.current = selectionFocus
@@ -1302,14 +1306,35 @@ export function Table({
13021306
}
13031307

13041308
useEffect(() => {
1305-
if (!tableData?.metadata || metadataSeededRef.current) return
1309+
if (!tableData?.metadata) return
13061310
if (!tableData.metadata.columnWidths && !tableData.metadata.columnOrder) return
1307-
metadataSeededRef.current = true
1308-
if (tableData.metadata.columnWidths) {
1309-
setColumnWidths(tableData.metadata.columnWidths)
1311+
// First load: seed both from the server and remember we've seeded.
1312+
if (!metadataSeededRef.current) {
1313+
metadataSeededRef.current = true
1314+
if (tableData.metadata.columnWidths) {
1315+
setColumnWidths(tableData.metadata.columnWidths)
1316+
}
1317+
if (tableData.metadata.columnOrder) {
1318+
setColumnOrder(tableData.metadata.columnOrder)
1319+
}
1320+
return
13101321
}
1311-
if (tableData.metadata.columnOrder) {
1312-
setColumnOrder(tableData.metadata.columnOrder)
1322+
// After first load: only re-seed `columnOrder` when the *set of columns*
1323+
// changes (e.g. a workflow group adds/removes outputs server-side). Pure
1324+
// reorders are left alone so an in-flight optimistic drag isn't clobbered
1325+
// by a refetch returning the pre-drag order.
1326+
const serverOrder = tableData.metadata.columnOrder
1327+
if (serverOrder) {
1328+
const localOrder = columnOrderRef.current
1329+
const serverSet = new Set(serverOrder)
1330+
const localSet = new Set(localOrder ?? [])
1331+
const setChanged =
1332+
!localOrder ||
1333+
serverSet.size !== localSet.size ||
1334+
serverOrder.some((n) => !localSet.has(n))
1335+
if (setChanged) {
1336+
setColumnOrder(serverOrder)
1337+
}
13131338
}
13141339
}, [tableData?.metadata])
13151340

@@ -2264,39 +2289,70 @@ export function Table({
22642289
const previousWidth = columnWidthsRef.current[columnToDelete] ?? null
22652290
const orderSnapshot = currentOrder ? [...currentOrder] : null
22662291

2267-
deleteColumnMutation.mutate(columnToDelete, {
2268-
onSuccess: () => {
2269-
deletedOriginalPositions.push(entry.position)
2270-
pushUndoRef.current({
2271-
type: 'delete-column',
2272-
columnName: columnToDelete,
2273-
columnType: entry.def?.type ?? 'string',
2274-
columnPosition: adjustedPosition >= 0 ? adjustedPosition : cols.length,
2275-
columnUnique: entry.def?.unique ?? false,
2276-
columnRequired: entry.def?.required ?? false,
2277-
cellData,
2278-
previousOrder: orderSnapshot,
2279-
previousWidth,
2292+
const onDeleted = () => {
2293+
deletedOriginalPositions.push(entry.position)
2294+
pushUndoRef.current({
2295+
type: 'delete-column',
2296+
columnName: columnToDelete,
2297+
columnType: entry.def?.type ?? 'string',
2298+
columnPosition: adjustedPosition >= 0 ? adjustedPosition : cols.length,
2299+
columnUnique: entry.def?.unique ?? false,
2300+
columnRequired: entry.def?.required ?? false,
2301+
cellData,
2302+
previousOrder: orderSnapshot,
2303+
previousWidth,
2304+
})
2305+
2306+
const { [columnToDelete]: _removedWidth, ...cleanedWidths } = columnWidthsRef.current
2307+
setColumnWidths(cleanedWidths)
2308+
columnWidthsRef.current = cleanedWidths
2309+
2310+
if (currentOrder) {
2311+
currentOrder = currentOrder.filter((n) => n !== columnToDelete)
2312+
setColumnOrder(currentOrder)
2313+
updateMetadataRef.current({
2314+
columnWidths: cleanedWidths,
2315+
columnOrder: currentOrder,
22802316
})
2317+
} else {
2318+
updateMetadataRef.current({ columnWidths: cleanedWidths })
2319+
}
22812320

2282-
const { [columnToDelete]: _removedWidth, ...cleanedWidths } = columnWidthsRef.current
2283-
setColumnWidths(cleanedWidths)
2284-
columnWidthsRef.current = cleanedWidths
2321+
deleteNext(index + 1)
2322+
}
22852323

2286-
if (currentOrder) {
2287-
currentOrder = currentOrder.filter((n) => n !== columnToDelete)
2288-
setColumnOrder(currentOrder)
2289-
updateMetadataRef.current({
2290-
columnWidths: cleanedWidths,
2291-
columnOrder: currentOrder,
2292-
})
2293-
} else {
2294-
updateMetadataRef.current({ columnWidths: cleanedWidths })
2295-
}
2324+
// Workflow-output columns are owned by a group: route the delete through
2325+
// `updateWorkflowGroup` so the same code path fires whether the user
2326+
// deselects the output in the sidebar or right-clicks Delete column.
2327+
// Falls back to deleting the whole group when this is its last output,
2328+
// since a group with zero outputs is invalid.
2329+
const groupId = entry.def?.workflowGroupId
2330+
const group = groupId
2331+
? workflowGroupsRef.current.find((g) => g.id === groupId)
2332+
: undefined
2333+
if (group) {
2334+
const remainingOutputs = group.outputs.filter((o) => o.columnName !== columnToDelete)
2335+
if (remainingOutputs.length === 0) {
2336+
deleteWorkflowGroupMutation.mutate(
2337+
{ groupId: group.id },
2338+
{ onSuccess: onDeleted }
2339+
)
2340+
} else {
2341+
updateWorkflowGroupMutation.mutate(
2342+
{
2343+
groupId: group.id,
2344+
workflowId: group.workflowId,
2345+
name: group.name,
2346+
dependencies: group.dependencies,
2347+
outputs: remainingOutputs,
2348+
},
2349+
{ onSuccess: onDeleted }
2350+
)
2351+
}
2352+
return
2353+
}
22962354

2297-
deleteNext(index + 1)
2298-
},
2299-
})
2355+
deleteColumnMutation.mutate(columnToDelete, { onSuccess: onDeleted })
23002356
}
23012357

23022358
setSelectionAnchor(null)

apps/sim/hooks/queries/tables.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -274,11 +274,14 @@ export function useTableRows({
274274
)
275275
// While an optimistic mutation is in flight, applying the socket delta
276276
// could clobber the optimistic state — defer to onSettled invalidate.
277+
// Mark stale without triggering a refetch (refetchType: 'none') so the
278+
// refetch races neither the in-flight optimistic update nor any
279+
// server-side post-response work the mutation is awaiting (e.g. backfill).
277280
if (queryClient.isMutating() > 0) {
278-
logger.info(
279-
`[STOP-DEBUG] socket row=${event.rowId} (mutation in flight → invalidate) incoming=${JSON.stringify(incomingExec)}`
280-
)
281-
queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) })
281+
queryClient.invalidateQueries({
282+
queryKey: tableKeys.rowsRoot(tableId),
283+
refetchType: 'none',
284+
})
282285
return
283286
}
284287
queryClient.setQueriesData<TableRowsResponse>(
@@ -328,7 +331,10 @@ export function useTableRows({
328331
onTableRowDeleted((event) => {
329332
if (event.tableId !== tableId) return
330333
if (queryClient.isMutating() > 0) {
331-
queryClient.invalidateQueries({ queryKey: tableKeys.rowsRoot(tableId) })
334+
queryClient.invalidateQueries({
335+
queryKey: tableKeys.rowsRoot(tableId),
336+
refetchType: 'none',
337+
})
332338
return
333339
}
334340
queryClient.setQueriesData<TableRowsResponse>(

apps/sim/lib/table/service.ts

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import type {
3232
BulkDeleteData,
3333
BulkOperationResult,
3434
BulkUpdateData,
35+
ColumnDefinition,
3536
CreateTableData,
3637
DeleteColumnData,
3738
DeleteWorkflowGroupData,
@@ -2725,7 +2726,33 @@ export async function updateWorkflowGroup(
27252726
throw new Error(`Column "${col.name}" already exists`)
27262727
}
27272728
}
2728-
nextColumns = [...nextColumns, ...newColDefs]
2729+
// Splice the new column defs into the group's contiguous run rather than
2730+
// appending at the end. The desired in-group order is `newOutputs` (the
2731+
// sidebar's BFS-of-the-workflow ordering); we walk it, anchor at the first
2732+
// surviving sibling's index in `nextColumns`, and emit each output's
2733+
// column def in turn.
2734+
const groupColNames = new Set(newOutputs.map((o) => o.columnName))
2735+
const firstGroupIdx = nextColumns.findIndex((c) => groupColNames.has(c.name))
2736+
const anchorIdx = firstGroupIdx === -1 ? nextColumns.length : firstGroupIdx
2737+
const newColByLowerName = new Map(newColDefs.map((c) => [c.name.toLowerCase(), c]))
2738+
const orderedGroupCols: ColumnDefinition[] = []
2739+
for (const out of newOutputs) {
2740+
const fresh = newColByLowerName.get(out.columnName.toLowerCase())
2741+
if (fresh) {
2742+
orderedGroupCols.push(fresh)
2743+
} else {
2744+
const existing = nextColumns.find(
2745+
(c) => c.name.toLowerCase() === out.columnName.toLowerCase()
2746+
)
2747+
if (existing) orderedGroupCols.push(existing)
2748+
}
2749+
}
2750+
const remaining = nextColumns.filter((c) => !groupColNames.has(c.name))
2751+
nextColumns = [
2752+
...remaining.slice(0, anchorIdx),
2753+
...orderedGroupCols,
2754+
...remaining.slice(anchorIdx),
2755+
]
27292756
}
27302757

27312758
const updatedGroup: WorkflowGroup = {
@@ -2742,9 +2769,27 @@ export async function updateWorkflowGroup(
27422769
workflowGroups: nextGroups,
27432770
}
27442771

2745-
const updatedColumnOrder = table.metadata?.columnOrder?.filter(
2772+
// `columnOrder` mirrors the schema layout. Drop removed columns, then splice
2773+
// the new ones in at the same anchor as `nextColumns` so the table renders
2774+
// them inside the group's contiguous run instead of at the tail.
2775+
let updatedColumnOrder = table.metadata?.columnOrder?.filter(
27462776
(n) => !removedColumnNames.has(n)
27472777
)
2778+
if (updatedColumnOrder && newColDefs.length > 0) {
2779+
const newColNamesLower = new Set(newColDefs.map((c) => c.name.toLowerCase()))
2780+
const orderWithoutNew = updatedColumnOrder.filter((n) => !newColNamesLower.has(n.toLowerCase()))
2781+
const groupColNames = new Set(newOutputs.map((o) => o.columnName))
2782+
const orderedGroupNames = newOutputs.map((o) => o.columnName)
2783+
const firstGroupOrderIdx = orderWithoutNew.findIndex((n) => groupColNames.has(n))
2784+
const anchorOrderIdx =
2785+
firstGroupOrderIdx === -1 ? orderWithoutNew.length : firstGroupOrderIdx
2786+
const remainingOrder = orderWithoutNew.filter((n) => !groupColNames.has(n))
2787+
updatedColumnOrder = [
2788+
...remainingOrder.slice(0, anchorOrderIdx),
2789+
...orderedGroupNames,
2790+
...remainingOrder.slice(anchorOrderIdx),
2791+
]
2792+
}
27482793
assertValidSchema(updatedSchema, updatedColumnOrder)
27492794

27502795
const updatedMetadata: TableMetadata | null =
@@ -2781,18 +2826,24 @@ export async function updateWorkflowGroup(
27812826

27822827
// Backfill the new outputs from execution logs so already-completed group
27832828
// runs surface the just-added columns without re-running the workflow.
2829+
// Awaited so the response only returns once row data is consistent — the
2830+
// client then refetches and sees the backfilled values immediately. A failed
2831+
// backfill is logged but doesn't fail the whole request, since the schema
2832+
// change has already committed.
27842833
if (added.length > 0) {
2785-
void backfillAddedGroupOutputs({
2786-
table: updatedTable,
2787-
groupId: data.groupId,
2788-
addedOutputs: added,
2789-
requestId,
2790-
}).catch((err) => {
2834+
try {
2835+
await backfillAddedGroupOutputs({
2836+
table: updatedTable,
2837+
groupId: data.groupId,
2838+
addedOutputs: added,
2839+
requestId,
2840+
})
2841+
} catch (err) {
27912842
logger.warn(
27922843
`[${requestId}] Backfill from execution logs failed for ${data.tableId} group ${data.groupId}:`,
27932844
err
27942845
)
2795-
})
2846+
}
27962847
}
27972848

27982849
return updatedTable

0 commit comments

Comments
 (0)