From 40028bc9c3cc17c3f48b7716ad316fb115807b38 Mon Sep 17 00:00:00 2001 From: Michael Kaufmann <5276337+wulfland@users.noreply.github.com> Date: Tue, 10 Feb 2026 09:57:17 +0100 Subject: [PATCH 1/4] Enhance workout recovery and auto-save logic to prevent saving completed workouts --- src/db/service.ts | 7 +++++++ src/hooks/useWorkoutSession.ts | 14 +++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/db/service.ts b/src/db/service.ts index 02a8a11..eda6404 100644 --- a/src/db/service.ts +++ b/src/db/service.ts @@ -605,6 +605,7 @@ export async function autoSaveWorkout( /** * Recover an active workout from localStorage + * Only returns incomplete workouts - completed workouts are not recovered */ export function recoverActiveWorkout(): | import('../types/models').Workout @@ -626,6 +627,12 @@ export function recoverActiveWorkout(): return value; }); + // Don't recover completed workouts - they should be cleared + if (workout.completed) { + localStorage.removeItem('activeWorkout'); + return null; + } + return workout; } catch (error) { console.error('Failed to recover active workout:', error); diff --git a/src/hooks/useWorkoutSession.ts b/src/hooks/useWorkoutSession.ts index a8b4ccf..a371768 100644 --- a/src/hooks/useWorkoutSession.ts +++ b/src/hooks/useWorkoutSession.ts @@ -64,12 +64,16 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { // Auto-save active workout useEffect(() => { - if (!workout || !isActive) { + // Only auto-save if workout is active and not completed + if (!workout || !isActive || workout.completed) { return; } const save = () => { - autoSaveWorkout(workout); + // Double-check before saving + if (workout && isActive && !workout.completed) { + autoSaveWorkout(workout); + } }; // Save immediately @@ -180,6 +184,9 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { async (feedback?: WorkoutFeedback) => { if (!workout) return; + // Clear auto-saved data FIRST to prevent any race conditions + clearAutoSavedWorkout(); + const duration = Math.round( (new Date().getTime() - workout.date.getTime()) / 60000 ); // minutes @@ -210,9 +217,6 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { await updateWorkout(workout.id, completedWorkout); } - // Clear auto-saved data - clearAutoSavedWorkout(); - // Reset state setWorkout(null); setIsActive(false); From 1d9437fe1e0de11ba9e186c15980a909dbed5c39 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 09:01:32 +0000 Subject: [PATCH 2/4] Initial plan From 314d4bb01e90df1d4639f162c5f5ab08e4f38830 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 09:05:37 +0000 Subject: [PATCH 3/4] Fix workout recovery and auto-save issues based on PR review feedback Co-authored-by: wulfland <5276337+wulfland@users.noreply.github.com> --- src/db/service.test.ts | 201 +++++++++++++++++++++++++++++++++ src/db/service.ts | 12 +- src/hooks/useWorkoutSession.ts | 85 +++++++++----- 3 files changed, 270 insertions(+), 28 deletions(-) diff --git a/src/db/service.test.ts b/src/db/service.test.ts index c0f993f..9e5742a 100644 --- a/src/db/service.test.ts +++ b/src/db/service.test.ts @@ -24,6 +24,9 @@ import { getMesocycle, updateMesocycle, deleteMesocycle, + recoverActiveWorkout, + autoSaveWorkout, + clearAutoSavedWorkout, } from '../db/service'; describe('Database Service - User Profiles', () => { @@ -749,3 +752,201 @@ describe('Database Service - Data Flow Integration', () => { expect(mesocycle?.status).toBe('active'); }); }); + +describe('Workout Recovery Functions', () => { + beforeEach(() => { + // Clear localStorage before each test + localStorage.clear(); + }); + + afterEach(() => { + localStorage.clear(); + }); + + describe('recoverActiveWorkout', () => { + it('should return null when no activeWorkout in localStorage', () => { + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + expect(result).toBeNull(); + }); + + it('should return null and clear localStorage when JSON is malformed', () => { + localStorage.setItem('activeWorkout', 'invalid-json{'); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + expect(result).toBeNull(); + expect(localStorage.getItem('activeWorkout')).toBeNull(); + }); + + it('should return null and clear localStorage when JSON is not an object (null)', () => { + localStorage.setItem('activeWorkout', 'null'); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + expect(result).toBeNull(); + expect(localStorage.getItem('activeWorkout')).toBeNull(); + }); + + it('should return null and clear localStorage when JSON is not an object (string)', () => { + localStorage.setItem('activeWorkout', '"just a string"'); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + expect(result).toBeNull(); + expect(localStorage.getItem('activeWorkout')).toBeNull(); + }); + + it('should return null and clear localStorage when JSON is not an object (number)', () => { + localStorage.setItem('activeWorkout', '42'); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + expect(result).toBeNull(); + expect(localStorage.getItem('activeWorkout')).toBeNull(); + }); + + it('should return null and clear localStorage when JSON is not an object (array)', () => { + localStorage.setItem('activeWorkout', '[]'); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + expect(result).toBeNull(); + expect(localStorage.getItem('activeWorkout')).toBeNull(); + }); + + it('should return null and clear localStorage when workout is completed', () => { + const completedWorkout = { + id: 'workout-1', + date: new Date().toISOString(), + exercises: [], + completed: true, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; + localStorage.setItem('activeWorkout', JSON.stringify(completedWorkout)); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + expect(result).toBeNull(); + expect(localStorage.getItem('activeWorkout')).toBeNull(); + }); + + it('should return incomplete workout with revived Date objects', () => { + const now = new Date(); + const incompleteWorkout = { + id: 'workout-1', + date: now.toISOString(), + exercises: [], + completed: false, + createdAt: now.toISOString(), + updatedAt: now.toISOString(), + }; + localStorage.setItem('activeWorkout', JSON.stringify(incompleteWorkout)); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + + expect(result).not.toBeNull(); + expect(result?.id).toBe('workout-1'); + expect(result?.completed).toBe(false); + // Check that dates were revived as Date objects + expect(result?.date).toBeInstanceOf(Date); + expect(result?.createdAt).toBeInstanceOf(Date); + expect(result?.updatedAt).toBeInstanceOf(Date); + // localStorage should NOT be cleared for incomplete workouts + expect(localStorage.getItem('activeWorkout')).not.toBeNull(); + }); + + it('should handle ISO date strings with milliseconds', () => { + const now = new Date(); + const incompleteWorkout = { + id: 'workout-1', + date: now.toISOString(), // Format: 2026-02-10T09:02:02.270Z + exercises: [], + completed: false, + createdAt: now.toISOString(), + updatedAt: now.toISOString(), + }; + localStorage.setItem('activeWorkout', JSON.stringify(incompleteWorkout)); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + + expect(result?.date).toBeInstanceOf(Date); + expect(result?.date.getTime()).toBeCloseTo(now.getTime(), -1); + }); + + it('should handle ISO date strings without milliseconds', () => { + const dateWithoutMs = '2026-02-10T09:02:02Z'; + const incompleteWorkout = { + id: 'workout-1', + date: dateWithoutMs, + exercises: [], + completed: false, + createdAt: dateWithoutMs, + updatedAt: dateWithoutMs, + }; + localStorage.setItem('activeWorkout', JSON.stringify(incompleteWorkout)); + // Using imported recoverActiveWorkout + const result = recoverActiveWorkout(); + + expect(result?.date).toBeInstanceOf(Date); + expect(result?.date.toISOString()).toBe('2026-02-10T09:02:02.000Z'); + }); + }); + + describe('autoSaveWorkout', () => { + it('should save workout to localStorage', () => { + // Using imported autoSaveWorkout + const workout = { + id: 'workout-1', + date: new Date(), + exercises: [], + completed: false, + createdAt: new Date(), + updatedAt: new Date(), + }; + + autoSaveWorkout(workout); + const saved = localStorage.getItem('activeWorkout'); + expect(saved).not.toBeNull(); + + const parsed = JSON.parse(saved!); + expect(parsed.id).toBe('workout-1'); + expect(parsed.completed).toBe(false); + }); + + it('should not throw on localStorage errors', () => { + // Using imported autoSaveWorkout + const workout = { + id: 'workout-1', + date: new Date(), + exercises: [], + completed: false, + createdAt: new Date(), + updatedAt: new Date(), + }; + + // Mock localStorage.setItem to throw + const originalSetItem = localStorage.setItem; + localStorage.setItem = () => { + throw new Error('localStorage is full'); + }; + + // Should not throw + expect(() => autoSaveWorkout(workout)).not.toThrow(); + + // Restore + localStorage.setItem = originalSetItem; + }); + }); + + describe('clearAutoSavedWorkout', () => { + it('should remove activeWorkout from localStorage', () => { + // Using imported clearAutoSavedWorkout + localStorage.setItem('activeWorkout', 'some-data'); + expect(localStorage.getItem('activeWorkout')).not.toBeNull(); + + clearAutoSavedWorkout(); + expect(localStorage.getItem('activeWorkout')).toBeNull(); + }); + + it('should not throw when activeWorkout does not exist', () => { + // Using imported clearAutoSavedWorkout + expect(() => clearAutoSavedWorkout()).not.toThrow(); + }); + }); +}); diff --git a/src/db/service.ts b/src/db/service.ts index eda6404..cd102e6 100644 --- a/src/db/service.ts +++ b/src/db/service.ts @@ -627,15 +627,23 @@ export function recoverActiveWorkout(): return value; }); + // Validate that we have a non-null object (but not an array) + if (!workout || typeof workout !== 'object' || Array.isArray(workout)) { + localStorage.removeItem('activeWorkout'); + return null; + } + // Don't recover completed workouts - they should be cleared - if (workout.completed) { + if ((workout as import('../types/models').Workout).completed) { localStorage.removeItem('activeWorkout'); return null; } - return workout; + return workout as import('../types/models').Workout; } catch (error) { console.error('Failed to recover active workout:', error); + // Clear corrupted data + localStorage.removeItem('activeWorkout'); return null; } } diff --git a/src/hooks/useWorkoutSession.ts b/src/hooks/useWorkoutSession.ts index a371768..740aaaa 100644 --- a/src/hooks/useWorkoutSession.ts +++ b/src/hooks/useWorkoutSession.ts @@ -62,6 +62,16 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { const [currentExerciseIndex, setCurrentExerciseIndex] = useState(0); const autoSaveTimerRef = useRef(null); + // Use refs to track current state for auto-save to avoid closure issues + const workoutRef = useRef(workout); + const isActiveRef = useRef(isActive); + + // Keep refs in sync with state + useEffect(() => { + workoutRef.current = workout; + isActiveRef.current = isActive; + }, [workout, isActive]); + // Auto-save active workout useEffect(() => { // Only auto-save if workout is active and not completed @@ -70,9 +80,12 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { } const save = () => { - // Double-check before saving - if (workout && isActive && !workout.completed) { - autoSaveWorkout(workout); + // Read from refs to get current state, not closure-captured values + const currentWorkout = workoutRef.current; + const currentIsActive = isActiveRef.current; + + if (currentWorkout && currentIsActive && !currentWorkout.completed) { + autoSaveWorkout(currentWorkout); } }; @@ -184,8 +197,11 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { async (feedback?: WorkoutFeedback) => { if (!workout) return; - // Clear auto-saved data FIRST to prevent any race conditions - clearAutoSavedWorkout(); + // Stop auto-save interval immediately to prevent race conditions + if (autoSaveTimerRef.current) { + window.clearInterval(autoSaveTimerRef.current); + autoSaveTimerRef.current = null; + } const duration = Math.round( (new Date().getTime() - workout.date.getTime()) / 60000 @@ -199,28 +215,39 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { updatedAt: new Date(), }; - // Save to database - if (workout.id.startsWith('temp-workout-')) { - // Create new workout - const id = await createWorkout({ - date: completedWorkout.date, - splitDayId: completedWorkout.splitDayId, - exercises: completedWorkout.exercises, - notes: completedWorkout.notes, - completed: true, - duration, - feedback: completedWorkout.feedback, - }); - completedWorkout.id = id; - } else { - // Update existing workout - await updateWorkout(workout.id, completedWorkout); - } + try { + // Save to database + if (workout.id.startsWith('temp-workout-')) { + // Create new workout + const id = await createWorkout({ + date: completedWorkout.date, + splitDayId: completedWorkout.splitDayId, + exercises: completedWorkout.exercises, + notes: completedWorkout.notes, + completed: true, + duration, + feedback: completedWorkout.feedback, + }); + completedWorkout.id = id; + } else { + // Update existing workout + await updateWorkout(workout.id, completedWorkout); + } + + // Only clear localStorage after successful DB save + clearAutoSavedWorkout(); - // Reset state - setWorkout(null); - setIsActive(false); - setCurrentExerciseIndex(0); + // Reset state + setWorkout(null); + setIsActive(false); + setCurrentExerciseIndex(0); + } catch (error) { + // On error, keep the workout in localStorage so it can be recovered + console.error('Failed to save workout to database:', error); + // Re-enable auto-save by setting isActive to false and back to true + // This will trigger the useEffect to restart the interval + throw error; + } }, [workout] ); @@ -228,6 +255,12 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { const cancelWorkout = useCallback(() => { if (!workout) return; + // Stop auto-save interval immediately + if (autoSaveTimerRef.current) { + window.clearInterval(autoSaveTimerRef.current); + autoSaveTimerRef.current = null; + } + // Clear auto-saved data clearAutoSavedWorkout(); From b59e4577f25385f952e11c94cd773cec0a68b96b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Feb 2026 09:06:49 +0000 Subject: [PATCH 4/4] Clean up code based on code review feedback Co-authored-by: wulfland <5276337+wulfland@users.noreply.github.com> --- src/db/service.test.ts | 14 -------------- src/db/service.ts | 8 +++----- src/hooks/useWorkoutSession.ts | 2 -- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/src/db/service.test.ts b/src/db/service.test.ts index 9e5742a..48c33b6 100644 --- a/src/db/service.test.ts +++ b/src/db/service.test.ts @@ -765,14 +765,12 @@ describe('Workout Recovery Functions', () => { describe('recoverActiveWorkout', () => { it('should return null when no activeWorkout in localStorage', () => { - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result).toBeNull(); }); it('should return null and clear localStorage when JSON is malformed', () => { localStorage.setItem('activeWorkout', 'invalid-json{'); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result).toBeNull(); expect(localStorage.getItem('activeWorkout')).toBeNull(); @@ -780,7 +778,6 @@ describe('Workout Recovery Functions', () => { it('should return null and clear localStorage when JSON is not an object (null)', () => { localStorage.setItem('activeWorkout', 'null'); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result).toBeNull(); expect(localStorage.getItem('activeWorkout')).toBeNull(); @@ -788,7 +785,6 @@ describe('Workout Recovery Functions', () => { it('should return null and clear localStorage when JSON is not an object (string)', () => { localStorage.setItem('activeWorkout', '"just a string"'); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result).toBeNull(); expect(localStorage.getItem('activeWorkout')).toBeNull(); @@ -796,7 +792,6 @@ describe('Workout Recovery Functions', () => { it('should return null and clear localStorage when JSON is not an object (number)', () => { localStorage.setItem('activeWorkout', '42'); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result).toBeNull(); expect(localStorage.getItem('activeWorkout')).toBeNull(); @@ -804,7 +799,6 @@ describe('Workout Recovery Functions', () => { it('should return null and clear localStorage when JSON is not an object (array)', () => { localStorage.setItem('activeWorkout', '[]'); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result).toBeNull(); expect(localStorage.getItem('activeWorkout')).toBeNull(); @@ -820,7 +814,6 @@ describe('Workout Recovery Functions', () => { updatedAt: new Date().toISOString(), }; localStorage.setItem('activeWorkout', JSON.stringify(completedWorkout)); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result).toBeNull(); expect(localStorage.getItem('activeWorkout')).toBeNull(); @@ -837,7 +830,6 @@ describe('Workout Recovery Functions', () => { updatedAt: now.toISOString(), }; localStorage.setItem('activeWorkout', JSON.stringify(incompleteWorkout)); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result).not.toBeNull(); @@ -862,7 +854,6 @@ describe('Workout Recovery Functions', () => { updatedAt: now.toISOString(), }; localStorage.setItem('activeWorkout', JSON.stringify(incompleteWorkout)); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result?.date).toBeInstanceOf(Date); @@ -880,7 +871,6 @@ describe('Workout Recovery Functions', () => { updatedAt: dateWithoutMs, }; localStorage.setItem('activeWorkout', JSON.stringify(incompleteWorkout)); - // Using imported recoverActiveWorkout const result = recoverActiveWorkout(); expect(result?.date).toBeInstanceOf(Date); @@ -890,7 +880,6 @@ describe('Workout Recovery Functions', () => { describe('autoSaveWorkout', () => { it('should save workout to localStorage', () => { - // Using imported autoSaveWorkout const workout = { id: 'workout-1', date: new Date(), @@ -910,7 +899,6 @@ describe('Workout Recovery Functions', () => { }); it('should not throw on localStorage errors', () => { - // Using imported autoSaveWorkout const workout = { id: 'workout-1', date: new Date(), @@ -936,7 +924,6 @@ describe('Workout Recovery Functions', () => { describe('clearAutoSavedWorkout', () => { it('should remove activeWorkout from localStorage', () => { - // Using imported clearAutoSavedWorkout localStorage.setItem('activeWorkout', 'some-data'); expect(localStorage.getItem('activeWorkout')).not.toBeNull(); @@ -945,7 +932,6 @@ describe('Workout Recovery Functions', () => { }); it('should not throw when activeWorkout does not exist', () => { - // Using imported clearAutoSavedWorkout expect(() => clearAutoSavedWorkout()).not.toThrow(); }); }); diff --git a/src/db/service.ts b/src/db/service.ts index cd102e6..bffb26a 100644 --- a/src/db/service.ts +++ b/src/db/service.ts @@ -607,9 +607,7 @@ export async function autoSaveWorkout( * Recover an active workout from localStorage * Only returns incomplete workouts - completed workouts are not recovered */ -export function recoverActiveWorkout(): - | import('../types/models').Workout - | null { +export function recoverActiveWorkout(): Workout | null { try { const saved = localStorage.getItem('activeWorkout'); if (!saved) { @@ -634,12 +632,12 @@ export function recoverActiveWorkout(): } // Don't recover completed workouts - they should be cleared - if ((workout as import('../types/models').Workout).completed) { + if ((workout as Workout).completed) { localStorage.removeItem('activeWorkout'); return null; } - return workout as import('../types/models').Workout; + return workout as Workout; } catch (error) { console.error('Failed to recover active workout:', error); // Clear corrupted data diff --git a/src/hooks/useWorkoutSession.ts b/src/hooks/useWorkoutSession.ts index 740aaaa..344a67b 100644 --- a/src/hooks/useWorkoutSession.ts +++ b/src/hooks/useWorkoutSession.ts @@ -244,8 +244,6 @@ export function useWorkoutSession(): UseWorkoutSessionReturn { } catch (error) { // On error, keep the workout in localStorage so it can be recovered console.error('Failed to save workout to database:', error); - // Re-enable auto-save by setting isActive to false and back to true - // This will trigger the useEffect to restart the interval throw error; } },