diff --git a/browser_tests/fixtures/components/SidebarTab.ts b/browser_tests/fixtures/components/SidebarTab.ts index 119afe11bfa..d2525cd60e0 100644 --- a/browser_tests/fixtures/components/SidebarTab.ts +++ b/browser_tests/fixtures/components/SidebarTab.ts @@ -386,11 +386,14 @@ export class AssetsSidebarTab extends SidebarTab { return this.page.locator('.p-contextmenu').getByText(label) } - override async open() { + override async open({ waitForAssets = true } = {}) { // Remove any toast notifications that may overlay the sidebar button await this.dismissToasts() await super.open() await this.generatedTab.waitFor({ state: 'visible' }) + if (waitForAssets) { + await this.waitForAssets() + } } /** Dismiss all visible toast notifications by clicking their close buttons. */ diff --git a/browser_tests/fixtures/helpers/ToastHelper.ts b/browser_tests/fixtures/helpers/ToastHelper.ts index 7fc175bcf80..cfe7581bc88 100644 --- a/browser_tests/fixtures/helpers/ToastHelper.ts +++ b/browser_tests/fixtures/helpers/ToastHelper.ts @@ -4,10 +4,16 @@ import type { Locator, Page } from '@playwright/test' export class ToastHelper { public readonly visibleToasts: Locator public readonly toastErrors: Locator + public readonly toastSuccesses: Locator + public readonly toastWarnings: Locator constructor(private readonly page: Page) { this.visibleToasts = page.locator('.p-toast-message:visible') this.toastErrors = page.locator('.p-toast-message.p-toast-message-error') + this.toastSuccesses = page.locator( + '.p-toast-message.p-toast-message-success' + ) + this.toastWarnings = page.locator('.p-toast-message.p-toast-message-warn') } async closeToasts(requireCount = 0): Promise { diff --git a/browser_tests/tests/sidebar/assets.spec.ts b/browser_tests/tests/sidebar/assets.spec.ts index cadd23d6aef..573edf05e72 100644 --- a/browser_tests/tests/sidebar/assets.spec.ts +++ b/browser_tests/tests/sidebar/assets.spec.ts @@ -82,6 +82,19 @@ const JOB_GAMMA_DETAIL: JobDetail = { } ] } + }, + workflow: { + extra_data: { + extra_pnginfo: { + workflow: { + version: 0.4, + last_node_id: 0, + last_link_id: 0, + nodes: [], + links: [] + } + } + } } } @@ -112,7 +125,7 @@ test.describe('Assets sidebar - empty states', () => { test('Shows empty-state copy for generated tab', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab - await tab.open() + await tab.open({ waitForAssets: false }) await expect(tab.emptyStateTitle('No generated files found')).toBeVisible() await expect(tab.emptyStateMessage).toBeVisible() @@ -120,7 +133,7 @@ test.describe('Assets sidebar - empty states', () => { test('Shows empty-state copy for imported tab', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab - await tab.open() + await tab.open({ waitForAssets: false }) await tab.switchToImported() await expect(tab.emptyStateTitle('No imported files found')).toBeVisible() @@ -129,7 +142,7 @@ test.describe('Assets sidebar - empty states', () => { test('No asset cards are rendered when empty', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab - await tab.open() + await tab.open({ waitForAssets: false }) await expect(tab.assetCards).toHaveCount(0) }) @@ -209,7 +222,6 @@ test.describe('Assets sidebar - grid view display', () => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await expect.poll(() => tab.assetCards.count()).toBeGreaterThanOrEqual(1) }) @@ -270,7 +282,6 @@ test.describe('Assets sidebar - view mode toggle', () => { test('Can switch to list view via settings menu', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() // Open settings menu and select list view await tab.openSettingsMenu() @@ -283,7 +294,6 @@ test.describe('Assets sidebar - view mode toggle', () => { test('Can switch back to grid view', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() // Switch to list view await tab.openSettingsMenu() @@ -326,7 +336,6 @@ test.describe('Assets sidebar - search', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const initialCount = await tab.assetCards.count() @@ -340,7 +349,6 @@ test.describe('Assets sidebar - search', () => { test('Clearing search restores all assets', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const initialCount = await tab.assetCards.count() @@ -355,7 +363,6 @@ test.describe('Assets sidebar - search', () => { test('Search with no matches shows empty state', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.searchInput.fill('nonexistent_file_xyz') await expect(tab.assetCards).toHaveCount(0) @@ -380,7 +387,6 @@ test.describe('Assets sidebar - selection', () => { test('Clicking an asset card selects it', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() // Click first asset card await tab.assetCards.first().click() @@ -392,7 +398,6 @@ test.describe('Assets sidebar - selection', () => { test('Ctrl+click adds to selection', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const cards = tab.assetCards await expect.poll(() => cards.count()).toBeGreaterThanOrEqual(2) @@ -411,7 +416,6 @@ test.describe('Assets sidebar - selection', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() // Select an asset await tab.assetCards.first().click() @@ -423,7 +427,6 @@ test.describe('Assets sidebar - selection', () => { test('Deselect all clears selection', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() // Select an asset await tab.assetCards.first().click() @@ -441,7 +444,6 @@ test.describe('Assets sidebar - selection', () => { test('Selection is cleared when switching tabs', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() // Select an asset await tab.assetCards.first().click() @@ -475,7 +477,6 @@ test.describe('Assets sidebar - context menu', () => { test('Right-clicking an asset shows context menu', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() // Right-click first asset await tab.assetCards.first().click({ button: 'right' }) @@ -490,7 +491,6 @@ test.describe('Assets sidebar - context menu', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click({ button: 'right' }) await comfyPage.page @@ -505,7 +505,6 @@ test.describe('Assets sidebar - context menu', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click({ button: 'right' }) await comfyPage.page @@ -520,7 +519,6 @@ test.describe('Assets sidebar - context menu', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click({ button: 'right' }) await comfyPage.page @@ -535,7 +533,6 @@ test.describe('Assets sidebar - context menu', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click({ button: 'right' }) await comfyPage.page @@ -550,7 +547,6 @@ test.describe('Assets sidebar - context menu', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click({ button: 'right' }) @@ -563,12 +559,79 @@ test.describe('Assets sidebar - context menu', () => { await expect(tab.contextMenuItem('Export workflow')).toBeVisible() }) + test('Cancelling export-workflow filename prompt does not show an error toast', async ({ + comfyPage + }) => { + // job-gamma is the first card; its detail carries a valid workflow so + // extraction succeeds and the filename prompt opens. + await comfyPage.assets.mockJobDetail('job-gamma', JOB_GAMMA_DETAIL) + + const tab = comfyPage.menu.assetsTab + await tab.open() + + await tab.assetCards.first().click({ button: 'right' }) + await tab.contextMenuItem('Export workflow').click() + + const promptDialog = comfyPage.page.getByRole('dialog', { + name: 'Export Workflow' + }) + await expect(promptDialog).toBeVisible() + + await comfyPage.page.keyboard.press('Escape') + await expect(promptDialog).toBeHidden() + + await expect(comfyPage.toast.toastErrors).toBeHidden({ timeout: 1500 }) + }) + + test('Confirming export-workflow prompt downloads the file and shows a success toast', async ({ + comfyPage + }) => { + await comfyPage.assets.mockJobDetail('job-gamma', JOB_GAMMA_DETAIL) + + const tab = comfyPage.menu.assetsTab + await tab.open() + + await tab.assetCards.first().click({ button: 'right' }) + await tab.contextMenuItem('Export workflow').click() + + const promptDialog = comfyPage.page.getByRole('dialog', { + name: 'Export Workflow' + }) + await expect(promptDialog).toBeVisible() + + const downloadPromise = comfyPage.page.waitForEvent('download') + await promptDialog.getByRole('button', { name: 'Confirm' }).click() + + const download = await downloadPromise + expect(download.suggestedFilename()).toBe('abstract_art.json') + + await expect(comfyPage.toast.toastSuccesses).toBeVisible() + }) + + test('Export-workflow shows a warning toast when the asset has no workflow', async ({ + comfyPage + }) => { + // Strip the workflow field so extraction yields null and the export + // action returns { success: false, error: 'No workflow…' }. + const { workflow: _, ...detailWithoutWorkflow } = JOB_GAMMA_DETAIL + await comfyPage.assets.mockJobDetail('job-gamma', detailWithoutWorkflow) + + const tab = comfyPage.menu.assetsTab + await tab.open() + + await tab.assetCards.first().click({ button: 'right' }) + await tab.contextMenuItem('Export workflow').click() + + // Filename prompt should be skipped: extraction fails before the prompt. + await expect(comfyPage.toast.toastWarnings).toBeVisible() + await expect(comfyPage.toast.toastSuccesses).toBeHidden({ timeout: 1500 }) + }) + test('Bulk context menu shows when multiple assets selected', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const cards = tab.assetCards await expect.poll(() => cards.count()).toBeGreaterThanOrEqual(2) @@ -623,7 +686,6 @@ test.describe('Assets sidebar - bulk actions', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click() @@ -636,7 +698,6 @@ test.describe('Assets sidebar - bulk actions', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click() @@ -647,7 +708,6 @@ test.describe('Assets sidebar - bulk actions', () => { test('Selection count displays correct number', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() // Select the two single-output assets (job-alpha, job-beta). // The count reflects total outputs, not cards — job-gamma has @@ -676,7 +736,6 @@ cloudTest.describe('Assets sidebar - cloud exports', { tag: '@cloud' }, () => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click() await expect(tab.downloadSelectedButton).toBeVisible() @@ -701,7 +760,6 @@ cloudTest.describe('Assets sidebar - cloud exports', { tag: '@cloud' }, () => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards .first() @@ -737,7 +795,6 @@ cloudTest.describe('Assets sidebar - cloud exports', { tag: '@cloud' }, () => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.nth(1).click() await comfyPage.page.keyboard.down('Control') @@ -784,7 +841,6 @@ test.describe('Assets sidebar - pagination', () => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const req = await firstRequest const url = new URL(req.url()) @@ -840,7 +896,6 @@ test.describe('Assets sidebar - delete confirmation', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() await tab.assetCards.first().click({ button: 'right' }) await tab.contextMenuItem('Delete').click() @@ -858,7 +913,6 @@ test.describe('Assets sidebar - delete confirmation', () => { }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const initialCount = await tab.assetCards.count() @@ -880,7 +934,6 @@ test.describe('Assets sidebar - delete confirmation', () => { test('Cancelling delete preserves asset', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const initialCount = await tab.assetCards.count() @@ -977,7 +1030,6 @@ test.describe('Assets sidebar - media type filter', () => { test('Unchecking image filter hides image assets', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const initialCount = tab.assetCards await expect( @@ -998,7 +1050,6 @@ test.describe('Assets sidebar - media type filter', () => { test('Re-enabling filter restores hidden assets', async ({ comfyPage }) => { const tab = comfyPage.menu.assetsTab await tab.open() - await tab.waitForAssets() const initialCount = await tab.assetCards.count() diff --git a/src/platform/assets/composables/useMediaAssetActions.test.ts b/src/platform/assets/composables/useMediaAssetActions.test.ts index c5f558e0b6d..60165076c96 100644 --- a/src/platform/assets/composables/useMediaAssetActions.test.ts +++ b/src/platform/assets/composables/useMediaAssetActions.test.ts @@ -78,13 +78,20 @@ vi.mock('@/composables/useCopyToClipboard', () => ({ }) })) +const mockExportWorkflowAction = vi.hoisted(() => vi.fn()) +const mockOpenWorkflowAction = vi.hoisted(() => vi.fn()) vi.mock('@/platform/workflow/core/services/workflowActionsService', () => ({ useWorkflowActionsService: () => ({ - openWorkflowAction: vi.fn(), - exportWorkflowAction: vi.fn() + openWorkflowAction: mockOpenWorkflowAction, + exportWorkflowAction: mockExportWorkflowAction }) })) +const mockExtractWorkflowFromAsset = vi.hoisted(() => vi.fn()) +vi.mock('@/platform/workflow/utils/workflowExtractionUtil', () => ({ + extractWorkflowFromAsset: mockExtractWorkflowFromAsset +})) + vi.mock('@/services/litegraphService', () => ({ useLitegraphService: () => ({ addNodeOnGraph: vi.fn().mockReturnValue( @@ -376,6 +383,109 @@ describe('useMediaAssetActions', () => { }) }) + describe('exportWorkflow', () => { + const successResult = { success: true } as const + const cancelledResult = { success: false, cancelled: true } as const + const failureResult = { success: false, error: 'boom' } as const + const noWorkflowResult = { + success: false, + error: 'No workflow data available' + } as const + + beforeEach(() => { + mockExtractWorkflowFromAsset.mockResolvedValue({ + workflow: { version: 0.4 }, + filename: 'export.json' + }) + }) + + it('does not show a toast when the user cancels the filename prompt', async () => { + mockExportWorkflowAction.mockResolvedValue(cancelledResult) + const actions = useMediaAssetActions() + + await actions.exportWorkflow(createMockAsset()) + + expect(useToast().add).not.toHaveBeenCalled() + }) + + it('shows a success toast on successful export', async () => { + mockExportWorkflowAction.mockResolvedValue(successResult) + const actions = useMediaAssetActions() + + await actions.exportWorkflow(createMockAsset()) + + expect(useToast().add).toHaveBeenCalledWith( + expect.objectContaining({ severity: 'success' }) + ) + }) + + it('shows an error toast on actual failure', async () => { + mockExportWorkflowAction.mockResolvedValue(failureResult) + const actions = useMediaAssetActions() + + await actions.exportWorkflow(createMockAsset()) + + expect(useToast().add).toHaveBeenCalledWith( + expect.objectContaining({ severity: 'error' }) + ) + }) + + it('shows a warning toast when the workflow is missing', async () => { + mockExportWorkflowAction.mockResolvedValue(noWorkflowResult) + const actions = useMediaAssetActions() + + await actions.exportWorkflow(createMockAsset()) + + expect(useToast().add).toHaveBeenCalledWith( + expect.objectContaining({ severity: 'warn' }) + ) + }) + + it('shows no toast when every asset in a bulk export is cancelled', async () => { + mockExportWorkflowAction.mockResolvedValue(cancelledResult) + const actions = useMediaAssetActions() + + await actions.exportMultipleWorkflows([ + createMockAsset({ id: 'a' }), + createMockAsset({ id: 'b' }) + ]) + + expect(useToast().add).not.toHaveBeenCalled() + }) + + it('shows a success toast for the succeeded subset when some bulk exports are cancelled', async () => { + mockExportWorkflowAction + .mockResolvedValueOnce(successResult) + .mockResolvedValueOnce(cancelledResult) + const actions = useMediaAssetActions() + + await actions.exportMultipleWorkflows([ + createMockAsset({ id: 'a' }), + createMockAsset({ id: 'b' }) + ]) + + expect(useToast().add).toHaveBeenCalledWith( + expect.objectContaining({ severity: 'success' }) + ) + }) + + it('shows a partial-success warning toast when some bulk exports fail outright', async () => { + mockExportWorkflowAction + .mockResolvedValueOnce(successResult) + .mockResolvedValueOnce(failureResult) + const actions = useMediaAssetActions() + + await actions.exportMultipleWorkflows([ + createMockAsset({ id: 'a' }), + createMockAsset({ id: 'b' }) + ]) + + expect(useToast().add).toHaveBeenCalledWith( + expect.objectContaining({ severity: 'warn' }) + ) + }) + }) + describe('downloadAssets', () => { it('downloads the injected media asset when called without explicit assets', () => { const mediaAsset = createMockMediaAsset({ diff --git a/src/platform/assets/composables/useMediaAssetActions.ts b/src/platform/assets/composables/useMediaAssetActions.ts index 6b0a9a631c6..b45fed34587 100644 --- a/src/platform/assets/composables/useMediaAssetActions.ts +++ b/src/platform/assets/composables/useMediaAssetActions.ts @@ -378,6 +378,8 @@ export function useMediaAssetActions() { filename ) + if (result.cancelled) return + if (!result.success) { const isNoWorkflow = result.error?.includes('No workflow') toast.add({ @@ -565,7 +567,7 @@ export function useMediaAssetActions() { if (result.success) { succeeded++ - } else { + } else if (!result.cancelled) { failed++ } } catch { @@ -573,6 +575,9 @@ export function useMediaAssetActions() { } } + // All cancelled + if (succeeded === 0 && failed === 0) return + if (failed === 0) { toast.add({ severity: 'success', diff --git a/src/platform/workflow/core/services/workflowActionsService.test.ts b/src/platform/workflow/core/services/workflowActionsService.test.ts new file mode 100644 index 00000000000..c2b2e9b6596 --- /dev/null +++ b/src/platform/workflow/core/services/workflowActionsService.test.ts @@ -0,0 +1,92 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' +import { useWorkflowActionsService } from './workflowActionsService' + +const mockPrompt = vi.hoisted(() => vi.fn()) +vi.mock('@/services/dialogService', () => ({ + useDialogService: () => ({ prompt: mockPrompt }) +})) + +const mockGetSetting = vi.hoisted(() => vi.fn()) +vi.mock('@/platform/settings/settingStore', () => ({ + useSettingStore: () => ({ get: mockGetSetting }) +})) + +const mockDownloadBlob = vi.hoisted(() => vi.fn()) +vi.mock('@/scripts/utils', () => ({ + downloadBlob: mockDownloadBlob +})) + +vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({ + useWorkflowStore: () => ({ createTemporary: vi.fn() }) +})) + +vi.mock('@/platform/workflow/core/services/workflowService', () => ({ + useWorkflowService: () => ({ openWorkflow: vi.fn() }) +})) + +const minimalWorkflow: ComfyWorkflowJSON = { + version: 0.4, + last_node_id: 0, + last_link_id: 0, + nodes: [], + links: [] +} + +describe('workflowActionsService.exportWorkflowAction', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns { cancelled: true } when the user dismisses the filename prompt', async () => { + mockGetSetting.mockReturnValue(true) + mockPrompt.mockResolvedValue(null) + const { exportWorkflowAction } = useWorkflowActionsService() + + const result = await exportWorkflowAction(minimalWorkflow, 'wf.json') + + expect(result).toEqual({ success: false, cancelled: true }) + expect(mockDownloadBlob).not.toHaveBeenCalled() + }) + + it('downloads with the prompted filename and returns success', async () => { + mockGetSetting.mockReturnValue(true) + mockPrompt.mockResolvedValue('custom') + const { exportWorkflowAction } = useWorkflowActionsService() + + const result = await exportWorkflowAction(minimalWorkflow, 'wf.json') + + expect(result).toEqual({ success: true }) + expect(mockDownloadBlob).toHaveBeenCalledWith( + 'custom.json', + expect.any(Blob) + ) + }) + + it('skips the prompt and uses the default filename when the setting is off', async () => { + mockGetSetting.mockReturnValue(false) + const { exportWorkflowAction } = useWorkflowActionsService() + + const result = await exportWorkflowAction(minimalWorkflow, 'default.json') + + expect(result).toEqual({ success: true }) + expect(mockPrompt).not.toHaveBeenCalled() + expect(mockDownloadBlob).toHaveBeenCalledWith( + 'default.json', + expect.any(Blob) + ) + }) + + it('returns the no-workflow error when given null', async () => { + const { exportWorkflowAction } = useWorkflowActionsService() + + const result = await exportWorkflowAction(null, 'wf.json') + + expect(result).toEqual({ + success: false, + error: 'No workflow data available' + }) + expect(mockDownloadBlob).not.toHaveBeenCalled() + }) +}) diff --git a/src/platform/workflow/core/services/workflowActionsService.ts b/src/platform/workflow/core/services/workflowActionsService.ts index ffff13fb762..bd7c8d8a6e8 100644 --- a/src/platform/workflow/core/services/workflowActionsService.ts +++ b/src/platform/workflow/core/services/workflowActionsService.ts @@ -42,6 +42,7 @@ export function useWorkflowActionsService() { defaultFilename: string ): Promise<{ success: boolean + cancelled?: boolean error?: string }> => { if (!workflow) { @@ -59,7 +60,7 @@ export function useWorkflowActionsService() { defaultValue: filename }) // User cancelled the prompt - if (!input) return { success: false } + if (!input) return { success: false, cancelled: true } filename = appendJsonExt(input) }