[refactor] Simplify conflict acknowledgment system and enhance UX (#4599)

This commit is contained in:
Jin Yi
2025-07-31 08:20:58 +09:00
parent 38e2fa8399
commit b67ace5195
15 changed files with 304 additions and 861 deletions

View File

@@ -1,9 +1,12 @@
import { createPinia, setActivePinia } from 'pinia'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { useConflictAcknowledgment } from '@/composables/useConflictAcknowledgment'
describe('useConflictAcknowledgment with useStorage refactor', () => {
beforeEach(() => {
// Set up Pinia for each test
setActivePinia(createPinia())
// Clear localStorage before each test
localStorage.clear()
// Reset modules to ensure fresh state
@@ -18,20 +21,20 @@ describe('useConflictAcknowledgment with useStorage refactor', () => {
const {
shouldShowConflictModal,
shouldShowRedDot,
acknowledgedPackageIds
shouldShowManagerBanner
} = useConflictAcknowledgment()
expect(shouldShowConflictModal.value).toBe(true)
expect(shouldShowRedDot.value).toBe(true)
expect(acknowledgedPackageIds.value).toEqual([])
expect(shouldShowRedDot.value).toBe(false) // No conflicts initially
expect(shouldShowManagerBanner.value).toBe(false) // No conflicts initially
})
it('should dismiss modal state correctly', () => {
const { dismissConflictModal, shouldShowConflictModal } =
const { markConflictsAsSeen, shouldShowConflictModal } =
useConflictAcknowledgment()
expect(shouldShowConflictModal.value).toBe(true)
dismissConflictModal()
markConflictsAsSeen()
expect(shouldShowConflictModal.value).toBe(false)
})
@@ -39,92 +42,78 @@ describe('useConflictAcknowledgment with useStorage refactor', () => {
const { dismissRedDotNotification, shouldShowRedDot } =
useConflictAcknowledgment()
expect(shouldShowRedDot.value).toBe(true)
expect(shouldShowRedDot.value).toBe(false) // No conflicts initially
dismissRedDotNotification()
expect(shouldShowRedDot.value).toBe(false)
})
it('should acknowledge conflicts correctly', () => {
const {
acknowledgeConflict,
isConflictAcknowledged,
acknowledgedPackageIds
} = useConflictAcknowledgment()
expect(acknowledgedPackageIds.value).toEqual([])
acknowledgeConflict('package1', 'version_conflict', '1.0.0')
expect(isConflictAcknowledged('package1', 'version_conflict')).toBe(true)
expect(isConflictAcknowledged('package1', 'other_conflict')).toBe(false)
expect(acknowledgedPackageIds.value).toContain('package1')
})
it('should reset state when ComfyUI version changes', () => {
const {
dismissConflictModal,
acknowledgeConflict,
checkComfyUIVersionChange,
shouldShowConflictModal,
acknowledgedPackageIds
} = useConflictAcknowledgment()
// Set up some state
dismissConflictModal()
acknowledgeConflict('package1', 'conflict1', '1.0.0')
expect(shouldShowConflictModal.value).toBe(false)
expect(acknowledgedPackageIds.value).toContain('package1')
// First check sets the initial version, no change yet
const changed1 = checkComfyUIVersionChange('1.0.0')
expect(changed1).toBe(false)
// Now check with different version should reset
const changed2 = checkComfyUIVersionChange('2.0.0')
expect(changed2).toBe(true)
expect(shouldShowConflictModal.value).toBe(true)
expect(acknowledgedPackageIds.value).toEqual([])
})
it('should track acknowledgment statistics correctly', () => {
const { acknowledgmentStats, dismissConflictModal, acknowledgeConflict } =
it('should dismiss warning banner correctly', () => {
const { dismissWarningBanner, shouldShowManagerBanner } =
useConflictAcknowledgment()
// Initial stats
expect(acknowledgmentStats.value).toEqual({
total_acknowledged: 0,
unique_packages: 0,
modal_dismissed: false,
red_dot_dismissed: false,
last_comfyui_version: ''
})
// Initially should not show banner (no conflicts)
expect(shouldShowManagerBanner.value).toBe(false)
// Update state
dismissConflictModal()
acknowledgeConflict('package1', 'conflict1', '1.0.0')
acknowledgeConflict('package2', 'conflict2', '1.0.0')
// Test dismissWarningBanner function exists and works
dismissWarningBanner()
expect(shouldShowManagerBanner.value).toBe(false)
})
// Check updated stats
expect(acknowledgmentStats.value.total_acknowledged).toBe(2)
expect(acknowledgmentStats.value.unique_packages).toBe(2)
expect(acknowledgmentStats.value.modal_dismissed).toBe(true)
it('should mark conflicts as seen', () => {
const {
markConflictsAsSeen,
shouldShowConflictModal,
shouldShowRedDot,
shouldShowManagerBanner
} = useConflictAcknowledgment()
// Mark conflicts as seen
markConflictsAsSeen()
// All UI elements should be dismissed
expect(shouldShowConflictModal.value).toBe(false)
expect(shouldShowRedDot.value).toBe(false)
expect(shouldShowManagerBanner.value).toBe(false)
})
it('should manage acknowledgment state correctly', () => {
const {
acknowledgmentState,
markConflictsAsSeen,
dismissRedDotNotification,
dismissWarningBanner
} = useConflictAcknowledgment()
// Initial state
expect(acknowledgmentState.value.modal_dismissed).toBe(false)
expect(acknowledgmentState.value.red_dot_dismissed).toBe(false)
expect(acknowledgmentState.value.warning_banner_dismissed).toBe(false)
// Update states
markConflictsAsSeen()
dismissRedDotNotification()
dismissWarningBanner()
// Check updated state
expect(acknowledgmentState.value.modal_dismissed).toBe(true)
expect(acknowledgmentState.value.red_dot_dismissed).toBe(true)
expect(acknowledgmentState.value.warning_banner_dismissed).toBe(true)
})
it('should use VueUse useStorage for persistence', () => {
// This test verifies that useStorage is being used by checking
// that values are automatically synced to localStorage
const { dismissConflictModal, acknowledgeConflict } =
const { markConflictsAsSeen, dismissWarningBanner } =
useConflictAcknowledgment()
dismissConflictModal()
acknowledgeConflict('test-pkg', 'test-conflict', '1.0.0')
markConflictsAsSeen()
dismissWarningBanner()
// VueUse useStorage should automatically persist to localStorage
// We can verify the keys exist (values will be stringified by VueUse)
expect(localStorage.getItem('Comfy.ConflictModalDismissed')).not.toBeNull()
expect(
localStorage.getItem('comfy_manager_conflict_banner_dismissed')
localStorage.getItem('Comfy.ConflictWarningBannerDismissed')
).not.toBeNull()
expect(localStorage.getItem('comfy_conflict_acknowledged')).not.toBeNull()
})
})

View File

@@ -1,426 +1,138 @@
import { createPinia, setActivePinia } from 'pinia'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { useConflictAcknowledgment } from '@/composables/useConflictAcknowledgment'
describe('useConflictAcknowledgment', () => {
// Mock localStorage
const mockLocalStorage = {
getItem: vi.fn(),
setItem: vi.fn(),
removeItem: vi.fn(),
clear: vi.fn()
}
beforeEach(() => {
// Reset localStorage mock
mockLocalStorage.getItem.mockClear()
mockLocalStorage.setItem.mockClear()
mockLocalStorage.removeItem.mockClear()
mockLocalStorage.clear.mockClear()
// Mock localStorage globally
Object.defineProperty(global, 'localStorage', {
value: mockLocalStorage,
writable: true
})
// Default mock returns
mockLocalStorage.getItem.mockReturnValue(null)
// Set up Pinia for each test
setActivePinia(createPinia())
// Clear localStorage before each test
localStorage.clear()
// Reset modules to ensure fresh state
vi.resetModules()
})
afterEach(() => {
vi.restoreAllMocks()
localStorage.clear()
})
describe('initial state loading', () => {
it('should load empty state when localStorage is empty', () => {
mockLocalStorage.getItem.mockReturnValue(null)
const { acknowledgmentState } = useConflictAcknowledgment()
expect(acknowledgmentState.value).toEqual({
modal_dismissed: false,
red_dot_dismissed: false,
acknowledged_conflicts: [],
last_comfyui_version: ''
warning_banner_dismissed: false
})
})
it('should load existing state from localStorage', () => {
mockLocalStorage.getItem.mockImplementation((key) => {
switch (key) {
case 'comfy_manager_conflict_banner_dismissed':
return 'true'
case 'comfy_help_center_conflict_seen':
return 'true'
case 'comfy_conflict_acknowledged':
return JSON.stringify([
{
package_id: 'TestPackage',
conflict_type: 'os',
timestamp: '2023-01-01T00:00:00.000Z',
comfyui_version: '0.3.41'
}
])
case 'comfyui.last_version':
return '0.3.41'
default:
return null
}
})
// Pre-populate localStorage
localStorage.setItem('Comfy.ConflictModalDismissed', 'true')
localStorage.setItem('Comfy.ConflictRedDotDismissed', 'true')
localStorage.setItem('Comfy.ConflictWarningBannerDismissed', 'true')
const { acknowledgmentState } = useConflictAcknowledgment()
expect(acknowledgmentState.value).toEqual({
modal_dismissed: true,
red_dot_dismissed: true,
acknowledged_conflicts: [
{
package_id: 'TestPackage',
conflict_type: 'os',
timestamp: '2023-01-01T00:00:00.000Z',
comfyui_version: '0.3.41'
}
],
last_comfyui_version: '0.3.41'
})
})
it.skip('should handle corrupted localStorage data gracefully', () => {
mockLocalStorage.getItem.mockImplementation((key) => {
if (key === 'comfy_conflict_acknowledged') {
return 'invalid-json'
}
return null
})
// VueUse's useStorage should handle corrupted data gracefully
const { acknowledgmentState } = useConflictAcknowledgment()
// Should fall back to default values when localStorage contains invalid JSON
expect(acknowledgmentState.value).toEqual({
modal_dismissed: false,
red_dot_dismissed: false,
acknowledged_conflicts: [],
last_comfyui_version: ''
warning_banner_dismissed: true
})
})
})
describe('ComfyUI version change detection', () => {
it('should detect version change and reset state', () => {
// Setup existing state
mockLocalStorage.getItem.mockImplementation((key) => {
switch (key) {
case 'comfyui.conflict.modal.dismissed':
return 'true'
case 'comfyui.conflict.red_dot.dismissed':
return 'true'
case 'comfyui.last_version':
return '0.3.40'
default:
return null
}
})
const consoleLogSpy = vi
.spyOn(console, 'log')
.mockImplementation(() => {})
const { checkComfyUIVersionChange, acknowledgmentState } =
describe('dismissal functions', () => {
it('should mark conflicts as seen with unified function', () => {
const { markConflictsAsSeen, acknowledgmentState } =
useConflictAcknowledgment()
const versionChanged = checkComfyUIVersionChange('0.3.41')
expect(versionChanged).toBe(true)
expect(acknowledgmentState.value.modal_dismissed).toBe(false)
expect(acknowledgmentState.value.red_dot_dismissed).toBe(false)
expect(acknowledgmentState.value.last_comfyui_version).toBe('0.3.41')
expect(consoleLogSpy).toHaveBeenCalledWith(
expect.stringContaining('ComfyUI version changed from 0.3.40 to 0.3.41')
)
consoleLogSpy.mockRestore()
})
it('should not detect version change for same version', () => {
mockLocalStorage.getItem.mockImplementation((key) => {
if (key === 'comfyui.last_version') {
return '0.3.41'
}
return null
})
const { checkComfyUIVersionChange } = useConflictAcknowledgment()
const versionChanged = checkComfyUIVersionChange('0.3.41')
expect(versionChanged).toBe(false)
})
it('should handle first run (no previous version)', () => {
mockLocalStorage.getItem.mockReturnValue(null)
const { checkComfyUIVersionChange } = useConflictAcknowledgment()
const versionChanged = checkComfyUIVersionChange('0.3.41')
expect(versionChanged).toBe(false)
})
})
describe('modal dismissal', () => {
it('should dismiss conflict modal and save to localStorage', () => {
const consoleLogSpy = vi
.spyOn(console, 'log')
.mockImplementation(() => {})
const { dismissConflictModal, acknowledgmentState } =
useConflictAcknowledgment()
dismissConflictModal()
markConflictsAsSeen()
expect(acknowledgmentState.value.modal_dismissed).toBe(true)
// useStorage handles localStorage synchronization internally
expect(consoleLogSpy).toHaveBeenCalledWith(
'[ConflictAcknowledgment] Conflict modal dismissed'
)
consoleLogSpy.mockRestore()
})
it('should dismiss red dot notification and save to localStorage', () => {
const consoleLogSpy = vi
.spyOn(console, 'log')
.mockImplementation(() => {})
it('should dismiss red dot notification', () => {
const { dismissRedDotNotification, acknowledgmentState } =
useConflictAcknowledgment()
dismissRedDotNotification()
expect(acknowledgmentState.value.red_dot_dismissed).toBe(true)
// useStorage handles localStorage synchronization internally
expect(consoleLogSpy).toHaveBeenCalledWith(
'[ConflictAcknowledgment] Red dot notification dismissed'
)
consoleLogSpy.mockRestore()
})
})
describe('conflict acknowledgment', () => {
it('should acknowledge a conflict and save to localStorage', () => {
const consoleLogSpy = vi
.spyOn(console, 'log')
.mockImplementation(() => {})
const dateSpy = vi
.spyOn(Date.prototype, 'toISOString')
.mockReturnValue('2023-01-01T00:00:00.000Z')
const { acknowledgeConflict, acknowledgmentState } =
it('should dismiss warning banner', () => {
const { dismissWarningBanner, acknowledgmentState } =
useConflictAcknowledgment()
acknowledgeConflict('TestPackage', 'os', '0.3.41')
dismissWarningBanner()
expect(acknowledgmentState.value.acknowledged_conflicts).toHaveLength(1)
expect(acknowledgmentState.value.acknowledged_conflicts[0]).toEqual({
package_id: 'TestPackage',
conflict_type: 'os',
timestamp: '2023-01-01T00:00:00.000Z',
comfyui_version: '0.3.41'
})
// useStorage handles localStorage synchronization internally
expect(acknowledgmentState.value.acknowledged_conflicts).toHaveLength(1)
expect(acknowledgmentState.value.acknowledged_conflicts[0]).toEqual({
package_id: 'TestPackage',
conflict_type: 'os',
timestamp: '2023-01-01T00:00:00.000Z',
comfyui_version: '0.3.41'
})
expect(consoleLogSpy).toHaveBeenCalledWith(
'[ConflictAcknowledgment] Acknowledged conflict for TestPackage:os'
)
dateSpy.mockRestore()
consoleLogSpy.mockRestore()
expect(acknowledgmentState.value.warning_banner_dismissed).toBe(true)
})
it('should replace existing acknowledgment for same package and conflict type', () => {
const { acknowledgeConflict, acknowledgmentState } =
it('should mark all conflicts as seen', () => {
const { markConflictsAsSeen, acknowledgmentState } =
useConflictAcknowledgment()
// First acknowledgment
acknowledgeConflict('TestPackage', 'os', '0.3.41')
expect(acknowledgmentState.value.acknowledged_conflicts).toHaveLength(1)
markConflictsAsSeen()
// Second acknowledgment for same package and conflict type
acknowledgeConflict('TestPackage', 'os', '0.3.42')
expect(acknowledgmentState.value.acknowledged_conflicts).toHaveLength(1)
expect(
acknowledgmentState.value.acknowledged_conflicts[0].comfyui_version
).toBe('0.3.42')
})
it('should allow multiple acknowledgments for different conflict types', () => {
const { acknowledgeConflict, acknowledgmentState } =
useConflictAcknowledgment()
acknowledgeConflict('TestPackage', 'os', '0.3.41')
acknowledgeConflict('TestPackage', 'accelerator', '0.3.41')
expect(acknowledgmentState.value.acknowledged_conflicts).toHaveLength(2)
})
})
describe('conflict checking', () => {
it('should check if conflict is acknowledged', () => {
const { acknowledgeConflict, isConflictAcknowledged } =
useConflictAcknowledgment()
// Initially not acknowledged
expect(isConflictAcknowledged('TestPackage', 'os')).toBe(false)
// After acknowledgment
acknowledgeConflict('TestPackage', 'os', '0.3.41')
expect(isConflictAcknowledged('TestPackage', 'os')).toBe(true)
// Different conflict type should not be acknowledged
expect(isConflictAcknowledged('TestPackage', 'accelerator')).toBe(false)
})
it('should remove conflict acknowledgment', () => {
const consoleLogSpy = vi
.spyOn(console, 'log')
.mockImplementation(() => {})
const {
acknowledgeConflict,
removeConflictAcknowledgment,
isConflictAcknowledged,
acknowledgmentState
} = useConflictAcknowledgment()
// Add acknowledgment
acknowledgeConflict('TestPackage', 'os', '0.3.41')
expect(isConflictAcknowledged('TestPackage', 'os')).toBe(true)
// Remove acknowledgment
removeConflictAcknowledgment('TestPackage', 'os')
expect(isConflictAcknowledged('TestPackage', 'os')).toBe(false)
expect(acknowledgmentState.value.acknowledged_conflicts).toHaveLength(0)
expect(consoleLogSpy).toHaveBeenCalledWith(
'[ConflictAcknowledgment] Removed acknowledgment for TestPackage:os'
)
consoleLogSpy.mockRestore()
expect(acknowledgmentState.value.modal_dismissed).toBe(true)
expect(acknowledgmentState.value.red_dot_dismissed).toBe(true)
expect(acknowledgmentState.value.warning_banner_dismissed).toBe(true)
})
})
describe('computed properties', () => {
it('should calculate shouldShowConflictModal correctly', () => {
const { shouldShowConflictModal, dismissConflictModal } =
const { shouldShowConflictModal, markConflictsAsSeen } =
useConflictAcknowledgment()
expect(shouldShowConflictModal.value).toBe(true)
dismissConflictModal()
markConflictsAsSeen()
expect(shouldShowConflictModal.value).toBe(false)
})
it('should calculate shouldShowRedDot correctly', () => {
it('should calculate shouldShowRedDot correctly based on conflicts', () => {
const { shouldShowRedDot, dismissRedDotNotification } =
useConflictAcknowledgment()
expect(shouldShowRedDot.value).toBe(true)
// Initially false because no conflicts exist
expect(shouldShowRedDot.value).toBe(false)
dismissRedDotNotification()
expect(shouldShowRedDot.value).toBe(false)
})
it('should calculate acknowledgedPackageIds correctly', () => {
const { acknowledgeConflict, acknowledgedPackageIds } =
it('should calculate shouldShowManagerBanner correctly', () => {
const { shouldShowManagerBanner, dismissWarningBanner } =
useConflictAcknowledgment()
expect(acknowledgedPackageIds.value).toEqual([])
// Initially false because no conflicts exist
expect(shouldShowManagerBanner.value).toBe(false)
acknowledgeConflict('Package1', 'os', '0.3.41')
acknowledgeConflict('Package2', 'accelerator', '0.3.41')
acknowledgeConflict('Package1', 'accelerator', '0.3.41') // Same package, different conflict
expect(acknowledgedPackageIds.value).toEqual(['Package1', 'Package2'])
})
it('should calculate acknowledgmentStats correctly', () => {
const { acknowledgeConflict, dismissConflictModal, acknowledgmentStats } =
useConflictAcknowledgment()
acknowledgeConflict('Package1', 'os', '0.3.41')
acknowledgeConflict('Package2', 'accelerator', '0.3.41')
dismissConflictModal()
expect(acknowledgmentStats.value).toEqual({
total_acknowledged: 2,
unique_packages: 2,
modal_dismissed: true,
red_dot_dismissed: false,
last_comfyui_version: ''
})
dismissWarningBanner()
expect(shouldShowManagerBanner.value).toBe(false)
})
})
describe('clear functionality', () => {
it('should clear all acknowledgments', () => {
const consoleLogSpy = vi
.spyOn(console, 'log')
.mockImplementation(() => {})
describe('localStorage persistence', () => {
it('should persist to localStorage automatically', () => {
const { markConflictsAsSeen, dismissWarningBanner } =
useConflictAcknowledgment()
const {
acknowledgeConflict,
dismissConflictModal,
clearAllAcknowledgments,
acknowledgmentState
} = useConflictAcknowledgment()
markConflictsAsSeen()
dismissWarningBanner()
// Add some data
acknowledgeConflict('Package1', 'os', '0.3.41')
dismissConflictModal()
// Clear all
clearAllAcknowledgments()
expect(acknowledgmentState.value).toEqual({
modal_dismissed: false,
red_dot_dismissed: false,
acknowledged_conflicts: [],
last_comfyui_version: ''
})
expect(consoleLogSpy).toHaveBeenCalledWith(
'[ConflictAcknowledgment] Cleared all acknowledgments'
)
consoleLogSpy.mockRestore()
})
})
describe.skip('localStorage error handling', () => {
it('should handle localStorage setItem errors gracefully', () => {
mockLocalStorage.setItem.mockImplementation(() => {
throw new Error('localStorage full')
})
const { dismissConflictModal, acknowledgmentState } = useConflictAcknowledgment()
// VueUse's useStorage should handle localStorage errors gracefully
expect(() => dismissConflictModal()).not.toThrow()
// State should still be updated in memory even if localStorage fails
expect(acknowledgmentState.value.modal_dismissed).toBe(true)
// VueUse useStorage should automatically persist to localStorage
expect(
localStorage.getItem('Comfy.ConflictModalDismissed')
).not.toBeNull()
expect(
localStorage.getItem('Comfy.ConflictWarningBannerDismissed')
).not.toBeNull()
})
})
})

View File

@@ -906,23 +906,11 @@ describe.skip('useConflictDetection with Registry Store', () => {
)
})
it('should expose acknowledgment state and methods', () => {
it('should expose conflict modal display method', () => {
const {
shouldShowConflictModal,
shouldShowRedDot,
acknowledgedPackageIds,
dismissConflictModal,
dismissRedDotNotification,
acknowledgePackageConflict,
shouldShowConflictModalAfterUpdate
} = useConflictDetection()
expect(shouldShowConflictModal).toBeDefined()
expect(shouldShowRedDot).toBeDefined()
expect(acknowledgedPackageIds).toBeDefined()
expect(dismissConflictModal).toBeDefined()
expect(dismissRedDotNotification).toBeDefined()
expect(acknowledgePackageConflict).toBeDefined()
expect(shouldShowConflictModalAfterUpdate).toBeDefined()
})
@@ -981,18 +969,8 @@ describe.skip('useConflictDetection with Registry Store', () => {
expect(result).toBe(true) // Should show modal when conflicts exist and not dismissed
})
it('should call acknowledgment methods when dismissing', () => {
const { dismissConflictModal, dismissRedDotNotification } =
useConflictDetection()
dismissConflictModal()
expect(mockAcknowledgment.dismissConflictModal).toHaveBeenCalled()
dismissRedDotNotification()
expect(mockAcknowledgment.dismissRedDotNotification).toHaveBeenCalled()
})
it('should acknowledge package conflicts with system version', async () => {
it('should detect system environment correctly', async () => {
// Mock system environment
mockSystemStatsStore.systemStats = {
system: {
@@ -1002,20 +980,12 @@ describe.skip('useConflictDetection with Registry Store', () => {
devices: []
}
const { acknowledgePackageConflict, detectSystemEnvironment } =
useConflictDetection()
const { detectSystemEnvironment } = useConflictDetection()
// First detect system environment
await detectSystemEnvironment()
// Detect system environment
const environment = await detectSystemEnvironment()
// Then acknowledge conflict
acknowledgePackageConflict('TestPackage', 'os')
expect(mockAcknowledgment.acknowledgeConflict).toHaveBeenCalledWith(
'TestPackage',
'os',
'0.3.41' // System version from mock data
)
expect(environment.comfyui_version).toBe('0.3.41')
})
})