Skip to content

Commit 83bb430

Browse files
committed
fix: address code review feedback on node replacement
- Add error toast in replaceNodesInPlace for user-visible failure feedback, returning empty array on error instead of throwing - Guard removeMissingNodesByType behind replacement success check (replaced.length > 0) to prevent stale error list updates - Sort buildMissingNodeGroups by priority for deterministic UI order (Swap Nodes 0 → Missing Node Packs 1 → Execution Errors) - Add aux_id fallback and cnr_id precedence tests for getCnrIdFromNode - Split replaceAllWarning from replaceWarning to fix i18n key mismatch between TabErrors tooltip and MissingNodesContent dialog
1 parent 0d58a92 commit 83bb430

File tree

5 files changed

+35
-8
lines changed

5 files changed

+35
-8
lines changed

src/components/rightSidePanel/errors/SwapNodeGroupRow.vue

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,9 @@ function handleLocateNode(nodeType: MissingNodeType) {
142142
}
143143
144144
function handleReplaceNode() {
145-
replaceNodesInPlace(props.group.nodeTypes)
146-
executionErrorStore.removeMissingNodesByType([props.group.type])
145+
const replaced = replaceNodesInPlace(props.group.nodeTypes)
146+
if (replaced.length > 0) {
147+
executionErrorStore.removeMissingNodesByType([props.group.type])
148+
}
147149
}
148150
</script>

src/components/rightSidePanel/errors/TabErrors.vue

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,11 @@ function handleOpenManagerInfo(packId: string) {
267267
268268
function handleReplaceAll(swapNodeGroups: SwapNodeGroup[]) {
269269
const allNodeTypes = swapNodeGroups.flatMap((g) => g.nodeTypes)
270-
replaceNodesInPlace(allNodeTypes)
271-
const typesToRemove = swapNodeGroups.map((g) => g.type)
272-
executionErrorStore.removeMissingNodesByType(typesToRemove)
270+
const replaced = replaceNodesInPlace(allNodeTypes)
271+
if (replaced.length > 0) {
272+
const typesToRemove = swapNodeGroups.map((g) => g.type)
273+
executionErrorStore.removeMissingNodesByType(typesToRemove)
274+
}
273275
}
274276
275277
function handleEnterSubgraph(nodeId: string) {

src/components/rightSidePanel/errors/useErrorGroups.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -537,19 +537,19 @@ export function useErrorGroups(
537537
groups.push({
538538
type: 'swap_nodes' as const,
539539
title: 'Swap Nodes',
540-
priority: 1
540+
priority: 0
541541
})
542542
}
543543

544544
if (missingPackGroups.value.length > 0) {
545545
groups.push({
546546
type: 'missing_node' as const,
547547
title: error.message,
548-
priority: 0
548+
priority: 1
549549
})
550550
}
551551

552-
return groups
552+
return groups.sort((a, b) => a.priority - b.priority)
553553
}
554554

555555
const allErrorGroups = computed<ErrorGroup[]>(() => {

src/platform/nodeReplacement/useNodeReplacement.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,15 @@ export function useNodeReplacement() {
283283
life: 3000
284284
})
285285
}
286+
} catch (error) {
287+
console.error('Failed to replace nodes:', error)
288+
toastStore.add({
289+
severity: 'error',
290+
summary: t('g.error', 'Error'),
291+
detail: t('nodeReplacement.replaceFailed', 'Failed to replace nodes'),
292+
life: 5000
293+
})
294+
return []
286295
} finally {
287296
changeTracker?.afterChange()
288297
}

src/workbench/extensions/manager/utils/missingNodeErrorUtil.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,20 @@ describe('getCnrIdFromNode', () => {
4949
expect(getCnrIdFromNode(node)).toBe('node-pack')
5050
})
5151

52+
it('returns aux_id when cnr_id is absent', () => {
53+
const node = {
54+
properties: { aux_id: 'node-aux-pack' }
55+
} as unknown as LGraphNode
56+
expect(getCnrIdFromNode(node)).toBe('node-aux-pack')
57+
})
58+
59+
it('prefers cnr_id over aux_id in node properties', () => {
60+
const node = {
61+
properties: { cnr_id: 'primary', aux_id: 'secondary' }
62+
} as unknown as LGraphNode
63+
expect(getCnrIdFromNode(node)).toBe('primary')
64+
})
65+
5266
it('returns undefined when node has no cnr_id or aux_id', () => {
5367
const node = { properties: {} } as unknown as LGraphNode
5468
expect(getCnrIdFromNode(node)).toBeUndefined()

0 commit comments

Comments
 (0)