feat: Add crypto-to-fiat payments and compliance router #45
Conversation
WalkthroughThis update introduces a comprehensive crypto-to-fiat payment workflow, including new database schema elements, backend API routes, validation schemas, and React components for compliance and bank account management. It adds support for bank account linking, compliance/KYC flows, payment method approval, and crypto-to-fiat payment options throughout the invoice lifecycle, with corresponding UI, backend, and validation logic. Changes
Sequence Diagram(s)Crypto-to-Fiat Payment Flow (Invoice Creation to Payment)sequenceDiagram
participant User
participant Frontend
participant ComplianceAPI
participant Backend
participant DB
participant PaymentAPI
User->>Frontend: Fill compliance form & bank account form
Frontend->>ComplianceAPI: Submit compliance info
ComplianceAPI-->>Frontend: Compliance status
Frontend->>Backend: Create payment details (bank account)
Backend->>DB: Insert payment details
DB-->>Backend: Payment details record
Backend-->>Frontend: Payment details created
User->>Frontend: Create invoice (enable crypto-to-fiat)
Frontend->>Backend: Submit invoice with paymentDetailsId, isCryptoToFiatAvailable
Backend->>DB: Store invoice with crypto-to-fiat fields
DB-->>Backend: Invoice record
Backend->>PaymentAPI: Create invoice on payment processor
PaymentAPI-->>Backend: Invoice created
Backend-->>Frontend: Invoice created
User->>Frontend: View invoice, initiate payment
Frontend->>Backend: Pay request (with requestId)
Backend->>DB: Fetch invoice, payment details, payers
DB-->>Backend: Invoice, payment details, payers
Backend->>PaymentAPI: Initiate payment (with clientUserId, paymentDetailsId)
PaymentAPI-->>Backend: Payment status
Backend-->>Frontend: Payment result
Compliance Agreement Signing (Embedded Flow)sequenceDiagram
participant User
participant Frontend
participant ComplianceAPI
participant Backend
participant DB
User->>Frontend: Open compliance form
Frontend->>ComplianceAPI: Fetch compliance status
ComplianceAPI-->>Frontend: Status (agreement pending)
User->>Frontend: Click "Sign Agreement"
Frontend->>Frontend: Open modal with agreement iframe
User->>Frontend: Complete/sign agreement in iframe
Frontend->>Frontend: Receive postMessage event from iframe
Frontend->>Backend: Update agreement status
Backend->>ComplianceAPI: Update agreement status
ComplianceAPI-->>Backend: Success
Backend->>DB: Update user agreementStatus
DB-->>Backend: Success
Backend-->>Frontend: Status updated
Frontend->>ComplianceAPI: Refetch compliance status
ComplianceAPI-->>Frontend: Status (agreement completed)
Webhook Event Handling (Settlement & Compliance Events)sequenceDiagram
participant PaymentAPI
participant WebhookHandler
participant DB
PaymentAPI->>WebhookHandler: Send webhook event (e.g., settlement.initiated)
WebhookHandler->>DB: Update request status based on event
DB-->>WebhookHandler: Success/failure
WebhookHandler-->>PaymentAPI: 200 OK / 404 Not Found
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
MantisClone
changed the title
- Added AccountPage component for managing user account settings. - Introduced BankAccountsSection for displaying and linking bank accounts. - Created BankAccountForm for adding new bank account details. - Enhanced invoice creation with payment details selection for crypto-to-fiat conversion. - Updated compliance router to handle payment details creation and linking with payers. - Added new database schema for payment details and associated payers.
- Updated webhook route to use requestId instead of paymentReference for payment status updates. - Added new statuses for settlement events and improved error handling for missing requests. - Modified invoice creation to support linking payment details and improved compliance checks. - Enhanced invoice form to manage bank account linking and payment details selection for crypto-to-fiat conversion. - Updated compliance router to support new API version and added user retrieval by email.
- Deleted AccountPage component and its associated BankAccountsSection to streamline the codebase. - Updated BankAccountForm to simplify the submission process. - Enhanced InvoiceForm to handle pending approval for payment details and added a modal for user feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
src/server/routers/invoice.ts (1)
242-250: 🛠️ Refactor suggestion
⚠️ Potential issueQuery params may contain
undefinedvalues
wallet,paymentDetailsReference,chain, andtokenare concatenated blindly; if any isundefinedthe final URL will contain…&chain=undefined, which most servers treat as the literal string"undefined".-let paymentEndpoint = `/v2/request/${invoice.requestId}/pay?wallet=${input.wallet}&clientUserId=${invoice.clientEmail}&paymentDetailsId=${invoice.paymentDetails?.payers[0].paymentDetailsReference}`; +const params = new URLSearchParams(); +if (input.wallet) params.set("wallet", input.wallet); +params.set("clientUserId", invoice.clientEmail); +if (invoice.paymentDetails?.payers[0]?.paymentDetailsReference) { + params.set("paymentDetailsId", invoice.paymentDetails.payers[0].paymentDetailsReference); +} +if (input.chain) params.set("chain", input.chain); +if (input.token) params.set("token", input.token); + +const paymentEndpoint = `/v2/request/${invoice.requestId}/pay?${params.toString()}`;This also guards against missing payers.
🧹 Nitpick comments (10)
src/components/ui/form.tsx (1)
18-18: Consider using a less-generic wrapper name thanForm.Re-exporting
FormProviderasFormcan be confusing because it shadows the intuitive expectation that<Form>would render an actual<form>element.
Renaming the alias (e.g.RHFormProvider) or exporting it with its original name improves readability and avoids accidental misuse when scanning JSX.src/components/header.tsx (1)
50-55: Highlight active navigation state for the new “Crypto to Fiat” link.The existing links don’t reflect an “active” state, so this isn’t a regression, but consider:
const isActive = pathname.startsWith("/crypto-to-fiat") ... className={cn( "text-zinc-900 hover:text-zinc-600 transition-colors", isActive && "font-semibold underline" )}Consistent active-state feedback improves usability, especially as the navigation grows.
src/components/invoice-preview.tsx (2)
9-15: MakepaymentDetailsIdtruly optional in the public API
paymentDetailsIdcan beundefined, but the prop itself is required.
This forces every caller to pass an explicitundefined, and TypeScript will not prevent someone from passing an empty string, which would then be treated as “valid”.-interface InvoicePreviewProps { - data: Partial<InvoiceFormValues>; - paymentDetailsId: string | undefined; -} +interface InvoicePreviewProps { + data: Partial<InvoiceFormValues>; + paymentDetailsId?: string; +}This keeps the public contract minimal and avoids boiler-plate in call-sites.
207-214: Account-number masker silently fails on formatted inputs
replace(/^\d+(?=\d{4})/, "****")only masks when the value is all digits.
Real bank strings often contain spaces or hyphens ("1234-5678-9012-3456"), so nothing would be masked.A more resilient approach:
- {selectedPaymentDetails.paymentDetails.accountNumber.replace( - /^\d+(?=\d{4})/, - "****", - )} + {selectedPaymentDetails.paymentDetails.accountNumber + .replace(/\s|-/g, "") // strip separators first + .replace(/^\d+(?=\d{4})/, "****")}This guarantees the leading part is obscured regardless of formatting.
src/components/compliance-status.tsx (1)
11-43: Tighten type-safety to eliminate “impossible” states
getStatusIcon/Text/Coloraccept a plainstring.
Passing anything other than"pending" | "completed" | "not_started"will silently fall into the default branch.-const getStatusIcon = (status: string) => { +type Status = "not_started" | "pending" | "completed"; +const getStatusIcon = (status: Status) => {Make the same change for the other helpers.
This lets TypeScript warn if a new status is introduced but not handled.src/components/compliance-form.tsx (1)
230-236: Open KYC URL without giving it control over the parent windowUsing bare
window.open(url, "_blank")allows the new tab to accesswindow.opener, which can navigate the original page (tab-napping).
Passnoopener,noreferrerin the features list:-window.open(complianceData?.kycUrl, "_blank"); +window.open(complianceData?.kycUrl, "_blank", "noopener,noreferrer");This blocks the attack vector at virtually no cost.
src/server/db/schema.ts (2)
89-91:updatedAtis never touched on UPDATEA default value only fires on INSERT. Without a DB trigger or application-side update,
updatedAtwill stay equal tocreatedAtforever.
Either:
- add a Postgres trigger (
ON UPDATE … SET updated_at = NOW()), or- update the column manually in every mutation touching
paymentDetailsTable.
141-144: Cascade delete may orphan invoices
paymentDetailsIdonrequestTablehasonDelete: "cascade". Deleting a bank account will silently nuke every invoice pointing to it, which could break audit trails.
ConsideronDelete: "set null"so historical invoices remain intact, or disallow deletion when invoices exist.src/server/routers/compliance.ts (1)
161-170: UnuseduserIdparameter and dead code
allowPaymentDetailsreceivesuserIdbut never uses it, which is misleading and may hide bugs (e.g. privilege escalation). Remove the parameter or apply it in an authorisation check.src/server/routers/invoice.ts (1)
25-31: Avoid sending empty optional fields to/v2/request
cryptoToFiatAvailableis forwarded even whenfalse, which could fail validation on strict APIs. Build the payload dynamically:- const response = await apiClient.post("/v2/request", { + const basePayload = { amount: totalAmount.toString(), payee: input.walletAddress, invoiceCurrency: input.invoiceCurrency, paymentCurrency: input.paymentCurrency, - cryptoToFiatAvailable: input.cryptoToFiatAvailable, + }; + if (input.cryptoToFiatAvailable) { + basePayload.cryptoToFiatAvailable = true; + } + const response = await apiClient.post("/v2/request", basePayload);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
package.json(2 hunks)src/app/api/webhook/route.ts(4 hunks)src/app/crypto-to-fiat/page.tsx(1 hunks)src/app/invoices/[ID]/page.tsx(2 hunks)src/app/invoices/create/page.tsx(1 hunks)src/components/bank-account-form.tsx(1 hunks)src/components/compliance-form.tsx(1 hunks)src/components/compliance-status.tsx(1 hunks)src/components/crypto-to-fiat.tsx(1 hunks)src/components/header.tsx(1 hunks)src/components/invoice-creator.tsx(4 hunks)src/components/invoice-form.tsx(3 hunks)src/components/invoice-preview.tsx(3 hunks)src/components/invoice-table.tsx(1 hunks)src/components/payment-section.tsx(2 hunks)src/components/ui/dialog.tsx(1 hunks)src/components/ui/form.tsx(1 hunks)src/lib/schemas/bank-account.ts(1 hunks)src/lib/schemas/compliance.ts(1 hunks)src/lib/schemas/invoice.ts(1 hunks)src/server/db/schema.ts(4 hunks)src/server/index.ts(1 hunks)src/server/routers/compliance.ts(1 hunks)src/server/routers/invoice.ts(14 hunks)src/server/routers/payment.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/server/routers/invoice.ts (1)
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:88-109
Timestamp: 2025-02-12T12:40:14.742Z
Learning: The payRequest endpoint in src/server/routers/invoice.ts is intentionally kept public (using publicProcedure) to allow invoice sharing and payment by anyone with the payment reference, similar to how payment links work in other payment systems.
🧬 Code Graph Analysis (10)
src/server/routers/payment.ts (1)
src/lib/axios.ts (1)
apiClient(3-8)
src/components/crypto-to-fiat.tsx (2)
src/server/db/schema.ts (1)
User(231-231)src/components/compliance-form.tsx (1)
ComplianceForm(57-497)
src/app/crypto-to-fiat/page.tsx (4)
src/server/auth/index.ts (1)
getCurrentSession(96-103)src/components/header.tsx (1)
Header(9-82)src/components/crypto-to-fiat.tsx (1)
CryptoToFiat(13-44)src/components/footer.tsx (1)
Footer(3-17)
src/components/compliance-status.tsx (1)
src/components/ui/card.tsx (2)
Card(79-79)CardContent(79-79)
src/components/invoice-creator.tsx (1)
src/components/invoice-preview.tsx (1)
InvoicePreview(12-248)
src/components/invoice-preview.tsx (3)
src/lib/schemas/invoice.ts (1)
InvoiceFormValues(51-51)src/trpc/react.tsx (1)
api(12-12)src/trpc/server.ts (1)
api(31-61)
src/app/api/webhook/route.ts (2)
src/server/db/index.ts (1)
db(10-12)src/server/db/schema.ts (3)
requestTable(111-145)userTable(41-49)paymentDetailsPayersTable(94-109)
src/server/routers/compliance.ts (6)
src/server/trpc.ts (3)
router(11-11)publicProcedure(12-12)protectedProcedure(39-39)src/lib/schemas/compliance.ts (1)
complianceFormSchema(3-20)src/lib/axios.ts (1)
apiClient(3-8)src/lib/schemas/bank-account.ts (2)
bankAccountSchema(3-106)BankAccountFormValues(108-108)src/server/db/index.ts (1)
db(10-12)src/server/db/schema.ts (3)
paymentDetailsTable(51-91)userTable(41-49)paymentDetailsPayersTable(94-109)
src/components/ui/form.tsx (2)
src/lib/utils.ts (1)
cn(4-6)src/components/ui/label.tsx (1)
Label(26-26)
src/components/ui/dialog.tsx (1)
src/lib/utils.ts (1)
cn(4-6)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build
🔇 Additional comments (40)
src/server/routers/payment.ts (1)
19-19: API endpoint updated to v2The change from "v1/pay" to "v2/pay" endpoint aligns with the PR objective of adding crypto-to-fiat payment support. This ensures the payments router uses the latest Request Network API version.
package.json (1)
21-21: Dependency updates support new crypto-to-fiat featuresThe package updates and additions align with the new functionality:
- New
@radix-ui/react-dialogdependency supports the modal dialog components- Updates to form handling libraries (
react-hook-form,@hookform/resolvers,zod) support enhanced validation for compliance formsAlso applies to: 27-27, 29-29, 55-55, 63-63
src/lib/schemas/bank-account.ts (5)
3-39: Well-structured bank account schema with comprehensive field validationThe schema appropriately defines both required fields (bank name, account name, etc.) and optional fields that may be needed based on currency and payment rail requirements.
40-50: Appropriate conditional validation for individual beneficiaries and US addressesThe refinement logic correctly enforces:
- Date of birth requirement for individual beneficiaries
- State requirement for US addresses
51-67: Comprehensive USD-specific validation logicThe USD validation correctly handles both domestic ACH transfers (requiring account number and routing number) and international SWIFT transfers (requiring SWIFT BIC and either IBAN or account number).
68-106: Currency-specific validation for EUR, GBP, and INRThe validation rules correctly implement the specific requirements for each currency:
- EUR: IBAN required (plus SWIFT BIC for swift rails)
- GBP: Both IBAN and SWIFT BIC required
- INR: Account number, SWIFT BIC, and IFSC required
These align with international banking standards for each currency.
108-108: Type export enables type safety throughout the applicationExporting the
BankAccountFormValuestype ensures type safety when using this schema in form handling and API calls.src/components/ui/dialog.tsx (5)
1-16: Dialog component properly configured for client-side renderingThe component correctly:
- Uses the "use client" directive for Next.js client components
- Imports necessary dependencies from Radix UI
- Creates aliased components for simpler usage
17-31: Well-implemented DialogOverlay with proper animation statesThe overlay implementation:
- Properly forwards refs
- Uses appropriate z-index for modal positioning
- Implements animation states for smooth transitions
- Preserves className extensibility
32-54: Accessible DialogContent with proper close buttonThe dialog content:
- Positions the dialog centered on screen
- Includes comprehensive animations for open/close states
- Provides an accessible close button with proper screen reader text
- Preserves className extensibility for styling customization
56-109: Well-structured dialog layout componentsThe additional layout components (Header, Footer, Title, Description) follow consistent patterns:
- Use appropriate styling defaults
- Preserve className extensibility
- Forward refs when extending Radix primitives
- Include proper displayName properties for debugging
111-122: Complete export of all dialog componentsAll necessary components are properly exported, providing a complete API for dialog usage throughout the application.
src/app/invoices/create/page.tsx (1)
39-44: Verify downstream typing & null-safety forcurrentUser.id.
user.idis now forwarded toInvoiceCreator.
Ensure:
InvoiceCreator’s prop type marksidasstring(or the correct type) and optional if a guest flow is still possible.- All calculations or API calls that use
idhandle the non-empty constraint or guard against undefined.If these points are already covered, no action is needed.
src/components/invoice-table.tsx (1)
251-251: Parameter update aligns with API changes.The change from
paymentReferencetorequestIdaligns with the broader architectural change in the codebase whererequestIdbecomes the primary identifier for invoice requests.src/lib/schemas/invoice.ts (1)
34-35: New fields enable crypto-to-fiat functionality.Two new fields have been added to the invoice schema:
cryptoToFiatAvailableto indicate whether crypto-to-fiat payment is available for an invoicepaymentDetailsIdto store a reference to the associated payment detailsThese additions support the PR's main feature of crypto-to-fiat payments and compliance handling. The validation rules (boolean with false default and optional string) are appropriate for these fields.
src/server/index.ts (1)
2-2: Compliance router integration.The compliance router is added to the application router, enabling frontend components to interact with compliance-related API endpoints. This appropriately extends the API surface to support the new crypto-to-fiat payment and compliance features.
Also applies to: 12-12
src/components/payment-section.tsx (2)
99-99: Parameter update aligns with API changes.The change from
paymentReferencetorequestIdaligns with the broader architectural change in the codebase whererequestIdbecomes the primary identifier for invoice requests.
281-281: Parameter update aligns with API changes.Similar to the previous change, this updates the parameter in the payment request mutation to use
requestIdinstead ofpaymentReference, maintaining consistency with backend API changes.src/app/invoices/[ID]/page.tsx (2)
29-31: Payment details retrieval looks goodThe code correctly fetches the payment details associated with the invoice using the
paymentDetailsIdfield, with a fallback to an empty string when the ID doesn't exist.
217-256: Bank account details UI implementation is completeThe implementation properly handles conditional rendering of bank account details with appropriate security measures (masking account numbers). The section only displays when payment details exist, and each field is conditionally rendered based on availability.
One minor suggestion for future enhancement:
- {paymentDetailsData.paymentDetails.accountNumber.replace( - /^\d+(?=\d{4})/, - "****", - )} + {paymentDetailsData.paymentDetails.accountNumber.replace( + /^.+(?=.{4}$)/, + "****", + )}The current regex might not properly mask non-numeric account numbers. The alternative would handle all character types.
src/components/invoice-creator.tsx (4)
24-25: Adding ID to currentUser interface is necessaryAdding the
idproperty to thecurrentUserinterface is necessary for proper user identification in the crypto-to-fiat payment flow.
71-71: Default setting for cryptoToFiatAvailable is appropriateSetting the default value of
cryptoToFiatAvailabletofalseensures that invoices don't use this feature unless explicitly enabled.
85-85: Improved error handlingExplicitly setting
form.formState.isSubmitSuccessful = falseensures proper form state management after submission errors.
105-108: Correctly passes payment details to preview componentThe code now properly passes the
paymentDetailsIdto theInvoicePreviewcomponent, enabling it to fetch and display bank account details in the preview.src/app/crypto-to-fiat/page.tsx (3)
1-6: Appropriate imports for crypto-to-fiat pageThe imports include all necessary components and utilities for the page functionality.
7-14: Authentication check is properly implementedThe page correctly checks for user authentication and redirects unauthenticated users to the home page, providing appropriate access control for the crypto-to-fiat feature.
15-31: Page structure is well organizedThe page layout includes a header with user info, a clearly labeled main content area with appropriate title and subtitle, and the
CryptoToFiatcomponent. The footer is also properly included. The structure follows the application's UI patterns.src/components/crypto-to-fiat.tsx (2)
1-12: Component imports are complete and organizedThe component imports all necessary UI components and icons, and properly imports the
Usertype from the schema and theComplianceFormcomponent.
13-44: Well-structured crypto-to-fiat componentThe component is well-structured with a clear user interface:
- Requires a
userprop of typeUser- Renders a styled card with appropriate header, content, and footer sections
- Includes a compliance warning with visual indicator explaining KYC requirements
- Integrates the
ComplianceFormcomponent with proper propsThe UI is consistent with the application's design system and provides clear guidance to users about compliance requirements.
src/components/bank-account-form.tsx (3)
82-91: Surface error feedback and prevent double-submission
onSubmit()silently logs the error toconsoleand leaves the form in an indeterminate state.
Consider surfacing the error to the user and disabling the submit button while the mutation is in-flight.async function onSubmit(data: BankAccountFormValues) { - try { - await mutateAsync({ + try { + setIsSubmitting(true); + await mutateAsync({ userId: user.id, paymentDetailsData: data, }); } catch (error) { - console.error("Error creating bank account:", error); + toast.error("Failed to save bank account. Please try again."); + console.error("Error creating bank account:", error); } finally { + setIsSubmitting(false); } }You’ll need an
isSubmittingstate and to pass it to the<Button disabled={isSubmitting}>.
This greatly improves UX and avoids accidental duplicate mutations.
[ suggest_essential_refactor ]
193-203: Verify currency casing consistencyAll currency codes are lowercase (
"usd","eur"…).
Backend schema and external APIs usually expect ISO-4217 uppercase ("USD").
If the Zod schemabankAccountSchemais case-sensitive and/or the API requires uppercase, these values will fail validation or cause 422 errors at runtime.Please confirm the expected casing end-to-end and, if necessary, switch to uppercase:
- <SelectItem value="usd">USD - US Dollar</SelectItem> + <SelectItem value="USD">USD - US Dollar</SelectItem>[ request_verification ]
267-281: Date-of-birth shown for businesses
dateOfBirthis displayed and marked required for every beneficiary type, yet it is meaningless whenbeneficiaryType === "business".
Either hide the field conditionally or relax validation for businesses to avoid user confusion and failed submissions.
[ suggest_optional_refactor ]src/app/api/webhook/route.ts (4)
47-52: Error message still references removed “payment reference” fieldLog / thrown messages still say payment reference although the key is now
requestId.
This makes debugging harder.- `No request found with payment reference: ${requestId}`, + `No request found with requestId: ${requestId}`,Apply the same change to every similar branch.
[ suggest_nitpick ]Also applies to: 64-69, 82-87, 101-106
55-70: Kill the boilerplate – extract a reusableupdateRequestStatushelperThe four settlement branches are almost identical. A tiny helper eliminates repetition, lowers cognitive load and makes future additions less error-prone.
+async function updateRequestStatus(tx: AnyPgTransaction, id: string, status: string) { + const res = await tx + .update(requestTable) + .set({ status }) + .where(eq(requestTable.requestId, id)) + .returning({ id: requestTable.id }); + + if (!res.length) { + throw new Error(`No request found with requestId: ${id}`); + } +} case "settlement.initiated": await db.transaction((tx) => updateRequestStatus(tx, requestId, "offramp_initiated")); break;DRY & safer.
[ suggest_optional_refactor ]
181-189:compliance.updateddoes not confirm update successUnlike other branches, this update isn’t wrapped in a transaction and does not check
rowCount.
If the user email is wrong the webhook will silently succeed and state will diverge.- await db - .update(userTable) - ... - .where(eq(userTable.email, body.clientUserId)); +const res = await db + .update(userTable) + ... + .where(eq(userTable.email, body.clientUserId)); +if (!res.rowCount) { + throw new Error(`No user found with email: ${body.clientUserId}`); +}[ flag_critical_issue ]
191-202: Potential column mismatch when updating payment details status
paymentDetailsPayersTable.paymentDetailsReferenceis compared tobody.paymentDetailsId.
But according to the schema,paymentDetailsReferencestores the external reference, whereaspaymentDetailsIdsounds like an internal UUID. Mixing them will leave rows untouched.Please confirm which identifier the webhook sends. If it’s the internal
paymentDetailsId, change the predicate to:- eq(paymentDetailsPayersTable.paymentDetailsReference, body.paymentDetailsId) + eq(paymentDetailsPayersTable.paymentDetailsId, body.paymentDetailsId)and add the usual “no row affected” guard.
[ flag_critical_issue ][ request_verification ]src/components/invoice-form.tsx (4)
112-134: Type/shape mismatch forlinkedPaymentDetailscauses runtime errors
linkedPaymentDetailsis declared as{ paymentDetails: PaymentDetails; paymentDetailsPayers: ... }[]but you populate it with
paymentDetailsData.paymentDetails.filter(...), whose elements are alreadyPaymentDetails & { paymentDetailsPayers: ... }.Downstream you access both variants – sometimes
detail.paymentDetails.id, sometimesdetail.accountName. This will break at runtime and TypeScript should already be complaining.Recommended fix: store the flat shape and update all usages:
// state - const [linkedPaymentDetails, setLinkedPaymentDetails] = useState< - | { - paymentDetails: PaymentDetails; - paymentDetailsPayers: (User & PaymentDetailsPayers)[]; - }[] - | undefined ->(undefined); +const [linkedPaymentDetails, setLinkedPaymentDetails] = + useState<(PaymentDetails & { paymentDetailsPayers: (User & PaymentDetailsPayers)[] })[]>(); // setter setLinkedPaymentDetails(validPaymentDetails); // consumers - detail.paymentDetails.id + detail.id - detail.paymentDetails.accountName + detail.accountNameAdjust all similar member accesses (
accountNumber,id, etc.).
Without this, the invoice form will throw or silently useundefinedvalues.
[ flag_critical_issue ]Also applies to: 183-204, 655-670
164-171:forminuseEffectdependency array is unnecessary
formis a stable object reference fromreact-hook-form, but React still treats it as a prop-like dependency. Including it makes the effect re-run on every render and may lead to performance issues or inadvertent infinite loops.Remove
formfrom the dependency list and rely on the specificform.watch(...)values you already list.
[ suggest_nitpick ]
174-211: Double submission risk & optimistic UI
handleFormSubmitsetsinvoiceCreatedafter awaitingonSubmit, but the button label is controlled byisLoading, which comes from the parent. If the parent forgets to setisLoadingback tofalse, the button stays disabled forever.Consider:
- Deriving local loading state instead of relying exclusively on parent prop.
- Disabling the submit button immediately inside
handleFormSubmitto prevent rapid double clicks.This will keep the UX consistent even if the parent component mis-manages
isLoading.
[ suggest_optional_refactor ]
214-221:error.messagemay be undefined
errorinonErroris typed asunknown. Accessingerror.messagewithout narrowing breaks type safety.onError: (error) => { - toast.error(`Failed to link bank account: ${error.message}`); + const msg = + error instanceof Error ? error.message : "Unknown error"; + toast.error(`Failed to link bank account: ${msg}`); },[ suggest_nitpick ]
…rk/easy-invoice into feat/crypto-to-fiat
- Added a new handleAgreementUpdate function to manage agreement status updates. - Implemented origin validation for postMessage events to ensure security. - Refactored the event listener to utilize the new update function for better code organization.
- Implemented a timeout effect that automatically closes the bank account modal after 1 minute if not approved. - Added a toast notification to inform users when the timeout occurs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/invoice-form.tsx (3)
74-88: Bank account modal timeout could be configurableThe hard-coded 1-minute timeout for the bank account modal approval might not be sufficient for all users. This could lead to frustration if the approval process takes longer than expected.
Consider making this timeout duration configurable or extending it:
- timeoutId = setTimeout(() => { - setShowBankAccountModal(false); - toast.error("Bank account approval timed out. Please try again."); - }, 60000); // 1 minute timeout + // Allow at least 3 minutes for bank account approval + timeoutId = setTimeout(() => { + setShowBankAccountModal(false); + toast.error("Bank account approval timed out. Please try again."); + }, 180000); // 3 minute timeout
643-730: Payment details selection has complex conditional renderingThe payment details selection UI has multiple nested conditions which makes it hard to follow. While functionally correct, this could be refactored for better readability.
Extract the payment details selection UI into a separate component:
type PaymentDetailsSelectionProps = { clientEmail: string; isLoadingUser: boolean; clientUserData: User | undefined; linkedPaymentDetails: { paymentDetails: PaymentDetails; paymentDetailsPayers: (User & PaymentDetailsPayers)[] }[] | undefined; form: UseFormReturn<InvoiceFormValues>; onAddBankAccount: () => void; }; const PaymentDetailsSelection = ({ clientEmail, isLoadingUser, clientUserData, linkedPaymentDetails, form, onAddBankAccount, }: PaymentDetailsSelectionProps) => { if (!clientEmail) { return <p className="text-sm text-gray-500">Enter client email to view payment details</p>; } if (isLoadingUser) { return <div className="text-sm text-gray-500">Checking client details...</div>; } if (!clientUserData) { return <p className="text-sm text-gray-500">Client not compliant. Please use a verified client</p>; } if (!clientUserData.isCompliant) { return <p className="text-sm text-gray-500">Client is not valid for Crypto to Fiat</p>; } if (linkedPaymentDetails && linkedPaymentDetails.length > 0) { return ( <div className="space-y-2"> <Select onValueChange={(value) => form.setValue("paymentDetailsId", value)} defaultValue={form.getValues("paymentDetailsId")} > <SelectTrigger> <SelectValue placeholder="Select payment details" /> </SelectTrigger> <SelectContent> {/* ... existing select items code ... */} </SelectContent> </Select> </div> ); } return ( <div className="space-y-4"> <p className="text-sm text-gray-500">No bank accounts found for this client</p> <Button type="button" variant="outline" onClick={onAddBankAccount} className="w-full" > Add Bank Account </Button> </div> ); }; // Then in the main component: // <PaymentDetailsSelection // clientEmail={clientEmail} // isLoadingUser={isLoadingUser} // clientUserData={clientUserData} // linkedPaymentDetails={linkedPaymentDetails} // form={form} // onAddBankAccount={() => setShowBankAccountModal(true)} // />
106-116: Polling interval for payment details could be configuredThe polling interval for payment details is hardcoded to 30 seconds. This value might need to be adjusted based on real-world usage patterns.
Consider making the polling interval configurable or commenting on why this specific interval was chosen:
api.compliance.getPaymentDetails.useQuery( { userId: clientUserData?.id ?? "" }, { enabled: !!clientUserData?.id, - // Add polling to refresh data every 30 seconds - refetchInterval: 30000, + // Poll every 30 seconds to check for payment detail status updates + // This interval balances API load with responsiveness + refetchInterval: 30000, // 30 seconds // Also refetch when the window regains focus refetchOnWindowFocus: true, }, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
src/components/invoice-form.tsx(3 hunks)
🔇 Additional comments (6)
src/components/invoice-form.tsx (6)
189-227: Form submission logic handles edge cases wellThe form submission handler properly validates that crypto-to-fiat payments have proper payment details that are approved before allowing submission. It also handles the different states appropriately.
229-263: Error handling for bank account linking is comprehensiveThe mutation for linking payment details includes proper error handling, success notifications, and updates the form state correctly.
267-308: Dialogs provide good user feedbackThe bank account and pending approval dialogs provide clear feedback to users about the status of their actions, including appropriate visual indicators.
630-641: Crypto-to-fiat toggle implementation is clearThe implementation of the crypto-to-fiat toggle is straightforward and correctly updates the form state.
732-744: Button state management is clear and user-friendlyThe submit button text and disabled state adapt correctly to the form's status, providing good feedback to the user.
3-41: Good imports organizationThe imports are well organized by categories (components, libraries, types) which improves readability.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/components/invoice-form.tsx (6)
74-88: Consider configuring a longer timeout for bank account modalThe current 1-minute timeout for bank account approval might be too short for real-world scenarios. Bank account verification processes can sometimes take longer, especially if there are external API calls or manual verification steps involved.
- timeoutId = setTimeout(() => { - setShowBankAccountModal(false); - toast.error("Bank account approval timed out. Please try again."); - }, 60000); // 1 minute timeout + timeoutId = setTimeout(() => { + setShowBankAccountModal(false); + toast.error("Bank account approval timed out. Please try again."); + }, 300000); // 5 minute timeout
98-116: Make refetch interval configurable via environment variablePolling every 30 seconds could generate significant load on your backend in production with many active users. Consider making this interval configurable via an environment variable.
// Add at the top of the file after imports + const PAYMENT_DETAILS_REFETCH_INTERVAL = + process.env.NEXT_PUBLIC_PAYMENT_DETAILS_REFETCH_INTERVAL ? + parseInt(process.env.NEXT_PUBLIC_PAYMENT_DETAILS_REFETCH_INTERVAL, 10) : + 30000; // Then in the query options // Add polling to refresh data every 30 seconds - refetchInterval: 30000, + refetchInterval: PAYMENT_DETAILS_REFETCH_INTERVAL,This suggestion assumes Next.js is being used, as implied by the file structure. You should verify that environment variables are properly configured in your project.
118-138: Simplify the filter function using optional chaining and early returnsThe filter function can be simplified for better readability and maintainability.
const filterValidPaymentDetails = ( paymentDetails: (PaymentDetails & { paymentDetailsPayers: (User & PaymentDetailsPayers)[] })[], clientEmail: string ): (PaymentDetails & { paymentDetailsPayers: (User & PaymentDetailsPayers)[] })[] => { return paymentDetails.filter((detail) => { - const hasMatchingPayer = detail.paymentDetailsPayers.some( - (payer: User & PaymentDetailsPayers) => payer.email === clientEmail - ); - - if (hasMatchingPayer) { - const matchingPayer = detail.paymentDetailsPayers.find( - (payer: User & PaymentDetailsPayers) => payer.email === clientEmail - ); - if (matchingPayer) { - return matchingPayer.isCompliant; - } - } - return false; + const matchingPayer = detail.paymentDetailsPayers.find( + (payer: User & PaymentDetailsPayers) => payer.email === clientEmail + ); + return matchingPayer?.isCompliant ?? false; }); };
297-310: Add loading state to the Close button in the pending approval dialogWhen a user is waiting for payment details approval, they might click the close button multiple times. Adding a disabled state during submission would prevent potential issues.
<Button variant="outline" onClick={() => setShowPendingApprovalModal(false)} + disabled={isLoading} className="mt-4" > - Close + {isLoading ? "Processing..." : "Close"} </Button>
736-749: Add proper ARIA attributes to the submit button for accessibilityThe submit button should have proper ARIA attributes to indicate its loading and success states for screen readers.
<Button type="submit" className="bg-black hover:bg-zinc-800 text-white transition-colors" disabled={isLoading} + aria-busy={isLoading} + aria-live="polite" > {isLoading ? "Creating..." : invoiceCreated ? "Invoice created" : "Create Invoice"} </Button>
56-751: Consider breaking down this large component into smaller, focused componentsThe
InvoiceFormcomponent is over 600 lines long, which makes it difficult to maintain and test. Consider splitting it into smaller components with focused responsibilities.You could break this component down into:
InvoiceFormHeader- For handling the invoice metadata (number, date, recurring status)InvoiceFormClient- For managing client information and compliance statusInvoiceFormItems- For managing the invoice line itemsInvoiceFormPayment- For handling payment details, crypto-to-fiat settings, and bank account linkingInvoiceFormFooter- For the notes and submit buttonThis would improve maintainability, testability, and ease future enhancements.
…yment details schema
- Added components for displaying payment details status, loading, error, and empty states. - Introduced a new PaymentDetailsSection to handle the display logic based on client email and user compliance. - Refactored payment details handling to improve readability and maintainability. - Implemented a helper function to check the approval status of payment details.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/invoice-form.tsx (1)
282-328: Helper functions are still recreated on every renderThe previous review already suggested extracting
filterValidPaymentDetailsandcheckPaymentDetailsApprovalout of theuseEffect.
Keeping them inside the hook recreates the functions on every dependency change, needlessly invalidating React’s dependency equality checks and complicating testing.Move both helpers to module scope (or
useCallbackif they need props/state).
🧹 Nitpick comments (1)
src/components/invoice-form.tsx (1)
239-252: PreferReturnType<typeof setTimeout>overNodeJS.Timeoutfor browser code
setTimeouton the browser returns anumber, whereas in Node it returnsNodeJS.Timeout.
Typing the variable asNodeJS.Timeoutcauses TS errors when the DOM libs are in scope (Type 'number' is not assignable to type 'Timeout').
The portable pattern is:-useEffect(() => { - let timeoutId: NodeJS.Timeout; +useEffect(() => { + let timeoutId: ReturnType<typeof setTimeout>;This keeps the code environment-agnostic and removes the need for
// @ts-ignorework-arounds later.
…oss components and schemas - Updated variable names in InvoiceCreator, InvoiceForm, and related schemas for consistency. - Ensured all references to cryptoToFiatAvailable are replaced with isCryptoToFiatAvailable to improve clarity.
- Updated PaymentDetailsStatus component to display "Unknown" when status is not provided. - Refactored type definitions for linkedPaymentDetails for better clarity. - Corrected a typo in the condition checking for crypto to fiat availability.
- Introduced paymentReference as a required field and originalRequestPaymentReference as an optional field in the request table schema to enhance payment tracking capabilities.
- Added paymentReference as a required field and originalRequestPaymentReference as an optional field in the request table schema to enhance payment tracking capabilities.
- Updated the InvoiceForm component to maintain the isSubmitting state while waiting for payment approval, preventing multiple submissions. - Added logic to reset isSubmitting when no longer waiting for approval, enhancing user experience and preventing unintended form submissions.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/components/invoice-form.tsx (2)
367-367: Make timeout duration configurable via environment variableThe bank account approval timeout is hardcoded to 60 seconds (defined as
BANK_ACCOUNT_APPROVAL_TIMEOUT). Consider making this configurable via environment variables.
323-326:⚠️ Potential issuePrevent multiple submissions during approval wait
When showing the pending approval modal, the function sets
isSubmittingto false before returning, which could allow multiple submissions while waiting for approval.if (payer.status === PaymentDetailsStatusEnum.PENDING) { setShowPendingApprovalModal(true); setWaitingForPaymentApproval(true); - // Keep isSubmitting true while waiting for approval to prevent multiple submissions + // Keep isSubmitting true to prevent duplicate submissions while waiting return; }The comment already suggests keeping isSubmitting true, but the code doesn't actually do this. Remove the comment or implement the described behavior.
🧹 Nitpick comments (4)
src/components/invoice-form.tsx (4)
357-357: Remove unnecessary dependency from useCallbackIncluding
isSubmittingin the useCallback dependency array can cause unnecessary function recreations when this state changes, but the function itself uses the latest value through closure.- [linkedPaymentDetails, onSubmit, form.setError, router, isSubmitting], + [linkedPaymentDetails, onSubmit, form.setError, router],
397-401: Consider making polling interval configurable via environment variableThe polling interval for payment details is hardcoded to 30 seconds. Consider making this configurable via environment variables for different environments.
{ enabled: !!clientUserData?.id, // Use the configurable constant for polling interval - refetchInterval: PAYMENT_DETAILS_POLLING_INTERVAL, + refetchInterval: process.env.PAYMENT_DETAILS_POLLING_INTERVAL + ? parseInt(process.env.PAYMENT_DETAILS_POLLING_INTERVAL, 10) + : PAYMENT_DETAILS_POLLING_INTERVAL, // Also refetch when the window regains focus refetchOnWindowFocus: true, }This allows for easier configuration across environments without code changes.
464-467: Optimize dependency array for payment approval effectThe dependency array includes the entire
formobject, which can cause unnecessary effect re-runs when any form field changes.}, [ waitingForPaymentApproval, linkedPaymentDetails, clientEmail, - form, + form.getValues, + form.setValue, handleFormSubmit, isSubmitting, ]);Specifying only the required form methods will reduce effect re-executions.
892-893: Consider clarifying crypto-to-fiat descriptionThe label "Allow payment to your bank account (Crypto-to-fiat payment)" is good, but could be made clearer about the process.
- Allow payment to your bank account (Crypto-to-fiat payment) + Allow payment to your bank account (Converts crypto to fiat before transfer)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(4 hunks)src/components/invoice-form.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (1)
src/components/invoice-form.tsx (1)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:316-319
Timestamp: 2025-05-19T13:00:48.781Z
Learning: The handleFormSubmit function in src/components/invoice-form.tsx correctly uses data.clientEmail from the form submission data to find matching payers, which is the proper implementation.
🔇 Additional comments (4)
src/components/invoice-form.tsx (4)
729-729: Good usage of min attribute for quantityAdding
min="1"to the quantity input field is a good practice to prevent zero or negative quantities.
113-124: Good defensive programming for null statusProperly handling potential null status values with a fallback to "Unknown" prevents runtime errors.
292-358: Form submission workflow handles edge cases wellThe form submission handler properly:
- Validates required fields
- Checks payment details approval status
- Shows appropriate UI for pending approval
- Handles different approval states correctly
247-257: Improved error messaging for compliance checksThe error messages clearly distinguish between "not completed KYC" and "KYC application rejected" scenarios, which gives users better guidance on how to proceed.
- Updated the complianceRouter to check if the current user is a payer for the payment details, in addition to verifying ownership. - Improved error handling to deny access if the user is neither the owner nor a payer with access, enhancing security and user experience.
MantisClone
left a comment
MantisClone
left a comment
There was a problem hiding this comment.
Approved :thumbs_up: Pending comment resolution and manual regression test success! 🚧
|
Please strongly consider updating the PR Description. |
bassgeta
left a comment
bassgeta
left a comment
There was a problem hiding this comment.
Whew, after a ton of playing around with my dev env, failing webhooks and manual labour, I can confirm that when everything is set up, it works like a charm!
Nicely done, all the long polling works as intended and gating of c2f features depending on KYC is also working exactly like it should.
Now let's get this thing rolling 😎
Co-authored-by: MantisClone <david.huntmateo@request.network>
Co-authored-by: MantisClone <david.huntmateo@request.network>
- Moved the `PaymentDetailsStatus` and `Gender` enums from `enums.ts` to their respective constants files for improved structure and maintainability. - Updated imports in relevant components and schemas to reflect the new locations of the enums, ensuring consistent usage across the application.
…rk/easy-invoice into feat/crypto-to-fiat
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/server/routers/compliance.ts (6)
25-26: Consider using more specific typing for API properties.Instead of
[key: string]: unknown, define a more precise type for additional API properties if you know what they can be, or use a union type to capture the expected variation. This provides better type safety.
90-96: Use appropriate error codes based on error types.The current implementation defaults to
INTERNAL_SERVER_ERRORfor all unhandled error types. Consider using more specific error codes where possible to provide better feedback to clients.- throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: - error instanceof Error - ? `Failed to submit compliance information: ${error.message}` - : "Failed to submit compliance information", - }); + throw new TRPCError({ + code: error instanceof TypeError || error instanceof SyntaxError ? "BAD_REQUEST" : "INTERNAL_SERVER_ERROR", + message: + error instanceof Error + ? `Failed to submit compliance information: ${error.message}` + : "Failed to submit compliance information", + });
295-296: More specific error handling for constraint violations.When a constraint error occurs, it may indicate a client error rather than a server error, depending on the specific constraint. Consider differentiating between different types of constraints.
- code = "INTERNAL_SERVER_ERROR"; - message = "Server error: database constraint violation"; + // Determine if this is a client-fixable constraint or a server issue + if (errorMessage.includes("foreign key constraint") || + errorMessage.includes("check constraint")) { + code = "BAD_REQUEST"; + message = "Invalid data provided: database constraint violation"; + } else { + code = "INTERNAL_SERVER_ERROR"; + message = "Server error: database constraint violation"; + }
512-517: Remove redundant authentication check.Since this is a
protectedProcedure, the user is already guaranteed to be authenticated. The check can be safely removed as it's redundant with the procedure's own authentication enforcement.- // Since this is a protected procedure, ctx.user should exist, but we'll check to satisfy TypeScript - if (!ctx.user) { - throw new TRPCError({ - code: "UNAUTHORIZED", - message: "You must be logged in to access payment details", - }); - }
525-526: Simplify null coalescing with type assertion.Since we're already in a context where
ctx.useris guaranteed to exist (due to the condition in line 520), there's no need for the optional chaining and null coalescing.where: (payers) => - eq(payers.payerId, ctx.user?.id ?? "") && + eq(payers.payerId, ctx.user.id) && eq(payers.paymentDetailsId, input),
52-52: Enhance error logging with more details.While logging the error is good, including more context about the operation being performed would make troubleshooting easier.
- console.error("Error submitting compliance info:", error); + console.error("Error submitting compliance info:", error, { input: JSON.stringify(input, null, 2) });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (2)
src/server/routers/compliance.ts(1 hunks)src/server/routers/invoice.ts(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/routers/invoice.ts
🧰 Additional context used
🧠 Learnings (1)
src/server/routers/compliance.ts (3)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic, payees (users who created invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers. The payee is identified by the userId field in the requestTable.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic, payees (creators of invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic for crypto-to-fiat payments, payees (users who created invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers. To verify if a user is a payee, check if their user ID matches the userId field in any request that references these payment details.
🔇 Additional comments (8)
src/server/routers/compliance.ts (8)
262-262: Good use of utility function for filtering defined values.The
filterDefinedValuesutility function helps eliminate undefined/null values before database operations, which prevents schema validation errors and improves code readability.
565-572: Good security practice: Explicit column selection.Limiting the returned columns to only what's needed is a good security practice that prevents accidental exposure of sensitive data.
381-381: Good use of enum for status consistency.Using the
PaymentDetailsStatus.PENDINGenum ensures consistent status values throughout the application.
358-359: Proper URL encoding for user input.Using
encodeURIComponentfor the email address in the URL path helps prevent path injection attacks and ensures the URL is correctly formatted.
110-111: Proper URL encoding for user input.Using
encodeURIComponentfor the clientUserId in the URL path helps prevent path injection attacks and ensures the URL is correctly formatted.
38-40: Appropriate use of protected procedure.Using
protectedProcedurefor submitting compliance information ensures that only authenticated users can access this endpoint, which is important for security.
171-182: Comprehensive default response for 404 errors.Providing a default response when a user isn't found in the compliance system ensures a consistent interface for clients and simplifies error handling.
472-472: Good defensive coding with null coalescing.Using the null coalescing operator
??helps prevent runtime errors by providing a default empty object whenrow.useris undefined.
- Moved currency and chain-related constants to dedicated files for better organization and maintainability. - Updated imports across various components and schemas to reflect the new structure, ensuring consistent usage throughout the application.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/invoice-form.tsx (3)
46-48: Consider making constant values configurable via environment variablesThe hardcoded polling interval and timeout values would be better defined as configurable settings, making the application more adaptable to different environments and requirements.
-// Constants -const PAYMENT_DETAILS_POLLING_INTERVAL = 30000; // 30 seconds in milliseconds -const BANK_ACCOUNT_APPROVAL_TIMEOUT = 60000; // 1 minute timeout for bank account approval +// Constants - values from environment variables with fallbacks +const PAYMENT_DETAILS_POLLING_INTERVAL = + Number(process.env.NEXT_PUBLIC_PAYMENT_DETAILS_POLLING_INTERVAL) || 30000; // 30 seconds in milliseconds +const BANK_ACCOUNT_APPROVAL_TIMEOUT = + Number(process.env.NEXT_PUBLIC_BANK_ACCOUNT_APPROVAL_TIMEOUT) || 60000; // 1 minute timeout for bank account approval
58-80: Type casting could be avoided with proper data transformationThe function uses a potentially unsafe type casting with
as unknown as LinkedPaymentDetail[]. While this might work correctly in practice, it bypasses TypeScript's type checking which could lead to subtle bugs.const filterValidPaymentDetails = ( paymentDetails: (PaymentDetails & { paymentDetailsPayers: (User & PaymentDetailsPayers)[]; })[], clientEmail: string, ): LinkedPaymentDetail[] => { const validPaymentDetails = paymentDetails.filter((detail) => { const hasMatchingPayer = detail.paymentDetailsPayers.some( (payer: User & PaymentDetailsPayers) => payer.email === clientEmail, ); if (hasMatchingPayer) { const matchingPayer = detail.paymentDetailsPayers.find( (payer: User & PaymentDetailsPayers) => payer.email === clientEmail, ); if (matchingPayer) { return matchingPayer.isCompliant; } } return false; }); - return validPaymentDetails as unknown as LinkedPaymentDetail[]; + // Transform the data to match the expected type without type casting + return validPaymentDetails.map(detail => ({ + paymentDetails: detail.paymentDetails, + paymentDetailsPayers: detail.paymentDetailsPayers + })); };
357-357: Dependency array includesisSubmittingwhich may cause unnecessary rerendersIncluding
isSubmittingin the useCallback dependency array may cause the function to be recreated when that state changes, potentially leading to unnecessary rerenders or infinite loops.}, - [linkedPaymentDetails, onSubmit, form.setError, router, isSubmitting], + [linkedPaymentDetails, onSubmit, form.setError, router], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (8)
src/app/invoices/[ID]/page.tsx(4 hunks)src/components/direct-payment.tsx(1 hunks)src/components/invoice-form.tsx(2 hunks)src/components/invoice-preview.tsx(3 hunks)src/components/invoice-table.tsx(3 hunks)src/components/payment-section.tsx(9 hunks)src/lib/schemas/invoice.ts(3 hunks)src/lib/schemas/payment.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/lib/schemas/payment.ts
- src/components/direct-payment.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/invoice-table.tsx
- src/components/invoice-preview.tsx
- src/app/invoices/[ID]/page.tsx
- src/lib/schemas/invoice.ts
- src/components/payment-section.tsx
🧰 Additional context used
🧠 Learnings (1)
src/components/invoice-form.tsx (2)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:316-319
Timestamp: 2025-05-19T13:00:48.790Z
Learning: The handleFormSubmit function in src/components/invoice-form.tsx correctly uses data.clientEmail from the form submission data to find matching payers, which is the proper implementation.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:451-453
Timestamp: 2025-05-20T12:59:44.665Z
Learning: In the Easy Invoice project, setTimeout is required when submitting a form after modal state changes in the crypto-to-fiat payment flow. Directly calling handleFormSubmit without setTimeout after closing modals and updating state causes issues.
🔇 Additional comments (8)
src/components/invoice-form.tsx (8)
112-125: Defensive status handling looks goodThe PaymentDetailsStatus component now correctly handles null status values with proper type checking, which prevents potential runtime errors. This is a good practice.
209-271: Well-structured component compositionThe PaymentDetailsSection component effectively handles various states (loading, errors, empty state) and provides a clear, organized way to display payment details. The improved error messages provide better clarity about compliance and KYC status.
361-374: Well-implemented timeout handlingThe timeout effect for the approval modal is well implemented with proper cleanup using clearTimeout in the effect's cleanup function. This prevents memory leaks and ensures the timeout is properly managed.
391-402: Good use of conditional fetching and pollingThe implementation of the payment details query with conditional fetching (
enabled: !!clientUserData?.id), configurable polling interval, and focus-based refetching is a good practice for keeping data fresh without unnecessary API calls.
480-510: Good error handling in bank account success handlerThe
handleBankAccountSuccessfunction includes proper error handling with specific error messages and graceful fallbacks. The use of toast notifications provides good feedback to the user.
532-563: Modal UX with clear user instructionsThe pending approval modal provides clear feedback to users with an animated spinner and informative text explaining what's happening. The modal also clarifies that users can close it and will be notified when approval is complete, which is good UX.
873-894: Good field clearing logic when toggling crypto-to-fiatThe implementation correctly clears irrelevant fields and errors when toggling between crypto-to-fiat and regular payment modes. The comments explaining the logic improve code readability.
951-963: Button state handling is user-friendlyThe submit button correctly reflects different states (normal, loading, completed) with appropriate text and disabled state. This provides clear feedback to users about the form submission status.
- Updated the access control logic to clarify that users can access payment details if they are either the owner or an authorized payer. - Improved comments for better understanding of the authorization flow, ensuring clearer code documentation.
- Extracted the form submission logic into a separate function, `submitAfterApproval`, to improve code readability and maintainability. - Updated the effect handling payment approval to utilize the new submission function, ensuring a cleaner flow and reducing code duplication. - Minor adjustments to the formatting of the `PaymentDetailsStatus` component for consistency.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/invoice-form.tsx (2)
58-80: Type casting could be made safer.The function
filterValidPaymentDetailsincludes a type cast at line 79 withas unknown as LinkedPaymentDetail[], which bypasses TypeScript's type checking and could potentially lead to runtime errors.- return validPaymentDetails as unknown as LinkedPaymentDetail[]; + // Explicitly map to ensure type safety + return validPaymentDetails.map(detail => ({ + paymentDetails: detail.paymentDetails, + paymentDetailsPayers: detail.paymentDetailsPayers + }));
475-516: Comprehensive bank account linking flow with proper error handling.The bank account success handler correctly:
- Links the payment method to the client
- Updates the form with the new payment details
- Refetches data to update the UI
- Provides user feedback through toast notifications
The error handling could be slightly improved with more specific error messages.
} catch (error) { - console.error("Error linking payment method:", error); + console.error("Error linking payment method to client:", error); toast.error("Failed to link payment method to client"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
src/components/invoice-form.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/components/invoice-form.tsx (2)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:316-319
Timestamp: 2025-05-19T13:00:48.790Z
Learning: The handleFormSubmit function in src/components/invoice-form.tsx correctly uses data.clientEmail from the form submission data to find matching payers, which is the proper implementation.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:451-453
Timestamp: 2025-05-20T12:59:44.665Z
Learning: In the Easy Invoice project, setTimeout is required when submitting a form after modal state changes in the crypto-to-fiat payment flow. Directly calling handleFormSubmit without setTimeout after closing modals and updating state causes issues.
🔇 Additional comments (13)
src/components/invoice-form.tsx (13)
46-49: Well-structured constants for configuration.The constants are clearly named and well-documented, making the code more maintainable. As suggested by a previous review comment, consider moving these to environment variables or a centralized config file if these values might need adjustment in different environments.
52-55: Good type definition for LinkedPaymentDetail.The type is properly structured to represent payment details with their associated payers, which improves code readability and type safety throughout the component.
113-125: Good defensive coding in PaymentDetailsStatus component.The component correctly handles potentially null status values with a ternary operator at line 123, which prevents runtime errors when the status is missing.
210-271: Well-structured PaymentDetailsSection with clear error states.The component properly handles different states (loading, error, empty) and incorporates meaningful error messages for better user experience. The conditional rendering logic is clean and easy to follow.
293-358: Comprehensive form submission handler with proper validation.The
handleFormSubmitfunction correctly handles:
- Multiple submission prevention
- Required payment details validation
- Payment details approval status checks
- Error handling with user-friendly messages
Keeping
isSubmittingtrue during approval waiting is a good approach to prevent duplicate submissions.
361-374: Modal timeout with proper cleanup.The timeout effect for the pending approval modal includes proper cleanup in the return function to prevent memory leaks, which is a best practice for React effects.
385-402: Data fetching with proper query dependencies.The TRPC queries are correctly configured with:
- Appropriate
enabledconditions to prevent unnecessary requests- Configurable polling intervals
- Window focus refetching for improved user experience
430-437: Extracted submission logic for better reusability.The
submitAfterApprovalfunction properly extracts the form submission logic, making it reusable across different parts of the component. The timeout is necessary based on the learnings from previous feedback.
440-473: Separate effect for monitoring payment approval status.This effect is focused on a single responsibility: monitoring payment approval status and triggering form submission when approved. It also handles the edge case where waiting is no longer needed but submission state is still active.
879-900: Clear toggle for crypto-to-fiat payments with field management.The checkbox implementation for enabling/disabling crypto-to-fiat payments correctly manages the form state:
- Updates the toggle state
- Clears irrelevant fields when toggling
- Clears any validation errors on those fields
This ensures that the form remains in a valid state after toggling the payment method.
902-940: Comprehensive payment method selection UI with proper compliance checks.The payment method selection UI correctly:
- Shows different states based on client compliance status
- Provides an intuitive interface for selecting payment methods
- Shows appropriate status indicators for each payment method
- Allows adding new payment methods when needed
This creates a seamless user experience while enforcing compliance requirements.
942-955: Conditional wallet address input with proper validation.When crypto-to-fiat is disabled, the wallet address input is correctly shown with appropriate validation error handling.
957-969: Informative submit button with proper state handling.The submit button correctly:
- Shows different labels based on the form state (creating, created, etc.)
- Disables during submission to prevent multiple submissions
- Uses appropriate styling for good visibility
This provides clear feedback to users about the form submission status.
- Introduced a constant for the compliance status polling interval, set to 30 seconds, to enhance configurability. - Updated the data fetching logic to use the new polling interval and enabled refetching on window focus, improving user experience and data accuracy.
…ling - Updated the `filterValidPaymentDetails` function to filter payment details based solely on compliance status, removing the client email dependency for improved clarity. - Refactored the `getPaymentDetails` procedure in `complianceRouter` to retrieve all payment details owned by a user, regardless of payer association, enhancing data retrieval logic. - Adjusted the invoice retrieval logic to find payment details based on the payer's email, ensuring accurate association with the client. - Introduced a new `handleBankAccountSuccess` function in `InvoiceForm` to streamline the process of linking payment methods and updating the UI accordingly.
- Introduced new SQL types for account, agreement, beneficiary, gender, KYC, payment details, and request statuses to enhance data integrity. - Created tables for `easyinvoice_payment_details` and `easyinvoice_payment_details_payers` to manage payment information effectively. - Updated the `easyinvoice_request` and `easyinvoice_user` tables to include new columns for request tracking and user compliance status. - Added foreign key constraints to ensure referential integrity between payment details and user records.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/server/routers/invoice.ts (1)
313-339: 🛠️ Refactor suggestionMissing authorization check in stopRecurrence.
The
stopRecurrenceprocedure is now protected, but it doesn't verify that the authenticated user has the right to stop the recurrence (e.g., is the creator of the invoice or the designated payer).const { requestId } = input; + // Get the request to check authorization + const invoiceRecord = await ctx.db.query.requestTable.findFirst({ + where: eq(requestTable.requestId, requestId), + }); + + if (!invoiceRecord) { + throw new TRPCError({ + code: "NOT_FOUND", + message: "Invoice not found", + }); + } + + // Ensure the user is authorized to stop the recurrence + if (invoiceRecord.userId !== ctx.user.id && invoiceRecord.invoicedTo !== ctx.user.id) { + throw new TRPCError({ + code: "FORBIDDEN", + message: "You are not authorized to stop this recurrence", + }); + } const request = await apiClient.patch( `/v2/request/${requestId}/stop-recurrence`, );
🧹 Nitpick comments (2)
src/server/routers/compliance.ts (1)
528-533: Redundant authentication check.Since this is a protected procedure, the
ctx.usercheck is redundant as the middleware already ensures user authentication. Consider removing this check to simplify the code.-// Since this is a protected procedure, ctx.user should exist, but we'll check to satisfy TypeScript -if (!ctx.user) { - throw new TRPCError({ - code: "UNAUTHORIZED", - message: "You must be logged in to access payment details", - }); -}src/components/invoice-form.tsx (1)
58-71: Type casting in filterValidPaymentDetails could be improved.The function uses
as unknown as LinkedPaymentDetail[]which bypasses TypeScript's type checking. Consider using proper type transformations instead.- return validPaymentDetails as unknown as LinkedPaymentDetail[]; + return validPaymentDetails.map(detail => ({ + paymentDetails: detail.paymentDetails, + paymentDetailsPayers: detail.paymentDetailsPayers + }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (4)
src/components/compliance-form.tsx(1 hunks)src/components/invoice-form.tsx(2 hunks)src/server/routers/compliance.ts(1 hunks)src/server/routers/invoice.ts(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/compliance-form.tsx
🧰 Additional context used
🧠 Learnings (3)
src/components/invoice-form.tsx (2)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:316-319
Timestamp: 2025-05-19T13:00:48.790Z
Learning: The handleFormSubmit function in src/components/invoice-form.tsx correctly uses data.clientEmail from the form submission data to find matching payers, which is the proper implementation.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/components/invoice-form.tsx:451-453
Timestamp: 2025-05-20T12:59:44.665Z
Learning: In the Easy Invoice project, setTimeout is required when submitting a form after modal state changes in the crypto-to-fiat payment flow. Directly calling handleFormSubmit without setTimeout after closing modals and updating state causes issues.
src/server/routers/compliance.ts (3)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic, payees (users who created invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers. The payee is identified by the userId field in the requestTable.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic, payees (creators of invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic for crypto-to-fiat payments, payees (users who created invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers. To verify if a user is a payee, check if their user ID matches the userId field in any request that references these payment details.
src/server/routers/invoice.ts (3)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic for crypto-to-fiat payments, payees (users who created invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers. To verify if a user is a payee, check if their user ID matches the userId field in any request that references these payment details.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic, payees (creators of invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers.
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:88-109
Timestamp: 2025-02-12T12:40:14.742Z
Learning: The payRequest endpoint in src/server/routers/invoice.ts is intentionally kept public (using publicProcedure) to allow invoice sharing and payment by anyone with the payment reference, similar to how payment links work in other payment systems.
🔇 Additional comments (26)
src/server/routers/compliance.ts (9)
26-27: Type safety improvement for additional API properties.Using
[key: string]: unknowninstead ofanyis a more type-safe approach for handling additional properties from the API. This prevents accidental access to non-existent properties and forces proper type checking.
39-99: Well structured error handling for submission API calls.The error handling in
submitComplianceInfoproperly maps HTTP status codes to appropriate TRPC error codes with custom messages. This approach provides more useful feedback to clients and makes debugging easier.
110-112: Proper URL encoding to prevent path injection.Good use of
encodeURIComponentto encode the clientUserId in the API path. This prevents potential path injection vulnerabilities when user-provided data is included in URL paths.
171-184: Complete default response for 404 error case.The default response returned when a user doesn't exist in the compliance system includes all expected fields with null values. This ensures type consistency for the frontend and prevents potential undefined access errors.
257-264: Extraction of filtering logic into a utility function.Using the
filterDefinedValuesutility to remove undefined/null values before API submission improves code maintainability and reduces duplication. This is a good refactor to ensure consistency across the codebase.
275-306: Comprehensive database error handling.The error handling for database operations appropriately distinguishes between different types of errors (validation, constraint, duplicate key) and maps them to user-friendly messages. This improves the developer and end-user experience.
327-339: Two-step authorization check for payment details access.Properly checking both user existence and compliance status with separate error messages provides clarity on why an operation might fail. The NOT_FOUND and FORBIDDEN error codes are appropriately used based on the error condition.
486-487: Defensive spread operator usage to prevent runtime errors.Using
...(row.user ?? {})prevents runtime errors when spreading potentially undefined values. This is a good defensive programming practice that ensures the code remains robust even when data is incomplete.
577-613: Security-focused field selection in getUserByEmail.Explicitly selecting only the necessary fields (
id,name,isCompliant) ingetUserByEmailenhances security by limiting the exposure of potentially sensitive user data.src/components/invoice-form.tsx (11)
47-49: Configuration variables defined as constants.Good practice to define configuration values like polling intervals and timeouts as named constants at the top of the file. This makes them easy to find and adjust if needed.
52-55: Clear type definition for payment details structure.The
LinkedPaymentDetailtype provides a clear interface for the complex nested data structure used throughout the component. This improves code readability and type safety.
104-116: Null-safe status formatting with defensive check.Good defensive programming in the
PaymentDetailsStatuscomponent by checking ifstatusis truthy before calling string methods. This prevents potential runtime errors if status is null or undefined.
201-262: Well-structured component composition for different UI states.The
PaymentDetailsSectioncomponent effectively encapsulates the complex conditional rendering logic for payment details, making the main component much cleaner. Each condition returns an appropriate UI component based on the current state.
291-309: Efficient data fetching with controlled refetching.The use of
refetchIntervalandrefetchOnWindowFocusin the payment details query ensures timely updates without unnecessary API calls. Theenabledcondition prevents fetching until the required data is available.
366-375: Form validation for crypto-to-fiat payment details.The form submission properly validates that payment details are selected when crypto-to-fiat is enabled, setting appropriate form errors and preventing submission until this requirement is met.
392-394: Keeping isSubmitting true during approval wait prevents multiple submissions.Good improvement to keep
isSubmittingtrue while waiting for payment approval, preventing multiple submissions if the user tries to submit again while waiting.
442-456: Timeout handling for pending approval modal.The timeout effect for the pending approval modal provides a good user experience by automatically closing the modal and showing an error message if approval takes too long. The cleanup function properly clears the timeout to prevent memory leaks.
889-910: Clear labeling for crypto-to-fiat functionality.The checkbox label clearly describes what "Crypto-to-fiat" means to users: "Allow payment to your bank account (Crypto-to-fiat payment)". This helps users understand the feature without technical knowledge.
952-965: Conditionally showing wallet address input when crypto-to-fiat is disabled.The form intelligently shows either payment details selection or wallet address input based on whether crypto-to-fiat is enabled, providing a clean user experience for both payment flows.
482-490:Details
✅ Verification successful
Submit after approval with necessary timeout.
Based on the retrieved learnings, the setTimeout in
submitAfterApprovalis necessary when submitting a form after modal state changes. This ensures that React state is fully updated before form submission.
🌐 Web query:
Why do you need setTimeout for React state updates before form submission?💡 Result:
In React,
setState(or the state updater function fromuseState) is asynchronous. This means that when you call a state update function, the state doesn't update immediately but schedules an update for the next render cycle.When it comes to form submission, if you update some state right before submitting—in the same event handler—and then immediately use the state value (e.g., to send data to a server), the updated state might not be reflected yet. The form submission handler will see the old state value because React batches state updates and doesn't flush them synchronously.
Using
setTimeoutwith a delay of 0 (or a short delay) is a common workaround to allow the state update to complete and the component to re-render before the form submission logic runs. Essentially,setTimeoutbreaks the execution context so that the state update can take effect, and then the form submission logic can access the latest state.Example:
const [inputValue, setInputValue] = React.useState(''); const handleSubmit = (event) => { event.preventDefault(); setInputValue('new value'); // Without setTimeout, this will still log the old value setTimeout(() => { console.log(inputValue); // Logs 'new value' // proceed with form submission using updated inputValue }, 0); };Why this happens:
- React batches multiple state updates to optimize performance.
- State updates are asynchronous to avoid blocking UI.
- Directly reading state after
setStatein the same tick won't reflect the new value.Alternative approaches:
- Use the state update callback form (if using class components).
- Use a controlled form approach where the latest form data is directly available from inputs rather than state.
- Use the
useEffecthook to react to state changes and trigger side effects.So,
setTimeoutis a practical workaround in some cases but not the only way to ensure you're working with the latest state before form submission.
Ensure form submission waits for modal state update
Per React’s asynchronous state updates, the
setTimeoutdelay allows the modal to fully close and state to settle before callinghandleFormSubmit, preventing submission with stale values.• Location:
src/components/invoice-form.tsx:482-490const submitAfterApproval = useCallback(() => { toast.success("Payment method approved! Creating invoice..."); setTimeout(() => { void handleFormSubmit(form.getValues()); }, 100); }, [form, handleFormSubmit]);src/server/routers/invoice.ts (6)
73-74: Storing crypto-to-fiat payment details in the database.Adding
isCryptoToFiatAvailableandpaymentDetailsIdto the database record ensures that the crypto-to-fiat payment flow can be properly tracked and managed throughout the invoice lifecycle.
78-83: Improved error handling with specific TRPC errors.Throwing a TRPC error with a specific code instead of returning a generic object with success: false provides better error handling and allows clients to properly respond to different error scenarios.
109-114: Essential client user verification.Adding a check to ensure the client user exists before creating an invoice is crucial for the crypto-to-fiat flow, as it relies on the client being a registered user who can complete compliance requirements.
127-138: Enhanced error logging with context details.The error logging now includes useful context such as the user ID, client email, and invoice number, making it easier to diagnose issues in production. Throwing a TRPC error with a specific code also improves client-side error handling.
307-307: Improved security by using protectedProcedure for stopRecurrence.Changing
stopRecurrencefrompublicProceduretoprotectedProcedureenhances security by ensuring only authenticated users can stop recurring payments.
21-29:Details
✅ Verification successful
Clear documentation for crypto-to-fiat payee address handling.
The FIXME comment clearly explains why a hardcoded payee address is used for crypto-to-fiat payments and when this logic can be removed. This helps future developers understand the implementation decision.
🏁 Script executed:
#!/bin/bash # Check if the CRYPTO_TO_FIAT_PAYEE_ADDRESS environment variable is defined in .env.example grep -r "CRYPTO_TO_FIAT_PAYEE_ADDRESS" --include=".env*" .Length of output: 105
Verified CRYPTO_TO_FIAT_PAYEE_ADDRESS environment variable
.env.exampleincludes aCRYPTO_TO_FIAT_PAYEE_ADDRESSplaceholder.- The existing FIXME comment clearly explains the temporary payee override and removal condition once the Request Network API handles crypto-to-fiat.
No further changes required.
- Updated the logic for fetching payment details in the PaymentPage component to handle cases where the paymentDetailsId is not present, returning null instead of an empty string. - Cleaned up the PaymentSection component by removing unnecessary whitespace. - Enhanced error handling in the invoice creation process by throwing a TRPCError for non-200 responses, ensuring better feedback on invoice creation failures. - Refactored the invoice retrieval logic to ensure proper handling of payment details, including checks for the existence of payers and improved messaging for missing payment details.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
drizzle/0005_fine_joystick.sql (2)
9-16: New table for payment details payersThis table enables the many-to-many relationship between payment details and payers, with appropriate status tracking. Using
payment_details_status(snake_case) while other fields use camelCase creates a naming inconsistency.Consider standardizing on either snake_case or camelCase for all column names in future migrations to maintain consistency.
18-53: New payment details table with comprehensive fieldsThe
easyinvoice_payment_detailstable contains extensive banking information fields to support various payment methods and regions. However, many fields are nullable, suggesting it handles multiple banking systems.Consider evaluating if this denormalized approach with many nullable fields is optimal for your use case, or if a more structured approach with separate tables for different banking systems might be more maintainable in the long run.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (5)
drizzle/0005_fine_joystick.sql(1 hunks)drizzle/meta/0005_snapshot.json(1 hunks)drizzle/meta/_journal.json(1 hunks)src/app/invoices/[ID]/page.tsx(4 hunks)src/server/routers/invoice.ts(13 hunks)
✅ Files skipped from review due to trivial changes (2)
- drizzle/meta/_journal.json
- drizzle/meta/0005_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/invoices/[ID]/page.tsx
🧰 Additional context used
🧠 Learnings (1)
src/server/routers/invoice.ts (3)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic for crypto-to-fiat payments, payees (users who created invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers. To verify if a user is a payee, check if their user ID matches the userId field in any request that references these payment details.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic, payees (creators of invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers.
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:88-109
Timestamp: 2025-02-12T12:40:14.742Z
Learning: The payRequest endpoint in src/server/routers/invoice.ts is intentionally kept public (using publicProcedure) to allow invoice sharing and payment by anyone with the payment reference, similar to how payment links work in other payment systems.
🔇 Additional comments (17)
src/server/routers/invoice.ts (14)
34-40: API endpoint version upgrade and crypto-to-fiat supportThe code now correctly uses the v2 API endpoints and passes the new
isCryptoToFiatAvailableflag.
47-53: Improved error handling with explicit error detailsThe error handling has been improved by throwing specific TRPCErrors with appropriate error codes and messages instead of returning generic error objects.
67-68: New request identifier usageThe code now uses
requestIdfrom the API response to store in the database, improving identifier consistency across the application.
89-94: Enhanced error handling when creating invoicesThis properly throws an explicit error when the database operation fails instead of returning a possibly unexpected null value.
132-137: Improved error logging with context detailsThe error logging now includes important context information (userId, clientEmail, invoiceNumber) which will make debugging issues much easier.
248-254: Input schema updated from paymentReference to requestIdThe code now consistently uses
requestIdinstead ofpaymentReferencethroughout the codebase, which improves consistency.
259-272: Enhanced invoice fetching with payment detailsThe code now properly retrieves payment details and their associated payers along with the invoice, which is necessary for crypto-to-fiat payments.
279-295: Improved null checking for payment detailsThe code now properly checks for null payment details and empty payers array, preventing potential runtime errors.
317-317: Security improvement: Stop recurrence now properly protectedThe
stopRecurrenceendpoint has been changed frompublicProceduretoprotectedProcedure, ensuring only authenticated users can stop recurring payments. This addresses a security concern from previous reviews.
320-328: Updated stop recurrence endpoint to use requestIdThe code now consistently uses
requestIdfor API calls and database queries instead of mixingpaymentReferenceand other identifiers.
353-372: Updated payment routes endpoint to use V2 APIThe
getPaymentRoutesendpoint now correctly uses the V2 API withrequestIdparameter.
393-396: Updated payment intent endpoint to use V2 APIThe
sendPaymentIntentendpoint now correctly uses the V2 API path.
296-296:⚠️ Potential issuePotential undefined variable usage
The
paymentDetailsPayersvariable is declared but may not be assigned ifinvoice.paymentDetailsis falsy. Then it's used in the template string, potentially causing undefined access.- let paymentEndpoint = `/v2/request/${invoice.requestId}/pay?wallet=${input.wallet}&clientUserId=${invoice.clientEmail}${paymentDetailsPayers ? `&paymentDetailsId=${paymentDetailsPayers?.externalPaymentDetailId}` : ""}`; + let paymentEndpoint = `/v2/request/${invoice.requestId}/pay?wallet=${input.wallet}&clientUserId=${invoice.clientEmail}`; + if (invoice.paymentDetails && paymentDetailsPayers) { + paymentEndpoint += `&paymentDetailsId=${paymentDetailsPayers.externalPaymentDetailId}`; + }Likely an incorrect or invalid review comment.
84-85:Details
❓ Verification inconclusive
Database schema extensions for crypto-to-fiat
These new fields properly track crypto-to-fiat availability and payment details association for invoices.
A previous review comment mentioned concerns about payees creating requests with payment details they don't own. Let's verify that there are checks elsewhere in the codebase to prevent this:
🏁 Script executed:
#!/bin/bash # Look for checks verifying a user owns payment details before connecting them to a request rg -A 3 "paymentDetailsId.*user" --glob "*.ts"Length of output: 48
#!/bin/bash
Display the invoice router’s create/update procedures to inspect paymentDetailsId handling
echo "----- invoice.ts: protectedProcedure usages -----"
rg -n -C 3 "protectedProcedure" src/server/routers/invoice.tsecho
echo "----- invoice.ts: paymentDetailsId references -----"
rg -n -C 5 "paymentDetailsId" src/server/routers/invoice.ts
#!/bin/bash
Inspect how paymentDetailsId is used in invoice creation and update
echo "===== invoice.ts: create invoice block ====="
rg -n -C 10 "protectedProcedure.*create" src/server/routers/invoice.tsecho
echo "===== invoice.ts: update invoice block ====="
rg -n -C 10 "protectedProcedure.*update" src/server/routers/invoice.tsecho
echo "===== invoice.ts: any direct paymentDetails lookup ====="
rg -n -C 5 "ctx.prisma.paymentDetails" src/server/routers/invoice.ts
Verify paymentDetailsId ownership before linking
I didn’t find any validation ensuring that the
paymentDetailsIdprovided ininputactually belongs to the authenticated user before creating or updating an invoice. Without this, a user could link someone else’s payment details.• Check in
src/server/routers/invoice.ts(create/update procedures) that you filter thepaymentDetailsrecord by both itsidand the currentctx.session.user.idbefore connecting it.
• If this check already exists, please point me to its implementation; otherwise, add it to prevent unauthorized associations.drizzle/0005_fine_joystick.sql (3)
1-8: Comprehensive enum types for payment workflowsThe new enum types provide a strong foundation for the crypto-to-fiat payment workflow and compliance tracking. The
request_statusenum now includes additional statuses to track the crypto-to-fiat payment lifecycle.
62-84: Foreign key constraints with appropriate delete behaviorsThe foreign key constraints maintain referential integrity with appropriate cascade or restrict behaviors. The
ON DELETE restrictfor payment details-user relation prevents deleting users with payment details, andON DELETE cascadefor payers ensures clean deletion of related records. TheON DELETE set nullfor invoice payment details references allows deleting payment details without affecting invoices.
55-61:Details
✅ Verification successful
Column changes for crypto-to-fiat support
The changes to existing tables properly support the new crypto-to-fiat functionality with appropriate flags and relations. The default values for compliance fields (all defaulting to 'pending' or false) imply all users need to go through compliance checks.
Verify if all users need compliance status tracking or only those involved in crypto-to-fiat payments:
🏁 Script executed:
#!/bin/bash # Check for conditional compliance logic based on payment type rg "isCryptoToFiatAvailable.*compliance|compliance.*isCryptoToFiatAvailable" --glob "*.ts"Length of output: 92
🏁 Script executed:
#!/bin/bash # Search for usage of isCryptoToFiatAvailable across common code files rg "isCryptoToFiatAvailable" --glob "*.ts" --glob "*.tsx" --glob "*.js" --glob "*.jsx" -n -C5 # Search for compliance fields to see if they’re conditionally referenced rg "isCompliant" --glob "*.ts" --glob "*.tsx" --glob "*.js" --glob "*.jsx" -n -C5 rg "agreement_status" --glob "*.ts" --glob "*.tsx" --glob "*.js" --glob "*.jsx" -n -C5 rg "kyc_status" --glob "*.ts" --glob "*.tsx" --glob "*.js" --glob "*.jsx" -n -C5Length of output: 26390
Compliance enforcement is scoped to crypto-to-fiat flows
The migration addsagreement_status,kyc_status, andisCompliantcolumns (all defaulting to “pending”/false) for every user, but compliance checks only occur whenisCryptoToFiatAvailableis true. Non-crypto-to-fiat paths continue to usewalletAddresswithout referencing compliance.• Migration (drizzle/0005_fine_joystick.sql): adds compliance columns on
easyinvoice_user.
• DB schema (src/server/db/schema.ts): defines defaults foragreementStatus,kycStatus,isCompliant.
• Invoice form (src/components/invoice-form.tsx) & schema (src/lib/schemas/invoice.ts): enforce compliance/payment details only ifisCryptoToFiatAvailable === true.
• Invoice router (src/server/routers/invoice.ts): includesisCryptoToFiatAvailableandpaymentDetailsIdbut doesn’t gate non-crypto flows.No further changes required.
- Updated the logic for determining the payee address in the invoice creation process to include error handling for missing environment variables. - Added a console error and a TRPCError throw to ensure proper feedback when the CRYPTO_TO_FIAT_PAYEE_ADDRESS is not set, improving server configuration reliability.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (1)
src/server/routers/invoice.ts(13 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/server/routers/invoice.ts (3)
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic for crypto-to-fiat payments, payees (users who created invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers. To verify if a user is a payee, check if their user ID matches the userId field in any request that references these payment details.
Learnt from: rodrigopavezi
PR: RequestNetwork/easy-invoice#45
File: src/server/routers/compliance.ts:441-484
Timestamp: 2025-05-19T08:47:38.063Z
Learning: In the payment details authorization logic, payees (creators of invoices/requests) should be allowed to access the payment details associated with their payers, in addition to the payment details owners and authorized payers.
Learnt from: aimensahnoun
PR: RequestNetwork/easy-invoice#2
File: src/server/routers/invoice.ts:88-109
Timestamp: 2025-02-12T12:40:14.742Z
Learning: The payRequest endpoint in src/server/routers/invoice.ts is intentionally kept public (using publicProcedure) to allow invoice sharing and payment by anyone with the payment reference, similar to how payment links work in other payment systems.
🧬 Code Graph Analysis (1)
src/server/routers/invoice.ts (4)
src/lib/axios.ts (1)
apiClient(3-8)src/server/db/index.ts (1)
db(10-12)src/server/db/schema.ts (2)
requestTable(151-188)PaymentDetailsPayers(278-280)src/server/trpc.ts (1)
protectedProcedure(39-39)
🔇 Additional comments (13)
src/server/routers/invoice.ts (13)
25-42: Well-handled environment variable check and crypto-to-fiat payee address.The added FIXME comment clearly explains the rationale for using a Request Network Foundation address for crypto-to-fiat transactions. The code properly handles the case where the environment variable is missing with a detailed error message. This prevents silent failures and ensures proper configuration.
44-56: API endpoint update to v2 is consistent throughout the file.The API endpoint has been properly updated from v1 to v2, and the new
isCryptoToFiatAvailableparameter is now included in the request payload.
58-63: Improved error handling with proper status code checking.This adds better error handling by checking the response status and throwing a specific TRPCError with the BAD_REQUEST code and a detailed message that includes the server's error response.
94-96: Database schema update to support crypto-to-fiat functionality.The database insert now includes the new fields
isCryptoToFiatAvailableandpaymentDetailsId, properly integrating with the schema changes shown in the context.
99-104: Good error handling for database operations.The code now properly checks if the insert operation succeeded and throws a specific error if it failed, improving error handling and debugging.
142-151: Enhanced error logging with structured context.The error logging now includes more context (userId, clientEmail, invoiceNumber) and structured information about the error, which will make debugging easier. The throw statement with TRPCError is more appropriate than returning a generic error object.
192-204: Consistent error handling approach across procedures.Similar to the improvements in the
createprocedure, thecreateFromInvoiceMeprocedure now has enhanced error logging with relevant context and properly throws a TRPCError.
269-283: Database query includes payment details and payers.The updated query now fetches related payment details and their payers, which is necessary for the crypto-to-fiat payment flow.
327-327: Security improvement: Change from public to protected procedure.The
stopRecurrenceprocedure has been changed frompublicProceduretoprotectedProcedure, which is a security improvement as it now requires user authentication.
330-330: Consistent use of requestId throughout the codebase.The code has been updated to consistently use
requestIdinstead ofpaymentReferencein input schemas and API calls, which improves clarity and consistency.Also applies to: 334-334, 337-337, 351-351
363-392: Updated getPaymentRoutes with v2 API endpoint.The
getPaymentRoutesquery has been properly updated to use the v2 API endpoint andrequestIdinstead ofpaymentReference.
404-404: Updated sendPaymentIntent with v2 API endpoint.The
sendPaymentIntentmutation has been properly updated to use the v2 API endpoint.
306-306:Details
❌ Incorrect review comment
Potential undefined reference in URL construction.
The URL construction uses
paymentDetailsPayers?.externalPaymentDetailId, but there's no guarantee that this property exists on thePaymentDetailsPayerstype based on the code provided.Ensure the correct property name is used. If
externalPaymentDetailIdis not a property ofPaymentDetailsPayers, use the correct property name:- let paymentEndpoint = `/v2/request/${invoice.requestId}/pay?wallet=${input.wallet}&clientUserId=${invoice.clientEmail}${paymentDetailsPayers ? `&paymentDetailsId=${paymentDetailsPayers?.externalPaymentDetailId}` : ""}`; + let paymentEndpoint = `/v2/request/${invoice.requestId}/pay?wallet=${input.wallet}&clientUserId=${invoice.clientEmail}${paymentDetailsPayers ? `&paymentDetailsId=${invoice.paymentDetailsId}` : ""}`;
🏁 Script executed:
#!/bin/bash # Check the PaymentDetailsPayers type definition to verify the property name grep -r "type PaymentDetailsPayers" --include="*.ts" . grep -r "paymentDetailsPayersTable" --include="*.ts" .Length of output: 1760
Remove incorrect undefined reference comment
The
externalPaymentDetailIdcolumn is defined onpaymentDetailsPayersTableinsrc/server/db/schema.ts, so it’s guaranteed on thePaymentDetailsPayerstype. The existing optional chaining is safe—no change needed.Likely an incorrect or invalid review comment.
- Renamed the variable `paymentDetails` to `paymentDetailsPayers` for better clarity in the invoice router, enhancing code readability and understanding of its purpose. - Updated the conditional check to reflect the new variable name, ensuring consistent error messaging for missing payment details.
Problem
The application currently only supports pure cryptocurrency payments, which limits user options and adoption. Users who want to receive payments in fiat currency after a crypto transaction must handle the conversion process separately, creating friction in the payment workflow. Additionally, there's no built-in compliance mechanism to ensure regulatory requirements are met for crypto-to-fiat conversions, exposing users to potential legal risks.
Changes
Fixes #40
Diagrams
graph TD subgraph Database Changes A[Request Table] --> B[New Fields] B --> C[cryptoToFiatAvailable] B --> D[paymentDetailsId] B --> E[recurrence] end subgraph API Changes F[Invoice Creation] --> G[Compliance Check] G --> H[Payment Details Validation] H --> I[Request Creation] end subgraph Webhook Handling J[Payment Events] --> K[Status Updates] K --> L[crypto_paid] K --> M[offramp_initiated] K --> N[offramp_pending] K --> O[offramp_failed] K --> P[paid] end subgraph UI Components Q[Invoice Form] --> R[Crypto to Fiat Toggle] R --> S[Payment Details Selection] S --> T[Bank Account Modal] U[Crypto to Fiat Page] --> V[Compliance Form] end style A fill:#f9f,stroke:#333,stroke-width:2px style F fill:#bbf,stroke:#333,stroke-width:2px style J fill:#dfd,stroke:#333,stroke-width:2px style Q fill:#ffd,stroke:#333,stroke-width:2pxCrypto to Fiat Implementation Changes
This diagram illustrates the changes introduced in the
feat/crypto-to-fiatbranch:Database Changes
cryptoToFiatAvailableflag to request tablepaymentDetailsIdfor bank account linkingrecurrencesupport for recurring paymentsAPI Changes
Webhook Handling
crypto_paid: Initial crypto payment receivedofframp_initiated: Fiat conversion startedofframp_pending: Conversion in progressofframp_failed: Conversion failedpaid: Final fiat payment completedUI Components
Technical Implementation Details
Compliance Integration
Payment Flow
Security Features
User Experience
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores