Skip to content

Conversation

@PinJinx
Copy link
Contributor

@PinJinx PinJinx commented Jan 16, 2026

Closes #175

📝 Description

This PR implements a comprehensive User Profile management system and robust database synchronization triggers.

It introduces a full-featured frontend Profile Page allowing users to view and edit their details (bio, skills, avatars) and see their social connection status. On the backend, it adds logic to handle profile updates and intelligently merge duplicate user accounts if a Discord ID links two different rows.

Additionally, this PR includes essential Database Triggers that automatically sync Supabase Auth users to our public users table upon registration and email verification, ensuring our user data is always consistent.

🔧 Changes Made

Frontend

  • New Profile Page (ProfilePage.tsx):
    • View/Edit mode toggle: Seamless switching between viewing and editing profile details.
    • Avatar Management: Added functionality to upload, replace, or remove profile pictures via Supabase Storage.
    • Skill Tags: Created a robust UI for adding "Languages" and "Frameworks" as dismissible tags. (This was implemented just because I saw how skills were stored as a dictionary in the backend and thought something like this might be what you were going for; I can change it if this is the wrong idea).
    • Social Status: Visual indicators showing if GitHub, Discord, or Slack are "Connected" or allowing manual entry in edit mode. (For now, it just asks to directly give the ID for each platform; this can be changed later where platform APIs are used to login).
  • New Component: SkillSection.tsx for reusable tag management.

Backend

  • Added get_user_details: Implemented logic to detect "Ghost" accounts (users with the same Discord ID but different User UUIDs) and merge their data dynamically so users don't lose context.
  • Added all the needed services for get_user_details and the edit endpoint to work.
  • New /edit Endpoint: Added an endpoint allowing updates to the user profile.
  • Pydantic Models: Added UserProfileEdit.

Database

  • Auth Sync Triggers:
    • add_user_to_table: Automatically inserts a row into public.users when a user signs up via Supabase Auth.
    • verify_user_on_registration: Updates is_verified status in public.users when the email is confirmed.

📷 Screenshots or Visual Changes (if applicable)

image image

🤝 Collaboration

nil

✅ Checklist

  • I have read the contributing guidelines.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if applicable).
  • Any dependent changes have been merged and published in downstream modules.

Summary by CodeRabbit

  • New Features

    • Full user profile management with in-place editing and Save/Cancel controls.
    • Avatar upload, replace and remove with progress/feedback.
    • Editable skills section (Languages & Frameworks) with add/remove.
    • Social fields (GitHub, Discord, Slack) show connected/not-connected status.
  • Chores

    • Profile data persistence and retrieval integrated with backend endpoints (requires signed-in users).

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

📝 Walkthrough

Walkthrough

Adds a token-authenticated profile API, DB functions/triggers for user sync and avatar storage policies, Supabase-backed services for fetching/updating profiles, new Pydantic models, and a fully editable frontend Profile page with avatar uploads and skill management.

Changes

Cohort / File(s) Summary
API Router
backend/app/api/router.py
Registers new profile router under /v1/profile with tag "Profile" via include_router
Profile Endpoints
backend/app/api/v1/profile.py
New APIRouter with GET / (fetch_details) and PATCH /edit (edit_details); token auth dependency, validation, structured error handling
Supabase DB Services
backend/app/database/supabase/services.py
Adds validate_user(token), get_user_details(user_id) (merges duplicate/discord-linked records), and update_user_details(user_id, updates) (field remapping and update)
DB Schema & Triggers
backend/database/01_create_integration_tables.sql
Adds avatar_path column in create script, functions/triggers to sync auth.userspublic.users, email verification trigger, RLS policies for organization_integrations, and avatars storage policies
DB Create Script
backend/app/database/supabase/scripts/create_db.sql
Adds avatar_path TEXT column to users table schema
Models
backend/app/models/database/supabase.py
Adds SkillsModel and UserProfileEdit models and avatar_path field on User model for profile validation and edit payloads
Frontend Profile UI
frontend/src/components/pages/ProfilePage.tsx
Replaces mock data with API-backed profile fetch/save, edit mode, avatar upload/removal to Supabase storage, skills management, and UI state (loading/saving)
Frontend Skill Component
frontend/src/components/ui/skills.tsx
New reusable SkillSection component for languages/frameworks with add/remove and animations
Frontend API Utils
frontend/src/lib/api.ts
Exports API_BASE_URL constant for use by frontend networking

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Frontend as Frontend App
    participant API as Backend API
    participant Auth as Supabase Auth
    participant DB as Database

    User->>Frontend: Open Profile Page
    Frontend->>Frontend: Read session token
    Frontend->>API: GET /v1/profile (Bearer token)
    API->>Auth: Validate token (supabase.auth.get_user)
    Auth-->>API: user id + metadata
    API->>DB: SELECT public.users (get_user_details)
    DB-->>API: user record(s)
    API->>API: Merge/normalize profile data
    API-->>Frontend: 200 OK with profile payload
    Frontend->>Frontend: Render profile
