Implemented profile goal setting UI + networking#126
Open
caitlynjin wants to merge 10 commits intomainfrom
Open
Implemented profile goal setting UI + networking#126caitlynjin wants to merge 10 commits intomainfrom
caitlynjin wants to merge 10 commits intomainfrom
Conversation
…itlyn/goal-setting
📝 WalkthroughWalkthroughThis PR introduces a goal-setting modal confirmation flow and migrates workout goal management from Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SetGoalsView
participant SetGoalsViewModel
participant GoalSettingModal
participant API as GraphQL API
participant Logger
User->>SetGoalsView: Adjust slider + tap Save/Next
SetGoalsView->>SetGoalsViewModel: isGoalChangeLocked(lastGoalChange)
SetGoalsViewModel->>SetGoalsViewModel: Parse date & validate 30-day cooldown
alt Goal locked or unsaved changes
SetGoalsViewModel-->>SetGoalsView: Return lock status
SetGoalsView->>GoalSettingModal: Show warning modal
User->>GoalSettingModal: Tap Continue or Cancel
end
alt User confirms
GoalSettingModal->>SetGoalsView: onContinue callback
SetGoalsView->>SetGoalsViewModel: setWorkoutGoal(userId, goal, completion)
SetGoalsViewModel->>API: Publish SetWorkoutGoalsMutation
API-->>SetGoalsViewModel: Success/Failure response
alt Mutation succeeds
SetGoalsViewModel->>Logger: Log success (DEBUG)
SetGoalsViewModel-->>SetGoalsView: completion(.success)
SetGoalsView->>SetGoalsView: Update inSavedState, dismiss modal
else Mutation fails
SetGoalsViewModel->>Logger: Log critical error
SetGoalsViewModel-->>SetGoalsView: completion(.failure)
SetGoalsView->>SetGoalsView: Handle error, keep modal visible
end
else User cancels
GoalSettingModal->>SetGoalsView: onCancel callback
SetGoalsView->>SetGoalsView: Dismiss modal without API call
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Uplift/ViewModels/SetGoalsViewModel.swift`:
- Around line 51-53: The timestamp parse failure currently returns false which
lets the client treat the cooldown as expired; instead make the conservative
choice and prevent unlocking on parse errors by changing the guard fallback to
return true (i.e., treat unparseable lastGoalChange as "still in cooldown").
Update the guard in SetGoalsViewModel where you call formatter.date(from:
lastGoalChange) (the lastChangeDate parsing block) to log the error as you do
and then return true so the 30-day guard is not bypassed.
- Around line 70-84: The receiveValue closure must validate the mutation payload
before reporting success: in the mutationPublisher receiveValue (the closure
parameter often named `result`), unwrap and verify
`result.data?.setWorkoutGoals` exists (following the pattern in ProfileViewModel
where `result.data` is validated) and only call `completion(.success(()))` when
that value is present; otherwise log a clear error via Logger.data and call
`completion(.failure(...))` with an appropriate Error (or map to an existing
network/validation error). Ensure this check is done inside SetGoalsViewModel's
mutation handling so the code doesn't ignore an empty/malformed response.
In `@Uplift/Views/Onboarding/SetGoalsView.swift`:
- Around line 99-101: Add an isSubmitting Bool on the view model and use it to
short-circuit both entry points that trigger network writes: in
GoalSettingModal.onContinue and wherever the Continue/Next buttons call
createUser and setWorkoutGoal, return early if isSubmitting is true; set
isSubmitting = true before starting the async createUser -> setWorkoutGoal chain
and set it back to false in all completion paths (success and failure) and also
ensure viewModel.showWarningModal is only set false after the chain completes
(or guarded by isSubmitting) so taps while the request is in flight are ignored.
- Around line 28-30: The view currently computes the 30-day lock solely from
user?.lastGoalChange (via the isGoalChangeLocked property calling
viewModel.isGoalChangeLocked(lastGoalChange:)), but after a successful save you
only flip local view state (lines ~89-93), leaving the backing User stale;
update the source-of-truth when save succeeds by persisting the new
lastGoalChange (e.g., set user.lastGoalChange = Date()) or by invoking the
viewModel method that refreshes/updates the backing User model so
isGoalChangeLocked will reflect the new cooldown across view dismissals and
reopenings.
- Around line 83-87: The current code in SetGoalsView.swift calls
viewModel.setWorkoutGoal with userId: Int(user.id) ?? 0 which silently uses 0 on
parse failure; change this to fail fast by validating user.id before calling
setWorkoutGoal — e.g., guard that user exists and that Int(user.id) successfully
converts, otherwise log a clear error (including the raw user.id) and return
early; update the call site that uses viewModel.setWorkoutGoal to pass the
validated Int userId instead of a fallback 0.
In `@Uplift/Views/Supporting/GoalSettingModal.swift`:
- Around line 28-32: Update the warning copy in GoalSettingModal so it matches
the view model's 30-day cooldown: replace the string in the Text view that
currently says "1 month" with "30 days" (e.g., "Goals can only be changed again
after 30 days, do you want to continue?") in the GoalSettingModal SwiftUI view
so the UI and the view model cooldown semantics are consistent.
- Around line 47-53: The Back button in GoalSettingModal is wired to
onContinue() which triggers saving and the 30-day lockout; change the Button
action to call the cancel/dismiss handler (e.g., onCancel() or the component's
existing cancel callback) instead of onContinue() so pressing "Back" follows the
cancel path and does not save the new goal; update the Button action from
onContinue() to the cancel method used by this view (replace onContinue() with
onCancel() or the appropriate cancel closure).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ac04267-0043-4f91-b518-72e3fdf470d8
⛔ Files ignored due to path filters (3)
Uplift/Resources/Assets.xcassets/clock_outlined.imageset/Clock-icon-new.pngis excluded by!**/*.pngUplift/Resources/Assets.xcassets/clock_outlined.imageset/Clock-icon-new@2x.pngis excluded by!**/*.pngUplift/Resources/Assets.xcassets/clock_outlined.imageset/Clock-icon-new@3x.pngis excluded by!**/*.png
📒 Files selected for processing (10)
Uplift.xcodeproj/project.pbxprojUplift/Core/UpliftApp.swiftUplift/Resources/Assets.xcassets/clock_outlined.imageset/Contents.jsonUplift/Utils/Constants.swiftUplift/ViewModels/MainViewModel.swiftUplift/ViewModels/ProfileViewModel.swiftUplift/ViewModels/SetGoalsViewModel.swiftUplift/Views/Onboarding/SetGoalsView.swiftUplift/Views/ProfileView.swiftUplift/Views/Supporting/GoalSettingModal.swift
💤 Files with no reviewable changes (1)
- Uplift/ViewModels/MainViewModel.swift
Comment on lines
+28
to
+30
| private var isGoalChangeLocked: Bool { | ||
| viewModel.isGoalChangeLocked(lastGoalChange: user?.lastGoalChange) | ||
| } |
There was a problem hiding this comment.
Update the cooldown source of truth after a successful profile save.
Lines 28-30 and 74-79 derive the lock entirely from the incoming user, but Lines 89-93 only flip local view state. Once this view is dismissed and opened again, the screen will recompute from the stale user.lastGoalChange until the parent refetches, which breaks the 30-day guard in the profile flow. Persist the new lock state locally or refresh the backing User model on success.
Also applies to: 74-79, 89-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Uplift/Views/Onboarding/SetGoalsView.swift` around lines 28 - 30, The view
currently computes the 30-day lock solely from user?.lastGoalChange (via the
isGoalChangeLocked property calling
viewModel.isGoalChangeLocked(lastGoalChange:)), but after a successful save you
only flip local view state (lines ~89-93), leaving the backing User stale;
update the source-of-truth when save succeeds by persisting the new
lastGoalChange (e.g., set user.lastGoalChange = Date()) or by invoking the
viewModel method that refreshes/updates the backing User model so
isGoalChangeLocked will reflect the new cooldown across view dismissals and
reopenings.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
Uplift/Views/Onboarding/SetGoalsView.swift (2)
28-30:⚠️ Potential issue | 🟠 MajorRefresh the cooldown source of truth after a successful profile save.
isGoalChangeLockedalways reads fromuser?.lastGoalChange, but success only toggles local view state. If this screen is dismissed and reopened before the parent refreshesUser, the stale timestamp can make the 30-day lock disappear.Also applies to: 74-79, 89-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Uplift/Views/Onboarding/SetGoalsView.swift` around lines 28 - 30, isGoalChangeLocked reads from user?.lastGoalChange (via viewModel.isGoalChangeLocked(lastGoalChange:)), but successful profile saves only toggle local UI state so the persisted cooldown timestamp can remain stale; on successful profile save/update (the handler that currently mutates local view state) update the source-of-truth lastGoalChange used by this view — e.g., set user?.lastGoalChange (or call the parent refresh/commit method in the view model) to the new timestamp returned from the save so isGoalChangeLocked reflects the updated cooldown immediately; apply the same fix to the other places that rely on lastGoalChange (the handlers referenced by the other blocks around the save flow).
82-103:⚠️ Potential issue | 🟠 MajorKeep the save/create paths single-flight and dismiss the modal only on success.
Line 101 clears
showWarningModalimmediately after startingsetWorkoutGoal, and neither this path nornextButtonblocks repeat taps. A fast double tap can queue duplicate writes, and failures still look like a successful dismissal. Add an in-flight guard, disable both triggers while the request is running, and move the modal dismissal into the.successpath.Also applies to: 275-291, 311-339
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Uplift/Views/Onboarding/SetGoalsView.swift` around lines 82 - 103, The save path currently starts viewModel.setWorkoutGoal and immediately clears viewModel.showWarningModal, allowing duplicate taps and hiding failures; add an "in-flight" guard (e.g., a Bool like isSaving either in this View or the ViewModel) that is checked at the start of the onContinue and nextButton handlers, set it true before calling viewModel.setWorkoutGoal and false on both success and failure, disable the buttons/triggers while isSaving is true, and move the viewModel.showWarningModal = false (modal dismissal) into the .success case where you also set inSavedState and viewModel.currWorkoutGoal; ensure failure keeps the modal visible and logs the error.Uplift/ViewModels/SetGoalsViewModel.swift (1)
70-83:⚠️ Potential issue | 🔴 CriticalDon't treat every emitted value as a successful goal save.
receiveValueignoresresultand always callscompletion(.success(())). If the mutation returns an empty/nullsetWorkoutGoalspayload, the profile flow still flips toSavedand locally starts the 30-day lock without a persisted change.Suggested fix
.sink { response in if case let .failure(error) = response { Logger.data.critical("Error in SetGoalsViewModel.setWorkoutGoal: \(error)") completion(.failure(error)) } - } receiveValue: { _ in + } receiveValue: { result in + guard result.data?.setWorkoutGoals != nil else { + let error = NSError( + domain: "SetGoalsViewModel", + code: 0, + userInfo: [NSLocalizedDescriptionKey: "Missing setWorkoutGoals payload"] + ) + Logger.data.error("SetWorkoutGoals returned no payload for userId \(userId)") + completion(.failure(error)) + return + } `#if` DEBUG Logger.data.log("User id \(userId) has set goal to \(workoutGoal)") `#endif` completion(.success(())) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Uplift/ViewModels/SetGoalsViewModel.swift` around lines 70 - 83, The receiveValue closure in SetGoalsViewModel.setWorkoutGoal currently ignores the mutation result and always returns success; change it to inspect the mutation response payload (the value passed into receiveValue from Network.client.mutationPublisher for SetWorkoutGoalsMutation), verify that the returned setWorkoutGoals (or equivalent field) is non-nil and indicates success, and only call completion(.success(())) when that payload is present/valid; if the payload is nil or indicates failure, log a meaningful error via Logger.data and call completion(.failure(...)) (create/use an appropriate Error for a missing/empty payload) so the profile flow won't flip to Saved on a non-persisted change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Uplift/Views/Onboarding/SetGoalsView.swift`:
- Around line 74-79: Clamp the seeded workout goal when the view appears: in the
SetGoalsView .onAppear block, instead of assigning user.workoutGoal verbatim to
viewModel.sliderWorkoutGoal and viewModel.currWorkoutGoal, coerce the optional
user.workoutGoal into the new valid range 1...7 (use min/max or a clamp helper)
and assign that sanitized Int/Double to viewModel.sliderWorkoutGoal and
viewModel.currWorkoutGoal; keep the existing guard on isOnboarding/user and
retain setting inSavedState = isGoalChangeLocked.
---
Duplicate comments:
In `@Uplift/ViewModels/SetGoalsViewModel.swift`:
- Around line 70-83: The receiveValue closure in
SetGoalsViewModel.setWorkoutGoal currently ignores the mutation result and
always returns success; change it to inspect the mutation response payload (the
value passed into receiveValue from Network.client.mutationPublisher for
SetWorkoutGoalsMutation), verify that the returned setWorkoutGoals (or
equivalent field) is non-nil and indicates success, and only call
completion(.success(())) when that payload is present/valid; if the payload is
nil or indicates failure, log a meaningful error via Logger.data and call
completion(.failure(...)) (create/use an appropriate Error for a missing/empty
payload) so the profile flow won't flip to Saved on a non-persisted change.
In `@Uplift/Views/Onboarding/SetGoalsView.swift`:
- Around line 28-30: isGoalChangeLocked reads from user?.lastGoalChange (via
viewModel.isGoalChangeLocked(lastGoalChange:)), but successful profile saves
only toggle local UI state so the persisted cooldown timestamp can remain stale;
on successful profile save/update (the handler that currently mutates local view
state) update the source-of-truth lastGoalChange used by this view — e.g., set
user?.lastGoalChange (or call the parent refresh/commit method in the view
model) to the new timestamp returned from the save so isGoalChangeLocked
reflects the updated cooldown immediately; apply the same fix to the other
places that rely on lastGoalChange (the handlers referenced by the other blocks
around the save flow).
- Around line 82-103: The save path currently starts viewModel.setWorkoutGoal
and immediately clears viewModel.showWarningModal, allowing duplicate taps and
hiding failures; add an "in-flight" guard (e.g., a Bool like isSaving either in
this View or the ViewModel) that is checked at the start of the onContinue and
nextButton handlers, set it true before calling viewModel.setWorkoutGoal and
false on both success and failure, disable the buttons/triggers while isSaving
is true, and move the viewModel.showWarningModal = false (modal dismissal) into
the .success case where you also set inSavedState and viewModel.currWorkoutGoal;
ensure failure keeps the modal visible and logs the error.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a557073c-c5b8-468d-8637-874a4951ec90
📒 Files selected for processing (5)
Uplift/Services/Networking/AuthorizationInterceptor.swiftUplift/Services/UserSessionManager.swiftUplift/ViewModels/SetGoalsViewModel.swiftUplift/Views/Onboarding/SetGoalsView.swiftUplift/Views/Supporting/GoalSettingModal.swift
✅ Files skipped from review due to trivial changes (1)
- Uplift/Services/UserSessionManager.swift
JiwonJeong414
approved these changes
Apr 5, 2026
Contributor
JiwonJeong414
left a comment
JiwonJeong414
left a comment
There was a problem hiding this comment.
Caitlyn ur my idol 🤩
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
2 participants
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.
Overview
Implemented workout goal setting for the profile page, including new UI components and networking. Major changes include differentiation between onboarding and profile versions of goal setting, and the once every 30 day limit for changing goals.
Changes Made
Goal Setting Logic
isGoalChangeLockedchecks whether the last workout goal change was within 30 days ago, and if so, the workout goal cannot be changed. Date formatting matches what backend gives to us.UI Changes
SetGoalsView,isOnboardingensures that the UI for onboarding vs profile versions of goal setting is different.GoalSettingModal, which pops up a warning after clicking “save” in the profile goal setting view. This is meant to warn users that they can only change their goal every 30 days. After pressing continue, the workout goal automatically saves.Screenshots
goal-setting-profile-edit.mp4
goal-setting-profile-30-days.mp4
Summary by CodeRabbit
New Features
Refactor