feat(profile): complete user profile editing (#63)#170
Conversation
Frontend /profile page (name, bio, preferences, avatar) built to the existing ProfilePage.test contract; real useProfileApi against better-auth GET/PATCH/ DELETE /users/me (replaces Clerk-era stubs). New avatar upload endpoint POST /users/me/avatar + FileUploadService.process_and_save_profile_picture (square, decompression-bomb guard, persist-before-delete). Export now injects the owner's profile (author_name + escaped "About the Author" bio) into PDF/ DOCX/EPUB/Markdown. Backend UserUpdate field-length + theme validation. Tests: backend avatar endpoint + service + export author injection + validation; frontend ProfilePage (un-ignored), ProfilePictureUpload, useProfileApi units + route-mocked E2E profile-editing.spec. Repoints the "Profile" nav to /profile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughThis PR implements full user profile editing ( ChangesUser profile editing feature
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
backend/app/services/file_upload_service.py (1)
248-322: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDownstream of the
validate_image_uploadconcern above.
process_and_save_profile_picturere-seeks/re-opensfile.fileaftervalidate_image_uploadruns; if that method'sverify()call invalidatesfile.file(see comment on lines 108-121), this method inherits the same failure mode. No separate fix needed here beyond addressing the root cause.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/services/file_upload_service.py` around lines 248 - 322, process_and_save_profile_picture inherits the same file-handle invalidation risk from validate_image_upload because it reuses file.file after validation. Fix the root cause in validate_image_upload so the stream remains usable afterward (or is properly reset/reopened), and ensure process_and_save_profile_picture can continue using Image.open(file.file) safely without adding a separate workaround here.
🧹 Nitpick comments (7)
backend/app/api/endpoints/export.py (1)
130-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated one-liner across four export handlers.
book = _with_author_info(book, current_user)is duplicated identically in all four export flows. Consider hoisting it to right after the ownership check (shared by all four) instead of repeating it inside each handler's try block, reducing duplication.Also applies to: 232-232, 324-324, 417-417
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/endpoints/export.py` at line 130, The author enrichment step is duplicated in all four export handlers, so move the _with_author_info(book, current_user) call into the shared flow right after the ownership check in export.py and remove the repeated calls from each handler’s try block. Use the existing book and current_user variables in the shared export setup so the four export paths reuse the same author-aware book instance without duplication.backend/app/services/file_upload_service.py (1)
301-304: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: exception chaining flagged by Ruff (B904/BLE001).
raise HTTPException(...)insideexcept Exception:(line 301-304) drops the original traceback context; considerraise ... from None(or the caught error) to make debugging clearer. Existing pattern elsewhere in the file has the same style, so this is optional.Also applies to: 320-320
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/services/file_upload_service.py` around lines 301 - 304, The HTTPException raises in FileUploadService drop the original exception context inside the broad except blocks, which Ruff flags as B904/BLE001. Update the affected raise paths in file_upload_service.py (including the image-processing branch and the similar handler elsewhere in the file) to use explicit exception chaining, either by raising from the caught exception or from None, and keep the pattern consistent with the surrounding try/except logic in the file_upload_service methods.Source: Linters/SAST tools
frontend/src/hooks/useProfileApi.ts (2)
5-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
interfaceovertypefor object shapes.
UserPreferences,UserProfile, andProfileUpdateDataare all object-shape aliases; the guideline for.ts/.tsxfiles says to preferinterface.♻️ Suggested refactor
-export type UserPreferences = { +export interface UserPreferences { theme: 'light' | 'dark' | 'system'; email_notifications: boolean; marketing_emails: boolean; -}; +} -export type UserProfile = { +export interface UserProfile { id: string; auth_id: string; email: string; first_name: string | null; last_name: string | null; display_name: string | null; avatar_url: string | null; bio: string | null; preferences: UserPreferences; -}; +} -export type ProfileUpdateData = { +export interface ProfileUpdateData { first_name?: string; last_name?: string; display_name?: string; bio?: string; preferences?: Partial<UserPreferences>; -}; +}As per coding guidelines: "Prefer
interfacefor defining object shapes in TypeScript."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/useProfileApi.ts` around lines 5 - 29, The exported object-shape aliases in useProfileApi should use interfaces instead of type aliases. Update UserPreferences, UserProfile, and ProfileUpdateData to interface declarations, preserving their current fields and optional/nullable types, so the TypeScript definitions follow the project’s interface preference.Source: Coding guidelines
35-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent async/await usage across the three methods.
getUserProfileandupdateUserProfilereturn authFetch(...)withoutawait, whiledeleteUserAccountusesawait. Functionally equivalent inside anasyncfunction, but inconsistent with the guideline to always use async/await for promises, and inconsistent within the same file.♻️ Suggested refactor for consistency
const getUserProfile = async (): Promise<UserProfile> => { - return authFetch<UserProfile>('/users/me'); + return await authFetch<UserProfile>('/users/me'); }; const updateUserProfile = async ( profileData: ProfileUpdateData ): Promise<UserProfile> => { - return authFetch<UserProfile>('/users/me', { + return await authFetch<UserProfile>('/users/me', { method: 'PATCH', body: JSON.stringify(profileData), }); };As per coding guidelines: "Always use async/await for promises."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/useProfileApi.ts` around lines 35 - 52, The three methods in useProfileApi are inconsistent in promise handling: getUserProfile and updateUserProfile return authFetch directly while deleteUserAccount awaits it. Update all three methods to use async/await consistently, and reference the useProfileApi, getUserProfile, updateUserProfile, and deleteUserAccount functions so the pattern matches the project guideline of always using async/await for promises.Source: Coding guidelines
frontend/src/hooks/useProfileApi.test.tsx (1)
1-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffTest uses Jest, not Vitest.
This file uses
jest.mock/jest.fn(), whereas the guideline forfrontend/**/*.test.{ts,tsx}specifies Vitest for unit testing. This appears consistent with the rest of the project's existing Jest setup (perfrontend/jest.config.cjsin this same PR), so it's likely a pre-existing convention gap rather than something introduced here — flagging for awareness only.As per coding guidelines: "Use Vitest for frontend unit testing and Playwright for E2E testing."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/useProfileApi.test.tsx` around lines 1 - 38, This test file is using Jest APIs in a frontend unit test that should follow the Vitest guideline. Update useProfileApi.test.tsx to use Vitest primitives instead of jest.mock, jest.fn, and jest.clearAllMocks by importing vi from vitest and keeping the existing useAuthFetch and renderHook test structure intact. Make sure the mock setup in beforeEach and the assertions still reference the same symbols, especially useProfileApi and useAuthFetch.Source: Coding guidelines
frontend/src/components/profile/ProfilePictureUpload.tsx (2)
41-66: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on the upload request.
If the request to
/users/me/avatarhangs (network issue, slow endpoint),uploadingstaystrueindefinitely with no way for the user to retry or cancel.♻️ Optional: add an AbortController-based timeout
setUploading(true); try { const formData = new FormData(); formData.append('file', file); - const res = await fetch(`${API_BASE_URL}/users/me/avatar`, { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); + const res = await fetch(`${API_BASE_URL}/users/me/avatar`, { method: 'POST', credentials: 'include', body: formData, + signal: controller.signal, }); + clearTimeout(timeoutId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/profile/ProfilePictureUpload.tsx` around lines 41 - 66, The upload flow in ProfilePictureUpload’s upload handler has no timeout, so a hanging fetch can leave uploading stuck true indefinitely. Update the request to /users/me/avatar to use an AbortController with a timeout, and make sure the abort is handled in the existing try/catch so the user gets a failure toast and can retry. Keep the cleanup in finally (setUploading(false) and resetting inputRef.current) so the UI always recovers.
1-16: 📐 Maintainability & Code Quality | 🔵 TrivialFilename should be kebab-case.
ProfilePictureUpload.tsxuses PascalCase; sibling files likeuser-button.tsxfollow kebab-case. As per path instructions,frontend/src/components/**/*.{ts,tsx}: Use kebab-case for component file names, rename toprofile-picture-upload.tsx(and update its imports infrontend/src/app/profile/page.tsxand the test file).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/profile/ProfilePictureUpload.tsx` around lines 1 - 16, The component file name for ProfilePictureUpload is using PascalCase instead of the required kebab-case convention. Rename the file to profile-picture-upload.tsx and update every import/reference to ProfilePictureUpload accordingly, especially in the profile page and related test file, so the component name and file path stay consistent with the project’s naming rules.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/services/export_service.py`:
- Around line 347-354: The “About the Author” section in export_service.py is
using the base reportlab styles instead of the template-aware ones. Update the
author bio block in the export flow to use the existing localized style
variables for the heading and paragraph (the same ones used by the chapter/body
sections, e.g. the resolved heading2/body styles) so the bio matches the
selected template’s font, size, and leading.
In `@frontend/src/app/profile/page.tsx`:
- Around line 28-35: Expose and persist display_name in the profile editor: the
current profileSchema and surrounding form flow only handle
firstName/lastName/bio/theme and boolean preferences, so the page never
hydrates, renders, or submits the existing display_name contract. Update the
profile form state, validation, and submission logic in profile/page.tsx
(including the form sections around the referenced profile data handling and
save handler) to read display_name from the profile response, render an editable
field, and include it in the update payload.
- Around line 76-91: The profile hydration in the getUserProfile flow is
overwriting in-progress edits because form.reset() runs after the async fetch
resolves. Update the profile/page.tsx logic around getUserProfile to use
async/await instead of .then/.catch, and add a hydration/dirty guard so reset
only happens before the form has been edited (or only on the first successful
load). Preserve the existing session-seeded defaults and avatar handling, but
skip applying fetched values when the form is already dirty.
- Around line 28-31: The profile form schema in profile/page.tsx is incorrectly
requiring firstName and lastName via profileSchema, which blocks updates for
users whose names are nullable/optional in the API contract. Update the schema
and related client-side validation logic to allow empty or missing names, or
normalize blank inputs to the backend-supported null/optional value before
submission, while keeping the existing bio/avatar/preferences behavior intact.
- Around line 134-140: The profile form validation is reading private
react-hook-form internals via control._formState, which can miss updates. Update
the firstName/lastName error handling in page.tsx to use public state from
fieldState.error or form.formState.errors instead of controlState/errors, and
add a visible bio validation message using the same public form error source so
invalid bio submissions surface feedback.
In `@frontend/src/components/profile/ProfilePictureUpload.test.tsx`:
- Around line 7-8: Migrate ProfilePictureUpload.test.tsx from Jest to Vitest by
replacing jest.mock/jest.fn/jest.clearAllMocks with
vi.mock/vi.fn/vi.clearAllMocks, and update any castings from as jest.Mock to
Vitest’s Mock type. Also switch the test setup to import
`@testing-library/jest-dom/vitest` so the assertions work under Vitest, using the
existing mocked symbols like useOptimizedClerkImage and toast as the main
locations to update.
In `@frontend/src/hooks/useProfileApi.test.tsx`:
- Line 2: The test is importing useProfileApi as a default import even though
useProfileApi.ts only provides a named export. Update the import in
useProfileApi.test.tsx to match the named export from useProfileApi, and make
sure all usages in the test still reference the hook symbol correctly in
renderHook and any mocks.
---
Duplicate comments:
In `@backend/app/services/file_upload_service.py`:
- Around line 248-322: process_and_save_profile_picture inherits the same
file-handle invalidation risk from validate_image_upload because it reuses
file.file after validation. Fix the root cause in validate_image_upload so the
stream remains usable afterward (or is properly reset/reopened), and ensure
process_and_save_profile_picture can continue using Image.open(file.file) safely
without adding a separate workaround here.
---
Nitpick comments:
In `@backend/app/api/endpoints/export.py`:
- Line 130: The author enrichment step is duplicated in all four export
handlers, so move the _with_author_info(book, current_user) call into the shared
flow right after the ownership check in export.py and remove the repeated calls
from each handler’s try block. Use the existing book and current_user variables
in the shared export setup so the four export paths reuse the same author-aware
book instance without duplication.
In `@backend/app/services/file_upload_service.py`:
- Around line 301-304: The HTTPException raises in FileUploadService drop the
original exception context inside the broad except blocks, which Ruff flags as
B904/BLE001. Update the affected raise paths in file_upload_service.py
(including the image-processing branch and the similar handler elsewhere in the
file) to use explicit exception chaining, either by raising from the caught
exception or from None, and keep the pattern consistent with the surrounding
try/except logic in the file_upload_service methods.
In `@frontend/src/components/profile/ProfilePictureUpload.tsx`:
- Around line 41-66: The upload flow in ProfilePictureUpload’s upload handler
has no timeout, so a hanging fetch can leave uploading stuck true indefinitely.
Update the request to /users/me/avatar to use an AbortController with a timeout,
and make sure the abort is handled in the existing try/catch so the user gets a
failure toast and can retry. Keep the cleanup in finally (setUploading(false)
and resetting inputRef.current) so the UI always recovers.
- Around line 1-16: The component file name for ProfilePictureUpload is using
PascalCase instead of the required kebab-case convention. Rename the file to
profile-picture-upload.tsx and update every import/reference to
ProfilePictureUpload accordingly, especially in the profile page and related
test file, so the component name and file path stay consistent with the
project’s naming rules.
In `@frontend/src/hooks/useProfileApi.test.tsx`:
- Around line 1-38: This test file is using Jest APIs in a frontend unit test
that should follow the Vitest guideline. Update useProfileApi.test.tsx to use
Vitest primitives instead of jest.mock, jest.fn, and jest.clearAllMocks by
importing vi from vitest and keeping the existing useAuthFetch and renderHook
test structure intact. Make sure the mock setup in beforeEach and the assertions
still reference the same symbols, especially useProfileApi and useAuthFetch.
In `@frontend/src/hooks/useProfileApi.ts`:
- Around line 5-29: The exported object-shape aliases in useProfileApi should
use interfaces instead of type aliases. Update UserPreferences, UserProfile, and
ProfileUpdateData to interface declarations, preserving their current fields and
optional/nullable types, so the TypeScript definitions follow the project’s
interface preference.
- Around line 35-52: The three methods in useProfileApi are inconsistent in
promise handling: getUserProfile and updateUserProfile return authFetch directly
while deleteUserAccount awaits it. Update all three methods to use async/await
consistently, and reference the useProfileApi, getUserProfile,
updateUserProfile, and deleteUserAccount functions so the pattern matches the
project guideline of always using async/await for promises.
🪄 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: 8ba39325-3e2a-43a0-8a82-8691c16b7951
📒 Files selected for processing (19)
AUTO_AUTHOR_USER_MANUAL.mdCLAUDE.mdbackend/app/api/endpoints/export.pybackend/app/api/endpoints/users.pybackend/app/schemas/user.pybackend/app/services/export_service.pybackend/app/services/file_upload_service.pybackend/tests/test_api/test_export_endpoints.pybackend/tests/test_api/test_routes/test_profile_pictures.pybackend/tests/test_api/test_routes/test_profile_updates.pybackend/tests/test_services/test_file_upload_service.pyfrontend/jest.config.cjsfrontend/src/app/profile/page.tsxfrontend/src/components/profile/ProfilePictureUpload.test.tsxfrontend/src/components/profile/ProfilePictureUpload.tsxfrontend/src/components/ui/user-button.tsxfrontend/src/e2e/profile-editing.spec.tsfrontend/src/hooks/useProfileApi.test.tsxfrontend/src/hooks/useProfileApi.ts
💤 Files with no reviewable changes (1)
- frontend/jest.config.cjs
| # About the Author (from the owner's profile bio). Escape the free-text | ||
| # bio — ReportLab's Paragraph parses XML-like markup, so unescaped | ||
| # characters like '<' would break PDF generation. | ||
| if book_data.get('author_bio'): | ||
| story.append(Spacer(1, 0.25*inch)) | ||
| story.append(Paragraph("About the Author", styles['Heading2'])) | ||
| story.append(Paragraph(html.escape(book_data['author_bio']), styles['Normal'])) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
"About the Author" PDF section ignores the resolved template styling.
The heading and body use the raw styles['Heading2'] / styles['Normal'] base styles instead of the locally-customized heading2_style and body_style defined above (which apply the template's heading_font/body_font, sizing, and leading). Every other section of the story (chapter headings, body text) is careful to use the template-aware styles, so when a professional export template is selected, the bio section will visually mismatch the rest of the document (wrong font/size).
🐛 Proposed fix to use the template-aware styles
if book_data.get('author_bio'):
story.append(Spacer(1, 0.25*inch))
- story.append(Paragraph("About the Author", styles['Heading2']))
- story.append(Paragraph(html.escape(book_data['author_bio']), styles['Normal']))
+ story.append(Paragraph("About the Author", heading2_style))
+ story.append(Paragraph(html.escape(book_data['author_bio']), body_style))📝 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.
| # About the Author (from the owner's profile bio). Escape the free-text | |
| # bio — ReportLab's Paragraph parses XML-like markup, so unescaped | |
| # characters like '<' would break PDF generation. | |
| if book_data.get('author_bio'): | |
| story.append(Spacer(1, 0.25*inch)) | |
| story.append(Paragraph("About the Author", styles['Heading2'])) | |
| story.append(Paragraph(html.escape(book_data['author_bio']), styles['Normal'])) | |
| # About the Author (from the owner's profile bio). Escape the free-text | |
| # bio — ReportLab's Paragraph parses XML-like markup, so unescaped | |
| # characters like '<' would break PDF generation. | |
| if book_data.get('author_bio'): | |
| story.append(Spacer(1, 0.25*inch)) | |
| story.append(Paragraph("About the Author", heading2_style)) | |
| story.append(Paragraph(html.escape(book_data['author_bio']), body_style)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/app/services/export_service.py` around lines 347 - 354, The “About
the Author” section in export_service.py is using the base reportlab styles
instead of the template-aware ones. Update the author bio block in the export
flow to use the existing localized style variables for the heading and paragraph
(the same ones used by the chapter/body sections, e.g. the resolved
heading2/body styles) so the bio matches the selected template’s font, size, and
leading.
| const profileSchema = z.object({ | ||
| firstName: z.string().min(1, 'First name is required').max(50, 'Max 50 characters'), | ||
| lastName: z.string().min(1, 'Last name is required').max(50, 'Max 50 characters'), | ||
| bio: z.string().max(BIO_MAX, `Max ${BIO_MAX} characters`).optional(), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not require nullable profile names on the client.
The API contract allows first_name/last_name to be null in UserProfile and optional in ProfileUpdateData, but .min(1) blocks saving bio/preferences/avatar changes for users without both names. Remove the required constraint or normalize blanks to the backend-supported value.
Possible schema adjustment
- firstName: z.string().min(1, 'First name is required').max(50, 'Max 50 characters'),
- lastName: z.string().min(1, 'Last name is required').max(50, 'Max 50 characters'),
+ firstName: z.string().trim().max(50, 'Max 50 characters'),
+ lastName: z.string().trim().max(50, 'Max 50 characters'),📝 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 profileSchema = z.object({ | |
| firstName: z.string().min(1, 'First name is required').max(50, 'Max 50 characters'), | |
| lastName: z.string().min(1, 'Last name is required').max(50, 'Max 50 characters'), | |
| bio: z.string().max(BIO_MAX, `Max ${BIO_MAX} characters`).optional(), | |
| const profileSchema = z.object({ | |
| firstName: z.string().trim().max(50, 'Max 50 characters'), | |
| lastName: z.string().trim().max(50, 'Max 50 characters'), | |
| bio: z.string().max(BIO_MAX, `Max ${BIO_MAX} characters`).optional(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/app/profile/page.tsx` around lines 28 - 31, The profile form
schema in profile/page.tsx is incorrectly requiring firstName and lastName via
profileSchema, which blocks updates for users whose names are nullable/optional
in the API contract. Update the schema and related client-side validation logic
to allow empty or missing names, or normalize blank inputs to the
backend-supported null/optional value before submission, while keeping the
existing bio/avatar/preferences behavior intact.
| const profileSchema = z.object({ | ||
| firstName: z.string().min(1, 'First name is required').max(50, 'Max 50 characters'), | ||
| lastName: z.string().min(1, 'Last name is required').max(50, 'Max 50 characters'), | ||
| bio: z.string().max(BIO_MAX, `Max ${BIO_MAX} characters`).optional(), | ||
| theme: z.enum(['light', 'dark', 'system']), | ||
| emailNotifications: z.boolean(), | ||
| marketingEmails: z.boolean(), | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Expose and persist display_name.
display_name exists in both the profile response and update payload contracts, but this page never hydrates, renders, or submits it. That leaves a profile field uneditable despite the profile-editing acceptance criteria.
Also applies to: 79-86, 98-107, 153-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/app/profile/page.tsx` around lines 28 - 35, Expose and persist
display_name in the profile editor: the current profileSchema and surrounding
form flow only handle firstName/lastName/bio/theme and boolean preferences, so
the page never hydrates, renders, or submits the existing display_name contract.
Update the profile form state, validation, and submission logic in
profile/page.tsx (including the form sections around the referenced profile data
handling and save handler) to read display_name from the profile response,
render an editable field, and include it in the update payload.
| if (getUserProfile) { | ||
| getUserProfile() | ||
| .then((p) => { | ||
| form.reset({ | ||
| firstName: p.first_name ?? firstName, | ||
| lastName: p.last_name ?? lastName, | ||
| bio: p.bio ?? '', | ||
| theme: p.preferences?.theme ?? 'system', | ||
| emailNotifications: p.preferences?.email_notifications ?? true, | ||
| marketingEmails: p.preferences?.marketing_emails ?? false, | ||
| }); | ||
| if (p.avatar_url) setAvatarUrl(p.avatar_url); | ||
| }) | ||
| .catch(() => { | ||
| /* keep session-seeded defaults */ | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Guard profile hydration from overwriting edits.
The async profile fetch calls form.reset() when it resolves; if the user starts editing the session-seeded form first, the late reset discards their input. Track hydration/loading or skip reset once the form is dirty, and convert this chain to async/await. As per coding guidelines, "**/*.{js,jsx,ts,tsx}: Always use async/await for promises."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/app/profile/page.tsx` around lines 76 - 91, The profile
hydration in the getUserProfile flow is overwriting in-progress edits because
form.reset() runs after the async fetch resolves. Update the profile/page.tsx
logic around getUserProfile to use async/await instead of .then/.catch, and add
a hydration/dirty guard so reset only happens before the form has been edited
(or only on the first successful load). Preserve the existing session-seeded
defaults and avatar handling, but skip applying fetched values when the form is
already dirty.
Source: Coding guidelines
| // Read validation errors off the live control state. This works with both the | ||
| // real react-hook-form Control and the mocked form used in unit tests. | ||
| const controlState = ( | ||
| form.control as unknown as { _formState?: { errors?: Record<string, { message?: string }> } } | ||
| )?._formState; | ||
| const errors = controlState?.errors ?? {}; | ||
| const bioValue = (form.watch('bio') as string) ?? ''; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify private React Hook Form state is not used for validation rendering.
rg -n "_formState|controlState" frontend/src/app/profile/page.tsxRepository: frankbria/auto-author
Length of output: 360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant sections around the cited ranges.
sed -n '120,220p' frontend/src/app/profile/page.tsx
# Map the file structure to identify the form fields and error rendering sites.
ast-grep outline frontend/src/app/profile/page.tsx --view expandedRepository: frankbria/auto-author
Length of output: 3886
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact line-numbered range covering the cited sections.
cat -n frontend/src/app/profile/page.tsx | sed -n '160,220p'Repository: frankbria/auto-author
Length of output: 2674
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the form setup and schema to confirm how validation state is exposed.
cat -n frontend/src/app/profile/page.tsx | sed -n '1,140p'Repository: frankbria/auto-author
Length of output: 5782
Use public form state for validation errors
control._formState is private RHF state and can miss updates; switch the firstName/lastName blocks to fieldState.error or form.formState.errors, and add a bio error message so invalid submissions actually surface feedback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/app/profile/page.tsx` around lines 134 - 140, The profile form
validation is reading private react-hook-form internals via control._formState,
which can miss updates. Update the firstName/lastName error handling in page.tsx
to use public state from fieldState.error or form.formState.errors instead of
controlState/errors, and add a visible bio validation message using the same
public form error source so invalid bio submissions surface feedback.
| jest.mock('@/hooks/useOptimizedClerkImage'); | ||
| jest.mock('@/lib/toast', () => ({ toast: { success: jest.fn(), error: jest.fn() } })); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no Jest globals remain in this frontend unit test.
rg -n "jest\.|as jest\.Mock" frontend/src/components/profile/ProfilePictureUpload.test.tsxRepository: frankbria/auto-author
Length of output: 1040
🏁 Script executed:
sed -n '1,120p' frontend/src/components/profile/ProfilePictureUpload.test.tsxRepository: frankbria/auto-author
Length of output: 3734
Migrate this test to Vitest APIs. frontend/src/components/profile/ProfilePictureUpload.test.tsx:7-8,25-26,55,65,74 still uses jest.mock, jest.fn, jest.clearAllMocks, and as jest.Mock; switch to vi.mock/vi.fn/vi.clearAllMocks and import Vitest’s Mock type (and @testing-library/jest-dom/vitest).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/profile/ProfilePictureUpload.test.tsx` around lines 7
- 8, Migrate ProfilePictureUpload.test.tsx from Jest to Vitest by replacing
jest.mock/jest.fn/jest.clearAllMocks with vi.mock/vi.fn/vi.clearAllMocks, and
update any castings from as jest.Mock to Vitest’s Mock type. Also switch the
test setup to import `@testing-library/jest-dom/vitest` so the assertions work
under Vitest, using the existing mocked symbols like useOptimizedClerkImage and
toast as the main locations to update.
Source: Coding guidelines
| @@ -0,0 +1,38 @@ | |||
| import { renderHook } from '@testing-library/react'; | |||
| import useProfileApi from './useProfileApi'; | |||
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Default import mismatch — useProfileApi.ts has no default export.
The source file at frontend/src/hooks/useProfileApi.ts only exports export const useProfileApi = () => {...} (named export). This test does import useProfileApi from './useProfileApi'; — a default import from a module with no default export. Under standard TS module resolution (without a CJS default-export interop shim) this fails to compile with error TS1192 "Module has no default export," and even where it compiles, the imported "default" would not be the callable hook — every test in this file (and renderHook(() => useProfileApi())) would break.
🐛 Proposed fix
-import useProfileApi from './useProfileApi';
+import { useProfileApi } from './useProfileApi';📝 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 useProfileApi from './useProfileApi'; | |
| import { useProfileApi } from './useProfileApi'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/hooks/useProfileApi.test.tsx` at line 2, The test is importing
useProfileApi as a default import even though useProfileApi.ts only provides a
named export. Update the import in useProfileApi.test.tsx to match the named
export from useProfileApi, and make sure all usages in the test still reference
the hook symbol correctly in renderHook and any mocks.
1 participant
Closes #63 [P3.5].
What
Full-stack user profile editing. The backend user model +
GET/PATCH/DELETE /users/mealready existed (better-auth); this closes the real gaps: the missing frontend page, avatar upload, validation, and profile→export author info.Frontend
app/profile/page.tsx— new/profilepage (name, bio, preferences, avatar, delete-account). Built to the pre-existingProfilePage.test.tsxcontract (un-ignored in jest config now that the page exists).hooks/useProfileApi.ts— replaced the Clerk-era "deprecated" stubs with real cookie-authenticatedgetUserProfile/updateUserProfile/deleteUserAccount.components/profile/ProfilePictureUpload.tsx— avatar display + upload (client-side type/size validation, multipart POST)./profile.Backend
POST /users/me/avatar— new endpoint;FileUploadService.process_and_save_profile_picture(square ≤400px, cloud/local, decompression-bomb guard). Persists the new URL before deleting the old avatar; cleans up on failure.UserUpdatevalidation — first/last name ≤50, display_name ≤100, bio ≤1000;themerestricted to light/dark/system.display_name→ author name,bio→ escaped "About the Author") is merged intobook_datafor PDF/DOCX/EPUB/Markdown.Acceptance criteria → evidence
profile-editing.spec.ts) edits & asserts the PATCH payload + success toast.<img>updates).src/e2e/profile-editing.spec.ts(2 tests, chromium green).Plan drift corrected
The Traycer/CodeRabbit plans referenced deleted/renamed code:
book_cover_upload.py(deleted in #93 — mirrored the live cover path inbooks.py+FileUploadServiceinstead) anduseOptimizedClerkImage(still present, legacy name — kept, the test requires it). Clerk test mocks → better-auth.Pre-PR review (codex, cross-family)
4 findings, all addressed: decompression-bomb guard (added), persist-before-delete-avatar (reordered + cleanup), bio XML-escaping for ReportLab (escaped). The relative-
/uploadsURL note is a pre-existing, app-wide pattern — see Known Limitations.Known limitations
/uploads/profile_pictures/...), served on the API origin. This matches the existing cover-image behavior (page.tsxrenderscover_image_urlthe same way); production uses cloud storage (absolute URLs). Not changed here to stay consistent with covers.Tests
profile-editing.spec.ts: 2/2 chromium.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation