mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-11 10:30:10 +00:00
feat: add setMany to settingStore for batch setting updates (#8767)
## Summary - Adds `setMany()` method to `settingStore` for updating multiple settings in a single API call via the existing `storeSettings` endpoint - Extracts shared setting-apply logic (`applySettingLocally`) to reduce duplication between `set()` and `setMany()` - Migrates all call sites where multiple settings were updated sequentially to use `setMany()` ## Call sites updated - `releaseStore.ts` — `handleSkipRelease`, `handleShowChangelog`, `handleWhatsNewSeen` (3 settings each) - `keybindingService.ts` — `persistUserKeybindings` (2 settings) - `coreSettings.ts` — `NavigationMode.onChange` (2 settings) ## Test plan - [x] Unit tests for `setMany` (batch update, skip unchanged, no-op when unchanged) - [x] Updated `releaseStore.test.ts` assertions to verify `setMany` usage - [x] Updated `useCoreCommands.test.ts` mock to include `setMany` - [x] All existing tests pass - [x] `pnpm typecheck`, `pnpm lint`, `pnpm format` pass Fixes #1079 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8767-feat-add-setMany-to-settingStore-for-batch-setting-updates-3036d73d36508161b8b6d298e1be1b7a) by [Unito](https://www.unito.io)
This commit is contained in:
committed by
GitHub
parent
061e96e488
commit
0288ea5b39
@@ -298,7 +298,10 @@ test.describe('Settings', () => {
|
||||
await input.press('Alt+n')
|
||||
|
||||
const requestPromise = comfyPage.page.waitForRequest(
|
||||
'**/api/settings/Comfy.Keybinding.NewBindings'
|
||||
(req) =>
|
||||
req.url().includes('/api/settings') &&
|
||||
!req.url().includes('/api/settings/') &&
|
||||
req.method() === 'POST'
|
||||
)
|
||||
|
||||
// Save keybinding
|
||||
|
||||
@@ -197,6 +197,7 @@ describe('useCoreCommands', () => {
|
||||
addSetting: vi.fn(),
|
||||
load: vi.fn(),
|
||||
set: vi.fn(),
|
||||
setMany: vi.fn(),
|
||||
exists: vi.fn(),
|
||||
getDefaultValue: vi.fn(),
|
||||
isReady: true,
|
||||
|
||||
@@ -137,14 +137,14 @@ export function useKeybindingService() {
|
||||
}
|
||||
|
||||
async function persistUserKeybindings() {
|
||||
await settingStore.set(
|
||||
'Comfy.Keybinding.NewBindings',
|
||||
Object.values(keybindingStore.getUserKeybindings())
|
||||
)
|
||||
await settingStore.set(
|
||||
'Comfy.Keybinding.UnsetBindings',
|
||||
Object.values(keybindingStore.getUserUnsetKeybindings())
|
||||
)
|
||||
await settingStore.setMany({
|
||||
'Comfy.Keybinding.NewBindings': Object.values(
|
||||
keybindingStore.getUserKeybindings()
|
||||
),
|
||||
'Comfy.Keybinding.UnsetBindings': Object.values(
|
||||
keybindingStore.getUserUnsetKeybindings()
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -170,13 +170,15 @@ export const CORE_SETTINGS: SettingParams[] = [
|
||||
const settingStore = useSettingStore()
|
||||
|
||||
if (newValue === 'standard') {
|
||||
// Update related settings to match standard mode - select + panning
|
||||
await settingStore.set('Comfy.Canvas.LeftMouseClickBehavior', 'select')
|
||||
await settingStore.set('Comfy.Canvas.MouseWheelScroll', 'panning')
|
||||
await settingStore.setMany({
|
||||
'Comfy.Canvas.LeftMouseClickBehavior': 'select',
|
||||
'Comfy.Canvas.MouseWheelScroll': 'panning'
|
||||
})
|
||||
} else if (newValue === 'legacy') {
|
||||
// Update related settings to match legacy mode - panning + zoom
|
||||
await settingStore.set('Comfy.Canvas.LeftMouseClickBehavior', 'panning')
|
||||
await settingStore.set('Comfy.Canvas.MouseWheelScroll', 'zoom')
|
||||
await settingStore.setMany({
|
||||
'Comfy.Canvas.LeftMouseClickBehavior': 'panning',
|
||||
'Comfy.Canvas.MouseWheelScroll': 'zoom'
|
||||
})
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -15,7 +15,8 @@ import { app } from '@/scripts/app'
|
||||
vi.mock('@/scripts/api', () => ({
|
||||
api: {
|
||||
getSettings: vi.fn(),
|
||||
storeSetting: vi.fn()
|
||||
storeSetting: vi.fn(),
|
||||
storeSettings: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
@@ -503,6 +504,85 @@ describe('useSettingStore', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('setMany', () => {
|
||||
it('should set multiple values and make a single API call', async () => {
|
||||
const onChange1 = vi.fn()
|
||||
const onChange2 = vi.fn()
|
||||
store.addSetting({
|
||||
id: 'Comfy.Release.Version',
|
||||
name: 'Release Version',
|
||||
type: 'hidden',
|
||||
defaultValue: '',
|
||||
onChange: onChange1
|
||||
})
|
||||
store.addSetting({
|
||||
id: 'Comfy.Release.Status',
|
||||
name: 'Release Status',
|
||||
type: 'hidden',
|
||||
defaultValue: 'skipped',
|
||||
onChange: onChange2
|
||||
})
|
||||
vi.clearAllMocks()
|
||||
|
||||
await store.setMany({
|
||||
'Comfy.Release.Version': '1.0.0',
|
||||
'Comfy.Release.Status': 'changelog seen'
|
||||
})
|
||||
|
||||
expect(store.get('Comfy.Release.Version')).toBe('1.0.0')
|
||||
expect(store.get('Comfy.Release.Status')).toBe('changelog seen')
|
||||
expect(onChange1).toHaveBeenCalledWith('1.0.0', '')
|
||||
expect(onChange2).toHaveBeenCalledWith('changelog seen', 'skipped')
|
||||
expect(api.storeSettings).toHaveBeenCalledTimes(1)
|
||||
expect(api.storeSettings).toHaveBeenCalledWith({
|
||||
'Comfy.Release.Version': '1.0.0',
|
||||
'Comfy.Release.Status': 'changelog seen'
|
||||
})
|
||||
expect(api.storeSetting).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should skip unchanged values', async () => {
|
||||
store.addSetting({
|
||||
id: 'Comfy.Release.Version',
|
||||
name: 'Release Version',
|
||||
type: 'hidden',
|
||||
defaultValue: ''
|
||||
})
|
||||
store.addSetting({
|
||||
id: 'Comfy.Release.Status',
|
||||
name: 'Release Status',
|
||||
type: 'hidden',
|
||||
defaultValue: 'skipped'
|
||||
})
|
||||
await store.set('Comfy.Release.Version', 'existing')
|
||||
vi.clearAllMocks()
|
||||
|
||||
await store.setMany({
|
||||
'Comfy.Release.Version': 'existing',
|
||||
'Comfy.Release.Status': 'changelog seen'
|
||||
})
|
||||
|
||||
expect(api.storeSettings).toHaveBeenCalledWith({
|
||||
'Comfy.Release.Status': 'changelog seen'
|
||||
})
|
||||
})
|
||||
|
||||
it('should not call API when all values are unchanged', async () => {
|
||||
store.addSetting({
|
||||
id: 'Comfy.Release.Version',
|
||||
name: 'Release Version',
|
||||
type: 'hidden',
|
||||
defaultValue: ''
|
||||
})
|
||||
await store.set('Comfy.Release.Version', 'existing')
|
||||
vi.clearAllMocks()
|
||||
|
||||
await store.setMany({ 'Comfy.Release.Version': 'existing' })
|
||||
|
||||
expect(api.storeSettings).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('getSettingInfo', () => {
|
||||
|
||||
@@ -92,23 +92,58 @@ export const useSettingStore = defineStore('setting', () => {
|
||||
}
|
||||
|
||||
/**
|
||||
* Set a setting value.
|
||||
* @param key - The key of the setting to set.
|
||||
* @param value - The value to set.
|
||||
* Apply a setting value locally: clone, migrate, fire onChange, and
|
||||
* update the in-memory store. Returns the migrated value, or
|
||||
* `undefined` when the value is unchanged and was skipped.
|
||||
*/
|
||||
async function set<K extends keyof Settings>(key: K, value: Settings[K]) {
|
||||
// Clone the incoming value to prevent external mutations
|
||||
function applySettingLocally<K extends keyof Settings>(
|
||||
key: K,
|
||||
value: Settings[K]
|
||||
): Settings[K] | undefined {
|
||||
const clonedValue = _.cloneDeep(value)
|
||||
const newValue = tryMigrateDeprecatedValue(
|
||||
settingsById.value[key],
|
||||
clonedValue
|
||||
)
|
||||
const oldValue = get(key)
|
||||
if (newValue === oldValue) return
|
||||
if (newValue === oldValue) return undefined
|
||||
|
||||
onChange(settingsById.value[key], newValue, oldValue)
|
||||
settingValues.value[key] = newValue
|
||||
await api.storeSetting(key, newValue)
|
||||
return newValue as Settings[K]
|
||||
}
|
||||
|
||||
/**
|
||||
* Set a setting value.
|
||||
* @param key - The key of the setting to set.
|
||||
* @param value - The value to set.
|
||||
*/
|
||||
async function set<K extends keyof Settings>(key: K, value: Settings[K]) {
|
||||
const applied = applySettingLocally(key, value)
|
||||
if (applied === undefined) return
|
||||
await api.storeSetting(key, applied)
|
||||
}
|
||||
|
||||
/**
|
||||
* Set multiple setting values in a single API call.
|
||||
* @param settings - A partial settings object with key-value pairs to set.
|
||||
*/
|
||||
async function setMany(settings: Partial<Settings>) {
|
||||
const updatedSettings: Partial<Settings> = {}
|
||||
|
||||
for (const key of Object.keys(settings) as (keyof Settings)[]) {
|
||||
const applied = applySettingLocally(
|
||||
key,
|
||||
settings[key] as Settings[typeof key]
|
||||
)
|
||||
if (applied !== undefined) {
|
||||
updatedSettings[key] = applied
|
||||
}
|
||||
}
|
||||
|
||||
if (Object.keys(updatedSettings).length > 0) {
|
||||
await api.storeSettings(updatedSettings)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -271,6 +306,7 @@ export const useSettingStore = defineStore('setting', () => {
|
||||
load,
|
||||
addSetting,
|
||||
set,
|
||||
setMany,
|
||||
get,
|
||||
exists,
|
||||
getDefaultValue
|
||||
|
||||
@@ -46,8 +46,9 @@ vi.mock('@/platform/settings/settingStore', () => {
|
||||
return null
|
||||
})
|
||||
const set = vi.fn()
|
||||
const setMany = vi.fn()
|
||||
return {
|
||||
useSettingStore: () => ({ get, set })
|
||||
useSettingStore: () => ({ get, set, setMany })
|
||||
}
|
||||
})
|
||||
|
||||
@@ -534,18 +535,11 @@ describe('useReleaseStore', () => {
|
||||
const settingStore = useSettingStore()
|
||||
await store.handleSkipRelease('1.2.0')
|
||||
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Version',
|
||||
'1.2.0'
|
||||
)
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Status',
|
||||
'skipped'
|
||||
)
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Timestamp',
|
||||
expect.any(Number)
|
||||
)
|
||||
expect(settingStore.setMany).toHaveBeenCalledWith({
|
||||
'Comfy.Release.Version': '1.2.0',
|
||||
'Comfy.Release.Status': 'skipped',
|
||||
'Comfy.Release.Timestamp': expect.any(Number)
|
||||
})
|
||||
})
|
||||
|
||||
it('should handle show changelog', async () => {
|
||||
@@ -554,18 +548,11 @@ describe('useReleaseStore', () => {
|
||||
const settingStore = useSettingStore()
|
||||
await store.handleShowChangelog('1.2.0')
|
||||
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Version',
|
||||
'1.2.0'
|
||||
)
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Status',
|
||||
'changelog seen'
|
||||
)
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Timestamp',
|
||||
expect.any(Number)
|
||||
)
|
||||
expect(settingStore.setMany).toHaveBeenCalledWith({
|
||||
'Comfy.Release.Version': '1.2.0',
|
||||
'Comfy.Release.Status': 'changelog seen',
|
||||
'Comfy.Release.Timestamp': expect.any(Number)
|
||||
})
|
||||
})
|
||||
|
||||
it('should handle whats new seen', async () => {
|
||||
@@ -574,18 +561,11 @@ describe('useReleaseStore', () => {
|
||||
const settingStore = useSettingStore()
|
||||
await store.handleWhatsNewSeen('1.2.0')
|
||||
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Version',
|
||||
'1.2.0'
|
||||
)
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Status',
|
||||
"what's new seen"
|
||||
)
|
||||
expect(settingStore.set).toHaveBeenCalledWith(
|
||||
'Comfy.Release.Timestamp',
|
||||
expect.any(Number)
|
||||
)
|
||||
expect(settingStore.setMany).toHaveBeenCalledWith({
|
||||
'Comfy.Release.Version': '1.2.0',
|
||||
'Comfy.Release.Status': "what's new seen",
|
||||
'Comfy.Release.Timestamp': expect.any(Number)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -208,9 +208,11 @@ export const useReleaseStore = defineStore('release', () => {
|
||||
return
|
||||
}
|
||||
|
||||
await settingStore.set('Comfy.Release.Version', version)
|
||||
await settingStore.set('Comfy.Release.Status', 'skipped')
|
||||
await settingStore.set('Comfy.Release.Timestamp', Date.now())
|
||||
await settingStore.setMany({
|
||||
'Comfy.Release.Version': version,
|
||||
'Comfy.Release.Status': 'skipped',
|
||||
'Comfy.Release.Timestamp': Date.now()
|
||||
})
|
||||
}
|
||||
|
||||
async function handleShowChangelog(version: string): Promise<void> {
|
||||
@@ -218,9 +220,11 @@ export const useReleaseStore = defineStore('release', () => {
|
||||
return
|
||||
}
|
||||
|
||||
await settingStore.set('Comfy.Release.Version', version)
|
||||
await settingStore.set('Comfy.Release.Status', 'changelog seen')
|
||||
await settingStore.set('Comfy.Release.Timestamp', Date.now())
|
||||
await settingStore.setMany({
|
||||
'Comfy.Release.Version': version,
|
||||
'Comfy.Release.Status': 'changelog seen',
|
||||
'Comfy.Release.Timestamp': Date.now()
|
||||
})
|
||||
}
|
||||
|
||||
async function handleWhatsNewSeen(version: string): Promise<void> {
|
||||
@@ -228,9 +232,11 @@ export const useReleaseStore = defineStore('release', () => {
|
||||
return
|
||||
}
|
||||
|
||||
await settingStore.set('Comfy.Release.Version', version)
|
||||
await settingStore.set('Comfy.Release.Status', "what's new seen")
|
||||
await settingStore.set('Comfy.Release.Timestamp', Date.now())
|
||||
await settingStore.setMany({
|
||||
'Comfy.Release.Version': version,
|
||||
'Comfy.Release.Status': "what's new seen",
|
||||
'Comfy.Release.Timestamp': Date.now()
|
||||
})
|
||||
}
|
||||
|
||||
// Fetch releases from API
|
||||
|
||||
@@ -1047,7 +1047,7 @@ export class ComfyApi extends EventTarget {
|
||||
/**
|
||||
* Stores a dictionary of settings for the current user
|
||||
*/
|
||||
async storeSettings(settings: Settings) {
|
||||
async storeSettings(settings: Partial<Settings>) {
|
||||
return this.fetchApi(`/settings`, {
|
||||
method: 'POST',
|
||||
body: JSON.stringify(settings)
|
||||
|
||||
Reference in New Issue
Block a user