From 1388dfe1b565f1e15b6c5f9828970be7eec00a77 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Wed, 4 Mar 2026 17:45:47 +0100 Subject: [PATCH] fix(focus-mode): allow switching tasks during Pomodoro breaks (#6726) Don't override user's task choice when a break ends or is skipped. Guard setCurrentTask dispatch with currentTaskId check in skipBreak$ and resumeTrackingOnBreakComplete$ effects, and pass undefined pausedTaskId when syncTrackingStartToSession$ auto-skips a break. --- .../store/focus-mode.effects.spec.ts | 65 +++++++++++++++++++ .../focus-mode/store/focus-mode.effects.ts | 14 ++-- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/app/features/focus-mode/store/focus-mode.effects.spec.ts b/src/app/features/focus-mode/store/focus-mode.effects.spec.ts index 5562deba55..5c24b3ba0f 100644 --- a/src/app/features/focus-mode/store/focus-mode.effects.spec.ts +++ b/src/app/features/focus-mode/store/focus-mode.effects.spec.ts @@ -737,6 +737,18 @@ describe('FocusModeEffects', () => { done(); }); }); + + // Bug #6726 fix: Don't override user's task switch during break + it('should NOT dispatch setCurrentTask when pausedTaskId exists but a different task is already being tracked', (done) => { + const pausedTaskId = 'original-task-id'; + currentTaskId$.next('different-task-id'); + actions$ = of(actions.completeBreak({ pausedTaskId })); + + effects.resumeTrackingOnBreakComplete$.pipe(toArray()).subscribe((actionsArr) => { + expect(actionsArr.length).toBe(0); + done(); + }); + }); }); describe('combined behavior', () => { @@ -811,6 +823,31 @@ describe('FocusModeEffects', () => { done(); }); }); + + // Bug #6726 fix: Don't override user's task switch during break + it('should NOT dispatch setCurrentTask when pausedTaskId exists but a different task is already being tracked', (done) => { + const pausedTaskId = 'original-task-id'; + currentTaskId$.next('different-task-id'); + actions$ = of(actions.skipBreak({ pausedTaskId })); + store.overrideSelector(selectors.selectMode, FocusModeMode.Countdown); + store.refreshState(); + + strategyFactoryMock.getStrategy.and.returnValue({ + initialSessionDuration: 25 * 60 * 1000, + shouldStartBreakAfterSession: false, + shouldAutoStartNextSession: false, + getBreakDuration: () => null, + }); + + const result: any[] = []; + effects.skipBreak$.subscribe({ + next: (action) => result.push(action), + complete: () => { + expect(result.length).toBe(0); + done(); + }, + }); + }); }); describe('cancelSession$', () => { @@ -1214,6 +1251,34 @@ describe('FocusModeEffects', () => { }, 50); }); + // Bug #6726 fix: When user starts tracking during active break, skipBreak should NOT carry stale pausedTaskId + it('should dispatch skipBreak with pausedTaskId undefined when user starts tracking during break', (done) => { + store.overrideSelector(selectFocusModeConfig, { + isSyncSessionWithTracking: true, + isSkipPreparation: false, + }); + store.overrideSelector( + selectors.selectTimer, + createMockTimer({ isRunning: true, purpose: 'break' }), + ); + store.overrideSelector(selectors.selectMode, FocusModeMode.Pomodoro); + store.overrideSelector(selectors.selectCurrentScreen, FocusScreen.Break); + store.overrideSelector(selectors.selectPausedTaskId, 'original-task-id'); + store.overrideSelector(selectors.selectIsResumingBreak, false); + store.refreshState(); + + effects = TestBed.inject(FocusModeEffects); + + setTimeout(() => { + currentTaskId$.next('new-task-id'); + }, 10); + + effects.syncTrackingStartToSession$.pipe(take(1)).subscribe((action) => { + expect(action).toEqual(actions.skipBreak({ pausedTaskId: undefined })); + done(); + }); + }); + it('should NOT dispatch when isFocusModeEnabled is false', (done) => { store.overrideSelector(selectFocusModeConfig, { isSyncSessionWithTracking: true, diff --git a/src/app/features/focus-mode/store/focus-mode.effects.ts b/src/app/features/focus-mode/store/focus-mode.effects.ts index e9a265986a..f913cd7f00 100644 --- a/src/app/features/focus-mode/store/focus-mode.effects.ts +++ b/src/app/features/focus-mode/store/focus-mode.effects.ts @@ -131,7 +131,8 @@ export class FocusModeEffects { } // User manually started tracking during break // Skip the break to sync with tracking (bug #5875 fix) - return of(actions.skipBreak({ pausedTaskId })); + // Bug #6726 fix: Don't pass pausedTaskId — the user already chose a new task + return of(actions.skipBreak({ pausedTaskId: undefined })); } // If no session active, start a new one (only from Main screen) if (timer.purpose === null && currentScreen === FocusScreen.Main) { @@ -468,11 +469,14 @@ export class FocusModeEffects { // (reducer clears pausedTaskId before effect reads state) // Effect 1: Resume tracking after break + // Bug #6726 fix: Don't override if user is already tracking a different task resumeTrackingOnBreakComplete$ = createEffect(() => this.actions$.pipe( ofType(actions.completeBreak), filter((action) => !!action.pausedTaskId), - map((action) => setCurrentTask({ id: action.pausedTaskId! })), + withLatestFrom(this.taskService.currentTaskId$), + filter(([_, currentTaskId]) => !currentTaskId), + map(([action]) => setCurrentTask({ id: action.pausedTaskId! })), ), ); @@ -515,13 +519,15 @@ export class FocusModeEffects { withLatestFrom( this.store.select(selectors.selectMode), this.store.select(selectFocusModeConfig), + this.taskService.currentTaskId$, ), - switchMap(([action, mode, config]) => { + switchMap(([action, mode, config, currentTaskId]) => { const strategy = this.strategyFactory.getStrategy(mode); const actionsToDispatch: any[] = []; // Resume task tracking if we paused it during break - if (action.pausedTaskId) { + // Bug #6726 fix: Don't override if user is already tracking a different task + if (action.pausedTaskId && !currentTaskId) { actionsToDispatch.push(setCurrentTask({ id: action.pausedTaskId })); }