Skip to content

Commit 26bb190

Browse files
committed
refactor: Simplify AssetBrowserModal and useModelUpload
1 parent 62febc8 commit 26bb190

File tree

3 files changed

+50
-62
lines changed

3 files changed

+50
-62
lines changed

src/platform/assets/components/AssetBrowserModal.test.ts

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,23 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
44

55
import AssetBrowserModal from '@/platform/assets/components/AssetBrowserModal.vue'
66
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
7-
8-
const mockAssetsStore = vi.hoisted(() => ({
9-
modelAssetsByNodeType: new Map<string, unknown[]>(),
10-
modelLoadingByNodeType: new Map<string, boolean>(),
11-
updateModelsForNodeType: vi.fn()
12-
}))
7+
import { useAssetsStore } from '@/stores/assetsStore'
138

149
vi.mock('@/i18n', () => ({
1510
t: (key: string, params?: Record<string, string>) =>
1611
params ? `${key}:${JSON.stringify(params)}` : key,
1712
d: (date: Date) => date.toLocaleDateString()
1813
}))
1914

20-
vi.mock('@/stores/assetsStore', () => ({
21-
useAssetsStore: () => mockAssetsStore
22-
}))
15+
vi.mock('@/stores/assetsStore', () => {
16+
const store = {
17+
modelAssetsByNodeType: new Map<string, unknown[]>(),
18+
modelLoadingByNodeType: new Map<string, boolean>(),
19+
updateModelsForNodeType: vi.fn(),
20+
updateModelsForTag: vi.fn()
21+
}
22+
return { useAssetsStore: () => store }
23+
})
2324

