feat: add profile avatar update functionality #991
Conversation
📝 WalkthroughWalkthroughImplements avatar upload and profile customization feature. Backend adds avatar routes with image processing (200x200 Lanczos resampling), validation (PNG/JPG, 5MB limit), and database persistence. Frontend provides upload UI with validation and preset avatar gallery. Consolidates response schemas (ErrorResponse, SuccessResponse) to shared common module. Changes
Sequence DiagramssequenceDiagram
participant User as User<br/>(Frontend)
participant FrontendAPI as Frontend<br/>Avatar API
participant Backend as Backend<br/>Avatar Routes
participant FileSystem as File<br/>System
participant Database as Database<br/>(Metadata)
User->>FrontendAPI: uploadAvatar(file)
FrontendAPI->>FrontendAPI: Validate type & size<br/>(PNG/JPG, ≤5MB)
alt Validation Fails
FrontendAPI-->>User: Error alert
else Validation Passes
FrontendAPI->>Backend: POST /avatars/upload<br/>(multipart/form-data)
Backend->>Backend: Validate extension<br/>& file size
alt Invalid File
Backend-->>FrontendAPI: 400 ErrorResponse
FrontendAPI-->>User: Error alert
else Valid File
Backend->>Backend: Process image<br/>(200x200, Lanczos)
Backend->>FileSystem: Save to<br/>uploads/avatars/
Backend->>Database: Update user<br/>preferences metadata
Backend-->>FrontendAPI: 200 + avatar_url
FrontendAPI->>FrontendAPI: Resolve full URL<br/>via getAvatarUrl()
FrontendAPI-->>User: onAvatarUpdate(url)<br/>+ update state
end
end
sequenceDiagram
participant User as User<br/>(Browser)
participant Backend as Backend<br/>Avatar Routes
participant FileSystem as File<br/>System
User->>Backend: GET /avatars/uploads/{filename}
Backend->>FileSystem: Locate file
alt File Found
FileSystem-->>Backend: File data
Backend-->>User: FileResponse<br/>(image/png or<br/>image/jpeg)
else File Not Found
Backend-->>User: 404 ErrorResponse
end
sequenceDiagram
participant User as User<br/>(Frontend)
participant ProfileCard as UserProfileCard<br/>Component
participant AvatarCard as AvatarUpdateCard<br/>Component
participant AvatarAPI as Avatar API
participant Backend as Backend<br/>User Prefs
User->>ProfileCard: Mount component
ProfileCard->>AvatarAPI: getUserPreferences()
AvatarAPI->>Backend: GET /user-preferences/
Backend-->>AvatarAPI: UserPreferencesData<br/>(including avatar)
AvatarAPI-->>ProfileCard: Store in state
ProfileCard->>AvatarCard: Render with<br/>currentAvatar
User->>AvatarCard: Upload new avatar
AvatarCard->>AvatarAPI: uploadAvatar(file)
AvatarAPI->>Backend: POST /avatars/upload
Backend-->>AvatarAPI: avatar_url
AvatarCard->>AvatarAPI: updateUserPreferences<br/>({ avatar: url })
AvatarAPI->>Backend: PUT /user-preferences/
Backend-->>AvatarAPI: Success
AvatarCard-->>ProfileCard: onAvatarUpdate(url)
ProfileCard->>ProfileCard: Update local state<br/>& localStorage
ProfileCard-->>User: Render updated avatar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/routes/face_clusters.py (1)
14-27: Fix import statement:ErrorResponseis not inapp.schemas.faces.All imported schemas exist in
app.schemas.facesexceptErrorResponse, which is defined inapp.schemas.common. The current import will fail at runtime. Split the imports:from app.schemas.faces import ( RenameClusterRequest, RenameClusterResponse, RenameClusterData, GetClustersResponse, GetClustersData, GlobalReclusterResponse, GlobalReclusterData, ClusterMetadata, GetClusterImagesResponse, GetClusterImagesData, ImageInCluster, ) from app.schemas.common import ErrorResponse
🤖 Fix all issues with AI agents
In @backend/app/routes/avatar.py:
- Around line 111-117: The get_avatar endpoint builds filepath with
os.path.join(UPLOAD_DIR, filename) without validating filename, allowing path
traversal; fix by rejecting any filename that is absolute or contains path
separators/parent references and by resolving the joined path and ensuring it is
inside UPLOAD_DIR before returning. Specifically, in get_avatar validate
filename (no os.sep or ".." or leading "/"), compute resolved_base =
Path(UPLOAD_DIR).resolve() and resolved_target = (resolved_base /
filename).resolve(), then check that resolved_target is inside resolved_base
(e.g., resolved_target.parts startswith or
resolved_target.as_posix().startswith(resolved_base.as_posix())), and raise
HTTPException(404) or 400 if the check fails; only call FileResponse on the
validated resolved_target.
- Around line 51-56: The current avatar saving code opens and resizes images but
may attempt to save modes with transparency or non-RGB color spaces as JPEG,
causing failures; before calling image.save(filepath, ...), ensure the PIL Image
is in RGB by checking image.mode and, if not 'RGB', call image.convert("RGB")
(handle modes like 'RGBA', 'LA', 'P', 'CMYK', etc.), then proceed to save using
the existing image.save(...) call with filepath and the same optimize/quality
options.
In @backend/app/schemas/album.py:
- Line 4: The route handlers are instantiating SuccessResponse with the wrong
field name `msg`; update all uses of SuccessResponse(...) that pass `msg=` to
instead pass `message=` so they match the schema defined in common.py; search
for `SuccessResponse(` in albums.py (the response constructions around lines
mentioned: ~158, 184, 280, 308, 348) and replace each `msg=` with `message=`.
In @backend/tests/test_avatar.py:
- Around line 53-65: The test test_upload_avatar_too_large incorrectly relies on
a compressible PNG and a lenient assertion; modify it to generate an actually
oversized payload (e.g., use create_test_image with random/noise pixels or
generate raw bytes/uncompressed BMP >5MB) and send that as the file to the
"/avatars/upload" endpoint, then assert the expected failure status (e.g.,
response.status_code == 400) to reliably verify the size-check logic in the
upload handler.
In @frontend/src/api/avatar.ts:
- Line 1: The API base URL is hardcoded in the constant API_BASE_URL; replace
this with an environment-driven value (e.g., read VITE_API_BASE_URL via
import.meta.env.VITE_API_BASE_URL) and provide a sensible fallback for local dev
if the env var is missing. Update the code that references API_BASE_URL to use
the new env-backed value and add instructions to create a .env file
(VITE_API_BASE_URL=...) for local development and set the appropriate env var in
staging/production.
🧹 Nitpick comments (10)
backend/app/schemas/common.py (1)
2-2: Remove unused import.The
Optionalimport is not used in this file.♻️ Proposed fix
from pydantic import BaseModel -from typing import Optionalbackend/app/routes/avatar.py (2)
16-16: Consider moving directory creation to application startup.Creating directories at module level executes during import and could fail if the filesystem isn't ready or permissions are incorrect. Consider moving this to a FastAPI startup event or creating the directory lazily within the endpoint.
♻️ Alternative approach using startup event
In your main application file (e.g.,
backend/main.py), add:@app.on_event("startup") async def create_upload_directories(): os.makedirs("uploads/avatars", exist_ok=True)Or create the directory lazily in the endpoint before first use.
18-19: Define response model for type safety and API documentation.The endpoint returns a dictionary but doesn't specify a
response_model. Consider creating a Pydantic model (e.g.,AvatarUploadResponse) for better type safety and OpenAPI documentation.♻️ Proposed response model
Add to your schemas file:
class AvatarUploadResponse(BaseModel): """Response model for avatar upload""" success: bool message: str avatar_url: strThen update the endpoint:
-@router.post("/upload") +@router.post("/upload", response_model=AvatarUploadResponse) async def upload_avatar(file: UploadFile = File(...)):frontend/src/constants/avatars.ts (1)
11-12: LGTM! Good practice using a named constant.The DEFAULT_AVATAR constant provides a semantic reference for the fallback avatar, which is better than hardcoding the path throughout the application. The value correctly references the first entry in the avatars array.
Optional: Add JSDoc comment for clarity
Consider adding a brief comment to document the purpose of this constant:
+/** Default avatar used as fallback when no custom avatar is uploaded */ export const DEFAULT_AVATAR = '/avatars/avatar1.png';backend/app/schemas/user_preferences.py (1)
3-3: Unused import:ErrorResponseis not referenced in this file.The
ErrorResponseimport from.commonappears to be unused in this schema file. It's likely a leftover from refactoring when the localErrorResponsedefinition was removed.🧹 Suggested fix
from pydantic import BaseModel from typing import Optional, Literal -from .common import ErrorResponsebackend/tests/test_avatar.py (2)
9-11: Consider using pytest's import configuration instead ofsys.pathmanipulation.Direct
sys.pathmodification is fragile and can cause import ordering issues. Prefer configuringpythonpathinpyproject.tomlorpytest.ini, or use aconftest.pyfixture.
67-93: Tests may have shared state issues.
test_get_user_preferences_with_avatardepends on state set bytest_update_user_preferences_with_avatarif run in sequence, but pytest doesn't guarantee test order. Each test should be independent by either resetting state or using fixtures.frontend/src/pages/SettingsPage/components/UserProfileCard.tsx (1)
52-63: Unnecessaryasyncand misleading loading state.
handleNameSaveis markedasyncbut performs only synchronous operations (localStorage.setItem,dispatch). TheisSavingstate creates misleading UX since the operation is instant. Thetry-catchwill never catch errors from these operations.♻️ Simplified implementation
- const handleNameSave = async () => { - setIsSaving(true); - try { - // Update localStorage and Redux state - localStorage.setItem('name', name); - dispatch(setName(name)); - } catch (error) { - console.error('Failed to save name:', error); - } finally { - setIsSaving(false); - } + const handleNameSave = () => { + localStorage.setItem('name', name); + dispatch(setName(name)); };frontend/src/components/AvatarUpdateCard.tsx (2)
26-30:'image/jpg'is not a standard MIME type.Browsers report
.jpgfiles as'image/jpeg'. The'image/jpg'entry is redundant and never matches. This works correctly but is misleading.🧹 Suggested fix
// Validate file type - if (!['image/png', 'image/jpeg', 'image/jpg'].includes(file.type)) { + if (!['image/png', 'image/jpeg'].includes(file.type)) { alert('Please select a PNG or JPG image'); return; }
90-96: Consider resetting the file input after upload.If a user uploads the same file again (e.g., after editing it externally), the
onChangeevent won't fire because the input value hasn't changed.♻️ Reset input after handling
const handleFileUpload = async (event: React.ChangeEvent<HTMLInputElement>) => { const file = event.target.files?.[0]; if (!file) return; + + // Reset input to allow re-selecting the same file + event.target.value = ''; // Validate file type
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
backend/app/routes/albums.pybackend/app/routes/avatar.pybackend/app/routes/face_clusters.pybackend/app/routes/folders.pybackend/app/routes/images.pybackend/app/routes/user_preferences.pybackend/app/schemas/album.pybackend/app/schemas/common.pybackend/app/schemas/faces.pybackend/app/schemas/facetagging.pybackend/app/schemas/folders.pybackend/app/schemas/images.pybackend/app/schemas/test.pybackend/app/schemas/user_preferences.pybackend/app/utils/folders.pybackend/main.pybackend/tests/test_avatar.pydocs/backend/backend_python/openapi.jsonfrontend/src/api/avatar.tsfrontend/src/components/AvatarUpdateCard.tsxfrontend/src/constants/avatars.tsfrontend/src/pages/SettingsPage/Settings.tsxfrontend/src/pages/SettingsPage/components/UserProfileCard.tsxscripts/setup.sh
💤 Files with no reviewable changes (1)
- backend/app/schemas/facetagging.py
🧰 Additional context used
🧬 Code graph analysis (13)
frontend/src/pages/SettingsPage/components/UserProfileCard.tsx (5)
frontend/src/api/avatar.ts (1)
avatarApi(15-64)frontend/src/features/onboardingSlice.ts (2)
setName(30-32)setAvatar(27-29)frontend/src/components/ui/card.tsx (1)
CardDescription(90-90)frontend/src/components/ui/avatar.tsx (3)
Avatar(51-51)AvatarImage(51-51)AvatarFallback(51-51)frontend/src/components/AvatarUpdateCard.tsx (1)
AvatarUpdateCard(14-126)
frontend/src/pages/SettingsPage/Settings.tsx (1)
frontend/src/pages/SettingsPage/components/UserProfileCard.tsx (1)
UserProfileCard(13-140)
backend/app/routes/avatar.py (2)
backend/app/database/metadata.py (2)
db_get_metadata(33-55)db_update_metadata(58-95)backend/app/schemas/common.py (1)
ErrorResponse(5-9)
backend/app/utils/folders.py (1)
backend/app/schemas/common.py (1)
ErrorResponse(5-9)
backend/app/schemas/folders.py (1)
backend/app/schemas/common.py (1)
ErrorResponse(5-9)
backend/app/routes/images.py (1)
backend/app/schemas/common.py (1)
ErrorResponse(5-9)
backend/app/routes/albums.py (1)
backend/app/schemas/common.py (2)
ErrorResponse(5-9)SuccessResponse(12-15)
backend/app/schemas/album.py (1)
backend/app/schemas/common.py (2)
ErrorResponse(5-9)SuccessResponse(12-15)
backend/app/schemas/images.py (1)
backend/app/schemas/common.py (1)
ErrorResponse(5-9)
backend/app/schemas/user_preferences.py (1)
backend/app/schemas/common.py (1)
ErrorResponse(5-9)
backend/app/schemas/faces.py (1)
backend/app/schemas/common.py (1)
ErrorResponse(5-9)
backend/app/routes/folders.py (1)
backend/app/schemas/common.py (1)
ErrorResponse(5-9)
backend/app/routes/user_preferences.py (1)
backend/app/schemas/common.py (1)
ErrorResponse(5-9)
🔇 Additional comments (22)
backend/app/schemas/folders.py (1)
3-3: LGTM! Good refactoring to use shared schema.The change to import
ErrorResponsefrom the common module properly consolidates error response schemas across the codebase.frontend/src/pages/SettingsPage/Settings.tsx (1)
7-7: LGTM! Clean integration of the avatar feature.The UserProfileCard component is properly imported and positioned logically at the top of the settings page. The integration follows the existing pattern used for other setting cards.
Also applies to: 17-19
backend/app/routes/folders.py (1)
32-32: LGTM! Good refactoring to centralize error schemas.Moving ErrorResponse to a common module reduces duplication and improves maintainability across the codebase.
backend/app/utils/folders.py (1)
11-11: LGTM! Consistent with the error schema centralization.This change aligns with the broader refactoring effort to use a shared ErrorResponse model across the application.
backend/app/routes/images.py (1)
4-4: LGTM! Completes the error schema centralization.This change is consistent with the refactoring applied across other route files, consolidating ErrorResponse into the common schemas module.
backend/app/schemas/images.py (1)
4-4: LGTM! Good refactoring to consolidate shared response schemas.The import of
ErrorResponsefrom the sharedcommonmodule eliminates duplication and aligns with the DRY principle. This consolidation pattern is consistently applied across multiple schema files in this PR.backend/app/routes/albums.py (1)
14-14: LGTM! Consistent refactoring to shared response schemas.The import of
ErrorResponseandSuccessResponsefrom the sharedcommonmodule aligns with the consolidation pattern applied throughout this PR. No functional changes to the route handlers.backend/main.py (2)
134-134: LGTM! Avatar router properly registered.The avatar router is correctly registered with the FastAPI app using the established pattern, with appropriate prefix
/avatarsand tag["Avatar"]. This aligns with the PR objectives to add avatar upload and management functionality.
28-28: Avatar router module is properly configured.The avatar router module exists at
backend/app/routes/avatar.pyand correctly defines the router. The import on line 28 is valid and will work as intended.backend/app/schemas/user_preferences.py (2)
6-11: LGTM!The
avatarfield is correctly added asOptional[str]with a default ofNone, allowing backward compatibility with existing data.
22-27: LGTM!The
avatarfield in the update request schema mirrors the data model correctly, allowing partial updates.backend/app/schemas/test.py (1)
41-41: LGTM!The removal of the local
ErrorResponsemodel aligns with the consolidation to the sharedcommon.pymodule.backend/tests/test_avatar.py (1)
23-37: LGTM!The successful upload test properly validates the response structure and avatar URL format.
frontend/src/pages/SettingsPage/components/UserProfileCard.tsx (2)
21-50: LGTM!The
useEffectproperly handles loading state, API errors with localStorage fallback, and cleanup. The empty dependency array is correct for a mount-only fetch.
65-78: LGTM!Proper async error handling with user feedback via alert. State updates for localStorage, Redux, and local state are correctly synchronized.
backend/app/routes/user_preferences.py (3)
63-66: LGTM!The validation correctly ensures at least one preference field is provided, now including
avatar.
80-82: LGTM!Avatar update handling follows the same pattern as other preference fields.
100-105: LGTM!Response construction properly includes the avatar field from the updated preferences.
frontend/src/components/AvatarUpdateCard.tsx (2)
38-49: LGTM!Proper async handling with loading state management and user-friendly error messages.
103-120: LGTM!The gallery grid is well-structured with proper visual feedback for selection state.
backend/app/schemas/faces.py (2)
6-28: LGTM!The new face tagging models are well-structured and follow the established patterns in the codebase.
97-101: LGTM!Minor formatting change with no functional impact.
| try: | ||
| image = Image.open(io.BytesIO(content)) | ||
| # Resize to 200x200 for consistency | ||
| image = image.resize((200, 200), Image.Resampling.LANCZOS) | ||
| image.save(filepath, optimize=True, quality=85) | ||
| except Exception as e: |
There was a problem hiding this comment.
Handle image modes before saving.
Images with transparency (RGBA) or other color modes (CMYK, LA, etc.) will fail when saved as JPEG since JPEG doesn't support transparency. Convert to RGB before saving to prevent runtime errors.
🐛 Proposed fix to handle different image modes
try:
image = Image.open(io.BytesIO(content))
+ # Convert to RGB if necessary (JPEG doesn't support transparency)
+ if image.mode in ('RGBA', 'LA', 'P', 'CMYK'):
+ # Create a white background for transparent images
+ if image.mode in ('RGBA', 'LA', 'P'):
+ background = Image.new('RGB', image.size, (255, 255, 255))
+ if image.mode == 'P':
+ image = image.convert('RGBA')
+ background.paste(image, mask=image.split()[-1] if image.mode in ('RGBA', 'LA') else None)
+ image = background
+ else:
+ image = image.convert('RGB')
+
# Resize to 200x200 for consistency
image = image.resize((200, 200), Image.Resampling.LANCZOS)
image.save(filepath, optimize=True, quality=85)📝 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.
| try: | |
| image = Image.open(io.BytesIO(content)) | |
| # Resize to 200x200 for consistency | |
| image = image.resize((200, 200), Image.Resampling.LANCZOS) | |
| image.save(filepath, optimize=True, quality=85) | |
| except Exception as e: | |
| try: | |
| image = Image.open(io.BytesIO(content)) | |
| # Convert to RGB if necessary (JPEG doesn't support transparency) | |
| if image.mode in ('RGBA', 'LA', 'P', 'CMYK'): | |
| # Create a white background for transparent images | |
| if image.mode in ('RGBA', 'LA', 'P'): | |
| background = Image.new('RGB', image.size, (255, 255, 255)) | |
| if image.mode == 'P': | |
| image = image.convert('RGBA') | |
| background.paste(image, mask=image.split()[-1] if image.mode in ('RGBA', 'LA') else None) | |
| image = background | |
| else: | |
| image = image.convert('RGB') | |
| # Resize to 200x200 for consistency | |
| image = image.resize((200, 200), Image.Resampling.LANCZOS) | |
| image.save(filepath, optimize=True, quality=85) | |
| except Exception as e: |
🤖 Prompt for AI Agents
In @backend/app/routes/avatar.py around lines 51 - 56, The current avatar saving
code opens and resizes images but may attempt to save modes with transparency or
non-RGB color spaces as JPEG, causing failures; before calling
image.save(filepath, ...), ensure the PIL Image is in RGB by checking image.mode
and, if not 'RGB', call image.convert("RGB") (handle modes like 'RGBA', 'LA',
'P', 'CMYK', etc.), then proceed to save using the existing image.save(...) call
with filepath and the same optimize/quality options.
| @router.get("/uploads/{filename}") | ||
| async def get_avatar(filename: str): | ||
| """Serve uploaded avatar files""" | ||
| filepath = os.path.join(UPLOAD_DIR, filename) | ||
| if not os.path.exists(filepath): | ||
| raise HTTPException(status_code=404, detail="Avatar not found") | ||
| return FileResponse(filepath) No newline at end of file |
There was a problem hiding this comment.
Path traversal vulnerability: validate filename parameter.
The filename parameter is used directly in os.path.join() without validation. An attacker could potentially access arbitrary files outside UPLOAD_DIR by using path traversal sequences like ../../../etc/passwd.
🔒 Proposed fix to prevent path traversal
@router.get("/uploads/{filename}")
async def get_avatar(filename: str):
"""Serve uploaded avatar files"""
+ # Prevent path traversal attacks
+ if ".." in filename or "/" in filename or "\\" in filename:
+ raise HTTPException(status_code=400, detail="Invalid filename")
+
filepath = os.path.join(UPLOAD_DIR, filename)
+
+ # Verify the resolved path is still within UPLOAD_DIR
+ if not os.path.abspath(filepath).startswith(os.path.abspath(UPLOAD_DIR)):
+ raise HTTPException(status_code=400, detail="Invalid filename")
+
if not os.path.exists(filepath):
raise HTTPException(status_code=404, detail="Avatar not found")
return FileResponse(filepath)🤖 Prompt for AI Agents
In @backend/app/routes/avatar.py around lines 111 - 117, The get_avatar endpoint
builds filepath with os.path.join(UPLOAD_DIR, filename) without validating
filename, allowing path traversal; fix by rejecting any filename that is
absolute or contains path separators/parent references and by resolving the
joined path and ensuring it is inside UPLOAD_DIR before returning. Specifically,
in get_avatar validate filename (no os.sep or ".." or leading "/"), compute
resolved_base = Path(UPLOAD_DIR).resolve() and resolved_target = (resolved_base
/ filename).resolve(), then check that resolved_target is inside resolved_base
(e.g., resolved_target.parts startswith or
resolved_target.as_posix().startswith(resolved_base.as_posix())), and raise
HTTPException(404) or 400 if the check fails; only call FileResponse on the
validated resolved_target.
| from pydantic import BaseModel, Field, field_validator | ||
| from typing import Optional, List | ||
| from pydantic_core.core_schema import ValidationInfo | ||
| from .common import ErrorResponse, SuccessResponse |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking SuccessResponse schema definition in common.py ==="
cat -n backend/app/schemas/common.py | grep -A 10 "class SuccessResponse"
echo -e "\n=== Checking SuccessResponse usage in albums.py around mentioned lines ==="
rg -n "SuccessResponse" backend/app/routes/albums.py -B 1 -A 1
echo -e "\n=== Detailed check of lines 158 and 184 in albums.py ==="
sed -n '155,160p' backend/app/routes/albums.py
echo "---"
sed -n '181,186p' backend/app/routes/albums.pyRepository: AOSSIE-Org/PictoPy
Length of output: 2793
Fix SuccessResponse field name in albums routes.
The SuccessResponse schema in common.py defines the field as message, but the route handlers in albums.py are using msg when instantiating responses (lines 158, 184, 280, 308, 348). Change all occurrences from msg= to message= to match the schema definition.
🤖 Prompt for AI Agents
In @backend/app/schemas/album.py at line 4, The route handlers are instantiating
SuccessResponse with the wrong field name `msg`; update all uses of
SuccessResponse(...) that pass `msg=` to instead pass `message=` so they match
the schema defined in common.py; search for `SuccessResponse(` in albums.py (the
response constructions around lines mentioned: ~158, 184, 280, 308, 348) and
replace each `msg=` with `message=`.
| def test_upload_avatar_too_large(): | ||
| """Test avatar upload with file too large""" | ||
| # Create a large image (simulate > 5MB) | ||
| large_image = create_test_image("PNG", size=(3000, 3000)) | ||
|
|
||
| response = client.post( | ||
| "/avatars/upload", | ||
| files={"file": ("large.png", large_image, "image/png")} | ||
| ) | ||
|
|
||
| # This might pass if the image is compressed enough, so we'll check the logic | ||
| # The actual size check happens in the endpoint | ||
| assert response.status_code in [200, 400] |
There was a problem hiding this comment.
Test does not reliably verify the file size limit.
A 3000×3000 PNG with a solid color compresses well below 5MB. The assertion assert response.status_code in [200, 400] makes this test pass regardless of the outcome, providing no validation that the size check works.
🐛 Suggested fix to reliably test size limit
def test_upload_avatar_too_large():
"""Test avatar upload with file too large"""
- # Create a large image (simulate > 5MB)
- large_image = create_test_image("PNG", size=(3000, 3000))
-
+ # Create content that exceeds 5MB
+ large_content = io.BytesIO(b"x" * (6 * 1024 * 1024)) # 6MB of data
+
response = client.post(
"/avatars/upload",
- files={"file": ("large.png", large_image, "image/png")}
+ files={"file": ("large.png", large_content, "image/png")}
)
-
- # This might pass if the image is compressed enough, so we'll check the logic
- # The actual size check happens in the endpoint
- assert response.status_code in [200, 400]
+
+ assert response.status_code == 400
+ data = response.json()
+ assert "size" in str(data).lower() or "large" in str(data).lower()Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @backend/tests/test_avatar.py around lines 53 - 65, The test
test_upload_avatar_too_large incorrectly relies on a compressible PNG and a
lenient assertion; modify it to generate an actually oversized payload (e.g.,
use create_test_image with random/noise pixels or generate raw
bytes/uncompressed BMP >5MB) and send that as the file to the "/avatars/upload"
endpoint, then assert the expected failure status (e.g., response.status_code ==
400) to reliably verify the size-check logic in the upload handler.
| @@ -0,0 +1,64 @@ | |||
| const API_BASE_URL = 'http://localhost:52123'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use environment variable for API base URL.
The API base URL is hardcoded to localhost:52123. This prevents the frontend from working in different environments (development, staging, production). Use an environment variable instead.
♻️ Proposed fix using environment variable
-const API_BASE_URL = 'http://localhost:52123';
+const API_BASE_URL = import.meta.env.VITE_API_BASE_URL || 'http://localhost:52123';Then create a .env file:
VITE_API_BASE_URL=http://localhost:52123And for production, set the appropriate URL.
🤖 Prompt for AI Agents
In @frontend/src/api/avatar.ts at line 1, The API base URL is hardcoded in the
constant API_BASE_URL; replace this with an environment-driven value (e.g., read
VITE_API_BASE_URL via import.meta.env.VITE_API_BASE_URL) and provide a sensible
fallback for local dev if the env var is missing. Update the code that
references API_BASE_URL to use the new env-backed value and add instructions to
create a .env file (VITE_API_BASE_URL=...) for local development and set the
appropriate env var in staging/production.
1 participant
Description
Added functionality to allow users to upload, preview, and update their profile avatar.
Changes
Screencast.from.2026-01-07.23-20-37.webm
Related Issue
Fixes #970
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.