mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-19 06:20:10 +00:00
fix: exclude missing assets from cloud mode dropdown (COM-14333) (#8747)
## Summary Fixes a bug where non-existent images appeared in the asset search dropdown when loading workflows that reference images the user doesn't have in cloud mode. ## Changes - Add `displayItems` prop to `FormDropdown` and `FormDropdownInput` for showing selected values that aren't in the dropdown list - Exclude `missingValueItem` from cloud asset mode `dropdownItems` while still displaying it in the input field via `displayItems` - Use localized error messages in `ImagePreview` for missing images (`g.imageDoesNotExist`, `g.unknownFile`) - Add tests for cloud asset mode behavior in `WidgetSelectDropdown.test.ts` ## Context The `missingValueItem` was originally added in PR #8276 for template workflows. This fix keeps that behavior for local mode but excludes it from cloud asset mode dropdown. Cloud users can't access files they don't own, so showing them as search results causes confusion. ## Testing - Added unit tests for cloud asset mode behavior - Verified existing tests pass - All quality gates pass: typecheck, lint, format, tests Fixes COM-14333 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8747-fix-exclude-missing-assets-from-cloud-mode-dropdown-COM-14333-3016d73d365081e3ab47c326d791257e) by [Unito](https://www.unito.io) --------- Co-authored-by: Alexander Brown <drjkl@comfy.org> Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
Binary file not shown.
|
Before Width: | Height: | Size: 93 KiB After Width: | Height: | Size: 93 KiB |
Binary file not shown.
|
Before Width: | Height: | Size: 79 KiB After Width: | Height: | Size: 80 KiB |
@@ -94,6 +94,8 @@
|
||||
"openNewIssue": "Open New Issue",
|
||||
"showReport": "Show Report",
|
||||
"imageFailedToLoad": "Image failed to load",
|
||||
"imageDoesNotExist": "Image does not exist",
|
||||
"unknownFile": "Unknown file",
|
||||
"reconnecting": "Reconnecting",
|
||||
"reconnected": "Reconnected",
|
||||
"delete": "Delete",
|
||||
|
||||
@@ -29,6 +29,8 @@ const i18n = createI18n({
|
||||
failedToDownloadImage: 'Failed to download image',
|
||||
calculatingDimensions: 'Calculating dimensions',
|
||||
imageFailedToLoad: 'Image failed to load',
|
||||
imageDoesNotExist: 'Image does not exist',
|
||||
unknownFile: 'Unknown file',
|
||||
loading: 'Loading'
|
||||
}
|
||||
}
|
||||
|
||||
@@ -321,10 +321,11 @@ const handleKeyDown = (event: KeyboardEvent) => {
|
||||
}
|
||||
|
||||
const getImageFilename = (url: string): string => {
|
||||
if (!url) return t('g.imageDoesNotExist')
|
||||
try {
|
||||
return new URL(url).searchParams.get('filename') || 'Unknown file'
|
||||
return new URL(url).searchParams.get('filename') || t('g.unknownFile')
|
||||
} catch {
|
||||
return 'Invalid URL'
|
||||
return t('g.imageDoesNotExist')
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
@@ -2,16 +2,31 @@ import { createTestingPinia } from '@pinia/testing'
|
||||
import { mount } from '@vue/test-utils'
|
||||
import type { VueWrapper } from '@vue/test-utils'
|
||||
import PrimeVue from 'primevue/config'
|
||||
import { computed } from 'vue'
|
||||
import type { ComponentPublicInstance } from 'vue'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createI18n } from 'vue-i18n'
|
||||
|
||||
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
|
||||
import type { FormDropdownItem } from '@/renderer/extensions/vueNodes/widgets/components/form/dropdown/types'
|
||||
import type { ComboInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
|
||||
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
|
||||
|
||||
import WidgetSelectDropdown from '@/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue'
|
||||
|
||||
const mockAssetsData = vi.hoisted(() => ({ items: [] as AssetItem[] }))
|
||||
vi.mock(
|
||||
'@/renderer/extensions/vueNodes/widgets/composables/useAssetWidgetData',
|
||||
() => ({
|
||||
useAssetWidgetData: () => ({
|
||||
category: computed(() => 'checkpoints'),
|
||||
assets: computed(() => mockAssetsData.items),
|
||||
isLoading: computed(() => false),
|
||||
error: computed(() => null)
|
||||
})
|
||||
})
|
||||
)
|
||||
|
||||
const i18n = createI18n({
|
||||
legacy: false,
|
||||
locale: 'en',
|
||||
@@ -306,3 +321,133 @@ describe('WidgetSelectDropdown custom label mapping', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('WidgetSelectDropdown cloud asset mode (COM-14333)', () => {
|
||||
interface CloudModeInstance extends ComponentPublicInstance {
|
||||
dropdownItems: FormDropdownItem[]
|
||||
displayItems: FormDropdownItem[]
|
||||
selectedSet: Set<string>
|
||||
}
|
||||
|
||||
const createTestAsset = (
|
||||
id: string,
|
||||
name: string,
|
||||
preview_url: string
|
||||
): AssetItem => ({
|
||||
id,
|
||||
name,
|
||||
preview_url,
|
||||
tags: []
|
||||
})
|
||||
|
||||
const createCloudModeWidget = (
|
||||
value: string = 'model.safetensors'
|
||||
): SimplifiedWidget<string | undefined> => ({
|
||||
name: 'test_model_select',
|
||||
type: 'combo',
|
||||
value,
|
||||
options: {
|
||||
values: [],
|
||||
nodeType: 'CheckpointLoaderSimple'
|
||||
}
|
||||
})
|
||||
|
||||
const mountCloudComponent = (
|
||||
widget: SimplifiedWidget<string | undefined>,
|
||||
modelValue: string | undefined
|
||||
): VueWrapper<CloudModeInstance> => {
|
||||
return mount(WidgetSelectDropdown, {
|
||||
props: {
|
||||
widget,
|
||||
modelValue,
|
||||
assetKind: 'model',
|
||||
isAssetMode: true,
|
||||
nodeType: 'CheckpointLoaderSimple'
|
||||
},
|
||||
global: {
|
||||
plugins: [PrimeVue, createTestingPinia(), i18n]
|
||||
}
|
||||
}) as unknown as VueWrapper<CloudModeInstance>
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mockAssetsData.items = []
|
||||
})
|
||||
|
||||
it('does not include missing items in cloud asset mode dropdown', () => {
|
||||
mockAssetsData.items = [
|
||||
createTestAsset(
|
||||
'asset-1',
|
||||
'existing_model.safetensors',
|
||||
'https://example.com/preview.jpg'
|
||||
)
|
||||
]
|
||||
|
||||
const widget = createCloudModeWidget('missing_model.safetensors')
|
||||
const wrapper = mountCloudComponent(widget, 'missing_model.safetensors')
|
||||
|
||||
const dropdownItems = wrapper.vm.dropdownItems
|
||||
expect(dropdownItems).toHaveLength(1)
|
||||
expect(dropdownItems[0].name).toBe('existing_model.safetensors')
|
||||
expect(
|
||||
dropdownItems.some((item) => item.name === 'missing_model.safetensors')
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('shows only available cloud assets in dropdown', () => {
|
||||
mockAssetsData.items = [
|
||||
createTestAsset(
|
||||
'asset-1',
|
||||
'model_a.safetensors',
|
||||
'https://example.com/a.jpg'
|
||||
),
|
||||
createTestAsset(
|
||||
'asset-2',
|
||||
'model_b.safetensors',
|
||||
'https://example.com/b.jpg'
|
||||
)
|
||||
]
|
||||
|
||||
const widget = createCloudModeWidget('model_a.safetensors')
|
||||
const wrapper = mountCloudComponent(widget, 'model_a.safetensors')
|
||||
|
||||
const dropdownItems = wrapper.vm.dropdownItems
|
||||
expect(dropdownItems).toHaveLength(2)
|
||||
expect(dropdownItems.map((item) => item.name)).toEqual([
|
||||
'model_a.safetensors',
|
||||
'model_b.safetensors'
|
||||
])
|
||||
})
|
||||
|
||||
it('returns empty dropdown when no cloud assets available', () => {
|
||||
mockAssetsData.items = []
|
||||
|
||||
const widget = createCloudModeWidget('missing_model.safetensors')
|
||||
const wrapper = mountCloudComponent(widget, 'missing_model.safetensors')
|
||||
|
||||
const dropdownItems = wrapper.vm.dropdownItems
|
||||
expect(dropdownItems).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('includes missing cloud asset in displayItems for input field visibility', () => {
|
||||
mockAssetsData.items = [
|
||||
createTestAsset(
|
||||
'asset-1',
|
||||
'existing_model.safetensors',
|
||||
'https://example.com/preview.jpg'
|
||||
)
|
||||
]
|
||||
|
||||
const widget = createCloudModeWidget('missing_model.safetensors')
|
||||
const wrapper = mountCloudComponent(widget, 'missing_model.safetensors')
|
||||
|
||||
const displayItems = wrapper.vm.displayItems
|
||||
expect(displayItems).toHaveLength(2)
|
||||
expect(displayItems[0].name).toBe('missing_model.safetensors')
|
||||
expect(displayItems[0].id).toBe('missing-missing_model.safetensors')
|
||||
expect(displayItems[1].name).toBe('existing_model.safetensors')
|
||||
|
||||
const selectedSet = wrapper.vm.selectedSet
|
||||
expect(selectedSet.has('missing-missing_model.safetensors')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -254,9 +254,8 @@ const baseModelFilteredAssetItems = computed<FormDropdownItem[]>(() =>
|
||||
|
||||
const allItems = computed<FormDropdownItem[]>(() => {
|
||||
if (props.isAssetMode && assetData) {
|
||||
if (missingValueItem.value) {
|
||||
return [missingValueItem.value, ...baseModelFilteredAssetItems.value]
|
||||
}
|
||||
// Cloud assets not in user's library shouldn't appear as search results (COM-14333).
|
||||
// Unlike local mode, cloud users can't access files they don't own.
|
||||
return baseModelFilteredAssetItems.value
|
||||
}
|
||||
return [
|
||||
@@ -282,6 +281,17 @@ const dropdownItems = computed<FormDropdownItem[]>(() => {
|
||||
}
|
||||
})
|
||||
|
||||
/**
|
||||
* Items used for display in the input field. In cloud mode, includes
|
||||
* missing items so users can see their selected value even if not in library.
|
||||
*/
|
||||
const displayItems = computed<FormDropdownItem[]>(() => {
|
||||
if (props.isAssetMode && assetData && missingValueItem.value) {
|
||||
return [missingValueItem.value, ...baseModelFilteredAssetItems.value]
|
||||
}
|
||||
return dropdownItems.value
|
||||
})
|
||||
|
||||
const mediaPlaceholder = computed(() => {
|
||||
const options = props.widget.options
|
||||
|
||||
@@ -332,18 +342,20 @@ const acceptTypes = computed(() => {
|
||||
const layoutMode = ref<LayoutMode>(props.defaultLayoutMode ?? 'grid')
|
||||
|
||||
watch(
|
||||
[modelValue, dropdownItems],
|
||||
([currentValue, _dropdownItems]) => {
|
||||
[modelValue, displayItems],
|
||||
([currentValue]) => {
|
||||
if (currentValue === undefined) {
|
||||
selectedSet.value.clear()
|
||||
return
|
||||
}
|
||||
|
||||
const item = dropdownItems.value.find((item) => item.name === currentValue)
|
||||
if (item) {
|
||||
const item = displayItems.value.find((item) => item.name === currentValue)
|
||||
if (!item) {
|
||||
selectedSet.value.clear()
|
||||
selectedSet.value.add(item.id)
|
||||
return
|
||||
}
|
||||
selectedSet.value.clear()
|
||||
selectedSet.value.add(item.id)
|
||||
},
|
||||
{ immediate: true }
|
||||
)
|
||||
@@ -461,6 +473,7 @@ function getMediaUrl(
|
||||
v-model:ownership-selected="ownershipSelected"
|
||||
v-model:base-model-selected="baseModelSelected"
|
||||
:items="dropdownItems"
|
||||
:display-items="displayItems"
|
||||
:placeholder="mediaPlaceholder"
|
||||
:multiple="false"
|
||||
:uploadable
|
||||
|
||||
@@ -18,6 +18,8 @@ import type { FormDropdownItem, LayoutMode, SortOption } from './types'
|
||||
|
||||
interface Props {
|
||||
items: FormDropdownItem[]
|
||||
/** Items used for display in the input field. Falls back to items if not provided. */
|
||||
displayItems?: FormDropdownItem[]
|
||||
placeholder?: string
|
||||
/**
|
||||
* If true, allows multiple selections. If a number is provided,
|
||||
@@ -193,6 +195,7 @@ async function customSearcher(
|
||||
:is-open
|
||||
:placeholder="placeholderText"
|
||||
:items
|
||||
:display-items
|
||||
:max-selectable
|
||||
:selected
|
||||
:uploadable
|
||||
|
||||
@@ -10,6 +10,8 @@ interface Props {
|
||||
isOpen?: boolean
|
||||
placeholder?: string
|
||||
items: FormDropdownItem[]
|
||||
/** Items used for display in the input field. Falls back to items if not provided. */
|
||||
displayItems?: FormDropdownItem[]
|
||||
selected: Set<string>
|
||||
maxSelectable: number
|
||||
uploadable: boolean
|
||||
@@ -28,7 +30,8 @@ const emit = defineEmits<{
|
||||
}>()
|
||||
|
||||
const selectedItems = computed(() => {
|
||||
return props.items.filter((item) => props.selected.has(item.id))
|
||||
const itemsToSearch = props.displayItems ?? props.items
|
||||
return itemsToSearch.filter((item) => props.selected.has(item.id))
|
||||
})
|
||||
|
||||
const theButtonStyle = computed(() =>
|
||||
|
||||
Reference in New Issue
Block a user