Loading
sequenceDiagram
    actor User
    participant Frontend as Frontend App
    participant Storage as Supabase Storage
    participant API as Backend API
    participant Auth as Supabase Auth
    participant DB as Database

    User->>Frontend: Edit profile, choose avatar
    Frontend->>Storage: Upload file to avatars bucket
    Storage-->>Frontend: public avatar URL
    Frontend->>API: PATCH /v1/profile/edit (Bearer token + payload)
    API->>Auth: Validate token
    Auth-->>API: user id
    API->>DB: UPDATE public.users (update_user_details)
    DB-->>API: updated user row
    API-->>Frontend: Updated profile response
    Frontend->>Frontend: Exit edit mode, show success
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • smokeyScraper
  • chandansgowda

Poem

🐰 New fields and routes, a hop and a cheer,
Avatars and skills now appear near and clear.
Tokens approved, triggers align,
Backend and frontend in soft carrot shine.
Hooray — profile polished, nice and dear! 🥕✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: implementing a profile page with frontend and backend services.
Linked Issues check ✅ Passed The PR comprehensively implements all coding requirements from issue #175: replaces mock frontend data with real backend API endpoints, implements backend profile retrieval/editing services, adds database synchronization triggers for user management, maintains TypeScript type safety, and consolidates Supabase interactions in the backend.
Out of Scope Changes check ✅ Passed All changes align with the linked issue #175 objectives; no out-of-scope modifications detected. The PR implements profile page features, backend services, database migrations, and API integrations as specified.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@PinJinx PinJinx changed the title feat: Implement profile edit, avatar upload, and user merging logic feat: Implement of profile page and necessary frontend and backend services Jan 16, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 13

🤖 Fix all issues with AI agents
In `@backend/app/api/v1/profile.py`:
- Around line 15-25: In get_user, remove the clear-text token logging and stop
concatenating the token into the HTTPException detail: delete the
print(f"Extracted token: {token}") and change the error detail to a generic
message (e.g., "Invalid or expired token") when validate_user(token) returns
falsy; if you need traceability, log only a truncated or hashed identifier
derived from token (never the raw token) or include context without the token
value, and keep all changes within the get_user function that uses security,
token, and validate_user.
- Around line 37-47: The returned "skills" field currently defaults to an empty
list which breaks the expected SkillsModel shape; update the response
construction in the profile endpoint(s) that build the user dict to ensure
"skills" is an object with the keys languages and frameworks (e.g.,
{"languages": [], "frameworks": []}) and, if user_details.get("skills") is
present but in a different shape, map/normalize it into that structure before
returning; apply this same normalization logic to both places that build the
user response (the two return blocks in profile.py that populate "skills").
- Around line 29-57: The fetch_details handler currently masks HTTPException and
will blow up if get_user_details returns None; after calling
get_user_details(id) in fetch_details, check if the result is None and raise an
HTTPException (e.g., 404 "User not found"); wrap the try/except so that you
catch HTTPException first and re-raise it unchanged (except HTTPException as he:
raise he), and keep a separate broad except Exception as e to log via
logger.error(...) and convert other errors into the 503 response as before;
reference fetch_details, get_user_details, HTTPException, and logger when making
these changes.