2425
vi.mock('@/stores/modelToNodeStore', () => ({
2526
useModelToNodeStore: () => ({
@@ -182,10 +183,12 @@ describe('AssetBrowserModal', () => {
182183
})
183184
}
184185

186+
const mockStore = useAssetsStore()
187+
185188
beforeEach(() => {
186-
mockAssetsStore.modelAssetsByNodeType.clear()
187-
mockAssetsStore.modelLoadingByNodeType.clear()
188-
mockAssetsStore.updateModelsForNodeType.mockReset()
189+
mockStore.modelAssetsByNodeType.clear()
190+
mockStore.modelLoadingByNodeType.clear()
191+
vi.clearAllMocks()
189192
})
190193

191194
describe('Integration with useAssetBrowser', () => {
@@ -194,10 +197,7 @@ describe('AssetBrowserModal', () => {
194197
createTestAsset('asset1', 'Model A', 'checkpoints'),
195198
createTestAsset('asset2', 'Model B', 'loras')
196199
]
197-
mockAssetsStore.modelAssetsByNodeType.set(
198-
'CheckpointLoaderSimple',
199-
assets
200-
)
200+
mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets)
201201

202202
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
203203
await flushPromises()
@@ -214,10 +214,7 @@ describe('AssetBrowserModal', () => {
214214
createTestAsset('c1', 'model.safetensors', 'checkpoints'),
215215
createTestAsset('l1', 'lora.pt', 'loras')
216216
]
217-
mockAssetsStore.modelAssetsByNodeType.set(
218-
'CheckpointLoaderSimple',
219-
assets
220-
)
217+
mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets)
221218

222219
const wrapper = createWrapper({
223220
nodeType: 'CheckpointLoaderSimple',
@@ -237,17 +234,14 @@ describe('AssetBrowserModal', () => {
237234
createWrapper({ nodeType: 'CheckpointLoaderSimple' })
238235
await flushPromises()
239236

240-
expect(mockAssetsStore.updateModelsForNodeType).toHaveBeenCalledWith(
237+
expect(mockStore.updateModelsForNodeType).toHaveBeenCalledWith(
241238
'CheckpointLoaderSimple'
242239
)
243240
})
244241

245242
it('displays cached assets immediately from store', async () => {
246243
const assets = [createTestAsset('asset1', 'Cached Model', 'checkpoints')]
247-
mockAssetsStore.modelAssetsByNodeType.set(
248-
'CheckpointLoaderSimple',
249-
assets
250-
)
244+
mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets)
251245

252246
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
253247

@@ -257,15 +251,33 @@ describe('AssetBrowserModal', () => {
257251
expect(gridAssets).toHaveLength(1)
258252
expect(gridAssets[0].name).toBe('Cached Model')
259253
})
254+
255+
it('triggers store refresh for asset type (tag) on mount', async () => {
256+
createWrapper({ assetType: 'models' })
257+
await flushPromises()
258+
259+
expect(mockStore.updateModelsForTag).toHaveBeenCalledWith('models')
260+
})
261+
262+
it('uses tag: prefix for cache key when assetType is provided', async () => {
263+
const assets = [createTestAsset('asset1', 'Tagged Model', 'models')]
264+
mockStore.modelAssetsByNodeType.set('tag:models', assets)
265+
266+
const wrapper = createWrapper({ assetType: 'models' })
267+
await flushPromises()
268+
269+
const assetGrid = wrapper.findComponent({ name: 'AssetGrid' })
270+
const gridAssets = assetGrid.props('assets') as AssetItem[]
271+
272+
expect(gridAssets).toHaveLength(1)
273+
expect(gridAssets[0].name).toBe('Tagged Model')
274+
})
260275
})
261276

262277
describe('Asset Selection', () => {
263278
it('emits asset-select event when asset is selected', async () => {
264279
const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')]
265-
mockAssetsStore.modelAssetsByNodeType.set(
266-
'CheckpointLoaderSimple',
267-
assets
268-
)
280+
mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets)
269281

270282
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
271283
await flushPromises()
@@ -278,10 +290,7 @@ describe('AssetBrowserModal', () => {
278290

279291
it('executes onSelect callback when provided', async () => {
280292
const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')]
281-
mockAssetsStore.modelAssetsByNodeType.set(
282-
'CheckpointLoaderSimple',
283-
assets
284-
)
293+
mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets)
285294

286295
const onSelect = vi.fn()
287296
const wrapper = createWrapper({
@@ -324,10 +333,7 @@ describe('AssetBrowserModal', () => {
324333
createTestAsset('asset1', 'Model A', 'checkpoints'),
325334
createTestAsset('asset2', 'Model B', 'loras')
326335
]
327-
mockAssetsStore.modelAssetsByNodeType.set(
328-
'CheckpointLoaderSimple',
329-
assets
330-
)
336+
mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets)
331337

332338
const wrapper = createWrapper({
333339
nodeType: 'CheckpointLoaderSimple',
@@ -360,10 +366,7 @@ describe('AssetBrowserModal', () => {
360366

361367
it('passes computed contentTitle to BaseModalLayout when no title prop', async () => {
362368
const assets = [createTestAsset('asset1', 'Model A', 'checkpoints')]
363-
mockAssetsStore.modelAssetsByNodeType.set(
364-
'CheckpointLoaderSimple',
365-
assets
366-
)
369+
mockStore.modelAssetsByNodeType.set('CheckpointLoaderSimple', assets)
367370

368371
const wrapper = createWrapper({ nodeType: 'CheckpointLoaderSimple' })
369372
await flushPromises()

src/platform/assets/components/AssetBrowserModal.vue

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464

6565
<script setup lang="ts">
6666
import { breakpointsTailwind, useBreakpoints } from '@vueuse/core'
67-
import { computed, provide, ref, watchEffect } from 'vue'
67+
import { computed, provide } from 'vue'
6868
import { useI18n } from 'vue-i18n'
6969
7070
import SearchBox from '@/components/common/SearchBox.vue'
@@ -124,22 +124,6 @@ const isLoading = computed(
124124
() => isStoreLoading.value && fetchedAssets.value.length === 0
125125
)
126126
127-
// Track if we've triggered a refresh for this key
128-
const refreshedKey = ref<string | null>(null)
129-
130-
// Trigger refresh when nodeType/assetType changes (runs on mount and prop change)
131-
watchEffect(async () => {
132-
const key = cacheKey.value
133-
if (key && key !== refreshedKey.value) {
134-
refreshedKey.value = key
135-
if (props.nodeType) {
136-
await assetStore.updateModelsForNodeType(props.nodeType)
137-
} else if (props.assetType) {
138-
await assetStore.updateModelsForTag(props.assetType)
139-
}
140-
}
141-
})
142-
143127
async function refreshAssets(): Promise<AssetItem[]> {
144128
if (props.nodeType) {
145129
return await assetStore.updateModelsForNodeType(props.nodeType)
@@ -150,6 +134,9 @@ async function refreshAssets(): Promise<AssetItem[]> {
150134
return []
151135
}
152136
137+
// Trigger background refresh on mount
138+
refreshAssets()
139+
153140
const { isUploadButtonEnabled, showUploadDialog } =
154141
useModelUpload(refreshAssets)
155142

src/platform/assets/composables/useModelUpload.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@ import UploadModelDialog from '@/platform/assets/components/UploadModelDialog.vu
33
import UploadModelDialogHeader from '@/platform/assets/components/UploadModelDialogHeader.vue'
44
import UploadModelUpgradeModal from '@/platform/assets/components/UploadModelUpgradeModal.vue'
55
import UploadModelUpgradeModalHeader from '@/platform/assets/components/UploadModelUpgradeModalHeader.vue'
6-
import type { AssetItem } from '@/platform/assets/schemas/assetSchema'
76
import { useDialogStore } from '@/stores/dialogStore'
8-
import type { UseAsyncStateReturn } from '@vueuse/core'
97
import { computed } from 'vue'
108

119
export function useModelUpload(
12-
execute?: UseAsyncStateReturn<AssetItem[], [], true>['execute']
10+
onUploadSuccess?: () => Promise<unknown> | void
1311
) {
1412
const dialogStore = useDialogStore()
1513
const { flags } = useFeatureFlags()
@@ -37,7 +35,7 @@ export function useModelUpload(
3735
component: UploadModelDialog,
3836
props: {
3937
onUploadSuccess: async () => {
40-
await execute?.()
38+
await onUploadSuccess?.()
4139
}
4240
},
4341
dialogComponentProps: {

0 commit comments

Comments
 (0)