From a0e518aa98585d92590bdc74b1f19c1b0980693d Mon Sep 17 00:00:00 2001 From: jaeone94 <89377375+jaeone94@users.noreply.github.com> Date: Sat, 28 Feb 2026 23:17:30 +0900 Subject: [PATCH] refactor(node-replacement): reorganize domain components and expand comprehensive test suite (#9301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Resolves six open issues by reorganizing node replacement components into a domain-driven folder structure, refactoring event handling to follow the emit pattern, and adding comprehensive test coverage across all affected modules. ## Changes - **What**: - Moved `SwapNodeGroupRow.vue` and `SwapNodesCard.vue` from `src/components/rightSidePanel/errors/` to `src/platform/nodeReplacement/components/` (Issues #9255) - Moved `useMissingNodeScan.ts` from `src/composables/` to `src/platform/nodeReplacement/missingNodeScan.ts`, renamed to reflect it is a plain function not a Vue composable (Issues #9254) - Refactored `SwapNodeGroupRow.vue` to emit a `'replace'` event instead of calling `useNodeReplacement()` and `useExecutionErrorStore()` directly; replacement logic now handled in `TabErrors.vue` (Issue #9267) - Added unit tests for `removeMissingNodesByType` (`executionErrorStore.test.ts`), `scanMissingNodes` (`missingNodeScan.test.ts`), and `swapNodeGroups` computed (`swapNodeGroups.test.ts`, `useErrorGroups.test.ts`) (Issue #9270) - Added placeholder detection tests covering unregistered-type detection when `has_errors` is false, and exclusion of registered types (`useNodeReplacement.test.ts`) (Issue #9271) - Added component tests for `MissingNodeCard` and `MissingPackGroupRow` covering rendering, expand/collapse, events, install states, and edge cases (Issue #9231) - Added component tests for `SwapNodeGroupRow` and `SwapNodesCard` (Issues #9255, #9267) ## Additional Changes (Post-Review) - **Edge case guard in placeholder detection** (`useNodeReplacement.ts`): When `last_serialization.type` is absent (old serialization format), the predicate falls back to `n.type`, which the app may have already run through `sanitizeNodeName` — stripping HTML special characters (`& < > " ' \` =`). In that case, a `Set.has()` lookup against the original unsanitized type name would silently miss, causing replacement to be skipped. Fixed by including sanitized variants of each target type in the `targetTypes` Set at construction time. For the overwhelmingly common case (no special characters in type names), the Set deduplicates the entries and runtime behavior is identical to before. A regression test was added to cover the specific scenario: `last_serialization.type` absent + live `n.type` already sanitized. ## Review Focus - `TabErrors.vue`: confirm the new `@replace` event handler correctly replaces nodes and removes them from missing nodes list (mirrors the old inline logic in `SwapNodeGroupRow`) - `missingNodeScan.ts`: filename/export name change from `useMissingNodeScan` — verify all call sites updated via `app.ts` - Test mocking strategy: module-level `vi.mock()` factories use closures over `ref`/plain objects to allow per-test overrides without global mutable state - Fixes #9231 - Fixes #9254 - Fixes #9255 - Fixes #9267 - Fixes #9270 - Fixes #9271 --- .../errors/MissingNodeCard.test.ts | 214 +++++++++ .../errors/MissingPackGroupRow.test.ts | 368 +++++++++++++++ .../rightSidePanel/errors/TabErrors.vue | 18 +- .../errors/swapNodeGroups.test.ts | 187 ++++++++ .../errors/useErrorGroups.test.ts | 439 ++++++++++++++++++ .../components/SwapNodeGroupRow.test.ts | 244 ++++++++++ .../components}/SwapNodeGroupRow.vue | 12 +- .../components/SwapNodesCard.test.ts | 129 +++++ .../components}/SwapNodesCard.vue | 6 +- .../nodeReplacement/missingNodeScan.test.ts | 232 +++++++++ .../nodeReplacement/missingNodeScan.ts} | 0 .../useNodeReplacement.test.ts | 324 +++++++++++++ .../nodeReplacement/useNodeReplacement.ts | 47 +- src/scripts/app.ts | 2 +- src/stores/executionErrorStore.test.ts | 159 +++++++ 15 files changed, 2356 insertions(+), 25 deletions(-) create mode 100644 src/components/rightSidePanel/errors/MissingNodeCard.test.ts create mode 100644 src/components/rightSidePanel/errors/MissingPackGroupRow.test.ts create mode 100644 src/components/rightSidePanel/errors/swapNodeGroups.test.ts create mode 100644 src/components/rightSidePanel/errors/useErrorGroups.test.ts create mode 100644 src/platform/nodeReplacement/components/SwapNodeGroupRow.test.ts rename src/{components/rightSidePanel/errors => platform/nodeReplacement/components}/SwapNodeGroupRow.vue (90%) create mode 100644 src/platform/nodeReplacement/components/SwapNodesCard.test.ts rename src/{components/rightSidePanel/errors => platform/nodeReplacement/components}/SwapNodesCard.vue (78%) create mode 100644 src/platform/nodeReplacement/missingNodeScan.test.ts rename src/{composables/useMissingNodeScan.ts => platform/nodeReplacement/missingNodeScan.ts} (100%) create mode 100644 src/stores/executionErrorStore.test.ts diff --git a/src/components/rightSidePanel/errors/MissingNodeCard.test.ts b/src/components/rightSidePanel/errors/MissingNodeCard.test.ts new file mode 100644 index 0000000000..08380b4795 --- /dev/null +++ b/src/components/rightSidePanel/errors/MissingNodeCard.test.ts @@ -0,0 +1,214 @@ +import { mount } from '@vue/test-utils' +import { createTestingPinia } from '@pinia/testing' +import PrimeVue from 'primevue/config' +import { ref } from 'vue' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createI18n } from 'vue-i18n' + +import type { MissingPackGroup } from '@/components/rightSidePanel/errors/useErrorGroups' + +const mockIsCloud = { value: false } +vi.mock('@/platform/distribution/types', () => ({ + get isCloud() { + return mockIsCloud.value + } +})) + +const mockApplyChanges = vi.fn() +const mockIsRestarting = ref(false) +vi.mock('@/workbench/extensions/manager/composables/useApplyChanges', () => ({ + useApplyChanges: () => ({ + isRestarting: mockIsRestarting, + applyChanges: mockApplyChanges + }) +})) + +const mockIsPackInstalled = vi.fn(() => false) +vi.mock('@/workbench/extensions/manager/stores/comfyManagerStore', () => ({ + useComfyManagerStore: () => ({ + isPackInstalled: mockIsPackInstalled + }) +})) + +const mockShouldShowManagerButtons = { value: false } +vi.mock('@/workbench/extensions/manager/composables/useManagerState', () => ({ + useManagerState: () => ({ + shouldShowManagerButtons: mockShouldShowManagerButtons + }) +})) + +vi.mock('./MissingPackGroupRow.vue', () => ({ + default: { + name: 'MissingPackGroupRow', + template: '
', + props: ['group', 'showInfoButton', 'showNodeIdBadge'], + emits: ['locate-node', 'open-manager-info'] + } +})) + +import MissingNodeCard from './MissingNodeCard.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { + en: { + rightSidePanel: { + missingNodePacks: { + ossMessage: 'Missing node packs detected. Install them.', + cloudMessage: 'Unsupported node packs detected.', + applyChanges: 'Apply Changes' + } + } + } + }, + missingWarn: false, + fallbackWarn: false +}) + +function makePackGroups(count = 2): MissingPackGroup[] { + return Array.from({ length: count }, (_, i) => ({ + packId: `pack-${i}`, + nodeTypes: [ + { type: `MissingNode${i}`, nodeId: String(i), isReplaceable: false } + ], + isResolving: false + })) +} + +function mountCard( + props: Partial<{ + showInfoButton: boolean + showNodeIdBadge: boolean + missingPackGroups: MissingPackGroup[] + }> = {} +) { + return mount(MissingNodeCard, { + props: { + showInfoButton: false, + showNodeIdBadge: false, + missingPackGroups: makePackGroups(), + ...props + }, + global: { + plugins: [createTestingPinia({ createSpy: vi.fn }), PrimeVue, i18n], + stubs: { + DotSpinner: { template: '' } + } + } + }) +} + +describe('MissingNodeCard', () => { + beforeEach(() => { + mockApplyChanges.mockClear() + mockIsPackInstalled.mockReset() + mockIsPackInstalled.mockReturnValue(false) + mockIsCloud.value = false + mockShouldShowManagerButtons.value = false + mockIsRestarting.value = false + }) + + describe('Rendering & Props', () => { + it('renders cloud message when isCloud is true', () => { + mockIsCloud.value = true + const wrapper = mountCard() + expect(wrapper.text()).toContain('Unsupported node packs detected') + }) + + it('renders OSS message when isCloud is false', () => { + const wrapper = mountCard() + expect(wrapper.text()).toContain('Missing node packs detected') + }) + + it('renders correct number of MissingPackGroupRow components', () => { + const wrapper = mountCard({ missingPackGroups: makePackGroups(3) }) + expect( + wrapper.findAllComponents({ name: 'MissingPackGroupRow' }) + ).toHaveLength(3) + }) + + it('renders zero rows when missingPackGroups is empty', () => { + const wrapper = mountCard({ missingPackGroups: [] }) + expect( + wrapper.findAllComponents({ name: 'MissingPackGroupRow' }) + ).toHaveLength(0) + }) + + it('passes props correctly to MissingPackGroupRow children', () => { + const wrapper = mountCard({ + showInfoButton: true, + showNodeIdBadge: true + }) + const row = wrapper.findComponent({ name: 'MissingPackGroupRow' }) + expect(row.props('showInfoButton')).toBe(true) + expect(row.props('showNodeIdBadge')).toBe(true) + }) + }) + + describe('Apply Changes Section', () => { + it('hides Apply Changes when manager is not enabled', () => { + mockShouldShowManagerButtons.value = false + const wrapper = mountCard() + expect(wrapper.text()).not.toContain('Apply Changes') + }) + + it('hides Apply Changes when manager enabled but no packs pending', () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(false) + const wrapper = mountCard() + expect(wrapper.text()).not.toContain('Apply Changes') + }) + + it('shows Apply Changes when at least one pack is pending restart', () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(true) + const wrapper = mountCard() + expect(wrapper.text()).toContain('Apply Changes') + }) + + it('displays spinner during restart', () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(true) + mockIsRestarting.value = true + const wrapper = mountCard() + expect(wrapper.find('[role="status"]').exists()).toBe(true) + }) + + it('disables button during restart', () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(true) + mockIsRestarting.value = true + const wrapper = mountCard() + const btn = wrapper.find('button') + expect(btn.attributes('disabled')).toBeDefined() + }) + + it('calls applyChanges when Apply Changes button is clicked', async () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(true) + const wrapper = mountCard() + const btn = wrapper.find('button') + await btn.trigger('click') + expect(mockApplyChanges).toHaveBeenCalledOnce() + }) + }) + + describe('Event Handling', () => { + it('emits locateNode when child emits locate-node', async () => { + const wrapper = mountCard() + const row = wrapper.findComponent({ name: 'MissingPackGroupRow' }) + await row.vm.$emit('locate-node', '42') + expect(wrapper.emitted('locateNode')).toBeTruthy() + expect(wrapper.emitted('locateNode')?.[0]).toEqual(['42']) + }) + + it('emits openManagerInfo when child emits open-manager-info', async () => { + const wrapper = mountCard() + const row = wrapper.findComponent({ name: 'MissingPackGroupRow' }) + await row.vm.$emit('open-manager-info', 'pack-0') + expect(wrapper.emitted('openManagerInfo')).toBeTruthy() + expect(wrapper.emitted('openManagerInfo')?.[0]).toEqual(['pack-0']) + }) + }) +}) diff --git a/src/components/rightSidePanel/errors/MissingPackGroupRow.test.ts b/src/components/rightSidePanel/errors/MissingPackGroupRow.test.ts new file mode 100644 index 0000000000..b923a4f0e9 --- /dev/null +++ b/src/components/rightSidePanel/errors/MissingPackGroupRow.test.ts @@ -0,0 +1,368 @@ +import { mount } from '@vue/test-utils' +import { createTestingPinia } from '@pinia/testing' +import PrimeVue from 'primevue/config' +import { ref } from 'vue' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createI18n } from 'vue-i18n' + +import type { MissingPackGroup } from '@/components/rightSidePanel/errors/useErrorGroups' + +const mockInstallAllPacks = vi.fn() +const mockIsInstalling = ref(false) +const mockIsPackInstalled = vi.fn(() => false) +const mockShouldShowManagerButtons = { value: false } +const mockOpenManager = vi.fn() +const mockMissingNodePacks = ref>([]) +const mockIsLoading = ref(false) + +vi.mock( + '@/workbench/extensions/manager/composables/nodePack/useMissingNodes', + () => ({ + useMissingNodes: () => ({ + missingNodePacks: mockMissingNodePacks, + isLoading: mockIsLoading + }) + }) +) + +vi.mock( + '@/workbench/extensions/manager/composables/nodePack/usePackInstall', + () => ({ + usePackInstall: () => ({ + isInstalling: mockIsInstalling, + installAllPacks: mockInstallAllPacks + }) + }) +) + +vi.mock('@/workbench/extensions/manager/stores/comfyManagerStore', () => ({ + useComfyManagerStore: () => ({ + isPackInstalled: mockIsPackInstalled + }) +})) + +vi.mock('@/workbench/extensions/manager/composables/useManagerState', () => ({ + useManagerState: () => ({ + shouldShowManagerButtons: mockShouldShowManagerButtons, + openManager: mockOpenManager + }) +})) + +vi.mock('@/workbench/extensions/manager/types/comfyManagerTypes', () => ({ + ManagerTab: { Missing: 'missing', All: 'all' } +})) + +import MissingPackGroupRow from './MissingPackGroupRow.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { + en: { + g: { + loading: 'Loading' + }, + rightSidePanel: { + locateNode: 'Locate node on canvas', + missingNodePacks: { + unknownPack: 'Unknown pack', + installNodePack: 'Install node pack', + installing: 'Installing...', + installed: 'Installed', + searchInManager: 'Search in Node Manager', + viewInManager: 'View in Manager', + collapse: 'Collapse', + expand: 'Expand' + } + } + } + }, + missingWarn: false, + fallbackWarn: false +}) + +function makeGroup( + overrides: Partial = {} +): MissingPackGroup { + return { + packId: 'my-pack', + nodeTypes: [ + { type: 'MissingA', nodeId: '10', isReplaceable: false }, + { type: 'MissingB', nodeId: '11', isReplaceable: false } + ], + isResolving: false, + ...overrides + } +} + +function mountRow( + props: Partial<{ + group: MissingPackGroup + showInfoButton: boolean + showNodeIdBadge: boolean + }> = {} +) { + return mount(MissingPackGroupRow, { + props: { + group: makeGroup(), + showInfoButton: false, + showNodeIdBadge: false, + ...props + }, + global: { + plugins: [createTestingPinia({ createSpy: vi.fn }), PrimeVue, i18n], + stubs: { + TransitionCollapse: { template: '
' }, + DotSpinner: { + template: '' + } + } + } + }) +} + +describe('MissingPackGroupRow', () => { + beforeEach(() => { + mockInstallAllPacks.mockClear() + mockOpenManager.mockClear() + mockIsPackInstalled.mockReset() + mockIsPackInstalled.mockReturnValue(false) + mockShouldShowManagerButtons.value = false + mockIsInstalling.value = false + mockMissingNodePacks.value = [] + mockIsLoading.value = false + }) + + describe('Basic Rendering', () => { + it('renders pack name from packId', () => { + const wrapper = mountRow() + expect(wrapper.text()).toContain('my-pack') + }) + + it('renders "Unknown pack" when packId is null', () => { + const wrapper = mountRow({ group: makeGroup({ packId: null }) }) + expect(wrapper.text()).toContain('Unknown pack') + }) + + it('renders loading text when isResolving is true', () => { + const wrapper = mountRow({ group: makeGroup({ isResolving: true }) }) + expect(wrapper.text()).toContain('Loading') + }) + + it('renders node count', () => { + const wrapper = mountRow() + expect(wrapper.text()).toContain('(2)') + }) + + it('renders count of 5 for 5 nodeTypes', () => { + const wrapper = mountRow({ + group: makeGroup({ + nodeTypes: Array.from({ length: 5 }, (_, i) => ({ + type: `Node${i}`, + nodeId: String(i), + isReplaceable: false + })) + }) + }) + expect(wrapper.text()).toContain('(5)') + }) + }) + + describe('Expand / Collapse', () => { + it('starts collapsed', () => { + const wrapper = mountRow() + expect(wrapper.text()).not.toContain('MissingA') + }) + + it('expands when chevron is clicked', async () => { + const wrapper = mountRow() + await wrapper.get('button[aria-label="Expand"]').trigger('click') + expect(wrapper.text()).toContain('MissingA') + expect(wrapper.text()).toContain('MissingB') + }) + + it('collapses when chevron is clicked again', async () => { + const wrapper = mountRow() + await wrapper.get('button[aria-label="Expand"]').trigger('click') + expect(wrapper.text()).toContain('MissingA') + await wrapper.get('button[aria-label="Collapse"]').trigger('click') + expect(wrapper.text()).not.toContain('MissingA') + }) + }) + + describe('Node Type List', () => { + async function expand(wrapper: ReturnType) { + await wrapper.get('button[aria-label="Expand"]').trigger('click') + } + + it('renders all nodeTypes when expanded', async () => { + const wrapper = mountRow({ + group: makeGroup({ + nodeTypes: [ + { type: 'NodeA', nodeId: '1', isReplaceable: false }, + { type: 'NodeB', nodeId: '2', isReplaceable: false }, + { type: 'NodeC', nodeId: '3', isReplaceable: false } + ] + }) + }) + await expand(wrapper) + expect(wrapper.text()).toContain('NodeA') + expect(wrapper.text()).toContain('NodeB') + expect(wrapper.text()).toContain('NodeC') + }) + + it('shows nodeId badge when showNodeIdBadge is true', async () => { + const wrapper = mountRow({ showNodeIdBadge: true }) + await expand(wrapper) + expect(wrapper.text()).toContain('#10') + }) + + it('hides nodeId badge when showNodeIdBadge is false', async () => { + const wrapper = mountRow({ showNodeIdBadge: false }) + await expand(wrapper) + expect(wrapper.text()).not.toContain('#10') + }) + + it('emits locateNode when Locate button is clicked', async () => { + const wrapper = mountRow({ showNodeIdBadge: true }) + await expand(wrapper) + await wrapper + .get('button[aria-label="Locate node on canvas"]') + .trigger('click') + expect(wrapper.emitted('locateNode')).toBeTruthy() + expect(wrapper.emitted('locateNode')?.[0]).toEqual(['10']) + }) + + it('does not show Locate for nodeType without nodeId', async () => { + const wrapper = mountRow({ + group: makeGroup({ + nodeTypes: [{ type: 'NoId', isReplaceable: false } as never] + }) + }) + await expand(wrapper) + expect( + wrapper.find('button[aria-label="Locate node on canvas"]').exists() + ).toBe(false) + }) + + it('handles mixed nodeTypes with and without nodeId', async () => { + const wrapper = mountRow({ + showNodeIdBadge: true, + group: makeGroup({ + nodeTypes: [ + { type: 'WithId', nodeId: '100', isReplaceable: false }, + { type: 'WithoutId', isReplaceable: false } as never + ] + }) + }) + await expand(wrapper) + expect(wrapper.text()).toContain('WithId') + expect(wrapper.text()).toContain('WithoutId') + expect( + wrapper.findAll('button[aria-label="Locate node on canvas"]') + ).toHaveLength(1) + }) + }) + + describe('Manager Integration', () => { + it('hides install UI when shouldShowManagerButtons is false', () => { + mockShouldShowManagerButtons.value = false + const wrapper = mountRow() + expect(wrapper.text()).not.toContain('Install node pack') + }) + + it('hides install UI when packId is null', () => { + mockShouldShowManagerButtons.value = true + const wrapper = mountRow({ group: makeGroup({ packId: null }) }) + expect(wrapper.text()).not.toContain('Install node pack') + }) + + it('shows "Search in Node Manager" when packId exists but pack not in registry', () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(false) + mockMissingNodePacks.value = [] + const wrapper = mountRow() + expect(wrapper.text()).toContain('Search in Node Manager') + }) + + it('shows "Installed" state when pack is installed', () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(true) + mockMissingNodePacks.value = [{ id: 'my-pack', name: 'My Pack' }] + const wrapper = mountRow() + expect(wrapper.text()).toContain('Installed') + }) + + it('shows spinner when installing', () => { + mockShouldShowManagerButtons.value = true + mockIsInstalling.value = true + mockMissingNodePacks.value = [{ id: 'my-pack', name: 'My Pack' }] + const wrapper = mountRow() + expect(wrapper.find('[role="status"]').exists()).toBe(true) + }) + + it('shows install button when not installed and pack found', () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(false) + mockMissingNodePacks.value = [{ id: 'my-pack', name: 'My Pack' }] + const wrapper = mountRow() + expect(wrapper.text()).toContain('Install node pack') + }) + + it('calls installAllPacks when Install button is clicked', async () => { + mockShouldShowManagerButtons.value = true + mockIsPackInstalled.mockReturnValue(false) + mockMissingNodePacks.value = [{ id: 'my-pack', name: 'My Pack' }] + const wrapper = mountRow() + await wrapper.get('button:not([aria-label])').trigger('click') + expect(mockInstallAllPacks).toHaveBeenCalledOnce() + }) + + it('shows loading spinner when registry is loading', () => { + mockShouldShowManagerButtons.value = true + mockIsLoading.value = true + const wrapper = mountRow() + expect(wrapper.find('[role="status"]').exists()).toBe(true) + }) + }) + + describe('Info Button', () => { + it('shows Info button when showInfoButton true and packId not null', () => { + const wrapper = mountRow({ showInfoButton: true }) + expect( + wrapper.find('button[aria-label="View in Manager"]').exists() + ).toBe(true) + }) + + it('hides Info button when showInfoButton is false', () => { + const wrapper = mountRow({ showInfoButton: false }) + expect( + wrapper.find('button[aria-label="View in Manager"]').exists() + ).toBe(false) + }) + + it('hides Info button when packId is null', () => { + const wrapper = mountRow({ + showInfoButton: true, + group: makeGroup({ packId: null }) + }) + expect( + wrapper.find('button[aria-label="View in Manager"]').exists() + ).toBe(false) + }) + + it('emits openManagerInfo when Info button is clicked', async () => { + const wrapper = mountRow({ showInfoButton: true }) + await wrapper.get('button[aria-label="View in Manager"]').trigger('click') + expect(wrapper.emitted('openManagerInfo')).toBeTruthy() + expect(wrapper.emitted('openManagerInfo')?.[0]).toEqual(['my-pack']) + }) + }) + + describe('Edge Cases', () => { + it('handles empty nodeTypes array', () => { + const wrapper = mountRow({ group: makeGroup({ nodeTypes: [] }) }) + expect(wrapper.text()).toContain('(0)') + }) + }) +}) diff --git a/src/components/rightSidePanel/errors/TabErrors.vue b/src/components/rightSidePanel/errors/TabErrors.vue index 7f40b9e63b..5043efe04d 100644 --- a/src/components/rightSidePanel/errors/TabErrors.vue +++ b/src/components/rightSidePanel/errors/TabErrors.vue @@ -109,6 +109,7 @@ :swap-node-groups="swapNodeGroups" :show-node-id-badge="showNodeIdBadge" @locate-node="handleLocateMissingNode" + @replace="handleReplaceGroup" /> @@ -179,14 +180,14 @@ import PropertiesAccordionItem from '../layout/PropertiesAccordionItem.vue' import FormSearchInput from '@/renderer/extensions/vueNodes/widgets/components/form/FormSearchInput.vue' import ErrorNodeCard from './ErrorNodeCard.vue' import MissingNodeCard from './MissingNodeCard.vue' -import SwapNodesCard from './SwapNodesCard.vue' +import SwapNodesCard from '@/platform/nodeReplacement/components/SwapNodesCard.vue' import Button from '@/components/ui/button/Button.vue' import DotSpinner from '@/components/common/DotSpinner.vue' import { usePackInstall } from '@/workbench/extensions/manager/composables/nodePack/usePackInstall' import { useMissingNodes } from '@/workbench/extensions/manager/composables/nodePack/useMissingNodes' import { useErrorGroups } from './useErrorGroups' +import type { SwapNodeGroup } from './useErrorGroups' import { useNodeReplacement } from '@/platform/nodeReplacement/useNodeReplacement' -import { useExecutionErrorStore } from '@/stores/executionErrorStore' const { t } = useI18n() const { copyToClipboard } = useCopyToClipboard() @@ -199,8 +200,7 @@ const { shouldShowManagerButtons, shouldShowInstallButton, openManager } = const { missingNodePacks } = useMissingNodes() const { isInstalling: isInstallingAll, installAllPacks: installAll } = usePackInstall(() => missingNodePacks.value) -const { replaceNodesInPlace } = useNodeReplacement() -const executionErrorStore = useExecutionErrorStore() +const { replaceGroup, replaceAllGroups } = useNodeReplacement() const searchQuery = ref('') @@ -264,12 +264,12 @@ function handleOpenManagerInfo(packId: string) { } } +function handleReplaceGroup(group: SwapNodeGroup) { + replaceGroup(group) +} + function handleReplaceAll() { - const allNodeTypes = swapNodeGroups.value.flatMap((g) => g.nodeTypes) - const replaced = replaceNodesInPlace(allNodeTypes) - if (replaced.length > 0) { - executionErrorStore.removeMissingNodesByType(replaced) - } + replaceAllGroups(swapNodeGroups.value) } function handleEnterSubgraph(nodeId: string) { diff --git a/src/components/rightSidePanel/errors/swapNodeGroups.test.ts b/src/components/rightSidePanel/errors/swapNodeGroups.test.ts new file mode 100644 index 0000000000..de497e3cbc --- /dev/null +++ b/src/components/rightSidePanel/errors/swapNodeGroups.test.ts @@ -0,0 +1,187 @@ +import { createPinia, setActivePinia } from 'pinia' +import { nextTick, ref } from 'vue' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { MissingNodeType } from '@/types/comfy' + +vi.mock('@/scripts/app', () => ({ + app: { + rootGraph: { + serialize: vi.fn(() => ({})), + getNodeById: vi.fn() + } + } +})) + +vi.mock('@/utils/graphTraversalUtil', () => ({ + getNodeByExecutionId: vi.fn(), + getExecutionIdByNode: vi.fn(), + getRootParentNode: vi.fn(() => null), + forEachNode: vi.fn(), + mapAllNodes: vi.fn(() => []) +})) + +vi.mock('@/platform/distribution/types', () => ({ + isCloud: false +})) + +vi.mock('@/i18n', () => ({ + st: vi.fn((_key: string, fallback: string) => fallback) +})) + +vi.mock('@/stores/comfyRegistryStore', () => ({ + useComfyRegistryStore: () => ({ + inferPackFromNodeName: vi.fn() + }) +})) + +vi.mock('@/utils/nodeTitleUtil', () => ({ + resolveNodeDisplayName: vi.fn(() => '') +})) + +vi.mock('@/utils/litegraphUtil', () => ({ + isLGraphNode: vi.fn(() => false) +})) + +vi.mock('@/utils/executableGroupNodeDto', () => ({ + isGroupNode: vi.fn(() => false) +})) + +import { useExecutionErrorStore } from '@/stores/executionErrorStore' +import { useErrorGroups } from './useErrorGroups' + +function makeMissingNodeType( + type: string, + opts: { + nodeId?: string + isReplaceable?: boolean + replacement?: { new_node_id: string } + } = {} +): MissingNodeType { + return { + type, + nodeId: opts.nodeId ?? '1', + isReplaceable: opts.isReplaceable ?? false, + replacement: opts.replacement + ? { + old_node_id: type, + new_node_id: opts.replacement.new_node_id, + old_widget_ids: null, + input_mapping: null, + output_mapping: null + } + : undefined + } +} + +describe('swapNodeGroups computed', () => { + beforeEach(() => { + setActivePinia(createPinia()) + }) + + function getSwapNodeGroups(nodeTypes: MissingNodeType[]) { + const store = useExecutionErrorStore() + store.surfaceMissingNodes(nodeTypes) + + const searchQuery = ref('') + const t = (key: string) => key + const { swapNodeGroups } = useErrorGroups(searchQuery, t) + return swapNodeGroups + } + + it('returns empty array when no missing nodes', () => { + const swap = getSwapNodeGroups([]) + expect(swap.value).toEqual([]) + }) + + it('returns empty array when no nodes are replaceable', () => { + const swap = getSwapNodeGroups([ + makeMissingNodeType('NodeA', { isReplaceable: false }), + makeMissingNodeType('NodeB', { isReplaceable: false }) + ]) + expect(swap.value).toEqual([]) + }) + + it('groups replaceable nodes by type', async () => { + const swap = getSwapNodeGroups([ + makeMissingNodeType('OldNode', { + nodeId: '1', + isReplaceable: true, + replacement: { new_node_id: 'NewNode' } + }), + makeMissingNodeType('OldNode', { + nodeId: '2', + isReplaceable: true, + replacement: { new_node_id: 'NewNode' } + }) + ]) + await nextTick() + expect(swap.value).toHaveLength(1) + expect(swap.value[0].type).toBe('OldNode') + expect(swap.value[0].newNodeId).toBe('NewNode') + expect(swap.value[0].nodeTypes).toHaveLength(2) + }) + + it('creates separate groups for different types', async () => { + const swap = getSwapNodeGroups([ + makeMissingNodeType('TypeA', { + nodeId: '1', + isReplaceable: true, + replacement: { new_node_id: 'NewA' } + }), + makeMissingNodeType('TypeB', { + nodeId: '2', + isReplaceable: true, + replacement: { new_node_id: 'NewB' } + }) + ]) + await nextTick() + expect(swap.value).toHaveLength(2) + expect(swap.value.map((g) => g.type)).toEqual(['TypeA', 'TypeB']) + }) + + it('sorts groups alphabetically by type', async () => { + const swap = getSwapNodeGroups([ + makeMissingNodeType('Zebra', { + nodeId: '1', + isReplaceable: true, + replacement: { new_node_id: 'NewZ' } + }), + makeMissingNodeType('Alpha', { + nodeId: '2', + isReplaceable: true, + replacement: { new_node_id: 'NewA' } + }) + ]) + await nextTick() + expect(swap.value[0].type).toBe('Alpha') + expect(swap.value[1].type).toBe('Zebra') + }) + + it('excludes string nodeType entries', async () => { + const swap = getSwapNodeGroups([ + 'StringGroupNode' as unknown as MissingNodeType, + makeMissingNodeType('OldNode', { + nodeId: '1', + isReplaceable: true, + replacement: { new_node_id: 'NewNode' } + }) + ]) + await nextTick() + expect(swap.value).toHaveLength(1) + expect(swap.value[0].type).toBe('OldNode') + }) + + it('sets newNodeId to undefined when replacement is missing', async () => { + const swap = getSwapNodeGroups([ + makeMissingNodeType('OldNode', { + nodeId: '1', + isReplaceable: true + // no replacement + }) + ]) + await nextTick() + expect(swap.value).toHaveLength(1) + expect(swap.value[0].newNodeId).toBeUndefined() + }) +}) diff --git a/src/components/rightSidePanel/errors/useErrorGroups.test.ts b/src/components/rightSidePanel/errors/useErrorGroups.test.ts new file mode 100644 index 0000000000..8a7f07a513 --- /dev/null +++ b/src/components/rightSidePanel/errors/useErrorGroups.test.ts @@ -0,0 +1,439 @@ +import { createPinia, setActivePinia } from 'pinia' +import { nextTick, ref } from 'vue' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { MissingNodeType } from '@/types/comfy' + +vi.mock('@/scripts/app', () => ({ + app: { + rootGraph: { + serialize: vi.fn(() => ({})), + getNodeById: vi.fn() + } + } +})) + +vi.mock('@/utils/graphTraversalUtil', () => ({ + getNodeByExecutionId: vi.fn(), + getExecutionIdByNode: vi.fn(), + getRootParentNode: vi.fn(() => null), + forEachNode: vi.fn(), + mapAllNodes: vi.fn(() => []) +})) + +vi.mock('@/platform/distribution/types', () => ({ + isCloud: false +})) + +vi.mock('@/i18n', () => ({ + st: vi.fn((_key: string, fallback: string) => fallback) +})) + +vi.mock('@/stores/comfyRegistryStore', () => ({ + useComfyRegistryStore: () => ({ + inferPackFromNodeName: vi.fn() + }) +})) + +vi.mock('@/utils/nodeTitleUtil', () => ({ + resolveNodeDisplayName: vi.fn(() => '') +})) + +vi.mock('@/utils/litegraphUtil', () => ({ + isLGraphNode: vi.fn(() => false) +})) + +vi.mock('@/utils/executableGroupNodeDto', () => ({ + isGroupNode: vi.fn(() => false) +})) + +import { useExecutionErrorStore } from '@/stores/executionErrorStore' +import { useErrorGroups } from './useErrorGroups' + +function makeMissingNodeType( + type: string, + opts: { + nodeId?: string + isReplaceable?: boolean + cnrId?: string + replacement?: { new_node_id: string } + } = {} +): MissingNodeType { + return { + type, + nodeId: opts.nodeId ?? '1', + isReplaceable: opts.isReplaceable ?? false, + cnrId: opts.cnrId, + replacement: opts.replacement + ? { + old_node_id: type, + new_node_id: opts.replacement.new_node_id, + old_widget_ids: null, + input_mapping: null, + output_mapping: null + } + : undefined + } +} + +function createErrorGroups() { + const store = useExecutionErrorStore() + const searchQuery = ref('') + const t = (key: string) => key + const groups = useErrorGroups(searchQuery, t) + return { store, searchQuery, groups } +} + +describe('useErrorGroups', () => { + beforeEach(() => { + setActivePinia(createPinia()) + }) + + describe('missingPackGroups', () => { + it('returns empty array when no missing nodes', () => { + const { groups } = createErrorGroups() + expect(groups.missingPackGroups.value).toEqual([]) + }) + + it('groups non-replaceable nodes by cnrId', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('NodeA', { cnrId: 'pack-1' }), + makeMissingNodeType('NodeB', { cnrId: 'pack-1', nodeId: '2' }), + makeMissingNodeType('NodeC', { cnrId: 'pack-2', nodeId: '3' }) + ]) + await nextTick() + + expect(groups.missingPackGroups.value).toHaveLength(2) + const pack1 = groups.missingPackGroups.value.find( + (g) => g.packId === 'pack-1' + ) + expect(pack1?.nodeTypes).toHaveLength(2) + const pack2 = groups.missingPackGroups.value.find( + (g) => g.packId === 'pack-2' + ) + expect(pack2?.nodeTypes).toHaveLength(1) + }) + + it('excludes replaceable nodes from missingPackGroups', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('OldNode', { + isReplaceable: true, + replacement: { new_node_id: 'NewNode' } + }), + makeMissingNodeType('MissingNode', { + nodeId: '2', + cnrId: 'some-pack' + }) + ]) + await nextTick() + + expect(groups.missingPackGroups.value).toHaveLength(1) + expect(groups.missingPackGroups.value[0].packId).toBe('some-pack') + }) + + it('groups nodes without cnrId under null packId', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('UnknownNode', { nodeId: '1' }), + makeMissingNodeType('AnotherUnknown', { nodeId: '2' }) + ]) + await nextTick() + + expect(groups.missingPackGroups.value).toHaveLength(1) + expect(groups.missingPackGroups.value[0].packId).toBeNull() + expect(groups.missingPackGroups.value[0].nodeTypes).toHaveLength(2) + }) + + it('sorts groups alphabetically with null packId last', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('NodeA', { cnrId: 'zebra-pack' }), + makeMissingNodeType('NodeB', { nodeId: '2' }), + makeMissingNodeType('NodeC', { cnrId: 'alpha-pack', nodeId: '3' }) + ]) + await nextTick() + + const packIds = groups.missingPackGroups.value.map((g) => g.packId) + expect(packIds).toEqual(['alpha-pack', 'zebra-pack', null]) + }) + + it('sorts nodeTypes within each group alphabetically by type then nodeId', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('NodeB', { cnrId: 'pack-1', nodeId: '2' }), + makeMissingNodeType('NodeA', { cnrId: 'pack-1', nodeId: '3' }), + makeMissingNodeType('NodeA', { cnrId: 'pack-1', nodeId: '1' }) + ]) + await nextTick() + + const group = groups.missingPackGroups.value[0] + const types = group.nodeTypes.map((n) => + typeof n === 'string' ? n : `${n.type}:${n.nodeId}` + ) + expect(types).toEqual(['NodeA:1', 'NodeA:3', 'NodeB:2']) + }) + + it('handles string nodeType entries', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + 'StringGroupNode' as unknown as MissingNodeType + ]) + await nextTick() + + expect(groups.missingPackGroups.value).toHaveLength(1) + expect(groups.missingPackGroups.value[0].packId).toBeNull() + }) + }) + + describe('allErrorGroups', () => { + it('returns empty array when no errors', () => { + const { groups } = createErrorGroups() + expect(groups.allErrorGroups.value).toEqual([]) + }) + + it('includes missing_node group when missing nodes exist', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('NodeA', { cnrId: 'pack-1' }) + ]) + await nextTick() + + const missingGroup = groups.allErrorGroups.value.find( + (g) => g.type === 'missing_node' + ) + expect(missingGroup).toBeDefined() + }) + + it('includes swap_nodes group when replaceable nodes exist', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('OldNode', { + isReplaceable: true, + replacement: { new_node_id: 'NewNode' } + }) + ]) + await nextTick() + + const swapGroup = groups.allErrorGroups.value.find( + (g) => g.type === 'swap_nodes' + ) + expect(swapGroup).toBeDefined() + }) + + it('includes both swap_nodes and missing_node when both exist', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('OldNode', { + isReplaceable: true, + replacement: { new_node_id: 'NewNode' } + }), + makeMissingNodeType('MissingNode', { + nodeId: '2', + cnrId: 'some-pack' + }) + ]) + await nextTick() + + const types = groups.allErrorGroups.value.map((g) => g.type) + expect(types).toContain('swap_nodes') + expect(types).toContain('missing_node') + }) + + it('swap_nodes has lower priority than missing_node', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('OldNode', { + isReplaceable: true, + replacement: { new_node_id: 'NewNode' } + }), + makeMissingNodeType('MissingNode', { + nodeId: '2', + cnrId: 'some-pack' + }) + ]) + await nextTick() + + const swapIdx = groups.allErrorGroups.value.findIndex( + (g) => g.type === 'swap_nodes' + ) + const missingIdx = groups.allErrorGroups.value.findIndex( + (g) => g.type === 'missing_node' + ) + expect(swapIdx).toBeLessThan(missingIdx) + }) + + it('includes execution error groups from node errors', async () => { + const { store, groups } = createErrorGroups() + store.lastNodeErrors = { + '1': { + class_type: 'KSampler', + dependent_outputs: [], + errors: [ + { + type: 'value_not_valid', + message: 'Value not valid', + details: 'some detail' + } + ] + } + } + await nextTick() + + const execGroups = groups.allErrorGroups.value.filter( + (g) => g.type === 'execution' + ) + expect(execGroups.length).toBeGreaterThan(0) + }) + + it('includes execution error from runtime errors', async () => { + const { store, groups } = createErrorGroups() + store.lastExecutionError = { + prompt_id: 'test-prompt', + timestamp: Date.now(), + node_id: 5, + node_type: 'KSampler', + executed: [], + exception_type: 'RuntimeError', + exception_message: 'CUDA out of memory', + traceback: ['line 1', 'line 2'], + current_inputs: {}, + current_outputs: {} + } + await nextTick() + + const execGroups = groups.allErrorGroups.value.filter( + (g) => g.type === 'execution' + ) + expect(execGroups.length).toBeGreaterThan(0) + }) + + it('includes prompt error when present', async () => { + const { store, groups } = createErrorGroups() + store.lastPromptError = { + type: 'prompt_no_outputs', + message: 'No outputs', + details: '' + } + await nextTick() + + const promptGroup = groups.allErrorGroups.value.find( + (g) => g.type === 'execution' && g.title === 'No outputs' + ) + expect(promptGroup).toBeDefined() + }) + }) + + describe('filteredGroups', () => { + it('returns all groups when search query is empty', async () => { + const { store, groups } = createErrorGroups() + store.lastNodeErrors = { + '1': { + class_type: 'KSampler', + dependent_outputs: [], + errors: [{ type: 'value_error', message: 'Bad value', details: '' }] + } + } + await nextTick() + + expect(groups.filteredGroups.value.length).toBeGreaterThan(0) + }) + + it('filters groups based on search query', async () => { + const { store, groups, searchQuery } = createErrorGroups() + store.lastNodeErrors = { + '1': { + class_type: 'KSampler', + dependent_outputs: [], + errors: [ + { + type: 'value_error', + message: 'Value error in sampler', + details: '' + } + ] + }, + '2': { + class_type: 'CLIPLoader', + dependent_outputs: [], + errors: [ + { + type: 'file_not_found', + message: 'File not found', + details: '' + } + ] + } + } + await nextTick() + + searchQuery.value = 'sampler' + await nextTick() + + const executionGroups = groups.filteredGroups.value.filter( + (g) => g.type === 'execution' + ) + for (const group of executionGroups) { + if (group.type !== 'execution') continue + const hasMatch = group.cards.some( + (c) => + c.title.toLowerCase().includes('sampler') || + c.errors.some((e) => e.message.toLowerCase().includes('sampler')) + ) + expect(hasMatch).toBe(true) + } + }) + }) + + describe('groupedErrorMessages', () => { + it('returns empty array when no errors', () => { + const { groups } = createErrorGroups() + expect(groups.groupedErrorMessages.value).toEqual([]) + }) + + it('collects unique error messages from node errors', async () => { + const { store, groups } = createErrorGroups() + store.lastNodeErrors = { + '1': { + class_type: 'KSampler', + dependent_outputs: [], + errors: [ + { type: 'err_a', message: 'Error A', details: '' }, + { type: 'err_b', message: 'Error B', details: '' } + ] + }, + '2': { + class_type: 'CLIPLoader', + dependent_outputs: [], + errors: [{ type: 'err_a', message: 'Error A', details: '' }] + } + } + await nextTick() + + const messages = groups.groupedErrorMessages.value + expect(messages).toContain('Error A') + expect(messages).toContain('Error B') + // Deduplication: Error A appears twice but should only be listed once + expect(messages.filter((m) => m === 'Error A')).toHaveLength(1) + }) + + it('includes missing node group title as message', async () => { + const { store, groups } = createErrorGroups() + store.setMissingNodeTypes([ + makeMissingNodeType('NodeA', { cnrId: 'pack-1' }) + ]) + await nextTick() + + expect(groups.groupedErrorMessages.value.length).toBeGreaterThan(0) + }) + }) + + describe('collapseState', () => { + it('returns a reactive object', () => { + const { groups } = createErrorGroups() + expect(groups.collapseState).toBeDefined() + expect(typeof groups.collapseState).toBe('object') + }) + }) +}) diff --git a/src/platform/nodeReplacement/components/SwapNodeGroupRow.test.ts b/src/platform/nodeReplacement/components/SwapNodeGroupRow.test.ts new file mode 100644 index 0000000000..4c199ca9bf --- /dev/null +++ b/src/platform/nodeReplacement/components/SwapNodeGroupRow.test.ts @@ -0,0 +1,244 @@ +import { mount } from '@vue/test-utils' +import { createTestingPinia } from '@pinia/testing' +import PrimeVue from 'primevue/config' +import { describe, expect, it, vi } from 'vitest' +import { createI18n } from 'vue-i18n' + +import type { SwapNodeGroup } from '@/components/rightSidePanel/errors/useErrorGroups' +import type { MissingNodeType } from '@/types/comfy' +import SwapNodeGroupRow from './SwapNodeGroupRow.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { + en: { + rightSidePanel: { + locateNode: 'Locate Node', + missingNodePacks: { + collapse: 'Collapse', + expand: 'Expand' + } + }, + nodeReplacement: { + willBeReplacedBy: 'This node will be replaced by:', + replaceNode: 'Replace Node', + unknownNode: 'Unknown' + } + } + }, + missingWarn: false, + fallbackWarn: false +}) + +function makeGroup(overrides: Partial = {}): SwapNodeGroup { + return { + type: 'OldNodeType', + newNodeId: 'NewNodeType', + nodeTypes: [ + { type: 'OldNodeType', nodeId: '1', isReplaceable: true }, + { type: 'OldNodeType', nodeId: '2', isReplaceable: true } + ], + ...overrides + } +} + +function mountRow( + props: Partial<{ + group: SwapNodeGroup + showNodeIdBadge: boolean + }> = {} +) { + return mount(SwapNodeGroupRow, { + props: { + group: makeGroup(), + showNodeIdBadge: false, + ...props + }, + global: { + plugins: [createTestingPinia({ createSpy: vi.fn }), PrimeVue, i18n], + stubs: { + TransitionCollapse: { template: '
' } + } + } + }) +} + +describe('SwapNodeGroupRow', () => { + describe('Basic Rendering', () => { + it('renders the group type name', () => { + const wrapper = mountRow() + expect(wrapper.text()).toContain('OldNodeType') + }) + + it('renders node count in parentheses', () => { + const wrapper = mountRow() + expect(wrapper.text()).toContain('(2)') + }) + + it('renders node count of 5 for 5 nodeTypes', () => { + const wrapper = mountRow({ + group: makeGroup({ + nodeTypes: Array.from({ length: 5 }, (_, i) => ({ + type: 'OldNodeType', + nodeId: String(i), + isReplaceable: true + })) + }) + }) + expect(wrapper.text()).toContain('(5)') + }) + + it('renders the replacement target name', () => { + const wrapper = mountRow() + expect(wrapper.text()).toContain('NewNodeType') + }) + + it('shows "Unknown" when newNodeId is undefined', () => { + const wrapper = mountRow({ + group: makeGroup({ newNodeId: undefined }) + }) + expect(wrapper.text()).toContain('Unknown') + }) + + it('renders "Replace Node" button', () => { + const wrapper = mountRow() + expect(wrapper.text()).toContain('Replace Node') + }) + }) + + describe('Expand / Collapse', () => { + it('starts collapsed — node list not visible', () => { + const wrapper = mountRow({ showNodeIdBadge: true }) + expect(wrapper.text()).not.toContain('#1') + }) + + it('expands when chevron is clicked', async () => { + const wrapper = mountRow({ showNodeIdBadge: true }) + await wrapper.get('button[aria-label="Expand"]').trigger('click') + expect(wrapper.text()).toContain('#1') + expect(wrapper.text()).toContain('#2') + }) + + it('collapses when chevron is clicked again', async () => { + const wrapper = mountRow({ showNodeIdBadge: true }) + await wrapper.get('button[aria-label="Expand"]').trigger('click') + expect(wrapper.text()).toContain('#1') + await wrapper.get('button[aria-label="Collapse"]').trigger('click') + expect(wrapper.text()).not.toContain('#1') + }) + + it('updates the toggle control state when expanded', async () => { + const wrapper = mountRow() + expect(wrapper.find('button[aria-label="Expand"]').exists()).toBe(true) + await wrapper.get('button[aria-label="Expand"]').trigger('click') + expect(wrapper.find('button[aria-label="Collapse"]').exists()).toBe(true) + }) + }) + + describe('Node Type List (Expanded)', () => { + async function expand(wrapper: ReturnType) { + await wrapper.get('button[aria-label="Expand"]').trigger('click') + } + + it('renders all nodeTypes when expanded', async () => { + const wrapper = mountRow({ + group: makeGroup({ + nodeTypes: [ + { type: 'OldNodeType', nodeId: '10', isReplaceable: true }, + { type: 'OldNodeType', nodeId: '20', isReplaceable: true }, + { type: 'OldNodeType', nodeId: '30', isReplaceable: true } + ] + }), + showNodeIdBadge: true + }) + await expand(wrapper) + expect(wrapper.text()).toContain('#10') + expect(wrapper.text()).toContain('#20') + expect(wrapper.text()).toContain('#30') + }) + + it('shows nodeId badge when showNodeIdBadge is true', async () => { + const wrapper = mountRow({ showNodeIdBadge: true }) + await expand(wrapper) + expect(wrapper.text()).toContain('#1') + expect(wrapper.text()).toContain('#2') + }) + + it('hides nodeId badge when showNodeIdBadge is false', async () => { + const wrapper = mountRow({ showNodeIdBadge: false }) + await expand(wrapper) + expect(wrapper.text()).not.toContain('#1') + expect(wrapper.text()).not.toContain('#2') + }) + + it('renders Locate button for each nodeType with nodeId', async () => { + const wrapper = mountRow({ showNodeIdBadge: true }) + await expand(wrapper) + expect(wrapper.findAll('button[aria-label="Locate Node"]')).toHaveLength( + 2 + ) + }) + + it('does not render Locate button for nodeTypes without nodeId', async () => { + const wrapper = mountRow({ + group: makeGroup({ + // Intentionally omits nodeId to test graceful handling of incomplete node data + nodeTypes: [ + { type: 'NoIdNode', isReplaceable: true } + ] as unknown as MissingNodeType[] + }) + }) + await expand(wrapper) + expect(wrapper.find('button[aria-label="Locate Node"]').exists()).toBe( + false + ) + }) + }) + + describe('Events', () => { + it('emits locate-node with correct nodeId', async () => { + const wrapper = mountRow({ showNodeIdBadge: true }) + await wrapper.get('button[aria-label="Expand"]').trigger('click') + const locateBtns = wrapper.findAll('button[aria-label="Locate Node"]') + await locateBtns[0].trigger('click') + expect(wrapper.emitted('locate-node')).toBeTruthy() + expect(wrapper.emitted('locate-node')?.[0]).toEqual(['1']) + + await locateBtns[1].trigger('click') + expect(wrapper.emitted('locate-node')?.[1]).toEqual(['2']) + }) + + it('emits replace with group when Replace button is clicked', async () => { + const group = makeGroup() + const wrapper = mountRow({ group }) + const replaceBtn = wrapper + .findAll('button') + .find((b) => b.text().includes('Replace Node')) + if (!replaceBtn) throw new Error('Replace button not found') + await replaceBtn.trigger('click') + expect(wrapper.emitted('replace')).toBeTruthy() + expect(wrapper.emitted('replace')?.[0][0]).toEqual(group) + }) + }) + + describe('Edge Cases', () => { + it('handles empty nodeTypes array', () => { + const wrapper = mountRow({ + group: makeGroup({ nodeTypes: [] }) + }) + expect(wrapper.text()).toContain('(0)') + }) + + it('handles string nodeType entries', async () => { + const wrapper = mountRow({ + group: makeGroup({ + // Intentionally uses a plain string entry to test legacy node type handling + nodeTypes: ['StringType'] as unknown as MissingNodeType[] + }) + }) + await wrapper.get('button[aria-label="Expand"]').trigger('click') + expect(wrapper.text()).toContain('StringType') + }) + }) +}) diff --git a/src/components/rightSidePanel/errors/SwapNodeGroupRow.vue b/src/platform/nodeReplacement/components/SwapNodeGroupRow.vue similarity index 90% rename from src/components/rightSidePanel/errors/SwapNodeGroupRow.vue rename to src/platform/nodeReplacement/components/SwapNodeGroupRow.vue index 82dcf3b6be..a2b2020b5b 100644 --- a/src/components/rightSidePanel/errors/SwapNodeGroupRow.vue +++ b/src/platform/nodeReplacement/components/SwapNodeGroupRow.vue @@ -102,9 +102,7 @@ import { useI18n } from 'vue-i18n' import Button from '@/components/ui/button/Button.vue' import TransitionCollapse from '@/components/rightSidePanel/layout/TransitionCollapse.vue' import type { MissingNodeType } from '@/types/comfy' -import type { SwapNodeGroup } from './useErrorGroups' -import { useNodeReplacement } from '@/platform/nodeReplacement/useNodeReplacement' -import { useExecutionErrorStore } from '@/stores/executionErrorStore' +import type { SwapNodeGroup } from '@/components/rightSidePanel/errors/useErrorGroups' const props = defineProps<{ group: SwapNodeGroup @@ -113,11 +111,10 @@ const props = defineProps<{ const emit = defineEmits<{ 'locate-node': [nodeId: string] + replace: [group: SwapNodeGroup] }>() const { t } = useI18n() -const { replaceNodesInPlace } = useNodeReplacement() -const executionErrorStore = useExecutionErrorStore() const expanded = ref(false) @@ -142,9 +139,6 @@ function handleLocateNode(nodeType: MissingNodeType) { } function handleReplaceNode() { - const replaced = replaceNodesInPlace(props.group.nodeTypes) - if (replaced.length > 0) { - executionErrorStore.removeMissingNodesByType([props.group.type]) - } + emit('replace', props.group) } diff --git a/src/platform/nodeReplacement/components/SwapNodesCard.test.ts b/src/platform/nodeReplacement/components/SwapNodesCard.test.ts new file mode 100644 index 0000000000..79453804db --- /dev/null +++ b/src/platform/nodeReplacement/components/SwapNodesCard.test.ts @@ -0,0 +1,129 @@ +import { mount } from '@vue/test-utils' +import { createTestingPinia } from '@pinia/testing' +import PrimeVue from 'primevue/config' +import { describe, expect, it, vi } from 'vitest' +import { createI18n } from 'vue-i18n' + +import type { SwapNodeGroup } from '@/components/rightSidePanel/errors/useErrorGroups' + +vi.mock('./SwapNodeGroupRow.vue', () => ({ + default: { + name: 'SwapNodeGroupRow', + template: '
', + props: ['group', 'showNodeIdBadge'], + emits: ['locate-node', 'replace'] + } +})) + +import SwapNodesCard from './SwapNodesCard.vue' + +const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { en: {} }, + missingWarn: false, + fallbackWarn: false +}) + +function makeGroups(count = 2): SwapNodeGroup[] { + return Array.from({ length: count }, (_, i) => ({ + type: `Type${i}`, + newNodeId: `NewType${i}`, + nodeTypes: [{ type: `Type${i}`, nodeId: String(i), isReplaceable: true }] + })) +} + +function mountCard( + props: Partial<{ + swapNodeGroups: SwapNodeGroup[] + showNodeIdBadge: boolean + }> = {} +) { + return mount(SwapNodesCard, { + props: { + swapNodeGroups: makeGroups(), + showNodeIdBadge: false, + ...props + }, + global: { + plugins: [createTestingPinia({ createSpy: vi.fn }), PrimeVue, i18n] + } + }) +} + +describe('SwapNodesCard', () => { + describe('Rendering', () => { + it('renders guidance message', () => { + const wrapper = mountCard() + expect(wrapper.find('p').exists()).toBe(true) + }) + + it('renders correct number of SwapNodeGroupRow components', () => { + const wrapper = mountCard({ swapNodeGroups: makeGroups(3) }) + expect( + wrapper.findAllComponents({ name: 'SwapNodeGroupRow' }) + ).toHaveLength(3) + }) + + it('renders zero rows when swapNodeGroups is empty', () => { + const wrapper = mountCard({ swapNodeGroups: [] }) + expect( + wrapper.findAllComponents({ name: 'SwapNodeGroupRow' }) + ).toHaveLength(0) + }) + + it('renders one row when swapNodeGroups has one entry', () => { + const wrapper = mountCard({ swapNodeGroups: makeGroups(1) }) + expect( + wrapper.findAllComponents({ name: 'SwapNodeGroupRow' }) + ).toHaveLength(1) + }) + + it('passes showNodeIdBadge to children', () => { + const wrapper = mountCard({ + swapNodeGroups: makeGroups(1), + showNodeIdBadge: true + }) + const row = wrapper.findComponent({ name: 'SwapNodeGroupRow' }) + expect(row.props('showNodeIdBadge')).toBe(true) + }) + + it('passes group prop to children', () => { + const groups = makeGroups(1) + const wrapper = mountCard({ swapNodeGroups: groups }) + const row = wrapper.findComponent({ name: 'SwapNodeGroupRow' }) + expect(row.props('group')).toEqual(groups[0]) + }) + }) + + describe('Events', () => { + it('bubbles locate-node event from child', async () => { + const wrapper = mountCard() + const row = wrapper.findComponent({ name: 'SwapNodeGroupRow' }) + await row.vm.$emit('locate-node', '42') + expect(wrapper.emitted('locate-node')).toBeTruthy() + expect(wrapper.emitted('locate-node')?.[0]).toEqual(['42']) + }) + + it('bubbles replace event from child', async () => { + const groups = makeGroups(1) + const wrapper = mountCard({ swapNodeGroups: groups }) + const row = wrapper.findComponent({ name: 'SwapNodeGroupRow' }) + await row.vm.$emit('replace', groups[0]) + expect(wrapper.emitted('replace')).toBeTruthy() + expect(wrapper.emitted('replace')?.[0][0]).toEqual(groups[0]) + }) + + it('bubbles events from correct child when multiple rows', async () => { + const groups = makeGroups(3) + const wrapper = mountCard({ swapNodeGroups: groups }) + const rows = wrapper.findAllComponents({ name: 'SwapNodeGroupRow' }) + + await rows[2].vm.$emit('locate-node', '99') + expect(wrapper.emitted('locate-node')?.[0]).toEqual(['99']) + + await rows[1].vm.$emit('replace', groups[1]) + expect(wrapper.emitted('replace')?.[0][0]).toEqual(groups[1]) + }) + }) +}) diff --git a/src/components/rightSidePanel/errors/SwapNodesCard.vue b/src/platform/nodeReplacement/components/SwapNodesCard.vue similarity index 78% rename from src/components/rightSidePanel/errors/SwapNodesCard.vue rename to src/platform/nodeReplacement/components/SwapNodesCard.vue index d27e24b7dc..f4b3fc601d 100644 --- a/src/components/rightSidePanel/errors/SwapNodesCard.vue +++ b/src/platform/nodeReplacement/components/SwapNodesCard.vue @@ -16,14 +16,15 @@ :group="group" :show-node-id-badge="showNodeIdBadge" @locate-node="emit('locate-node', $event)" + @replace="emit('replace', $event)" />
diff --git a/src/platform/nodeReplacement/missingNodeScan.test.ts b/src/platform/nodeReplacement/missingNodeScan.test.ts new file mode 100644 index 0000000000..cf13fc75e9 --- /dev/null +++ b/src/platform/nodeReplacement/missingNodeScan.test.ts @@ -0,0 +1,232 @@ +import { createPinia, setActivePinia } from 'pinia' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph' +import { LiteGraph } from '@/lib/litegraph/src/litegraph' + +vi.mock('@/lib/litegraph/src/litegraph', async (importOriginal) => { + const actual = await importOriginal>() + return { + ...actual, + LiteGraph: { + ...(actual.LiteGraph as Record), + registered_node_types: {} as Record + } + } +}) + +vi.mock('@/utils/graphTraversalUtil', () => ({ + collectAllNodes: vi.fn(), + getExecutionIdByNode: vi.fn() +})) + +vi.mock('@/workbench/extensions/manager/utils/missingNodeErrorUtil', () => ({ + getCnrIdFromNode: vi.fn(() => null) +})) + +vi.mock('@/i18n', () => ({ + st: vi.fn((_key: string, fallback: string) => fallback) +})) + +vi.mock('@/platform/distribution/types', () => ({ + isCloud: false +})) + +vi.mock('@/stores/settingStore', () => ({ + useSettingStore: () => ({ + get: vi.fn(() => true) + }) +})) + +vi.mock('@/platform/settings/settingStore', () => ({ + useSettingStore: () => ({ + get: vi.fn(() => true) + }) +})) + +import { + collectAllNodes, + getExecutionIdByNode +} from '@/utils/graphTraversalUtil' +import { getCnrIdFromNode } from '@/workbench/extensions/manager/utils/missingNodeErrorUtil' +import { useNodeReplacementStore } from '@/platform/nodeReplacement/nodeReplacementStore' +import { rescanAndSurfaceMissingNodes } from './missingNodeScan' +import { useExecutionErrorStore } from '@/stores/executionErrorStore' + +function mockNode( + id: number, + type: string, + overrides: Partial = {} +): LGraphNode { + return { + id, + type, + last_serialization: { type }, + ...overrides + } as unknown as LGraphNode +} + +function mockGraph(): LGraph { + return {} as unknown as LGraph +} + +function getMissingNodesError( + store: ReturnType +) { + const error = store.missingNodesError + if (!error) throw new Error('Expected missingNodesError to be defined') + return error +} + +describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => { + beforeEach(() => { + setActivePinia(createPinia()) + vi.clearAllMocks() + // Reset registered_node_types + const reg = LiteGraph.registered_node_types as Record + for (const key of Object.keys(reg)) { + delete reg[key] + } + }) + + it('returns empty when all nodes are registered', () => { + const reg = LiteGraph.registered_node_types as Record + reg['KSampler'] = {} + + vi.mocked(collectAllNodes).mockReturnValue([mockNode(1, 'KSampler')]) + vi.mocked(getExecutionIdByNode).mockReturnValue(null) + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + expect(store.missingNodesError).toBeNull() + }) + + it('detects unregistered nodes as missing', () => { + vi.mocked(collectAllNodes).mockReturnValue([ + mockNode(1, 'OldNode'), + mockNode(2, 'AnotherOldNode') + ]) + vi.mocked(getExecutionIdByNode).mockReturnValue(null) + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + const error = getMissingNodesError(store) + expect(error.nodeTypes).toHaveLength(2) + }) + + it('skips registered nodes and lists only unregistered', () => { + const reg = LiteGraph.registered_node_types as Record + reg['RegisteredNode'] = {} + + vi.mocked(collectAllNodes).mockReturnValue([ + mockNode(1, 'RegisteredNode'), + mockNode(2, 'UnregisteredNode') + ]) + vi.mocked(getExecutionIdByNode).mockReturnValue(null) + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + const error = getMissingNodesError(store) + expect(error.nodeTypes).toHaveLength(1) + const missing = error.nodeTypes[0] + expect(typeof missing !== 'string' && missing.type).toBe('UnregisteredNode') + }) + + it('uses executionId when available for nodeId', () => { + vi.mocked(collectAllNodes).mockReturnValue([mockNode(1, 'Missing')]) + vi.mocked(getExecutionIdByNode).mockReturnValue('exec-42') + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + const error = getMissingNodesError(store) + const missing = error.nodeTypes[0] + expect(typeof missing !== 'string' && missing.nodeId).toBe('exec-42') + }) + + it('falls back to node.id when executionId is null', () => { + vi.mocked(collectAllNodes).mockReturnValue([mockNode(99, 'Missing')]) + vi.mocked(getExecutionIdByNode).mockReturnValue(null) + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + const error = getMissingNodesError(store) + const missing = error.nodeTypes[0] + expect(typeof missing !== 'string' && missing.nodeId).toBe('99') + }) + + it('populates cnrId from getCnrIdFromNode', () => { + vi.mocked(collectAllNodes).mockReturnValue([mockNode(1, 'Missing')]) + vi.mocked(getExecutionIdByNode).mockReturnValue(null) + vi.mocked(getCnrIdFromNode).mockReturnValue('comfy-nodes/my-pack') + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + const error = getMissingNodesError(store) + const missing = error.nodeTypes[0] + expect(typeof missing !== 'string' && missing.cnrId).toBe( + 'comfy-nodes/my-pack' + ) + }) + + it('marks node as replaceable when replacement exists', () => { + vi.mocked(collectAllNodes).mockReturnValue([mockNode(1, 'OldNode')]) + vi.mocked(getExecutionIdByNode).mockReturnValue(null) + + const replacementStore = useNodeReplacementStore() + replacementStore.replacements = { + OldNode: [ + { + old_node_id: 'OldNode', + new_node_id: 'NewNode', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + } + ] + } + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + const error = getMissingNodesError(store) + const missing = error.nodeTypes[0] + expect(typeof missing !== 'string' && missing.isReplaceable).toBe(true) + expect( + typeof missing !== 'string' && missing.replacement?.new_node_id + ).toBe('NewNode') + }) + + it('marks node as not replaceable when no replacement', () => { + vi.mocked(collectAllNodes).mockReturnValue([mockNode(1, 'OldNode')]) + vi.mocked(getExecutionIdByNode).mockReturnValue(null) + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + const error = getMissingNodesError(store) + const missing = error.nodeTypes[0] + expect(typeof missing !== 'string' && missing.isReplaceable).toBe(false) + }) + + it('uses last_serialization.type over node.type', () => { + const node = mockNode(1, 'LiveType') + node.last_serialization = { + type: 'OriginalType' + } as unknown as LGraphNode['last_serialization'] + vi.mocked(collectAllNodes).mockReturnValue([node]) + vi.mocked(getExecutionIdByNode).mockReturnValue(null) + + rescanAndSurfaceMissingNodes(mockGraph()) + + const store = useExecutionErrorStore() + const error = getMissingNodesError(store) + const missing = error.nodeTypes[0] + expect(typeof missing !== 'string' && missing.type).toBe('OriginalType') + }) +}) diff --git a/src/composables/useMissingNodeScan.ts b/src/platform/nodeReplacement/missingNodeScan.ts similarity index 100% rename from src/composables/useMissingNodeScan.ts rename to src/platform/nodeReplacement/missingNodeScan.ts diff --git a/src/platform/nodeReplacement/useNodeReplacement.test.ts b/src/platform/nodeReplacement/useNodeReplacement.test.ts index 72c0d80f93..7f3af48110 100644 --- a/src/platform/nodeReplacement/useNodeReplacement.test.ts +++ b/src/platform/nodeReplacement/useNodeReplacement.test.ts @@ -44,6 +44,15 @@ vi.mock('@/i18n', () => ({ params ? `${key}:${JSON.stringify(params)}` : key })) +const { mockRemoveMissingNodesByType } = vi.hoisted(() => ({ + mockRemoveMissingNodesByType: vi.fn() +})) +vi.mock('@/stores/executionErrorStore', () => ({ + useExecutionErrorStore: vi.fn(() => ({ + removeMissingNodesByType: mockRemoveMissingNodesByType + })) +})) + import { app } from '@/scripts/app' import { collectAllNodes } from '@/utils/graphTraversalUtil' import { useNodeReplacement } from './useNodeReplacement' @@ -651,4 +660,319 @@ describe('useNodeReplacement', () => { expect(result).toEqual(['ImageBatch']) }) }) + + describe('placeholder detection predicate', () => { + /** + * replaceNodesInPlace calls collectAllNodes with a predicate. + * These tests capture the predicate by inspecting the mock call + * and verify it matches only nodes whose serialized type is in + * the targetTypes set — regardless of has_errors or registered_node_types. + */ + + function capturedPredicate(): (n: LGraphNode) => boolean { + const calls = vi.mocked(collectAllNodes).mock.calls + expect(calls.length).toBeGreaterThan(0) + return calls[calls.length - 1][1] as (n: LGraphNode) => boolean + } + + it('should detect placeholder when type is in targetTypes even if has_errors is false', () => { + const placeholder = createPlaceholderNode(1, 'OldNode') + placeholder.has_errors = false + const graph = createMockGraph([placeholder]) + placeholder.graph = graph + Object.assign(app, { rootGraph: graph }) + + vi.mocked(collectAllNodes).mockReturnValue([]) + + const { replaceNodesInPlace } = useNodeReplacement() + replaceNodesInPlace([ + makeMissingNodeType('OldNode', { + new_node_id: 'NewNode', + old_node_id: 'OldNode', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ]) + + const predicate = capturedPredicate() + expect(predicate(placeholder)).toBe(true) + }) + + it('should detect placeholder when type is in targetTypes even if type is registered', () => { + // Simulate the pack being reinstalled — type is now registered + ;(LiteGraph.registered_node_types as Record)['OldNode'] = + {} + + const placeholder = createPlaceholderNode(1, 'OldNode') + const graph = createMockGraph([placeholder]) + placeholder.graph = graph + Object.assign(app, { rootGraph: graph }) + + vi.mocked(collectAllNodes).mockReturnValue([]) + + const { replaceNodesInPlace } = useNodeReplacement() + replaceNodesInPlace([ + makeMissingNodeType('OldNode', { + new_node_id: 'NewNode', + old_node_id: 'OldNode', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ]) + + const predicate = capturedPredicate() + expect(predicate(placeholder)).toBe(true) + + // Cleanup + delete (LiteGraph.registered_node_types as Record)[ + 'OldNode' + ] + }) + + it('should exclude nodes whose type is NOT in targetTypes', () => { + const unrelatedNode = createPlaceholderNode(1, 'UnrelatedNode') + const graph = createMockGraph([unrelatedNode]) + unrelatedNode.graph = graph + Object.assign(app, { rootGraph: graph }) + + vi.mocked(collectAllNodes).mockReturnValue([]) + + const { replaceNodesInPlace } = useNodeReplacement() + replaceNodesInPlace([ + makeMissingNodeType('SomeOtherNode', { + new_node_id: 'NewNode', + old_node_id: 'SomeOtherNode', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ]) + + const predicate = capturedPredicate() + expect(predicate(unrelatedNode)).toBe(false) + }) + + it('should exclude nodes without last_serialization', () => { + const freshNode = createPlaceholderNode(1, 'OldNode') + freshNode.last_serialization = + undefined as unknown as LGraphNode['last_serialization'] + const graph = createMockGraph([freshNode]) + Object.assign(app, { rootGraph: graph }) + + vi.mocked(collectAllNodes).mockReturnValue([]) + + const { replaceNodesInPlace } = useNodeReplacement() + replaceNodesInPlace([ + makeMissingNodeType('OldNode', { + new_node_id: 'NewNode', + old_node_id: 'OldNode', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ]) + + const predicate = capturedPredicate() + expect(predicate(freshNode)).toBe(false) + }) + + it('should fall back to node.type when last_serialization.type is undefined', () => { + const node = createPlaceholderNode(1, 'FallbackType') + node.last_serialization!.type = undefined as unknown as string + node.type = 'FallbackType' + const graph = createMockGraph([node]) + Object.assign(app, { rootGraph: graph }) + + vi.mocked(collectAllNodes).mockReturnValue([]) + + const { replaceNodesInPlace } = useNodeReplacement() + replaceNodesInPlace([ + makeMissingNodeType('FallbackType', { + new_node_id: 'NewNode', + old_node_id: 'FallbackType', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ]) + + const predicate = capturedPredicate() + expect(predicate(node)).toBe(true) + }) + + it('should match node via sanitized type when last_serialization.type is absent and live type contains HTML special chars', () => { + // Simulates an old serialization format (no last_serialization.type) + // where app.ts has already run sanitizeNodeName on n.type, + // stripping '&' from "OldNode&Special" → "OldNodeSpecial". + // targetTypes still holds the original unsanitized name "OldNode&Special", + // so the predicate must fall back to checking sanitizeNodeName(originalType). + const node = createPlaceholderNode(1, 'OldNodeSpecial') + node.last_serialization!.type = undefined as unknown as string + // Simulate what sanitizeNodeName does to '&' in the live type + node.type = 'OldNodeSpecial' // '&' already stripped by sanitizeNodeName + const graph = createMockGraph([node]) + Object.assign(app, { rootGraph: graph }) + + vi.mocked(collectAllNodes).mockReturnValue([]) + + const { replaceNodesInPlace } = useNodeReplacement() + replaceNodesInPlace([ + // targetTypes will contain the original name with '&' + makeMissingNodeType('OldNode&Special', { + new_node_id: 'NewNode', + old_node_id: 'OldNode&Special', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ]) + + const predicate = capturedPredicate() + // Without the sanitize fallback this would return false. + expect(predicate(node)).toBe(true) + }) + }) + + describe('replaceGroup', () => { + it('calls removeMissingNodesByType with replaced types on success', () => { + const placeholder = createPlaceholderNode(1, 'OldNode') + const graph = createMockGraph([placeholder]) + placeholder.graph = graph + Object.assign(app, { rootGraph: graph }) + + const newNode = createNewNode() + vi.mocked(collectAllNodes).mockReturnValue([placeholder]) + vi.mocked(LiteGraph.createNode).mockReturnValue(newNode) + + const { replaceGroup } = useNodeReplacement() + replaceGroup({ + type: 'OldNode', + nodeTypes: [ + makeMissingNodeType('OldNode', { + new_node_id: 'NewNode', + old_node_id: 'OldNode', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ] + }) + + expect(mockRemoveMissingNodesByType).toHaveBeenCalledWith(['OldNode']) + }) + + it('does not call removeMissingNodesByType when no nodes are replaced', () => { + const graph = createMockGraph([]) + Object.assign(app, { rootGraph: graph }) + vi.mocked(collectAllNodes).mockReturnValue([]) + + const { replaceGroup } = useNodeReplacement() + replaceGroup({ + type: 'OldNode', + nodeTypes: [ + makeMissingNodeType('OldNode', { + new_node_id: 'NewNode', + old_node_id: 'OldNode', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ] + }) + + expect(mockRemoveMissingNodesByType).not.toHaveBeenCalled() + }) + }) + + describe('replaceAllGroups', () => { + it('calls removeMissingNodesByType with all successfully replaced types', () => { + const p1 = createPlaceholderNode(1, 'TypeA') + const p2 = createPlaceholderNode(2, 'TypeB') + const graph = createMockGraph([p1, p2]) + p1.graph = graph + p2.graph = graph + Object.assign(app, { rootGraph: graph }) + + vi.mocked(collectAllNodes).mockReturnValue([p1, p2]) + vi.mocked(LiteGraph.createNode) + .mockReturnValueOnce(createNewNode()) + .mockReturnValueOnce(createNewNode()) + + const { replaceAllGroups } = useNodeReplacement() + replaceAllGroups([ + { + type: 'TypeA', + nodeTypes: [ + makeMissingNodeType('TypeA', { + new_node_id: 'NewA', + old_node_id: 'TypeA', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ] + }, + { + type: 'TypeB', + nodeTypes: [ + makeMissingNodeType('TypeB', { + new_node_id: 'NewB', + old_node_id: 'TypeB', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ] + } + ]) + + expect(mockRemoveMissingNodesByType).toHaveBeenCalledWith( + expect.arrayContaining(['TypeA', 'TypeB']) + ) + }) + + it('removes only the types that were actually replaced when some fail', () => { + const p1 = createPlaceholderNode(1, 'TypeA') + const graph = createMockGraph([p1]) + p1.graph = graph + Object.assign(app, { rootGraph: graph }) + + // Only TypeA appears as a placeholder; TypeB has no matching node + vi.mocked(collectAllNodes).mockReturnValue([p1]) + vi.mocked(LiteGraph.createNode).mockReturnValueOnce(createNewNode()) + + const { replaceAllGroups } = useNodeReplacement() + replaceAllGroups([ + { + type: 'TypeA', + nodeTypes: [ + makeMissingNodeType('TypeA', { + new_node_id: 'NewA', + old_node_id: 'TypeA', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ] + }, + { + type: 'TypeB', + nodeTypes: [ + makeMissingNodeType('TypeB', { + new_node_id: 'NewB', + old_node_id: 'TypeB', + old_widget_ids: null, + input_mapping: null, + output_mapping: null + }) + ] + } + ]) + + // Only TypeA was replaced; TypeB had no matching placeholder + expect(mockRemoveMissingNodesByType).toHaveBeenCalledWith(['TypeA']) + }) + }) }) diff --git a/src/platform/nodeReplacement/useNodeReplacement.ts b/src/platform/nodeReplacement/useNodeReplacement.ts index 3706ebc102..bd0cba856f 100644 --- a/src/platform/nodeReplacement/useNodeReplacement.ts +++ b/src/platform/nodeReplacement/useNodeReplacement.ts @@ -7,9 +7,15 @@ import type { NodeReplacement } from '@/platform/nodeReplacement/types' import { useToastStore } from '@/platform/updates/common/toastStore' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' import { app, sanitizeNodeName } from '@/scripts/app' +import { useExecutionErrorStore } from '@/stores/executionErrorStore' import type { MissingNodeType } from '@/types/comfy' import { collectAllNodes } from '@/utils/graphTraversalUtil' +interface ReplacementGroup { + type: string + nodeTypes: MissingNodeType[] +} + /** Compares sanitized type strings to match placeholder → missing node type. */ function findMatchingType( node: LGraphNode, @@ -230,15 +236,23 @@ export function useNodeReplacement() { // and the missing nodes detected at that point — not from the current // registered_node_types. This ensures replacement still works even if // the user has since installed the missing node pack. - const targetTypes = new Set( - selectedTypes.map((t) => (typeof t === 'string' ? t : t.type)) - ) + // Also include sanitized variants so that when the fallback path reads + // n.type (which app.ts may have already run through sanitizeNodeName), + // we can still match against the original type stored in selectedTypes. + const targetTypes = new Set([ + ...selectedTypes.map((t) => (typeof t === 'string' ? t : t.type)), + ...selectedTypes.map((t) => + sanitizeNodeName(typeof t === 'string' ? t : t.type) + ) + ]) try { const placeholders = collectAllNodes(graph, (n) => { if (!n.last_serialization) return false // Prefer the original serialized type; fall back to the live type // for nodes whose serialization predates the type field. + // n.type may have been sanitized by app.ts (HTML special chars stripped); + // the sanitized variants in targetTypes ensure we still match correctly. const originalType = n.last_serialization.type ?? n.type return !!originalType && targetTypes.has(originalType) }) @@ -314,7 +328,32 @@ export function useNodeReplacement() { return replacedTypes } + /** + * Replaces all nodes in a single swap group and removes successfully + * replaced types from the execution error store. + */ + function replaceGroup(group: ReplacementGroup): void { + const replaced = replaceNodesInPlace(group.nodeTypes) + if (replaced.length > 0) { + useExecutionErrorStore().removeMissingNodesByType(replaced) + } + } + + /** + * Replaces every available node across all swap groups and removes + * the succeeded types from the execution error store. + */ + function replaceAllGroups(groups: ReplacementGroup[]): void { + const allNodeTypes = groups.flatMap((g) => g.nodeTypes) + const replaced = replaceNodesInPlace(allNodeTypes) + if (replaced.length > 0) { + useExecutionErrorStore().removeMissingNodesByType(replaced) + } + } + return { - replaceNodesInPlace + replaceNodesInPlace, + replaceGroup, + replaceAllGroups } } diff --git a/src/scripts/app.ts b/src/scripts/app.ts index ca12dadac7..40efcacfec 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -80,7 +80,7 @@ import type { ExtensionManager } from '@/types/extensionTypes' import type { NodeExecutionId } from '@/types/nodeIdentification' import { graphToPrompt } from '@/utils/executionUtil' import { getCnrIdFromProperties } from '@/workbench/extensions/manager/utils/missingNodeErrorUtil' -import { rescanAndSurfaceMissingNodes } from '@/composables/useMissingNodeScan' +import { rescanAndSurfaceMissingNodes } from '@/platform/nodeReplacement/missingNodeScan' import { anyItemOverlapsRect } from '@/utils/mathUtil' import { collectAllNodes, diff --git a/src/stores/executionErrorStore.test.ts b/src/stores/executionErrorStore.test.ts new file mode 100644 index 0000000000..f6c166e74a --- /dev/null +++ b/src/stores/executionErrorStore.test.ts @@ -0,0 +1,159 @@ +import { createPinia, setActivePinia } from 'pinia' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { MissingNodeType } from '@/types/comfy' + +// Mock dependencies +vi.mock('@/i18n', () => ({ + st: vi.fn((_key: string, fallback: string) => fallback) +})) + +vi.mock('@/platform/distribution/types', () => ({ + isCloud: false +})) + +vi.mock('@/stores/settingStore', () => ({ + useSettingStore: vi.fn(() => ({ + get: vi.fn(() => false) + })) +})) + +import { useExecutionErrorStore } from './executionErrorStore' + +describe('executionErrorStore — missing node operations', () => { + beforeEach(() => { + setActivePinia(createPinia()) + }) + + describe('setMissingNodeTypes', () => { + it('sets missingNodesError with provided types', () => { + const store = useExecutionErrorStore() + const types: MissingNodeType[] = [ + { type: 'NodeA', nodeId: '1', isReplaceable: false } + ] + store.setMissingNodeTypes(types) + + expect(store.missingNodesError).not.toBeNull() + expect(store.missingNodesError?.nodeTypes).toHaveLength(1) + expect(store.hasMissingNodes).toBe(true) + }) + + it('clears missingNodesError when given empty array', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + { type: 'NodeA', nodeId: '1', isReplaceable: false } + ]) + expect(store.missingNodesError).not.toBeNull() + + store.setMissingNodeTypes([]) + expect(store.missingNodesError).toBeNull() + expect(store.hasMissingNodes).toBe(false) + }) + + it('deduplicates string entries by value', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + 'NodeA', + 'NodeA', + 'NodeB' + ] as MissingNodeType[]) + + expect(store.missingNodesError?.nodeTypes).toHaveLength(2) + }) + + it('deduplicates object entries by nodeId when present', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + { type: 'NodeA', nodeId: '1', isReplaceable: false }, + { type: 'NodeA', nodeId: '1', isReplaceable: false }, + { type: 'NodeA', nodeId: '2', isReplaceable: false } + ]) + + // Same nodeId='1' deduplicated, nodeId='2' kept + expect(store.missingNodesError?.nodeTypes).toHaveLength(2) + }) + + it('deduplicates object entries by type when nodeId is absent', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + { type: 'NodeA', isReplaceable: false }, + { type: 'NodeA', isReplaceable: true } + ] as MissingNodeType[]) + + // Same type, no nodeId → deduplicated + expect(store.missingNodesError?.nodeTypes).toHaveLength(1) + }) + + it('keeps distinct nodeIds even when type is the same', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + { type: 'NodeA', nodeId: '1', isReplaceable: false }, + { type: 'NodeA', nodeId: '2', isReplaceable: false }, + { type: 'NodeA', nodeId: '3', isReplaceable: false } + ]) + + expect(store.missingNodesError?.nodeTypes).toHaveLength(3) + }) + }) + + describe('removeMissingNodesByType', () => { + it('removes matching types from the missing nodes list', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + { type: 'NodeA', nodeId: '1', isReplaceable: false }, + { type: 'NodeB', nodeId: '2', isReplaceable: false }, + { type: 'NodeC', nodeId: '3', isReplaceable: false } + ]) + + store.removeMissingNodesByType(['NodeA', 'NodeC']) + + expect(store.missingNodesError?.nodeTypes).toHaveLength(1) + const remaining = store.missingNodesError?.nodeTypes[0] + expect(typeof remaining !== 'string' && remaining?.type).toBe('NodeB') + }) + + it('clears missingNodesError when all types are removed', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + { type: 'NodeA', nodeId: '1', isReplaceable: false } + ]) + + store.removeMissingNodesByType(['NodeA']) + + expect(store.missingNodesError).toBeNull() + expect(store.hasMissingNodes).toBe(false) + }) + + it('does nothing when missingNodesError is null', () => { + const store = useExecutionErrorStore() + expect(store.missingNodesError).toBeNull() + + // Should not throw + store.removeMissingNodesByType(['NodeA']) + expect(store.missingNodesError).toBeNull() + }) + + it('does nothing when removing non-existent types', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + { type: 'NodeA', nodeId: '1', isReplaceable: false } + ]) + + store.removeMissingNodesByType(['NonExistent']) + + expect(store.missingNodesError?.nodeTypes).toHaveLength(1) + }) + + it('handles removing from string entries', () => { + const store = useExecutionErrorStore() + store.setMissingNodeTypes([ + 'StringNodeA', + 'StringNodeB' + ] as MissingNodeType[]) + + store.removeMissingNodesByType(['StringNodeA']) + + expect(store.missingNodesError?.nodeTypes).toHaveLength(1) + }) + }) +})