From 9d3bb5e4ec5589e31d24b141bb3eff51aa5838db Mon Sep 17 00:00:00 2001 From: Yourz Date: Sat, 24 Jan 2026 18:02:23 +0800 Subject: [PATCH] fix: update for reviews --- src/locales/en/main.json | 1 + .../workflow/persistence/base/draftCache.ts | 3 +- .../useWorkflowPersistence.test.ts | 57 +++++++++++++++++++ .../composables/useWorkflowPersistence.ts | 24 +++++--- .../persistence/stores}/draftCache.test.ts | 12 +--- .../persistence/stores/workflowDraftStore.ts | 7 ++- 6 files changed, 85 insertions(+), 19 deletions(-) rename {tests-ui/tests/platform/workflow/persistence => src/platform/workflow/persistence/composables}/useWorkflowPersistence.test.ts (82%) rename {tests-ui/tests/platform/workflow/persistence => src/platform/workflow/persistence/stores}/draftCache.test.ts (88%) diff --git a/src/locales/en/main.json b/src/locales/en/main.json index 01376627c..8b512e7f0 100644 --- a/src/locales/en/main.json +++ b/src/locales/en/main.json @@ -1749,6 +1749,7 @@ "nothingToQueue": "Nothing to queue", "pleaseSelectOutputNodes": "Please select output nodes", "failedToQueue": "Failed to queue", + "failedToSaveDraft": "Failed to save workflow draft", "failedExecutionPathResolution": "Could not resolve path to selected nodes", "no3dScene": "No 3D scene to apply texture", "failedToApplyTexture": "Failed to apply texture", diff --git a/src/platform/workflow/persistence/base/draftCache.ts b/src/platform/workflow/persistence/base/draftCache.ts index 2ac15e4b9..634f355de 100644 --- a/src/platform/workflow/persistence/base/draftCache.ts +++ b/src/platform/workflow/persistence/base/draftCache.ts @@ -29,10 +29,11 @@ export const upsertDraft = ( snapshot: WorkflowDraftSnapshot, limit: number = MAX_DRAFTS ): DraftCacheState => { + const effectiveLimit = Math.max(1, limit) const drafts = { ...state.drafts, [path]: snapshot } const order = touchEntry(state.order, path) - while (order.length > limit) { + while (order.length > effectiveLimit) { const oldest = order.shift() if (!oldest) continue if (oldest !== path) { diff --git a/tests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts b/src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts similarity index 82% rename from tests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts rename to src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts index 1922897be..46bec7b7b 100644 --- a/tests-ui/tests/platform/workflow/persistence/useWorkflowPersistence.test.ts +++ b/src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts @@ -1,5 +1,6 @@ import { createPinia, setActivePinia } from 'pinia' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as I18n from 'vue-i18n' import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore' import { useWorkflowPersistence } from '@/platform/workflow/persistence/composables/useWorkflowPersistence' @@ -16,6 +17,23 @@ vi.mock('@/platform/settings/settingStore', () => ({ })) })) +const mockToastAdd = vi.fn() +vi.mock('primevue', () => ({ + useToast: () => ({ + add: mockToastAdd + }) +})) + +vi.mock('vue-i18n', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useI18n: () => ({ + t: (key: string) => key + }) + } +}) + const loadBlankWorkflow = vi.fn() vi.mock('@/platform/workflow/core/services/workflowService', () => ({ useWorkflowService: () => ({ @@ -23,6 +41,15 @@ vi.mock('@/platform/workflow/core/services/workflowService', () => ({ }) })) +vi.mock( + '@/platform/workflow/templates/composables/useTemplateUrlLoader', + () => ({ + useTemplateUrlLoader: () => ({ + loadTemplateFromUrlParams: vi.fn() + }) + }) +) + const executeCommand = vi.fn() vi.mock('@/stores/commandStore', () => ({ useCommandStore: () => ({ @@ -65,6 +92,9 @@ vi.mock('@/scripts/app', () => ({ graph: { serialize: () => mocks.serializeMock() }, + rootGraph: { + serialize: () => mocks.serializeMock() + }, loadGraphData: (...args: unknown[]) => mocks.loadGraphDataMock(...args), canvas: {} } @@ -82,6 +112,7 @@ describe('useWorkflowPersistence', () => { localStorage.clear() sessionStorage.clear() vi.clearAllMocks() + mockToastAdd.mockClear() useWorkflowDraftStore().reset() mocks.state.graphChangedHandler = null mocks.state.currentGraph = { initial: true } @@ -194,4 +225,30 @@ describe('useWorkflowPersistence', () => { workflowStore.openWorkflows.map((workflow) => workflow?.path) ).toContain('workflows/Unsaved Workflow.json') }) + + it('shows error toast when draft save fails', async () => { + const workflowStore = useWorkflowStore() + const draftStore = useWorkflowDraftStore() + + const workflow = workflowStore.createTemporary('FailingDraft.json') + await workflowStore.openWorkflow(workflow) + + useWorkflowPersistence() + expect(mocks.state.graphChangedHandler).toBeTypeOf('function') + + vi.spyOn(draftStore, 'saveDraft').mockImplementation(() => { + throw new Error('Storage quota exceeded') + }) + + mocks.state.currentGraph = { title: 'Test' } + mocks.state.graphChangedHandler!() + await vi.advanceTimersByTimeAsync(800) + + expect(mockToastAdd).toHaveBeenCalledWith( + expect.objectContaining({ + severity: 'error', + detail: expect.any(String) + }) + ) + }) }) diff --git a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts index 7f44290e0..48bf40d79 100644 --- a/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts +++ b/src/platform/workflow/persistence/composables/useWorkflowPersistence.ts @@ -1,5 +1,7 @@ +import { useToast } from 'primevue' import { tryOnScopeDispose } from '@vueuse/core' import { computed, ref, watch } from 'vue' +import { useI18n } from 'vue-i18n' import { useRoute, useRouter } from 'vue-router' import { @@ -21,6 +23,7 @@ import { getStorageValue, setStorageValue } from '@/scripts/utils' import { useCommandStore } from '@/stores/commandStore' export function useWorkflowPersistence() { + const { t } = useI18n() const workflowStore = useWorkflowStore() const settingStore = useSettingStore() const route = useRoute() @@ -28,6 +31,7 @@ export function useWorkflowPersistence() { const templateUrlLoader = useTemplateUrlLoader() const TEMPLATE_NAMESPACE = PRESERVED_QUERY_NAMESPACES.TEMPLATE const workflowDraftStore = useWorkflowDraftStore() + const toast = useToast() const ensureTemplateQueryFromIntent = async () => { hydratePreservedQuery(TEMPLATE_NAMESPACE) @@ -69,7 +73,12 @@ export function useWorkflowPersistence() { }) } catch (error) { console.error('Failed to save draft', error) - // If draft store fails, don't continue saving to storage + toast.add({ + severity: 'error', + summary: t('g.error'), + detail: t('toastMessages.failedToSaveDraft'), + life: 3000 + }) return } @@ -155,7 +164,6 @@ export function useWorkflowPersistence() { setStorageValue('Comfy.PreviousWorkflow', activeWorkflowKey) // When the activeWorkflow changes, the graph has already been loaded. // Saving the current state of the graph to the localStorage. - // Use debounced version to avoid immediate save on tab switch persistCurrentWorkflow() } ) @@ -189,13 +197,14 @@ export function useWorkflowPersistence() { ) // Get storage values before setting watchers - const storedWorkflows = JSON.parse( + const parsedWorkflows = JSON.parse( getStorageValue('Comfy.OpenWorkflowsPaths') || '[]' - ) as string[] - const storedActiveIndex = JSON.parse( + ) + const storedWorkflows = Array.isArray(parsedWorkflows) ? parsedWorkflows : [] + const parsedIndex = JSON.parse( getStorageValue('Comfy.ActiveWorkflowIndex') || '-1' - ) as number - + ) + const storedActiveIndex = typeof parsedIndex === 'number' ? parsedIndex : -1 watch(restoreState, ({ paths, activeIndex }) => { if (workflowPersistenceEnabled.value) { setStorageValue('Comfy.OpenWorkflowsPaths', JSON.stringify(paths)) @@ -220,6 +229,7 @@ export function useWorkflowPersistence() { 'Failed to parse workflow draft, creating with default', err ) + workflowDraftStore.removeDraft(path) workflowStore.createTemporary(draft.name) } }) diff --git a/tests-ui/tests/platform/workflow/persistence/draftCache.test.ts b/src/platform/workflow/persistence/stores/draftCache.test.ts similarity index 88% rename from tests-ui/tests/platform/workflow/persistence/draftCache.test.ts rename to src/platform/workflow/persistence/stores/draftCache.test.ts index ca10f3ab0..d30ec350e 100644 --- a/tests-ui/tests/platform/workflow/persistence/draftCache.test.ts +++ b/src/platform/workflow/persistence/stores/draftCache.test.ts @@ -1,15 +1,7 @@ import { describe, expect, it } from 'vitest' -import { - MAX_DRAFTS, - type WorkflowDraftSnapshot, - createDraftCacheState, - mostRecentDraftPath, - moveDraft, - removeDraft, - touchEntry, - upsertDraft -} from '@/platform/workflow/persistence/base/draftCache' +import { MAX_DRAFTS, createDraftCacheState, mostRecentDraftPath, moveDraft, removeDraft, touchEntry, upsertDraft } from '@/platform/workflow/persistence/base/draftCache'; +import type { WorkflowDraftSnapshot } from '@/platform/workflow/persistence/base/draftCache'; const createSnapshot = (name: string): WorkflowDraftSnapshot => ({ data: JSON.stringify({ name }), diff --git a/src/platform/workflow/persistence/stores/workflowDraftStore.ts b/src/platform/workflow/persistence/stores/workflowDraftStore.ts index 4098f8824..40cca16b5 100644 --- a/src/platform/workflow/persistence/stores/workflowDraftStore.ts +++ b/src/platform/workflow/persistence/stores/workflowDraftStore.ts @@ -87,7 +87,12 @@ export const useWorkflowDraftStore = defineStore('workflowDraft', () => { if (!payload) return false try { const workflow = JSON.parse(payload) - await comfyApp.loadGraphData(workflow, true, true, workflowName) + await comfyApp.loadGraphData( + workflow, + /* clean= */ true, + /* restore_view= */ true, + workflowName + ) return true } catch (err) { console.error('Failed to load persisted workflow', err)