feat: Add profile image upload and display support #1214
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds profile image upload and removal server actions, integrates profile image management in the Settings UI with FileInput, extends comment types to include authorImage, and centralizes avatar rendering by adding an imageUrl prop to the Avatar component across multiple UI areas. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (UI)
participant SettingsUI as Settings UI
participant Server as Server Action
participant S3 as S3
participant DB as Database
participant Cache as Next.js Cache
Browser->>SettingsUI: select file / click remove
SettingsUI->>SettingsUI: validate file / show preview
SettingsUI->>Server: uploadProfileImage(FormData) or removeProfileImage()
Server->>Server: auth check, validate file, generate key
Server->>S3: upload file
S3-->>Server: return URL
Server->>DB: update user.image (set URL or null)
DB-->>Server: ack
Server->>Cache: revalidate /dashboard/settings/account & /dashboard/layout
Cache-->>Server: ack
Server-->>SettingsUI: { success, imageUrl? }
SettingsUI-->>Browser: update preview / UI state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/s/[videoId]/_components/ShareVideo.tsx (1)
200-206: Add missingauthorImagefield to comment mapping.The
authorImagefield is missing from the comment objects passed toCapVideoPlayer, even though the component expects it (line 53 ofCapVideoPlayer.tsx). This prevents author images from being displayed in comment stamps.Apply this diff to include the
authorImagefield:comments={commentsData.map((comment) => ({ id: comment.id, type: comment.type, timestamp: comment.timestamp, content: comment.content, authorName: comment.authorName, + authorImage: comment.authorImage, }))}
🧹 Nitpick comments (4)
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (1)
272-277: LGTM! Consider simplifying the nullish coalescing.The refactoring to use the
Avatarcomponent consistently with theimageUrlprop is well-executed and aligns with the pattern adopted across the codebase. This simplifies the rendering logic by eliminating conditional branches.The
?? undefinedon line 275 is redundant. Since bothnullandundefinedare falsy and the Avatar component checksif (imageUrl), you can simplify this:<Avatar letterClass="text-xs lg:text-md" name={user.name ?? "User"} - imageUrl={user.image ?? undefined} + imageUrl={user.image} className="flex-shrink-0 size-[24px] text-gray-12" />If
user.imageis typed asstring | nulland TypeScript requires explicit type conversion, you could also use:- imageUrl={user.image ?? undefined} + imageUrl={user.image || undefined}apps/web/actions/account/upload-profile-image.ts (2)
49-49: Consider removing unnecessarysanitizeFilecall for images.The
sanitizeFileutility (fromapps/web/lib/sanitizeFile.ts) only performs sanitization for SVG files and returns non-SVG files unchanged. Since this action only accepts PNG and JPEG images (enforced at lines 38-40), the sanitization step is effectively a no-op that adds unnecessary overhead.Apply this diff to remove the redundant call:
- const sanitizedFile = await sanitizeFile(file); let imageUrl: string | undefined; await Effect.gen(function* () { const [bucket] = yield* S3Buckets.getBucketAccess(Option.none()); const bodyBytes = yield* Effect.promise(async () => { - const buf = await sanitizedFile.arrayBuffer(); + const buf = await file.arrayBuffer(); return new Uint8Array(buf); }); yield* bucket.putObject(fileKey, bodyBytes, { contentType: file.type, });
75-77: Simplify imageUrl validation logic.The check
typeof imageUrl !== "string" || imageUrl.length === 0is redundant sinceimageUrlis initialized asundefinedand only assigned a string value within the Effect workflow. If the workflow completes successfully,imageUrlwill always be a non-empty string.Apply this diff to simplify:
- if (typeof imageUrl !== "string" || imageUrl.length === 0) { + if (!imageUrl) { throw new Error("Failed to resolve uploaded profile image URL"); } - - const finalImageUrl = imageUrl; await db() .update(users) - .set({ image: finalImageUrl }) + .set({ image: imageUrl }) .where(eq(users.id, user.id)); revalidatePath("/dashboard/settings/account"); revalidatePath("/dashboard", "layout"); - return { success: true, imageUrl: finalImageUrl } as const; + return { success: true, imageUrl } as const;packages/ui/src/components/Avatar.tsx (1)
78-88: Consider adding error handling for broken images.The
<img>element could fail to load if the URL is invalid or the image is unavailable. Consider adding anonErrorhandler to fallback to the initials rendering.Apply this diff to add error handling:
export const Avatar: React.FC<AvatarProps> = ({ name, imageUrl, className = "", letterClass = "text-xs", }) => { + const [imageError, setImageError] = useState(false); + - if (imageUrl) { + if (imageUrl && !imageError) { return ( <img src={imageUrl} alt={name ?? "Avatar"} className={clsx("rounded-full object-cover size-4", className)} loading="lazy" referrerPolicy="no-referrer" + onError={() => setImageError(true)} /> ); }Note: You'll need to import
useStatefrom React at the top of the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/web/actions/account/remove-profile-image.ts(1 hunks)apps/web/actions/account/upload-profile-image.ts(1 hunks)apps/web/actions/videos/new-comment.ts(1 hunks)apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx(1 hunks)apps/web/app/(org)/dashboard/settings/account/Settings.tsx(5 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsx(2 hunks)apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx(1 hunks)apps/web/app/s/[videoId]/Share.tsx(1 hunks)apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx(1 hunks)apps/web/app/s/[videoId]/_components/CommentStamp.tsx(2 hunks)apps/web/app/s/[videoId]/_components/ShareHeader.tsx(1 hunks)apps/web/app/s/[videoId]/_components/ShareVideo.tsx(1 hunks)apps/web/app/s/[videoId]/_components/Sidebar.tsx(1 hunks)apps/web/app/s/[videoId]/_components/Toolbar.tsx(2 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsx(1 hunks)apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx(2 hunks)apps/web/app/s/[videoId]/page.tsx(1 hunks)apps/web/components/FileInput.tsx(5 hunks)packages/ui/src/components/Avatar.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/web/actions/videos/new-comment.tsapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/actions/account/upload-profile-image.tsapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/s/[videoId]/page.tsxapps/web/actions/account/remove-profile-image.tspackages/ui/src/components/Avatar.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/s/[videoId]/_components/ShareVideo.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/components/FileInput.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/web/actions/videos/new-comment.tsapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/actions/account/upload-profile-image.tsapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/s/[videoId]/page.tsxapps/web/actions/account/remove-profile-image.tspackages/ui/src/components/Avatar.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/s/[videoId]/_components/ShareVideo.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/components/FileInput.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/actions/videos/new-comment.tsapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/actions/account/upload-profile-image.tsapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/s/[videoId]/page.tsxapps/web/actions/account/remove-profile-image.tsapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/s/[videoId]/_components/ShareVideo.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/components/FileInput.tsx
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must execute in Next.js Server Actions under apps/web/actions; do not invoke AI providers elsewhere
Files:
apps/web/actions/videos/new-comment.tsapps/web/actions/account/upload-profile-image.tsapps/web/actions/account/remove-profile-image.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components
Files:
apps/web/actions/videos/new-comment.tsapps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/actions/account/upload-profile-image.tsapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/s/[videoId]/page.tsxapps/web/actions/account/remove-profile-image.tsapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/s/[videoId]/_components/ShareVideo.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsxapps/web/components/FileInput.tsx
apps/web/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))
Files:
apps/web/app/(org)/dashboard/settings/account/Settings.tsxapps/web/app/s/[videoId]/_components/ShareHeader.tsxapps/web/app/(org)/dashboard/_components/Navbar/Top.tsxapps/web/app/s/[videoId]/page.tsxapps/web/app/s/[videoId]/_components/CapVideoPlayer.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsxapps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsxapps/web/app/s/[videoId]/_components/ShareVideo.tsxapps/web/app/s/[videoId]/Share.tsxapps/web/app/s/[videoId]/_components/CommentStamp.tsxapps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsxapps/web/app/s/[videoId]/_components/Toolbar.tsxapps/web/app/s/[videoId]/_components/Sidebar.tsx
packages/ui/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Component files in
packages/uishould use PascalCase naming if they define React/Solid components.
Files:
packages/ui/src/components/Avatar.tsx
🧬 Code graph analysis (9)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3)
apps/web/actions/account/upload-profile-image.ts (1)
uploadProfileImage(22-94)apps/web/actions/account/remove-profile-image.ts (1)
removeProfileImage(9-25)apps/web/components/FileInput.tsx (1)
FileInput(38-283)
apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
packages/ui/src/components/Avatar.tsx (1)
Avatar(72-108)
apps/web/actions/account/upload-profile-image.ts (4)
apps/web/lib/sanitizeFile.ts (1)
sanitizeFile(4-17)apps/web/lib/server.ts (1)
runPromise(123-135)packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (1)
users(58-117)
apps/web/app/(org)/dashboard/_components/Navbar/Top.tsx (1)
packages/ui/src/components/Avatar.tsx (1)
Avatar(72-108)
apps/web/app/s/[videoId]/page.tsx (1)
packages/database/schema.ts (1)
users(58-117)
apps/web/actions/account/remove-profile-image.ts (2)
packages/database/index.ts (1)
db(18-25)packages/database/schema.ts (1)
users(58-117)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsx (1)
packages/ui/src/components/Avatar.tsx (1)
Avatar(72-108)
apps/web/app/s/[videoId]/_components/CommentStamp.tsx (1)
packages/ui/src/components/Avatar.tsx (1)
Avatar(72-108)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx (1)
packages/ui/src/components/Avatar.tsx (1)
Avatar(72-108)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (19)
apps/web/actions/videos/new-comment.ts (1)
70-70: LGTM!The addition of
authorImageto the comment response payload aligns with the PR's objective to propagate author images throughout the application.apps/web/actions/account/upload-profile-image.ts (1)
22-93: LGTM with optional improvements!The implementation correctly handles:
- Authentication and authorization
- File type and size validation
- S3 upload with proper key generation
- Multiple environment configurations for URL construction
- Database updates and cache revalidation
- Error handling and reporting
The function follows the coding guidelines for server actions and Effect usage.
apps/web/app/s/[videoId]/_components/tabs/Activity/Comment.tsx (1)
81-81: LGTM!The addition of
imageUrl={comment.authorImage ?? undefined}correctly propagates the author's image to the Avatar component, enabling avatar rendering with custom images.apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx (1)
53-53: LGTM!The addition of the optional
authorImagefield to the comment type in Props correctly extends the data shape to support avatar rendering with custom images.apps/web/app/s/[videoId]/page.tsx (1)
664-664: LGTM!The addition of
authorImage: users.imageto the comment query projection correctly fetches the author's profile image alongside other comment data, enabling avatar rendering throughout the application.apps/web/app/s/[videoId]/_components/tabs/Activity/Comments.tsx (1)
91-91: LGTM!The addition of
authorImage: user?.image ?? nullto bothoptimisticCommentandoptimisticReplyensures that optimistic UI updates correctly display author avatars during the pending state before server confirmation.Also applies to: 134-134
apps/web/actions/account/remove-profile-image.ts (1)
9-24: LGTM!The implementation correctly handles profile image removal by:
- Verifying user authentication
- Updating the database to set the image field to null
- Triggering cache revalidation for affected paths
- Returning a consistent success response
apps/web/app/s/[videoId]/_components/ShareVideo.tsx (1)
32-32: LGTM!The addition of
authorImage: string | nullto theCommentWithAuthortype correctly extends the data shape to include the author's profile image.apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)
240-245: LGTM! Unified avatar rendering.The refactor to use a single
Avatarcomponent withimageUrlprop simplifies the rendering logic and aligns with the broader architectural changes in this PR.apps/web/app/s/[videoId]/Share.tsx (1)
26-35: LGTM! Type extensions are consistent.The addition of
authorImagefields to bothCommentWithAuthorandCommentTypeproperly extends the comment data model to support author profile images. The nullable typing is appropriate for optional user images.apps/web/app/s/[videoId]/_components/CommentStamp.tsx (2)
5-13: LGTM! Type extension is consistent.The addition of
authorImage?: string | nullto the comment interface properly supports displaying author profile images in comment stamps.
64-70: LGTM! Consistent avatar rendering.The Avatar component usage with
imageUrl={comment.authorImage ?? undefined}aligns with the unified rendering approach across the application.apps/web/app/s/[videoId]/_components/Toolbar.tsx (2)
39-52: Verify the authorName change on Line 42.Line 42 changes
authorNamefromComment.CommentId.make(...)touser?.name || "Anonymous". This appears to be a bug fix (using CommentId for a name seems incorrect), but it's not mentioned in the AI-generated summary.Please confirm this is an intentional fix. If so, consider mentioning it in the PR description as it's a correctness improvement beyond just adding profile images.
Additionally, the
authorImageaddition on Line 43 is consistent with the PR objectives.
81-94: LGTM! Consistent optimistic comment creation.The addition of
authorImage: user?.image ?? nullto the optimistic text comment is consistent with the emoji comment handling and the broader PR changes.apps/web/app/s/[videoId]/_components/Sidebar.tsx (1)
16-19: LGTM! Type extension is consistent.The addition of
authorImage?: string | nullto the localCommentTypedefinition maintains consistency with the comment type extensions throughout the application.apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MemberSelect.tsx (2)
189-206: LGTM! Simplified avatar rendering.The refactor to use
AvatarwithimageUrl={opt.image}for all dropdown options eliminates conditional rendering and aligns with the unified avatar approach across the application.
216-246: LGTM! Consistent selected member rendering.The Avatar usage with
imageUrl={tag.image}for selected members matches the dropdown options pattern and maintains consistency across the component.apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx (1)
163-178: LGTM! Unified member avatar rendering.The refactor to use
AvatarwithimageUrl={member.image || undefined}simplifies the rendering logic and aligns with the consistent avatar approach throughout the PR.packages/ui/src/components/Avatar.tsx (1)
67-67: ****The review comment's concern about validating
imageUrlfor "arbitrary strings from user data" is not supported by the codebase.The
imageUrlvalues come from controlled server-side sources:
- User/comment/member records in the database (e.g.,
data.ownerImage,comment.authorImage,member.image)- These are populated via
upload-profile-image.ts, which sanitizes files server-side before generating S3 URLs- URLs are constructed deterministically from trusted AWS configuration (
CAP_AWS_BUCKET_URL,CAP_AWS_ENDPOINT, or S3 hostname) with sanitized file keysThis is a controlled server-side flow, not arbitrary untrusted input. The
referrerPolicy="no-referrer"is already in place for privacy. URL validation, CSP, or CDN proxying are not necessary for this use case.Likely an incorrect or invalid review comment.
1 participant
Introduces server actions for uploading and removing user profile images, updates account settings UI to allow image management, and refactors all avatar displays to support showing user profile images. Also propagates author images through comments and related components, ensuring consistent avatar rendering across the app.
Summary by CodeRabbit