From c1db367422bc74767ed4b0b7a388771a53b0154d Mon Sep 17 00:00:00 2001 From: Terry Jia Date: Fri, 4 Jul 2025 19:52:18 -0400 Subject: [PATCH] add installedVersion (#4337) Co-authored-by: bymyself --- src/components/graph/GraphCanvas.vue | 4 + src/constants/coreSettings.ts | 7 + src/schemas/apiSchema.ts | 1 + src/services/newUserService.ts | 74 +++ .../tests/services/newUserService.test.ts | 442 ++++++++++++++++++ 5 files changed, 528 insertions(+) create mode 100644 src/services/newUserService.ts create mode 100644 tests-ui/tests/services/newUserService.test.ts diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 8ef41fb34..3c0d20f99 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -78,6 +78,7 @@ import { app as comfyApp } from '@/scripts/app' import { ChangeTracker } from '@/scripts/changeTracker' import { IS_CONTROL_WIDGET, updateControlWidgetLabel } from '@/scripts/widgets' import { useColorPaletteService } from '@/services/colorPaletteService' +import { newUserService } from '@/services/newUserService' import { useWorkflowService } from '@/services/workflowService' import { useCommandStore } from '@/stores/commandStore' import { useExecutionStore } from '@/stores/executionStore' @@ -302,6 +303,9 @@ onMounted(async () => { CORE_SETTINGS.forEach((setting) => { settingStore.addSetting(setting) }) + + await newUserService().initializeIfNewUser(settingStore) + // @ts-expect-error fixme ts strict error await comfyApp.setup(canvasRef.value) canvasStore.canvas = comfyApp.canvas diff --git a/src/constants/coreSettings.ts b/src/constants/coreSettings.ts index 295ea01fd..b01b6ab0e 100644 --- a/src/constants/coreSettings.ts +++ b/src/constants/coreSettings.ts @@ -747,6 +747,13 @@ export const CORE_SETTINGS: SettingParams[] = [ defaultValue: false, versionAdded: '1.8.7' }, + { + id: 'Comfy.InstalledVersion', + name: 'The frontend version that was running when the user first installed ComfyUI', + type: 'hidden', + defaultValue: null, + versionAdded: '1.24.0' + }, { id: 'LiteGraph.ContextMenu.Scaling', name: 'Scale node combo widget menus (lists) when zoomed in', diff --git a/src/schemas/apiSchema.ts b/src/schemas/apiSchema.ts index da637612e..db55f61f0 100644 --- a/src/schemas/apiSchema.ts +++ b/src/schemas/apiSchema.ts @@ -451,6 +451,7 @@ const zSettings = z.object({ 'Comfy.Toast.DisableReconnectingToast': z.boolean(), 'Comfy.Workflow.Persist': z.boolean(), 'Comfy.TutorialCompleted': z.boolean(), + 'Comfy.InstalledVersion': z.string().nullable(), 'Comfy.Node.AllowImageSizeDraw': z.boolean(), 'Comfy-Desktop.AutoUpdate': z.boolean(), 'Comfy-Desktop.SendStatistics': z.boolean(), diff --git a/src/services/newUserService.ts b/src/services/newUserService.ts new file mode 100644 index 000000000..ff4a76f03 --- /dev/null +++ b/src/services/newUserService.ts @@ -0,0 +1,74 @@ +import type { useSettingStore } from '@/stores/settingStore' + +let pendingCallbacks: Array<() => Promise> = [] +let isNewUserDetermined = false +let isNewUserCached: boolean | null = null + +export const newUserService = () => { + function checkIsNewUser( + settingStore: ReturnType + ): boolean { + const isNewUserSettings = + Object.keys(settingStore.settingValues).length === 0 || + !settingStore.get('Comfy.TutorialCompleted') + const hasNoWorkflow = !localStorage.getItem('workflow') + const hasNoPreviousWorkflow = !localStorage.getItem( + 'Comfy.PreviousWorkflow' + ) + + return isNewUserSettings && hasNoWorkflow && hasNoPreviousWorkflow + } + + async function registerInitCallback(callback: () => Promise) { + if (isNewUserDetermined) { + if (isNewUserCached) { + try { + await callback() + } catch (error) { + console.error('New user initialization callback failed:', error) + } + } + } else { + pendingCallbacks.push(callback) + } + } + + async function initializeIfNewUser( + settingStore: ReturnType + ) { + if (isNewUserDetermined) return + + isNewUserCached = checkIsNewUser(settingStore) + isNewUserDetermined = true + + if (!isNewUserCached) { + pendingCallbacks = [] + return + } + + await settingStore.set( + 'Comfy.InstalledVersion', + __COMFYUI_FRONTEND_VERSION__ + ) + + for (const callback of pendingCallbacks) { + try { + await callback() + } catch (error) { + console.error('New user initialization callback failed:', error) + } + } + + pendingCallbacks = [] + } + + function isNewUser(): boolean | null { + return isNewUserDetermined ? isNewUserCached : null + } + + return { + registerInitCallback, + initializeIfNewUser, + isNewUser + } +} diff --git a/tests-ui/tests/services/newUserService.test.ts b/tests-ui/tests/services/newUserService.test.ts new file mode 100644 index 000000000..e940a785b --- /dev/null +++ b/tests-ui/tests/services/newUserService.test.ts @@ -0,0 +1,442 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mockLocalStorage = vi.hoisted(() => ({ + getItem: vi.fn(), + setItem: vi.fn(), + removeItem: vi.fn(), + clear: vi.fn() +})) + +Object.defineProperty(window, 'localStorage', { + value: mockLocalStorage, + writable: true +}) + +vi.mock('@/config/version', () => ({ + __COMFYUI_FRONTEND_VERSION__: '1.24.0' +})) + +//@ts-expect-error Define global for the test +global.__COMFYUI_FRONTEND_VERSION__ = '1.24.0' + +describe('newUserService', () => { + let service: ReturnType< + typeof import('@/services/newUserService').newUserService + > + let mockSettingStore: any + let newUserService: typeof import('@/services/newUserService').newUserService + + beforeEach(async () => { + vi.clearAllMocks() + + vi.resetModules() + + const module = await import('@/services/newUserService') + newUserService = module.newUserService + + service = newUserService() + + mockSettingStore = { + settingValues: {}, + get: vi.fn(), + set: vi.fn() + } + + mockLocalStorage.getItem.mockReturnValue(null) + }) + + describe('checkIsNewUser logic', () => { + it('should identify new user when all conditions are met', async () => { + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(true) + }) + + it('should identify new user when settings exist but TutorialCompleted is undefined', async () => { + mockSettingStore.settingValues = { 'some.setting': 'value' } + + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(true) + }) + + it('should identify existing user when tutorial is completed', async () => { + mockSettingStore.settingValues = { 'Comfy.TutorialCompleted': true } + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return true + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(false) + }) + + it('should identify existing user when workflow exists', async () => { + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockImplementation((key: string) => { + if (key === 'workflow') return 'some-workflow' + return null + }) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(false) + }) + + it('should identify existing user when previous workflow exists', async () => { + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockImplementation((key: string) => { + if (key === 'Comfy.PreviousWorkflow') return 'some-previous-workflow' + return null + }) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(false) + }) + + it('should identify new user when tutorial is explicitly false', async () => { + mockSettingStore.settingValues = { 'Comfy.TutorialCompleted': false } + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return false + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(true) + }) + + it('should identify existing user when has both settings and tutorial completed', async () => { + mockSettingStore.settingValues = { + 'some.setting': 'value', + 'Comfy.TutorialCompleted': true + } + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return true + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(false) + }) + + it('should identify existing user when only one condition fails', async () => { + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockImplementation((key: string) => { + if (key === 'workflow') return 'some-workflow' + if (key === 'Comfy.PreviousWorkflow') return null + return null + }) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(false) + }) + }) + + describe('registerInitCallback', () => { + it('should execute callback immediately if new user is already determined', async () => { + const mockCallback = vi.fn().mockResolvedValue(undefined) + + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + expect(service.isNewUser()).toBe(true) + + await service.registerInitCallback(mockCallback) + + expect(mockCallback).toHaveBeenCalledTimes(1) + }) + + it('should queue callbacks when user status is not determined', async () => { + const mockCallback = vi.fn().mockResolvedValue(undefined) + + await service.registerInitCallback(mockCallback) + + expect(mockCallback).not.toHaveBeenCalled() + expect(service.isNewUser()).toBeNull() + }) + + it('should handle callback errors gracefully', async () => { + const mockCallback = vi + .fn() + .mockRejectedValue(new Error('Callback error')) + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + await service.registerInitCallback(mockCallback) + + expect(consoleSpy).toHaveBeenCalledWith( + 'New user initialization callback failed:', + expect.any(Error) + ) + consoleSpy.mockRestore() + }) + }) + + describe('initializeIfNewUser', () => { + it('should set installed version for new users', async () => { + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(mockSettingStore.set).toHaveBeenCalledWith( + 'Comfy.InstalledVersion', + '1.24.0' + ) + }) + + it('should not set installed version for existing users', async () => { + mockSettingStore.settingValues = { 'some.setting': 'value' } + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return true + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(mockSettingStore.set).not.toHaveBeenCalled() + }) + + it('should execute pending callbacks for new users', async () => { + const mockCallback1 = vi.fn().mockResolvedValue(undefined) + const mockCallback2 = vi.fn().mockResolvedValue(undefined) + + await service.registerInitCallback(mockCallback1) + await service.registerInitCallback(mockCallback2) + + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(mockCallback1).toHaveBeenCalledTimes(1) + expect(mockCallback2).toHaveBeenCalledTimes(1) + }) + + it('should not execute pending callbacks for existing users', async () => { + const mockCallback = vi.fn().mockResolvedValue(undefined) + + await service.registerInitCallback(mockCallback) + + mockSettingStore.settingValues = { 'some.setting': 'value' } + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return true + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(mockCallback).not.toHaveBeenCalled() + }) + + it('should handle callback errors during initialization', async () => { + const mockCallback = vi.fn().mockRejectedValue(new Error('Init error')) + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + await service.registerInitCallback(mockCallback) + + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(consoleSpy).toHaveBeenCalledWith( + 'New user initialization callback failed:', + expect.any(Error) + ) + consoleSpy.mockRestore() + }) + + it('should not reinitialize if already determined', async () => { + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + expect(mockSettingStore.set).toHaveBeenCalledTimes(1) + + await service.initializeIfNewUser(mockSettingStore) + expect(mockSettingStore.set).toHaveBeenCalledTimes(1) + }) + + it('should correctly determine new user status', async () => { + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + // Before initialization, isNewUser should return null + expect(service.isNewUser()).toBeNull() + + await service.initializeIfNewUser(mockSettingStore) + + // After initialization, isNewUser should return true for a new user + expect(service.isNewUser()).toBe(true) + + // Should set the installed version for new users + expect(mockSettingStore.set).toHaveBeenCalledWith( + 'Comfy.InstalledVersion', + expect.any(String) + ) + }) + }) + + describe('isNewUser', () => { + it('should return null before determination', () => { + expect(service.isNewUser()).toBeNull() + }) + + it('should return cached result after determination', async () => { + mockSettingStore.settingValues = {} + mockSettingStore.get.mockReturnValue(undefined) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(true) + }) + }) + + describe('edge cases', () => { + it('should handle settingStore.get returning false as not completed', async () => { + mockSettingStore.settingValues = { 'Comfy.TutorialCompleted': false } + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return false + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + expect(service.isNewUser()).toBe(true) + }) + + it('should handle multiple callback registrations after initialization', async () => { + const mockCallback1 = vi.fn().mockResolvedValue(undefined) + const mockCallback2 = vi.fn().mockResolvedValue(undefined) + + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service.initializeIfNewUser(mockSettingStore) + + await service.registerInitCallback(mockCallback1) + await service.registerInitCallback(mockCallback2) + + expect(mockCallback1).toHaveBeenCalledTimes(1) + expect(mockCallback2).toHaveBeenCalledTimes(1) + }) + }) + + describe('state sharing between instances', () => { + it('should share state between multiple service instances', async () => { + const service1 = newUserService() + const service2 = newUserService() + + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service1.initializeIfNewUser(mockSettingStore) + + expect(service2.isNewUser()).toBe(true) + expect(service1.isNewUser()).toBe(service2.isNewUser()) + }) + + it('should execute callbacks registered on different instances', async () => { + const service1 = newUserService() + const service2 = newUserService() + + const mockCallback1 = vi.fn().mockResolvedValue(undefined) + const mockCallback2 = vi.fn().mockResolvedValue(undefined) + + await service1.registerInitCallback(mockCallback1) + await service2.registerInitCallback(mockCallback2) + + mockSettingStore.settingValues = {} + mockSettingStore.get.mockImplementation((key: string) => { + if (key === 'Comfy.TutorialCompleted') return undefined + return undefined + }) + mockLocalStorage.getItem.mockReturnValue(null) + + await service1.initializeIfNewUser(mockSettingStore) + + expect(mockCallback1).toHaveBeenCalledTimes(1) + expect(mockCallback2).toHaveBeenCalledTimes(1) + }) + }) +})