fix: remove last name requirement + redirection to onboarding bug #952
Conversation
WalkthroughRevises onboarding and account settings flows: makes last name optional in onboarding, updates dashboard onboarding guard to allow 1-character names, and refactors account name update to use React Query mutation with local state and pending UI. No exported API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Settings as Account Settings UI
participant RQ as React Query
participant API as /api/settings/user/name
User->>Settings: Edit firstName / lastName
User->>Settings: Submit form
Settings->>RQ: mutate({ firstName, lastName })
RQ->>API: POST JSON body
API-->>RQ: 200 OK / error
alt success
RQ-->>Settings: onSuccess
Settings->>Settings: Toast "Saved"
Settings->>Settings: router.refresh()
Settings->>User: Button returns to idle
else error
RQ-->>Settings: onError
Settings->>Settings: Toast error
Settings->>User: Button returns to idle
end
sequenceDiagram
autonumber
actor User
participant Onboard as Onboarding UI
participant API as Onboarding API
User->>Onboard: Enter firstName (lastName optional)
Onboard->>Onboard: Enable Submit when firstName present
User->>Onboard: Click Submit
Onboard->>API: POST { firstName, lastName? "" }
API-->>Onboard: Response
Onboard->>User: Proceed / show error
sequenceDiagram
autonumber
participant Layout as Dashboard Layout
participant Router as Next Router
participant Store as User Data
Layout->>Store: Load user
alt not logged in
Layout->>Router: Redirect to login
else logged in
alt user.name falsy or length === 0
Layout->>Router: Redirect to onboarding
else
Layout->>Layout: Render dashboard
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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/(org)/dashboard/settings/account/Settings.tsx (1)
55-61: Make Inputs controlled (defaultValue -> value) to avoid mixed controlled/uncontrolled stateYou’re tracking firstName/lastName in state and wiring onChange, but using defaultValue leaves the Inputs uncontrolled after mount. Bind value to state to avoid React warnings and ensure consistent updates.
-<Input +<Input type="text" placeholder="First name" - onChange={(e) => setFirstName(e.target.value)} - defaultValue={firstName as string} + onChange={(e) => setFirstName(e.target.value)} + value={firstName} id="firstName" name="firstName" + maxLength={255} + disabled={updateNamePending} />-<Input +<Input type="text" placeholder="Last name" - onChange={(e) => setLastName(e.target.value)} - defaultValue={lastName as string} + onChange={(e) => setLastName(e.target.value)} + value={lastName} id="lastName" name="lastName" + maxLength={255} + disabled={updateNamePending} />Also applies to: 64-71
🧹 Nitpick comments (9)
apps/web/app/(org)/dashboard/layout.tsx (1)
29-31: Guard allows whitespace-only names; trim to avoid bypassing onboardingAs written, a name of " " passes the guard and skips onboarding. Trim before checking length to keep the fix (allow 1-char names) while preventing empty/whitespace names from slipping through.
Apply this diff:
-if (!user.name || user.name.length === 0) { +if (!user.name || user.name.trim().length === 0) { redirect("/onboarding"); }apps/web/app/(org)/onboarding/Onboarding.tsx (2)
84-94: Prevent whitespace-only first names from enabling SubmitUse trim() in the disabled predicate so " " doesn’t enable the button, aligning with the dashboard guard.
- disabled={!firstName || loading || isRedirecting} + disabled={!firstName.trim() || loading || isRedirecting}
17-25: Normalize lastName to null and consider Server Action + Query for consistencyTwo small improvements:
- Send null (not empty string) when lastName is omitted to keep the DB clean and consistent with optional semantics.
- Our guidelines ask for TanStack Query v5 mutations that call Server Actions and perform precise cache updates. Consider mirroring the Settings page approach (useMutation) or moving to a server action here as well.
Minimal normalization change:
- body: JSON.stringify({ firstName, lastName }), + body: JSON.stringify({ + firstName: firstName.trim(), + lastName: lastName.trim() ? lastName.trim() : null, + }),If you’d like, I can draft a small server action and a v5 mutation wired to it plus cache updates.
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (6)
5-5: Prefer precise cache updates; import useQueryClient for v5 cache writesGuidelines ask to avoid broad invalidations/refresh. Import useQueryClient so we can update the current user cache precisely in onSuccess.
-import { useMutation } from "@tanstack/react-query"; +import { useMutation, useQueryClient } from "@tanstack/react-query";
17-18: Wire a QueryClient and stop relying on router.refreshSet user name locally in the client cache on success to avoid a full tree reload. This also keeps UI instantly in sync.
const router = useRouter(); +const queryClient = useQueryClient();
29-32: Replace router.refresh with precise cache update to the current userUpdate the user cache keys directly; if you have a canonical key (e.g., ["currentUser"]), update that rather than refreshing the entire app.
-onSuccess: () => { - toast.success("Name updated successfully"); - router.refresh(); -}, +onSuccess: (_data) => { + toast.success("Name updated successfully"); + queryClient.setQueryData(["currentUser"], (prev: any) => + prev ? { ...prev, name: firstName.trim(), lastName: lastName.trim() ? lastName.trim() : null } : prev, + ); +},
19-23: Pass variables to mutate to avoid stale closures and centralize trimmingPassing values as variables is the recommended v5 pattern and prevents surprises if state changes between render and mutation execution.
-const { mutate: updateName, isPending: updateNamePending } = useMutation({ - mutationFn: async () => { +const { mutate: updateName, isPending: updateNamePending } = useMutation({ + mutationFn: async ({ firstName, lastName }: { firstName: string; lastName: string | null }) => { const res = await fetch("/api/settings/user/name", { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ firstName, lastName }), + body: JSON.stringify({ firstName, lastName }), }); … }, … }); - onSubmit={(e) => { + onSubmit={(e) => { e.preventDefault(); - updateName(); + updateName({ + firstName: firstName.trim(), + lastName: lastName.trim() ? lastName.trim() : null, + }); }}Also applies to: 40-44
82-88: Avoid undefined in controlled email valueCast-as-string can still pass undefined at runtime. Default to empty string to silence React warnings.
- value={user?.email as string} + value={user?.email ?? ""}
91-99: Disable Save unless trimmed firstName is non-emptyMatches the onboarding guard and prevents “space-only” submissions.
- disabled={!firstName || updateNamePending} + disabled={!firstName.trim() || updateNamePending}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/app/(org)/dashboard/layout.tsx(1 hunks)apps/web/app/(org)/dashboard/settings/account/Settings.tsx(4 hunks)apps/web/app/(org)/onboarding/Onboarding.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration
Files:
apps/web/app/(org)/onboarding/Onboarding.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/settings/account/Settings.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps/web,packages/ui}/**/*.{ts,tsx}: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'
Files:
apps/web/app/(org)/onboarding/Onboarding.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/settings/account/Settings.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/web/app/(org)/onboarding/Onboarding.tsxapps/web/app/(org)/dashboard/layout.tsxapps/web/app/(org)/dashboard/settings/account/Settings.tsx
🧬 Code graph analysis (1)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (1)
packages/database/schema.ts (1)
users(45-87)
⏰ 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 (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
| const { mutate: updateName, isPending: updateNamePending } = useMutation({ | ||
| mutationFn: async () => { | ||
| await fetch("/api/settings/user/name", { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ firstName, lastName }), | ||
| }); | ||
|
|
||
| if (response.ok) { | ||
| toast.success("Name updated successfully"); | ||
| router.refresh(); | ||
| } else { | ||
| toast.error("Failed to update name"); | ||
| } | ||
| } catch (error) { | ||
| console.error("Error updating name:", error); | ||
| toast.error("An error occurred while updating name"); | ||
| } | ||
| }; | ||
| }, | ||
| onSuccess: () => { | ||
| toast.success("Name updated successfully"); | ||
| router.refresh(); | ||
| }, | ||
| onError: () => { | ||
| toast.error("Failed to update name"); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Bug: mutation reports success on HTTP errors; check res.ok and surface server message
The mutation resolves even on 4xx/5xx, so onSuccess fires and we show a success toast on failures. Validate response.ok and throw with a meaningful message.
-const { mutate: updateName, isPending: updateNamePending } = useMutation({
- mutationFn: async () => {
- await fetch("/api/settings/user/name", {
- method: "POST",
- headers: {
- "Content-Type": "application/json",
- },
- body: JSON.stringify({ firstName, lastName }),
- });
- },
- onSuccess: () => {
- toast.success("Name updated successfully");
- router.refresh();
- },
- onError: () => {
- toast.error("Failed to update name");
- },
-});
+const { mutate: updateName, isPending: updateNamePending } = useMutation({
+ mutationFn: async () => {
+ const res = await fetch("/api/settings/user/name", {
+ method: "POST",
+ headers: { "Content-Type": "application/json" },
+ body: JSON.stringify({
+ firstName: firstName.trim(),
+ lastName: lastName.trim() ? lastName.trim() : null,
+ }),
+ });
+ if (!res.ok) {
+ const message = await res.text().catch(() => "");
+ throw new Error(message || `Failed to update name (${res.status})`);
+ }
+ return res.json().catch(() => null);
+ },
+ onSuccess: () => {
+ toast.success("Name updated successfully");
+ router.refresh();
+ },
+ onError: (err: unknown) => {
+ const message = err instanceof Error ? err.message : "Failed to update name";
+ toast.error(message);
+ },
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { mutate: updateName, isPending: updateNamePending } = useMutation({ | |
| mutationFn: async () => { | |
| await fetch("/api/settings/user/name", { | |
| method: "POST", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| body: JSON.stringify({ firstName, lastName }), | |
| }); | |
| if (response.ok) { | |
| toast.success("Name updated successfully"); | |
| router.refresh(); | |
| } else { | |
| toast.error("Failed to update name"); | |
| } | |
| } catch (error) { | |
| console.error("Error updating name:", error); | |
| toast.error("An error occurred while updating name"); | |
| } | |
| }; | |
| }, | |
| onSuccess: () => { | |
| toast.success("Name updated successfully"); | |
| router.refresh(); | |
| }, | |
| onError: () => { | |
| toast.error("Failed to update name"); | |
| }, | |
| }); | |
| const { mutate: updateName, isPending: updateNamePending } = useMutation({ | |
| mutationFn: async () => { | |
| const res = await fetch("/api/settings/user/name", { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify({ | |
| firstName: firstName.trim(), | |
| lastName: lastName.trim() ? lastName.trim() : null, | |
| }), | |
| }); | |
| if (!res.ok) { | |
| // Attempt to read server-provided error message, or fallback to status code | |
| const message = await res.text().catch(() => ""); | |
| throw new Error(message || `Failed to update name (${res.status})`); | |
| } | |
| // Return JSON if any, or null | |
| return res.json().catch(() => null); | |
| }, | |
| onSuccess: () => { | |
| toast.success("Name updated successfully"); | |
| router.refresh(); | |
| }, | |
| onError: (err: unknown) => { | |
| const message = err instanceof Error | |
| ? err.message | |
| : "Failed to update name"; | |
| toast.error(message); | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/settings/account/Settings.tsx around lines 19 to
36, the mutation currently treats any fetch completion as success; change the
mutationFn to inspect the fetch Response: await the fetch, then if (!res.ok)
read the error body (preferably as JSON and fallback to text) and throw a new
Error with that message so the mutation rejects; keep the existing
onSuccess/onError handlers but ensure onError receives the thrown Error to
surface the server message in the toast.
Summary by CodeRabbit