Conversation
🦋 Changeset detectedLatest commit: 846895d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughAdds Bridge provider offramp support: external-account CRUD, offramp-aware /quote responses (including Optimism deposit info), transfer/static-template helpers, webhook transfer handling with Offramp analytics and push notifications, strengthened response validation, tests, i18n, and a release changeset. ChangesBridge Offramp Implementation
Sequence DiagramsequenceDiagram
participant Client
participant API as /quote Endpoint
participant BridgeUtil as bridge.getOfframpDepositDetails
participant BridgeAPI as Bridge API
participant Webhook as Webhook Handler
participant Push as Push Notifications
participant Segment as Segment Analytics
Client->>API: POST /quote (offramp, externalAccountId)
API->>BridgeUtil: getOfframpDepositDetails(externalAccountId, account, customer)
BridgeUtil->>BridgeAPI: getExternalAccount(customerId, externalAccountId)
BridgeAPI-->>BridgeUtil: external account data
BridgeUtil->>BridgeAPI: getStaticTemplates(customerId)
BridgeAPI-->>BridgeUtil: static templates
BridgeUtil->>BridgeAPI: createTransfer (if no matching template)
BridgeAPI-->>BridgeUtil: transfer created
BridgeUtil-->>API: offramp deposit details (Optimism address)
API-->>Client: quote + depositInfo
BridgeAPI->>Webhook: transfer.updated.status_transitioned (funds_received)
Webhook->>Push: Send "Withdrawal in progress" notification
Push-->>Client: Notification delivered
BridgeAPI->>Webhook: transfer.updated.status_transitioned (payment_processed)
Webhook->>Segment: track(Offramp with amount, currency, usdcAmount)
Webhook->>Push: Send "Your funds are on the way to your bank" notification
Push-->>Client: Completion notification
Webhook-->>BridgeAPI: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 188fbc9c-c51a-4982-af69-203c14ad6608
📒 Files selected for processing (10)
.changeset/swift-bridges-cross.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/i18n/es.jsonserver/i18n/pt.jsonserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.tsserver/utils/segment.ts
There was a problem hiding this comment.
Code Review
This pull request introduces bridge offramp functionality, allowing users to withdraw funds to external bank accounts. Key changes include new API endpoints for managing external accounts (POST, GET, PATCH, DELETE /external-account), support for offramp directions in the ramp API, and handling of Bridge webhook events for transfer status updates. The update also includes push notifications for withdrawal progress and Segment tracking for offramp events. Feedback focuses on relaxing validation constraints for street addresses and bank account numbers to accommodate shorter valid inputs, as well as refactoring repeated authentication and customer retrieval logic into a shared middleware.
There was a problem hiding this comment.
Actionable comments posted: 9
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 86f72493-a0b9-4f8a-89db-f41aa19e65b9
📒 Files selected for processing (10)
.changeset/swift-bridges-cross.mdserver/api/ramp.tsserver/hooks/bridge.tsserver/i18n/es.jsonserver/i18n/pt.jsonserver/test/api/ramp.test.tsserver/test/hooks/bridge.test.tsserver/test/utils/bridge.test.tsserver/utils/ramps/bridge.tsserver/utils/segment.ts
| export function updateExternalAccount( | ||
| customer: InferOutput<typeof CustomerResponse>, | ||
| externalAccountId: string, | ||
| update: InferInput<typeof UpdateExternalAccountInput>, | ||
| ) { | ||
| return request( | ||
| BridgeExternalAccount, | ||
| `/customers/${customer.id}/external_accounts/${externalAccountId}`, | ||
| {}, | ||
| { | ||
| address: update.address && { | ||
| street_line_1: update.address.streetLine1, | ||
| street_line_2: update.address.streetLine2, | ||
| city: update.address.city, | ||
| state: update.address.state, | ||
| postal_code: update.address.postalCode, | ||
| country: update.address.country, | ||
| }, | ||
| account: update.account && { | ||
| checking_or_savings: update.account.checkingOrSavings, | ||
| routing_number: update.account.routingNumber, | ||
| }, | ||
| } satisfies InferInput<typeof UpdateExternalAccount>, | ||
| "PUT", | ||
| ).then( | ||
| (externalAccount) => | ||
| ({ | ||
| addressValid: externalAccount.beneficiary_address_valid, | ||
| bankName: externalAccount.bank_name, | ||
| currency: parse(picklist(FiatCurrency), OfframpFiat[externalAccount.currency]), | ||
| id: externalAccount.id, | ||
| ownerName: externalAccount.account_owner_name, | ||
| }) satisfies InferOutput<typeof ExternalAccount>, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.ts" | head -20Repository: exactly/exa
Length of output: 609
🏁 Script executed:
cat server/utils/ramps/bridge.ts | head -50Repository: exactly/exa
Length of output: 1269
🏁 Script executed:
wc -l server/utils/ramps/bridge.tsRepository: exactly/exa
Length of output: 89
🏁 Script executed:
rg "getExternalAccount|removeExternalAccount" server/utils/ramps/bridge.ts -A 20Repository: exactly/exa
Length of output: 2715
🏁 Script executed:
rg "NOT_FOUND|EXTERNAL_ACCOUNT_NOT_FOUND" server/utils/ramps/ -B 2 -A 2Repository: exactly/exa
Length of output: 3748
🏁 Script executed:
rg "ServiceError" server/utils/ramps/bridge.ts -B 3 -A 5Repository: exactly/exa
Length of output: 2769
🏁 Script executed:
rg "ErrorCodes\." server/utils/ramps/bridge.tsRepository: exactly/exa
Length of output: 2643
🏁 Script executed:
sed -n '351,385p' server/utils/ramps/bridge.tsRepository: exactly/exa
Length of output: 1294
Add 404 normalization to updateExternalAccount() for consistency.
getExternalAccount() and removeExternalAccount() normalize Bridge 404s to ErrorCodes.EXTERNAL_ACCOUNT_NOT_FOUND, but updateExternalAccount() lets the ServiceError escape. This causes the API endpoint to return 500 for stale/deleted external account IDs instead of the proper error response.
Suggested normalization
export function updateExternalAccount(
customer: InferOutput<typeof CustomerResponse>,
externalAccountId: string,
update: InferInput<typeof UpdateExternalAccountInput>,
) {
return request(
BridgeExternalAccount,
`/customers/${customer.id}/external_accounts/${externalAccountId}`,
{},
{
address: update.address && {
street_line_1: update.address.streetLine1,
street_line_2: update.address.streetLine2,
city: update.address.city,
state: update.address.state,
postal_code: update.address.postalCode,
country: update.address.country,
},
account: update.account && {
checking_or_savings: update.account.checkingOrSavings,
routing_number: update.account.routingNumber,
},
} satisfies InferInput<typeof UpdateExternalAccount>,
"PUT",
- ).then(
+ )
+ .catch((error: unknown) => {
+ if (
+ error instanceof ServiceError &&
+ typeof error.cause === "string" &&
+ error.cause.includes(BridgeApiErrorCodes.NOT_FOUND)
+ ) {
+ throw new Error(ErrorCodes.EXTERNAL_ACCOUNT_NOT_FOUND);
+ }
+ throw error;
+ })
+ .then(
(externalAccount) =>
({
addressValid: externalAccount.beneficiary_address_valid,
bankName: externalAccount.bank_name,
currency: parse(picklist(FiatCurrency), OfframpFiat[externalAccount.currency]),
id: externalAccount.id,
ownerName: externalAccount.account_owner_name,
}) satisfies InferOutput<typeof ExternalAccount>,
);
}|
✅ All tests passed. |
Summary by CodeRabbit
New Features
Localization
Tests