In `@backend/app/database/supabase/services.py`:
- Around line 205-207: Remove the sensitive debug print that can leak PII:
delete the line print(f"Auth response: {response}") in the logic that calls
supabase.auth.get_user(jwt=token) (the variable response and the call to
supabase.auth.get_user(jwt=token) identify the spot), and if logging is required
replace it with a non-PII message (e.g., log that auth lookup succeeded/failed)
or use a privacy-safe logger that omits user metadata before checking
response.user.
- Around line 136-140: The merge loop treats any falsy merged_user value as
"missing" (using if not merged_user.get(key)), which will overwrite valid falsy
values like False or 0; update the check in the block iterating
secondary_user.items() so you only consider a field missing when it is actually
None or an empty container/string (e.g., merged_user.get(key) is None or
merged_user.get(key) == '' or (isinstance(merged_user.get(key), list) and
len(merged_user.get(key)) == 0)), and only then assign merged_user[key] = value
if the secondary value is not None/empty; adjust the condition around
merged_user, secondary_user, and key accordingly to preserve legitimate falsy
values such as False and 0.

In `@backend/app/models/database/supabase.py`:
- Around line 243-250: The example in Config.json_schema_extra does not match
the SkillsModel shape; update the "skills" entry to be an object with
"languages" and "frameworks" arrays (e.g. "skills": {"languages": ["python"],
"frameworks": ["fastapi", "react"]}) so it matches SkillsModel; keep the
existing top-level keys (display_name, bio, github) and adjust the "skills"
structure accordingly in the Config class to reflect the nested schema used by
SkillsModel.

In `@backend/database/01_create_integration_tables.sql`:
- Around line 46-63: The INSERT into public.users sets is_verified based on
NEW.email_confirmed_at but leaves verified_at NULL; update the VALUES clause in
the INSERT (the public.users insertion) to populate verified_at as
NEW.email_confirmed_at when present (otherwise NULL) so that verified_at and
is_verified remain consistent; locate the INSERT statement referencing NEW.id,
NEW.email, NEW.raw_user_meta_data, NEW.created_at and NEW.email_confirmed_at and
add the verified_at value accordingly.
- Around line 40-44: The SECURITY DEFINER functions create_users_at_db and
verify_users_at_db need a fixed search_path to prevent privilege escalation;
update each CREATE OR REPLACE FUNCTION declaration to include a SET search_path
clause (e.g., SET search_path = pg_catalog, public or a specific schema list
appropriate for this DB) so the function runs with a locked search_path rather
than inheriting the caller's. Locate the CREATE OR REPLACE FUNCTION lines for
create_users_at_db and verify_users_at_db and add the SET search_path = ...
clause immediately in the function declaration (keeping LANGUAGE plpgsql and
SECURITY DEFINER).
- Around line 85-90: The UPDATE in the trigger uses the wrong NEW field: change
the assignment setting verified_at = NEW.confirmed_at to use the correct column
verified_at = NEW.email_confirmed_at so the trigger writes the actual email
confirmation timestamp; update the UPDATE statement that targets public.users
(the block that sets is_verified, verified_at, updated_at for WHERE id = NEW.id)
to reference NEW.email_confirmed_at and keep the rest of the logic unchanged.

In `@frontend/src/components/pages/ProfilePage.tsx`:
- Around line 112-142: handleAvatarUpload currently uploads a new avatar but
never deletes the previous file, leaving orphaned files; update
handleAvatarUpload to detect the previous avatar path from profile.avatar_url
(parse the storage object path or filename from the publicUrl), then after a
successful new upload call
supabase.storage.from('avatars').remove([oldFilePath]) (handle and log errors
but do not block the user flow), and finally update profile.avatar_url to the
new publicUrl; ensure you reference the existing symbols: handleAvatarUpload,
profile.avatar_url, supabase.storage.from('avatars').upload and
supabase.storage.from('avatars').remove, and keep setUploadingAvatar and toast
behavior unchanged.
- Around line 156-163: The early return inside handleSave after checking session
leaves isSaving true; to fix, either perform the session validation before
calling setIsSaving(true) or ensure you reset isSaving before returning (e.g.,
call setIsSaving(false) right before the toast/error return) so the finally
cleanup is not relied upon for that path; locate the handleSave function and
update the session check or add a setIsSaving(false) prior to the return so the
save button is never left disabled.
- Around line 62-63: The code returns early when supabase.auth.getSession()
yields sessionError or no session (checking sessionError and session) without
user feedback; update the ProfilePage logic to surface an error to the user
instead of silently returning by setting an error state or invoking the existing
UI alert/toast mechanism (e.g., setError/setAlert or showToast) with a message
derived from sessionError?.message or a fallback like "No active session; please
sign in", and then return; ensure the finally block still clears loading so the
UI removes the spinner while the error message is visible.
- Around line 145-148: The current handleRemoveAvatar only clears local state;
update it to also delete the file from Supabase Storage: inside
handleRemoveAvatar (or a helper it calls) derive the storage file path from
profile.avatar_url, call your Supabase client (e.g., supabase or
supabaseClient).storage.from(<your-bucket-name>).remove([filePath]), handle the
returned error/success, and then update setProfile(prev => ({...prev,
avatar_url: ''})) only after a successful delete (or still clear UI but surface
errors). Show a toast for success and for failure (include error.message) and
ensure you guard for empty/undefined avatar_url before attempting deletion.
🧹 Nitpick comments (9)
frontend/src/lib/api.ts (1)

