feat: Feature: Profile Settings / Edit Profile - #198
Conversation
|
@Oluwatos94 is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a guarded server-side /settings page and a client-side tabbed settings UI (Profile, Notifications, Wallet, Danger Zone). Extends User shape and update/delete mutations, adds client components for each tab, and wires optimistic updates, confirmation dialogs, and session/cache handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant Server as Server (/settings)
participant SettingsClient as SettingsClient (client)
participant ProfileTab as ProfileTab
participant Hooks as useUpdateUserMutation / useDeleteAccountMutation
participant AuthAPI as authClient / QueryClient
Browser->>Server: GET /settings
Server->>AuthAPI: getCurrentUser()
AuthAPI-->>Server: validated user
Server-->>Browser: render SettingsClient(user)
Browser->>SettingsClient: interact (edit / open danger zone / wallet)
SettingsClient->>ProfileTab: submit changed values
ProfileTab->>Hooks: mutateAsync(updateUser)
Hooks->>AuthAPI: authClient.updateUser(params)
AuthAPI-->>Hooks: response (success/error)
Hooks-->>ProfileTab: resolve/reject
Hooks->>AuthAPI: invalidate authKeys.session() (on success)
Browser->>SettingsClient: confirm delete account
SettingsClient->>Hooks: mutateAsync(deleteAccount)
Hooks->>AuthAPI: authClient.deleteUser()
AuthAPI-->>Hooks: success
Hooks->>AuthAPI: queryClient.clear()
Hooks-->>Browser: navigation to /auth
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
|
@Oluwatos94 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
components/settings/profile-tab.tsx (1)
75-83: Type session cache correctly to prevent shape mistakes in optimistic updates.The cache key
authKeys.session()stores session data with structure{ user?: ..., ... }, but the code retrieves and types it asProfileFormValues(the form shape). This weakens type safety on the optimistic update and rollback paths. The rollback at line 88 silently acceptspreviouswithout a type guard, which could mask mismatches at runtime.Create a
SessionCachetype matching the actual session structure and apply it consistently:🔧 Proposed refactor
+type SessionCache = { user?: Partial<ProfileFormValues> } & Record<string, unknown>; + - const previous = queryClient.getQueryData<ProfileFormValues>( + const previous = queryClient.getQueryData<SessionCache>( authKeys.session(), ); - queryClient.setQueryData(authKeys.session(), (old: unknown) => { + queryClient.setQueryData<SessionCache>(authKeys.session(), (old) => { if (!old || typeof old !== "object") return old; - const session = old as { user?: Partial<ProfileFormValues> }; + const session = old as SessionCache; return { ...session, user: { ...session.user, ...values } }; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/settings/profile-tab.tsx` around lines 75 - 83, Define a SessionCache type reflecting the actual cache shape (e.g. { user?: Partial<ProfileFormValues> } plus other session fields) and use it as the generic for queryClient.getQueryData and queryClient.setQueryData calls that reference authKeys.session(); change the local variable previous to be typed (previous: SessionCache | undefined) and add a type guard before using previous in the rollback so you only pass a correctly shaped value back to queryClient.setQueryData, keeping ProfileFormValues for form values only and ensuring all optimistic update and rollback code (the anonymous updater passed to setQueryData and the rollback path that uses previous) operate on SessionCache rather than ProfileFormValues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/settings/settings-client.tsx`:
- Around line 18-25: The profileDefaults initialization (profileDefaults)
currently sets bio, github, twitter, and website to empty strings, which causes
existing server-side values to be overwritten on save; update the flow to load
or preserve real profile data instead: either extend the User type and ensure
getCurrentUser() returns bio/github/twitter/website and use those values in
profileDefaults, or fetch the full user profile before rendering the
SettingsForm and initialize profileDefaults from that payload, or implement
dirty-tracking in the form submission logic (submit only fields that changed) so
unchanged server values aren’t sent as empty strings. Ensure you update
references to profileDefaults and any form submit handler to use the chosen
approach and the User/getCurrentUser() symbols to locate the code.
In `@components/settings/notifications-tab.tsx`:
- Around line 58-63: handleSave currently only sleeps then shows toast.success
without persisting preferences; update it so it does not claim success until a
real save occurs: inside handleSave (referencing the handleSave function,
setIsPending, and toast.success/toast.error) either call the real save API or,
if the API is not yet wired, replace the success toast with a clear
informational/error toast like "Preferences not yet saved — feature coming soon"
and avoid implying persistence; also ensure setIsPending is cleared in a finally
block and any errors use toast.error so UI state and messaging remain correct.
- Around line 84-104: The mapped list uses the shorthand Fragment <>...</> which
cannot accept keys; change the fragment wrapper around the mapped JSX to the
explicit Fragment with a key (e.g., <Fragment key={key}>) so React can properly
reconcile the items in eventKeys.map; locate the map over eventKeys in the
component (references: eventKeys, eventLabels, prefs, toggleChannel, Switch,
Label) and replace the shorthand fragment with an explicit Fragment keyed by the
current key.
In `@hooks/use-user-mutations.ts`:
- Around line 16-23: The updateUser response handling should check for an error
payload before inspecting response.data; inside the try block for
authClient.updateUser, first test response.error and if present throw a new
Error that includes response.error.message/details, then only validate
response.data and throw the generic "Failed to update user profile" if data is
missing. Update the logic around authClient.updateUser/response to prioritize
response.error and surface the real error message.
---
Nitpick comments:
In `@components/settings/profile-tab.tsx`:
- Around line 75-83: Define a SessionCache type reflecting the actual cache
shape (e.g. { user?: Partial<ProfileFormValues> } plus other session fields) and
use it as the generic for queryClient.getQueryData and queryClient.setQueryData
calls that reference authKeys.session(); change the local variable previous to
be typed (previous: SessionCache | undefined) and add a type guard before using
previous in the rollback so you only pass a correctly shaped value back to
queryClient.setQueryData, keeping ProfileFormValues for form values only and
ensuring all optimistic update and rollback code (the anonymous updater passed
to setQueryData and the rollback path that uses previous) operate on
SessionCache rather than ProfileFormValues.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e63efa0-74ae-46ee-b569-749065aed6fc
📒 Files selected for processing (7)
app/settings/page.tsxapp/settings/settings-client.tsxcomponents/settings/danger-zone-tab.tsxcomponents/settings/notifications-tab.tsxcomponents/settings/profile-tab.tsxcomponents/settings/wallet-tab.tsxhooks/use-user-mutations.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/settings/notifications-tab.tsx (1)
58-68:⚠️ Potential issue | 🟠 Major
handleSavestill doesn’t persist preferences, so the core save flow is incomplete.Line 58-Line 68 only updates UI state/toast; preferences are not written anywhere durable. Users lose changes on refresh/navigation.
🔧 Proposed direction (wire actual persistence contract)
-export function NotificationsTab() { - const [prefs, setPrefs] = useState<NotificationPrefs>(defaultPrefs); +export function NotificationsTab({ + initialPrefs = defaultPrefs, + onSave, +}: { + initialPrefs?: NotificationPrefs; + onSave: (prefs: NotificationPrefs) => Promise<void>; +}) { + const [prefs, setPrefs] = useState<NotificationPrefs>(initialPrefs); const [isPending, setIsPending] = useState(false); const handleSave = async () => { setIsPending(true); try { - // TODO: wire to backend notification preferences endpoint - toast.info( - "Notification preferences saved locally. Backend sync coming soon.", - ); + await onSave(prefs); + toast.success("Notification preferences saved."); + } catch { + toast.error("Failed to save notification preferences."); } finally { setIsPending(false); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/settings/notifications-tab.tsx` around lines 58 - 68, handleSave currently only toggles UI state and shows a toast (setIsPending, toast.info) but never persists the user's notification preferences, so changes are lost; update handleSave to call the backend persistence contract: gather the current preferences state (e.g., notificationPrefs or the form state used by this component), send an authenticated request to the notifications/preferences endpoint (using fetch/axios via your app's api client), await and check the response, handle errors (show toast.error and keep state consistent), and only clear setIsPending and show success toast after a successful save; ensure the method name handleSave and state setters remain unchanged while wiring the network call and error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/settings/profile-tab.tsx`:
- Around line 92-97: The form isn't being reset after a successful save: after
awaiting mutateAsync(changedValues) you should clear the form's dirty state by
resetting it to the saved values (either the returned payload from mutateAsync
or the changedValues) instead of leaving fields dirty; update the success path
(after mutateAsync) to call the form reset helper (e.g., reset(...) or
equivalent) so the UI and react-hook-form state match the saved session, leaving
the existing error rollback logic that uses
queryClient.setQueryData(authKeys.session(), previous) unchanged.
In `@hooks/use-user-mutations.ts`:
- Around line 16-18: Replace the forced cast by enabling the type-inference
plugin: add the inferAdditionalFields plugin to the Auth client initialization
so authClient.updateUser correctly infers its parameter type, then remove the
manual cast in the use-user-mutations call to authClient.updateUser (and drop
the workaround UpdateUserParams interface). Ensure your server auth config
declares the extra fields under user.additionalFields with input: true so the
plugin can infer them.
---
Duplicate comments:
In `@components/settings/notifications-tab.tsx`:
- Around line 58-68: handleSave currently only toggles UI state and shows a
toast (setIsPending, toast.info) but never persists the user's notification
preferences, so changes are lost; update handleSave to call the backend
persistence contract: gather the current preferences state (e.g.,
notificationPrefs or the form state used by this component), send an
authenticated request to the notifications/preferences endpoint (using
fetch/axios via your app's api client), await and check the response, handle
errors (show toast.error and keep state consistent), and only clear setIsPending
and show success toast after a successful save; ensure the method name
handleSave and state setters remain unchanged while wiring the network call and
error handling.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc691697-9d77-454e-88cc-390ed23ab895
📒 Files selected for processing (5)
app/settings/settings-client.tsxcomponents/settings/notifications-tab.tsxcomponents/settings/profile-tab.tsxhooks/use-user-mutations.tslib/server-auth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/settings/settings-client.tsx
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
A few items before merge.
components/settings/notifications-tab.tsx:58-68 is a fake save. handleSave just shows toast.info("...saved locally. Backend sync coming soon.") and there's no actual local persistence either — prefs is component state that resets on remount. This is the same UX trap pattern PRs #178 and #186 had to remove. Either disable the Save button with a [Coming soon] label, or wire it to localStorage or a real endpoint.
profile-tab.tsx:84-88 types the session cache as ProfileFormValues, but the cache actually stores { user?: ... }. Define a SessionCache type and use it on both getQueryData and setQueryData so the optimistic update and rollback stay type-safe.
The form fields use shadcn's FormField / FormItem directly. The project's chosen wrapper is FormFieldWrapper, see application-dialog.tsx. Switch to it for consistency.
use-user-mutations.ts:16-18 casts params as Parameters<typeof authClient.updateUser>[0] to pass bio, github, twitter, website. Please confirm the better-auth schema actually accepts those fields, otherwise they'll be silently dropped on the server.
Minor: lib/server-auth.ts:115 uses id: u.id! after a wider cast. The previous code didn't need that because the Zod-validated shape guaranteed id. Drop the ! and either trust the validated shape or narrow with a runtime check.
Please address all CodeRabbit findings as well.
|
Hello @Oluwatos94 |
Hi @Benjtalkshow, sorry I would fix this later today. |
Please be fast. The wave is ending in few hours. Thanks |
Yeah, but I believe the point still counts even after wave ended right? |
Kindly finlize this PR soon or i will close the PR and unassign you. |
|
@Benjtalkshow Review fixed, sorry it long. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/settings/notifications-tab.tsx (1)
70-75:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftConnect save flow to backend before showing persisted-success messaging.
At Line [74], the success toast implies durable persistence, but this path only writes to
localStorage. That misses the “persist preferences to backend” objective and will not survive cross-device/session expectations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/settings/notifications-tab.tsx` around lines 70 - 75, The save handler handleSave currently writes prefs to localStorage (STORAGE_KEY) and immediately shows a success toast; instead call the backend persistence API (e.g., a function like saveNotificationPreferences or updateUserPreferences) with prefs, await its response and verify success before calling toast.success, and only then update localStorage if you still want a client-side cache; on failure log the error (processLogger or console.error) and show toast.error, ensure setIsPending(false) runs in finally, and keep error handling around the API call so durable persistence (cross-device) is achieved rather than only localStorage.
🧹 Nitpick comments (2)
components/settings/profile-tab.tsx (1)
93-95: ⚡ Quick winUse the existing root-error slot for mutation failures.
Line 181 renders
form.formState.errors.root, but the catch path never sets it. Adding a root error gives inline feedback consistent with the form UX.🩹 Suggested tweak
} catch { queryClient.setQueryData(authKeys.session(), previous); + form.setError("root", { + type: "server", + message: "Failed to save profile changes. Please try again.", + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/settings/profile-tab.tsx` around lines 93 - 95, The catch block that reverts the optimistic session (queryClient.setQueryData(authKeys.session(), previous)) should also set a root-level form error so the UI renders form.formState.errors.root; update the catch to call the form error setter (e.g., form.setError("root", { type: "server", message: "<user-facing message or err.message>" })) or equivalent, including the server error message when available, and keep the existing queryClient revert call.lib/server-auth.ts (1)
103-108: ⚡ Quick winPrefer typing
SessionUserover assertingvalidatedSession.user.The cast at Line 103 sidesteps compile-time checks. Defining these fields on
SessionUserkeeps the parser/result contract type-safe and avoids silent drift.♻️ Suggested refactor
interface SessionUser { id?: string; name?: string | null; email?: string | null; image?: string | null; + bio?: string | null; + github?: string | null; + twitter?: string | null; + website?: string | null; } @@ - const u = validatedSession.user as typeof validatedSession.user & { - bio?: string | null; - github?: string | null; - twitter?: string | null; - website?: string | null; - }; + const u = validatedSession.user;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/server-auth.ts` around lines 103 - 108, The code currently force-casts validatedSession.user to add optional profile fields (the cast around validatedSession.user), which bypasses compile-time checks; instead extend or update the SessionUser type/interface to include bio?: string | null, github?: string | null, twitter?: string | null, website?: string | null and use that SessionUser type for validatedSession.user (or change the session return type) so the compiler knows these fields exist without assertion; locate the SessionUser/type definition and add these optional properties, then remove the cast around validatedSession.user in this file so the code uses the strongly-typed SessionUser directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/settings/notifications-tab.tsx`:
- Around line 36-43: The JSON from localStorage (read in loadPrefs using
STORAGE_KEY and cast to NotificationPrefs) must be validated/normalized before
use: instead of directly casting JSON.parse(...) to NotificationPrefs, check
required fields and types or perform a shallow/deep merge with defaultPrefs to
fill missing keys and enforce shapes, then return that validated object; apply
the same validation/merge whenever the component reads/writes prefs (the code
paths that dereference nested keys where NotificationPrefs is used) so
components never access undefined nested properties.
In `@components/settings/profile-tab.tsx`:
- Around line 22-50: The URL validators for image, github, twitter, and website
currently use z.string().url() which allows non-http(s) schemes; update each
field's schema to additionally validate the protocol by adding a refine that,
when the value is a non-empty string, constructs new URL(value) and ensures its
protocol is exactly "http:" or "https:" (return a descriptive error message like
"URL must use http or https"); keep the existing .or(z.literal("")) and
.optional() semantics so empty strings remain allowed and preserve trimming and
max rules for bio.
---
Duplicate comments:
In `@components/settings/notifications-tab.tsx`:
- Around line 70-75: The save handler handleSave currently writes prefs to
localStorage (STORAGE_KEY) and immediately shows a success toast; instead call
the backend persistence API (e.g., a function like saveNotificationPreferences
or updateUserPreferences) with prefs, await its response and verify success
before calling toast.success, and only then update localStorage if you still
want a client-side cache; on failure log the error (processLogger or
console.error) and show toast.error, ensure setIsPending(false) runs in finally,
and keep error handling around the API call so durable persistence
(cross-device) is achieved rather than only localStorage.
---
Nitpick comments:
In `@components/settings/profile-tab.tsx`:
- Around line 93-95: The catch block that reverts the optimistic session
(queryClient.setQueryData(authKeys.session(), previous)) should also set a
root-level form error so the UI renders form.formState.errors.root; update the
catch to call the form error setter (e.g., form.setError("root", { type:
"server", message: "<user-facing message or err.message>" })) or equivalent,
including the server error message when available, and keep the existing
queryClient revert call.
In `@lib/server-auth.ts`:
- Around line 103-108: The code currently force-casts validatedSession.user to
add optional profile fields (the cast around validatedSession.user), which
bypasses compile-time checks; instead extend or update the SessionUser
type/interface to include bio?: string | null, github?: string | null, twitter?:
string | null, website?: string | null and use that SessionUser type for
validatedSession.user (or change the session return type) so the compiler knows
these fields exist without assertion; locate the SessionUser/type definition and
add these optional properties, then remove the cast around validatedSession.user
in this file so the code uses the strongly-typed SessionUser directly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 518d8095-8054-4689-8d0c-0ff606ace694
📒 Files selected for processing (3)
components/settings/notifications-tab.tsxcomponents/settings/profile-tab.tsxlib/server-auth.ts
| function loadPrefs(): NotificationPrefs { | ||
| if (typeof window === "undefined") return defaultPrefs; | ||
| try { | ||
| const stored = localStorage.getItem(STORAGE_KEY); | ||
| return stored ? (JSON.parse(stored) as NotificationPrefs) : defaultPrefs; | ||
| } catch { | ||
| return defaultPrefs; | ||
| } |
There was a problem hiding this comment.
Validate stored payload shape before using it as NotificationPrefs.
At Line [40], a raw cast from JSON.parse can return partial/invalid objects; then Line [66] dereferences nested keys and can throw at runtime. Add schema/shape validation (or safe merge with defaultPrefs) before setting state.
🔧 Suggested hardening
function loadPrefs(): NotificationPrefs {
if (typeof window === "undefined") return defaultPrefs;
try {
const stored = localStorage.getItem(STORAGE_KEY);
- return stored ? (JSON.parse(stored) as NotificationPrefs) : defaultPrefs;
+ if (!stored) return defaultPrefs;
+ const parsed = JSON.parse(stored) as Partial<NotificationPrefs>;
+ return {
+ ...defaultPrefs,
+ newBounty: { ...defaultPrefs.newBounty, ...parsed.newBounty },
+ applicationUpdate: {
+ ...defaultPrefs.applicationUpdate,
+ ...parsed.applicationUpdate,
+ },
+ bountyCompleted: {
+ ...defaultPrefs.bountyCompleted,
+ ...parsed.bountyCompleted,
+ },
+ mentions: { ...defaultPrefs.mentions, ...parsed.mentions },
+ digestCadence:
+ parsed.digestCadence === "off" ||
+ parsed.digestCadence === "daily" ||
+ parsed.digestCadence === "weekly"
+ ? parsed.digestCadence
+ : defaultPrefs.digestCadence,
+ };
} catch {
return defaultPrefs;
}
}Also applies to: 64-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/settings/notifications-tab.tsx` around lines 36 - 43, The JSON
from localStorage (read in loadPrefs using STORAGE_KEY and cast to
NotificationPrefs) must be validated/normalized before use: instead of directly
casting JSON.parse(...) to NotificationPrefs, check required fields and types or
perform a shallow/deep merge with defaultPrefs to fill missing keys and enforce
shapes, then return that validated object; apply the same validation/merge
whenever the component reads/writes prefs (the code paths that dereference
nested keys where NotificationPrefs is used) so components never access
undefined nested properties.
| image: z | ||
| .string() | ||
| .trim() | ||
| .url("Must be a valid URL") | ||
| .or(z.literal("")) | ||
| .optional(), | ||
| bio: z | ||
| .string() | ||
| .trim() | ||
| .max(500, "Bio must be 500 characters or less") | ||
| .optional(), | ||
| github: z | ||
| .string() | ||
| .trim() | ||
| .url("Must be a valid URL") | ||
| .or(z.literal("")) | ||
| .optional(), | ||
| twitter: z | ||
| .string() | ||
| .trim() | ||
| .url("Must be a valid URL") | ||
| .or(z.literal("")) | ||
| .optional(), | ||
| website: z | ||
| .string() | ||
| .trim() | ||
| .url("Must be a valid URL") | ||
| .or(z.literal("")) | ||
| .optional(), |
There was a problem hiding this comment.
Restrict profile link protocols to http/https.
z.string().url() allows syntactically valid URLs, including non-web schemes. For user-supplied profile links, this weakens XSS/phishing posture if these values are later rendered into href/src.
🔒 Suggested hardening
+const optionalHttpUrl = z
+ .string()
+ .trim()
+ .url("Must be a valid URL")
+ .refine((value) => {
+ const protocol = new URL(value).protocol;
+ return protocol === "http:" || protocol === "https:";
+ }, "Must use an http(s) URL")
+ .or(z.literal(""))
+ .optional();
+
const profileSchema = z.object({
@@
- image: z
- .string()
- .trim()
- .url("Must be a valid URL")
- .or(z.literal(""))
- .optional(),
+ image: optionalHttpUrl,
@@
- github: z
- .string()
- .trim()
- .url("Must be a valid URL")
- .or(z.literal(""))
- .optional(),
+ github: optionalHttpUrl,
@@
- twitter: z
- .string()
- .trim()
- .url("Must be a valid URL")
- .or(z.literal(""))
- .optional(),
+ twitter: optionalHttpUrl,
@@
- website: z
- .string()
- .trim()
- .url("Must be a valid URL")
- .or(z.literal(""))
- .optional(),
+ website: optionalHttpUrl,
});In Zod v4, does z.string().url() accept non-http(s) schemes such as javascript: or data:?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/settings/profile-tab.tsx` around lines 22 - 50, The URL validators
for image, github, twitter, and website currently use z.string().url() which
allows non-http(s) schemes; update each field's schema to additionally validate
the protocol by adding a refine that, when the value is a non-empty string,
constructs new URL(value) and ensures its protocol is exactly "http:" or
"https:" (return a descriptive error message like "URL must use http or https");
keep the existing .or(z.literal("")) and .optional() semantics so empty strings
remain allowed and preserve trimming and max rules for bio.
Add the inferAdditionalFields client plugin mirroring the bio, github, twitter, and website fields configured on the better-auth backend. This makes authClient.updateUser accept those fields without needing the Parameters<typeof authClient.updateUser>[0] cast in use-user-mutations.ts. Note: this assumes the backend's better-auth user config has these additionalFields registered. If not, the runtime call will silently drop them regardless of the cast, so the schema must be aligned for the save to actually persist.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hooks/use-user-mutations.ts (1)
6-13: ⚡ Quick winDerive
UpdateUserParamsfrom the client instead of mirroring it manually.Now that
authClientalready infers the extra profile fields, this exported interface is a second source of truth. UsingParameters<typeof authClient.updateUser>[0]would keep the hook aligned with Better Auth’s generated input shape and remove the manual mirroring. (better-auth.com)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-user-mutations.ts` around lines 6 - 13, Replace the manually defined UpdateUserParams interface with a derived type from the auth client so it stays in sync: change usages of the exported UpdateUserParams to use Parameters<typeof authClient.updateUser>[0] (or create a type alias like type UpdateUserParams = Parameters<typeof authClient.updateUser>[0]) and export that instead; update imports/exports in hooks/use-user-mutations.ts and any places referencing UpdateUserParams to use the derived type so the hook aligns with the generated authClient shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/use-user-mutations.ts`:
- Around line 46-52: The deleteAccount helper currently calls
authClient.deleteUser() without performing a re-auth step; update the
deleteAccount function to require and accept a re-auth credential (e.g., a
password) or trigger a fresh-session reauthentication before calling
authClient.deleteUser, wiring that credential into the deleteUser call (or call
the authClient.reauthenticate/refresh method first) and surface errors
appropriately; verify the Better Auth config and ensure deleteAccount
validates/propagates the re-auth result so users on stale sessions will be
prompted for a password or a fresh session prior to invoking deleteUser.
---
Nitpick comments:
In `@hooks/use-user-mutations.ts`:
- Around line 6-13: Replace the manually defined UpdateUserParams interface with
a derived type from the auth client so it stays in sync: change usages of the
exported UpdateUserParams to use Parameters<typeof authClient.updateUser>[0] (or
create a type alias like type UpdateUserParams = Parameters<typeof
authClient.updateUser>[0]) and export that instead; update imports/exports in
hooks/use-user-mutations.ts and any places referencing UpdateUserParams to use
the derived type so the hook aligns with the generated authClient shape.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71e13b48-1698-4492-9ddf-df7a47323a63
📒 Files selected for processing (2)
hooks/use-user-mutations.tslib/auth-client.ts
| async function deleteAccount() { | ||
| const response = await authClient.deleteUser(); | ||
| if (response.error) { | ||
| throw new Error(response.error.message || "Failed to delete account"); | ||
| } | ||
| return response.data; | ||
| } |
There was a problem hiding this comment.
deleteUser() needs a re-auth proof, not just the confirmation phrase.
Better Auth only allows client-side deletion with a password or a fresh session when no password is supplied, so this helper can fail for users on a stale session unless the flow asks for re-authentication first. Please verify the auth config and wire in the required password/fresh-session step before relying on this mutation. (better-auth.com)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/use-user-mutations.ts` around lines 46 - 52, The deleteAccount helper
currently calls authClient.deleteUser() without performing a re-auth step;
update the deleteAccount function to require and accept a re-auth credential
(e.g., a password) or trigger a fresh-session reauthentication before calling
authClient.deleteUser, wiring that credential into the deleteUser call (or call
the authClient.reauthenticate/refresh method first) and surface errors
appropriately; verify the Better Auth config and ensure deleteAccount
validates/propagates the re-auth result so users on stale sessions will be
prompted for a password or a fresh session prior to invoking deleteUser.
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
All four items from the previous round are addressed cleanly. Notifications now persist to localStorage with proper SSR-safe loading, the session cache has a real SessionCache type, the form fields use FormFieldWrapper, and the id: u.id! is replaced with a proper null guard. Nice add on the form.reset(values) after save too.
Pushed a small commit (f1711ad) on your branch removing the last unsafe cast in use-user-mutations.ts. Added the inferAdditionalFields client plugin to lib/auth-client.ts mirroring bio, github, twitter, website, so authClient.updateUser is now properly typed for them and no cast is needed.
Merging this in.
2 participants
Summary
zod validation and optimistic update with rollback on error
enables)
useDeleteAccountMutation
Files Created
Files Modified
Test plan
closes #183
Summary by CodeRabbit