feat: Add user profile API and editable profile page solve issue #222 #225
Conversation
- Create GET/PUT /v1/users/me endpoints for profile management - Add database migration with users table and auto-sync trigger - Rewrite ProfilePage with real data integration - Make email field read-only - Add loading states and error handling - Include toast notifications for UX feedback BREAKING CHANGE: Requires database migration (02_create_users_table.sql)
📝 WalkthroughWalkthroughIntroduces a complete user profile feature with backend API endpoints (GET/PUT Changes
Sequence DiagramsequenceDiagram
participant User as User (Browser)
participant Frontend as Frontend (React)
participant Auth as Auth System
participant API as Backend API
participant DB as Database
User->>Frontend: Navigate to /profile
Frontend->>Auth: Request current user
Auth-->>Frontend: Return user_id (JWT)
Frontend->>API: GET /v1/users/me
API->>DB: Query users table WHERE id = user_id
DB-->>API: Return profile data
API-->>Frontend: UserProfileResponse
Frontend-->>User: Display profile (read-only)
User->>Frontend: Click Edit Profile
Frontend-->>User: Show edit form (email read-only)
User->>Frontend: Edit display_name, bio, location
Frontend-->>User: Local state updates
User->>Frontend: Click Save Changes
Frontend->>API: PUT /v1/users/me with UserProfileUpdateRequest
API->>DB: UPDATE users SET... WHERE id = user_id (RLS enforced)
DB-->>API: Return updated row
API-->>Frontend: UserProfileResponse (updated)
Frontend-->>User: Show success toast
Frontend-->>User: Display updated profile
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @frontend/src/components/pages/ProfilePage.tsx:
- Around line 39-54: handleSave currently submits without checking that
editedProfile.display_name is present; add a client-side validation at the start
of handleSave to check if editedProfile.display_name (trimmed) is non-empty, and
if it is empty call toast.error('Display Name is required') (or set a validation
state) and return early without calling apiClient.updateUserProfile or toggling
setIsSaving; ensure you trim the value, reference editedProfile.display_name in
the check, and keep the existing try/catch/finally behavior unchanged for
successful paths.
In @IMPLEMENTATION_SUMMARY.md:
- Around line 135-136: The summary in IMPLEMENTATION_SUMMARY.md incorrectly
claims "No breaking changes" and "Backwards compatible" despite the PR requiring
a DB migration (02_create_users_table.sql) and an explicit "BREAKING CHANGE:
migration required" note; update the summary to reflect that a migration is
required (e.g., replace the checkmarks with a clear statement that a migration
must be run before deployment and mark it as a breaking change), and ensure the
document references 02_create_users_table.sql so readers know which migration to
apply.
🧹 Nitpick comments (5)
backend/app/api/v1/users.py (2)
37-68: Improve error logging to include traceback.The endpoint logic is solid with proper authentication, error handling, and response construction. However, the error logging can be improved.
📝 Use logger.exception for automatic traceback inclusion
Replace
logger.errorwithlogger.exceptionin the except block to automatically include the stack trace, making debugging easier:except HTTPException: raise except Exception as e: - logger.error(f"Error fetching user profile: {str(e)}") + logger.exception("Error fetching user profile") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed to fetch user profile" ) from eNote:
logger.exceptionautomatically includes the exception details, sostr(e)is not needed.
71-139: Improve error logging and consider reducing code duplication.The update endpoint is well-structured with proper validation and error handling. Two areas for improvement:
Error logging (recommended): Same issue as the GET endpoint - use
logger.exceptioninstead oflogger.errorat line 135.Code duplication (optional): The
UserProfileResponseconstruction pattern is repeated three times in this file (lines 99-109, 120-130, and similar in the GET endpoint).📝 Recommended: Fix error logging
except HTTPException: raise except Exception as e: - logger.error(f"Error updating user profile: {str(e)}") + logger.exception("Error updating user profile") raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed to update user profile" ) from e♻️ Optional: Extract UserProfileResponse construction to helper function
Consider extracting the response construction into a helper function to reduce duplication:
def _build_profile_response(user) -> UserProfileResponse: """Build UserProfileResponse from user object.""" return UserProfileResponse( id=str(user.id), email=user.email, display_name=user.display_name, avatar_url=user.avatar_url, bio=user.bio, location=user.location, github_username=user.github_username, discord_username=user.discord_username, slack_username=user.slack_username, preferred_languages=user.preferred_languages )Then use
return _build_profile_response(user)in all three places.frontend/src/components/pages/ProfilePage.tsx (2)
14-16: Consider addingloadProfileto the dependency array or usinguseCallback.The
useEffectreferencesloadProfilebut the function is not in the dependency array. While this works due to how the function is defined, it can trigger ESLint warnings about exhaustive deps. Consider wrappingloadProfileinuseCallbackor defining it inside the effect.
282-296:preferred_languagesis in the update model but not editable in the UI.The
UserProfileUpdateRequestincludespreferred_languages, andeditedProfileinitializes it, but there's no input to edit this field. Consider either adding an editor for preferred languages or removing it fromeditedProfileinitialization to avoid confusion.backend/database/02_create_users_table.sql (1)
70-90: Consider adding error handling to prevent auth signup failures.If the
INSERT INTO public.usersfails (e.g., unexpected constraint violation), it will cause the entire auth user creation to fail since this is anAFTER INSERTtrigger. Consider wrapping in an exception handler to log errors without blocking signup.Proposed improvement
CREATE OR REPLACE FUNCTION public.handle_new_user() RETURNS TRIGGER AS $$ BEGIN + BEGIN INSERT INTO public.users (id, email, display_name, avatar_url) VALUES ( NEW.id, NEW.email, COALESCE(NEW.raw_user_meta_data->>'display_name', split_part(NEW.email, '@', 1), 'User'), NEW.raw_user_meta_data->>'avatar_url' ); + EXCEPTION WHEN OTHERS THEN + RAISE WARNING 'Failed to create user profile for %: %', NEW.id, SQLERRM; + END; RETURN NEW; END; $$ LANGUAGE plpgsql SECURITY DEFINER;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
frontend/package-lock.jsonis excluded by!**/package-lock.jsonlanding/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
IMPLEMENTATION_SUMMARY.mdPR_SUMMARY.mdPULL_REQUEST_TEMPLATE.mdbackend/app/api/router.pybackend/app/api/v1/users.pybackend/database/02_create_users_table.sqlfrontend/src/components/pages/ProfilePage.tsxfrontend/src/lib/api.ts
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/lib/api.ts (1)
backend/app/api/v1/users.py (1)
UserProfileUpdateRequest(28-34)
frontend/src/components/pages/ProfilePage.tsx (2)
frontend/src/lib/api.ts (3)
UserProfile(52-63)UserProfileUpdateRequest(65-71)apiClient(219-219)backend/app/api/v1/users.py (1)
UserProfileUpdateRequest(28-34)
🪛 LanguageTool
PR_SUMMARY.md
[style] ~5-~5: Consider using a different verb for a more formal wording.
Context: ...le Profile Page ## 📋 Summary This PR fixes the issue where user profile informatio...
(FIX_RESOLVE)
[style] ~175-~175: It’s more common nowadays to write this noun as one word.
Context: ...mation - Signup didn't properly capture user name - Email was editable (security issue) ...
(RECOMMENDED_COMPOUNDS)
🪛 Ruff (0.14.10)
backend/app/api/v1/users.py
38-38: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
44-47: Abstract raise to an inner function
(TRY301)
64-64: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
64-64: Use explicit conversion flag
Replace with conversion flag
(RUF010)
74-74: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
95-98: Abstract raise to an inner function
(TRY301)
115-118: Abstract raise to an inner function
(TRY301)
135-135: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
135-135: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (16)
PULL_REQUEST_TEMPLATE.md (1)
1-77: LGTM! Clear documentation for maintainers.The PR template provides comprehensive guidance covering database migration steps, backend restart instructions, testing procedures, and security considerations. The structure is clear and actionable.
backend/app/api/router.py (1)
5-5: LGTM! Router registration follows established pattern.The users router is correctly imported and registered with the appropriate prefix and tag, consistent with the existing auth, health, and integrations routers.
Also applies to: 27-31
frontend/src/lib/api.ts (2)
51-71: LGTM! Type definitions align with backend.The
UserProfileandUserProfileUpdateRequestinterfaces are well-defined and match the backend Pydantic models. Optional fields are correctly marked, and the field types are appropriate.
183-202: LGTM! API methods correctly implemented.The
getUserProfile()andupdateUserProfile()methods follow the established pattern in the ApiClient class. They correctly:
- Use the axios instance with auth token injection
- Target the appropriate endpoints (
/v1/users/me)- Return properly typed promises
- Extract response data consistently
backend/app/api/v1/users.py (2)
1-11: LGTM! Clean imports and setup.The imports are appropriate, logger is properly configured, and the router instance is correctly initialized.
14-34: LGTM! Well-defined Pydantic models.The models are properly structured:
UserProfileResponseincludes all profile fields for read operationsUserProfileUpdateRequestcorrectly excludes non-editable fields (email, id, integration usernames)- Types and Optional fields are appropriate
- Models align with frontend TypeScript interfaces
PR_SUMMARY.md (1)
1-232: LGTM! Comprehensive PR documentation.The PR summary provides excellent documentation covering all aspects of the feature:
- Clear description of changes (backend endpoints, database schema, frontend updates)
- Detailed setup instructions for both backend and frontend
- Thorough testing procedures
- Security considerations
- Future enhancement roadmap
The documentation will be valuable for maintainers and future contributors.
frontend/src/components/pages/ProfilePage.tsx (6)
18-37: LGTM!The
loadProfilefunction has proper error handling with try/catch/finally, appropriate loading state management, and correctly initializeseditedProfilefrom the fetched data.
56-68: LGTM!The cancel handler correctly resets the edited profile to the current saved values and exits edit mode.
70-95: LGTM!Good user experience with clear loading indicator and a retry mechanism for failed profile loads.
106-140: LGTM!Good UX with proper button state management - buttons are disabled during save to prevent double submission, and the save button shows contextual "Saving..." text.
159-163: LGTM!Good fallback pattern using ui-avatars.com service with proper URL encoding of the display name.
227-261: LGTM!Social platform usernames are correctly displayed as read-only fields with appropriate conditional rendering.
backend/database/02_create_users_table.sql (3)
1-38: LGTM!The table schema is well-designed with appropriate foreign key constraints, cascading deletes, and sensible defaults. The structure aligns with the frontend
UserProfileinterface.
53-68: LGTM!RLS policies correctly restrict users to viewing and updating only their own profile. The absence of INSERT/DELETE policies is appropriate since inserts happen via the
SECURITY DEFINERtrigger and deletes cascade fromauth.users.
47-51: No action needed. Theupdate_updated_at_column()function is properly defined in the previous migrationbackend/database/01_create_integration_tables.sql, which executes before this migration. The trigger will work correctly.Likely an incorrect or invalid review comment.
| const handleSave = async () => { | ||
| if (!profile) return; | ||
|
|
||
| setIsSaving(true); | ||
| try { | ||
| const updatedProfile = await apiClient.updateUserProfile(editedProfile); | ||
| setProfile(updatedProfile); | ||
| setIsEditing(false); | ||
| toast.success('Profile updated successfully!'); | ||
| } catch (error) { | ||
| console.error('Error updating profile:', error); | ||
| toast.error('Failed to update profile'); | ||
| } finally { | ||
| setIsSaving(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add client-side validation for required display_name field.
The UI marks Display Name as required (*), but handleSave doesn't validate that display_name is non-empty before submitting. This could result in a backend validation error or an empty display name being saved.
Proposed fix
const handleSave = async () => {
if (!profile) return;
+
+ if (!editedProfile.display_name?.trim()) {
+ toast.error('Display name is required');
+ return;
+ }
setIsSaving(true);
try {📝 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.
| const handleSave = async () => { | |
| if (!profile) return; | |
| setIsSaving(true); | |
| try { | |
| const updatedProfile = await apiClient.updateUserProfile(editedProfile); | |
| setProfile(updatedProfile); | |
| setIsEditing(false); | |
| toast.success('Profile updated successfully!'); | |
| } catch (error) { | |
| console.error('Error updating profile:', error); | |
| toast.error('Failed to update profile'); | |
| } finally { | |
| setIsSaving(false); | |
| } | |
| }; | |
| const handleSave = async () => { | |
| if (!profile) return; | |
| if (!editedProfile.display_name?.trim()) { | |
| toast.error('Display name is required'); | |
| return; | |
| } | |
| setIsSaving(true); | |
| try { | |
| const updatedProfile = await apiClient.updateUserProfile(editedProfile); | |
| setProfile(updatedProfile); | |
| setIsEditing(false); | |
| toast.success('Profile updated successfully!'); | |
| } catch (error) { | |
| console.error('Error updating profile:', error); | |
| toast.error('Failed to update profile'); | |
| } finally { | |
| setIsSaving(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In @frontend/src/components/pages/ProfilePage.tsx around lines 39 - 54,
handleSave currently submits without checking that editedProfile.display_name is
present; add a client-side validation at the start of handleSave to check if
editedProfile.display_name (trimmed) is non-empty, and if it is empty call
toast.error('Display Name is required') (or set a validation state) and return
early without calling apiClient.updateUserProfile or toggling setIsSaving;
ensure you trim the value, reference editedProfile.display_name in the check,
and keep the existing try/catch/finally behavior unchanged for successful paths.
IMPLEMENTATION_SUMMARY.md
Outdated
| - ✅ No breaking changes | ||
| - ✅ Backwards compatible |
There was a problem hiding this comment.
Inconsistent claim about breaking changes.
Line 135 states "No breaking changes" but the PR requires a database migration (02_create_users_table.sql) to be run before deployment. The PR summary also explicitly notes "BREAKING CHANGE: migration required." Consider updating this documentation to reflect that accurately.
Proposed fix
-- ✅ No breaking changes
+- ⚠️ Breaking change: Database migration required before deployment
- ✅ Backwards compatible📝 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.
| - ✅ No breaking changes | |
| - ✅ Backwards compatible | |
| - ⚠️ Breaking change: Database migration required before deployment | |
| - ✅ Backwards compatible |
🤖 Prompt for AI Agents
In @IMPLEMENTATION_SUMMARY.md around lines 135 - 136, The summary in
IMPLEMENTATION_SUMMARY.md incorrectly claims "No breaking changes" and
"Backwards compatible" despite the PR requiring a DB migration
(02_create_users_table.sql) and an explicit "BREAKING CHANGE: migration
required" note; update the summary to reflect that a migration is required
(e.g., replace the checkmarks with a clear statement that a migration must be
run before deployment and mark it as a breaking change), and ensure the document
references 02_create_users_table.sql so readers know which migration to apply.
1 participant
Fix User Profile Data & Add Editable Profile Page
Closes #222
📝 Description
Fixes the issue where user profile information was incorrect or missing after signup and adds a fully functional editable profile page.
Problem Solved:
Solution Implemented:
🔧 Changes Made
Backend Changes
backend/app/api/v1/users.py- User profile API endpoints (GET & PUT /v1/users/me)backend/database/02_create_users_table.sql- Database schema with triggerbackend/app/api/router.py- Registered users routerFrontend Changes
frontend/src/lib/api.ts- Added user profile API methods and TypeScript typesfrontend/src/components/pages/ProfilePage.tsx- Complete rebuild with real data integrationKey Features
🚨 Required Setup for Maintainers
STEP 1: Apply Database Migration (REQUIRED FIRST)
Run the entire
backend/database/02_create_users_table.sqlfile in your Supabase Dashboard → SQL EditorThis creates:
public.userstable with profile fieldsSTEP 2: Restart Backend Server
cd backend python -m uvicorn main:app --reload --port 8000STEP 3: Test the Flow
/signupand create a new account/profile🤝 Collaboration
Developed independently based on issue requirements.
✅ Checklist
🔒 Security Considerations
📦 Files Changed
backend/app/api/v1/users.pybackend/database/02_create_users_table.sqlbackend/app/api/router.pyfrontend/src/lib/api.tsfrontend/src/components/pages/ProfilePage.tsxSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.