feat: Enhance User Profile Form with Session Data and Update Action #55
Conversation
WalkthroughProfileForm was converted to an action-based client flow that accepts an optional Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant F as ProfileForm (client)
participant A as useAction (client)
participant S as updateProfile (server)
participant T as Toast
U->>F: Click submit
activate F
F->>A: execute(formData)
activate A
rect #f0f8ff
Note over F,A: isExecuting = true (submit disabled)
end
A->>S: invoke updateProfile(ctx.session, formData)
activate S
alt success
S-->>A: success response
A-->>T: toast.success(message)
else error
S-->>A: error
A-->>T: toast.error(message)
end
deactivate S
deactivate A
rect #f5fff0
Note over F,A: isExecuting = false (submit enabled)
end
deactivate F
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/models/user/ProfileForm.tsx (1)
20-22: Schema mismatch: Email field validated but not updated.The
profileFormSchemavalidates the email field and it's included in form submissions, but theupdateProfileaction insrc/actions/user.tsonly updates thenamefield. Since the email input is disabled (line 58), consider either:
- Removing email from the form schema and only validate/submit
name, or- Clarifying why email validation is necessary for a read-only field
The current implementation unnecessarily validates and submits email data that the action ignores.
Apply this diff to remove email from the schema if it's truly read-only:
const profileFormSchema = z.object({ name: z .string() .min(2, { message: 'Username must be at least 2 characters.' }) .max(30, { message: 'Username must not be longer than 30 characters.' - }), - email: z.string().email({ - message: 'Please enter a valid email address.' - }) + }) });And update the default values:
const form = useForm<ProfileFormValues>({ resolver: zodResolver(profileFormSchema), defaultValues: { - name: user.name, - email: user.email + name: user.name } });Also applies to: 40-41, 58-58
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/models/user/ProfileForm.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/models/user/ProfileForm.tsx (2)
src/hooks/use-action.ts (1)
useAction(6-22)src/actions/user.ts (1)
updateProfile(19-29)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/models/user/ProfileForm.tsx (1)
1-9: Critical: Missing 'use client' directive.This component uses client-side hooks (useForm, useAction) and features (toast notifications) but lacks the 'use client' directive. In Next.js App Router, components are server components by default, and this will cause runtime errors.
Add the directive at the top of the file:
+'use client'; + import { Button } from '@/components/ui/button';
🧹 Nitpick comments (3)
src/components/models/user/ProfileForm.tsx (3)
27-27: Consider tightening the user prop type.The current type allows both the object and its properties to be optional (
user?: { name?: string; email?: string }), which is overly permissive. Consider making the properties required when the object is provided.-export function ProfileForm({ user }: { user?: { name?: string; email?: string } }) { +export function ProfileForm({ user }: { user?: { name: string; email: string } }) {This provides a clearer contract: if a user object is provided, it must include both name and email properties.
32-34: Consider more comprehensive error handling.The error handling covers basic cases but doesn't fully utilize next-safe-action's error structure (serverError, validationErrors, fetchError). While the current approach works for server errors due to
handleServerErrorconfiguration, validation errors would not display field-specific messages.onError: (error) => { - const message = typeof error === 'string' ? error : error?.message ?? 'Failed to update profile'; - toast.error(message); + if (error?.serverError) { + toast.error(error.serverError); + } else if (error?.validationErrors) { + // Handle field-specific validation errors + const firstError = Object.values(error.validationErrors)[0]; + toast.error(Array.isArray(firstError) ? firstError[0] : 'Validation failed'); + } else { + toast.error('Failed to update profile'); + } }
11-23: Email field is validated but not used in updates.The email field is included in the form schema with validation (lines 20-22) and submitted to the action (line 47), but the
updateProfileaction only updates the name field (as shown in src/actions/user.ts lines 23-24). Since the email input is disabled (line 59) and read-only, it shouldn't be validated or submitted.Based on learnings
Remove email from the schema since it's display-only:
const profileFormSchema = z.object({ name: z .string() .min(2, { message: 'Username must be at least 2 characters.' }) .max(30, { message: 'Username must not be longer than 30 characters.' - }), - email: z.string().email({ - message: 'Please enter a valid email address.' - }) + }) });And update the form to handle email separately:
const form = useForm<ProfileFormValues>({ resolver: zodResolver(profileFormSchema), defaultValues: { name: user?.name ?? '' } });Keep the email field in JSX for display but outside the form validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/models/user/ChangePasswordForm.tsx(1 hunks)src/components/models/user/ProfileForm.tsx(3 hunks)src/contexts/LoadingContext.tsx(1 hunks)src/lib/action.ts(1 hunks)src/lib/session.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/models/user/ProfileForm.tsx (1)
src/actions/user.ts (1)
updateProfile(19-29)
🔇 Additional comments (5)
src/components/models/user/ChangePasswordForm.tsx (1)
1-2: LGTM! Correct client directive for interactive form.The 'use client' directive is necessary since this component uses React hooks (useForm, useState, useAction) and client-side features (toast notifications).
src/lib/session.ts (1)
1-2: LGTM! Correct server directive for session management.The 'use server' directive is appropriate since this module uses server-only APIs (cookies() from next/headers) and handles sensitive session data that must never execute on the client.
src/contexts/LoadingContext.tsx (1)
1-2: LGTM! Correct client directive for React Context.The 'use client' directive is necessary since this module uses React hooks (createContext, useContext, useState) which require client-side execution.
src/components/models/user/ProfileForm.tsx (1)
62-62: LGTM! Proper loading state management.Disabling the button while the action executes prevents double submissions and provides clear feedback to users.
src/lib/action.ts (1)
27-27: No issues found. Headers removal is safe.Verification confirms that
ctx.headersis never accessed anywhere in the codebase. All actions and middleware exclusively accessctx.sessionandctx.user, both of which remain available in the middleware context. The change at line 27 does not break any existing functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/models/user/ProfileForm.tsx (1)
55-59: Remove dead code for email error display.Since
profileFormSchema,form.formState.errors.emailwill never be populated. Line 58 is unreachable dead code.Apply this diff to remove the dead code:
<div className='grid gap-2'> <Label htmlFor='email'>Email</Label> <Input id='email' type='email' {...form.register('email')} disabled /> - {form.formState.errors.email && <p className='text-sm text-red-500'>{form.formState.errors.email.message}</p>} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/models/user/ProfileForm.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/models/user/ProfileForm.tsx (1)
src/actions/user.ts (1)
updateProfile(19-29)
🔇 Additional comments (7)
src/components/models/user/ProfileForm.tsx (7)
1-1: LGTM!The
'use client'directive is correctly added for a component using client-side hooks likeuseFormanduseAction.
9-11: LGTM!The imports are correct and align with the patterns used in other components (SignupForm, SigninForm, Header).
13-22: LGTM!The schema correctly validates only the
namefield, which aligns with theupdateProfileaction that only updates the name. The email field rendered in the UI is intentionally disabled and read-only.
26-26: LGTM!The optional
userprop with proper TypeScript typing addresses the validation concerns from previous reviews.
27-35: LGTM!The
useActionhook is correctly implemented with:
- Proper import from
next-safe-action/hooks- Robust error handling with type checks
- User-friendly toast notifications for success and error states
44-46: LGTM!The submit handler correctly passes form data to the
executefunction, which only includes thenamefield as expected by theupdateProfileaction.
60-60: LGTM!The submit button correctly disables during action execution, preventing duplicate submissions and providing clear user feedback.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/models/user/ProfileForm.tsx (1)
45-47: Clean submission flow and proper button state management.The form submission correctly calls
execute(data)and the button properly reflects the executing state withdisabled={isExecuting}. The form data includes the email field, but since the server action only processes thenamefield (and the email is disabled), this works correctly—the server's schema validation will handle the input appropriately.Optional refinement: Since the email field is disabled and only for display, you could simplify by not registering it with the form (just render as
<Input value={user?.email ?? ''} disabled />). However, the current implementation works correctly.Also applies to: 61-61
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/actions/analytics.ts(3 hunks)src/components/models/user/ProfileForm.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/actions/analytics.ts (1)
src/lib/action.ts (1)
authActionClient(31-63)
src/components/models/user/ProfileForm.tsx (2)
src/hooks/use-action.ts (1)
useAction(6-22)src/actions/user.ts (1)
updateProfile(19-29)
🪛 GitHub Actions: Lint and Build
src/actions/analytics.ts
[error] 9-9: Type error: The 'this' context of type 'SafeActionClient<string, "flattened", ZodObject<{ actionName: ZodString; }, $strip>, { actionName: string; }, false, { session: IronSession; } & { ...; }, ... 4 more ..., { ...; }>' is not assignable to method's 'this' of type 'never'.
🪛 GitHub Check: lint-and-build
src/actions/analytics.ts
[warning] 9-9:
'ctx' is defined but never used. Allowed unused args must match /^_/u
🔇 Additional comments (6)
src/components/models/user/ProfileForm.tsx (3)
1-1: LGTM! Correct imports and client directive.The component correctly uses the 'use client' directive and imports
useActionfromnext-safe-action/hooks, matching the pattern used in other forms (SignupForm, SigninForm). This addresses previous concerns about using the wrong hook.Also applies to: 9-11
26-35: Excellent improvements! All previous issues addressed.The implementation now correctly:
- Makes the
userprop optional with proper defaults- Uses
useActionfromnext-safe-action/hookswith the correct API (execute,isExecuting)- Includes robust error handling that checks error types before displaying
- Provides user feedback via toast notifications
This resolves all critical and major issues from previous reviews.
37-43: Proper form initialization with both fields.The form correctly initializes both
nameanddefaultValues, ensuring the disabled email field displays the user's email address. This addresses the previous issue where the email field would appear empty.src/actions/analytics.ts (3)
1-5: Imports updated appropriately for the new action client pattern.The switch from
protectedActiontoauthActionClientand the addition of Zod are consistent with the new schema-based action pattern.
29-32: Critical: MissingProjectTypeimport, but suggested fix has incorrect import path.The code uses
ProjectTypeon line 29 without importing it. However, the suggested import path in the review comment is wrong. Based on usage throughout the codebase,ProjectTypeshould be imported from@/generated/prisma, not@prisma/client.Correct import:
import { prisma } from '@/lib/prisma'; import { authActionClient } from '@/lib/action'; import { z } from 'zod'; +import { ProjectType } from '@/generated/prisma';Likely an incorrect or invalid review comment.
9-43: Clarify analytics scope and resolve unusedctxparameter.The verification confirms the review comment is accurate:
ctxis accepted but never referenced in the function- All queries retrieve global data (no user filtering)
- This pattern is unique; all other
authActionClientactions extract and usectx.session.user.idHowever, the code design is ambiguous. The function is called globally on the home page (after authentication) while user-specific queries are handled separately, suggesting analytics may be intentionally global. Without documentation, this is unclear.
Decide: Are analytics intended to be global (all users see same stats) or user-scoped (each user sees their own stats)? Then either:
- If global: Rename
ctxto_ctxand add a comment explaining the intentional design- If user-scoped: Add
.where({ userId: ctx.session.user.id })filtering to the Prisma queries
|
The inline validation messages in the profile form will definitely improve user feedback; nice touch. |
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 (2)
src/actions/analytics.ts (2)
18-26: Monthly aggregation is incorrect (groups by exact timestamps, not months).
groupBy({ by: ['createdAt'] })yields near-unique buckets. Formatting to "MMM YYYY" after the fact doesn’t aggregate counts. Use DB-level month truncation or aggregate in application code.Postgres example:
const rows = await prisma.$queryRaw<{ month: Date; count: number }[]>` SELECT date_trunc('month', "createdAt") AS month, count(*)::int AS count FROM "Project" /* + optional WHERE for tenant scope */ GROUP BY 1 ORDER BY 1 ASC `; const formatted = rows.map(r => ({ name: r.month.toLocaleString('default', { month: 'short', year: 'numeric' }), value: r.count }));If portability is required, fetch createdAt and reduce in JS into month buckets.
9-26: Add user scope to all analytics queries to prevent data leakage.The three queries read across the entire Project table without filtering by the user. Currently, any authenticated user can see analytics for all projects in the database. Your authActionClient middleware provides
_ctx.user.id, which should be used to filter queries:const userId = _ctx.user.id; const totalProjects = await prisma.project.count({ where: { userId } }); const projectsByType = await prisma.project.groupBy({ by: ['type'], where: { userId }, _count: { type: true } }); const projectsByCreationMonth = await prisma.project.groupBy({ by: ['createdAt'], where: { userId }, _count: { createdAt: true }, orderBy: { createdAt: 'asc' } });
🧹 Nitpick comments (1)
src/actions/analytics.ts (1)
11-15: Prefer counting_alland nullish coalescing; minor perf tidy.
- Use
_count._allto represent group size robustly (avoids subtle null cases).- Use
?? 0instead of|| 0.- Optional: build a Map for O(1) lookups when mapping enum values.
Apply this diff:
const projectsByType = await prisma.project.groupBy({ by: ['type'], _count: { - type: true + _all: true } }); const formattedProjectsByType = Object.values(ProjectType).map((type) => ({ name: type, - value: projectsByType.find((item) => item.type === type)?._count.type || 0 + value: projectsByType.find((item) => item.type === type)?._count._all ?? 0 }));Optional Map pattern:
const counts = new Map(projectsByType.map(({ type, _count }) => [type, _count._all])); const formatted = Object.values(ProjectType).map(type => ({ name: type, value: counts.get(type) ?? 0 }));Also applies to: 28-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/actions/analytics.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/actions/analytics.ts (1)
src/lib/action.ts (1)
authActionClient(31-63)
🪛 GitHub Actions: Lint and Build
src/actions/analytics.ts
[error] 8-8: Type error: The 'this' context of type 'SafeActionClient<...>' is not assignable to method's 'this' of type 'never'.
🔇 Additional comments (2)
src/actions/analytics.ts (2)
5-6: Good fix on imports.Importing
ProjectTypeand zod resolves earlier missing-type issues and aligns with the current action pattern.
8-8: Fix action callback params to resolve the build error.You're destructuring non-existent keys (
_parsedInput,_ctx). Per next-safe-action library, the callback receivesparsedInputandctx(among other fields). This mismatch forces a wrong overload and triggers the'this' context ... never'error.Apply this diff:
-export const getProjectAnalytics = authActionClient.schema(z.object({})).action(async ({ _parsedInput, _ctx }) => { +export const getProjectAnalytics = authActionClient + .schema(z.object({})) + .action(async ({ parsedInput: _parsedInput, ctx: _ctx }) => {Alternatively, if the params aren't needed:
action(async () => { ... })oraction(async (_args) => { ... }).After applying, verify with:
pnpm typecheck
| projectsByType: formattedProjectsByType, | ||
| projectsByCreationMonth: formattedProjectsByCreationMonth |
There was a problem hiding this comment.
🧩 Analysis chain
API shape change: verify front-end consumers.
You now return projectsByType (formerly status-based). Ensure all callers/renderers are updated (labels, keys, legends).
🏁 Script executed:
#!/bin/bash
# Find consumers of analytics payload keys
rg -nP --type=ts --type=tsx -C2 '\bprojectsBy(Type|Status)\b'Length of output: 27
I'll generate corrected shell scripts to search for consumers of these analytics keys without the unsupported file type flag:
---
🏁 Script executed:
```shell
#!/bin/bash
# Find consumers of analytics payload keys using glob patterns
rg -nP -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx' 'projectsByType|projectsByStatus' -C2
Length of output: 2324
🏁 Script executed:
#!/bin/bash
# Also check the analytics.ts file to understand the full context of the change
cat src/actions/analytics.ts | head -60Length of output: 1201
🏁 Script executed:
#!/bin/bash
# Search for files importing from analytics
rg -n "from.*analytics|import.*analytics" -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Length of output: 191
🏁 Script executed:
#!/bin/bash
# Look for any references to the analytics function/export name
rg -nP "getAnalytics|analytics" -g '*.ts' -g '*.tsx' -C1 | head -100Length of output: 806
I found a critical mismatch. Let me verify how the data is being passed to the component:
🏁 Script executed:
#!/bin/bash
# Check how page.tsx is passing analytics result to ProjectStats
cat src/app/(general)/page.tsx | grep -A 20 "getProjectAnalytics\|ProjectStats"Length of output: 166
🏁 Script executed:
#!/bin/bash
# Get the full page.tsx to see the component usage
cat src/app/(general)/page.tsxLength of output: 117
🏁 Script executed:
#!/bin/bash
# Find and display the page.tsx file using fd
fd 'page.tsx' src/app --exec cat {}Length of output: 2869
🏁 Script executed:
#!/bin/bash
# Find HomePageContainer to see how ProjectStats is used
fd 'HomePageContainer' --type f -e tsx -e ts | head -5Length of output: 47
🏁 Script executed:
#!/bin/bash
# Find and display HomePageContainer
fd 'HomePageContainer.tsx' --exec cat {}Length of output: 1306
🏁 Script executed:
#!/bin/bash
# Search for ALL usages of ProjectStats component
rg -n "ProjectStats" -g '*.ts' -g '*.tsx'Length of output: 348
🏁 Script executed:
#!/bin/bash
# Search for usages of analyticsResult variable
rg -n "analyticsResult" -g '*.ts' -g '*.tsx'Length of output: 113
Update ProjectStats component to match new API shape.
ProjectStats interface and implementation still reference projectsByStatus, but analytics.ts now returns projectsByType. The component is currently unused, but the prop mismatch must be fixed before it's integrated:
- Line 8: Update interface property name
- Line 12: Update destructured prop name
- Line 43: Update data binding in BarChart
All three locations in src/components/models/analytics/ProjectStats.tsx need the same change: projectsByStatus → projectsByType.
🤖 Prompt for AI Agents
In src/components/models/analytics/ProjectStats.tsx around lines 8, 12 and 43,
the component still uses the old prop name projectsByStatus which no longer
matches the analytics API that returns projectsByType; update the interface
property at line 8 to projectsByType, rename the destructured prop at line 12
from projectsByStatus to projectsByType, and change the data binding passed into
the BarChart at line 43 to use projectsByType instead of projectsByStatus so the
prop names align with src/actions/analytics.ts.
2 participants
Overview: This PR updates the user profile form to load initial user data and integrate with the user update action, improving the overall user experience.
Changes
src/components/models/user/ProfileForm.tsxfrom the session or provided props.ProfileForm.tsxhas been updated to call the appropriate user update action (e.g.,src/actions/user.ts).src/components/models/user/ProfileForm.tsxand a single import.Summary by CodeRabbit
New Features
Other