8-9: Consider extending ApiClient instead of exporting API_BASE_URL for direct fetch calls.

While exporting API_BASE_URL works, ProfilePage.tsx now makes raw fetch() calls duplicating the auth header logic already handled by the ApiClient interceptor. Consider adding profile methods to ApiClient to maintain a single, consistent approach to API communication with built-in error handling and auth.

Example extension
// In ApiClient class
async getProfile(): Promise<UserProfile> {
    const response = await this.client.get('/v1/profile/');
    return response.data;
}

async updateProfile(data: ProfileUpdateRequest): Promise<void> {
    await this.client.patch('/v1/profile/edit', data);
}
frontend/src/components/ui/skills.tsx (3)

36-58: Use skill name as key instead of index for stable animations.

Using index as part of the key (${type}-${index}) can cause animation glitches when items are removed from the middle of the list. Since skills should be unique within each type, use the item value directly.

Suggested fix
-                <motion.span 
-                  key={`${type}-${index}`}
+                <motion.span 
+                  key={`${type}-${item}`}

49-56: Add accessible label to the remove button.

Screen readers won't understand the purpose of this button. Add an aria-label for accessibility.

Suggested fix
                   {isEditing && (
                     <button 
                       onClick={() => onRemove(type, item)}
                       className="ml-1.5 hover:text-red-400 transition-colors"
+                      aria-label={`Remove ${item}`}
                     >
                       <X size={12} />
                     </button>
                   )}

64-78: Consider making the Plus icon clickable or adding an aria-label to the input.

The Plus icon suggests interactivity but does nothing. Either make it trigger onAdd on click, or add an aria-label to the input explaining that Enter submits the value.

frontend/src/components/pages/ProfilePage.tsx (3)

118-121: Handle files without extensions gracefully.

If a file has no extension, split('.').pop() returns the filename itself (not undefined as one might expect), but files with names like "photo" will produce odd paths. Consider validating the file type or providing a fallback.

Suggested fix
      const file = event.target.files[0];
-     const fileExt = file.name.split('.').pop();
-     const fileName = `${Math.random()}.${fileExt}`;
+     const fileExt = file.name.includes('.') ? file.name.split('.').pop() : 'jpg';
+     const fileName = `${crypto.randomUUID()}.${fileExt}`;

289-293: Avoid hardcoded external fallback image URL.

