Skip to content

Commit 6c6d018

Browse files
committed
Address review findings: use finally block, fix misleading comment, clean up tests
- Use finally block to always reset lifecycleActionInProgress (Major #1) - Fix misleading "single assignment" comment in boardCrudStore (Minor #5) - Remove redundant test with no meaningful assertions (Minor #3) - Rename resolveDelete to resolvePush for clarity (Minor #4)
1 parent fced42e commit 6c6d018

3 files changed

Lines changed: 9 additions & 36 deletions

File tree

frontend/taskdeck-web/src/components/board/BoardSettingsModal.vue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ async function handleLifecycleTransition() {
108108
}
109109
} catch (error) {
110110
console.error('Failed to update board lifecycle state:', error)
111+
} finally {
111112
lifecycleActionInProgress.value = false
112113
}
113114
}

frontend/taskdeck-web/src/store/board/boardCrudStore.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,11 @@ export function createBoardCrudActions(state: BoardState, helpers: BoardHelpers)
139139
state.error.value = null
140140
await boardsApi.deleteBoard(boardId)
141141

142-
// Clear current board state in a single assignment to avoid cascading
143-
// reactive updates. Previously, each `.value = …` triggered its own
144-
// flush cycle, which caused watchers/computed properties in mounted
145-
// views (BoardView, BoardCanvas, etc.) to re-evaluate on every
146-
// intermediate state — the root cause of the ~30 s freeze in #519.
142+
// Clear detailed state for the current board before removing it from the
143+
// main boards list. This prevents any watchers on the `boards` array
144+
// from accidentally accessing stale detail state (like cards, labels, etc.)
145+
// that belongs to the board being deleted. The primary performance fix
146+
// for #519 is unmounting the BoardView before this action is called.
147147
const isCurrent = state.currentBoard.value?.id === boardId
148148
if (isCurrent) {
149149
state.currentBoard.value = null

frontend/taskdeck-web/src/tests/components/BoardSettingsModal.spec.ts

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -245,38 +245,10 @@ describe('BoardSettingsModal', () => {
245245
confirmSpy.mockRestore()
246246
})
247247

248-
it('should show loading label while archive action is in progress', async () => {
249-
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true)
250-
let resolveDelete: () => void
251-
mockStore.deleteBoard.mockReturnValue(new Promise<void>((resolve) => { resolveDelete = resolve }))
252-
253-
const wrapper = mount(BoardSettingsModal, {
254-
props: {
255-
board,
256-
isOpen: true,
257-
},
258-
})
259-
260-
const archiveButton = wrapper
261-
.findAll('button')
262-
.find((btn) => btn.text().includes('Move to Archive'))
263-
// Trigger without await so we can inspect intermediate state
264-
void archiveButton?.trigger('click')
265-
await wrapper.vm.$nextTick()
266-
await wrapper.vm.$nextTick()
267-
268-
// The close event fires immediately, so the modal will already be hidden,
269-
// but the button label should have been set to the in-progress state
270-
expect(wrapper.emitted('close')).toBeTruthy()
271-
272-
resolveDelete!()
273-
confirmSpy.mockRestore()
274-
})
275-
276248
it('should disable lifecycle button while action is in progress', async () => {
277249
const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true)
278-
let resolveDelete: () => void
279-
mockRouter.push.mockReturnValue(new Promise<void>((resolve) => { resolveDelete = resolve }))
250+
let resolvePush: () => void
251+
mockRouter.push.mockReturnValue(new Promise<void>((resolve) => { resolvePush = resolve }))
280252

281253
const wrapper = mount(BoardSettingsModal, {
282254
props: {
@@ -298,7 +270,7 @@ describe('BoardSettingsModal', () => {
298270
expect(buttonAfterClick?.exists()).toBe(true)
299271
expect((buttonAfterClick?.element as HTMLButtonElement).disabled).toBe(true)
300272

301-
resolveDelete!()
273+
resolvePush!()
302274
confirmSpy.mockRestore()
303275
})
304276

0 commit comments

Comments
 (0)