mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-22 05:19:03 +00:00
## 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)
251 lines
7.0 KiB
TypeScript
251 lines
7.0 KiB
TypeScript
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<typeof zSharedWorkflowResponse>
|
|
|
|
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<void>
|
|
}
|
|
|
|
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<SharedWorkflowImportMocks> {
|
|
let isRecording = false
|
|
let importEndpointCalled = false
|
|
let importBody: ImportPublishedAssetsRequest | undefined
|
|
let resolvePublicInclusiveInputAssetResponseAfterImport: () => void = () => {}
|
|
let publicInclusiveInputAssetResponseAfterImport = new Promise<void>(
|
|
(resolve) => {
|
|
resolvePublicInclusiveInputAssetResponseAfterImport = resolve
|
|
}
|
|
)
|
|
const requestEvents: SharedWorkflowRequestEvent[] = []
|
|
|
|
function resetPublicInclusiveInputAssetResponseWaiter() {
|
|
publicInclusiveInputAssetResponseAfterImport = new Promise<void>(
|
|
(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) ?? []
|
|
)
|
|
}
|