User change password #1660
User change password #1660
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a user change password feature by consolidating user settings, adding email change functionality, and updating the Clerk authentication integration. The changes streamline the user settings interface by merging social profile fields into the main personal form and replacing the separate socials tab with email management.
- Consolidated social fields (GitHub, Discord, subscription preferences) into the personal details form
- Added a new email change feature with OTP verification using Clerk
- Updated Clerk client usage patterns across the codebase for consistency
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/server/api/routers/users/update.ts | Added GitHub, Discord, and subscription fields; enhanced error handling and Clerk metadata storage |
| src/server/api/routers/users/update-socials.ts | Removed separate socials update endpoint |
| src/server/api/routers/users/update-email.ts | Added new email update endpoint with Clerk integration |
| src/server/api/routers/users/index.ts | Replaced updateSocials with updateEmail in router exports |
| src/middleware.ts | Updated Clerk auth usage to use session-based pattern |
| src/app/profile/settings/@personal/form.tsx | Integrated social fields and subscription checkbox into personal form |
| src/app/profile/settings/@email/form.tsx | Added email change form with OTP verification |
| package.json | Updated Clerk dependencies to newer versions |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import { TRPCError } from "@trpc/server" | ||
| import { Ratelimit } from "@upstash/ratelimit" | ||
| import { eq, sql } from "drizzle-orm" | ||
| import type { InferSelectModel } from "drizzle-orm" |
There was a problem hiding this comment.
The InferSelectModel import is unused in this file. Remove the unused import to keep the code clean.
| import { useReverification, useUser } from "@clerk/nextjs" | ||
| import { EmailAddressResource } from "@clerk/types" | ||
| import { zodResolver } from "@hookform/resolvers/zod" | ||
| import { se } from "date-fns/locale" |
There was a problem hiding this comment.
The se import from date-fns/locale is unused in this file. Remove the unused import.
| import { se } from "date-fns/locale" |
| } catch (err: unknown) { | ||
| // Narrow the error type | ||
| if (err instanceof Error) { | ||
| throw new Error(`Failed to update user metadata: ${err?.message}`) |
There was a problem hiding this comment.
The optional chaining err?.message is unnecessary since you've already narrowed the type to Error in the previous condition. Use err.message directly.
| throw new Error(`Failed to update user metadata: ${err?.message}`) | |
| throw new Error(`Failed to update user metadata: ${err.message}`) |
| toast({ | ||
| title: "Update successful", | ||
| description: "Your personal details have been updated successfully.", | ||
| }) |
There was a problem hiding this comment.
The success toast is being shown in the onSubmit function, but the mutation might still fail. Move this toast to the onSuccess callback of the mutation to ensure it only shows when the update actually succeeds.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // ) | ||
|
|
||
| try { | ||
| await updateClerkUserEmail(user, clerkUser.primaryEmailAddressId) |
There was a problem hiding this comment.
The function updateClerkUserEmail is called with user and clerkUser.primaryEmailAddressId but the function definition expects userId: string, oldEmailAddressId: string, newEmailAddress: string. The parameters don't match the expected signature.
| return user | ||
| }) | ||
|
|
||
| const updateClerkUserEmail = async (user: InferSelectModel<typeof User>, oldEmailAddressId: string) => { |
There was a problem hiding this comment.
The function signature expects 2 parameters but is called with 3 parameters on line 56. Either update the function signature to accept newEmailAddress: string as the third parameter or fix the function call.
| title: "Email updated", | ||
| description: "Your email has been updated successfully.", | ||
| }) | ||
| window.location.reload() |
There was a problem hiding this comment.
Using window.location.reload() forces a full page refresh which can disrupt user experience. Consider using the router to navigate or update the UI state instead.
| github: input.github?.trim(), | ||
| discord: input.discord?.trim(), |
There was a problem hiding this comment.
The trim() method is called on potentially undefined values. This will throw an error if input.github or input.discord is undefined. Use input.github?.trim() ?? null or similar.
| github: input.github?.trim(), | |
| discord: input.discord?.trim(), | |
| github: input.github?.trim() ?? null, | |
| discord: input.discord?.trim() ?? null, |
Change Summary
[Briefly summarise the changes that you made. Just high-level stuff]
ClerkChange Form
Fill this up (NA if not available). If a certain criteria is not met, can you please give a reason.
Other Information
[Is there anything in particular in the review that I should be aware of?]