feat(desktop): rework account settings with editable profile#944
feat(desktop): rework account settings with editable profile#944saddlepaddle merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughThe changes introduce client-side user data management in the AccountSettings component using live queries, replace static UI with dynamic profile editing for name and avatar uploads, add two new TRPC endpoints for profile and avatar updates with validation and blob storage integration, and remove the version display feature from settings search. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant React as AccountSettings Component
participant LiveQuery as Live Query
participant API as TRPC API
participant Blob as Blob Storage
participant DB as Database
User->>React: Load Account Settings
React->>LiveQuery: Subscribe to user data
LiveQuery->>DB: Fetch current user
DB-->>LiveQuery: Return user data
LiveQuery-->>React: Emit user snapshot
React->>React: Render profile with name/avatar
User->>React: Edit name & blur
React->>API: Call updateProfile({ name })
API->>DB: Update user name
DB-->>API: Return updated user
API-->>React: Return success + updated user
React->>React: Show toast confirmation
User->>React: Click upload avatar
React->>React: Open file picker
User->>React: Select image
React->>React: Encode to base64
React->>API: Call uploadAvatar({ fileData, fileName, mimeType })
API->>API: Validate MIME type & size
API->>Blob: Delete old avatar (if exists)
API->>Blob: Upload new image
Blob-->>API: Return blob URL
API->>DB: Update user.image with blob URL
DB-->>API: Return updated user
API-->>React: Return success + new URL
React->>React: Update avatar preview + show toast
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
94034ca to
a2e5856
Compare
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
- Add updateProfile and uploadAvatar mutations to user router - Remove version section from account settings - Add editable name field with onBlur save - Add avatar upload with hover edit overlay - Use Electric SQL collections for real-time data sync
a2e5856 to
fe99469
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/trpc/src/router/user/user.ts`:
- Around line 101-124: The upload flow can leave an orphaned blob if put(...)
succeeds but the subsequent db.update(users)...where(...) fails; after calling
put(pathname, buffer, ... ) capture the blob result and, if the DB update
throws, call the storage deletion routine for that blob (use the same identifier
you passed to put — e.g., pathname or blob.url) to remove the uploaded file, log
any deletion errors without masking the original DB error, and then rethrow the
TRPCError as currently done; update the try/catch around put and db.update to
perform this cleanup using the put return value (blob) and ensure deletion
failures are handled separately.
🧹 Nitpick comments (4)
packages/trpc/src/router/user/user.ts (1)
68-87: Extract magic values to named constants.Per coding guidelines, magic numbers and hardcoded values should be extracted to named constants at module top for clarity and maintainability.
Suggested refactor
+const ALLOWED_AVATAR_MIME_TYPES = ["image/png", "image/jpeg", "image/webp"]; +const MAX_AVATAR_SIZE_MB = 4.5; + export const userRouter = {Then update the usage:
- const allowedMimeTypes = ["image/png", "image/jpeg", "image/webp"]; - if (!allowedMimeTypes.includes(input.mimeType)) { + if (!ALLOWED_AVATAR_MIME_TYPES.includes(input.mimeType)) {- if (sizeInMB > 4.5) { + if (sizeInMB > MAX_AVATAR_SIZE_MB) { throw new TRPCError({ code: "BAD_REQUEST", - message: `File too large (${sizeInMB.toFixed(2)}MB). Maximum size is 4.5MB`, + message: `File too large (${sizeInMB.toFixed(2)}MB). Maximum size is ${MAX_AVATAR_SIZE_MB}MB`,apps/desktop/src/renderer/routes/_authenticated/settings/account/components/AccountSettings/AccountSettings.tsx (3)
41-46: Consider filtering in the query instead of post-fetch.The current approach fetches all users and filters client-side. If the collection grows, this could become inefficient. Consider filtering by
currentUserIdin the query if the live query API supports it.
79-81: Log errors with context before showing toast.Per coding guidelines, errors should not be swallowed silently. Add logging with context for debugging.
Suggested fix
- } catch { + } catch (error) { + console.error("[AccountSettings/handleAvatarUpload] Failed:", error); toast.error("Failed to update avatar"); }
95-98: Log errors with context before showing toast.Same as above - add error logging for observability.
Suggested fix
- } catch { + } catch (error) { + console.error("[AccountSettings/handleNameBlur] Failed:", error); toast.error("Failed to update name"); setNameValue(user.name ?? ""); }
| try { | ||
| const blob = await put(pathname, buffer, { | ||
| access: "public", | ||
| contentType: input.mimeType, | ||
| }); | ||
|
|
||
| const [updatedUser] = await db | ||
| .update(users) | ||
| .set({ image: blob.url }) | ||
| .where(eq(users.id, userId)) | ||
| .returning(); | ||
|
|
||
| return { | ||
| success: true, | ||
| url: blob.url, | ||
| user: updatedUser, | ||
| }; | ||
| } catch (error) { | ||
| console.error("[user/uploadAvatar] Upload failed:", error); | ||
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", | ||
| message: "Failed to upload avatar", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Potential orphan blob if DB update fails.
If put() succeeds but the subsequent db.update() fails, the uploaded blob remains in storage without being referenced. Consider cleaning up the blob on DB failure to prevent storage leaks.
Suggested fix
try {
const blob = await put(pathname, buffer, {
access: "public",
contentType: input.mimeType,
});
- const [updatedUser] = await db
- .update(users)
- .set({ image: blob.url })
- .where(eq(users.id, userId))
- .returning();
+ let updatedUser;
+ try {
+ [updatedUser] = await db
+ .update(users)
+ .set({ image: blob.url })
+ .where(eq(users.id, userId))
+ .returning();
+ } catch (dbError) {
+ // Clean up orphan blob
+ await del(blob.url).catch(() => {});
+ throw dbError;
+ }
return {
success: true,
url: blob.url,
user: updatedUser,
};🤖 Prompt for AI Agents
In `@packages/trpc/src/router/user/user.ts` around lines 101 - 124, The upload
flow can leave an orphaned blob if put(...) succeeds but the subsequent
db.update(users)...where(...) fails; after calling put(pathname, buffer, ... )
capture the blob result and, if the DB update throws, call the storage deletion
routine for that blob (use the same identifier you passed to put — e.g.,
pathname or blob.url) to remove the uploaded file, log any deletion errors
without masking the original DB error, and then rethrow the TRPCError as
currently done; update the try/catch around put and db.update to perform this
cleanup using the put return value (blob) and ensure deletion failures are
handled separately.
Summary
updateProfileanduploadAvatarmutations to user router for editing user profileTest plan
Summary by CodeRabbit
Release Notes
New Features
Improvements
Removed
✏️ Tip: You can customize this high-level summary in your review settings.