feat: use webhook conversion info for all payments and requests #163
Conversation
…helpers to get USD amount for requests, subscription payments and recurring payments
WalkthroughThreads ConversionInfo through webhook flows, introduces typed ClientPaymentBody and RecurringPaymentInstallment, adds conversion utilities to consolidate USD values, removes MultiCurrencyStatCard and legacy currency helpers, updates dashboard components to display consolidated USD totals with non-USD indicators, and adds DB migration for conversionInfo fields. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Webhook POST
participant Conv as getConversionInfo()
participant Builder as getClientPaymentBody()
participant Logic as addClientPayment / addPaymentToRecurringPayment
participant DB as Database
Client->>Conv: parse conversionInfo from webhookBody
Conv-->>Client: ConversionInfo | null
alt client payment
Client->>Builder: getClientPaymentBody(webhookBody, conversionInfo)
Builder-->>Client: ClientPaymentBody
Client->>Logic: addClientPayment(ClientPaymentBody, clientId)
Logic->>DB: insert client payment (with conversionInfo)
DB-->>Logic: ok
end
alt recurring payment
Client->>Logic: addPaymentToRecurringPayment(externalPaymentId, RecurringPaymentInstallment{..., conversionInfo})
Logic->>DB: append installment (typed)
DB-->>Logic: ok
end
Client->>DB: updateRequest(requestId, { ...requestData })
DB-->>Client: updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🔇 Additional comments (3)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/routers/subscription-plan.ts (1)
185-203: Coalesce conversionInfo to null to match the type contractGuard against undefined coming from DB/joins.
- conversionInfo: payment.conversionInfo, + conversionInfo: payment.conversionInfo ?? null,
🧹 Nitpick comments (1)
src/lib/types/index.ts (1)
57-64: Add a shared schema for ConversionInfoConsider a Zod schema (server-side) for ConversionInfo to validate webhook payloads/DB reads and keep the shape consistent across boundaries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/app/api/webhook/route.ts(8 hunks)src/components/dashboard/invoices-received.tsx(3 hunks)src/components/dashboard/invoices-sent.tsx(3 hunks)src/components/dashboard/subscriptions.tsx(3 hunks)src/components/multi-currency-stat-card.tsx(0 hunks)src/components/subscription-plans/blocks/payments-table.tsx(5 hunks)src/components/subscription-plans/blocks/subscribers-table.tsx(6 hunks)src/components/view-recurring-payments/blocks/completed-payments.tsx(1 hunks)src/lib/helpers/conversion.ts(1 hunks)src/lib/helpers/currency.ts(0 hunks)src/lib/types/index.ts(1 hunks)src/server/db/schema.ts(5 hunks)src/server/routers/subscription-plan.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/lib/helpers/currency.ts
- src/components/multi-currency-stat-card.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
src/components/subscription-plans/blocks/payments-table.tsx (2)
src/components/stat-card.tsx (1)
StatCard(9-21)src/lib/helpers/conversion.ts (1)
consolidateSubscriptionPaymentUsdValues(82-90)
src/server/db/schema.ts (1)
src/lib/types/index.ts (1)
ConversionInfo(57-64)
src/components/dashboard/invoices-sent.tsx (2)
src/lib/helpers/conversion.ts (1)
consolidateRequestUsdValues(74-80)src/components/stat-card.tsx (1)
StatCard(9-21)
src/lib/types/index.ts (1)
src/server/routers/ecommerce.ts (1)
ecommerceRouter(14-182)
src/lib/helpers/conversion.ts (2)
src/lib/types/index.ts (2)
ConversionInfo(57-64)SubscriptionPayment(37-51)src/server/db/schema.ts (2)
Request(486-486)RecurringPaymentInstallment(88-93)
src/components/subscription-plans/blocks/subscribers-table.tsx (3)
src/lib/types/index.ts (1)
SubscriptionWithDetails(29-35)src/components/stat-card.tsx (1)
StatCard(9-21)src/lib/helpers/conversion.ts (1)
consolidateRecurringPaymentUsdValues(92-106)
src/components/dashboard/invoices-received.tsx (2)
src/lib/helpers/conversion.ts (1)
consolidateRequestUsdValues(74-80)src/components/stat-card.tsx (1)
StatCard(9-21)
src/components/view-recurring-payments/blocks/completed-payments.tsx (1)
src/server/db/schema.ts (1)
RecurringPaymentInstallment(88-93)
src/components/dashboard/subscriptions.tsx (3)
src/lib/types/index.ts (1)
SubscriptionWithDetails(29-35)src/lib/helpers/conversion.ts (1)
consolidateRecurringPaymentUsdValues(92-106)src/components/stat-card.tsx (1)
StatCard(9-21)
src/app/api/webhook/route.ts (2)
src/server/db/schema.ts (4)
ClientPayment(497-497)ecommerceClientTable(305-331)requestTable(188-226)RecurringPaymentInstallment(88-93)src/lib/types/index.ts (1)
ConversionInfo(57-64)
🔇 Additional comments (4)
src/lib/types/index.ts (1)
50-51: Contract: prefer null over undefined for conversionInfoGood addition. Ensure all mappers/serializers coalesce to null when no data so the shape matches the declared type.
src/components/subscription-plans/blocks/subscribers-table.tsx (2)
183-200: USD consolidation per subscription — LGTMAggregation via consolidateRecurringPaymentUsdValues over active subscribers looks correct.
264-265: EmptyState icon change — LGTMThe switch to Users fits the context.
src/components/subscription-plans/blocks/payments-table.tsx (1)
151-153: USD consolidation on filtered set — LGTMUsing consolidateSubscriptionPaymentUsdValues over the filtered list is correct.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/components/dashboard/invoices-sent.tsx (2)
79-85: Consider improving accessibility of the asterisk note.The asterisk note lacks semantic structure and may not be announced properly by screen readers. Consider using an aria-label or aria-describedby to associate the note with the stat card.
- <StatCard + <StatCard + aria-describedby={hasNonUsdValues ? "usd-conversion-note" : undefined} title="Total Payments" value={`$${Number(totalInUsd).toLocaleString()}`} icon={<DollarSign className="h-4 w-4 text-muted-foreground" />} /> {hasNonUsdValues && ( - <p className="text-xs text-muted-foreground text-center mt-1"> + <p id="usd-conversion-note" className="text-xs text-muted-foreground text-center mt-1"> * Excludes non-USD invoices without conversion info </p> )}
76-76: Standardize USD formatting with explicit locale and precision
Convert the stringtotalInUsdto a number and use.toLocaleString('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 })for consistent USD formatting:- value={`$${Number(totalInUsd).toLocaleString()}`} + value={`$${Number(totalInUsd).toLocaleString('en-US', { minimumFractionDigits: 2, maximumFractionDigits: 2 })}`}src/components/subscription-plans/blocks/subscribers-table.tsx (4)
211-215: Format USD with Intl.NumberFormat (avoid manual '$' + toLocaleString)Ensures consistent currency formatting and 2 decimal places.
- value={`$${totalRevenue.toLocaleString()}`} + value={new Intl.NumberFormat(undefined, { + style: "currency", + currency: "USD", + minimumFractionDigits: 2, + maximumFractionDigits: 2, + }).format(totalRevenue)}
210-223: Prevent footnote overlap; avoid absolute positioningCurrent absolute -bottom-5 can overlap following content. Keep it in normal flow with margin. Copy reads well; retain as-is.
- <div className="relative"> + <div> <StatCard title="Total Revenue" - value={`$${totalRevenue.toLocaleString()}`} + value={new Intl.NumberFormat(undefined, { + style: "currency", + currency: "USD", + minimumFractionDigits: 2, + maximumFractionDigits: 2, + }).format(totalRevenue)} icon={<DollarSign className="h-4 w-4 text-muted-foreground" />} /> - {hasNonUsdValues && ( - <div className="absolute -bottom-5 left-0 right-0 text-center"> - <p className="text-xs text-muted-foreground"> - * Excludes non-USD subscriptions without conversion info - </p> - </div> - )} + {hasNonUsdValues && ( + <p className="mt-1 text-center text-xs text-muted-foreground"> + * Excludes non-USD subscriptions without conversion info + </p> + )} </div>
205-209: Optional: icon semanticsConsider Users for “Active Subscriptions” to better match subscriber count.
- <StatCard + <StatCard title="Active Subscriptions" value={`${activeSubscribers.length}`} - icon={<CreditCard className="h-4 w-4 text-muted-foreground" />} + icon={<Users className="h-4 w-4 text-muted-foreground" />} />
183-200: Harden USD total accumulation (guard parseFloat + NaN check)
ReplaceNumber(totalInUsd)withparseFloatand guard againstNaN; simplifyhasNonUsdValuesassignment:- totalRevenue += Number(totalInUsd); - if (subHasNonUsdValues) { - hasNonUsdValues = true; - } + const amount = parseFloat(totalInUsd); + if (!Number.isNaN(amount)) { + totalRevenue += amount; + } + if (subHasNonUsdValues) { + hasNonUsdValues = true; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
drizzle/meta/_journal.json(1 hunks)src/components/dashboard/invoices-received.tsx(3 hunks)src/components/dashboard/invoices-sent.tsx(3 hunks)src/components/subscription-plans/blocks/payments-table.tsx(5 hunks)src/components/subscription-plans/blocks/subscribers-table.tsx(6 hunks)src/lib/helpers/conversion.ts(1 hunks)src/server/routers/subscription-plan.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- drizzle/meta/_journal.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/server/routers/subscription-plan.ts
- src/components/dashboard/invoices-received.tsx
- src/lib/helpers/conversion.ts
- src/components/subscription-plans/blocks/payments-table.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/dashboard/invoices-sent.tsx (2)
src/lib/helpers/conversion.ts (1)
consolidateRequestUsdValues(77-83)src/components/stat-card.tsx (1)
StatCard(9-21)
src/components/subscription-plans/blocks/subscribers-table.tsx (3)
src/lib/types/index.ts (1)
SubscriptionWithDetails(29-35)src/components/stat-card.tsx (1)
StatCard(9-21)src/lib/helpers/conversion.ts (1)
consolidateRecurringPaymentUsdValues(95-109)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
src/components/dashboard/invoices-sent.tsx (2)
12-12: LGTM! Clean import of the new conversion helper.The import is appropriate for the new USD consolidation functionality.
53-55: Good defensive handling with empty array fallback.The empty array fallback (
invoices || []) prevents potential runtime errors when invoices are undefined.src/components/subscription-plans/blocks/subscribers-table.tsx (1)
264-265: EmptyState icon swap to Users — LGTMMatches the subscribers context.
bassgeta
force-pushed
the
feat/143-payment-conversion-info
branch
from
bf5029b to
69a1455
Compare
MantisClone
left a comment
MantisClone
left a comment
There was a problem hiding this comment.
LGTM!
Clean implementation of USD consolidation using webhook conversion data. All critical aspects verified:
✅ Security: HMAC signature validation, transaction-safe DB ops, null-safe handling
✅ Architecture: Display-only formatting with Number() — all transaction amounts remain strings/BigNumber
✅ Quality: All CodeRabbit issues resolved, backward-compatible migration, no technical debt
Ready to merge.
Pull Request ReviewSummaryThis PR adds Changes
Review Findings✅ Good
|
Problem
The multi currency display was a bit ugly, we should use conversion values from our API.
Solution
Alas, unpaid invoices do not have that data so I had to create some helpers to pull that value out :)
Changes
helpers/conversion.tsfile which does the following:paymentItemobjectMultiCurrencyStatCardto now use these helpers and display an asterisk note under the ordinaryStatCardif we have payments not in stablecoins without conversionHow to test it
Probably just open up easy invoice and go over the following pages:
/dashboard/pay/dashboard/get-paid/dashboard/subscriptions/subscription-plans#subscribers/subscription-plans#paymentsand verify that:
Resolves #143
Summary by CodeRabbit
New Features
Bug Fixes