Skip to content

[Goals for Onboarding]#104

Open
isiahpwilliams wants to merge 14 commits intomainfrom
preston/goals-onboarding
Open

[Goals for Onboarding]#104
isiahpwilliams wants to merge 14 commits intomainfrom
preston/goals-onboarding

Conversation

@isiahpwilliams
Copy link

@isiahpwilliams isiahpwilliams commented Mar 4, 2026

Overview

Implemented the "set your goals" page from the figma and added it to the onboarding flow. Currently, it does not upload the information to the database but that will be for future changes. I also made the base view model for the page with a few functions that will need to be updated when implementing the networking features. Finished networking for the onboarding pages allowing users to update their goals after creating their account. Handled encryption of access and refresh tokens and created an auth interceptor to allow us to send requests to backend without manually adding the http header with each request.

Changes Made

Refactored screen to separate usage for onboarding and settings (WorkoutReminderScreen.kt)
Designed a base view model for the screen (ProfileCreationViewModel.kt)
Updated create user function to enable users to add a goal (UserInfoRepository.kt)
Made small changes in MainNavigationWrapper.kt and DatastoreRepository.kt to enable above changes
Created a token manager to easily encrypt and decrypt the tokens (TokenManager.kt)
Engineered an auth interceptor to add authentication to each backend request we make (AuthInterceptor.kt)

Test Coverage

I was able to manually test and am successfully able to drag the slider over and move to the next page and create a user and view their goals. I also added logging for onboarding and users are successfully created and their goals are added if that page isn't skipped.

Screenshots (delete if not applicable)

Goals Onboarding Screen Screenshot 2026-03-04 at 12 37 39 PM Screenshot 2026-03-04 at 12 37 59 PM Screenshot 2026-03-10 at 9 47 48 PM These are the onboarding screens that allow users to adjust a slider to set a goal for the number of days they want to work out per week and a picture of an account successfully being created after the goal logic was added.

@AndrewCheung360
Copy link
Member

AndrewCheung360 commented Mar 7, 2026

Nice job on your first PR, and thanks for your work! I appreciate you keeping the PR nice and concise -- makes it easier for me to review!

A few things though:

It looks like the code for the set goals UI along with the workout reminders is already there in WorkoutRemindersScreen which can be modified slightly to fit the figma design, so it would probably be better to make use of that instead. I do apologize for not checking in sooner; I did not know that the screen you were assigned was basically the same as the one we already had.

For the VM code, it seems like it doesn't exactly have the functions or states that you might need aside from the goal slider value, so you could either not include it and use placeholders in the UI or try to include the states and functions that make sense for that screen with TODOs for their functionality unless it is straightforward

Also, we should not include the .DS_Store file.

image

@isiahpwilliams
Copy link
Author

I took account of your changes to reuse an old screen which I did. I was successfully able to add the networking logic to enable a user to set a goal during the onboarding process, which I tested using my own account. Also removed some unused imports and deleted unused files.

@isiahpwilliams isiahpwilliams changed the title [Goals for Onboarding][1/2] [Goals for Onboarding] Mar 11, 2026
@AndrewCheung360 AndrewCheung360 requested a review from Copilot March 11, 2026 20:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends the onboarding flow to include a goals (workout days/week) step, persists that goal/skip choice during user creation, and adds GraphQL support for login + goal setting.

Changes:

  • Add a Goals onboarding route/screen and wire Profile Creation to navigate into it.
  • Update onboarding ViewModel + repository to pass/store “skip goal” + goal value, login the user, and call SetWorkoutGoals.
  • Add new DataStore keys and update GraphQL operations/schema accordingly.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
app/src/main/java/com/cornellappdev/uplift/ui/viewmodels/onboarding/ProfileCreationViewModel.kt Adds goal/skip state and onboarding navigation/create-user wiring
app/src/main/java/com/cornellappdev/uplift/ui/screens/reminders/WorkoutReminderScreen.kt Introduces onboarding/settings wrappers and refactors core screen structure
app/src/main/java/com/cornellappdev/uplift/ui/screens/onboarding/ProfileCreationScreen.kt Routes “Get started” to the goals onboarding step
app/src/main/java/com/cornellappdev/uplift/ui/MainNavigationWrapper.kt Adds GoalsOnboarding destination to root navigation
app/src/main/java/com/cornellappdev/uplift/data/repositories/UserInfoRepository.kt Expands createUser to login, store tokens, and set workout goal
app/src/main/java/com/cornellappdev/uplift/data/repositories/DatastoreRepository.kt Adds DataStore keys for goal + tokens
app/src/main/graphql/User.graphql Updates SetWorkoutGoals signature and adds LoginUser/DeleteUser mutations
app/src/main/graphql/schema.graphqls Updates schema types/fields to match new backend expectations
app/build.gradle Flips ONBOARDING_FLAG for debug build type

💡 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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 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.

