From 95b5207c066de7d73e138ce3b789287707be2c0a Mon Sep 17 00:00:00 2001 From: Dante Date: Wed, 20 May 2026 10:33:47 +0900 Subject: [PATCH 1/9] fix: stabilize multi-output expansion + simplify cloud output fetch (FE-227) (#12006) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Two fixes for the cloud LoadImage form dropdown: 1. **Cloud root-cause fix** — outputs now come from a single `getAssetsByTag('output')` call instead of walking the jobs API and per-job `resolveOutputAssetItems` detail fetches. Per Christian's [Slack feedback](https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778051260476369?thread_ts=1776716352.588229&cid=C0A4XMHANP3): *"on cloud, we can just grab the assets with a single GET, filtering with input or output tag."* Sidebar's job-stack UX is untouched. 2. **Local / defense-in-depth** — even when the watch+expansion path is in play (still used by local), batch all in-flight `resolveOutputAssetItems` for the current `media` snapshot via `Promise.all`, committing once into `resolvedByJobId`. This kills the progressive head-shift symptom even on the legacy path. The first attempt at (1) (`6a1a083c9`, reverted in `c175962e8`) broke select+load on cloud prod because the dropdown wrote `asset.name` (human filename) into the widget value, but cloud's `/api/view` resolves output files by **`asset_hash`** (the blake3-keyed filename). Verified against cloud prod that every output row carries `asset_hash` and that cloud's own `preview_url` is hash-keyed, not name-keyed. Re-introduced in `d7693377` with the dropdown value derived from `asset.asset_hash || asset.name`, with the human filename retained as the display label. - Fixes FE-227 ## Cloud / local divergence — what this PR clarifies | | input | output (this PR) | | ------------ | ---------------------------------------------- | -------------------------------------------------------- | | **cloud** | `getAssetsByTag('input')` (already correct) | **`getAssetsByTag('output')` (new)** | | **local** | `/files/input` (FS-listing) | `getHistory` + per-job expansion (unchanged) | Both directions are now symmetric on cloud: tag-based listing, hash-keyed values. Local stays on the legacy path because core ComfyUI doesn't have the assets/tags model — that's the deeper convergence Jacob/Luke flagged in the FE-556 thread (now BE-757), which is BE/Core work and not this PR. ## Red-Green verification | Commit | CI: Tests Unit | Purpose | |--------|----------------|---------| | `3e8d42e7` test | :red_circle: [failure (25413987208)](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/25413987208) | Asserts the head of the list does not shift while one of two multi-output jobs is still resolving. | | `fe2608d4` fix (atomic batch) | :green_circle: [success (25414246791)](https://github.com/Comfy-Org/ComfyUI_frontend/actions/runs/25414246791) | Resolutions awaited via `Promise.all` and merged in one `resolvedByJobId` update. | | `6a1a083c` simplification (broken) | — | First attempt — used `asset.name`, broke select+load on cloud prod. | | `c175962e` revert | — | Rolled back the broken simplification while diagnosis was in flight. | | `d7693377` simplification (fixed) | pending | Re-introduces `useFlatOutputAssets` and uses `asset.asset_hash` for the dropdown value. Adds 7 unit tests covering hash-as-value, name-fallback, pagination, dedupe, and error path. | ## screenshot/ video ### before https://github.com/user-attachments/assets/239aa447-a260-4713-926c-04dd80a30408 ### after https://github.com/user-attachments/assets/d68228c6-33f5-4bf0-ad24-bb83c876fdc2 ## Test plan - [x] New `useFlatOutputAssets.test.ts` — 7 tests for tag-based fetching, pagination, dedupe, error path. - [x] `useWidgetSelectItems.test.ts` — atomic-batching regression test + new tests asserting hash-as-value and local name-fallback. 35 tests pass. - [x] `WidgetSelectDropdown.test.ts` — 5 tests pass with the new conditional source. - [x] CI red on test-only commit, CI green on first fix commit. - [ ] CI green on the simplification (re-introduce) commit. - [ ] Manual verification on cloud build: open LoadImage → switch to Outputs → scroll → list head stays stable; select an output → LoadImage preview loads (was broken in `6a1a083c`, restored in `d7693377`). --- .../composables/media/useFlatOutputAssets.ts | 27 +++ src/platform/assets/services/assetService.ts | 2 + .../assets/utils/assetMetadataUtils.ts | 10 + .../components/WidgetSelectDropdown.vue | 6 +- .../composables/useWidgetSelectItems.test.ts | 195 ++++++++++++++++++ .../composables/useWidgetSelectItems.ts | 75 ++++--- src/stores/assetsStore.test.ts | 139 ++++++++++++- src/stores/assetsStore.ts | 77 ++++++- 8 files changed, 502 insertions(+), 29 deletions(-) create mode 100644 src/platform/assets/composables/media/useFlatOutputAssets.ts diff --git a/src/platform/assets/composables/media/useFlatOutputAssets.ts b/src/platform/assets/composables/media/useFlatOutputAssets.ts new file mode 100644 index 0000000000..dcf4986ff0 --- /dev/null +++ b/src/platform/assets/composables/media/useFlatOutputAssets.ts @@ -0,0 +1,27 @@ +import { storeToRefs } from 'pinia' + +import { useAssetsStore } from '@/stores/assetsStore' + +import type { IAssetsProvider } from './IAssetsProvider' + +export function useFlatOutputAssets(): IAssetsProvider { + const store = useAssetsStore() + const { + flatOutputAssets, + flatOutputLoading, + flatOutputError, + flatOutputHasMore, + flatOutputIsLoadingMore + } = storeToRefs(store) + + return { + media: flatOutputAssets, + loading: flatOutputLoading, + error: flatOutputError, + fetchMediaList: store.updateFlatOutputs, + refresh: store.updateFlatOutputs, + loadMore: store.loadMoreFlatOutputs, + hasMore: flatOutputHasMore, + isLoadingMore: flatOutputIsLoadingMore + } +} diff --git a/src/platform/assets/services/assetService.ts b/src/platform/assets/services/assetService.ts index 3ac5dc2c71..4c53574777 100644 --- a/src/platform/assets/services/assetService.ts +++ b/src/platform/assets/services/assetService.ts @@ -180,6 +180,8 @@ const DEFAULT_LIMIT = 500 const INPUT_ASSETS_WITH_PUBLIC_LIMIT = 500 export const MODELS_TAG = 'models' +export const INPUT_TAG = 'input' +export const OUTPUT_TAG = 'output' /** Asset tag used by the backend for placeholder records that are not installed. */ export const MISSING_TAG = 'missing' const DEFAULT_EXCLUDED_ASSET_TAGS = [MISSING_TAG] diff --git a/src/platform/assets/utils/assetMetadataUtils.ts b/src/platform/assets/utils/assetMetadataUtils.ts index ff81ef0495..7d669b2053 100644 --- a/src/platform/assets/utils/assetMetadataUtils.ts +++ b/src/platform/assets/utils/assetMetadataUtils.ts @@ -198,3 +198,13 @@ export function getAssetCardTitle(asset: AssetItem): string { if (curatedName && curatedName !== asset.name) return curatedName return getAssetDisplayFilename(asset) } + +/** + * Returns the filename component the cloud `/api/view` endpoint resolves + * for this asset — `asset_hash` when present (cloud assets are hash-keyed + * in storage), otherwise `asset.name`. Use this when constructing widget + * values or media URLs that must round-trip through the view endpoint. + */ +export function getAssetUrlFilename(asset: AssetItem): string { + return asset.asset_hash || asset.name +} diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue index 541ac04a3b..10773c1b8a 100644 --- a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue +++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue @@ -5,6 +5,8 @@ import { useI18n } from 'vue-i18n' import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps' import { SUPPORTED_EXTENSIONS_ACCEPT } from '@/extensions/core/load3d/constants' import { useAssetsApi } from '@/platform/assets/composables/media/useAssetsApi' +import { useFlatOutputAssets } from '@/platform/assets/composables/media/useFlatOutputAssets' +import { isCloud } from '@/platform/distribution/types' import FormDropdown from '@/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.vue' import { AssetKindKey } from '@/renderer/extensions/vueNodes/widgets/components/form/dropdown/types' import type { LayoutMode } from '@/renderer/extensions/vueNodes/widgets/components/form/dropdown/types' @@ -47,7 +49,9 @@ const modelValue = defineModel({ const { t } = useI18n() -const outputMediaAssets = useAssetsApi('output') +const outputMediaAssets = isCloud + ? useFlatOutputAssets() + : useAssetsApi('output') const transformCompatProps = useTransformCompatOverlayProps() diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts index 4c688642dd..d669d6a489 100644 --- a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts +++ b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.test.ts @@ -681,6 +681,201 @@ describe('useWidgetSelectItems', () => { expect(dropdownItems.value[0].name).toBe('preview.png [output]') consoleWarnSpy.mockRestore() }) + + it('does not expand a hash-keyed asset even if its metadata reports outputCount > 1', async () => { + // Defense against future cloud-schema changes: if a flat output row + // ever ships with both asset_hash AND multi-output user_metadata, the + // watcher must NOT replace it with synthesized AssetItems lacking the + // hash, or select+load reverts to the FE-227 broken state. + mockMediaAssets.media.value = [ + { + id: 'asset-flat-1', + name: 'z-image-turbo_00093_.png', + asset_hash: + '039b051670f08941649419dcecea41cb9057f2895388f2e8165ec99df3af0b13.png', + tags: ['output'], + user_metadata: { + jobId: 'job-future', + nodeId: '9', + subfolder: '', + outputCount: 4, + allOutputs: [ + { + filename: 'should-not-replace.png', + subfolder: '', + type: 'output', + nodeId: '9', + mediaType: 'images' + } + ] + } + } + ] + + const { dropdownItems, filterSelected } = useWidgetSelectItems( + createDefaultOptions({ + values: () => [], + modelValue: ref(undefined) + }) + ) + filterSelected.value = 'outputs' + await nextTick() + await nextTick() + + expect(mockResolveOutputAssetItems).not.toHaveBeenCalled() + expect(dropdownItems.value).toHaveLength(1) + expect(dropdownItems.value[0].name).toBe( + '039b051670f08941649419dcecea41cb9057f2895388f2e8165ec99df3af0b13.png [output]' + ) + }) + + it('uses asset_hash (not human filename) as the dropdown value when present, so cloud /view can resolve by hash', async () => { + mockMediaAssets.media.value = [ + { + id: 'asset-out-1', + name: 'z-image-turbo_00093_.png', + asset_hash: + '039b051670f08941649419dcecea41cb9057f2895388f2e8165ec99df3af0b13.png', + preview_url: '/api/view?filename=039b...0b13.png', + tags: ['output'] + } + ] + + const { dropdownItems, filterSelected } = useWidgetSelectItems( + createDefaultOptions({ + values: () => [], + modelValue: ref(undefined) + }) + ) + filterSelected.value = 'outputs' + await nextTick() + + expect(dropdownItems.value).toHaveLength(1) + // The value (item.name) — what becomes modelValue on click — must be the + // hash-keyed path so /api/view resolves it. Cloud's hash is in + // asset_hash, not asset.name (which is the human filename). + expect(dropdownItems.value[0].name).toBe( + '039b051670f08941649419dcecea41cb9057f2895388f2e8165ec99df3af0b13.png [output]' + ) + // The label keeps the human filename for the dropdown UI. + expect(dropdownItems.value[0].label).toContain('z-image-turbo_00093_.png') + }) + + it('falls back to asset.name when asset_hash is absent (local/history path)', async () => { + mockMediaAssets.media.value = [ + { + id: 'local-1', + name: 'ComfyUI_00001_.png', + tags: ['output'] + } + ] + + const { dropdownItems, filterSelected } = useWidgetSelectItems( + createDefaultOptions({ + values: () => [], + modelValue: ref(undefined) + }) + ) + filterSelected.value = 'outputs' + await nextTick() + + expect(dropdownItems.value).toHaveLength(1) + expect(dropdownItems.value[0].name).toBe('ComfyUI_00001_.png [output]') + }) + + it('does not partially expand the list while some multi-output jobs are still resolving (FE-227)', async () => { + mockMediaAssets.media.value = [ + makeMultiOutputAsset('job-FIRST', 'previewFirst.png', '1', 3), + makeMultiOutputAsset('job-SECOND', 'previewSecond.png', '2', 2) + ] + + let resolveFirst!: (items: AssetItem[]) => void + let resolveSecond!: (items: AssetItem[]) => void + const firstPromise = new Promise((res) => { + resolveFirst = res + }) + const secondPromise = new Promise((res) => { + resolveSecond = res + }) + + mockResolveOutputAssetItems.mockImplementation( + async (meta: { jobId: string }) => { + if (meta.jobId === 'job-FIRST') return firstPromise + if (meta.jobId === 'job-SECOND') return secondPromise + return [] + } + ) + + const { dropdownItems, filterSelected } = useWidgetSelectItems( + createDefaultOptions({ + values: () => [], + modelValue: ref(undefined) + }) + ) + filterSelected.value = 'outputs' + await nextTick() + + expect(dropdownItems.value.map((i) => i.name)).toEqual([ + 'previewFirst.png [output]', + 'previewSecond.png [output]' + ]) + + resolveSecond([ + { + id: 'job-SECOND-2--out2a.png', + name: 'out2a.png', + preview_url: '', + tags: ['output'] + }, + { + id: 'job-SECOND-2--out2b.png', + name: 'out2b.png', + preview_url: '', + tags: ['output'] + } + ]) + + await nextTick() + await nextTick() + + expect(dropdownItems.value.map((i) => i.name)).toEqual([ + 'previewFirst.png [output]', + 'previewSecond.png [output]' + ]) + + resolveFirst([ + { + id: 'job-FIRST-1--out1a.png', + name: 'out1a.png', + preview_url: '', + tags: ['output'] + }, + { + id: 'job-FIRST-1--out1b.png', + name: 'out1b.png', + preview_url: '', + tags: ['output'] + }, + { + id: 'job-FIRST-1--out1c.png', + name: 'out1c.png', + preview_url: '', + tags: ['output'] + } + ]) + + await vi.waitFor(() => { + expect(dropdownItems.value).toHaveLength(5) + }) + + expect(dropdownItems.value.map((i) => i.name)).toEqual([ + 'out1a.png [output]', + 'out1b.png [output]', + 'out1c.png [output]', + 'out2a.png [output]', + 'out2b.png [output]' + ]) + }) }) describe('output asset subfolder', () => { diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts index 12c1584d79..b82c5286af 100644 --- a/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts +++ b/src/renderer/extensions/vueNodes/widgets/composables/useWidgetSelectItems.ts @@ -14,7 +14,8 @@ import { getAssetBaseModels, getAssetDisplayFilename, getAssetDisplayName, - getAssetFilename + getAssetFilename, + getAssetUrlFilename } from '@/platform/assets/utils/assetMetadataUtils' import type { FilterOption, @@ -110,7 +111,6 @@ export function useWidgetSelectItems(options: UseWidgetSelectItemsOptions) { }) const resolvedByJobId = shallowRef(new Map()) - const pendingJobIds = new Set() watch( () => outputMediaAssets.media.value, @@ -118,10 +118,22 @@ export function useWidgetSelectItems(options: UseWidgetSelectItemsOptions) { let cancelled = false onCleanup(() => { cancelled = true - pendingJobIds.clear() }) + const seenJobIds = new Set() + const jobsToResolve: Array<{ + jobId: string + meta: ReturnType + createdAt?: string + }> = [] + for (const asset of assets) { + // Hash-keyed assets are leaf rows from the cloud `/assets` API and + // already carry their own URL-resolvable filename. Expanding them via + // resolveOutputAssetItems would synthesize sibling AssetItems without + // an asset_hash and reintroduce the FE-227 hash→name fallback bug. + if (asset.asset_hash) continue + const meta = getOutputAssetMetadata(asset.user_metadata) if (!meta) continue @@ -129,29 +141,41 @@ export function useWidgetSelectItems(options: UseWidgetSelectItemsOptions) { if ( outputCount <= 1 || resolvedByJobId.value.has(meta.jobId) || - pendingJobIds.has(meta.jobId) + seenJobIds.has(meta.jobId) ) continue - pendingJobIds.add(meta.jobId) - void resolveOutputAssetItems(meta, { createdAt: asset.created_at }) - .then((resolved) => { - if (cancelled || !resolved.length) return - const next = new Map(resolvedByJobId.value) - next.set(meta.jobId, resolved) - resolvedByJobId.value = next - }) - .catch((error) => { - console.warn( - 'Failed to resolve multi-output job', - meta.jobId, - error - ) - }) - .finally(() => { - pendingJobIds.delete(meta.jobId) - }) + seenJobIds.add(meta.jobId) + jobsToResolve.push({ + jobId: meta.jobId, + meta, + createdAt: asset.created_at + }) } + + if (jobsToResolve.length === 0) return + + void Promise.all( + jobsToResolve.map(({ jobId, meta, createdAt }) => + resolveOutputAssetItems(meta!, { createdAt }) + .then((resolved) => ({ jobId, resolved })) + .catch((error) => { + console.warn('Failed to resolve multi-output job', jobId, error) + return { jobId, resolved: [] as AssetItem[] } + }) + ) + ).then((results) => { + if (cancelled) return + + const next = new Map(resolvedByJobId.value) + let changed = false + for (const { jobId, resolved } of results) { + if (!resolved.length) continue + next.set(jobId, resolved) + changed = true + } + if (changed) resolvedByJobId.value = next + }) }, { immediate: true } ) @@ -193,13 +217,14 @@ export function useWidgetSelectItems(options: UseWidgetSelectItemsOptions) { if (getMediaTypeFromFilename(asset.name) !== targetMediaType) continue if (seen.has(asset.id)) continue seen.add(asset.id) + const filenameForUrl = getAssetUrlFilename(asset) const subfolder = kind === 'mesh' ? getOutputAssetMetadata(asset.user_metadata)?.subfolder : undefined const pathWithSubfolder = subfolder - ? `${subfolder}/${asset.name}` - : asset.name + ? `${subfolder}/${filenameForUrl}` + : filenameForUrl const annotatedPath = `${pathWithSubfolder} [output]` if (missing.has(annotatedPath)) continue const displayLabel = `${getAssetDisplayFilename(asset)} [output]` @@ -208,7 +233,7 @@ export function useWidgetSelectItems(options: UseWidgetSelectItemsOptions) { preview_url: kind === 'mesh' ? '' - : asset.preview_url || getMediaUrl(asset.name, 'output', kind), + : asset.preview_url || getMediaUrl(filenameForUrl, 'output', kind), name: annotatedPath, label: getDisplayLabel(displayLabel, labelFn) }) diff --git a/src/stores/assetsStore.test.ts b/src/stores/assetsStore.test.ts index f5fb088cdc..af822532f8 100644 --- a/src/stores/assetsStore.test.ts +++ b/src/stores/assetsStore.test.ts @@ -5,6 +5,7 @@ import { nextTick, watch } from 'vue' import { useAssetsStore } from '@/stores/assetsStore' import { api } from '@/scripts/api' +import type { AssetItem } from '@/platform/assets/schemas/assetSchema' import type { JobListItem } from '@/platform/remote/comfyui/jobs/jobTypes' import { assetService } from '@/platform/assets/services/assetService' @@ -30,7 +31,9 @@ vi.mock('@/platform/assets/services/assetService', () => ({ updateAsset: vi.fn(), addAssetTags: vi.fn(), removeAssetTags: vi.fn() - } + }, + INPUT_TAG: 'input', + OUTPUT_TAG: 'output' })) // Mock distribution type - hoisted so it can be changed per test @@ -1502,3 +1505,137 @@ describe('assetsStore - Deletion State and Input Mapping', () => { }) }) }) + +describe('assetsStore - Flat Output Assets (cloud-only)', () => { + const FLAT_OUTPUT_PAGE_SIZE = 200 + + const makeAsset = ( + id: string, + name: string, + asset_hash?: string + ): AssetItem => ({ + id, + name, + asset_hash, + size: 0, + tags: ['output'] + }) + + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + vi.clearAllMocks() + }) + + it('fetches outputs via getAssetsByTag with the output tag and page size', async () => { + vi.mocked(assetService.getAssetsByTag).mockResolvedValueOnce([ + makeAsset('a1', 'image1.png', 'hash1.png'), + makeAsset('a2', 'image2.png', 'hash2.png') + ]) + + const store = useAssetsStore() + await store.updateFlatOutputs() + + expect(assetService.getAssetsByTag).toHaveBeenCalledWith( + 'output', + true, + expect.objectContaining({ limit: FLAT_OUTPUT_PAGE_SIZE, offset: 0 }) + ) + expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['a1', 'a2']) + }) + + it('marks hasMore=false when the page is short', async () => { + vi.mocked(assetService.getAssetsByTag).mockResolvedValueOnce([ + makeAsset('a1', 'one.png') + ]) + + const store = useAssetsStore() + await store.updateFlatOutputs() + + expect(store.flatOutputHasMore).toBe(false) + }) + + it('marks hasMore=true when a full page is returned', async () => { + const fullPage = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) => + makeAsset(`a${i}`, `f${i}.png`) + ) + vi.mocked(assetService.getAssetsByTag).mockResolvedValueOnce(fullPage) + + const store = useAssetsStore() + await store.updateFlatOutputs() + + expect(store.flatOutputHasMore).toBe(true) + }) + + it('appends and dedupes on loadMoreFlatOutputs', async () => { + const firstPage = Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) => + makeAsset(`a${i}`, `f${i}.png`) + ) + const secondPage = [ + makeAsset('a0', 'duplicate.png'), + makeAsset('newId', 'new.png') + ] + vi.mocked(assetService.getAssetsByTag) + .mockResolvedValueOnce(firstPage) + .mockResolvedValueOnce(secondPage) + + const store = useAssetsStore() + await store.updateFlatOutputs() + await store.loadMoreFlatOutputs() + + expect(store.flatOutputAssets).toHaveLength(FLAT_OUTPUT_PAGE_SIZE + 1) + expect(store.flatOutputAssets.at(-1)?.id).toBe('newId') + }) + + it('records error and clears media on initial-fetch failure', async () => { + const err = new Error('network down') + vi.mocked(assetService.getAssetsByTag).mockRejectedValueOnce(err) + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + try { + const store = useAssetsStore() + const result = await store.updateFlatOutputs() + + expect(result).toEqual([]) + expect(store.flatOutputError).toBe(err) + expect(store.flatOutputLoading).toBe(false) + } finally { + consoleSpy.mockRestore() + } + }) + + it('refresh resets pagination', async () => { + vi.mocked(assetService.getAssetsByTag) + .mockResolvedValueOnce( + Array.from({ length: FLAT_OUTPUT_PAGE_SIZE }, (_, i) => + makeAsset(`a${i}`, `f${i}.png`) + ) + ) + .mockResolvedValueOnce([makeAsset('fresh', 'fresh.png')]) + + const store = useAssetsStore() + await store.updateFlatOutputs() + await store.updateFlatOutputs() + + expect(store.flatOutputAssets.map((a) => a.id)).toEqual(['fresh']) + expect(store.flatOutputHasMore).toBe(false) + }) + + it('dedupes concurrent fetches into a single request', async () => { + let resolvePage!: (assets: AssetItem[]) => void + const pagePromise = new Promise((res) => { + resolvePage = res + }) + vi.mocked(assetService.getAssetsByTag).mockReturnValueOnce(pagePromise) + + const store = useAssetsStore() + const p1 = store.updateFlatOutputs() + const p2 = store.updateFlatOutputs() + + expect(vi.mocked(assetService.getAssetsByTag)).toHaveBeenCalledTimes(1) + + resolvePage([makeAsset('shared-1', 'shared.png', 'h.png')]) + await Promise.all([p1, p2]) + + expect(store.flatOutputAssets.map((x) => x.id)).toEqual(['shared-1']) + }) +}) diff --git a/src/stores/assetsStore.ts b/src/stores/assetsStore.ts index e6792c0de0..645df3c8fc 100644 --- a/src/stores/assetsStore.ts +++ b/src/stores/assetsStore.ts @@ -10,7 +10,11 @@ import type { AssetItem, TagsOperationResult } from '@/platform/assets/schemas/assetSchema' -import { assetService } from '@/platform/assets/services/assetService' +import { + INPUT_TAG, + OUTPUT_TAG, + assetService +} from '@/platform/assets/services/assetService' import type { PaginationOptions } from '@/platform/assets/services/assetService' import { isCloud } from '@/platform/distribution/types' import type { JobListItem } from '@/platform/remote/comfyui/jobs/jobTypes' @@ -46,7 +50,7 @@ async function fetchInputFilesFromAPI(): Promise { * Fetch input files from cloud service */ async function fetchInputFilesFromCloud(): Promise { - return await assetService.getAssetsByTag('input', false, { + return await assetService.getAssetsByTag(INPUT_TAG, false, { limit: INPUT_LIMIT }) } @@ -89,6 +93,7 @@ function mapHistoryToAssets(historyItems: JobListItem[]): AssetItem[] { const BATCH_SIZE = 200 const MAX_HISTORY_ITEMS = 1000 // Maximum items to keep in memory +const FLAT_OUTPUT_PAGE_SIZE = 200 export const useAssetsStore = defineStore('assets', () => { const assetDownloadStore = useAssetDownloadStore() @@ -255,6 +260,65 @@ export const useAssetsStore = defineStore('assets', () => { } } + const flatOutputAssets = ref([]) + const flatOutputLoading = ref(false) + const flatOutputError = ref(null) + const flatOutputOffset = ref(0) + const flatOutputHasMore = ref(true) + const flatOutputIsLoadingMore = ref(false) + const flatOutputSeenIds = new Set() + let flatOutputInFlight: Promise | null = null + + async function fetchFlatOutputs(loadMore: boolean): Promise { + if (flatOutputInFlight) return flatOutputInFlight + + if (loadMore) { + if (!flatOutputHasMore.value) return flatOutputAssets.value + flatOutputIsLoadingMore.value = true + } else { + flatOutputLoading.value = true + flatOutputOffset.value = 0 + flatOutputHasMore.value = true + flatOutputSeenIds.clear() + } + flatOutputError.value = null + + flatOutputInFlight = (async () => { + try { + const page = await assetService.getAssetsByTag(OUTPUT_TAG, true, { + limit: FLAT_OUTPUT_PAGE_SIZE, + offset: flatOutputOffset.value + }) + const fresh = loadMore + ? page.filter((asset) => !flatOutputSeenIds.has(asset.id)) + : page + for (const asset of fresh) flatOutputSeenIds.add(asset.id) + flatOutputAssets.value = loadMore + ? [...flatOutputAssets.value, ...fresh] + : page + flatOutputOffset.value += page.length + flatOutputHasMore.value = page.length === FLAT_OUTPUT_PAGE_SIZE + return flatOutputAssets.value + } catch (err) { + flatOutputError.value = err + console.error('Failed to fetch output assets:', err) + return loadMore ? flatOutputAssets.value : [] + } finally { + if (loadMore) flatOutputIsLoadingMore.value = false + else flatOutputLoading.value = false + flatOutputInFlight = null + } + })() + + return flatOutputInFlight + } + + const updateFlatOutputs = () => fetchFlatOutputs(false) + const loadMoreFlatOutputs = async () => { + if (flatOutputIsLoadingMore.value) return + await fetchFlatOutputs(true) + } + /** * Patch preview_id/preview_url for a single asset already in memory, * matched by name. Used after persistThumbnail succeeds so an open Asset @@ -810,6 +874,15 @@ export const useAssetsStore = defineStore('assets', () => { loadMoreHistory, setAssetPreview, + // Flat output assets (cloud-only, tag-based) + flatOutputAssets, + flatOutputLoading, + flatOutputError, + flatOutputHasMore, + flatOutputIsLoadingMore, + updateFlatOutputs, + loadMoreFlatOutputs, + // Input mapping helpers inputAssetsByFilename, getInputName, From 98a8a614e85a23e46e75ed8f49cdf561328d205f Mon Sep 17 00:00:00 2001 From: jaeone94 <89377375+jaeone94@users.noreply.github.com> Date: Wed, 20 May 2026 11:59:44 +0900 Subject: [PATCH 2/9] fix: avoid false missing media errors after importing shared workflow assets (#12333) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Import published media assets for shared workflows before loading the graph so the first missing-media scan sees the user's newly imported references instead of surfacing a false missing asset error. cc FE-773 ## Changes - **What**: Moves the shared workflow import step ahead of `loadGraphData` for the copy-and-open flow, while still allowing the workflow to open with a warning path if asset import fails. - **What**: Clears the shared workflow URL intent consistently on failure paths, including graph load failure after an import attempt, so reloads do not repeatedly replay the same shared workflow side effects. - **What**: Invalidates the input asset cache after published asset import so graph loading and missing-media resolution can observe the refreshed media state. - **What**: Adds a global loading spinner while shared workflow asset import and graph load are in progress, with `role="status"`, `aria-live`, reduced-motion-safe animation, and body teleporting so it stays visible above blocking UI. - **What**: Adds stable TestIds for the shared workflow dialog and updates existing shared workflow E2E selectors away from copy-dependent role text. - **What**: Adds a cloud E2E regression fixture and spec covering the critical flow: shared URL opens the dialog, the user confirms asset import, published media is imported before the public-inclusive input asset scan, the workflow loads, the share query is removed, and missing media UI is not surfaced. - **Breaking**: None. - **Dependencies**: None. ## Root Cause Shared workflow graph loading triggered the missing-media pipeline before the user-selected published media import had completed. Because `include_public=true` does not include published assets, the pre-import scan could classify shared media as missing even when the user was about to import those assets into their own library. ## Review Focus - The ordering in `useSharedWorkflowUrlLoader`: import published assets first, then load the graph, while keeping import failure non-fatal for workflow opening. - The failure cleanup behavior: the shared URL/preserved query intent is now cleared for graph load failures too, avoiding repeated reload-triggered imports. - The spinner behavior in `App.vue`: it uses the existing `workspaceStore.spinner` boolean and intentionally keeps broader ref-counted spinner ownership as follow-up work. - The E2E sentinel in `sharedWorkflowMissingMedia.spec.ts`: it asserts no public-inclusive input asset scan occurs before `/api/assets/import`, then waits for a settling window to ensure the missing-media overlay does not appear. ## Validation - `pnpm format` - `pnpm lint` (passed with existing unrelated warnings only) - `pnpm typecheck` - `pnpm test:unit` - Commit hook: lint-staged formatting/linting, `pnpm typecheck`, `pnpm typecheck:browser` - Push hook: `pnpm knip --cache` (passed with existing tag hint only) ## Follow-Up - Consider a ref-counted or scoped global spinner API so long-running flows do not directly toggle `workspaceStore.spinner`. - Consider separating shared workflow load status into orthogonal result fields instead of encoding partial success in a single string union. - Consider moving published asset import/cache invalidation behind an asset-service-owned API boundary. - Backend follow-up remains needed for `include_public=true` not including published assets; this PR only removes the frontend false positive when the user explicitly imports the shared media. ## Screenshots Before https://github.com/user-attachments/assets/dc790046-237c-4dd8-b773-2507f9a66650 After https://github.com/user-attachments/assets/6517cd38-2c3d-4bfe-a990-35892b7e50ae https://github.com/user-attachments/assets/d89dc3d3-75d9-4251-998b-0c354414e25b ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12333-fix-avoid-false-missing-media-errors-after-importing-shared-workflow-assets-3656d73d365081b38634dcb7625cfc32) by [Unito](https://www.unito.io) --- browser_tests/fixtures/selectors.ts | 10 +- .../fixtures/sharedWorkflowImportFixture.ts | 250 ++++++++++++++++++ .../tests/sharedWorkflowMissingMedia.spec.ts | 147 ++++++++++ browser_tests/tests/templates.spec.ts | 2 +- src/locales/en/main.json | 1 + .../OpenSharedWorkflowDialogContent.test.ts | 20 ++ .../OpenSharedWorkflowDialogContent.vue | 76 +++++- .../useSharedWorkflowUrlLoader.test.ts | 159 ++++++++++- .../composables/useSharedWorkflowUrlLoader.ts | 111 ++++---- .../services/workflowShareService.test.ts | 10 + .../sharing/services/workflowShareService.ts | 3 + src/stores/dialogStore.test.ts | 36 +++ src/stores/dialogStore.ts | 29 ++ 13 files changed, 786 insertions(+), 68 deletions(-) create mode 100644 browser_tests/fixtures/sharedWorkflowImportFixture.ts create mode 100644 browser_tests/tests/sharedWorkflowMissingMedia.spec.ts diff --git a/browser_tests/fixtures/selectors.ts b/browser_tests/fixtures/selectors.ts index ef1f9e09d8..91d98b054d 100644 --- a/browser_tests/fixtures/selectors.ts +++ b/browser_tests/fixtures/selectors.ts @@ -76,7 +76,15 @@ export const TestIds = { publishTabPanel: 'publish-tab-panel', apiSignin: 'api-signin-dialog', updatePassword: 'update-password-dialog', - cloudNotification: 'cloud-notification-dialog' + cloudNotification: 'cloud-notification-dialog', + openSharedWorkflow: 'open-shared-workflow-dialog', + openSharedWorkflowTitle: 'open-shared-workflow-title', + openSharedWorkflowClose: 'open-shared-workflow-close', + openSharedWorkflowErrorClose: 'open-shared-workflow-error-close', + openSharedWorkflowCancel: 'open-shared-workflow-cancel', + openSharedWorkflowOpenWithoutImporting: + 'open-shared-workflow-open-without-importing', + openSharedWorkflowConfirm: 'open-shared-workflow-confirm' }, keybindings: { presetMenu: 'keybinding-preset-menu' diff --git a/browser_tests/fixtures/sharedWorkflowImportFixture.ts b/browser_tests/fixtures/sharedWorkflowImportFixture.ts new file mode 100644 index 0000000000..6b6c2894fc --- /dev/null +++ b/browser_tests/fixtures/sharedWorkflowImportFixture.ts @@ -0,0 +1,250 @@ +import { test as base } from '@playwright/test' +import type { Page } from '@playwright/test' +import type { + Asset, + ImportPublishedAssetsRequest, + ListAssetsResponse +} from '@comfyorg/ingest-types' +import type { z } from 'zod' + +import type { zSharedWorkflowResponse } from '@/platform/workflow/sharing/schemas/shareSchemas' +import type { AssetInfo } from '@/schemas/apiSchema' + +type SharedWorkflowResponse = z.input + +export const sharedWorkflowImportScenario = { + shareId: 'shared-missing-media-e2e', + workflowId: 'shared-missing-media-workflow', + publishedAssetId: 'published-input-asset-1', + inputFileName: 'shared_imported_image.png' +} as const + +export type SharedWorkflowRequestEvent = + | 'import' + | 'input-assets-including-public-before-import' + | 'input-assets-including-public-after-import' + +export interface SharedWorkflowImportMocks { + resetAndStartRecording: () => void + getImportBody: () => ImportPublishedAssetsRequest | undefined + getRequestEvents: () => SharedWorkflowRequestEvent[] + waitForPublicInclusiveInputAssetResponseAfterImport: () => Promise +} + +const defaultInputFileName = '00000000000000000000000Aexample.png' + +const sharedWorkflowAsset: AssetInfo = { + id: sharedWorkflowImportScenario.publishedAssetId, + name: sharedWorkflowImportScenario.inputFileName, + preview_url: '', + storage_url: '', + model: false, + public: false, + in_library: false +} + +const defaultInputAsset: Asset = { + id: 'default-input-asset', + name: defaultInputFileName, + asset_hash: defaultInputFileName, + size: 1_024, + mime_type: 'image/png', + tags: ['input'], + created_at: '2026-05-01T00:00:00Z', + updated_at: '2026-05-01T00:00:00Z', + last_access_time: '2026-05-01T00:00:00Z' +} + +const importedInputAsset: Asset = { + id: 'imported-input-asset', + name: sharedWorkflowImportScenario.inputFileName, + asset_hash: sharedWorkflowImportScenario.inputFileName, + size: 1_024, + mime_type: 'image/png', + tags: ['input'], + created_at: '2026-05-01T00:00:00Z', + updated_at: '2026-05-01T00:00:00Z', + last_access_time: '2026-05-01T00:00:00Z' +} + +const sharedWorkflowResponse: SharedWorkflowResponse = { + share_id: sharedWorkflowImportScenario.shareId, + workflow_id: sharedWorkflowImportScenario.workflowId, + name: 'Shared Missing Media Workflow', + listed: true, + publish_time: '2026-05-01T00:00:00Z', + workflow_json: { + version: 0.4, + last_node_id: 10, + last_link_id: 0, + nodes: [ + { + id: 10, + type: 'LoadImage', + pos: [50, 200], + size: [315, 314], + flags: {}, + order: 0, + mode: 0, + inputs: [], + outputs: [ + { + name: 'IMAGE', + type: 'IMAGE', + links: null + }, + { + name: 'MASK', + type: 'MASK', + links: null + } + ], + properties: { + 'Node name for S&R': 'LoadImage' + }, + widgets_values: [sharedWorkflowImportScenario.inputFileName, 'image'] + } + ], + links: [], + groups: [], + config: {}, + extra: { + ds: { + offset: [0, 0], + scale: 1 + } + } + }, + assets: [sharedWorkflowAsset] +} + +export const sharedWorkflowImportFixture = base.extend<{ + sharedWorkflowImportMocks: SharedWorkflowImportMocks +}>({ + sharedWorkflowImportMocks: async ({ page }, use) => { + const mocks = await mockSharedWorkflowImportFlow(page) + await use(mocks) + } +}) + +async function mockSharedWorkflowImportFlow( + page: Page +): Promise { + let isRecording = false + let importEndpointCalled = false + let importBody: ImportPublishedAssetsRequest | undefined + let resolvePublicInclusiveInputAssetResponseAfterImport: () => void = () => {} + let publicInclusiveInputAssetResponseAfterImport = new Promise( + (resolve) => { + resolvePublicInclusiveInputAssetResponseAfterImport = resolve + } + ) + const requestEvents: SharedWorkflowRequestEvent[] = [] + + function resetPublicInclusiveInputAssetResponseWaiter() { + publicInclusiveInputAssetResponseAfterImport = new Promise( + (resolve) => { + resolvePublicInclusiveInputAssetResponseAfterImport = resolve + } + ) + } + + function recordRequestEvent(event: SharedWorkflowRequestEvent) { + if (isRecording) requestEvents.push(event) + } + + await page.route( + `**/workflows/published/${sharedWorkflowImportScenario.shareId}`, + async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(sharedWorkflowResponse) + }) + } + ) + + await page.route('**/api/assets/import', async (route) => { + recordRequestEvent('import') + importBody = route.request().postDataJSON() as ImportPublishedAssetsRequest + importEndpointCalled = true + + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({}) + }) + }) + + // Excludes `/api/assets/import` so the specific route above + // remains isolated from the general asset listing mock. + await page.route(/\/api\/assets(?=\?|$)/, async (route) => { + const url = new URL(route.request().url()) + const includeTags = getTagParam(url, 'include_tags') + const isInputAssetRequest = includeTags.includes('input') + const includesPublicAssets = + url.searchParams.get('include_public') === 'true' + const isPublicInclusiveInputAssetRequest = + isInputAssetRequest && includesPublicAssets + const isAfterImportPublicInclusiveInputAssetRequest = + isPublicInclusiveInputAssetRequest && importEndpointCalled + + if (isPublicInclusiveInputAssetRequest) { + recordRequestEvent( + importEndpointCalled + ? 'input-assets-including-public-after-import' + : 'input-assets-including-public-before-import' + ) + } + + const allAssets = [ + defaultInputAsset, + ...(importEndpointCalled ? [importedInputAsset] : []) + ] + const assets = includeTags.length + ? allAssets.filter((asset) => + includeTags.every((tag) => asset.tags?.includes(tag)) + ) + : allAssets + + const response: ListAssetsResponse = { + assets, + total: assets.length, + has_more: false + } + + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(response) + }) + + if (isAfterImportPublicInclusiveInputAssetRequest) { + resolvePublicInclusiveInputAssetResponseAfterImport() + } + }) + + return { + resetAndStartRecording: () => { + isRecording = true + importEndpointCalled = false + importBody = undefined + requestEvents.length = 0 + resetPublicInclusiveInputAssetResponseWaiter() + }, + getImportBody: () => importBody, + getRequestEvents: () => [...requestEvents], + waitForPublicInclusiveInputAssetResponseAfterImport: () => + publicInclusiveInputAssetResponseAfterImport + } +} + +function getTagParam(url: URL, key: string): string[] { + return ( + url.searchParams + .get(key) + ?.split(',') + .map((tag) => tag.trim()) + .filter(Boolean) ?? [] + ) +} diff --git a/browser_tests/tests/sharedWorkflowMissingMedia.spec.ts b/browser_tests/tests/sharedWorkflowMissingMedia.spec.ts new file mode 100644 index 0000000000..6a1a14801b --- /dev/null +++ b/browser_tests/tests/sharedWorkflowMissingMedia.spec.ts @@ -0,0 +1,147 @@ +import { expect, mergeTests } from '@playwright/test' + +import { comfyPageFixture } from '@e2e/fixtures/ComfyPage' +import type { ComfyPage } from '@e2e/fixtures/ComfyPage' +import { TestIds } from '@e2e/fixtures/selectors' +import { + sharedWorkflowImportFixture, + sharedWorkflowImportScenario +} from '@e2e/fixtures/sharedWorkflowImportFixture' +import type { SharedWorkflowImportMocks } from '@e2e/fixtures/sharedWorkflowImportFixture' +import { PropertiesPanelHelper } from '@e2e/tests/propertiesPanel/PropertiesPanelHelper' +import type { WorkspaceStore } from '@e2e/types/globals' + +const IMPORT_ORDER_TIMEOUT_MS = 5_000 + +async function expectImportPrecedesPublicInclusiveInputAssetScan( + mocks: SharedWorkflowImportMocks +): Promise { + await expect(async () => { + const events = mocks.getRequestEvents() + const importIndex = events.indexOf('import') + const afterImportIndex = events.indexOf( + 'input-assets-including-public-after-import' + ) + + expect( + events, + 'public-inclusive input assets must not be scanned before import' + ).not.toContain('input-assets-including-public-before-import') + expect(importIndex, `events: ${events.join(',')}`).toBeGreaterThanOrEqual(0) + expect(afterImportIndex, `events: ${events.join(',')}`).toBeGreaterThan( + importIndex + ) + }).toPass({ timeout: IMPORT_ORDER_TIMEOUT_MS }) +} + +async function getCachedMissingMediaWarningNames( + comfyPage: ComfyPage +): Promise { + return await comfyPage.page.evaluate(() => { + const workflow = (window.app!.extensionManager as WorkspaceStore).workflow + .activeWorkflow + if (!workflow) return null + + return ( + workflow.pendingWarnings?.missingMediaCandidates?.map( + (candidate) => candidate.name + ) ?? [] + ) + }) +} + +async function expectNoMissingMediaAfterPublicInclusiveAssetScan( + comfyPage: ComfyPage, + mocks: SharedWorkflowImportMocks +): Promise { + await mocks.waitForPublicInclusiveInputAssetResponseAfterImport() + await comfyPage.nextFrame() + + await expect( + comfyPage.page.getByTestId(TestIds.dialogs.errorOverlay) + ).toBeHidden() + await expect + .poll(() => getCachedMissingMediaWarningNames(comfyPage)) + .toEqual([]) +} + +async function openPanelAndExpectNoMissingMedia( + comfyPage: ComfyPage +): Promise { + const page = comfyPage.page + const errorOverlay = page.getByTestId(TestIds.dialogs.errorOverlay) + await expect(errorOverlay).toBeHidden() + + const panel = new PropertiesPanelHelper(page) + await panel.open(comfyPage.actionbar.propertiesButton) + await expect( + panel.root.getByTestId(TestIds.propertiesPanel.errorsTab) + ).toBeHidden() + await expect(page.getByTestId(TestIds.dialogs.missingMediaGroup)).toHaveCount( + 0 + ) +} + +const test = mergeTests(comfyPageFixture, sharedWorkflowImportFixture) + +test.describe('Shared workflow missing media', { tag: '@cloud' }, () => { + // Missing media only surfaces the overlay when the Errors tab is enabled + // (src/stores/executionErrorStore.ts). + test.use({ + initialSettings: { + 'Comfy.RightSidePanel.ShowErrorsTab': true + } + }) + + test.beforeEach(async ({ comfyPage, sharedWorkflowImportMocks }) => { + sharedWorkflowImportMocks.resetAndStartRecording() + await comfyPage.setup({ + clearStorage: false, + url: `/?share=${sharedWorkflowImportScenario.shareId}` + }) + }) + + test('imports shared media before loading workflow so missing media is not surfaced', async ({ + comfyPage, + sharedWorkflowImportMocks + }) => { + const { page } = comfyPage + + const dialog = page.getByTestId(TestIds.dialogs.openSharedWorkflow) + await expect( + dialog.getByTestId(TestIds.dialogs.openSharedWorkflowTitle) + ).toBeVisible() + + await dialog.getByTestId(TestIds.dialogs.openSharedWorkflowConfirm).click() + + await expect + .poll(() => + page.evaluate(() => + window.app!.graph.nodes.map((node) => ({ + type: node.type, + value: node.widgets?.[0]?.value + })) + ) + ) + .toEqual([ + { + type: 'LoadImage', + value: sharedWorkflowImportScenario.inputFileName + } + ]) + await expectImportPrecedesPublicInclusiveInputAssetScan( + sharedWorkflowImportMocks + ) + await expectNoMissingMediaAfterPublicInclusiveAssetScan( + comfyPage, + sharedWorkflowImportMocks + ) + + expect(sharedWorkflowImportMocks.getImportBody()).toEqual({ + published_asset_ids: [sharedWorkflowImportScenario.publishedAssetId], + share_id: sharedWorkflowImportScenario.shareId + }) + expect(new URL(page.url()).searchParams.has('share')).toBe(false) + await openPanelAndExpectNoMissingMedia(comfyPage) + }) +}) diff --git a/browser_tests/tests/templates.spec.ts b/browser_tests/tests/templates.spec.ts index 06b53a9bbc..fecd1dd78c 100644 --- a/browser_tests/tests/templates.spec.ts +++ b/browser_tests/tests/templates.spec.ts @@ -143,7 +143,7 @@ test.describe('Templates', { tag: ['@slow', '@workflow'] }, () => { }) await expect( - comfyPage.page.getByRole('heading', { name: 'Open shared workflow' }) + comfyPage.page.getByTestId(TestIds.dialogs.openSharedWorkflowTitle) ).toBeVisible() await expect(comfyPage.templates.content).toBeHidden() diff --git a/src/locales/en/main.json b/src/locales/en/main.json index 3d4cc8dd96..22a88af40f 100644 --- a/src/locales/en/main.json +++ b/src/locales/en/main.json @@ -3225,6 +3225,7 @@ "copyAssetsAndOpen": "Import assets & open workflow", "openWorkflow": "Open workflow", "openWithoutImporting": "Open without importing", + "opening": "Opening shared workflow...", "importFailed": "Failed to import workflow assets", "loadError": "Could not load this shared workflow. Please try again later." }, diff --git a/src/platform/workflow/sharing/components/OpenSharedWorkflowDialogContent.test.ts b/src/platform/workflow/sharing/components/OpenSharedWorkflowDialogContent.test.ts index a6320fe13c..1fd7e8f9c5 100644 --- a/src/platform/workflow/sharing/components/OpenSharedWorkflowDialogContent.test.ts +++ b/src/platform/workflow/sharing/components/OpenSharedWorkflowDialogContent.test.ts @@ -34,6 +34,7 @@ const i18n = createI18n({ copyAssetsAndOpen: 'Copy assets & open workflow', openWorkflow: 'Open workflow', openWithoutImporting: 'Open without importing', + opening: 'Opening shared workflow...', loadError: 'Could not load this shared workflow. Please try again later.' }, @@ -292,6 +293,25 @@ describe('OpenSharedWorkflowDialogContent', () => { expect(onConfirm).toHaveBeenCalledWith(assetsPayload) }) + it('shows opening status and disables actions while opening', async () => { + mockGetSharedWorkflow.mockResolvedValue(assetsPayload) + const { container } = renderComponent({ openingAction: 'copy-and-open' }) + await flushPromises() + + expect(screen.getByRole('status').textContent).toContain( + 'Opening shared workflow...' + ) + expect(container.textContent).not.toContain( + 'Opening the workflow will create a new copy in your workspace' + ) + expect(screen.getByTestId('open-shared-workflow-close')).toBeEnabled() + expect(screen.getByTestId('open-shared-workflow-cancel')).toBeDisabled() + expect( + screen.getByTestId('open-shared-workflow-open-without-importing') + ).toBeDisabled() + expect(screen.getByTestId('open-shared-workflow-confirm')).toBeDisabled() + }) + it('filters out assets already in library', async () => { const mixedPayload = makePayload({ assets: [ diff --git a/src/platform/workflow/sharing/components/OpenSharedWorkflowDialogContent.vue b/src/platform/workflow/sharing/components/OpenSharedWorkflowDialogContent.vue index 0d6750a1b6..eb3130c8de 100644 --- a/src/platform/workflow/sharing/components/OpenSharedWorkflowDialogContent.vue +++ b/src/platform/workflow/sharing/components/OpenSharedWorkflowDialogContent.vue @@ -1,12 +1,24 @@