From 7feaefd39cd5b64fac09fe7f8877ec5db3217eb9 Mon Sep 17 00:00:00 2001 From: Benjamin Lu Date: Fri, 20 Feb 2026 01:40:16 -0800 Subject: [PATCH] fix: disable job asset actions when preview output is missing (#8700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Motivation - Prevent context-menu actions from operating on stale or non-existent assets when a completed job has no `previewOutput`, since `Inspect asset`, `Add to current workflow`, and `Download` should be inactive in that case. - Also ensure the `Inspect asset` entry is disabled when the optional inspect callback is not provided to avoid unexpected behavior. ### Description - Added an optional `disabled?: boolean` field to the `MenuEntry` type returned by `useJobMenu` and computed `hasPreviewAsset` to detect when `taskRef.previewOutput` is present. - Mark `inspect-asset`, `add-to-current`, and `download` entries as `disabled` when the preview is missing (and also when the inspect callback is missing for `inspect-asset`), and keep `delete` omitted when no preview exists. - Updated `JobContextMenu.vue` to pass `:disabled="entry.disabled"` to the rendered `Button` and to short-circuit the action emit when an entry is disabled. - Expanded `useJobMenu` unit tests to assert enabled states when a preview exists and disabled states when preview or inspect handler is missing. ### Testing - Ran `pnpm vitest src/composables/queue/useJobMenu.test.ts` and all tests passed (36 tests). - Ran `pnpm lint` and the linter reported no warnings or errors. - Ran `pnpm typecheck` (`vue-tsc --noEmit`) and type checking completed without errors. ------ [Codex Task](https://chatgpt.com/codex/tasks/task_e_69864ed56e408330b88c3e9def1b5fb5) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8700-fix-disable-job-asset-actions-when-preview-output-is-missing-2ff6d73d365081b8b72ccadf0ae43e9d) by [Unito](https://www.unito.io) --- .../queue/job/JobContextMenu.test.ts | 81 +++++++++++++++++++ src/components/queue/job/JobContextMenu.vue | 2 + src/composables/queue/useJobMenu.test.ts | 21 +++++ src/composables/queue/useJobMenu.ts | 8 +- 4 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 src/components/queue/job/JobContextMenu.test.ts diff --git a/src/components/queue/job/JobContextMenu.test.ts b/src/components/queue/job/JobContextMenu.test.ts new file mode 100644 index 0000000000..57c248e70d --- /dev/null +++ b/src/components/queue/job/JobContextMenu.test.ts @@ -0,0 +1,81 @@ +import { mount } from '@vue/test-utils' +import { describe, expect, it, vi } from 'vitest' + +import JobContextMenu from '@/components/queue/job/JobContextMenu.vue' +import type { MenuEntry } from '@/composables/queue/useJobMenu' + +const buttonStub = { + props: { + disabled: { + type: Boolean, + default: false + } + }, + template: ` +
+ +
+ ` +} + +const createEntries = (): MenuEntry[] => [ + { key: 'enabled', label: 'Enabled action', onClick: vi.fn() }, + { + key: 'disabled', + label: 'Disabled action', + disabled: true, + onClick: vi.fn() + }, + { kind: 'divider', key: 'divider-1' } +] + +const mountComponent = (entries: MenuEntry[]) => + mount(JobContextMenu, { + props: { entries }, + global: { + stubs: { + Popover: { + template: '
' + }, + Button: buttonStub + } + } + }) + +describe('JobContextMenu', () => { + it('passes disabled state to action buttons', () => { + const wrapper = mountComponent(createEntries()) + + const buttons = wrapper.findAll('.button-stub') + expect(buttons).toHaveLength(2) + expect(buttons[0].attributes('data-disabled')).toBe('false') + expect(buttons[1].attributes('data-disabled')).toBe('true') + }) + + it('emits action for enabled entries', async () => { + const entries = createEntries() + const wrapper = mountComponent(entries) + + await wrapper.findAll('.button-stub')[0].trigger('click') + + expect(wrapper.emitted('action')).toEqual([[entries[0]]]) + }) + + it('does not emit action for disabled entries', async () => { + const wrapper = mountComponent([ + { + key: 'disabled', + label: 'Disabled action', + disabled: true, + onClick: vi.fn() + } + ]) + + await wrapper.get('.button-stub').trigger('click') + + expect(wrapper.emitted('action')).toBeUndefined() + }) +}) diff --git a/src/components/queue/job/JobContextMenu.vue b/src/components/queue/job/JobContextMenu.vue index 5ed96ad9ec..c8c4e1bddf 100644 --- a/src/components/queue/job/JobContextMenu.vue +++ b/src/components/queue/job/JobContextMenu.vue @@ -26,6 +26,7 @@ variant="textonly" size="sm" :aria-label="entry.label" + :disabled="entry.disabled" @click="onEntry(entry)" > { 'd3', 'delete' ]) + + expect( + findActionEntry(jobMenuEntries.value, 'inspect-asset')?.disabled + ).toBe(false) + expect( + findActionEntry(jobMenuEntries.value, 'add-to-current')?.disabled + ).toBe(false) + expect(findActionEntry(jobMenuEntries.value, 'download')?.disabled).toBe( + false + ) + const inspectEntry = findActionEntry(jobMenuEntries.value, 'inspect-asset') await inspectEntry?.onClick?.() expect(inspectSpy).toHaveBeenCalledWith(currentItem.value) @@ -760,6 +771,7 @@ describe('useJobMenu', () => { await nextTick() const inspectEntry = findActionEntry(jobMenuEntries.value, 'inspect-asset') expect(inspectEntry?.onClick).toBeUndefined() + expect(inspectEntry?.disabled).toBe(true) }) it('omits delete asset entry when no preview exists', async () => { @@ -767,6 +779,15 @@ describe('useJobMenu', () => { setCurrentItem(createJobItem({ state: 'completed', taskRef: {} })) await nextTick() + expect( + findActionEntry(jobMenuEntries.value, 'inspect-asset')?.disabled + ).toBe(true) + expect( + findActionEntry(jobMenuEntries.value, 'add-to-current')?.disabled + ).toBe(true) + expect(findActionEntry(jobMenuEntries.value, 'download')?.disabled).toBe( + true + ) expect(jobMenuEntries.value.some((entry) => entry.key === 'delete')).toBe( false ) diff --git a/src/composables/queue/useJobMenu.ts b/src/composables/queue/useJobMenu.ts index 0dd9908538..7c573657ea 100644 --- a/src/composables/queue/useJobMenu.ts +++ b/src/composables/queue/useJobMenu.ts @@ -29,6 +29,7 @@ export type MenuEntry = key: string label: string icon?: string + disabled?: boolean onClick?: () => void | Promise } | { kind: 'divider'; key: string } @@ -239,13 +240,14 @@ export function useJobMenu( const item = currentMenuItem() const state = item?.state if (!state) return [] - const hasDeletableAsset = !!item?.taskRef?.previewOutput + const hasPreviewAsset = !!item?.taskRef?.previewOutput if (state === 'completed') { return [ { key: 'inspect-asset', label: st('queue.jobMenu.inspectAsset', 'Inspect asset'), icon: 'icon-[lucide--zoom-in]', + disabled: !hasPreviewAsset || !onInspectAsset, onClick: onInspectAsset ? () => { const item = currentMenuItem() @@ -260,12 +262,14 @@ export function useJobMenu( 'Add to current workflow' ), icon: 'icon-[comfy--node]', + disabled: !hasPreviewAsset, onClick: addOutputLoaderNode }, { key: 'download', label: st('queue.jobMenu.download', 'Download'), icon: 'icon-[lucide--download]', + disabled: !hasPreviewAsset, onClick: downloadPreviewAsset }, { kind: 'divider', key: 'd1' }, @@ -289,7 +293,7 @@ export function useJobMenu( onClick: copyJobId }, { kind: 'divider', key: 'd3' }, - ...(hasDeletableAsset + ...(hasPreviewAsset ? [ { key: 'delete',