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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## kyc-limit #903 +/- ##
=============================================
- Coverage 72.13% 72.02% -0.12%
=============================================
Files 226 226
Lines 8401 8417 +16
Branches 2732 2737 +5
=============================================
+ Hits 6060 6062 +2
- Misses 2105 2120 +15
+ Partials 236 235 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| error instanceof APIError && | ||
| (error.text === "already approved" || error.text === "pending") | ||
| ) | ||
| toast.show(t("Your request is being reviewed"), { |
There was a problem hiding this comment.
🚩 Missing Spanish translation for "Your request is being reviewed"
The i18n key "Your request is being reviewed" used at src/components/card/SpendingLimits.tsx:66 has no corresponding entry in src/i18n/es.json. The existing key "Error verifying identity" does have a translation (src/i18n/es.json:579). i18next will fall back to the English key string, so Spanish-language users will see English text for this toast. Not a code bug, but a content gap worth addressing.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13207cb420
ℹ️ 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".
| error instanceof APIError && | ||
| (error.text === "already approved" || error.text === "pending") | ||
| ) |
There was a problem hiding this comment.
no panda in the card-limit KYC error branch
This branch only treats APIError texts "already approved" and "pending" as expected outcomes, but the same /api/kyc card-limit flow also returns "no panda" for existing card users without a linked Panda profile. In that case the modal is already closed and the user only gets a generic failure toast, with no fallback path to request a limit increase, which is a regression from the previous Intercom-based flow for that cohort.
Useful? React with 👍 / 👎.
| const [{ Client }, { inquiryId, sessionToken }] = await Promise.all([ | ||
| import("persona"), | ||
| tokens ?? getKYCTokens("manteca", await getRedirectURI()), | ||
| tokens ?? getKYCTokens(scope, await getRedirectURI()), | ||
| ]); |
There was a problem hiding this comment.
pagehide listener when token bootstrap rejects
On web, the pagehide listener is registered before fetching Persona tokens, but if getKYCTokens(scope, ...) rejects (for common card-limit states like pending/already approved), execution exits before any cleanup callback runs. Repeated retries can accumulate orphaned global listeners until a later pagehide event, causing avoidable leaks and stale controller aborts.
Useful? React with 👍 / 👎.
aguxez
force-pushed
the
kyc-limit
branch
2 times, most recently
from
0ff1b50 to
53b9786
Compare
1 participant
This is part 2 of 2 in a stack made with GitButler:
Summary by CodeRabbit
New Features
Improvements