Uh oh!
There was an error while loading. Please reload this page.
fix: prohibit ename update pictique - #684
Conversation
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughClient UI now displays a read-only "eName" (synced from eVault) and a separate editable public username; client PATCH payload no longer includes handle. Server APIs and service layer no longer accept or update handle from the request—only name and avatar are applied. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (UI)
participant Server as API Server
participant Service as UserService
participant DB as Database
participant eVault as eVault (External)
rect rgba(200,220,255,0.5)
Client->>Server: GET /api/users/me
Server->>Service: fetch current user
Service->>DB: SELECT user
DB-->>Service: user { id, name, handle, avatarUrl }
Service-->>Server: user
Server-->>Client: user payload (eName = handle)
end
rect rgba(200,255,200,0.5)
Note over eVault,DB: handle/eName is maintained by eVault sync (external)
end
rect rgba(255,240,200,0.5)
Client->>Server: PATCH /api/users { name, avatar } (no handle)
Server->>Service: updateProfile(userId, { name, avatarUrl })
Service->>DB: UPDATE users SET name=?, avatarUrl=? WHERE id=?
DB-->>Service: updated user
Service-->>Server: updated user
Server-->>Client: updated user payload
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
coodos
left a comment
There was a problem hiding this comment.
eName == w3id == handle in this case, updating the user's real Name is fine, the username aka the UUID is not
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platforms/pictique-api/src/controllers/UserController.ts`:
- Around line 181-187: If this.userService.findById(userId) can return null and
you must not clobber eVault-synced handle, first guard for a missing user and
return a 404 (or throw) before calling updateProfile; do not include handle:
user?.handle in the payload—only pass fields that should be updated (e.g.,
avatarUrl: avatar ?? user.avatarUrl, name: name ?? user.name) so updateProfile
cannot overwrite a newer handle, or ensure updateProfile explicitly ignores
handle if present; reference the findById call and the updateProfile invocation
in UserController.
🧹 Nitpick comments (1)
platforms/pictique/src/routes/(protected)/settings/account/username/+page.svelte (1)
85-91: Considerreadonlyinstead ofdisabledfor eName.Disabled fields are not focusable or copyable;
readonlykeeps the text selectable and improves accessibility while still preventing edits.Proposed tweak
- <Input type="text" placeholder="Your eName" bind:value={handle} disabled class="cursor-not-allowed opacity-70" />+ <Input+ type="text"+ placeholder="Your eName"+ bind:value={handle}+ readonly+ aria-readonly="true"+ class="cursor-not-allowed opacity-70"+ />
| const user = await this.userService.findById(userId); | ||
| // Note: handle is not updatable to preserve eVault sync | ||
| const updatedUser = await this.userService.updateProfile(userId, { | ||
| handle: handle ?? user?.handle, | ||
| handle: user?.handle, | ||
| avatarUrl: avatar ?? user?.avatarUrl, | ||
| name: name ?? user?.name, |
There was a problem hiding this comment.
Avoid clobbering eName and guard missing user.
If findById returns null, the update may write undefined values. Also, writing handle: user?.handle can overwrite a newer eVault-synced handle between read and update. Prefer a 404 guard and omit handle from the update payload (or ensure the service ignores it).
Proposed fix
- const user = await this.userService.findById(userId);+ const user = await this.userService.findById(userId);+ if (!user) {+ return res.status(404).json({ error: "User not found" });+ }- // Note: handle is not updatable to preserve eVault sync+ // Note: handle is not updatable to preserve eVault sync (omit from update payload)
const updatedUser = await this.userService.updateProfile(userId, {
- handle: user?.handle,- avatarUrl: avatar ?? user?.avatarUrl,- name: name ?? user?.name,+ avatarUrl: avatar ?? user.avatarUrl,+ name: name ?? user.name,
});🤖 Prompt for AI Agents
In `@platforms/pictique-api/src/controllers/UserController.ts` around lines 181 -
187, If this.userService.findById(userId) can return null and you must not
clobber eVault-synced handle, first guard for a missing user and return a 404
(or throw) before calling updateProfile; do not include handle: user?.handle in
the payload—only pass fields that should be updated (e.g., avatarUrl: avatar ??
user.avatarUrl, name: name ?? user.name) so updateProfile cannot overwrite a
newer handle, or ensure updateProfile explicitly ignores handle if present;
reference the findById call and the updateProfile invocation in UserController.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Description of change
remove client side pictique "name" update code to reduce confusion with ename updation (default name)
Issue Number
closes#247
Type of change
How the change has been tested
manual
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.