From 7901e14318cf48c53235d7f275d0073d802dbfdd Mon Sep 17 00:00:00 2001 From: Benjamin Lu Date: Thu, 26 Feb 2026 19:49:08 -0800 Subject: [PATCH] fix: make docked job history toggle persistence-safe (#9265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Follow-up to #9215 to keep Docked Job History toggle behavior deterministic even when settings persistence fails. ## Changes - Close the actions popover immediately when toggling Docked Job History. - Use settingStore.setMany(...) when switching from docked to floating mode. - Set sidebarTabStore.activeSidebarTabId = 'job-history' before persisting when switching from floating to docked mode. - Wrap persistence calls in try/catch without rollback so locally-applied UI state remains deterministic. - Expand QueueOverlayHeader tests to cover setMany, popover close behavior, and persistence-failure paths. ## Testing - pnpm test:unit -- src/components/queue/QueueOverlayHeader.test.ts - pnpm typecheck - pnpm exec eslint src/components/queue/JobHistoryActionsMenu.vue src/components/queue/QueueOverlayHeader.test.ts - pnpm lint (fails in this branch due pre-existing stylelint errors in generated apps/desktop-ui/dist/**/*.css files, unrelated to this change) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9265-fix-make-docked-job-history-toggle-persistence-safe-3146d73d3650818f86c4dfdd57669abd) by [Unito](https://www.unito.io) --- .../queue/JobHistoryActionsMenu.vue | 24 +++++--- .../queue/QueueOverlayHeader.test.ts | 58 +++++++++++++++---- 2 files changed, 62 insertions(+), 20 deletions(-) diff --git a/src/components/queue/JobHistoryActionsMenu.vue b/src/components/queue/JobHistoryActionsMenu.vue index 58a21c5e7c..e1ad17551e 100644 --- a/src/components/queue/JobHistoryActionsMenu.vue +++ b/src/components/queue/JobHistoryActionsMenu.vue @@ -20,7 +20,7 @@ class="w-full justify-between text-sm font-light" variant="textonly" size="sm" - @click="onToggleDockedJobHistory" + @click="onToggleDockedJobHistory(close)" > void) => { emit('clearHistory') } -const onToggleDockedJobHistory = async () => { - if (isQueuePanelV2Enabled.value) { - await settingStore.set('Comfy.Queue.QPOV2', false) - await settingStore.set('Comfy.Queue.History.Expanded', true) +const onToggleDockedJobHistory = async (close: () => void) => { + close() + + try { + if (isQueuePanelV2Enabled.value) { + await settingStore.setMany({ + 'Comfy.Queue.QPOV2': false, + 'Comfy.Queue.History.Expanded': true + }) + return + } + + sidebarTabStore.activeSidebarTabId = 'job-history' + await settingStore.set('Comfy.Queue.QPOV2', true) + } catch { return } - - await settingStore.set('Comfy.Queue.QPOV2', true) - sidebarTabStore.activeSidebarTabId = 'job-history' } diff --git a/src/components/queue/QueueOverlayHeader.test.ts b/src/components/queue/QueueOverlayHeader.test.ts index ef3e22c32f..2c6cbb29ff 100644 --- a/src/components/queue/QueueOverlayHeader.test.ts +++ b/src/components/queue/QueueOverlayHeader.test.ts @@ -27,6 +27,7 @@ const mockGetSetting = vi.fn<(key: string) => boolean | undefined>((key) => key === 'Comfy.Queue.QPOV2' ? true : undefined ) const mockSetSetting = vi.fn() +const mockSetMany = vi.fn() const mockSidebarTabStore = { activeSidebarTabId: null as string | null } @@ -34,7 +35,8 @@ const mockSidebarTabStore = { vi.mock('@/platform/settings/settingStore', () => ({ useSettingStore: () => ({ get: mockGetSetting, - set: mockSetSetting + set: mockSetSetting, + setMany: mockSetMany }) })) @@ -88,6 +90,7 @@ describe('QueueOverlayHeader', () => { beforeEach(() => { popoverCloseSpy.mockClear() mockSetSetting.mockClear() + mockSetMany.mockClear() mockSidebarTabStore.activeSidebarTabId = null mockGetSetting.mockImplementation((key: string) => key === 'Comfy.Queue.QPOV2' ? true : undefined @@ -144,17 +147,13 @@ describe('QueueOverlayHeader', () => { ) await dockedJobHistoryButton.trigger('click') - expect(mockSetSetting).toHaveBeenCalledTimes(2) - expect(mockSetSetting).toHaveBeenNthCalledWith( - 1, - 'Comfy.Queue.QPOV2', - false - ) - expect(mockSetSetting).toHaveBeenNthCalledWith( - 2, - 'Comfy.Queue.History.Expanded', - true - ) + expect(popoverCloseSpy).toHaveBeenCalledTimes(1) + expect(mockSetMany).toHaveBeenCalledTimes(1) + expect(mockSetMany).toHaveBeenCalledWith({ + 'Comfy.Queue.QPOV2': false, + 'Comfy.Queue.History.Expanded': true + }) + expect(mockSetSetting).not.toHaveBeenCalled() expect(mockSidebarTabStore.activeSidebarTabId).toBe(null) }) @@ -169,8 +168,43 @@ describe('QueueOverlayHeader', () => { ) await dockedJobHistoryButton.trigger('click') + expect(popoverCloseSpy).toHaveBeenCalledTimes(1) expect(mockSetSetting).toHaveBeenCalledTimes(1) expect(mockSetSetting).toHaveBeenCalledWith('Comfy.Queue.QPOV2', true) + expect(mockSetMany).not.toHaveBeenCalled() + expect(mockSidebarTabStore.activeSidebarTabId).toBe('job-history') + }) + + it('keeps docked target open even when enabling persistence fails', async () => { + mockGetSetting.mockImplementation((key: string) => + key === 'Comfy.Queue.QPOV2' ? false : undefined + ) + mockSetSetting.mockRejectedValueOnce(new Error('persistence failed')) + const wrapper = mountHeader() + + const dockedJobHistoryButton = wrapper.get( + '[data-testid="docked-job-history-action"]' + ) + await dockedJobHistoryButton.trigger('click') + + expect(popoverCloseSpy).toHaveBeenCalledTimes(1) + expect(mockSetSetting).toHaveBeenCalledWith('Comfy.Queue.QPOV2', true) expect(mockSidebarTabStore.activeSidebarTabId).toBe('job-history') }) + + it('closes the menu when disabling persistence fails', async () => { + mockSetMany.mockRejectedValueOnce(new Error('persistence failed')) + const wrapper = mountHeader() + + const dockedJobHistoryButton = wrapper.get( + '[data-testid="docked-job-history-action"]' + ) + await dockedJobHistoryButton.trigger('click') + + expect(popoverCloseSpy).toHaveBeenCalledTimes(1) + expect(mockSetMany).toHaveBeenCalledWith({ + 'Comfy.Queue.QPOV2': false, + 'Comfy.Queue.History.Expanded': true + }) + }) })