mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-13 09:00:16 +00:00
fix: omit job_asset_name_filters when all job outputs selected (#9684)
## Summary - When bulk exporting, `job_asset_name_filters` was always sent for every job, restricting each job to only the assets the user clicked on. For multi-output jobs, this meant the ZIP only contained 1 asset per job instead of all outputs. - Now compares selected asset count per job against `outputCount` metadata and omits the filter for fully-selected jobs, so the backend returns all assets. ## Test plan - [x] Unit tests: all outputs selected → no filter sent - [x] Unit tests: subset selected → filter sent - [x] Unit tests: mixed selection → filter only for partial jobs - [x] Unit tests: multiple fully-selected jobs → no filters - [x] Typecheck, lint pass - [ ] Manual: bulk export multi-output jobs in cloud env, verify ZIP contains all outputs 🤖 Generated with [Claude Code](https://claude.com/claude-code) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9684-fix-omit-job_asset_name_filters-when-all-job-outputs-selected-31f6d73d3650814482f8c59d05027d79) by [Unito](https://www.unito.io) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Alexander Brown <drjkl@comfy.org>
This commit is contained in:
@@ -119,17 +119,31 @@ vi.mock('@/platform/assets/utils/assetTypeUtil', () => ({
|
||||
getAssetType: mockGetAssetType
|
||||
}))
|
||||
|
||||
const mockGetOutputAssetMetadata = vi.hoisted(() =>
|
||||
vi.fn().mockReturnValue(null)
|
||||
)
|
||||
vi.mock('../schemas/assetMetadataSchema', () => ({
|
||||
getOutputAssetMetadata: vi.fn().mockReturnValue(null)
|
||||
getOutputAssetMetadata: mockGetOutputAssetMetadata
|
||||
}))
|
||||
|
||||
const mockDeleteAsset = vi.hoisted(() => vi.fn())
|
||||
const mockCreateAssetExport = vi.hoisted(() =>
|
||||
vi.fn().mockResolvedValue({ task_id: 'test-task-id', status: 'pending' })
|
||||
)
|
||||
vi.mock('../services/assetService', () => ({
|
||||
assetService: {
|
||||
deleteAsset: mockDeleteAsset
|
||||
deleteAsset: mockDeleteAsset,
|
||||
createAssetExport: mockCreateAssetExport
|
||||
}
|
||||
}))
|
||||
|
||||
const mockTrackExport = vi.hoisted(() => vi.fn())
|
||||
vi.mock('@/stores/assetExportStore', () => ({
|
||||
useAssetExportStore: () => ({
|
||||
trackExport: mockTrackExport
|
||||
})
|
||||
}))
|
||||
|
||||
vi.mock('@/scripts/api', () => ({
|
||||
api: {
|
||||
deleteItem: vi.fn(),
|
||||
@@ -259,6 +273,106 @@ describe('useMediaAssetActions', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('downloadMultipleAssets - job_asset_name_filters', () => {
|
||||
beforeEach(() => {
|
||||
mockIsCloud.value = true
|
||||
mockCreateAssetExport.mockClear()
|
||||
mockTrackExport.mockClear()
|
||||
mockGetAssetType.mockReturnValue('output')
|
||||
mockGetOutputAssetMetadata.mockImplementation(
|
||||
(meta: Record<string, unknown> | undefined) =>
|
||||
meta && 'jobId' in meta ? meta : null
|
||||
)
|
||||
})
|
||||
|
||||
function createOutputAsset(
|
||||
id: string,
|
||||
name: string,
|
||||
jobId: string,
|
||||
outputCount?: number
|
||||
): AssetItem {
|
||||
return createMockAsset({
|
||||
id,
|
||||
name,
|
||||
tags: ['output'],
|
||||
user_metadata: { jobId, nodeId: '1', subfolder: '', outputCount }
|
||||
})
|
||||
}
|
||||
|
||||
it('should omit name filters for job-level selections (outputCount known)', async () => {
|
||||
const assets = [
|
||||
createOutputAsset('a1', 'img1.png', 'job1', 3),
|
||||
createOutputAsset('a2', 'img2.png', 'job1', 3),
|
||||
createOutputAsset('a3', 'img3.png', 'job1', 3)
|
||||
]
|
||||
|
||||
const actions = useMediaAssetActions()
|
||||
actions.downloadMultipleAssets(assets)
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockCreateAssetExport).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
const payload = mockCreateAssetExport.mock.calls[0][0]
|
||||
expect(payload.job_ids).toEqual(['job1'])
|
||||
expect(payload.job_asset_name_filters).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should omit name filters for multiple job-level selections', async () => {
|
||||
const j1a = createOutputAsset('a1', 'out1a.png', 'job1', 2)
|
||||
const j1b = createOutputAsset('a2', 'out1b.png', 'job1', 2)
|
||||
const j2 = createOutputAsset('a3', 'out2.png', 'job2', 1)
|
||||
|
||||
const actions = useMediaAssetActions()
|
||||
actions.downloadMultipleAssets([j1a, j1b, j2])
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockCreateAssetExport).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
const payload = mockCreateAssetExport.mock.calls[0][0]
|
||||
expect(payload.job_ids).toEqual(['job1', 'job2'])
|
||||
expect(payload.job_asset_name_filters).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should include name filters when outputCount is unknown', async () => {
|
||||
const asset1 = createOutputAsset('a1', 'img1.png', 'job1')
|
||||
const asset2 = createOutputAsset('a2', 'img2.png', 'job2')
|
||||
|
||||
const actions = useMediaAssetActions()
|
||||
actions.downloadMultipleAssets([asset1, asset2])
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockCreateAssetExport).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
const payload = mockCreateAssetExport.mock.calls[0][0]
|
||||
expect(payload.job_asset_name_filters).toEqual({
|
||||
job1: ['img1.png'],
|
||||
job2: ['img2.png']
|
||||
})
|
||||
})
|
||||
|
||||
it('should mix: omit filters for known outputCount, keep for unknown', async () => {
|
||||
const j1a = createOutputAsset('a1', 'img1a.png', 'job1', 2)
|
||||
const j1b = createOutputAsset('a2', 'img1b.png', 'job1', 2)
|
||||
const j2 = createOutputAsset('a3', 'img2.png', 'job2')
|
||||
|
||||
const actions = useMediaAssetActions()
|
||||
actions.downloadMultipleAssets([j1a, j1b, j2])
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockCreateAssetExport).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
const payload = mockCreateAssetExport.mock.calls[0][0]
|
||||
expect(payload.job_ids).toEqual(['job1', 'job2'])
|
||||
expect(payload.job_asset_name_filters).toEqual({
|
||||
job2: ['img2.png']
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('deleteAssets - model cache invalidation', () => {
|
||||
beforeEach(() => {
|
||||
mockIsCloud.value = true
|
||||
|
||||
@@ -146,7 +146,10 @@ export function useMediaAssetActions() {
|
||||
if (!jobIds.includes(jobId)) {
|
||||
jobIds.push(jobId)
|
||||
}
|
||||
if (metadata?.jobId && asset.name) {
|
||||
// Only add name filters when outputCount is unknown.
|
||||
// When outputCount is set, the asset is a job-level selection
|
||||
// from the gallery and the user wants all outputs for that job.
|
||||
if (metadata?.jobId && asset.name && metadata.outputCount == null) {
|
||||
if (!jobAssetNameFilters[metadata.jobId]) {
|
||||
jobAssetNameFilters[metadata.jobId] = []
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user