feat: improve multi-package selection handling (#5116)

* feat: improve multi-package selection handling

- Check each package individually for conflicts in install dialog
- Show only packages with actual conflicts in warning dialog
- Hide action buttons for mixed installed/uninstalled selections
- Display dynamic status based on selected packages priority
- Deduplicate conflict information across multiple packages
- Fix PackIcon blur background opacity

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* refactor: extract multi-package logic into reusable composables

- Create usePackageSelection composable for installation state management
- Create usePackageStatus composable for status priority logic
- Refactor InfoPanelMultiItem to use new composables
- Reduce component complexity by separating business logic
- Improve code reusability across components

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: directory modified

* test: add comprehensive tests for multi-package selection composables

- Add tests for usePacksSelection composable
  - Test installation status filtering
  - Test selection state determination (all/none/mixed)
  - Test dynamic status changes

- Add tests for usePacksStatus composable
  - Test import failure detection
  - Test status priority handling
  - Test integration with conflict detection store

- Fix existing test mocking issues
  - Update es-toolkit/compat mock to use async import
  - Add Pinia setup for store-dependent tests
  - Update vue-i18n mock to preserve all exports

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Jin Yi
2025-08-28 00:00:09 +09:00
committed by GitHub
parent 267e07e26d
commit 91e462dae8
10 changed files with 978 additions and 34 deletions

View File

@@ -0,0 +1,378 @@
import { createPinia, setActivePinia } from 'pinia'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { ref } from 'vue'
import { usePacksSelection } from '@/composables/nodePack/usePacksSelection'
import { useComfyManagerStore } from '@/stores/comfyManagerStore'
import type { components } from '@/types/comfyRegistryTypes'
vi.mock('vue-i18n', async () => {
const actual = await vi.importActual('vue-i18n')
return {
...actual,
useI18n: () => ({
t: vi.fn((key) => key)
})
}
})
type NodePack = components['schemas']['Node']
describe('usePacksSelection', () => {
let managerStore: ReturnType<typeof useComfyManagerStore>
let mockIsPackInstalled: ReturnType<typeof vi.fn>
const createMockPack = (id: string): NodePack => ({
id,
name: `Pack ${id}`,
description: `Description for pack ${id}`,
category: 'Nodes',
author: 'Test Author',
license: 'MIT',
repository: 'https://github.com/test/pack',
tags: [],
status: 'NodeStatusActive'
})
beforeEach(() => {
vi.clearAllMocks()
const pinia = createPinia()
setActivePinia(pinia)
managerStore = useComfyManagerStore()
// Mock the isPackInstalled method
mockIsPackInstalled = vi.fn()
managerStore.isPackInstalled = mockIsPackInstalled
})
afterEach(() => {
vi.restoreAllMocks()
})
describe('installedPacks', () => {
it('should filter and return only installed packs', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2'),
createMockPack('pack3')
])
mockIsPackInstalled.mockImplementation((id: string) => {
return id === 'pack1' || id === 'pack3'
})
const { installedPacks } = usePacksSelection(nodePacks)
expect(installedPacks.value).toHaveLength(2)
expect(installedPacks.value[0].id).toBe('pack1')
expect(installedPacks.value[1].id).toBe('pack3')
expect(mockIsPackInstalled).toHaveBeenCalledTimes(3)
})
it('should return empty array when no packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(false)
const { installedPacks } = usePacksSelection(nodePacks)
expect(installedPacks.value).toHaveLength(0)
})
it('should update when nodePacks ref changes', () => {
const nodePacks = ref<NodePack[]>([createMockPack('pack1')])
mockIsPackInstalled.mockReturnValue(true)
const { installedPacks } = usePacksSelection(nodePacks)
expect(installedPacks.value).toHaveLength(1)
// Add more packs
nodePacks.value = [
createMockPack('pack1'),
createMockPack('pack2'),
createMockPack('pack3')
]
expect(installedPacks.value).toHaveLength(3)
})
})
describe('notInstalledPacks', () => {
it('should filter and return only not installed packs', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2'),
createMockPack('pack3')
])
mockIsPackInstalled.mockImplementation((id: string) => {
return id === 'pack1'
})
const { notInstalledPacks } = usePacksSelection(nodePacks)
expect(notInstalledPacks.value).toHaveLength(2)
expect(notInstalledPacks.value[0].id).toBe('pack2')
expect(notInstalledPacks.value[1].id).toBe('pack3')
})
it('should return all packs when none are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(false)
const { notInstalledPacks } = usePacksSelection(nodePacks)
expect(notInstalledPacks.value).toHaveLength(2)
})
})
describe('isAllInstalled', () => {
it('should return true when all packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(true)
const { isAllInstalled } = usePacksSelection(nodePacks)
expect(isAllInstalled.value).toBe(true)
})
it('should return false when not all packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockImplementation((id: string) => id === 'pack1')
const { isAllInstalled } = usePacksSelection(nodePacks)
expect(isAllInstalled.value).toBe(false)
})
it('should return true for empty array', () => {
const nodePacks = ref<NodePack[]>([])
const { isAllInstalled } = usePacksSelection(nodePacks)
expect(isAllInstalled.value).toBe(true)
})
})
describe('isNoneInstalled', () => {
it('should return true when no packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(false)
const { isNoneInstalled } = usePacksSelection(nodePacks)
expect(isNoneInstalled.value).toBe(true)
})
it('should return false when some packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockImplementation((id: string) => id === 'pack1')
const { isNoneInstalled } = usePacksSelection(nodePacks)
expect(isNoneInstalled.value).toBe(false)
})
it('should return true for empty array', () => {
const nodePacks = ref<NodePack[]>([])
const { isNoneInstalled } = usePacksSelection(nodePacks)
expect(isNoneInstalled.value).toBe(true)
})
})
describe('isMixed', () => {
it('should return true when some but not all packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2'),
createMockPack('pack3')
])
mockIsPackInstalled.mockImplementation((id: string) => {
return id === 'pack1' || id === 'pack2'
})
const { isMixed } = usePacksSelection(nodePacks)
expect(isMixed.value).toBe(true)
})
it('should return false when all packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(true)
const { isMixed } = usePacksSelection(nodePacks)
expect(isMixed.value).toBe(false)
})
it('should return false when no packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(false)
const { isMixed } = usePacksSelection(nodePacks)
expect(isMixed.value).toBe(false)
})
it('should return false for empty array', () => {
const nodePacks = ref<NodePack[]>([])
const { isMixed } = usePacksSelection(nodePacks)
expect(isMixed.value).toBe(false)
})
})
describe('selectionState', () => {
it('should return "all-installed" when all packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(true)
const { selectionState } = usePacksSelection(nodePacks)
expect(selectionState.value).toBe('all-installed')
})
it('should return "none-installed" when no packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(false)
const { selectionState } = usePacksSelection(nodePacks)
expect(selectionState.value).toBe('none-installed')
})
it('should return "mixed" when some packs are installed', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2'),
createMockPack('pack3')
])
mockIsPackInstalled.mockImplementation((id: string) => id === 'pack1')
const { selectionState } = usePacksSelection(nodePacks)
expect(selectionState.value).toBe('mixed')
})
it('should update when installation status changes', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
mockIsPackInstalled.mockReturnValue(false)
const { selectionState } = usePacksSelection(nodePacks)
expect(selectionState.value).toBe('none-installed')
// Change mock to simulate installation
mockIsPackInstalled.mockReturnValue(true)
// Force reactivity update
nodePacks.value = [...nodePacks.value]
expect(selectionState.value).toBe('all-installed')
})
})
describe('edge cases', () => {
it('should handle packs with undefined ids', () => {
const nodePacks = ref<NodePack[]>([
{ ...createMockPack('pack1'), id: undefined as any },
createMockPack('pack2')
])
mockIsPackInstalled.mockImplementation((id: string) => id === 'pack2')
const { installedPacks, notInstalledPacks } = usePacksSelection(nodePacks)
expect(installedPacks.value).toHaveLength(1)
expect(installedPacks.value[0].id).toBe('pack2')
expect(notInstalledPacks.value).toHaveLength(1)
})
it('should handle dynamic changes to pack installation status', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
const installationStatus: Record<string, boolean> = {
pack1: false,
pack2: false
}
mockIsPackInstalled.mockImplementation(
(id: string) => installationStatus[id] || false
)
const { installedPacks, notInstalledPacks, selectionState } =
usePacksSelection(nodePacks)
expect(selectionState.value).toBe('none-installed')
expect(installedPacks.value).toHaveLength(0)
expect(notInstalledPacks.value).toHaveLength(2)
// Simulate installing pack1
installationStatus.pack1 = true
nodePacks.value = [...nodePacks.value] // Trigger reactivity
expect(selectionState.value).toBe('mixed')
expect(installedPacks.value).toHaveLength(1)
expect(notInstalledPacks.value).toHaveLength(1)
// Simulate installing pack2
installationStatus.pack2 = true
nodePacks.value = [...nodePacks.value] // Trigger reactivity
expect(selectionState.value).toBe('all-installed')
expect(installedPacks.value).toHaveLength(2)
expect(notInstalledPacks.value).toHaveLength(0)
})
})
})

