Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces the Intercom message flow with an immediate modal close and a card-limit scoped Persona KYC initiation; generalizes Persona inquiry state to use a scope discriminator and adds a new cardLimit KYC entrypoint; expands server token API to accept "cardLimit" scope. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as SpendingLimits UI
participant Server as KYC Token Server
participant Persona as Persona Service
User->>UI: Click "Increase spending limit"
UI->>UI: onClose()
UI->>Server: request getKYCTokens("cardLimit")
Server-->>UI: return { inquiryId, sessionToken }
UI->>Persona: startCardLimitKYC(inquiryId, sessionToken)
Persona->>Persona: initialize cardLimit inquiry
Persona-->>UI: InquiryResult (complete / cancel / error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a new user flow for increasing card spending limits by integrating with the Persona identity verification service. It generalizes the existing Persona integration to support multiple types of KYC inquiries, making the system more flexible and scalable for future identity verification needs. The change provides a direct, automated path for users to request higher limits. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
✅ All tests passed. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/card/SpendingLimits.tsx (1)
47-50:⚠️ Potential issue | 🟠 MajorHandle non-success outcomes from
startCardLimitKYC().The handler only logs errors via
.catch(reportError)but doesn't surface resolved non-success statuses ("error","cancel") or backend rejection codes ("already approved","pending","failed") to the user. When the backend rejects because the inquiry is already approved or pending, the button appears broken with no feedback.Consider branching on the result status and displaying appropriate toasts or modals:
🛠️ Suggested approach
<Button onPress={() => { - startCardLimitKYC().catch(reportError); + startCardLimitKYC() + .then((result) => { + if (result.status === "complete") { + // optionally show success feedback or close modal + onClose(); + } + // "cancel" can be silent - user intentionally dismissed + }) + .catch((error) => { + // surface API errors like "already approved", "pending" + toast.show(t("Unable to increase spending limit"), { native: true }); + reportError(error); + }); }} primary >
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e9a0dfa3-5166-45d6-89ec-250e0cecdb5a
📒 Files selected for processing (3)
src/components/card/SpendingLimits.tsxsrc/utils/persona.tssrc/utils/server.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/persona.ts (1)
170-175:⚠️ Potential issue | 🟠 MajorDefer or poll the cache refresh until the webhook confirms the card limit update.
The
cardLimitwrite happens asynchronously in the Persona webhook (server/hooks/persona.ts:271), not in the request path. WhenonCompleteinvalidates["card", "details"]immediately (lines 170-175 web, 210-213 native), a refetch can race the webhook write and cache the old limit. Because the promise resolves right after scheduling the invalidate, there is no second refresh to pick up the new limit once the webhook completes.Either wait for a webhook acknowledgment before invalidating, or implement polling to ensure the new limit is fetched after the backend update lands.
♻️ Duplicate comments (1)
src/components/card/SpendingLimits.tsx (1)
47-50:⚠️ Potential issue | 🟠 MajorStill unresolved: handle
startCardLimitKYC()outcomes in the button handler.
startCardLimitKYC()now fulfills with{ status: "cancel" | "complete" | "error" }for SDK outcomes and only rejects preflight/API failures. This handler still only logs rejections, sopending/already approved/Persona error paths can leave the CTA looking inert and skip any explicit UX or refetch handling. Await the promise here and branch on both the resolved status and the caught API codes.example
- <Button - onPress={() => { - startCardLimitKYC().catch(reportError); - }} + <Button + onPress={async () => { + try { + const result = await startCardLimitKYC(); + if (result.status === "error") { + // show toast / inline state + } + } catch (error) { + reportError(error); + // map pending/already approved to ux + refresh path + } + }} primary >
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ebe6e209-809b-438e-833c-d8347e0478e5
📒 Files selected for processing (3)
src/components/card/SpendingLimits.tsxsrc/utils/persona.tssrc/utils/server.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/card/SpendingLimits.tsx (1)
48-50:⚠️ Potential issue | 🟠 MajorHandle the resolved
startCardLimitKYC()statuses here.
src/utils/persona.ts:startCardLimitKYC()now resolves{ status: "cancel" | "complete" | "error" }for Persona-side outcomes, so this fire-and-forget call silently drops the"error"path afteronClose()has already dismissed the sheet. Please await the result and branch onstatusinstead of relying only on.catch(reportError).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 123d3047-b3d3-489f-bf9d-592d77170f68
📒 Files selected for processing (3)
src/components/card/SpendingLimits.tsxsrc/utils/persona.tssrc/utils/server.ts
This is part 2 of 2 in a stack made with GitButler:
Summary by CodeRabbit
New Features
Improvements