…rding/ProfileCreationViewModel.kt

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…rding/ProfileCreationViewModel.kt

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…serInfoRepository.kt

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 14 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.

Comment on lines +39 to +47
private fun createUser() = viewModelScope.launch {
val state = getStateValue()
val user = state.user
val name = user?.displayName ?: ""
val email = user?.email ?: ""
val netId = email.substring(0, email.indexOf('@'))
if (userInfoRepository.createUser(email, name, netId)) {
val email = user?.email
if (email.isNullOrBlank()) {
Log.e("Error", "Cannot create user: missing or blank email")
userInfoRepository.signOut()
return@launch
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createUser() reads state.user, but user is only populated asynchronously in init { viewModelScope.launch { ... } }. If the user taps Next/Skip before that coroutine runs, state.user will still be null and this path will sign the user out due to a missing email. Consider fetching firebaseAuth.currentUser directly inside createUser() (or populating user synchronously) so onboarding can’t race with initialization.

Copilot uses AI. Check for mistakes.
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.lifecycle.compose.collectAsStateWithLifecycle
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import: collectAsStateWithLifecycle is imported but not referenced anywhere in this file; please remove it to avoid warnings/linters failing.

Suggested change
import androidx.lifecycle.compose.collectAsStateWithLifecycle

Copilot uses AI. Check for mistakes.
import com.cornellappdev.uplift.ui.components.goalsetting.GoalSlider
import com.cornellappdev.uplift.ui.components.goalsetting.WorkoutReminders
import com.cornellappdev.uplift.ui.components.general.UpliftButton
import com.cornellappdev.uplift.ui.viewmodels.onboarding.ProfileCreationViewModel
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused import: ProfileCreationViewModel is imported but not used in this file; please remove it to avoid warnings/linters failing.

Suggested change
import com.cornellappdev.uplift.ui.viewmodels.onboarding.ProfileCreationViewModel

Copilot uses AI. Check for mistakes.
Comment on lines 64 to 69
/**
* @param reminders: list of reminders
* @param onRemindersChange: callback for when reminders are changed
* @param goalValue: value of the goal slider
* @param onGoalValueChange: callback for when the goal slider value is changed
* @param onBackClick: callback for when the back button is clicked
* @param isOnboarding: whether the user is in onboarding
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KDoc for WorkoutReminderScreen no longer documents the required goalValue parameter (it was removed from the comment but the parameter is still present). Please update the KDoc so it matches the function signature.

Copilot uses AI. Check for mistakes.
Comment on lines +105 to +112
preferences[PreferencesKeys.ACCESS_TOKEN] = accessToken
preferences[PreferencesKeys.REFRESH_TOKEN] = refreshToken
if (!skip) {
preferences[PreferencesKeys.GOAL] = goal
}
}
}

Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access/refresh tokens are being persisted into the app DataStore (PreferencesKeys.ACCESS_TOKEN / REFRESH_TOKEN). Preferences DataStore is not encrypted by default, so this stores credentials at rest in plaintext. Since you already introduced TokenManager with EncryptedSharedPreferences, tokens should be stored only there (or move to an encrypted DataStore) and remove these DataStore writes.

Suggested change
preferences[PreferencesKeys.ACCESS_TOKEN] = accessToken
preferences[PreferencesKeys.REFRESH_TOKEN] = refreshToken
if (!skip) {
preferences[PreferencesKeys.GOAL] = goal
}
}
}
if (!skip) {
preferences[PreferencesKeys.GOAL] = goal
}
}
tokenManager.saveTokens(accessToken, refreshToken)
}

Copilot uses AI. Check for mistakes.
Comment on lines +31 to +32
val ACCESS_TOKEN = stringPreferencesKey("accessToken")
val REFRESH_TOKEN = stringPreferencesKey("refreshToken")
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding ACCESS_TOKEN / REFRESH_TOKEN to PreferencesKeys implies these credentials will live in (unencrypted) Preferences DataStore. This is not appropriate for auth tokens; please remove these keys and keep tokens exclusively in an encrypted store (e.g., TokenManager / EncryptedSharedPreferences).

Suggested change
val ACCESS_TOKEN = stringPreferencesKey("accessToken")
val REFRESH_TOKEN = stringPreferencesKey("refreshToken")

Copilot uses AI. Check for mistakes.
Comment on lines +21 to +25
} catch (e: Exception) {
Log.e("TokenManager", "Failed to initialize EncryptedSharedPreferences", e)
context.deleteSharedPreferences(fileName)
context.getSharedPreferences(fileName, Context.MODE_PRIVATE)
}
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TokenManager falls back to plain SharedPreferences when EncryptedSharedPreferences initialization fails (catch block). That means access/refresh tokens may be stored unencrypted on some devices, defeating the purpose of introducing encrypted storage. Prefer failing closed here (e.g., clear tokens and force re-auth) rather than silently storing tokens in plaintext.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants