feat: ✨ user settings#430
Conversation
realethantran
temporarily deployed
to
staging-430
GitHub Actions
Inactive
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/profile/profile-settings.tsx">
<violation number="1" location="src/components/profile/profile-settings.tsx:82">
P2: Username availability can become incorrect because async check results are applied even when they are stale.</violation>
<violation number="2" location="src/components/profile/profile-settings.tsx:86">
P2: Discard should clear the pending username debounce timer to prevent stale availability updates after reset.</violation>
</file>
<file name="src/lib/auth/user.ts">
<violation number="1" location="src/lib/auth/user.ts:7">
P1: If `displayName` contains only non-Latin characters (e.g. `"김민수"`, `"🎉🎉"`), every regex replacement strips everything and `base` becomes `""`. The function then returns `""` (or `"2"`, `"3"`, …) as the username. Add an empty-string guard after computing `base` to fall back to a sensible default like `"user"`.</violation>
</file>
<file name="src/server/actions/user/profile/action.ts">
<violation number="1" location="src/server/actions/user/profile/action.ts:41">
P2: Handle unique-constraint errors on profile update; the current check-then-update flow can throw on concurrent username claims.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
realethantran
temporarily deployed
to
staging-430
GitHub Actions
Inactive
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/profile/profile-settings.tsx">
<violation number="1" location="src/components/profile/profile-settings.tsx:81">
P2: Stale username availability responses are still applied when input changes to an early-return state because the counter is not invalidated on those paths.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
i hooked the avatars to the availability sidebar |
i hooked the member's avatar and displayname when invited to a meeting |
realethantran
left a comment
realethantran
left a comment
There was a problem hiding this comment.
The profile pictures on the availability bar are really nice! Overall, the only main issue is that the two Avatar instances are missing referrerPolicy: "no-referrer." This is good to include since it prevents Google's CDN from rejecting the image request for the profile picture.
| icon={ | ||
| <Avatar | ||
| alt={member.displayName} | ||
| src={member.profilePicture ?? undefined} |
There was a problem hiding this comment.
make sure to add referrerPolicy: "no-referrer" to prevent them from being blocked by Google's CDN
| icon={ | ||
| <Avatar | ||
| alt={member.displayName} | ||
| src={member.profilePicture ?? undefined} |
There was a problem hiding this comment.
same comment as before: make sure to add referrerPolicy: "no-referrer" to prevent them from being blocked by Google's CDN
realethantran
temporarily deployed
to
staging-430
GitHub Actions
Inactive
| <Box sx={{ px: 8, py: 8 }}> | ||
| <Typography | ||
| variant="h4" | ||
| sx={{ fontWeight: 600, fontFamily: "Figtree, sans-serif" }} | ||
| > | ||
| Settings | ||
| </Typography> | ||
|
|
||
| <Box sx={{ mt: 8 }}> | ||
| <ProfileSettings user={session.user} /> | ||
| </Box> | ||
| <Box sx={{ mt: 8, display: { md: "none" } }}> |
There was a problem hiding this comment.
nit: perfer tailwind className styles over sx
| <Box sx={{ px: 8, py: 8 }}> | ||
| <Typography | ||
| variant="h4" | ||
| sx={{ fontWeight: 600, fontFamily: "Figtree, sans-serif" }} |
There was a problem hiding this comment.
nit: figtree font is already applied globally
feat: optimized for mobile |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/components/profile/profile-settings.tsx">
<violation number="1" location="src/components/profile/profile-settings.tsx:228">
P2: Don't hide `ProfileContent` at `md` and up unless it is rendered elsewhere; this removes the appearance settings from the desktop profile page.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
ethancha0
left a comment
ethancha0
left a comment
There was a problem hiding this comment.
Awesome job Ethan! I'm so excited to see this feature get merged. This was definitely a lengthy task, which involved retrieving data from OAuth, handling logic for unique usernames, and hooking it onto the notifications + availability sidebar.
In the future, you should use your best judgment between using an MUI component and a standard browser component. For example, using div instead of an MUI box is likely best in most situations for code review purposes.
Also, please format your PR descriptions using this format in the future (be more specific with the test plan, and also bullet the summary for easier reading
Other than that, amazing job Ethan!
realethantran
temporarily deployed
to
staging-430
GitHub Actions
Inactive
ethancha0
left a comment
ethancha0
left a comment
There was a problem hiding this comment.
⠀⠀⠀⠀⠀⠀⠀⢠⣤⣀⠀⠀⠀⠀⢀⣀⣤⣤⠀⠀⠀⠀⠀⠀⠀
⠀⠀⢀⢀⠀⠀⠀⢸⡿⠛⠛⠛⠛⠛⠉⠛⢿⣿⠀⠀⠀⠀⠀⠀⠀
⠀⠠⣿⣿⣿⣄⠀⣼⠀⠀⣁⣀⣀⣀⡀⠁⠀⢹⡀⠀⠀⠀⠀⠀⠀
⠀⢸⣿⣿⣿⣿⡷⠋⠀⠀⠀⠀⠀⠀⠀⠀⠈⠘⠣⡀⠀⠀⠀⠀⠀
⠀⠈⣿⣿⡿⠋⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠘⣷⣦⡀⠀⠀
⠀⠀⢹⣿⡇⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⣠⣿⣿⣿⣦⠀
⠀⠀⣸⣿⣿⣶⣶⣶⣾⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣇
⠀⣤⡟⠛⠋⠉⠙⢿⣿⣿⣿⣿⣿⣿⣿⣿⣿⠟⠉⠈⠋⠈⢿⣿⡿
⢀⡉⠀⠀⣀⣤⣄⢈⣿⣿⣿⣿⣿⣿⣿⣿⣿⢀⣤⣤⣄⠀⠀⣴⡄
⠘⢇⠀⠰⣿⣿⢟⢼⣿⣿⣿⣿⣿⣿⣿⣿⡿⢜⠿⠿⠿⠀⡀⠀⠀
⠀⠀⠁⠀⠀⠀⠁⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠉⠀⠀⠈⠀⠀⠀
2 participants
Description
This PR implements core features for the user settings. There are 5 new fields added to the members table within the database: google_name, username, year, school, and profile_picture. The user settings are accessible through the profile page, in which users can update their display name, username, and optionally, their year and school. The display name defaults to the user's google_name, while their default username is automatically generated based off their display name. The profile picture is set to the user's Google account profile picture, which is now visible throughout the website, such as when inviting members to a new group or meeting.
Recording/Screenshots
Issues