diff --git a/src/workbench/extensions/manager/composables/nodePack/usePackInstall.test.ts b/src/workbench/extensions/manager/composables/nodePack/usePackInstall.test.ts deleted file mode 100644 index dde2e13d39..0000000000 --- a/src/workbench/extensions/manager/composables/nodePack/usePackInstall.test.ts +++ /dev/null @@ -1,146 +0,0 @@ -import { createTestingPinia } from '@pinia/testing' -import { setActivePinia } from 'pinia' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' - -import type { components } from '@/types/comfyRegistryTypes' -import { usePackInstall } from '@/workbench/extensions/manager/composables/nodePack/usePackInstall' - -type NodePack = components['schemas']['Node'] - -const toastAdd = vi.fn() -const installPackCall = vi.fn() -const installPackClear = vi.fn() -const isPackInstalled = vi.fn(() => false) - -vi.mock('@/platform/updates/common/toastStore', () => ({ - useToastStore: vi.fn(() => ({ - add: toastAdd - })) -})) - -vi.mock('@/workbench/extensions/manager/stores/comfyManagerStore', () => ({ - useComfyManagerStore: vi.fn(() => ({ - installPack: { - call: installPackCall, - clear: installPackClear - }, - isPackInstalled, - isPackInstalling: vi.fn(() => false) - })) -})) - -vi.mock( - '@/workbench/extensions/manager/composables/useNodeConflictDialog', - () => ({ - useNodeConflictDialog: vi.fn(() => ({ - show: vi.fn() - })) - }) -) - -vi.mock( - '@/workbench/extensions/manager/composables/useConflictDetection', - () => ({ - useConflictDetection: vi.fn(() => ({ - checkNodeCompatibility: vi.fn(() => ({ - hasConflict: false, - conflicts: [] - })) - })) - }) -) - -vi.mock('vue-i18n', () => ({ - useI18n: vi.fn(() => ({ - t: (key: string, ...args: unknown[]) => { - const named = args.find( - (a): a is Record => - typeof a === 'object' && a !== null && !Array.isArray(a) - ) - if (named && 'count' in named) { - return `${key}:${String(named.count)}` - } - return key - } - })) -})) - -const createMockPack = (id: string): NodePack => - ({ - id, - name: `Pack ${id}`, - repository: 'https://github.com/test/pack', - publisher: { name: 'TestPublisher' }, - latest_version: { version: '1.0.0' } - }) as unknown as NodePack - -describe('usePackInstall', () => { - beforeEach(() => { - setActivePinia(createTestingPinia({ stubActions: false })) - toastAdd.mockClear() - installPackCall.mockReset() - installPackClear.mockClear() - isPackInstalled.mockReturnValue(false) - vi.spyOn(console, 'error').mockImplementation(() => {}) - }) - - afterEach(() => { - vi.restoreAllMocks() - }) - - it('does not show a toast when all installations succeed', async () => { - installPackCall.mockResolvedValue(undefined) - const packs = [createMockPack('a'), createMockPack('b')] - const { performInstallation } = usePackInstall(() => packs) - - await performInstallation(packs) - - expect(toastAdd).not.toHaveBeenCalled() - expect(installPackClear).toHaveBeenCalledTimes(1) - }) - - it('shows a single error toast summarising failures when installations fail', async () => { - installPackCall - .mockRejectedValueOnce(new Error('boom-a')) - .mockRejectedValueOnce(new Error('boom-b')) - const packs = [createMockPack('a'), createMockPack('b')] - const { performInstallation } = usePackInstall(() => packs) - - await performInstallation(packs) - - expect(toastAdd).toHaveBeenCalledTimes(1) - const message = toastAdd.mock.calls[0][0] - expect(message.severity).toBe('error') - expect(message.summary).toBe('manager.installFailureToast.summary') - expect(message.detail).toBe('manager.installFailureToast.detail:2') - }) - - it('shows the toast even when only one of many installs fails', async () => { - installPackCall - .mockResolvedValueOnce(undefined) - .mockRejectedValueOnce(new Error('boom')) - .mockResolvedValueOnce(undefined) - const packs = [ - createMockPack('a'), - createMockPack('b'), - createMockPack('c') - ] - const { performInstallation } = usePackInstall(() => packs) - - await performInstallation(packs) - - expect(toastAdd).toHaveBeenCalledTimes(1) - const message = toastAdd.mock.calls[0][0] - expect(message.detail).toBe('manager.installFailureToast.detail:1') - }) - - it('always clears the install cache, even on failure', async () => { - installPackCall.mockRejectedValue(new Error('boom')) - const packs = [createMockPack('a')] - const { performInstallation } = usePackInstall(() => packs) - - await performInstallation(packs) - - expect(installPackClear).toHaveBeenCalledTimes(1) - }) -}) diff --git a/src/workbench/extensions/manager/composables/nodePack/usePackInstall.ts b/src/workbench/extensions/manager/composables/nodePack/usePackInstall.ts index ea95cd0e9e..db0e14fb18 100644 --- a/src/workbench/extensions/manager/composables/nodePack/usePackInstall.ts +++ b/src/workbench/extensions/manager/composables/nodePack/usePackInstall.ts @@ -1,7 +1,6 @@ import { computed } from 'vue' import { useI18n } from 'vue-i18n' -import { useToastStore } from '@/platform/updates/common/toastStore' import type { components } from '@/types/comfyRegistryTypes' import type { components as ManagerComponents } from '@/workbench/extensions/manager/types/generatedManagerTypes' import { useConflictDetection } from '@/workbench/extensions/manager/composables/useConflictDetection' @@ -63,16 +62,6 @@ export function usePackInstall( '[usePackInstall] Some installations failed:', failures.map((f) => f.reason) ) - useToastStore().add({ - severity: 'error', - summary: t('manager.installFailureToast.summary'), - detail: t( - 'manager.installFailureToast.detail', - { count: failures.length }, - failures.length - ), - life: 8000 - }) } } finally { managerStore.installPack.clear() diff --git a/src/workbench/extensions/manager/stores/comfyManagerStore.test.ts b/src/workbench/extensions/manager/stores/comfyManagerStore.test.ts index 1571624b33..0eaae8e44c 100644 --- a/src/workbench/extensions/manager/stores/comfyManagerStore.test.ts +++ b/src/workbench/extensions/manager/stores/comfyManagerStore.test.ts @@ -1,6 +1,6 @@ import { createTestingPinia } from '@pinia/testing' import { setActivePinia } from 'pinia' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { nextTick, ref } from 'vue' import { useComfyManagerService } from '@/workbench/extensions/manager/services/comfyManagerService' @@ -55,6 +55,21 @@ vi.mock('vue-i18n', () => ({ })) })) +const toastAddMock = vi.fn() + +vi.mock('@/platform/updates/common/toastStore', () => ({ + useToastStore: vi.fn(() => ({ + add: toastAddMock + })) +})) + +vi.mock('@/i18n', () => ({ + t: (key: string, params?: Record) => { + if (params && 'count' in params) return `${key}:${String(params.count)}` + return key + } +})) + interface EnabledDisabledTestCase { desc: string installed: Record @@ -79,6 +94,8 @@ describe('useComfyManagerStore', () => { beforeEach(() => { setActivePinia(createTestingPinia({ stubActions: false })) vi.clearAllMocks() + toastAddMock.mockClear() + vi.useFakeTimers() mockManagerService = { isLoading: ref(false), error: ref(null), @@ -102,6 +119,10 @@ describe('useComfyManagerStore', () => { vi.mocked(useComfyManagerService).mockReturnValue(mockManagerService) }) + afterEach(() => { + vi.useRealTimers() + }) + const testCases: EnabledDisabledTestCase[] = [ { desc: 'Two enabled versions', @@ -501,4 +522,85 @@ describe('useComfyManagerStore', () => { expect(store.isPackInstalled('disabled-pack')).toBe(true) }) }) + + describe('installation failure toast', () => { + const setTaskHistory = ( + store: ReturnType, + history: Record + ) => { + store.taskHistory = history + } + + const errorTask = ( + id: string + ): ManagerComponents['schemas']['TaskHistoryItem'] => ({ + ui_id: id, + client_id: 'test', + kind: 'install', + result: 'failed', + status: { status_str: 'error', completed: false, messages: ['boom'] }, + timestamp: new Date().toISOString() + }) + + const successTask = ( + id: string + ): ManagerComponents['schemas']['TaskHistoryItem'] => ({ + ui_id: id, + client_id: 'test', + kind: 'install', + result: 'success', + status: { status_str: 'success', completed: true, messages: [] }, + timestamp: new Date().toISOString() + }) + + it('shows an error toast when a task fails', async () => { + const store = useComfyManagerStore() + setTaskHistory(store, { a: errorTask('a') }) + await nextTick() + await vi.runAllTimersAsync() + + expect(toastAddMock).toHaveBeenCalledTimes(1) + const message = toastAddMock.mock.calls[0][0] + expect(message.severity).toBe('error') + expect(message.summary).toBe('manager.installFailureToast.summary') + expect(message.detail).toBe('manager.installFailureToast.detail:1') + }) + + it('does not show a toast when all tasks succeed', async () => { + const store = useComfyManagerStore() + setTaskHistory(store, { a: successTask('a'), b: successTask('b') }) + await nextTick() + await vi.runAllTimersAsync() + + expect(toastAddMock).not.toHaveBeenCalled() + }) + + it('coalesces multiple failures that land in quick succession', async () => { + const store = useComfyManagerStore() + setTaskHistory(store, { a: errorTask('a') }) + await nextTick() + setTaskHistory(store, { a: errorTask('a'), b: errorTask('b') }) + await nextTick() + await vi.runAllTimersAsync() + + expect(toastAddMock).toHaveBeenCalledTimes(1) + expect(toastAddMock.mock.calls[0][0].detail).toBe( + 'manager.installFailureToast.detail:2' + ) + }) + + it('does not show a toast when failures are cleared via resetTaskState', async () => { + const store = useComfyManagerStore() + setTaskHistory(store, { a: errorTask('a') }) + await nextTick() + await vi.runAllTimersAsync() + toastAddMock.mockClear() + + store.resetTaskState() + await nextTick() + await vi.runAllTimersAsync() + + expect(toastAddMock).not.toHaveBeenCalled() + }) + }) }) diff --git a/src/workbench/extensions/manager/stores/comfyManagerStore.ts b/src/workbench/extensions/manager/stores/comfyManagerStore.ts index c6ae8a8e3a..55b90c3522 100644 --- a/src/workbench/extensions/manager/stores/comfyManagerStore.ts +++ b/src/workbench/extensions/manager/stores/comfyManagerStore.ts @@ -1,4 +1,4 @@ -import { useEventListener, whenever } from '@vueuse/core' +import { useDebounceFn, useEventListener, whenever } from '@vueuse/core' import { defineStore } from 'pinia' import { v4 as uuidv4 } from 'uuid' import { ref, watch } from 'vue' @@ -6,6 +6,7 @@ import { ref, watch } from 'vue' import { t } from '@/i18n' import { useCachedRequest } from '@/composables/useCachedRequest' import { useServerLogs } from '@/composables/useServerLogs' +import { useToastStore } from '@/platform/updates/common/toastStore' import { api } from '@/scripts/api' import { app } from '@/scripts/app' @@ -112,6 +113,30 @@ export const useComfyManagerStore = defineStore('comfyManager', () => { { deep: true } ) + const pendingFailureCount = ref(0) + + const flushFailureToast = useDebounceFn(() => { + if (pendingFailureCount.value === 0) return + const count = pendingFailureCount.value + pendingFailureCount.value = 0 + useToastStore().add({ + severity: 'error', + summary: t('manager.installFailureToast.summary'), + detail: t('manager.installFailureToast.detail', { count }, count), + life: 8000 + }) + }, 300) + + watch( + () => failedTasksIds.value.length, + (newCount, oldCount) => { + const delta = newCount - (oldCount ?? 0) + if (delta <= 0) return + pendingFailureCount.value += delta + void flushFailureToast() + } + ) + const getPackId = (pack: ManagerPackInstalled) => pack.cnr_id || pack.aux_id const isInstalledPackId = (packName: string | undefined): boolean =>