mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-22 21:38:52 +00:00
fix: include share_id when importing published assets (FE-603) (#12055)
*PR Created by the Glary-Bot Agent* https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778150003248989 FE-603 --- ## Summary Send `share_id` alongside `published_asset_ids` from `workflowShareService.importPublishedAssets`, and type / zod-validate the body against `ImportPublishedAssetsRequest` + `zImportPublishedAssetsRequest` from `@comfyorg/ingest-types`. This is a **parameter-schema change**, not a live bug fix. The original `BAD_REQUEST: share_id is required` failure is **no longer reproducible in production** — the backend rolled back the required-field enforcement in [BE-855](https://linear.app/comfyorg/issue/BE-855) (cloud [#3587](https://github.com/Comfy-Org/cloud/pull/3587) / [#3588](https://github.com/Comfy-Org/cloud/pull/3588)). Today the import succeeds without `share_id`. - Closes FE-603 ## Why we still ship this Frontend goes first so the backend can later re-tighten the contract without breaking shared-workflow imports a second time. Agreed sequence in [Slack](https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778535349236419) — BE-6 was split into two sub-issues to make the chain explicit: 1. **[BE-898](https://linear.app/comfyorg/issue/BE-898)** (blocks FE-603) — BE makes `share_id` **optional**, validating when present. [cloud PR #3633](https://github.com/Comfy-Org/cloud/pull/3633), In Review. 2. **FE-603** (this PR) — FE sends `share_id`. 3. **[BE-899](https://linear.app/comfyorg/issue/BE-899)** (blocked by FE-603) — BE flips `share_id` back to **required** after FE-603 deploys. Steps 1 and 2 ship in order: BE-898 → FE-603 → BE-899. ## Changes - `workflowShareService.importPublishedAssets` now takes `shareId: string` and types the request body with `ImportPublishedAssetsRequest`. The body is parsed through `zImportPublishedAssetsRequest` before the network call so future contract drift surfaces as a typecheck or zod parse failure rather than a silent runtime 400. - `useSharedWorkflowUrlLoader.ts` threads `payload.shareId` (already in scope from `SharedWorkflowPayload`) into the import call. - Unit tests assert `share_id` is sent and that an empty `share_id` is rejected before fetch. ## Verification - `pnpm typecheck` — clean - `pnpm lint` — 0 errors / 0 warnings on changed files (3 pre-existing warnings in unrelated files) - `pnpm format` — applied - `pnpm vitest run` on both affected files — 34/34 passing ### Live in-browser smoke test (Vite dev server) Loaded the built module in the browser, intercepted `fetch`, and exercised the new code path. The new `importPublishedAssets` produces the correct wire format and zod rejects bad input before fetch: ```json // Happy path — sent to /api/assets/import (POST) { "published_asset_ids": ["pa-1", "pa-2", "pa-3"], "share_id": "share-abc" } ``` ```text // Empty share_id — zod rejects, fetch is never called [ { code: "too_small", minimum: 1, path: ["share_id"], message: "String must contain at least 1 character(s)" } ] fetchCallsBetweenAttempts: 0 ``` ## Why typecheck didn't catch the original miss `api.fetchApi(route: string, options?: RequestInit)` accepts the standard DOM `RequestInit`, so `body` is just `BodyInit | null`. Once the call site does `JSON.stringify({ ... })`, the inline object is erased into a string and TS has no schema to enforce. There was no `@ts-ignore` or `as any` — the generated types were simply never imported. This PR plugs that one call site; the same pattern should be applied wherever the frontend hits ingest endpoints (the broader hey-zod migration gap mentioned in #bug-dump). Reported in #bug-dump. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12055-fix-include-share_id-when-importing-published-assets-3596d73d365081c0a1c7e69102f5cfcc) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org>
This commit is contained in:
@@ -244,7 +244,7 @@ describe('useSharedWorkflowUrlLoader', () => {
|
||||
const { loadSharedWorkflowFromUrl } = useSharedWorkflowUrlLoader()
|
||||
await loadSharedWorkflowFromUrl()
|
||||
|
||||
expect(mockImportPublishedAssets).toHaveBeenCalledWith(['a1'])
|
||||
expect(mockImportPublishedAssets).toHaveBeenCalledWith(['a1'], 'share-id-1')
|
||||
})
|
||||
|
||||
it('does not call import when user chooses open-only', async () => {
|
||||
@@ -348,7 +348,7 @@ describe('useSharedWorkflowUrlLoader', () => {
|
||||
const { loadSharedWorkflowFromUrl } = useSharedWorkflowUrlLoader()
|
||||
await loadSharedWorkflowFromUrl()
|
||||
|
||||
expect(mockImportPublishedAssets).toHaveBeenCalledWith(['a1'])
|
||||
expect(mockImportPublishedAssets).toHaveBeenCalledWith(['a1'], 'share-id-1')
|
||||
})
|
||||
|
||||
it('restores preserved share query before loading', async () => {
|
||||
|
||||
@@ -161,7 +161,8 @@ export function useSharedWorkflowUrlLoader() {
|
||||
if (result.action === 'copy-and-open' && nonOwnedAssets.length > 0) {
|
||||
try {
|
||||
await workflowShareService.importPublishedAssets(
|
||||
nonOwnedAssets.map((a) => a.id)
|
||||
nonOwnedAssets.map((a) => a.id),
|
||||
payload.shareId
|
||||
)
|
||||
} catch (importError) {
|
||||
console.error(
|
||||
|
||||
@@ -334,16 +334,45 @@ describe(useWorkflowShareService, () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('imports published assets via POST /assets/import', async () => {
|
||||
it('imports published assets via POST /assets/import with share_id', async () => {
|
||||
mockFetchApi.mockResolvedValue(mockJsonResponse({}, true, 200))
|
||||
|
||||
const service = useWorkflowShareService()
|
||||
await service.importPublishedAssets(['pa-1', 'pa-2'])
|
||||
await service.importPublishedAssets(['pa-1', 'pa-2'], 'share-id-1')
|
||||
|
||||
expect(mockFetchApi).toHaveBeenCalledWith('/assets/import', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ published_asset_ids: ['pa-1', 'pa-2'] })
|
||||
body: JSON.stringify({
|
||||
published_asset_ids: ['pa-1', 'pa-2'],
|
||||
share_id: 'share-id-1'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
it('omits share_id from the payload when not provided', async () => {
|
||||
mockFetchApi.mockResolvedValue(mockJsonResponse({}, true, 200))
|
||||
|
||||
const service = useWorkflowShareService()
|
||||
await service.importPublishedAssets(['pa-1'])
|
||||
|
||||
expect(mockFetchApi).toHaveBeenCalledWith('/assets/import', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ published_asset_ids: ['pa-1'] })
|
||||
})
|
||||
})
|
||||
|
||||
it('omits share_id from the payload when shareId is an empty string', async () => {
|
||||
mockFetchApi.mockResolvedValue(mockJsonResponse({}, true, 200))
|
||||
|
||||
const service = useWorkflowShareService()
|
||||
await service.importPublishedAssets(['pa-1'], '')
|
||||
|
||||
expect(mockFetchApi).toHaveBeenCalledWith('/assets/import', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ published_asset_ids: ['pa-1'] })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -352,9 +381,9 @@ describe(useWorkflowShareService, () => {
|
||||
|
||||
const service = useWorkflowShareService()
|
||||
|
||||
await expect(service.importPublishedAssets(['bad-id'])).rejects.toThrow(
|
||||
'Failed to import assets: 400'
|
||||
)
|
||||
await expect(
|
||||
service.importPublishedAssets(['bad-id'], 'share-id-1')
|
||||
).rejects.toThrow('Failed to import assets: 400')
|
||||
})
|
||||
|
||||
it('throws when shared workflow payload is invalid', async () => {
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import type { ImportPublishedAssetsRequest } from '@comfyorg/ingest-types'
|
||||
|
||||
import type {
|
||||
PublishPrefill,
|
||||
SharedWorkflowPayload,
|
||||
@@ -255,11 +257,19 @@ export function useWorkflowShareService() {
|
||||
return workflow
|
||||
}
|
||||
|
||||
async function importPublishedAssets(assetIds: string[]): Promise<void> {
|
||||
async function importPublishedAssets(
|
||||
assetIds: string[],
|
||||
shareId?: string
|
||||
): Promise<void> {
|
||||
const body: ImportPublishedAssetsRequest = {
|
||||
published_asset_ids: assetIds,
|
||||
...(shareId ? { share_id: shareId } : {})
|
||||
}
|
||||
|
||||
const response = await api.fetchApi('/assets/import', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ published_asset_ids: assetIds })
|
||||
body: JSON.stringify(body)
|
||||
})
|
||||
|
||||
if (!response.ok) {
|
||||
|
||||
Reference in New Issue
Block a user