feat: improve workout management#137
Merged
Merged
Conversation
… better state management
…dule on plan overview
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds soft-delete for completed workouts and filters/dedupes query results, provides a delete mutation and history UI, refactors weight/reps state handling to a unified setter, introduces WeeklyScheduleDisplay with plan integration, tweaks home/start behaviour, and removes an Expo androidNavigationBar config. ChangesWorkout Management and Schedule Features
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
isotronic
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
components/WeeklyScheduleDisplay.tsx (1)
38-44: ⚡ Quick winConsider memoising the workout lookup for performance.
The workout-by-ID Map and schedule map are rebuilt on every render. Wrapping this logic in
useMemowould avoid unnecessary recalculations whenworkoutsandscheduleEntrieshaven't changed.♻️ Proposed refactor with useMemo
+import React, { useMemo } from "react"; ... - // Build day_of_week → workout name lookup - const workoutById = new Map(workouts.map((w) => [w.id, w])); - const scheduleMap: Record<number, string> = {}; - for (const entry of scheduleEntries) { - const workout = workoutById.get(entry.workout_id); - scheduleMap[entry.day_of_week] = workout?.name || "Workout"; - } + const scheduleMap = useMemo(() => { + const workoutById = new Map(workouts.map((w) => [w.id, w])); + const map: Record<number, string> = {}; + for (const entry of scheduleEntries) { + const workout = workoutById.get(entry.workout_id); + map[entry.day_of_week] = workout?.name || "Workout"; + } + return map; + }, [workouts, scheduleEntries]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/WeeklyScheduleDisplay.tsx` around lines 38 - 44, Wrap the computation of workoutById and scheduleMap in a useMemo so they are only recomputed when inputs change: compute workoutById = new Map(workouts.map(...)) and scheduleMap from scheduleEntries inside a single useMemo with [workouts, scheduleEntries] as dependencies (preserve the current shapes/types and the fallback "Workout" for missing names); then replace the existing top-level consts with the memoized values so renders reuse the maps when workouts and scheduleEntries are unchanged.app/(app)/(tabs)/index.tsx (1)
342-354: ⚡ Quick winExtract the repeated highlight condition to a named boolean.
The same complex condition appears in both the card border styling (lines 344-348) and the button mode logic (lines 389-393). This duplication increases maintenance burden and the risk of inconsistent updates.
♻️ Proposed refactor
Extract the condition to a named boolean immediately before the card rendering (around line 340):
); }); + const shouldHighlightFirstWorkout = + !workoutInProgress && + index === 0 && + !weeklyGoalReached && + !isRestDay && + completedTodayWorkoutIds.size === 0; + return ( <Pressable key={index} style={[ styles.workoutCard, - !workoutInProgress && - index === 0 && - !weeklyGoalReached && - !isRestDay && - completedTodayWorkoutIds.size === 0 + shouldHighlightFirstWorkout ? { borderWidth: 1, borderColor: Colors.dark.tint, } : null, ]}Then update the button mode:
<Button mode={ - !workoutInProgress && - index === 0 && - !weeklyGoalReached && - !isRestDay && - completedTodayWorkoutIds.size === 0 + shouldHighlightFirstWorkout ? "contained" : "outlined" }Also applies to: 388-395
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(app)/(tabs)/index.tsx around lines 342 - 354, Extract the repeated complex condition into a named boolean (e.g., shouldHighlightCard) defined immediately before the card render using the existing variables workoutInProgress, index, weeklyGoalReached, isRestDay, and completedTodayWorkoutIds; then replace the inline expression used in the style array (previously applied to styles.workoutCard) and the duplicated button mode logic with this new boolean so both places use shouldHighlightCard consistently and remain a boolean value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(app)/(tabs)/(stats)/history-details.tsx:
- Around line 151-163: The onPressIn handler calls
deleteMutation.mutate(Number(id)) without validating id; parse and validate the
route param (id) before invoking deleteMutation.mutate: use const parsedId =
Number(id) or parseInt(id, 10) and check Number.isFinite(parsedId) &&
Number.isInteger(parsedId) (and optionally parsedId > 0); if validation fails,
show an Alert (or no-op) instead of calling deleteMutation.mutate; update the
onPressIn block that contains deleteMutation.mutate(Number(id)) to perform this
guard and only call deleteMutation.mutate(parsedId) when valid.
In `@components/WeeklyScheduleDisplay.tsx`:
- Around line 10-13: The code builds a Map from WorkoutRef entries that may have
id=null/undefined, causing lookups by numeric schedule.workout_id to fail;
before constructing the Map in WeeklyScheduleDisplay, filter the workouts array
(the data used to build workoutMap) to only include entries where workout.id is
a valid number (e.g., id !== null && id !== undefined), then build the Map from
that filtered array so lookups by numeric workout_id succeed; optionally tighten
the WorkoutRef usage by asserting or narrowing id to number when inserting into
the Map and when consuming the Map for schedule rendering.
In `@utils/database.ts`:
- Around line 885-903: The code that builds sets in exercisesMap currently uses
"weight: convertedWeight || null" which coerces a valid 0 into null; in the
block that pushes into exercisesMap[row.exercise_id].sets (around the
convertedWeight calculation), replace that expression with a check that
preserves 0 and only sets null for non-numeric values, e.g. use
Number.isFinite(convertedWeight) ? convertedWeight : null (or an explicit !==
null/!== undefined and !Number.isNaN check) so legitimate zero weights remain 0.
- Around line 1337-1356: The soft-delete in deleteCompletedWorkout sets
is_deleted flags but fetchCompletedWorkoutById still returns rows by cw.id;
update fetchCompletedWorkoutById to ignore soft-deleted records by adding
is_deleted = FALSE (or = 0) filters to the completed_workouts query and any
joined/subqueries that read completed_exercises or completed_sets so deleted
workouts (and their child rows) are excluded when fetching by id; ensure the
function (fetchCompletedWorkoutById) and any helper queries use these guards
consistently.
---
Nitpick comments:
In `@app/`(app)/(tabs)/index.tsx:
- Around line 342-354: Extract the repeated complex condition into a named
boolean (e.g., shouldHighlightCard) defined immediately before the card render
using the existing variables workoutInProgress, index, weeklyGoalReached,
isRestDay, and completedTodayWorkoutIds; then replace the inline expression used
in the style array (previously applied to styles.workoutCard) and the duplicated
button mode logic with this new boolean so both places use shouldHighlightCard
consistently and remain a boolean value.
In `@components/WeeklyScheduleDisplay.tsx`:
- Around line 38-44: Wrap the computation of workoutById and scheduleMap in a
useMemo so they are only recomputed when inputs change: compute workoutById =
new Map(workouts.map(...)) and scheduleMap from scheduleEntries inside a single
useMemo with [workouts, scheduleEntries] as dependencies (preserve the current
shapes/types and the fallback "Workout" for missing names); then replace the
existing top-level consts with the memoized values so renders reuse the maps
when workouts and scheduleEntries are unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8f64d2e5-fea2-4ec7-a400-8702d5f83e2d
📒 Files selected for processing (10)
app.config.jsapp/(app)/(tabs)/(plans)/overview.tsxapp/(app)/(tabs)/(stats)/history-details.tsxapp/(app)/(tabs)/index.tsxapp/(app)/(workout)/workout-session.tsxcomponents/WeeklyScheduleDisplay.tsxhooks/useCompletedWorkoutsQuery.tshooks/useDeleteCompletedWorkoutMutation.tsstore/activeWorkoutStore.tsutils/database.ts
💤 Files with no reviewable changes (2)
- app.config.js
- store/activeWorkoutStore.ts
isotronic
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/(app)/(tabs)/(stats)/history-details.tsx (1)
152-158:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse strict ID parsing to prevent deleting the wrong workout.
The
idroute parameter fromuseLocalSearchParams()has typestring | string[] | undefined, which is not strictly validated. Line 152 usesparseInt(String(id), 10), which is permissive: it returns12from inputs like"12abc"or from arrays like["12abc"]. The subsequent guard checks (finite, integer, positive) would all pass, allowing a malformed parameter to target the wrong record for deletion. Use strict parsing with a regex check on the raw string before conversion to ensure only valid numeric IDs proceed to the mutation.Suggested fix
- const parsedId = parseInt(String(id), 10); - if ( - !Number.isFinite(parsedId) || - !Number.isInteger(parsedId) || - parsedId <= 0 - ) - return; + const rawId = Array.isArray(id) ? id[0] : id; + const parsedId = Number(rawId); + const isValidId = + typeof rawId === "string" && + /^\d+$/.test(rawId) && + Number.isInteger(parsedId) && + parsedId > 0; + if (!isValidId) { + Alert.alert("Error", "Invalid workout ID."); + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(app)/(tabs)/(stats)/history-details.tsx around lines 152 - 158, The route param `id` from useLocalSearchParams() is being loosely parsed via parseInt(String(id), 10) (see parsedId) which allows inputs like "12abc" or arrays to slip through; change the guard to first ensure id is a single string (typeof id === 'string'), test it against a strict digits-only regex (e.g. /^\d+$/) and only then call parseInt to produce parsedId, returning early for any non-string, array, or non-matching values so the subsequent Number.isFinite/Integer/positive checks are redundant and cannot act on malformed input.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@app/`(app)/(tabs)/(stats)/history-details.tsx:
- Around line 152-158: The route param `id` from useLocalSearchParams() is being
loosely parsed via parseInt(String(id), 10) (see parsedId) which allows inputs
like "12abc" or arrays to slip through; change the guard to first ensure id is a
single string (typeof id === 'string'), test it against a strict digits-only
regex (e.g. /^\d+$/) and only then call parseInt to produce parsedId, returning
early for any non-string, array, or non-matching values so the subsequent
Number.isFinite/Integer/positive checks are redundant and cannot act on
malformed input.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3a09ce04-0d44-4e53-8c60-95abf79ec9fe
📒 Files selected for processing (4)
app/(app)/(tabs)/(stats)/history-details.tsxapp/(app)/(tabs)/index.tsxcomponents/WeeklyScheduleDisplay.tsxutils/database.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- utils/database.ts
- app/(app)/(tabs)/index.tsx
- components/WeeklyScheduleDisplay.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add deletion support and soft-delete filtering for completed workouts, improve workout session input handling, and surface weekly plan schedules in the UI.
New Features:
Bug Fixes:
Enhancements:
Summary by CodeRabbit
New Features
Improvements