Using an external Unsplash URL introduces external dependency, potential CORS issues, and privacy implications (user's browser makes requests to third-party). Consider using a local placeholder or a generic avatar component.

Suggested approach
- src={profile.avatar_url || "https://images.unsplash.com/photo-1494790108377-be9c29b29330?w=200"}
+ src={profile.avatar_url || "/default-avatar.svg"}

Add a simple SVG placeholder to your public assets.


59-109: Consider preserving original profile state for cancel functionality.

The current implementation doesn't store the original profile data, so clicking "Cancel" exits edit mode but retains any unsaved changes in the UI. Users might expect Cancel to revert their changes.

backend/app/models/database/supabase.py (1)

230-233: Avoid mutable list defaults in SkillsModel.

Class-level list defaults can be shared and will trip Ruff RUF012; use Field(default_factory=list) instead.

♻️ Proposed fix
 class SkillsModel(BaseModel):
-    languages: List[str] = []
-    frameworks: List[str] = []
+    languages: List[str] = Field(default_factory=list)
+    frameworks: List[str] = Field(default_factory=list)
backend/app/database/supabase/services.py (1)

197-199: Preserve traceback when re-raising update errors.

Use logger.exception and bare raise to keep the original stack trace.

♻️ Proposed fix
-    except Exception as e:
-        logger.error(f"Error updating user details for {user_id}: {e}")
-        raise e
+    except Exception:
+        logger.exception(f"Error updating user details for {user_id}")
+        raise
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa66519 and 133f5a3.

📒 Files selected for processing (8)
  • backend/app/api/router.py
  • backend/app/api/v1/profile.py
  • backend/app/database/supabase/services.py
  • backend/app/models/database/supabase.py
  • backend/database/01_create_integration_tables.sql
  • frontend/src/components/pages/ProfilePage.tsx
  • frontend/src/components/ui/skills.tsx
  • frontend/src/lib/api.ts
🧰 Additional context used
🧬 Code graph analysis (2)
backend/app/api/v1/profile.py (2)
backend/app/database/supabase/services.py (3)
  • get_user_details (83-154)
  • validate_user (202-226)
  • update_user_details (156-199)
backend/app/models/database/supabase.py (1)
  • UserProfileEdit (234-251)
frontend/src/components/pages/ProfilePage.tsx (3)
frontend/src/lib/supabaseClient.ts (1)
  • supabase (12-12)
frontend/src/lib/api.ts (1)
  • API_BASE_URL (8-9)
backend/app/models/database/supabase.py (1)
  • User (7-69)
🪛 Ruff (0.14.11)
backend/app/api/v1/profile.py

16-16: 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)


30-30: 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)


33-33: Unpacked variable name is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


35-35: Abstract raise to an inner function

(TRY301)


49-49: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


63-63: 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)


72-72: Abstract raise to an inner function

(TRY301)


76-79: Abstract raise to an inner function

(TRY301)


84-84: Abstract raise to an inner function

(TRY301)


100-100: Use raise without specifying exception name

Remove exception name

(TRY201)


102-102: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

backend/app/database/supabase/services.py

150-150: Consider moving this statement to an else block

(TRY300)


195-195: Consider moving this statement to an else block

(TRY300)


198-198: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


199-199: Use raise without specifying exception name

Remove exception name

(TRY201)

backend/app/models/database/supabase.py

244-251: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

🔇 Additional comments (3)
backend/database/01_create_integration_tables.sql (2)

71-77: Trigger wiring for user insert looks good.


96-104: Email-confirmation transition trigger is appropriately scoped.

backend/app/api/router.py (1)

5-5: Profile router registration looks good.

Nice, registering the new router under /v1/profile keeps the API surface organized.

Also applies to: 27-31

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@PinJinx
Copy link
Contributor Author

PinJinx commented Jan 16, 2026

Hi @smokeyScraper,

This is a fairly large PR, but I’ve done my best to keep the code clean and well-commented throughout. I have a few implementation details and setup questions I'd like your feedback on:

  1. Supabase Storage Setup For the avatar upload feature to work, we need a public bucket named avatars.

SQL Policies: I have the RLS policies ready (for authenticated uploads/public reads). Should I append them to database/01_create_integration_tables.sql in this PR, or would you prefer a separate SQL migration file?
update: I have added it along with integration with all necessary policies to create and setup the buckets

  1. Skills Structure I implemented the frontend UI to map to the backend's dictionary structure (separating "Languages" and "Frameworks"). Let me know if this structure aligns with your vision or if you prefer something else.

  2. Profile Fields I aligned the frontend fields with the current database schema. If you feel any fields are missing or unnecessary, just let me know, and I can adjust the schema and UI in a follow-up commit.

Everything has been thoroughly tested locally. I will keep an eye on the CodeRabbit review and bundle any syntax/style fixes into a single cleanup commit to keep the history tidy.

