add newUserService and unit tests

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'
@@ -303,15 +304,7 @@ onMounted(async () => {
     settingStore.addSetting(setting)
   })
 
-  // to determine if the user is "new user" or not, we check if the settingValues is empty. only it is empty, we set Comfy.InstalledVersion.
-  // more info: user settings are loaded in the BE folder ComfyUI/user/default/comfy.settings.json and new instance of ComfyUI will have no settings.
-  // if this settings file is not empty, it means the user has already used ComfyUI before, and we should not set the Comfy.InstalledVersion, according to https://github.com/Comfy-Org/ComfyUI_frontend/issues/4073
-  if (Object.keys(settingStore.settingValues).length == 0) {
-    await settingStore.set(
-      'Comfy.InstalledVersion',
-      __COMFYUI_FRONTEND_VERSION__
-    )
-  }
+  await newUserService().initializeIfNewUser(settingStore)
 
   // @ts-expect-error fixme ts strict error
   await comfyApp.setup(canvasRef.value)

src/services/newUserService.ts

@@ -0,0 +1,77 @@
+import type { useSettingStore } from '@/stores/settingStore'
+
+export const newUserService = () => {
+  let isNewUserDetermined = false
+  let isNewUserCached: boolean | null = null
+  let pendingCallbacks: Array<() => Promise<void>> = []
+
+  function checkIsNewUser(
+    settingStore: ReturnType<typeof useSettingStore>
+  ): 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<void>) {
+    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<typeof useSettingStore>
+  ) {
+    if (isNewUserDetermined) return
+
+    isNewUserCached = checkIsNewUser(settingStore)
+
+    console.log(`New user status determined: ${isNewUserCached}`)
+
+    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
+  }
+}

tests-ui/tests/services/newUserService.test.ts

@@ -0,0 +1,388 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { newUserService } from '@/services/newUserService'
+
+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 newUserService>
+  let mockSettingStore: any
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    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 log new user status', async () => {
+      const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
+
+      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 status determined: true'
+      )
+      consoleSpy.mockRestore()
+    })
+  })
+
+  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)
+    })
+  })
+})