feat: migrate account update to dedicated api#1649
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 39 seconds. ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughRemoved an internal account update API route and replaced client-side update flows with a shared Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI
participant Hook as Hook\n(useOrgSettings / useUser)
participant Privy as Privy\n(getAccessToken)
participant ClientLib as updateAccountProfile
participant AccountsAPI as Accounts API
UI->>Hook: trigger save(name,image,instruction,knowledges)
Hook->>Privy: getAccessToken()
Privy-->>Hook: accessToken
Hook->>ClientLib: updateAccountProfile({accessToken, accountId, ...})
ClientLib->>AccountsAPI: PATCH /api/accounts (Bearer token, body)
AccountsAPI-->>ClientLib: 200 { data: AccountWithDetails }
ClientLib-->>Hook: AccountWithDetails
Hook->>Hook: normalize/map data, set state
Hook-->>UI: updated org/user state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Confidence score: 3/5
- There is some user-impacting risk here: in
hooks/useOrgSettings.ts,save()can reject whenupdateAccountProfilefails instead of returningfalse, which can surface as unhandled failures in profile updates. - In
hooks/useUser.tsx, missing access-token handling for wallet-connected users without Privy auth can cause profile saves to silently no-op, so users may think changes were saved when they were not. - Given one medium-severity/high-confidence error path and another likely UX-facing auth gap, this is mergeable with caution but benefits from a quick fix pass first. Pay close attention to
hooks/useOrgSettings.tsandhooks/useUser.tsx- profile save failure handling and auth-token gating behavior.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/useUser.tsx">
<violation number="1" location="hooks/useUser.tsx:68">
P2: Missing access token handling causes profile save to silently no-op for wallet-connected users without Privy auth. Prompt login or show an error instead of returning silently.</violation>
</file>
<file name="hooks/useOrgSettings.ts">
<violation number="1" location="hooks/useOrgSettings.ts:177">
P2: Handle `updateAccountProfile` errors in `save()` and return `false` on failure instead of letting the promise reject.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| const data = await updateAccountProfile({ | ||
| accessToken, | ||
| accountId: orgId, | ||
| name, | ||
| image, | ||
| instruction, | ||
| knowledges, | ||
| }); |
There was a problem hiding this comment.
P2: Handle updateAccountProfile errors in save() and return false on failure instead of letting the promise reject.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useOrgSettings.ts, line 177:
<comment>Handle `updateAccountProfile` errors in `save()` and return `false` on failure instead of letting the promise reject.</comment>
<file context>
@@ -141,30 +169,32 @@ const useOrgSettings = (orgId: string | null) => {
}
- return false;
+
+ const data = await updateAccountProfile({
+ accessToken,
+ accountId: orgId,
</file context>
| const data = await updateAccountProfile({ | |
| accessToken, | |
| accountId: orgId, | |
| name, | |
| image, | |
| instruction, | |
| knowledges, | |
| }); | |
| const data = await updateAccountProfile({ | |
| accessToken, | |
| accountId: orgId, | |
| name, | |
| image, | |
| instruction, | |
| knowledges, | |
| }).catch(() => null); | |
| if (!data) { | |
| return false; | |
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00a9f6161a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| return false; | ||
|
|
||
| const data = await updateAccountProfile({ |
There was a problem hiding this comment.
save() is used as a boolean-returning action (const success = await save() in OrgSettings), but this call now propagates exceptions when the API returns a non-2xx response (e.g., expired token/401 or validation/400). Because save() no longer converts that failure into false, handleSave has no try/catch path and the rejection becomes unhandled instead of a normal failed save state, which regresses error handling compared with the previous implementation.
Useful? React with 👍 / 👎.
![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)
hooks/useOrgSettings.ts (1)
144-173:⚠️ Potential issue | 🟡 MinorMissing error handling in save function.
Unlike
fetchOrgDetailswhich has a try-catch with error logging, thesavefunction lacks a catch block. IfupdateAccountProfilethrows (e.g., network error, 4xx/5xx response), the error will propagate unhandled andsetIsSaving(false)in thefinallyblock is the only cleanup that occurs. Consider whether the caller handles this or if silent error handling (like inuseUser.tsx) is intended.🛡️ Proposed fix to add error handling
try { const accessToken = await getAccessToken(); if (!accessToken) { return false; } const data = await updateAccountProfile({ accessToken, accountId: orgId, name, image, instruction, knowledges, }); setOrgData({ id: data.id, name: data.name || "", image: data.image || "", instruction: data.instruction || "", knowledges: normalizeKnowledges(data.knowledges), }); await queryClient.invalidateQueries({ queryKey: ["accountOrganizations"] }); return true; + } catch (error) { + console.error("Error saving org settings:", error); + return false; } finally { setIsSaving(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useOrgSettings.ts` around lines 144 - 173, The save callback lacks a catch block so failures from updateAccountProfile (or getAccessToken) will propagate; wrap the await calls inside a try-catch (inside the existing try/finally) and on error log the error and return false (or rethrow if desired) instead of letting it bubble uncaught; specifically update the save function that calls getAccessToken and updateAccountProfile, catch exceptions, call processLogger.error (or console.error) with the error, ensure setIsSaving(false) remains in the finally, and only call queryClient.invalidateQueries and setOrgData when the update succeeds.
🧹 Nitpick comments (5)
lib/accounts/updateAccountProfile.ts (2)
38-40: Consider extracting error details from response body.The current error message only includes the HTTP status code. The API may return useful error details in the response body (e.g., validation errors, specific failure reasons) that would aid debugging.
♻️ Proposed enhancement to include error details
if (!response.ok) { - throw new Error(`Failed to update account: ${response.status}`); + const errorBody = await response.text(); + throw new Error(`Failed to update account: ${response.status} - ${errorBody}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/accounts/updateAccountProfile.ts` around lines 38 - 40, The error thrown on non-OK responses in updateAccountProfile.ts currently only includes response.status; change the failure path in the updateAccountProfile function to read and include response body details (try response.json(), fallback to response.text()) and append those details to the thrown Error or include them as a property on the Error object so logs surface API validation/failure messages along with the HTTP status; ensure parsing is safe (catch JSON parse errors) and still throws a clear Error when body parsing fails.
42-43: Add defensive handling for JSON parsing.If the API returns a malformed response,
response.json()will throw an unhandled exception. Wrapping this in a try-catch provides a clearer error message.♻️ Proposed defensive JSON parsing
- const data: UpdateAccountProfileResponse = await response.json(); - return data.data; + try { + const data: UpdateAccountProfileResponse = await response.json(); + return data.data; + } catch { + throw new Error("Failed to parse account update response"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/accounts/updateAccountProfile.ts` around lines 42 - 43, The call to response.json() in updateAccountProfile (where you assign const data: UpdateAccountProfileResponse = await response.json()) should be wrapped in a try-catch to defensively handle malformed JSON; catch JSON parsing errors, log or throw a new error that includes context (e.g., "Failed to parse UpdateAccountProfile response") and the original error, and only return data.data when parsing succeeds so the function (and callers) receive a clear, contextual error instead of an unhandled exception.hooks/useOrgSettings.ts (1)
149-152: Silent failure when access token unavailable.Returning
falsewithout any user feedback whengetAccessToken()returns null may leave users confused about why their save didn't work. Consider adding a toast notification or logging to aid debugging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useOrgSettings.ts` around lines 149 - 152, The code silently returns false when getAccessToken() yields null (accessToken variable) in the hook; update the save flow in useOrgSettings (where getAccessToken() is called) to notify the user and log the failure instead of silently returning—e.g., call the app's toast/error notification utility with a clear message like "Unable to save: missing access token" and also log the error (using console.error or the project's logger) before returning false so both users and developers get feedback.hooks/useUser.tsx (2)
60-90: Inconsistent return contract compared to useOrgSettings.save().
useOrgSettings.save()returnsbooleanindicating success/failure, whileuseUser.save()returnsvoid. Consumers of these hooks may expect consistent behavior. Consider aligning the return types for predictable API usage.♻️ Proposed fix to return boolean for consistency
- const save = async () => { + const save = async (): Promise<boolean> => { if (!userData?.account_id) { - return; + return false; } setUpdating(true); try { const accessToken = await getAccessToken(); if (!accessToken) { - return; + return false; } const data = await updateAccountProfile({ accessToken, accountId: userData.account_id, instruction, organization, name, image, jobTitle, roleType, companyName, }); setUserData(data); setIsModalOpen(false); + return true; } catch { - // Error handled silently + return false; } finally { setUpdating(false); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useUser.tsx` around lines 60 - 90, The save function in useUser (save) currently returns void while useOrgSettings.save() returns a boolean; update save to return a boolean success flag: return false immediately when preconditions fail (no userData?.account_id or missing accessToken), return true after successful update (after setUserData and setIsModalOpen), and return false from the catch block on error; preserve the finally behavior (setUpdating(false)). Reference functions/variables: save, getAccessToken, updateAccountProfile, setUserData, setIsModalOpen, setUpdating.
67-70: Silent failure on missing access token lacks user feedback.Same pattern as
useOrgSettings- returning early without notifying the user may cause confusion. The user clicked save but nothing visibly happened.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useUser.tsx` around lines 67 - 70, The early return in useUser after calling getAccessToken (const accessToken = await getAccessToken(); if (!accessToken) { return; }) silently fails and provides no user feedback; update this branch to surface an error/notification (e.g., call the app's existing toast/notification utility or set an error state) and abort the save flow explicitly instead of silently returning so the user sees a meaningful message; ensure you reference getAccessToken and the accessToken check inside the useUser hook and keep behavior consistent with useOrgSettings (show notification and stop further execution).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useUser.tsx`:
- Around line 85-86: The empty catch block in the useUser hook is swallowing
errors silently; update the catch in the try/catch around the asynchronous user
logic (the catch in hooks/useUser.tsx inside the useUser hook) to at minimum log
the error via console.error or processLogger and show user feedback using the
imported toast (e.g., toast.error) so errors are visible during debugging and
the user gets notified; ensure the catch accepts an error parameter (e) and
include that error in the log/toast message for context.
---
Outside diff comments:
In `@hooks/useOrgSettings.ts`:
- Around line 144-173: The save callback lacks a catch block so failures from
updateAccountProfile (or getAccessToken) will propagate; wrap the await calls
inside a try-catch (inside the existing try/finally) and on error log the error
and return false (or rethrow if desired) instead of letting it bubble uncaught;
specifically update the save function that calls getAccessToken and
updateAccountProfile, catch exceptions, call processLogger.error (or
console.error) with the error, ensure setIsSaving(false) remains in the finally,
and only call queryClient.invalidateQueries and setOrgData when the update
succeeds.
---
Nitpick comments:
In `@hooks/useOrgSettings.ts`:
- Around line 149-152: The code silently returns false when getAccessToken()
yields null (accessToken variable) in the hook; update the save flow in
useOrgSettings (where getAccessToken() is called) to notify the user and log the
failure instead of silently returning—e.g., call the app's toast/error
notification utility with a clear message like "Unable to save: missing access
token" and also log the error (using console.error or the project's logger)
before returning false so both users and developers get feedback.
In `@hooks/useUser.tsx`:
- Around line 60-90: The save function in useUser (save) currently returns void
while useOrgSettings.save() returns a boolean; update save to return a boolean
success flag: return false immediately when preconditions fail (no
userData?.account_id or missing accessToken), return true after successful
update (after setUserData and setIsModalOpen), and return false from the catch
block on error; preserve the finally behavior (setUpdating(false)). Reference
functions/variables: save, getAccessToken, updateAccountProfile, setUserData,
setIsModalOpen, setUpdating.
- Around line 67-70: The early return in useUser after calling getAccessToken
(const accessToken = await getAccessToken(); if (!accessToken) { return; })
silently fails and provides no user feedback; update this branch to surface an
error/notification (e.g., call the app's existing toast/notification utility or
set an error state) and abort the save flow explicitly instead of silently
returning so the user sees a meaningful message; ensure you reference
getAccessToken and the accessToken check inside the useUser hook and keep
behavior consistent with useOrgSettings (show notification and stop further
execution).
In `@lib/accounts/updateAccountProfile.ts`:
- Around line 38-40: The error thrown on non-OK responses in
updateAccountProfile.ts currently only includes response.status; change the
failure path in the updateAccountProfile function to read and include response
body details (try response.json(), fallback to response.text()) and append those
details to the thrown Error or include them as a property on the Error object so
logs surface API validation/failure messages along with the HTTP status; ensure
parsing is safe (catch JSON parse errors) and still throws a clear Error when
body parsing fails.
- Around line 42-43: The call to response.json() in updateAccountProfile (where
you assign const data: UpdateAccountProfileResponse = await response.json())
should be wrapped in a try-catch to defensively handle malformed JSON; catch
JSON parsing errors, log or throw a new error that includes context (e.g.,
"Failed to parse UpdateAccountProfile response") and the original error, and
only return data.data when parsing succeeds so the function (and callers)
receive a clear, contextual error instead of an unhandled exception.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c7b1808a-a8a0-4258-ba3e-83ec9ceed5e0
⛔ Files ignored due to path filters (1)
types/Knowledge.tsis excluded by none and included by none
📒 Files selected for processing (5)
app/api/account/update/route.tsxhooks/useOrgSettings.tshooks/useUser.tsxlib/accounts/normalizeKnowledges.tslib/accounts/updateAccountProfile.ts
💤 Files with no reviewable changes (1)
- app/api/account/update/route.tsx
| } catch { | ||
| // Error handled silently |
There was a problem hiding this comment.
Empty catch block swallows errors without feedback.
The comment "Error handled silently" describes what is happening but not why this is the desired behavior. Silent error swallowing makes debugging difficult. Consider at minimum logging the error, or providing user feedback via toast (which is already imported).
🛡️ Proposed fix with user feedback
- } catch {
- // Error handled silently
+ } catch (error) {
+ console.error("Failed to save profile:", error);
+ toast.error("Failed to save profile. Please try again.");
} finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useUser.tsx` around lines 85 - 86, The empty catch block in the useUser
hook is swallowing errors silently; update the catch in the try/catch around the
asynchronous user logic (the catch in hooks/useUser.tsx inside the useUser hook)
to at minimum log the error via console.error or processLogger and show user
feedback using the imported toast (e.g., toast.error) so errors are visible
during debugging and the user gets notified; ensure the catch accepts an error
parameter (e) and include that error in the log/toast message for context.
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/useOrgSettings.ts">
<violation number="1" location="hooks/useOrgSettings.ts:20">
P1: Custom agent: **Code Structure and Size Limits for Readability and Single Responsibility**
Rule 1 (file size limit under 100 LOC) is violated: this hook remains far over the limit and this change adds more lines instead of splitting it into smaller focused hooks.</violation>
<violation number="2" location="hooks/useOrgSettings.ts:85">
P2: Validate/normalize `knowledges` from API responses before storing in state; the current direct assignment trusts unvalidated network JSON as `Knowledge[]`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
hooks/useOrgSettings.ts
Outdated
| knowledges?: Knowledge[]; | ||
| } | ||
|
|
||
| interface GetAccountResponse { |
There was a problem hiding this comment.
P1: Custom agent: Code Structure and Size Limits for Readability and Single Responsibility
Rule 1 (file size limit under 100 LOC) is violated: this hook remains far over the limit and this change adds more lines instead of splitting it into smaller focused hooks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useOrgSettings.ts, line 20:
<comment>Rule 1 (file size limit under 100 LOC) is violated: this hook remains far over the limit and this change adds more lines instead of splitting it into smaller focused hooks.</comment>
<file context>
@@ -17,6 +17,11 @@ interface OrgData {
knowledges?: Knowledge[];
}
+interface GetAccountResponse {
+ status: "success";
+ account: AccountWithDetails;
</file context>
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/useOrgSettings.ts">
<violation number="1" location="hooks/useOrgSettings.ts:92">
P2: Custom agent: **Enforce Clear Code Style and Maintainability Practices**
Restore an explicit response type here; `response.json()` returns `any`, so `data` is now implicitly `any` and breaks the strict typing requirement.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
|
Tested the deployed chat preview against the deployed API preview via the browser UI. Deployments used:
What I verified:
So the migrated account update flow is working end to end from the chat preview against the API preview. |
sweetmantech
force-pushed
the
codex/migrate-account-update-chat
branch
from
8176c39 to
d2cb78b
Compare
Browser Testing Results (Preview Deployment)Preview URL: Account update migration
|
2 participants
Summary
Verification
Summary by cubic
Moved profile updates to the dedicated
PATCH /api/accountswithBearertokens from@privy-io/react-auth. Removed the legacy route; added a small client helper; aligned org settings response handling; kept localknowledgesnormalization.Refactors
app/api/account/update/route.tsx; addedlib/accounts/updateAccountProfile.ts(callsPATCH /api/accountswithAuthorization: BearerviagetAccessToken).hooks/useUserandhooks/useOrgSettingsto useupdateAccountProfile; aligned org settings response handling; restored local state type; invalidates["accountOrganizations"]after save.knowledgesnormalization (reverted shared types/trusted response changes).Migration
/api/account/updatecalls withupdateAccountProfileor directPATCH /api/accounts.Written for commit d2cb78b. Summary will update on new commits.
Summary by CodeRabbit