Thanks!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@frontend/src/components/pages/ProfilePage.tsx`:
- Around line 265-268: The response error handling in ProfilePage.tsx assumes
errorData.detail is a string and may render “[object Object]”; update the error
handling inside the profile update response block (the if (!response.ok) branch)
to detect when errorData.detail is an object and format a human-readable
message—e.g., extract service/status/error fields when present or fallback to
JSON.stringify(detail)—and then throw a new Error with that formatted message so
the UI shows useful text instead of “[object Object]”.
- Around line 214-224: In handleRemoveAvatar, don't throw the error from
supabase.storage.from('avatars').remove(...) because the onClick caller doesn't
catch it; instead catch the failure and show a user-facing toast (e.g.,
toast.error) with the error message and abort the rest of the flow so setProfile
and success toast are not run; locate the async function handleRemoveAvatar and
replace the current throw error path with a graceful error-handling branch that
logs or shows the error and returns early.
♻️ Duplicate comments (2)
backend/app/api/v1/profile.py (2)

20-24: Critical: Remove token from error response.

The access token is being concatenated into the error detail, which leaks credentials to the client. This is a security vulnerability.

🔒 Proposed fix
     result = await validate_user(token)
     if not result:
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED,
-            detail="Invalid or expired token"+token,
+            detail="Invalid or expired token",
         )

101-101: Return skills in the same shape as SkillsModel.

The edit_details endpoint defaults skills to [], but fetch_details (line 48) correctly uses {"languages": [], "frameworks": []}. This inconsistency breaks typed clients expecting the SkillsModel structure.

🐛 Proposed fix
-            "skills": updated_user.get("skills") or [],
+            "skills": updated_user.get("skills") or {"languages": [], "frameworks": []},
🧹 Nitpick comments (4)
backend/app/models/database/supabase.py (1)

246-257: Consider using model_config instead of nested Config class for Pydantic v2 consistency.

The IndexedRepository model (line 230) uses the Pydantic v2 style model_config = ConfigDict(...), but UserProfileEdit uses the legacy v1 class Config pattern. While both work, consistency within the file is preferred.

♻️ Suggested refactor
 class UserProfileEdit(BaseModel):
     display_name: Optional[str] = None
     bio: Optional[str] = None
     avatar_url: Optional[str] = None
     avatar_path: Optional[str] = None
     discord: Optional[str] = None
     github: Optional[str] = None
     slack: Optional[str] = None
     skills: Optional[SkillsModel] = None

-    class Config:
-        json_schema_extra = {
+    model_config = ConfigDict(
+        json_schema_extra={
             "example": {
                 "display_name": "Dev Guru",
                 "bio": "Full stack developer interested in AI",
                 "skills": {
                     "languages": ["python"],
                     "frameworks": ["fastapi", "react"]
                 },
                 "github": "devguru_code"
             }
         }
+    )
backend/app/database/supabase/services.py (2)

136-147: Merge logic fix looks good; consider removing redundant skills handling.

The falsy value check at lines 139-141 is correctly fixed. However, lines 144-145 are now redundant since the general merge loop already handles skills (empty {} is in the exclusion list).

♻️ Suggested cleanup
                 for key, value in secondary_user.items():
                     # If primary user doesn't have a value for this field (None or empty string/list)
                     # but the secondary user does, we adopt the secondary user's value.
                     primary_value = merged_user.get(key)
                     if primary_value in (None, "", [], {}) and value not in (None, "", [], {}):
                         merged_user[key] = value
-                # Special handling for Skills (Dictionaries)
-                # If both have skills, we might want to ensure we don't lose the old ones
-                if not merged_user.get("skills") and secondary_user.get("skills"):
-                     merged_user["skills"] = secondary_user["skills"]

                 return merged_user

197-199: Use bare raise to preserve the full traceback.

Using raise e can truncate the traceback in some cases. A bare raise preserves the complete exception context.

♻️ Suggested fix
     except Exception as e:
         logger.error(f"Error updating user details for {user_id}: {e}")
-        raise e
+        raise
backend/app/api/v1/profile.py (1)

52-61: Consider using logger.exception for better error diagnostics.

logging.exception automatically includes the full traceback, which aids debugging. Currently, only the exception message is logged.

♻️ Suggested improvement
     except Exception as e:
-        logger.error(f"Fetching profile details failed: {e}")
+        logger.exception("Fetching profile details failed")
         raise HTTPException(
             ...
         ) from e

Apply similarly to the edit_details exception handler.

Also applies to: 106-115

Comment on lines +214 to +224
const handleRemoveAvatar = async () => {
if (!profile.avatar_path) return;
const { error } = await supabase.storage
.from('avatars')
.remove([profile.avatar_path]);
if (error) {
throw error;
}
setProfile((prev) => ({ ...prev, avatar_url: '' ,avatar_path: ''}));
toast.success("Avatar removed. Click 'Save Changes' to apply.");
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Handle storage deletion errors gracefully.

If supabase.storage.remove fails, the throw error at line 220 will cause an unhandled promise rejection since the caller (onClick) doesn't catch it. Consider showing an error toast instead of throwing.

🐛 Proposed fix
     const handleRemoveAvatar = async () => {
         if (!profile.avatar_path) return;
-        const { error } = await supabase.storage
-            .from('avatars')
-            .remove([profile.avatar_path]);
-        if (error) {
-            throw error;
+        try {
+            const { error } = await supabase.storage
+                .from('avatars')
+                .remove([profile.avatar_path]);
+            if (error) {
+                console.error('Failed to delete avatar:', error);
+                toast.error('Failed to remove avatar from storage');
+                return;
+            }
+            setProfile((prev) => ({ ...prev, avatar_url: '', avatar_path: '' }));
+            toast.success("Avatar removed. Click 'Save Changes' to apply.");
+        } catch (err) {
+            console.error('Error removing avatar:', err);
+            toast.error('Failed to remove avatar');
         }
-        setProfile((prev) => ({ ...prev, avatar_url: '' ,avatar_path: ''}));
-        toast.success("Avatar removed. Click 'Save Changes' to apply.");
     };
📝 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.

Suggested change
const handleRemoveAvatar = async () => {
if (!profile.avatar_path) return;
const { error } = await supabase.storage
.from('avatars')
.remove([profile.avatar_path]);
if (error) {
throw error;
}
setProfile((prev) => ({ ...prev, avatar_url: '' ,avatar_path: ''}));
toast.success("Avatar removed. Click 'Save Changes' to apply.");
};
const handleRemoveAvatar = async () => {
if (!profile.avatar_path) return;
try {
const { error } = await supabase.storage
.from('avatars')
.remove([profile.avatar_path]);
if (error) {
console.error('Failed to delete avatar:', error);
toast.error('Failed to remove avatar from storage');
return;
}
setProfile((prev) => ({ ...prev, avatar_url: '', avatar_path: '' }));
toast.success("Avatar removed. Click 'Save Changes' to apply.");
} catch (err) {
console.error('Error removing avatar:', err);
toast.error('Failed to remove avatar');
}
};
🤖 Prompt for AI Agents
In `@frontend/src/components/pages/ProfilePage.tsx` around lines 214 - 224, In
handleRemoveAvatar, don't throw the error from
supabase.storage.from('avatars').remove(...) because the onClick caller doesn't
catch it; instead catch the failure and show a user-facing toast (e.g.,
toast.error) with the error message and abort the rest of the flow so setProfile
and success toast are not run; locate the async function handleRemoveAvatar and
replace the current throw error path with a graceful error-handling branch that
logs or shows the error and returns early.

Comment on lines +265 to +268
if (!response.ok) {
const errorData = await response.json();
throw new Error(errorData.detail || 'Failed to update profile');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Handle structured error responses.

The backend returns detail as either a string (for 4xx) or an object with service/status/error keys (for 503). The current code assumes it's always a string, which may display [object Object].

🐛 Proposed fix
             if (!response.ok) {
                 const errorData = await response.json();
-                throw new Error(errorData.detail || 'Failed to update profile');
+                const detail = errorData.detail;
+                const message = typeof detail === 'string' 
+                    ? detail 
+                    : detail?.error || 'Failed to update profile';
+                throw new Error(message);
             }
📝 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.

Suggested change
if (!response.ok) {
const errorData = await response.json();
throw new Error(errorData.detail || 'Failed to update profile');
}
if (!response.ok) {
const errorData = await response.json();
const detail = errorData.detail;
const message = typeof detail === 'string'
? detail
: detail?.error || 'Failed to update profile';
throw new Error(message);
}
🤖 Prompt for AI Agents
In `@frontend/src/components/pages/ProfilePage.tsx` around lines 265 - 268, The
response error handling in ProfilePage.tsx assumes errorData.detail is a string
and may render “[object Object]”; update the error handling inside the profile
update response block (the if (!response.ok) branch) to detect when
errorData.detail is an object and format a human-readable message—e.g., extract
service/status/error fields when present or fallback to
JSON.stringify(detail)—and then throw a new Error with that formatted message so
the UI shows useful text instead of “[object Object]”.

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.

FEATURE REQUEST: Connect Frontend With Backend User System

1 participant