[TASK-15611] Simplefi integration#1309
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds SimpleFi QR payment support across QR recognition, routing, UI, history/receipt mapping, assets, and a new service; implements WebSocket status handling and order-not-ready retry flows; integrates SimpleFi into existing payment and receipt displays. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/(mobile-ui)/qr-pay/page.tsx (2)
463-477: Fix balance check for SimpleFi USER_SPECIFIED pre‑quoteSkip balance evaluation until a SimpleFi quote exists; otherwise ARS is treated as USD.
Apply:
useEffect(() => { - if (!usdAmount || usdAmount === '0.00' || isNaN(Number(usdAmount)) || balance === undefined) { + // For SimpleFi USER_SPECIFIED without a quote yet, skip balance check + if ( + paymentProcessor === 'SIMPLEFI' && + simpleFiQrData?.type === 'SIMPLEFI_USER_SPECIFIED' && + !simpleFiPayment + ) { + setBalanceErrorMessage(null) + return + } + + if (!usdAmount || usdAmount === '0.00' || isNaN(Number(usdAmount)) || balance === undefined) { setBalanceErrorMessage(null) return } @@ - }, [usdAmount, balance]) + }, [usdAmount, balance, paymentProcessor, simpleFiQrData, simpleFiPayment])
809-818: Guard Image src against nullmethodIcon can be null; Next/Image requires a valid src. Add a conditional.
Apply:
- <div className="flex items-center justify-center rounded-full bg-white"> - <Image - src={methodIcon} - alt="Payment method" - width={48} - height={48} - className="h-12 w-12 rounded-full object-cover" - /> - </div> + <div className="flex items-center justify-center rounded-full bg-white"> + {methodIcon ? ( + <Image + src={methodIcon} + alt="Payment method" + width={48} + height={48} + className="h-12 w-12 rounded-full object-cover" + /> + ) : ( + <Icon name="qr-code" size={32} /> + )} + </div>
🧹 Nitpick comments (3)
src/components/Global/DirectSendQR/utils.ts (1)
212-239: Consider adding JSDoc documentation for the parser.The
parseSimpleFiQrfunction correctly checks patterns in order of specificity (STATIC → DYNAMIC → USER_SPECIFIED), preventing false matches. However, adding JSDoc would improve maintainability.Apply this diff to add documentation:
+/** + * Parses a SimpleFi QR code URL and returns the extracted data with type discrimination. + * Attempts to match patterns in order of specificity: STATIC, DYNAMIC, then USER_SPECIFIED. + * + * @param data - The SimpleFi QR code URL string + * @returns Parsed SimpleFi QR data or null if the URL doesn't match any known pattern + * + * @example + * parseSimpleFiQr('https://pagar.simplefi.tech/merchant/static') + * // => { type: 'SIMPLEFI_STATIC', merchantSlug: 'merchant' } + */ export const parseSimpleFiQr = (data: string): SimpleFiQrData | null => {src/app/(mobile-ui)/qr-pay/page.tsx (2)
227-237: usdAmount fallback can misrepresent balance for SimpleFi USER_SPECIFIEDFor SimpleFi USER_SPECIFIED before a quote exists, usdAmount falls back to ARS amount, skewing balance checks and button gating. Prefer using only quoted USD or adjust the balance check to skip until the quote exists. See suggested fix in the balance check effect below.
485-514: Retry path should honor response.currencySame as initial fetch: don’t assume ARS.
Apply:
- setCurrency({ - code: 'ARS', - symbol: 'ARS', - price: Number(response.price), - }) + setCurrency({ + code: response.currency, + symbol: response.currency, + price: Number(response.price), + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/assets/payment-apps/simplefi-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
src/app/(mobile-ui)/qr-pay/page.tsx(17 hunks)src/assets/payment-apps/index.ts(1 hunks)src/components/Global/DirectSendQR/__tests__/recognizeQr.test.ts(1 hunks)src/components/Global/DirectSendQR/index.tsx(1 hunks)src/components/Global/DirectSendQR/utils.ts(5 hunks)src/components/TransactionDetails/TransactionDetailsReceipt.tsx(2 hunks)src/components/TransactionDetails/transactionTransformer.ts(2 hunks)src/services/simplefi.ts(1 hunks)src/utils/history.utils.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/services/simplefi.ts (2)
src/utils/sentry.utils.ts (1)
fetchWithSentry(36-150)src/constants/general.consts.ts (1)
PEANUT_API_URL(43-47)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (2)
src/hooks/useTransactionHistory.ts (1)
EHistoryEntryType(11-11)src/utils/general.utils.ts (1)
formatCurrency(382-384)
src/app/(mobile-ui)/qr-pay/page.tsx (6)
src/services/simplefi.ts (2)
SimpleFiQrPaymentResponse(16-23)simplefiApi(53-73)src/components/Global/DirectSendQR/utils.ts (2)
SimpleFiQrData(210-210)parseSimpleFiQr(212-239)src/context/authContext.tsx (1)
useAuth(191-197)src/utils/history.utils.ts (1)
HistoryEntry(89-130)src/hooks/useWebSocket.ts (1)
useWebSocket(19-186)src/utils/general.utils.ts (2)
isTxReverted(1300-1303)formatNumberForDisplay(334-380)
src/utils/history.utils.ts (1)
src/hooks/useTransactionHistory.ts (1)
EHistoryEntryType(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (27)
src/assets/payment-apps/index.ts (1)
7-7: LGTM!The SimpleFi asset export follows the existing pattern and integrates cleanly with the payment apps collection.
src/components/Global/DirectSendQR/index.tsx (1)
296-303: LGTM!The SimpleFi QR types are correctly routed to the
/qr-paypage with preserved casing and appropriate cache-busting. The implementation follows the existing pattern for other payment processor QR codes.src/components/TransactionDetails/transactionTransformer.ts (2)
312-318: LGTM!The SimpleFi QR payment handling correctly maps to
qr_paymentdirection and formats the merchant name appropriately by replacing hyphens and capitalizing words.
380-391: Verify the intentional distinction between CANCELED/CANCELLED and EXPIRED/expired.The status mapping now treats uppercase
CANCELEDandEXPIRED(lines 385-386) asfailed, whileCANCELLEDandEXPIRED(line 390) map tocancelledstatus. This distinction may be intentional to handle different backend status codes, but could lead to confusion.Please confirm:
- Is the distinction between
CANCELED(failed) vsCANCELLED(cancelled) intentional?- Same for
EXPIREDvsexpired?- Are these different statuses from different systems (e.g., SimpleFi uses uppercase while others use mixed case)?
If these are truly distinct statuses with different semantics, consider adding a comment explaining the difference.
src/components/Global/DirectSendQR/__tests__/recognizeQr.test.ts (1)
40-43: LGTM!The test cases comprehensively cover all three SimpleFi QR variants (STATIC, DYNAMIC, USER_SPECIFIED) with appropriate URL patterns.
src/utils/history.utils.ts (5)
1-1: LGTM!The SimpleFi asset import aligns with the new payment processor integration.
21-21: LGTM!The
SIMPLEFI_QR_PAYMENTentry type follows the established pattern for payment processor integrations.
68-72: Lowercase status variants added for SimpleFi integration.The lowercase status values (
approved,pending,refunded,canceled,expired) complement the existing uppercase variants. This appears to accommodate SimpleFi's status format, which differs from other integrations.Note: This relates to the status mapping distinction observed in
transactionTransformer.tswhere uppercase and lowercase variants are handled differently.
141-141: LGTM!SimpleFi QR payments correctly included in the receipt-bearing transaction types.
170-172: LGTM!The SimpleFi avatar mapping is straightforward and consistent with the Manteca QR payment pattern.
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (2)
217-217: LGTM!SimpleFi QR payments correctly included in the receipt sharing logic.
265-283: Verify the unified amount calculation handles all previous scenarios correctly.The amount calculation has been significantly refactored to a unified pipeline, consolidating what was previously multi-branch logic. While this simplifies the code, please ensure:
- BigInt to Number conversion (line 281): For typical USD amounts, this is safe, but verify that no edge cases exist where precision loss could occur.
- Currency-specific formatting: Confirm that the removal of previous currency-specific branches doesn't affect display for non-USD transactions (e.g., ARS, BRL).
- Token vs. currency amount handling: Verify that token amounts and currency amounts are handled correctly across all transaction types (deposits, withdrawals, QR payments, etc.).
Consider adding integration tests to validate amount display across different transaction types and currencies, especially for:
- Bank deposits with currency conversion
- QR payments (Manteca, SimpleFi)
- Token-based transactions
- Transactions with reward tokens
src/components/Global/DirectSendQR/utils.ts (4)
18-20: LGTM!The SimpleFi QR type variants are well-structured and follow the existing enum pattern.
32-34: LGTM!All SimpleFi variants correctly map to the "SimpleFi" display name for consistent user-facing presentation.
66-90: LGTM!The SimpleFi regex patterns correctly distinguish between STATIC, DYNAMIC, and USER_SPECIFIED QR types, and the ordering in
REGEXES_BY_TYPEensures the most specific pattern (STATIC) is checked before the more general pattern (USER_SPECIFIED).
194-210: LGTM!The SimpleFi QR data type definitions are well-structured with clear discriminated unions, enabling type-safe handling of different QR variants.
src/app/(mobile-ui)/qr-pay/page.tsx (11)
76-87: Processor routing LGTMClear separation of SimpleFi vs Manteca by qrType. No issues.
95-106: State reset covers new SimpleFi fieldsGood coverage; avoids stale WS waits and IDs.
169-174: WebSocket hookup LGTMScoped onHistoryEntry callback works for SimpleFi only due to pending ID check.
175-191: Timeout safety net LGTM5‑minute timeout clears waiting state and informs user. Solid UX fallback.
247-251: Icon mapping LGTMSIMPLEFI variants map to a single asset; consistent with other QR types.
320-330: Merchant name derivation LGTMSlug prettification is fine; dynamic fallback is reasonable.
602-624: Order-not-ready UX LGTMClear guidance and retry/cancel options. Good.
720-799: SimpleFi success view LGTM (after currency fix)Displays ARS and USD correctly; receipt wiring looks consistent. Will be fully correct once currency derives from API.
834-841: TokenAmountInput disable logic LGTMCorrectly locks amounts when a quote exists or a lock is created.
858-880: Pay button loading/label LGTMGood UX while awaiting WS confirmation; disables during waiting state.
39-42: HistoryEntry import matches canonical definitionuseTransactionHistoryre-exportsHistoryEntryfromsrc/utils/history.utils.ts, so the imported type already aligns with the WebSocket payload.
kushagrasarathe
left a comment
kushagrasarathe
left a comment
There was a problem hiding this comment.
pr looks good, left some qns, nits, suggestions and comments, also please resolve cr comments before merging
|
@jjramirezn two more things:
|
2 participants
If approved dont merge
WhatsApp.Video.2025-10-09.at.23.44.23.mp4