Conversation
There was a problem hiding this comment.
Pull request overview
Implements the new Profile page end-to-end (UI + ViewModel + repository) and updates GraphQL schema/queries/models to match the latest backend changes.
Changes:
- Added profile data loading via
ProfileRepository+ProfileViewModeland wired it into the Profile screen UI. - Updated GraphQL schema and
User/Workoutfragments + mutations to align with backend changes (e.g., workoutGoal asInt). - Added/updated supporting UI components (history empty state, weekly progress, progress arc) and small logging/debug improvements in Check-In and login flows.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/drawable/ic_bag.png | Adds an empty-state icon for workout history. |
| app/src/main/java/com/cornellappdev/uplift/util/Functions.kt | Adds Calendar.timeAgoString() helper for history “time ago” text. |
| app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/profile/ProfileViewModel.kt | New Profile VM: loads profile + formats workout history + computes weekly progress state. |
| app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/profile/CheckInViewModel.kt | Logs success/failure of backend workout logging. |
| app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/onboarding/LoginViewModel.kt | Syncs existing user data into DataStore before navigating to Home. |
| app/src/main/java/com/cornellappdev/uplift/ui/screens/profile/ProfileScreen.kt | Reworks Profile screen to use ProfileViewModel state and new navigation callbacks. |
| app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WorkoutSection.kt | Adds a workouts section composable (currently duplicated). |
| app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WorkoutProgressArc.kt | Improves arc rendering for zero/complete goal cases; tweaks labels. |
| app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WeeklyProgressTracker.kt | Adds guard for insufficient day data. |
| app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.kt | Adds “time ago” display and empty state UI for history. |
| app/src/main/java/com/cornellappdev/uplift/ui/components/profile/ProfileHeaderSection.kt | Updates header to display netId and handle long names. |
| app/src/main/java/com/cornellappdev/uplift/ui/MainNavigationWrapper.kt | Wires ProfileScreen into navigation and introduces ProfileViewModel creation in wrapper. |
| app/src/main/java/com/cornellappdev/uplift/data/repositories/UserInfoRepository.kt | Fixes create-user DataStore writes; adds syncUserToDataStore; expands user model mapping. |
| app/src/main/java/com/cornellappdev/uplift/data/repositories/ProfileRepository.kt | New repository fetching user/workouts/weekly days and setting workout goal. |
| app/src/main/java/com/cornellappdev/uplift/data/repositories/CheckInRepository.kt | Improves userId parsing/logging and adds extra error logging for logWorkout. |
| app/src/main/java/com/cornellappdev/uplift/data/models/UserInfo.kt | Expands UserInfo model to include profile-related fields. |
| app/src/main/graphql/schema.graphqls | Updates schema for user streak/goal fields, workout fields, and mutations. |
| app/src/main/graphql/User.graphql | Updates fragments/queries/mutations (e.g., workoutGoal: Int, adds workout gymName). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,10 +1,18 @@ | |||
| package com.cornellappdev.uplift.data.models | |||
| import com.cornellappdev.uplift.type.DateTime | |||
There was a problem hiding this comment.
com.cornellappdev.uplift.type.DateTime is imported but not used in this data class. Remove the unused import to avoid lint warnings and keep the model minimal.
| import com.cornellappdev.uplift.type.DateTime |
| val historyItems = profile.workouts.map { | ||
| HistoryItem( | ||
| gymName = it.gymName, | ||
| time = formatTime.format( | ||
| Instant.ofEpochMilli(it.timestamp) | ||
| ), | ||
| date = formatDate.format( | ||
| Instant.ofEpochMilli(it.timestamp) | ||
| ), |
There was a problem hiding this comment.
HistoryItem is built with both time (formatted as h:mm a) and date formatted as MMMM d, yyyy • h:mm a. In HistoryItemRow the UI renders "$date · $time", which will duplicate the time portion (e.g., "March 29, 2024 • 1:00 PM · 1:00 PM"). Either remove the time from formatDate or stop appending time in the row.
| val checkInUiState = checkInViewModel.collectUiStateValue() | ||
| val confettiUiState = confettiViewModel.collectUiStateValue() | ||
|
|
There was a problem hiding this comment.
checkInViewModel and confettiViewModel are referenced here but are not declared in MainNavigationWrapper (they’re only created later inside the Box when CHECK_IN_FLAG is true). This will not compile; remove these lines or instantiate the view models before collecting their UI state (and avoid duplicating the later declarations).
| val checkInUiState = checkInViewModel.collectUiStateValue() | |
| val confettiUiState = confettiViewModel.collectUiStateValue() |
| import androidx.compose.ui.Alignment | ||
| import androidx.compose.ui.Modifier | ||
| import androidx.compose.ui.focus.focusModifier | ||
| import androidx.compose.ui.graphics.Color | ||
| import androidx.compose.ui.layout.ContentScale |
There was a problem hiding this comment.
import androidx.compose.ui.focus.focusModifier (and some of the other new imports around it) is unused here, and focusModifier is not part of the public Compose API in recent versions—this may fail compilation. Please remove the unused/invalid imports to avoid build errors.
| activeStreak = user.activeStreak ?: 0, | ||
| maxStreak = user.maxStreak ?: 0, |
There was a problem hiding this comment.
In the updated GraphQL schema, activeStreak/maxStreak are now non-null (Int!). After regeneration, these fields will be non-null Kotlin Ints, so using the Elvis operator (user.activeStreak ?: 0) will not compile. Update this mapping to treat them as non-null (and only use null-coalescing for fields that remain nullable, e.g. workoutGoal).
| activeStreak = user.activeStreak ?: 0, | |
| maxStreak = user.maxStreak ?: 0, | |
| activeStreak = user.activeStreak, | |
| maxStreak = user.maxStreak, |
| val profileViewModel: ProfileViewModel = hiltViewModel() | ||
|
|
There was a problem hiding this comment.
ProfileViewModel is being created in MainNavigationWrapper via hiltViewModel(), even though this file’s own comment says future view models should be added in the screen they’re used in. Consider removing this and letting ProfileScreen create its own hiltViewModel() (or instantiate it inside the composable<UpliftRootRoute.Profile> block) so the ViewModel is properly scoped to the Profile destination.
| import android.R.attr.name | ||
| import android.annotation.SuppressLint | ||
| import android.net.Uri | ||
| import androidx.compose.foundation.layout.Arrangement | ||
| import androidx.compose.foundation.layout.Column | ||
| import androidx.compose.foundation.layout.Spacer | ||
| import androidx.compose.foundation.layout.fillMaxSize | ||
| import androidx.compose.foundation.layout.height | ||
| import androidx.compose.foundation.layout.padding | ||
| import androidx.compose.foundation.layout.windowInsetsEndWidth | ||
| import androidx.compose.foundation.rememberScrollState | ||
| import androidx.compose.foundation.verticalScroll |
There was a problem hiding this comment.
There are several unused imports here (e.g. android.R.attr.name, windowInsetsEndWidth, rememberScrollState, verticalScroll, viewModel, ProfileRepository, UserInfoRepository, etc.). Please remove unused imports to keep the file clean and avoid lint/CI failures if unused-imports are enforced.
| import androidx.compose.ui.unit.dp | ||
|
|
||
| @Composable | ||
| private fun WorkoutsSectionContent( | ||
| workoutsCompleted: Int, | ||
| workoutGoal: Int, | ||
| daysOfMonth: List<Int>, | ||
| completedDays: List<Boolean>, | ||
| reminderItems: List<ReminderItem>, | ||
| historyItems: List<HistoryItem>, | ||
| navigateToGoalsSection: () -> Unit, | ||
| navigateToRemindersSection: () -> Unit, | ||
| navigateToHistorySection: () -> Unit | ||
| ) { | ||
| Column( | ||
| modifier = Modifier.fillMaxSize() | ||
| ) { | ||
| GoalsSection( | ||
| workoutsCompleted = workoutsCompleted, | ||
| workoutGoal = workoutGoal, | ||
| daysOfMonth = daysOfMonth, | ||
| completedDays = completedDays, | ||
| onClick = navigateToGoalsSection, | ||
| ) | ||
| Spacer(modifier = Modifier.height(24.dp)) | ||
|
|
||
| HistorySection( | ||
| historyItems = historyItems, | ||
| onClick = navigateToHistorySection, | ||
| modifier = Modifier.weight(1f) | ||
| ) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
This new file duplicates the WorkoutsSectionContent composable that is already defined/used in ProfileScreen.kt, but this version is private and is not referenced anywhere. Consider deleting this file or making it the single shared implementation to avoid dead code and future drift between two copies.
| import androidx.compose.ui.unit.dp | |
| @Composable | |
| private fun WorkoutsSectionContent( | |
| workoutsCompleted: Int, | |
| workoutGoal: Int, | |
| daysOfMonth: List<Int>, | |
| completedDays: List<Boolean>, | |
| reminderItems: List<ReminderItem>, | |
| historyItems: List<HistoryItem>, | |
| navigateToGoalsSection: () -> Unit, | |
| navigateToRemindersSection: () -> Unit, | |
| navigateToHistorySection: () -> Unit | |
| ) { | |
| Column( | |
| modifier = Modifier.fillMaxSize() | |
| ) { | |
| GoalsSection( | |
| workoutsCompleted = workoutsCompleted, | |
| workoutGoal = workoutGoal, | |
| daysOfMonth = daysOfMonth, | |
| completedDays = completedDays, | |
| onClick = navigateToGoalsSection, | |
| ) | |
| Spacer(modifier = Modifier.height(24.dp)) | |
| HistorySection( | |
| historyItems = historyItems, | |
| onClick = navigateToHistorySection, | |
| modifier = Modifier.weight(1f) | |
| ) | |
| } | |
| } | |
| import androidx.compose.ui.unit.dp |
| import android.R.attr.fontFamily | ||
| import android.R.attr.fontWeight |
There was a problem hiding this comment.
android.R.attr.fontFamily and android.R.attr.fontWeight are imported but not used. Please remove these unused imports (and prefer Compose’s FontFamily/FontWeight types when needed) to keep the file clean.
| import android.R.attr.fontFamily | |
| import android.R.attr.fontWeight |
| val ok = response.data?.logWorkout?.workoutFields != null && !response.hasErrors() | ||
| if (!ok) { | ||
| Log.e("CheckInRepository", "LogWorkout errors=${response.errors}") | ||
| Log.e("CheckInRepository", "LogWorkout response data=${response.data}") |
There was a problem hiding this comment.
Logging the full GraphQL response.data on failures can leak user/workout data into logs (which may be captured on devices or in crash reports). Prefer logging only the error messages/codes, or gate verbose response logging behind a debug-only flag.
| Log.e("CheckInRepository", "LogWorkout response data=${response.data}") |
melissavelasquezz
force-pushed
the
melissa/workout-history-ui
branch
from
10f1633 to
361c307
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| diffDays in 2..6 -> "$diffDays days ago" | ||
|
|
||
| diffWeeks == 1L -> "1 week ago" | ||
| diffWeeks in 2..4 -> "$diffWeeks weeks ago" | ||
|
|
||
| diffMonths == 1L -> "1 month ago" | ||
| diffMonths in 2..11 -> "$diffMonths months ago" |
There was a problem hiding this comment.
timeAgoString() uses diffDays in 2..6, diffWeeks in 2..4, and diffMonths in 2..11 but diffDays/diffWeeks/diffMonths are Long. This won’t compile because those ranges are IntRange. Use 2L..6L / 2L..4L / 2L..11L (or convert the diffs to Int safely) so the when branches type-check.
| diffDays in 2..6 -> "$diffDays days ago" | |
| diffWeeks == 1L -> "1 week ago" | |
| diffWeeks in 2..4 -> "$diffWeeks weeks ago" | |
| diffMonths == 1L -> "1 month ago" | |
| diffMonths in 2..11 -> "$diffMonths months ago" | |
| diffDays in 2L..6L -> "$diffDays days ago" | |
| diffWeeks == 1L -> "1 week ago" | |
| diffWeeks in 2L..4L -> "$diffWeeks weeks ago" | |
| diffMonths == 1L -> "1 month ago" | |
| diffMonths in 2L..11L -> "$diffMonths months ago" |
| val confettiViewModel: ConfettiViewModel = hiltViewModel() | ||
| val checkInViewModel: CheckInViewModel = hiltViewModel() |
There was a problem hiding this comment.
The comment at the top of MainNavigationWrapper asks that new view models be created in the screens where they’re used, but this adds ConfettiViewModel/CheckInViewModel here. These two instances are also unused because new instances are created again inside the CHECK_IN_FLAG block, which will trigger unused-variable lint warnings. Remove the top-level hiltViewModel() calls and keep the scoped ones (or pass the single instances down so you don’t create duplicates).
| val confettiViewModel: ConfettiViewModel = hiltViewModel() | |
| val checkInViewModel: CheckInViewModel = hiltViewModel() |
There was a problem hiding this comment.
Might change after check in flag is removed
| gymDays = uiState.totalGymDays, | ||
| streaks = uiState.activeStreak, | ||
| profilePictureUri = uiState.profileImage?.let { Uri.parse(it) }, | ||
| onPhotoSelected = {}, | ||
| netId = uiState.netId |
There was a problem hiding this comment.
profileImage is coming from encodedImage in the backend model; parsing it as a Uri and feeding it into PhotoPicker/Coil likely won’t render unless the backend value is a real URI (e.g., content://, file://, or https://). If this is base64/encoded content (as the name suggests), decode it to an ImageBitmap/ByteArray for display or change PhotoPicker to accept a raw model (e.g., String/data URI) rather than forcing a Uri.parse(...).
| if (daysOfMonth.size < daysOfWeek.size) { | ||
| return | ||
| } |
There was a problem hiding this comment.
WeeklyProgressTracker returns early when daysOfMonth.size < daysOfWeek.size, which means the entire tracker silently disappears instead of rendering partial/padded data. Consider padding daysOfMonth the same way completedDays is padded (or showing a fallback UI) so the component always renders predictably.
There was a problem hiding this comment.
^^ Maybe ask design for how we should handle this edge case?
| if (progressAngle > 0f) { | ||
| if (isComplete) { | ||
| drawArc( | ||
| brush = gradientBrush, | ||
| startAngle = startAngle, | ||
| sweepAngle = progressAngle, | ||
| useCenter = false, | ||
| topLeft = topLeft, | ||
| size = arcSize, | ||
| style = Stroke(width = strokeWidth, cap = StrokeCap.Round) | ||
| ) | ||
| } else { | ||
| drawArc( | ||
| color = PRIMARY_YELLOW, | ||
| startAngle = startAngle, | ||
| sweepAngle = progressAngle, | ||
| useCenter = false, | ||
| topLeft = topLeft, | ||
| size = arcSize, | ||
| style = Stroke(width = strokeWidth, cap = StrokeCap.Round) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
After switching ProgressArc to draw the arc/circle directly, the private helpers drawProgressArc(...) and drawArcSliderOuterCircle(...) appear to no longer be used anywhere in this file. Removing them (or reusing them from the new implementation) will avoid dead code and potential lint warnings.
| name: String, | ||
| gymDays: Int, | ||
| streaks: Int, | ||
| badges: Int | ||
| netID: String, | ||
| modifier: Modifier = Modifier |
There was a problem hiding this comment.
ProfileHeaderInfoDisplay uses the parameter name netID, while the rest of the codebase (and the public ProfileHeaderSection API) uses netId. Renaming this parameter to netId would keep casing consistent and avoid confusion when wiring props through.
| Column( | ||
| modifier = Modifier.fillMaxSize() | ||
| ) { |
There was a problem hiding this comment.
WorkoutsSectionContent uses Column(modifier = Modifier.fillMaxSize()) but it’s placed inside the main ProfileScreen Column without a weight. With the outer screen no longer scrollable, this can cause the workouts section to measure at full screen height in addition to the header, leading to clipped/overflowing content. Consider removing fillMaxSize() here and/or giving WorkoutsSectionContent a Modifier.weight(1f) from the parent so it only consumes the remaining space.
| Column( | |
| modifier = Modifier.fillMaxSize() | |
| ) { | |
| Column { |
app/src/main/res/drawable/ic_bag.png
Outdated
There was a problem hiding this comment.
Is it possible to export as svg instead?
| } | ||
| composable<UpliftRootRoute.Profile> { | ||
| ProfileScreen() | ||
| ProfileScreen(toSettings = {}, toGoals = {}, toHistory = {}) |
There was a problem hiding this comment.
We might be able to make use of the root navigation repository for navigation in the vm instead of passing from here
| suspend fun logWorkoutFromCheckIn(gymId: Int): Boolean { | ||
| val userId = userInfoRepository.getUserIdFromDataStore()?.toIntOrNull() ?: return false | ||
| val userIdString = userInfoRepository.getUserIdFromDataStore() | ||
| val userId = userIdString?.toIntOrNull() |
There was a problem hiding this comment.
Nit: Not sure if you need to separate into two variables if only used once
| import javax.inject.Inject | ||
| import javax.inject.Singleton | ||
|
|
||
| data class ProfileData( |
There was a problem hiding this comment.
For the data classes, it might be good to separate out into the models folder
| private val apolloClient: ApolloClient | ||
| ) { | ||
| suspend fun getProfile(): Result<ProfileData> { | ||
| return try{ |
There was a problem hiding this comment.
I recommend runCatching over try catch
| } | ||
|
|
||
| suspend fun setWorkoutGoal(userId: Int, goal: Int): Result<Unit> { | ||
| return try { |
There was a problem hiding this comment.
recommend runCatching here
| ) { | ||
| suspend fun getProfile(): Result<ProfileData> { | ||
| return try{ | ||
| val netId = userInfoRepository.getNetIdFromDataStore() |
There was a problem hiding this comment.
Nit: For several of the function calls or apollo queries, it might be good to separate it out from this function for better separation and clarity
| Log.e("ProfileRepo", "Workout query errors: ${workoutResponse.errors}") | ||
| } | ||
|
|
||
| val workouts = if (workoutResponse.hasErrors()) { |
There was a problem hiding this comment.
a couple of the hasErrors in this function only log or just set to empty list, but we should probably be throwing exceptions and handling them in the vm
| val workoutDomain = workouts.map { | ||
| WorkoutDomain( | ||
| gymName = it.workoutFields.gymName, | ||
| timestamp = Instant.parse(it.workoutFields.workoutTime.toString()).toEpochMilli() |
There was a problem hiding this comment.
do we need. to convert workout time to string before the conversion or can we do this directly?
| ).execute() | ||
|
|
||
| if (userResponse.hasErrors()) { | ||
| Log.e("ProfileRepo", "User query errors: ${userResponse.errors}") |
There was a problem hiding this comment.
Nit: for "ProfileRepo" you can create a private const val TAG
| .execute() | ||
|
|
||
| if (response.hasErrors()) { | ||
| Result.failure(Exception("Goal update failed")) |
There was a problem hiding this comment.
Nit: for the exceptions like this one and the other ones in the file, it's fine how you have it, but if it is a specific one, you can probably create exception classes to better handle them specifically instead of catching general exceptions and relying on the message
| val userId = user.id.toIntOrNull() | ||
| ?: return Result.failure(IllegalStateException("Invalid user ID: ${user.id}")) | ||
|
|
||
| val workoutResponse = apolloClient |
There was a problem hiding this comment.
Some of these queries can be run in parallel to save time eg. Workouts and Weekly
| return try { | ||
| val user = getUserByNetId(netId) ?: return false | ||
|
|
||
| storeId(user.id) |
There was a problem hiding this comment.
You can combine into a singular disk write instead of multiple disk writes (eg. multiple datastore.edit calls instead of one)
| storeUsername(name) | ||
| storeEmail(email) | ||
|
|
||
| val createdUser = response.data?.createUser?.userFields ?: return false |
There was a problem hiding this comment.
For this file in create user, you might want to work w/ preston or check his PR branch out just to ensure there is no merge conflicts or overlap here
| painter = painterResource(id = R.drawable.ic_bag), | ||
| contentDescription = null, | ||
| modifier = Modifier | ||
| .width(64.99967.dp) |
There was a problem hiding this comment.
Very specific values lol, can we round to the nearest int or an even num?
| HistoryList(historyItems) | ||
| } | ||
| } else { | ||
| Box( |
| SectionTitleText("My Workout History", onClick) | ||
| Spacer(modifier = Modifier.height(12.dp)) | ||
| if (historyItems.isNotEmpty()) { | ||
| Column( |
There was a problem hiding this comment.
Is this Column needed or could you just apply the styling to the history list composable itself?
| val time = historyItem.time | ||
| val dayOfWeek = historyItem.dayOfWeek | ||
| val date = historyItem.date | ||
| val calendar = Calendar.getInstance().apply { |
There was a problem hiding this comment.
Not too ideal to recompute every time it recomposes, so could use remember or format in the VM
|
|
||
| Box(modifier = Modifier.fillMaxWidth()) { | ||
| ConnectingLines(daysOfWeek, lastCompletedIndex) | ||
|
|
There was a problem hiding this comment.
Nit: we could remove space
| .coerceIn(0f, 1f) | ||
| } | ||
| // Setup animation | ||
| val animatedProgress = remember { Animatable(0f) } |
There was a problem hiding this comment.
Might need to add progress to remember key param to animate from previous progress
There was a problem hiding this comment.
if drawProgressArc or drawArcSliderOuterCircle will not be used in the future, we can get rid of them
| // Calculate progress percentage | ||
| val progress = (workoutsCompleted.toFloat() / workoutGoal.toFloat()).coerceIn(0f, 1f) | ||
| val isZero = workoutsCompleted <= 0 || workoutGoal <= 0 | ||
| val isComplete = workoutGoal > 0 && workoutsCompleted >= workoutGoal |
There was a problem hiding this comment.
can simplify to workoutGoal in 1..workoutsCompleted
| HistoryItem("Teagle Down", "12:00 PM", "Fri", "March 29, 2024"), | ||
| HistoryItem("Helen Newman", "10:00 AM", "Fri", "March 29, 2024"), | ||
| ) | ||
| fun ProfileScreen( |
There was a problem hiding this comment.
We could still have a preview if we extract the UI screen content w/o the vm into another composable and called it in profile screen while passing in whatever states or functions it needs from the vm
| ) | ||
| fun ProfileScreen( | ||
| viewModel: ProfileViewModel = hiltViewModel(), | ||
| toSettings:() -> Unit, |
There was a problem hiding this comment.
Might not need these nav functions here if we use rootNavigationRepository in the vm
| val uiState by viewModel.uiStateFlow.collectAsState() | ||
|
|
||
| val scrollState = rememberScrollState() | ||
| LaunchedEffect(Unit) { |
There was a problem hiding this comment.
Not sure if we should trigger the reload here. Maybe we can put it in the init of the vm instead
| val result = profileRepository.getProfile() | ||
|
|
||
| if (result.isSuccess) { | ||
| val profile = result.getOrNull()!! |
There was a problem hiding this comment.
probably better to not use the !!: val profile = result.getOrNull() ?: return@launch
| HistoryItem( | ||
| gymName = it.gymName, | ||
| time = formatTime.format( | ||
| Instant.ofEpochMilli(it.timestamp) |
There was a problem hiding this comment.
this instant is reused a couple of times, so we can make a variable to share
|
|
||
|
|
||
| fun updateWorkoutGoal(goal: Int) = viewModelScope.launch { | ||
| val userId = userInfoRepository.getUserIdFromDataStore()?.toIntOrNull() ?: return@launch |
There was a problem hiding this comment.
If we only need userId from userInfoRepository, it might be better to just change profile repository setgoal function to just take a goal and get the id in the data layer to avoid injecting another repo to this vm
| name = uiState.name, | ||
| gymDays = uiState.totalGymDays, | ||
| streaks = uiState.activeStreak, | ||
| profilePictureUri = uiState.profileImage?.let { Uri.parse(it) }, |
There was a problem hiding this comment.
probably shouldn't do parsing in the UI and let vm or data layer handle the conversion and just take in the Uri
# Conflicts: # app/src/main/java/com/cornellappdev/uplift/data/repositories/UserInfoRepository.kt
# Conflicts: # app/src/main/java/com/cornellappdev/uplift/ui/MainNavigationWrapper.kt
melissavelasquezz
force-pushed
the
melissa/workout-history-ui
branch
from
361c307 to
80817e7
Compare
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive profile feature with user data persistence, workout history tracking, and goal management. Changes span GraphQL schema updates, new data models and repositories, ViewModel implementation with date/week computations, UI component refactoring, and navigation adjustments. The onboarding flow now syncs user data to the local store. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ProfileScreen
participant ProfileViewModel
participant ProfileRepository
participant UserInfoRepository
participant ApolloClient
participant DataStore
User->>ProfileScreen: View Profile
ProfileScreen->>ProfileViewModel: reload()
activate ProfileViewModel
ProfileViewModel->>ProfileRepository: getProfile()
activate ProfileRepository
ProfileRepository->>UserInfoRepository: Obtain netId/userId
activate UserInfoRepository
UserInfoRepository->>DataStore: Retrieve stored credentials
DataStore-->>UserInfoRepository: userId, netId
deactivate UserInfoRepository
ProfileRepository->>ApolloClient: Query user by netId
activate ApolloClient
ApolloClient-->>ProfileRepository: User data + workoutHistory
deactivate ApolloClient
ProfileRepository->>ProfileRepository: Transform workouts to WorkoutDomain
ProfileRepository-->>ProfileViewModel: ProfileData result
deactivate ProfileRepository
ProfileViewModel->>ProfileViewModel: Format dates/times<br/>Compute week metrics<br/>Build historyItems
ProfileViewModel-->>ProfileScreen: Update ProfileUiState
deactivate ProfileViewModel
ProfileScreen-->>User: Display profile + history
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
📝 Coding Plan
Comment Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.kt (1)
76-81:⚠️ Potential issue | 🟡 MinorBug: Divider shown after last visible item when list has more than 5 items.
The iteration uses
historyItems.take(5)but the divider condition checks against the originalhistoryItems.size. When the list has >5 items, the divider incorrectly appears after the 5th (last visible) item.🐛 Proposed fix
private fun HistoryList( historyItems: List<HistoryItem>, modifier: Modifier = Modifier ) { + val displayedItems = historyItems.take(5) Column(modifier = modifier) { - historyItems.take(5).forEachIndexed { index, historyItem -> + displayedItems.forEachIndexed { index, historyItem -> HistoryItemRow(historyItem = historyItem) - if (index != historyItems.size - 1) { + if (index != displayedItems.size - 1) { HorizontalDivider(color = GRAY01) } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.kt` around lines 76 - 81, The divider logic compares index against historyItems.size while iterating historyItems.take(5), causing a divider to render after the 5th visible item when the source list is longer; update the condition to compare index to the taken list's last index (e.g., use historyItems.take(5).size or capture the sublist into a val like visibleHistory = historyItems.take(5) and iterate visibleHistory.forEachIndexed, then only render HorizontalDivider when index != visibleHistory.size - 1) so the divider is not shown after the last visible HistoryItemRow.
🧹 Nitpick comments (10)
app/src/main/java/com/cornellappdev/uplift/ui/components/profile/ProfileHeaderSection.kt (1)
97-100: Remove commented-out badges block before merge.Line 97–100 leaves dead UI code in-place. Please delete it (or track follow-up in an issue) to keep this component clean.
🧹 Suggested cleanup
-// ProfileHeaderInfo( -// label = "Badges", -// amount = badges -// )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/ProfileHeaderSection.kt` around lines 97 - 100, Remove the dead commented-out UI block for ProfileHeaderInfo (the three commented lines referencing label = "Badges" and amount = badges) from ProfileHeaderSection.kt; locate the commented block around the ProfileHeaderInfo usage and delete those lines (or convert to a tracked TODO/issue if badges are intended later) so the component contains no leftover commented UI code.app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WorkoutProgressArc.kt (2)
57-63: Consider reusingisZerofor progress calculation.The
whenchecks duplicate the conditions already captured byisZero. This could be simplified:♻️ Suggested simplification
- val progress = when { - workoutGoal <= 0 -> 0f - workoutsCompleted <= 0 -> 0f - else -> (workoutsCompleted.toFloat() / workoutGoal.toFloat()) - .coerceIn(0f, 1f) - } + val progress = if (isZero) 0f + else (workoutsCompleted.toFloat() / workoutGoal.toFloat()).coerceIn(0f, 1f)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WorkoutProgressArc.kt` around lines 57 - 63, Replace the duplicated zero checks in the progress calculation by reusing the existing isZero boolean: compute progress as 0f if isZero is true, otherwise compute (workoutsCompleted.toFloat() / workoutGoal.toFloat()).coerceIn(0f, 1f); update the code that assigns progress (the val progress in WorkoutProgressArc.kt) to reference isZero instead of re-checking workoutGoal and workoutsCompleted.
94-97: Minor style nits: remove semicolons and extract gradient colors.
- Trailing semicolons on lines 94-95 are not idiomatic Kotlin.
- The gradient colors
(0xFFFCF4A1, 0xFFFDB041, 0xFFFE8F13)appear here and again at line 230-232. Consider extracting to a shared constant for consistency and easier maintenance.♻️ Extract gradient colors
// At file level or in a companion object private val PROGRESS_GRADIENT_COLORS = listOf( Color(0xFFFCF4A1), Color(0xFFFDB041), Color(0xFFFE8F13) )Then use:
val gradientBrush = Brush.horizontalGradient(PROGRESS_GRADIENT_COLORS) // and Brush.linearGradient(colors = PROGRESS_GRADIENT_COLORS)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WorkoutProgressArc.kt` around lines 94 - 97, Remove the unnecessary trailing semicolons on the startAngle and maxSweepAngle declarations and extract the repeated gradient color list into a single shared constant (e.g., PROGRESS_GRADIENT_COLORS) so both gradientBrush and the other Brush.linearGradient usage reuse it; update gradientBrush (and the other occurrences) to call Brush.horizontalGradient or Brush.linearGradient with that constant to keep colors consistent and maintainable while leaving startAngle and maxSweepAngle as plain Kotlin vals without semicolons.app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.kt (2)
32-33: Unused import.
timeAgoStringis imported but not used—theagovalue is passed in pre-computed viaHistoryItem.🧹 Remove unused import
-import com.cornellappdev.uplift.util.timeAgoString🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.kt` around lines 32 - 33, The import timeAgoString in HistorySection.kt is unused because the ago value is already computed on the HistoryItem and passed into the UI; remove the unused import to clean up imports (delete the line importing timeAgoString) and keep the existing usage of HistoryItem. Ensure no other references to timeAgoString remain in the file before committing.
21-21: Unused and deprecated import.
focusModifieris imported but not used in this file, and it's deprecated in recent Compose versions.🧹 Remove unused import
-import androidx.compose.ui.focus.focusModifier🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.kt` at line 21, Remove the unused and deprecated import focusModifier from HistorySection.kt; locate the import statement "import androidx.compose.ui.focus.focusModifier" at the top of the file and delete it, then rebuild to ensure no references to focusModifier remain (if any UI focus behavior is required, replace usages with the current Compose focus APIs such as Modifier.focusRequester/Modifier.onFocusChanged).app/src/main/java/com/cornellappdev/uplift/ui/screens/profile/ProfileScreen.kt (1)
31-31: Unused import.The
viewModelimport fromandroidx.lifecycle.viewmodel.composeappears to be unused sincehiltViewModel()is used on line 47.♻️ Proposed fix
-import androidx.lifecycle.viewmodel.compose.viewModel🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/screens/profile/ProfileScreen.kt` at line 31, Remove the unused import androidx.lifecycle.viewmodel.compose.viewModel from ProfileScreen.kt; locate the import line near the top (the one referencing viewModel) and delete it since the composable uses hiltViewModel() (see hiltViewModel usage in the ProfileScreen composable) to avoid an unused import warning.app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/onboarding/LoginViewModel.kt (1)
45-53: Consider retry or graceful degradation on sync failure.When
syncUserToDataStorefails (line 50-51), the user is signed out despite successful Firebase authentication. This could frustrate users if the failure is due to transient network issues. Consider:
- Retrying the sync operation
- Navigating to Home with cached/partial data
- Showing an error toast without signing out
The current approach is safe but may be overly aggressive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/onboarding/LoginViewModel.kt` around lines 45 - 53, The current handling in LoginViewModel when userInfoRepository.syncUserToDataStore(netId) returns false immediately signs the user out; instead update the logic in the block that checks userInfoRepository.hasUser(netId) to (1) attempt a small retry loop (e.g., 2-3 attempts with short backoff) around syncUserToDataStore(netId) inside LoginViewModel, (2) if retries still fail, avoid calling userInfoRepository.signOut() and instead navigate to Home via rootNavigationRepository.navigate(UpliftRootRoute.Home) while showing a transient error/toast indicating partial sync, and (3) optionally fall back to cached/partial data from userInfoRepository (use existing getters) and schedule a background sync; modify the branch that currently calls userInfoRepository.signOut() accordingly so signOut is only used for unrecoverable errors.app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/profile/ProfileViewModel.kt (2)
6-6: Remove unused import.
coil.util.CoilUtils.resultis imported but not used in this file.🧹 Proposed fix
import androidx.lifecycle.viewModelScope -import coil.util.CoilUtils.result import com.cornellappdev.uplift.data.repositories.ProfileRepository🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/profile/ProfileViewModel.kt` at line 6, Remove the unused import "coil.util.CoilUtils.result" from ProfileViewModel.kt; locate the import statement importing CoilUtils.result at the top of the file (in the ProfileViewModel class file) and delete that import line so there are no unused CoilUtils symbols left.
155-158: Remove unused formatter.
formatDayOfWeekis declared but never used in this file.🧹 Proposed fix
private val formatDate = DateTimeFormatter .ofPattern("MMMM d, yyyy") .withLocale(Locale.US) .withZone(ZoneId.systemDefault()) - - private val formatDayOfWeek = DateTimeFormatter - .ofPattern("EEE") - .withLocale(Locale.US) - .withZone(ZoneId.systemDefault()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/profile/ProfileViewModel.kt` around lines 155 - 158, Remove the unused DateTimeFormatter declared as formatDayOfWeek in ProfileViewModel; locate the private val formatDayOfWeek = DateTimeFormatter.ofPattern("EEE").withLocale(Locale.US).withZone(ZoneId.systemDefault()) and delete this declaration (or, if it was intended to be used, replace its usage where appropriate), leaving no unused imports or references to formatDayOfWeek in ProfileViewModel.app/src/main/java/com/cornellappdev/uplift/data/models/ProfileData.kt (1)
3-3: Remove unused import.
android.net.Uriis imported but not used in this data class.🧹 Proposed fix
package com.cornellappdev.uplift.data.models -import android.net.Uri - data class ProfileData(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/cornellappdev/uplift/data/models/ProfileData.kt` at line 3, The import android.net.Uri at the top of ProfileData.kt is unused; remove that import line so the file only imports what ProfileData uses (clean up unused imports in the ProfileData data class file).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/cornellappdev/uplift/data/models/UserInfo.kt`:
- Around line 11-16: In UserInfo.kt update the nullability to match the GraphQL
schema by changing the activeStreak and maxStreak properties from nullable Int?
to non-nullable Int; locate the UserInfo class (fields like encodedImage,
activeStreak, maxStreak, streakStart, workoutGoal, totalGymDays) and remove the
question marks on activeStreak and maxStreak so their types are Int not Int?.
In
`@app/src/main/java/com/cornellappdev/uplift/data/repositories/AuthInterceptor.kt`:
- Line 14: Remove the development debug log in AuthInterceptor that prints
authentication presence; locate the android.util.Log.d("AuthInterceptor", ...)
call inside the AuthInterceptor class/method and either delete it or wrap it
with a debug-only guard (e.g., check BuildConfig.DEBUG) so it is not executed in
release builds; ensure no other calls leak auth state in production and keep
logging minimal and safe for release.
In
`@app/src/main/java/com/cornellappdev/uplift/data/repositories/ProfileRepository.kt`:
- Around line 47-53: The mapping of workoutFields.workoutTime into
Instant.parse(...) is unsafe because Apollo's DateTime scalar isn't configured
and may deserialize as Any; update your Apollo config in app/build.gradle to map
the DateTime scalar (e.g., use mapScalarToKotlinString("DateTime") for a quick
fix or mapScalar("DateTime", "kotlinx.datetime.Instant",
"com.apollographql.adapters.InstantAdapter") for proper typing), then change the
ProfileRepository mapping that builds workoutDomain (reference: workoutDomain,
WorkoutDomain, workoutFields.workoutTime, Instant.parse) to use the correctly
typed value provided by the scalar mapping instead of calling toString() and
parsing at runtime.
In
`@app/src/main/java/com/cornellappdev/uplift/data/repositories/UserInfoRepository.kt`:
- Around line 144-164: The syncUserToDataStore function currently passes a
hardcoded skip = false into storeUserFields, which can overwrite a user's real
onboarding preference; update syncUserToDataStore to derive skip from the
fetched user object (e.g., use a boolean field like user.skippedGoalSetup or
user.skippedGoal if present) and pass that value to storeUserFields(id =
user.id, username = user.name, netId = user.netId, email = user.email, skip =
user.<skipField>, goal = user.workoutGoal ?: 0); if the user model lacks such a
field, call the backend to retrieve the preference before storing or explicitly
document the intentional default behavior.
In
`@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WeeklyProgressTracker.kt`:
- Around line 56-61: The bug is that completedDays can contain entries beyond
the visible week so lastCompletedIndex and rendering use out-of-range trues; fix
by clamping/trimming completedDays to at most daysOfWeek.size before creating
paddedCompletedDays and before computing lastCompletedIndex — e.g., derive a
local trimmedCompletedDays = completedDays.take(daysOfWeek.size) (or slice to
daysOfWeek.size) then build paddedCompletedDays from trimmedCompletedDays and
compute lastCompletedIndex from trimmedCompletedDays/paddedCompletedDays; update
references to use these trimmed values in WeeklyProgressTracker rendering logic.
In
`@app/src/main/java/com/cornellappdev/uplift/ui/screens/profile/ProfileScreen.kt`:
- Around line 82-84: The onPhotoSelected handler passed to the profile picture
component is currently a no-op (onPhotoSelected = {}), so photo selection does
nothing; update the handler in ProfileScreen.kt to either wire it to the proper
update function (e.g., call the ViewModel's upload/update method or a function
that updates uiState.profileImage) or, if left intentionally unimplemented,
replace the empty lambda with a TODO/log statement (e.g., a call to Timber.d or
a TODO comment) to make the intent explicit and help future implementers locate
the callback.
In `@app/src/main/java/com/cornellappdev/uplift/util/Functions.kt`:
- Around line 203-219: The when-block returning human-readable age has a gap for
~331–364 days because diffMonths can be 12 while diffYears is still 0, causing
the else -> "Today" to hit; update the month/year checks in that when expression
(which uses diffDays, diffWeeks, diffMonths, diffYears) so that months cover
2L..12L (or add an explicit case diffMonths >= 12L -> "1 year ago") and make the
year check use diffYears >= 1L (or keep diffYears == 1L and diffYears > 1L but
ensure months don't exclude 12), ensuring no 12-month value falls through to the
else branch.
---
Outside diff comments:
In
`@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.kt`:
- Around line 76-81: The divider logic compares index against historyItems.size
while iterating historyItems.take(5), causing a divider to render after the 5th
visible item when the source list is longer; update the condition to compare
index to the taken list's last index (e.g., use historyItems.take(5).size or
capture the sublist into a val like visibleHistory = historyItems.take(5) and
iterate visibleHistory.forEachIndexed, then only render HorizontalDivider when
index != visibleHistory.size - 1) so the divider is not shown after the last
visible HistoryItemRow.
---
Nitpick comments:
In `@app/src/main/java/com/cornellappdev/uplift/data/models/ProfileData.kt`:
- Line 3: The import android.net.Uri at the top of ProfileData.kt is unused;
remove that import line so the file only imports what ProfileData uses (clean up
unused imports in the ProfileData data class file).
In
`@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/ProfileHeaderSection.kt`:
- Around line 97-100: Remove the dead commented-out UI block for
ProfileHeaderInfo (the three commented lines referencing label = "Badges" and
amount = badges) from ProfileHeaderSection.kt; locate the commented block around
the ProfileHeaderInfo usage and delete those lines (or convert to a tracked
TODO/issue if badges are intended later) so the component contains no leftover
commented UI code.
In
`@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.kt`:
- Around line 32-33: The import timeAgoString in HistorySection.kt is unused
because the ago value is already computed on the HistoryItem and passed into the
UI; remove the unused import to clean up imports (delete the line importing
timeAgoString) and keep the existing usage of HistoryItem. Ensure no other
references to timeAgoString remain in the file before committing.
- Line 21: Remove the unused and deprecated import focusModifier from
HistorySection.kt; locate the import statement "import
androidx.compose.ui.focus.focusModifier" at the top of the file and delete it,
then rebuild to ensure no references to focusModifier remain (if any UI focus
behavior is required, replace usages with the current Compose focus APIs such as
Modifier.focusRequester/Modifier.onFocusChanged).
In
`@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WorkoutProgressArc.kt`:
- Around line 57-63: Replace the duplicated zero checks in the progress
calculation by reusing the existing isZero boolean: compute progress as 0f if
isZero is true, otherwise compute (workoutsCompleted.toFloat() /
workoutGoal.toFloat()).coerceIn(0f, 1f); update the code that assigns progress
(the val progress in WorkoutProgressArc.kt) to reference isZero instead of
re-checking workoutGoal and workoutsCompleted.
- Around line 94-97: Remove the unnecessary trailing semicolons on the
startAngle and maxSweepAngle declarations and extract the repeated gradient
color list into a single shared constant (e.g., PROGRESS_GRADIENT_COLORS) so
both gradientBrush and the other Brush.linearGradient usage reuse it; update
gradientBrush (and the other occurrences) to call Brush.horizontalGradient or
Brush.linearGradient with that constant to keep colors consistent and
maintainable while leaving startAngle and maxSweepAngle as plain Kotlin vals
without semicolons.
In
`@app/src/main/java/com/cornellappdev/uplift/ui/screens/profile/ProfileScreen.kt`:
- Line 31: Remove the unused import
androidx.lifecycle.viewmodel.compose.viewModel from ProfileScreen.kt; locate the
import line near the top (the one referencing viewModel) and delete it since the
composable uses hiltViewModel() (see hiltViewModel usage in the ProfileScreen
composable) to avoid an unused import warning.
In
`@app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/onboarding/LoginViewModel.kt`:
- Around line 45-53: The current handling in LoginViewModel when
userInfoRepository.syncUserToDataStore(netId) returns false immediately signs
the user out; instead update the logic in the block that checks
userInfoRepository.hasUser(netId) to (1) attempt a small retry loop (e.g., 2-3
attempts with short backoff) around syncUserToDataStore(netId) inside
LoginViewModel, (2) if retries still fail, avoid calling
userInfoRepository.signOut() and instead navigate to Home via
rootNavigationRepository.navigate(UpliftRootRoute.Home) while showing a
transient error/toast indicating partial sync, and (3) optionally fall back to
cached/partial data from userInfoRepository (use existing getters) and schedule
a background sync; modify the branch that currently calls
userInfoRepository.signOut() accordingly so signOut is only used for
unrecoverable errors.
In
`@app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/profile/ProfileViewModel.kt`:
- Line 6: Remove the unused import "coil.util.CoilUtils.result" from
ProfileViewModel.kt; locate the import statement importing CoilUtils.result at
the top of the file (in the ProfileViewModel class file) and delete that import
line so there are no unused CoilUtils symbols left.
- Around line 155-158: Remove the unused DateTimeFormatter declared as
formatDayOfWeek in ProfileViewModel; locate the private val formatDayOfWeek =
DateTimeFormatter.ofPattern("EEE").withLocale(Locale.US).withZone(ZoneId.systemDefault())
and delete this declaration (or, if it was intended to be used, replace its
usage where appropriate), leaving no unused imports or references to
formatDayOfWeek in ProfileViewModel.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e7562c89-d712-45b7-8e15-df3feef4d7a7
📒 Files selected for processing (21)
app/build.gradleapp/src/main/graphql/User.graphqlapp/src/main/graphql/schema.graphqlsapp/src/main/java/com/cornellappdev/uplift/data/models/ProfileData.ktapp/src/main/java/com/cornellappdev/uplift/data/models/UserInfo.ktapp/src/main/java/com/cornellappdev/uplift/data/models/WorkoutDomain.ktapp/src/main/java/com/cornellappdev/uplift/data/repositories/AuthInterceptor.ktapp/src/main/java/com/cornellappdev/uplift/data/repositories/CheckInRepository.ktapp/src/main/java/com/cornellappdev/uplift/data/repositories/ProfileRepository.ktapp/src/main/java/com/cornellappdev/uplift/data/repositories/UserInfoRepository.ktapp/src/main/java/com/cornellappdev/uplift/ui/MainNavigationWrapper.ktapp/src/main/java/com/cornellappdev/uplift/ui/components/profile/ProfileHeaderSection.ktapp/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/HistorySection.ktapp/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WeeklyProgressTracker.ktapp/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WorkoutProgressArc.ktapp/src/main/java/com/cornellappdev/uplift/ui/screens/profile/ProfileScreen.ktapp/src/main/java/com/cornellappdev/uplift/ui/viewmodels/onboarding/LoginViewModel.ktapp/src/main/java/com/cornellappdev/uplift/ui/viewmodels/profile/CheckInViewModel.ktapp/src/main/java/com/cornellappdev/uplift/ui/viewmodels/profile/ProfileViewModel.ktapp/src/main/java/com/cornellappdev/uplift/util/Functions.ktapp/src/main/res/drawable/ic_dufflebag.xml
| val encodedImage: String?, | ||
| val activeStreak: Int?, | ||
| val maxStreak: Int?, | ||
| val streakStart: String?, | ||
| val workoutGoal: Int?, | ||
| val totalGymDays: Int |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the GraphQL schema nullability for user fields
rg -n "activeStreak|maxStreak" app/src/main/graphql/schema.graphqlsRepository: cuappdev/uplift-android
Length of output: 113
Align field nullability with GraphQL schema.
activeStreak and maxStreak are declared as Int! (non-nullable) in the GraphQL schema, but defined as Int? here. Update to match the schema contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/uplift/data/models/UserInfo.kt` around
lines 11 - 16, In UserInfo.kt update the nullability to match the GraphQL schema
by changing the activeStreak and maxStreak properties from nullable Int? to
non-nullable Int; locate the UserInfo class (fields like encodedImage,
activeStreak, maxStreak, streakStart, workoutGoal, totalGymDays) and remove the
question marks on activeStreak and maxStreak so their types are Int not Int?.
| ) : Interceptor { | ||
| override fun intercept(chain: Interceptor.Chain): Response { | ||
| val token = tokenManager.getAccessToken() | ||
| android.util.Log.d("AuthInterceptor", "token present = ${token != null}") |
There was a problem hiding this comment.
Remove debug log statement before release.
This debug log exposes authentication state information (token present = true/false) in production builds. While it doesn't leak the token itself, it's a development artifact that should be removed or guarded behind a debug build check.
🛡️ Proposed fix
- android.util.Log.d("AuthInterceptor", "token present = ${token != null}")
+ // Remove or guard with BuildConfig.DEBUG if needed for debugging:
+ // if (BuildConfig.DEBUG) {
+ // android.util.Log.d("AuthInterceptor", "token present = ${token != null}")
+ // }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| android.util.Log.d("AuthInterceptor", "token present = ${token != null}") | |
| // Remove or guard with BuildConfig.DEBUG if needed for debugging: | |
| // if (BuildConfig.DEBUG) { | |
| // android.util.Log.d("AuthInterceptor", "token present = ${token != null}") | |
| // } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/uplift/data/repositories/AuthInterceptor.kt`
at line 14, Remove the development debug log in AuthInterceptor that prints
authentication presence; locate the android.util.Log.d("AuthInterceptor", ...)
call inside the AuthInterceptor class/method and either delete it or wrap it
with a debug-only guard (e.g., check BuildConfig.DEBUG) so it is not executed in
release builds; ensure no other calls leak auth state in production and keep
logging minimal and safe for release.
| val workoutDomain = workouts.map { | ||
| WorkoutDomain( | ||
| gymName = it.workoutFields.gymName, | ||
| timestamp = Instant.parse(it.workoutFields.workoutTime.toString()) | ||
| .toEpochMilli() | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how DateTime scalar is configured in Apollo setup
rg -n "DateTime|scalar" --type=kotlin app/src/main/java/com/cornellappdev/uplift/
fd -e kt -x grep -l "ScalarAdapter\|customScalarAdapters" {} \;Repository: cuappdev/uplift-android
Length of output: 1148
🏁 Script executed:
#!/bin/bash
# 1. Check the ProfileRepository.kt file context around lines 47-53
echo "=== ProfileRepository.kt context ==="
sed -n '40,65p' app/src/main/java/com/cornellappdev/uplift/data/repositories/ProfileRepository.kt
# 2. Find GraphQL schema files
echo -e "\n=== GraphQL schema files ==="
find . -name "*.graphql" -o -name "schema.json" | head -20
# 3. Check Apollo configuration
echo -e "\n=== Apollo configuration files ==="
find . -name "apollo.config.*" -o -name "build.gradle*" | head -20Repository: cuappdev/uplift-android
Length of output: 1410
🏁 Script executed:
#!/bin/bash
# 1. Check User.graphql for workoutTime and DateTime definition
echo "=== User.graphql content ==="
cat app/src/main/graphql/User.graphql
# 2. Check app/build.gradle for Apollo configuration
echo -e "\n=== app/build.gradle Apollo configuration ==="
grep -A 20 -B 5 "apollo\|scalar" app/build.gradle | head -100
# 3. Look for generated Apollo code
echo -e "\n=== Generated Apollo directories ==="
find app -type d -name "generated" -o -type d -name "apolloClient" -o -type d -name "apollo" | head -20Repository: cuappdev/uplift-android
Length of output: 4116
🏁 Script executed:
#!/bin/bash
# 1. Find generated Apollo types
echo "=== Looking for generated Apollo types ==="
find app/build -type f -name "*.kt" 2>/dev/null | grep -i "user\|workout" | head -20
# 2. Check if there's a build directory
echo -e "\n=== Build generated directories ==="
find app/build/generated -type d 2>/dev/null | head -20
# 3. Search for any kotlin file that might show DateTime definition
echo -e "\n=== Searching for DateTime type definition in generated code ==="
find app/build -type f -name "*.kt" 2>/dev/null -exec grep -l "DateTime\|workoutTime" {} \; | head -10
# 4. If generated files exist, check one
if [ -f "$(find app/build/generated -name '*User*.kt' 2>/dev/null | head -1)" ]; then
echo -e "\n=== Sample generated User type ==="
cat "$(find app/build/generated -name '*User*.kt' 2>/dev/null | head -1)" | head -100
fiRepository: cuappdev/uplift-android
Length of output: 212
🏁 Script executed:
#!/bin/bash
# 1. Search for usage of workoutTime elsewhere in the codebase
echo "=== Usage of workoutTime in codebase ==="
rg -n "workoutTime" --type=kotlin app/src/
# 2. Check if there are any DateTime-related imports in ProfileRepository
echo -e "\n=== ProfileRepository.kt full context (1-80 lines) ==="
head -80 app/src/main/java/com/cornellappdev/uplift/data/repositories/ProfileRepository.ktRepository: cuappdev/uplift-android
Length of output: 3430
🌐 Web query:
Apollo GraphQL 4.1.1 DateTime scalar default mapping Kotlin Android
💡 Result:
In Apollo Kotlin 4.1.1 (Android), a GraphQL scalar DateTime is not mapped to a Kotlin date type by default—you must map the scalar yourself in Gradle using mapScalar(...). Apollo’s docs show mapScalar(...) as the mechanism for custom scalars and that AnyAdapter exists as the generic fallback adapter. [1]
Option A (quick): map DateTime to String
apollo {
service("service") {
mapScalarToKotlinString("DateTime")
}
}(Example of mapping a date-time scalar to String is commonly used in Android integrations.) [2]
Option B (recommended): map DateTime to kotlinx.datetime.Instant
Apollo’s date/time adapters were moved out of the core artifacts in v4. [3] Add the adapters dependency, then map the scalar to Instant with the provided adapter:
dependencies {
implementation("com.apollographql.adapters:apollo-adapters-datetime:<version>")
}
apollo {
service("service") {
mapScalar("DateTime", "kotlinx.datetime.Instant", "com.apollographql.adapters.InstantAdapter")
}
}Apollo’s release notes show the InstantAdapter class name for mapping to kotlinx.datetime.Instant, and Apollo’s KDoc notes older KotlinxInstantAdapter is deprecated in favor of the new adapters coordinates. [4][5]
Sources:
[1] Apollo Kotlin docs – Custom scalars (mapScalar, AnyAdapter) (apollographql.com)
[2] AWS Amplify Android example mapping a DateTime scalar to String (docs.amplify.aws)
[3] Apollo Kotlin docs – Migrating to v4 (artifacts moved; mapScalar) (apollographql.com)
[4] Apollo Kotlin GitHub releases (example mapping with com.apollographql.adapters.InstantAdapter) (github.com)
[5] Apollo Kotlin KDoc (older KotlinxInstantAdapter deprecated; new apollo-adapters-datetime coordinates) (apollographql.com)
Citations:
- 1: https://www.apollographql.com/docs/kotlin/v5/essentials/custom-scalars
- 2: https://docs.amplify.aws/android/build-a-backend/data/aws-appsync-apollo-extensions/?utm_source=openai
- 3: https://www.apollographql.com/docs/deploy-preview/be689022ee507e54beb06001/kotlin/migration/4.0
- 4: https://github.com/apollographql/apollo-kotlin/releases?utm_source=openai
- 5: https://www.apollographql.com/docs/kotlin/kdoc/older/4.2.0/apollo-adapters/com.apollographql.apollo.adapter/-kotlinx-instant-adapter/index.html
Add explicit DateTime scalar mapping to prevent runtime parsing errors.
The code assumes workoutTime serializes to ISO-8601 format via toString(), but Apollo 4.1.1 requires explicit mapScalar configuration for custom scalars like DateTime. Without it, the scalar defaults to AnyAdapter, making the runtime type and serialization behavior unpredictable. Instant.parse() will throw DateTimeParseException if toString() doesn't produce a parseable format.
Configure the DateTime scalar in app/build.gradle:
apollo {
service("service") {
packageName.set("com.cornellappdev.uplift")
mapScalarToKotlinString("DateTime") // Quick: maps to String
// OR use apollo-adapters for better type safety:
// mapScalar("DateTime", "kotlinx.datetime.Instant", "com.apollographql.adapters.InstantAdapter")
}
}
Then update the mapping to use the properly typed value directly instead of calling toString().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/uplift/data/repositories/ProfileRepository.kt`
around lines 47 - 53, The mapping of workoutFields.workoutTime into
Instant.parse(...) is unsafe because Apollo's DateTime scalar isn't configured
and may deserialize as Any; update your Apollo config in app/build.gradle to map
the DateTime scalar (e.g., use mapScalarToKotlinString("DateTime") for a quick
fix or mapScalar("DateTime", "kotlinx.datetime.Instant",
"com.apollographql.adapters.InstantAdapter") for proper typing), then change the
ProfileRepository mapping that builds workoutDomain (reference: workoutDomain,
WorkoutDomain, workoutFields.workoutTime, Instant.parse) to use the correctly
typed value provided by the scalar mapping instead of calling toString() and
parsing at runtime.
| suspend fun syncUserToDataStore(netId: String): Boolean { | ||
| return try { | ||
| if (!loginAndStoreTokens(netId)) return false | ||
| val user = getUserByNetId(netId) ?: return false | ||
|
|
||
| storeUserFields( | ||
| id = user.id, | ||
| username = user.name, | ||
| netId = user.netId, | ||
| email = user.email, | ||
| skip = false, | ||
| goal = user.workoutGoal ?: 0 | ||
| ) | ||
|
|
||
| Log.d("UserInfoRepositoryImpl", "Synced existing user to DataStore: ${user.id}") | ||
| true | ||
| } catch (e: Exception) { | ||
| Log.e("UserInfoRepositoryImpl", "Error syncing user to DataStore", e) | ||
| false | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoded skip = false may not reflect user's actual preference.
In syncUserToDataStore, skip is hardcoded to false on line 154. If the user originally skipped goal setup during onboarding, this value won't be preserved when syncing. Consider fetching the actual skip preference from the backend if available, or document this behavior as intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/uplift/data/repositories/UserInfoRepository.kt`
around lines 144 - 164, The syncUserToDataStore function currently passes a
hardcoded skip = false into storeUserFields, which can overwrite a user's real
onboarding preference; update syncUserToDataStore to derive skip from the
fetched user object (e.g., use a boolean field like user.skippedGoalSetup or
user.skippedGoal if present) and pass that value to storeUserFields(id =
user.id, username = user.name, netId = user.netId, email = user.email, skip =
user.<skipField>, goal = user.workoutGoal ?: 0); if the user model lacks such a
field, call the backend to retrieve the preference before storing or explicitly
document the intentional default behavior.
There was a problem hiding this comment.
Might be able to ignore this if this is referring to login skip since the user won't be skipping if they sign in, but if this is referring to skipping goal setting, we might want to rename for clarity and make sure that the vm for that screen handles the skipping logic properly (prob separate PR since Preston worked on that screen + vm)
There was a problem hiding this comment.
@AndrewCheung360, you're right — skip here refers to skipping goal setup during onboarding, not login. A returning user signing in wouldn't be "skipping login," so the concern is about whether we're correctly preserving their original goal-setting preference.
The tricky part is that UserInfo (and the backend schema) doesn't expose a boolean skip preference — only workoutGoal: Int?. So there's no clean way to derive it from the fetched user. Hardcoding skip = false is arguably reasonable for returning users (since a non-null workoutGoal implies they set one), but it does silently overwrite the stored SKIP flag for anyone who originally skipped. Two practical suggestions:
- Rename for clarity —
skip→ something likeskippedGoalSetupacrossstoreUserFieldsand callsites to make the intent explicit. - Infer from
workoutGoal— a lightweight fix here would be to passskip = (user.workoutGoal == null || user.workoutGoal == 0)so that a user with no goal is treated as having skipped, rather than blindly hardcodingfalse. This isn't perfect but is better than always overwriting.
Deferring the VM-level handling to a separate PR (since Preston owns that screen) is the right call. Would you like me to open a follow-up issue to track the rename and skip-inference fix?
| val paddedCompletedDays = | ||
| if (completedDays.size < daysOfWeek.size) { | ||
| completedDays + List(daysOfWeek.size - completedDays.size) { false } | ||
| } else { | ||
| completedDays | ||
| } |
There was a problem hiding this comment.
Clamp completedDays to 7 before computing progress state.
At Line 56, values beyond the visible week are preserved. If a later extra entry is true, lastCompletedIndex (Line 63) can exceed the displayed range and incorrectly render all connecting lines as solid.
Suggested fix
- val paddedCompletedDays =
- if (completedDays.size < daysOfWeek.size) {
- completedDays + List(daysOfWeek.size - completedDays.size) { false }
- } else {
- completedDays
- }
+ val paddedCompletedDays =
+ completedDays
+ .take(daysOfWeek.size)
+ .let { it + List(daysOfWeek.size - it.size) { false } }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val paddedCompletedDays = | |
| if (completedDays.size < daysOfWeek.size) { | |
| completedDays + List(daysOfWeek.size - completedDays.size) { false } | |
| } else { | |
| completedDays | |
| } | |
| val paddedCompletedDays = | |
| completedDays | |
| .take(daysOfWeek.size) | |
| .let { it + List(daysOfWeek.size - it.size) { false } } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/uplift/ui/components/profile/workouts/WeeklyProgressTracker.kt`
around lines 56 - 61, The bug is that completedDays can contain entries beyond
the visible week so lastCompletedIndex and rendering use out-of-range trues; fix
by clamping/trimming completedDays to at most daysOfWeek.size before creating
paddedCompletedDays and before computing lastCompletedIndex — e.g., derive a
local trimmedCompletedDays = completedDays.take(daysOfWeek.size) (or slice to
daysOfWeek.size) then build paddedCompletedDays from trimmedCompletedDays and
compute lastCompletedIndex from trimmedCompletedDays/paddedCompletedDays; update
references to use these trimmed values in WeeklyProgressTracker rendering logic.
| profilePictureUri = uiState.profileImage, | ||
| onPhotoSelected = {}, | ||
| netId = uiState.netId |
There was a problem hiding this comment.
Photo selection handler is a no-op.
onPhotoSelected = {} means selecting a profile photo will have no effect. If this is intentional for the initial implementation, consider adding a TODO comment or logging to clarify.
📝 Suggestion to clarify intent
- onPhotoSelected = {},
+ onPhotoSelected = { /* TODO: Implement photo upload in ProfileViewModel */ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| profilePictureUri = uiState.profileImage, | |
| onPhotoSelected = {}, | |
| netId = uiState.netId | |
| profilePictureUri = uiState.profileImage, | |
| onPhotoSelected = { /* TODO: Implement photo upload in ProfileViewModel */ }, | |
| netId = uiState.netId |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/cornellappdev/uplift/ui/screens/profile/ProfileScreen.kt`
around lines 82 - 84, The onPhotoSelected handler passed to the profile picture
component is currently a no-op (onPhotoSelected = {}), so photo selection does
nothing; update the handler in ProfileScreen.kt to either wire it to the proper
update function (e.g., call the ViewModel's upload/update method or a function
that updates uiState.profileImage) or, if left intentionally unimplemented,
replace the empty lambda with a TODO/log statement (e.g., a call to Timber.d or
a TODO comment) to make the intent explicit and help future implementers locate
the callback.
| return when { | ||
| diffDays < 1 -> "Today" | ||
|
|
||
| diffDays == 1L -> "1 day ago" | ||
| diffDays in 2L..6L -> "$diffDays days ago" | ||
|
|
||
| diffWeeks == 1L -> "1 week ago" | ||
| diffWeeks in 2L..4L -> "$diffWeeks weeks ago" | ||
|
|
||
| diffMonths == 1L -> "1 month ago" | ||
| diffMonths in 2L..11L -> "$diffMonths months ago" | ||
|
|
||
| diffYears == 1L -> "1 year ago" | ||
| diffYears > 1L -> "$diffYears years ago" | ||
|
|
||
| else -> "Today" | ||
| } |
There was a problem hiding this comment.
Bug: Dates 331-364 days ago incorrectly return "Today".
There's a gap in the when conditions. When diffDays is approximately 331-364:
diffMonths= 11-12 (12 falls outside2L..11L)diffYears= 0 (not yet 365 days)
This causes the else branch to trigger, incorrectly returning "Today".
🐛 Proposed fix
diffMonths == 1L -> "1 month ago"
- diffMonths in 2L..11L -> "$diffMonths months ago"
+ diffMonths >= 2L -> "$diffMonths months ago"
diffYears == 1L -> "1 year ago"
diffYears > 1L -> "$diffYears years ago"
-
- else -> "Today"
}
}Alternatively, keep the existing structure and add explicit handling for 12+ months:
diffMonths in 2L..11L -> "$diffMonths months ago"
+ diffMonths >= 12L -> "$diffMonths months ago"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/cornellappdev/uplift/util/Functions.kt` around lines
203 - 219, The when-block returning human-readable age has a gap for ~331–364
days because diffMonths can be 12 while diffYears is still 0, causing the else
-> "Today" to hit; update the month/year checks in that when expression (which
uses diffDays, diffWeeks, diffMonths, diffYears) so that months cover 2L..12L
(or add an explicit case diffMonths >= 12L -> "1 year ago") and make the year
check use diffYears >= 1L (or keep diffYears == 1L and diffYears > 1L but ensure
months don't exclude 12), ensuring no 12-month value falls through to the else
branch.
| workoutGoal: Int, | ||
| daysOfMonth: List<Int>, | ||
| completedDays: List<Boolean>, | ||
| reminderItems: List<ReminderItem>, |
There was a problem hiding this comment.
We can prob remove this and the navigateToReminderSection if they won't be used
AndrewCheung360
left a comment
AndrewCheung360
left a comment
There was a problem hiding this comment.
Overall LGTM, but you can check the coderabbit comments to make sure if there are any changes we should make or keep note of as git issues
Overview
Profile Page implementation that aligns with the latest backend schema and new designs.
Changes Made
Updated the schema and user GraphQL queries/models to match the latest backend changes
Updated the User data model/class accordingly
Reworked the profile page UI and components to match the latest designs
Implemented the Profile ViewModel and repository logic to load profile data from backend
Added additional testing/debugging work around workout logging through the Check-In ViewModel and repository
Test Coverage
Manually tested
Next Steps (delete if not applicable)
Further test profile page and workout history once logWorkout is fully authenticated and working
Add networking and logic for settings and full workout history
Add loading page
Screenshots
Screen Shot\
Summary by CodeRabbit
Release Notes
New Features
Improvements