feat(ui): add ui for user profile#116
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new customer profile management page is introduced at ChangesProfile Management Page Implementation
Sequence DiagramsequenceDiagram
actor User
participant NavBar as NavBar Component
participant Browser as Browser
participant Server as Server (load)
participant Client as Profile Page
participant API as API Endpoints
User->>NavBar: Click "Tài khoản" menu
NavBar->>Browser: Navigate to /me/profile
Browser->>Server: Request /me/profile
Server->>Server: Check locals.user role
alt Not authenticated
Server-->>Browser: Redirect 302 to /login
else Not customer role
Server-->>Browser: Redirect 303 to /
else Customer authenticated
Server->>API: Fetch /api/me/profile
API-->>Server: Return profile data
Server-->>Browser: Render with data
end
Browser->>Client: Load profile page
Client->>Client: Pre-fill form values from $page.data.profile
User->>Client: Edit profile & click submit
Client->>Client: Validate with Zod schema
alt Validation fails
Client->>Client: Show field errors
else Validation passes
Client->>API: PATCH /api/me/profile (or /api/me/security)
alt Success (200)
API-->>Client: Return updated data
Client->>Client: Show success toast
Client->>Server: invalidateAll() reload data
else Error (400)
API-->>Client: Return field errors
Client->>Client: Display field-specific errors
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 26 minutes and 5 seconds.Comment |
Review Summary by QodoAdd user profile management page with account settings
WalkthroughsDescription• Add user profile management page at /me/profile • Implement personal information update form with validation • Add email change functionality with password verification • Add password change functionality with confirmation • Update navbar to link to new profile page Diagramflowchart LR
A["User Profile Page"] --> B["Personal Info Form"]
A --> C["Email Change Form"]
A --> D["Password Change Form"]
B --> E["API: PATCH /api/me/profile"]
C --> F["API: PATCH /api/me/security"]
D --> F
E --> G["Update Profile Data"]
F --> H["Update Security Data"]
I["Navbar"] -- "Navigate to" --> A
File Changes1. src/routes/(customer)/me/profile/+page.svelte
|
Code Review by Qodo
1. Profile prefill uses data.profile
|
| const user = $derived($page.data.profile); | ||
|
|
||
| // ─── Profile Form ────────────────────────── | ||
| let profileForm = $state({ | ||
| full_name: '', | ||
| phone: '', | ||
| date_of_birth: '', | ||
| gender: 'male' as 'male' | 'female' | 'other', | ||
| }); | ||
|
|
||
| let profileErrors = $state<Record<string, string>>({}); | ||
| let isProfileLoading = $state(false); | ||
|
|
||
| $effect(() => { | ||
| if (user) { | ||
| profileForm = { | ||
| full_name: user.full_name ?? '', | ||
| phone: user.phone ?? '', | ||
| date_of_birth: user.date_of_birth ? user.date_of_birth.split('T')[0] : '', | ||
| gender: user.gender ?? 'male', | ||
| }; | ||
| } |
There was a problem hiding this comment.
1. Profile prefill uses data.profile 📎 Requirement gap ≡ Correctness
Form Thông tin cá nhân đang pre-fill từ $page.data.profile (data fetch riêng) thay vì từ data.user như yêu cầu compliance. Điều này vi phạm tiêu chí “Profile form được pre-fill từ data.user” và có thể gây sai/không đồng bộ dữ liệu so với nguồn chuẩn từ layout.
Agent Prompt
## Issue description
Profile form đang pre-fill từ `$page.data.profile` thay vì `data.user` theo compliance.
## Issue Context
Checklist yêu cầu dữ liệu user hiện tại được `+layout.server.ts` truyền xuống qua `data.user` và dùng để điền sẵn form (tên/SĐT/ngày sinh/giới tính).
## Fix Focus Areas
- src/routes/(customer)/me/profile/+page.svelte[33-54]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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/lib/components/customer/layout/CustomerNavbar.svelte (1)
224-232:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDesktop dropdown update is correct, but the mobile bottom nav still targets
/profileLine 226 correctly navigates to
/me/profile, matching the new route. However, line 329 (the mobile bottom nav "Cá nhân" link) still resolves touser ? '/profile' : '/login'. Since/profileis not a valid route (the page lives at/me/profile), mobile users will hit a 404. Please update it for consistency.🐛 Proposed fix (line 329 in the mobile nav section)
- href={resolve(user ? '/profile' : '/login')} + href={resolve(user ? '/me/profile' : '/login')}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/components/customer/layout/CustomerNavbar.svelte` around lines 224 - 232, The mobile bottom nav link for the "Cá nhân" item still resolves to '/profile' which no longer exists; change the ternary that builds the href from user ? '/profile' : '/login' to user ? '/me/profile' : '/login' (and keep using resolve(...) if other nav links call resolve) so it matches the desktop DropdownMenuItem navigation to '/me/profile' (see DropdownMenuItem onclick and the mobile "Cá nhân" link that uses the user variable and resolve).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routes/`(customer)/me/profile/+page.server.ts:
- Around line 20-24: The load function is returning the whole response object
from await res.json() as profile, but the API returns { data: profile }, so the
component sees profile.data instead of the flat profile; update the load logic
to extract the inner data (e.g., const { data: profile } = await res.json(); or
const profile = (await res.json()).data;) and return that profile along with
locals.user so $page.data.profile contains the actual profile fields used by the
component.
- Line 3: The import for PageServerLoad is using the wrong relative path and
pulls the parent route group's generated types; update the import statement that
currently references '../../$types' so it imports from './$types' instead (the
symbol to change is the PageServerLoad import line) to restore correct
page-level type resolution for the load function.
In `@src/routes/`(customer)/me/profile/+page.svelte:
- Around line 59-62: The data object currently passes phone: '' into
updateProfileSchema, causing the regex to run because empty string is a valid
string but should be treated as absent; update the mapping that builds data from
profileForm so the phone field is converted to undefined when it's an empty or
whitespace-only string (e.g., use a trimmed-check on profileForm.phone and set
phone to undefined if empty), leaving other fields unchanged; reference the
profileForm variable and the data construction here and make the change so
updateProfileSchema.optional()/nullable() can accept missing phone values.
---
Outside diff comments:
In `@src/lib/components/customer/layout/CustomerNavbar.svelte`:
- Around line 224-232: The mobile bottom nav link for the "Cá nhân" item still
resolves to '/profile' which no longer exists; change the ternary that builds
the href from user ? '/profile' : '/login' to user ? '/me/profile' : '/login'
(and keep using resolve(...) if other nav links call resolve) so it matches the
desktop DropdownMenuItem navigation to '/me/profile' (see DropdownMenuItem
onclick and the mobile "Cá nhân" link that uses the user variable and resolve).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6a67e7aa-2a9c-4cec-972c-170d0e5bda2b
📒 Files selected for processing (3)
src/lib/components/customer/layout/CustomerNavbar.sveltesrc/routes/(customer)/me/profile/+page.server.tssrc/routes/(customer)/me/profile/+page.svelte
| @@ -0,0 +1,26 @@ | |||
| // src/routes/(customer)/me/profile/+page.server.ts | |||
| import { redirect } from '@sveltejs/kit'; | |||
| import type { PageServerLoad } from '../../$types'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify: confirm all sibling/similar server route files use ./$types
rg -n "from.*\$types" --type=ts src/routes/ -g '!*.d.ts'Repository: Tixtac-Project/tixtac
Length of output: 47
🏁 Script executed:
# Check if the target file exists
ls -la "src/routes/(customer)/me/profile/+page.server.ts" 2>/dev/null && echo "File exists" || echo "File does not exist"Repository: Tixtac-Project/tixtac
Length of output: 174
🏁 Script executed:
# List the structure of src/routes to understand it
find src/routes -type f -name "+page.server.ts" 2>/dev/null | head -20Repository: Tixtac-Project/tixtac
Length of output: 665
🏁 Script executed:
# Try a broader search for $types imports with different patterns
rg -n "PageServerLoad\|PageLoad\|\$types" src/routes/ --type=ts 2>/dev/null | head -30Repository: Tixtac-Project/tixtac
Length of output: 47
🏁 Script executed:
cat -n "src/routes/(customer)/me/profile/+page.server.ts"Repository: Tixtac-Project/tixtac
Length of output: 822
🏁 Script executed:
# Check a few other +page.server.ts files to see their import patterns
for file in "src/routes/(auth)/login/+page.server.ts" "src/routes/(customer)/+page.server.ts" "src/routes/(customer)/events/+page.server.ts"; do
echo "=== $file ===";
head -5 "$file" 2>/dev/null || echo "Not found";
doneRepository: Tixtac-Project/tixtac
Length of output: 891
🏁 Script executed:
# Search for all PageServerLoad imports and their paths
rg "import.*PageServerLoad" src/routes/ --type=tsRepository: Tixtac-Project/tixtac
Length of output: 1386
🏁 Script executed:
# Check if $types.d.ts files exist in the repo
find . -name "$types.d.ts" -o -name "*.d.ts" -path "*routes*" 2>/dev/null | head -20Repository: Tixtac-Project/tixtac
Length of output: 47
Use ./$types instead of ../../$types
All other route files in the codebase use ./$types to import PageServerLoad. This file is the only exception with ../../$types, which incorrectly resolves to the parent route group's generated types rather than the page's own types, breaking type safety for the load function's return value.
🔧 Proposed fix
-import type { PageServerLoad } from '../../$types';
+import type { PageServerLoad } from './$types';📝 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.
| import type { PageServerLoad } from '../../$types'; | |
| import type { PageServerLoad } from './$types'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/`(customer)/me/profile/+page.server.ts at line 3, The import for
PageServerLoad is using the wrong relative path and pulls the parent route
group's generated types; update the import statement that currently references
'../../$types' so it imports from './$types' instead (the symbol to change is
the PageServerLoad import line) to restore correct page-level type resolution
for the load function.
Mô tả
Implement trang /me/profile cho phép người dùng quản lý toàn bộ thông tin tài khoản gồm:
Closes #109
Loại thay đổi
Screenshots / Demo
Checklist
bun run dev)bun run check)bun run lint)bun run format)feat:,fix:,chore:,...)Ghi chú cho reviewer
Summary by CodeRabbit
New Features
Bug Fixes