View File

@@ -0,0 +1,384 @@
import { createPinia, setActivePinia } from 'pinia'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { ref } from 'vue'
import { usePacksStatus } from '@/composables/nodePack/usePacksStatus'
import { useConflictDetectionStore } from '@/stores/conflictDetectionStore'
import type { components } from '@/types/comfyRegistryTypes'
import type { ConflictDetectionResult } from '@/types/conflictDetectionTypes'
type NodePack = components['schemas']['Node']
type NodeStatus = components['schemas']['NodeStatus']
type NodeVersionStatus = components['schemas']['NodeVersionStatus']
describe('usePacksStatus', () => {
let conflictDetectionStore: ReturnType<typeof useConflictDetectionStore>
const createMockPack = (
id: string,
status?: NodeStatus | NodeVersionStatus
): NodePack => ({
id,
name: `Pack ${id}`,
description: `Description for pack ${id}`,
category: 'Nodes',
author: 'Test Author',
license: 'MIT',
repository: 'https://github.com/test/pack',
tags: [],
status: (status || 'NodeStatusActive') as NodeStatus
})
const createMockConflict = (
packageId: string,
type: 'import_failed' | 'banned' | 'pending' = 'import_failed'
): ConflictDetectionResult => ({
package_id: packageId,
package_name: `Pack ${packageId}`,
has_conflict: true,
conflicts: [
{
type,
current_value: 'current',
required_value: 'required'
}
],
is_compatible: false
})
beforeEach(() => {
vi.clearAllMocks()
setActivePinia(createPinia())
conflictDetectionStore = useConflictDetectionStore()
})
afterEach(() => {
vi.restoreAllMocks()
})
describe('hasImportFailed', () => {
it('should return true when at least one pack has import_failed conflict', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2'),
createMockPack('pack3')
])
// Set up mock conflicts
conflictDetectionStore.setConflictedPackages([
createMockConflict('pack2', 'import_failed'),
createMockConflict('pack3', 'banned')
])
const { hasImportFailed } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(true)
})
it('should return false when no pack has import_failed conflict', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
// Set up mock conflicts with no import_failed
conflictDetectionStore.setConflictedPackages([
createMockConflict('pack1', 'pending'),
createMockConflict('pack2', 'banned')
])
const { hasImportFailed } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(false)
})
it('should return false when no conflicts exist', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
conflictDetectionStore.setConflictedPackages([])
const { hasImportFailed } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(false)
})
it('should handle packs without ids', () => {
const nodePacks = ref<NodePack[]>([
{ ...createMockPack('pack1'), id: undefined as any },
createMockPack('pack2')
])
conflictDetectionStore.setConflictedPackages([
createMockConflict('pack2', 'import_failed')
])
const { hasImportFailed } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(true)
})
it('should update when conflicts change', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
conflictDetectionStore.setConflictedPackages([])
const { hasImportFailed } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(false)
// Add import_failed conflict
conflictDetectionStore.setConflictedPackages([
createMockConflict('pack1', 'import_failed')
])
expect(hasImportFailed.value).toBe(true)
})
})
describe('overallStatus', () => {
it('should prioritize banned status over all others', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeStatusActive'),
createMockPack('pack2', 'NodeStatusBanned'),
createMockPack('pack3', 'NodeVersionStatusDeleted')
])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeStatusBanned')
})
it('should prioritize version banned over deleted and active', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeStatusActive'),
createMockPack('pack2', 'NodeVersionStatusBanned'),
createMockPack('pack3', 'NodeVersionStatusDeleted')
])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeVersionStatusBanned')
})
it('should prioritize deleted status appropriately', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeStatusActive'),
createMockPack('pack2', 'NodeStatusDeleted'),
createMockPack('pack3', 'NodeVersionStatusActive')
])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeStatusDeleted')
})
it('should prioritize version deleted over flagged and active', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeVersionStatusFlagged'),
createMockPack('pack2', 'NodeVersionStatusDeleted'),
createMockPack('pack3', 'NodeVersionStatusActive')
])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeVersionStatusDeleted')
})
it('should prioritize flagged status over pending and active', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeVersionStatusPending'),
createMockPack('pack2', 'NodeVersionStatusFlagged'),
createMockPack('pack3', 'NodeVersionStatusActive')
])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeVersionStatusFlagged')
})
it('should prioritize pending status over active', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeVersionStatusActive'),
createMockPack('pack2', 'NodeVersionStatusPending'),
createMockPack('pack3', 'NodeStatusActive')
])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeVersionStatusPending')
})
it('should return NodeStatusActive when all packs are active', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeStatusActive'),
createMockPack('pack2', 'NodeStatusActive')
])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeStatusActive')
})
it('should return NodeStatusActive as default when all packs have no status', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
const { overallStatus } = usePacksStatus(nodePacks)
// Since createMockPack sets status to 'NodeStatusActive' by default
expect(overallStatus.value).toBe('NodeStatusActive')
})
it('should handle empty pack array', () => {
const nodePacks = ref<NodePack[]>([])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeVersionStatusActive')
})
it('should update when pack statuses change', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeStatusActive'),
createMockPack('pack2', 'NodeStatusActive')
])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeStatusActive')
// Change one pack to banned
nodePacks.value = [
createMockPack('pack1', 'NodeStatusBanned'),
createMockPack('pack2', 'NodeStatusActive')
]
expect(overallStatus.value).toBe('NodeStatusBanned')
})
})
describe('integration with import failures', () => {
it('should return NodeVersionStatusActive when import failures exist (handled separately)', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeStatusActive'),
createMockPack('pack2', 'NodeStatusActive')
])
conflictDetectionStore.setConflictedPackages([
createMockConflict('pack1', 'import_failed')
])
const { hasImportFailed, overallStatus } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(true)
// When import failed exists, it returns NodeVersionStatusActive
expect(overallStatus.value).toBe('NodeVersionStatusActive')
})
it('should return NodeVersionStatusActive when import failures exist even with banned status', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeStatusBanned'),
createMockPack('pack2', 'NodeStatusActive')
])
conflictDetectionStore.setConflictedPackages([
createMockConflict('pack2', 'import_failed')
])
const { hasImportFailed, overallStatus } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(true)
// Import failed takes priority and returns NodeVersionStatusActive
expect(overallStatus.value).toBe('NodeVersionStatusActive')
})
})
describe('edge cases', () => {
it('should handle multiple conflicts per package', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
conflictDetectionStore.setConflictedPackages([
{
package_id: 'pack1',
package_name: 'Pack pack1',
has_conflict: true,
conflicts: [
{
type: 'pending',
current_value: 'current1',
required_value: 'required1'
},
{
type: 'import_failed',
current_value: 'current2',
required_value: 'required2'
}
],
is_compatible: false
}
])
const { hasImportFailed } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(true)
})
it('should handle packs with no conflicts in store', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1'),
createMockPack('pack2')
])
const { hasImportFailed } = usePacksStatus(nodePacks)
expect(hasImportFailed.value).toBe(false)
})
it('should handle mixed status types correctly', () => {
const nodePacks = ref<NodePack[]>([
createMockPack('pack1', 'NodeStatusBanned'),
createMockPack('pack2', 'NodeVersionStatusBanned'),
createMockPack('pack3', 'NodeStatusDeleted'),
createMockPack('pack4', 'NodeVersionStatusDeleted'),
createMockPack('pack5', 'NodeVersionStatusFlagged'),
createMockPack('pack6', 'NodeVersionStatusPending'),
createMockPack('pack7', 'NodeStatusActive'),
createMockPack('pack8', 'NodeVersionStatusActive')
])
const { overallStatus } = usePacksStatus(nodePacks)
// Should return the highest priority status (NodeStatusBanned)
expect(overallStatus.value).toBe('NodeStatusBanned')
})
it('should be reactive to nodePacks changes', () => {
const nodePacks = ref<NodePack[]>([])
const { overallStatus } = usePacksStatus(nodePacks)
expect(overallStatus.value).toBe('NodeVersionStatusActive')
// Add packs
nodePacks.value = [
createMockPack('pack1', 'NodeStatusDeleted'),
createMockPack('pack2', 'NodeStatusActive')
]
expect(overallStatus.value).toBe('NodeStatusDeleted')
// Add a higher priority status
nodePacks.value.push(createMockPack('pack3', 'NodeStatusBanned'))
expect(overallStatus.value).toBe('NodeStatusBanned')
})
})
})