feat: profile photo upload - #117
Merged
Merged
Conversation
Adds the persistence the profile page needs to change a user's photo: - `profile_photo_url` on `user`, nullable, holding the R2 object key. - `PATCH /api/users/me/photo` sets or clears it. No requireRole — every role has a profile photo, unlike the student-only notification route. - `POST /api/uploads/presigned-url` takes an optional `purpose: 'report' | 'avatar'` selecting the key prefix, so avatars can be given their own lifecycle rules later. Omitting it keeps the existing `reports/` behaviour, so current callers are unaffected. The profile DTO returns a *resolved* URL (public or presigned GET), like items and claims do, which makes `toUserProfileDto` async. The validator therefore accepts only an `avatars/<uuid>.<ext>` key and rejects absolute URLs: a presigned GET expires within the hour, so a client echoing a fetched value back must fail loudly rather than persist a URL that rots. The audit entry records only whether the photo was set or cleared — the object key and URL are both prohibited audit detail values.
The button was a stub — no onClick, no file input. It now opens a file picker, uploads the image to R2 under the avatar prefix, and persists the key. A Remove button appears only when a photo is set and restores the initials fallback. The photo saves on selection rather than with the Save button. handleSave already chains two requests behind a single pass/fail message, so folding a third failure point into it would make a failed upload indistinguishable from a failed name save; and a preview that waits for Save leaves the avatar showing something the database does not have. Upload failures roll back to the previous photo and report on their own error line. The photo lives in a small external store rather than local state so the navbar avatar can track it, including the optimistic preview and rollback. Threading it as a prop would touch every page that renders the navbar and still not update live.
The account menu always rendered the mdiAccountCircle icon. It now shows the user's photo when one is set, reading it from the shared store so it updates the moment the profile page changes it — no reload, and no prop threaded through the pages that render the navbar. clearAuthSession resets the store, so a photo cannot survive into the next account signed in on the same browser.
📝 WalkthroughWalkthroughAdds profile photo storage and validation, purpose-aware avatar uploads, resolved profile-photo responses, shared client photo state, profile settings controls, and navigation avatar rendering with integration tests. ChangesProfile photo lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
actor User
participant ProfilePage
participant useProfileForm
participant UploadsAPI
participant UsersAPI
participant ProfilePhotoStore
User->>ProfilePage: select profile photo
ProfilePage->>useProfileForm: handlePhotoSelected(file)
useProfileForm->>UploadsAPI: request avatar presigned upload
useProfileForm->>UploadsAPI: upload image
useProfileForm->>UsersAPI: persist avatar object key
UsersAPI-->>useProfileForm: resolved profilePhotoUrl
useProfileForm->>ProfilePhotoStore: update shared photo
ProfilePhotoStore-->>ProfilePage: render uploaded photo
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/src/routes/users.ts (1)
425-477: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDelete the old avatar object on photo update/clear.
loadActiveUserProfile()fetches the previousprofilePhotoUrl, butPATCH /me/photoonly updates the key and never removes the prior R2 object. Add a best-effortDeleteObjectCommandforexisting.profile_photo_urlwhen it is present and different fromprofilePhotoUrlafter a successful transaction, without making request failures depend on the delete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/routes/users.ts` around lines 425 - 477, After the successful transaction in the PATCH /me/photo handler, compare existing.profile_photo_url with the new profilePhotoUrl and issue a best-effort DeleteObjectCommand for the old object when it is present and changed, including when the new value is null. Perform deletion after the database update and swallow/log deletion failures so they do not affect the response; reuse the existing R2/S3 client and object-key extraction utilities.foundit-ui/hooks/useProfileForm.ts (1)
52-92: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winDuplicate
/api/users/mefetch on profile-page mount.
useProfilePhoto()(line 58,enableddefaultstrue) fires its own fetch to/api/users/meon mount if the photo hasn't loaded yet, whileloadProfile()(lines 79-92) independently fetches the same endpoint and already callssetProfilePhoto(data.profilePhotoUrl ?? null). On a fresh session, the profile page issues two concurrent requests for the same data.Since this hook already seeds the shared store itself, disable the redundant fetch:
⚡ Proposed fix
- const photoUrl = useProfilePhoto(); + // loadProfile() below already fetches the full profile (including the + // photo) and seeds the store, so skip the hook's own fetch here. + const photoUrl = useProfilePhoto(false);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@foundit-ui/hooks/useProfileForm.ts` around lines 52 - 92, Disable the initial fetch performed by useProfilePhoto in useProfileForm, while retaining the existing loadProfile request and its setProfilePhoto(data.profilePhotoUrl ?? null) store update. Pass the hook’s enabled option as false so the profile page makes only the single /api/users/me request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@foundit-ui/app/profile/page.tsx`:
- Around line 241-262: Update handlePhotoRemove to set photoStatus to 'removing'
instead of 'uploading'. In the profile photo buttons, keep the Change Photo
loading state and “Uploading...” label limited to photoStatus === 'uploading',
and disable both buttons whenever photoStatus !== 'idle'.
- Around line 197-206: Update useProfilePhoto/profilePhotoStore to proactively
refresh profilePhotoUrl periodically while it is set, especially after the
server is ready, so signed URLs do not expire during a session. In
foundit-ui/app/profile/page.tsx (lines 197-206) and
foundit-ui/components/Navbar.tsx (lines 143-153), add onError handling to
replace failed image loads with the existing initials/icon placeholder.
---
Outside diff comments:
In `@backend/src/routes/users.ts`:
- Around line 425-477: After the successful transaction in the PATCH /me/photo
handler, compare existing.profile_photo_url with the new profilePhotoUrl and
issue a best-effort DeleteObjectCommand for the old object when it is present
and changed, including when the new value is null. Perform deletion after the
database update and swallow/log deletion failures so they do not affect the
response; reuse the existing R2/S3 client and object-key extraction utilities.
In `@foundit-ui/hooks/useProfileForm.ts`:
- Around line 52-92: Disable the initial fetch performed by useProfilePhoto in
useProfileForm, while retaining the existing loadProfile request and its
setProfilePhoto(data.profilePhotoUrl ?? null) store update. Pass the hook’s
enabled option as false so the profile page makes only the single /api/users/me
request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c0b96c9a-860f-41d5-b45f-f50d78cbb077
📒 Files selected for processing (20)
backend/docs/audit-events.mdbackend/prisma/migrations/20260725120000_add_user_profile_photo/migration.sqlbackend/prisma/schema.prismabackend/src/routes/uploads.tsbackend/src/routes/users.tsbackend/src/utils/auditEvents.tsbackend/src/validators/users.tsbackend/tests/uploads.integration.test.tsbackend/tests/users.integration.test.tsfoundit-ui/app/profile/page.tsxfoundit-ui/components/Navbar.tsxfoundit-ui/hooks/useProfileForm.tsfoundit-ui/hooks/useProfilePhoto.tsfoundit-ui/lib/api/users.tsfoundit-ui/tests/components/Navbar.test.tsxfoundit-ui/tests/hooks/useProfileForm.test.tsfoundit-ui/types/users.tsfoundit-ui/utils/auth.tsfoundit-ui/utils/handleImageUpload.tsfoundit-ui/utils/profilePhotoStore.ts
Comment on lines
+197
to
+206
| {photoUrl ? ( | ||
| <Image | ||
| src={photoUrl} | ||
| alt="" | ||
| w="80px" | ||
| h="80px" | ||
| rounded="full" | ||
| objectFit="cover" | ||
| flexShrink={0} | ||
| /> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching relevant names:\n'
git ls-files | rg '(^|/)(page\.tsx|Navbar\.tsx|users\.ts|routes\.ts|profilePhotoStore|useProfilePhoto)' || true
printf '\nProfile page Image/photoUrl usages:\n'
rg -n "photoUrl|setProfilePhoto|ProfilePhoto|onError|initials|avatar|Image" foundit-ui/app/profile foundit-ui/components/Navbar.tsx foundit-ui -g '*.tsx' -g '*.ts' || true
printf '\nRelevant snippets:\n'
for f in foundit-ui/app/profile/page.tsx foundit-ui/components/Navbar.tsx; do
echo "--- $f ---"
wc -l "$f"
ast-grep outline "$f" --view compact || true
done
echo "--- users doc/snippet ---"
f=$(fd -a 'users\.ts$' . | grep 'foundit-ui/lib/api/users.ts' | head -n1 || true)
if [ -n "${f:-}" ]; then
wc -l "$f"
sed -n '1,180p' "$f"
else
fd 'users\.ts$' .
fiRepository: 86unj/Foundit
Length of output: 30690
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- backend users routes profile photo references ---'
rg -n "profilePhoto|avatars|profile photo|profile photo|photo" backend/src/routes users.ts backend/src/routes/users.ts backend/src/routes/admin/users.ts || true
echo '--- full users.ts ---'
sed -n '1,120p' foundit-ui/lib/api/users
echo '--- profile page relevant sections ---'
sed -n '160,240p' foundit-ui/app/profile/page.tsx
echo '--- navbar relevant sections ---'
sed -n '110,160p' foundit-ui/components/Navbar.tsx
echo '--- profile photo store/hook ---'
sed -n '1,90p' foundit-ui/utils/profilePhotoStore.ts
sed -n '1,75p' foundit-ui/hooks/useProfilePhoto.ts
echo '--- auth clear/clearProfilePhoto references ---'
sed -n '1,50p' foundit-ui/utils/auth.ts
echo '--- tests around fallback/loading/expiration relevant assertions ---'
rg -n "expired|expires|onError|clearProfilePhoto|photoUrl|avatar|initials|account" foundit-ui/tests foundit-ui -g '*.tsx' -g '*.ts' || trueRepository: 86unj/Foundit
Length of output: 6731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- frontend files ---'
sed -n '1,120p' foundit-ui/lib/api/users.ts
sed -n '160,240p' foundit-ui/app/profile/page.tsx
sed -n '110,160p' foundit-ui/components/Navbar.tsx
sed -n '1,90p' foundit-ui/utils/profilePhotoStore.ts
sed -n '1,75p' foundit-ui/hooks/useProfilePhoto.ts
sed -n '1,50p' foundit-ui/utils/auth.ts
echo '--- backend users/uploads relevant snippets ---'
sed -n '1,80p' backend/src/routes/users.ts
sed -n '360,470p' backend/src/routes/users.ts
sed -n '1,120p' backend/src/routes/uploads.ts
rg -n "EXPIRES|expires|Expires|PRESIGN|Presigned|duration|ttl|ttlSeconds|profilePhotoUrl|avatar" backend/src -g '*.ts' || true
echo '--- frontend tests/profile photo relevant ---'
sed -n '60,105p' foundit-ui/tests/components/Navbar.test.tsx
sed -n '1,180p' foundit-ui/tests/hooks/useProfileForm.test.tsRepository: 86unj/Foundit
Length of output: 32754
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- backend imageUrl resolver ---'
sed -n '1,80p' backend/src/utils/imageUrl.ts
echo '--- imports/constants that determine signed/base URL behavior ---'
rg -n "R2_PUBLIC|PUBLIC_BASE|AWS|S3_PUBLIC|baseUrl|publicBase|bucket|resolveImageUrl|getSignedUrl|expiresIn" backend/src -g '*.ts' || trueRepository: 86unj/Foundit
Length of output: 4496
Refresh expired profile photo URLs and add fallback UI coverage.
When R2_PUBLIC_BASE_URL is unset, profilePhotoUrl is a signed R2 GET URL with a 1-hour lifetime, but foundit-ui loads and caches it once per session in useProfilePhoto/profilePhotoStore. After the server is ready, refresh the URL periodically/proactively while profilePhotoUrl is set, and add an onError fallback to both foundit-ui/app/profile/page.tsx and foundit-ui/components/Navbar.tsx so failed photo loads fall back to the initials/icon placeholder.
📍 Affects 2 files
foundit-ui/app/profile/page.tsx#L197-L206(this comment)foundit-ui/components/Navbar.tsx#L143-L153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@foundit-ui/app/profile/page.tsx` around lines 197 - 206, Update
useProfilePhoto/profilePhotoStore to proactively refresh profilePhotoUrl
periodically while it is set, especially after the server is ready, so signed
URLs do not expire during a session. In foundit-ui/app/profile/page.tsx (lines
197-206) and foundit-ui/components/Navbar.tsx (lines 143-153), add onError
handling to replace failed image loads with the existing initials/icon
placeholder.
Comment on lines
+241
to
+262
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| borderColor="gray.300" | ||
| loading={photoStatus === 'uploading'} | ||
| loadingText="Uploading..." | ||
| onClick={() => photoInputRef.current?.click()} | ||
| > | ||
| Change Photo | ||
| </Button> | ||
|
|
||
| {photoUrl && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| color="red.600" | ||
| disabled={photoStatus === 'uploading'} | ||
| onClick={handlePhotoRemove} | ||
| > | ||
| Remove | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
"Change Photo" shows "Uploading..." while removing a photo.
Both handlePhotoSelected and handlePhotoRemove set the shared photoStatus to 'uploading', so clicking "Remove" also puts the "Change Photo" button into its loading state with the label "Uploading...", which is misleading since nothing is being uploaded.
🔧 Proposed fix (distinguish busy reasons)
- const [photoStatus, setPhotoStatus] = useState<'idle' | 'uploading'>('idle');
+ const [photoStatus, setPhotoStatus] = useState<
+ 'idle' | 'uploading' | 'removing'
+ >('idle');Then set 'removing' in handlePhotoRemove and gate the "Change Photo" button's loading/loadingText on photoStatus === 'uploading' only, while using photoStatus !== 'idle' for disabling both buttons.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@foundit-ui/app/profile/page.tsx` around lines 241 - 262, Update
handlePhotoRemove to set photoStatus to 'removing' instead of 'uploading'. In
the profile photo buttons, keep the Change Photo loading state and
“Uploading...” label limited to photoStatus === 'uploading', and disable both
buttons whenever photoStatus !== 'idle'.
This was referenced Jul 29, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wires up the Change Photo button on
/profile, which was a stub with noonClickand no file input, and shows the resulting photo in the navbar avatar.There was nowhere to store a photo — no column on
user, nothing in the profile DTO — so this spans backend and frontend.Behaviour
avatars/prefix, and persisted immediately.Notable decisions
The photo saves on selection, not with the Save button.
handleSavealready chains two requests behind a single pass/fail message; folding a third failure point in would make a failed upload indistinguishable from a failed name save. A preview that waits for Save also leaves the avatar showing something the database does not have.The stored value is an object key; the API returns a resolved URL. This matches how items and claims expose images, and makes
toUserProfileDtoasync. The validator therefore accepts only anavatars/<uuid>.<ext>key and rejects absolute URLs — a presigned GET expires within the hour, so a client echoing a fetched value back must fail loudly rather than persist a URL that rots.The photo lives in a small external store rather than component state. Threading it as a prop would touch every page rendering the navbar and still not update live.
clearAuthSessionresets it so a photo cannot survive into the next account on the same browser.purposeon the presigned-URL route is optional and defaults toreport, so every existing caller is unchanged.Migration
20260725120000_add_user_profile_photo— one nullableVARCHAR(500), no data rewritten. Already applied to the shared Neon dev database viamigrate deploy. Existing code never selects the column, so nothing else is affected.Verification
Backend 173 tests, frontend 85; typecheck, lint and build clean in both packages. Each of the three commits typechecks standalone.
Driven in headless Chromium against the real API and real R2: upload → avatar renders and decodes → persists across reload → navbar updates without a reload → visible on other pages → Remove reverts everything. Both validator guards confirmed live (400 on an echoed URL, 400 on a
reports/key).Two bugs surfaced while writing the tests and are fixed here:
'constructor' in KEY_PREFIX_BY_PURPOSEreturned true via the prototype chain, and the module-level store was silently satisfying later test assertions.Known gap
Replacing a photo orphans the previous R2 object — nothing deletes it. The existing report-photo flow has the same gap, so it is left as-is rather than widening scope here. Worth a follow-up ticket.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests