fix: admin payment revenue calc uses payments table with refund deduc… - #66
Conversation
…tion, add order/juspay/txn IDs to export - Revenue and avg order value now calculated from payments table (amount - refund_amount) instead of bookings.total_amount - Includes both success and refunded payment statuses to match HDFC portal figures - Added juspay_order_id to SQL query SELECT and GROUP BY - Added Order ID, Juspay ID, and Transaction ID columns to payments DataGrid (and CSV export)
📝 WalkthroughWalkthroughModified payment statistics calculation in the admin service to aggregate revenue metrics from payment records instead of bookings, including filtering by payment status. Extended payment listings to include Juspay Order ID. Added three new columns to the admin payments view for displaying Order ID, Juspay ID, and Transaction ID with enhanced formatting. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adjusts admin payment analytics and exports to better align revenue calculations with gateway reporting (including refunds) and expose additional payment identifiers in the admin payments table/export.
Changes:
- Revenue and average order value are recalculated from
payments(amount - refund_amount) and includesuccess+refundedpayment statuses. juspay_order_idis added to the admin payments query (SELECT + GROUP BY).- Admin Payments DataGrid adds Order/Booking ID, Juspay ID, and Transaction ID columns (and thus into CSV export).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/views/admin/payments/PaymentAnalysisView.js | Adds new identifier columns (Order/Booking ID, Juspay ID, Transaction ID) to the transactions grid/export. |
| src/services/admin-service.js | Extends payments query to include juspay_order_id and updates KPI revenue/AOV calculations to use payments net amounts (incl. refunds). |
Comments suppressed due to low confidence (1)
src/services/admin-service.js:553
getPaymentsWithTicketsselectsp.transaction_id/p.juspay_order_idviaLEFT JOIN payments p ON p.booking_id = b.id. But payments are created once per multi-event order and linked only to the first booking, with the other booking IDs stored inpayments.raw_payload.bookingIds; for those additional bookings these columns will be NULL, which can make the new export columns incomplete/misleading. If multi-event orders are expected in this view/export, consider joining payments to bookings viaraw_payload.bookingIds(or storing a payment row per booking) so every booking row can resolve its order/transaction IDs.
let query = `
SELECT b.id, b.quantity, b.total_amount, b.status, b.booked_at,
u.id AS user_id, u.name AS user_name, u.email AS user_email, u.phone AS user_phone,
e.id AS event_id, e.name AS event_name,
COUNT(t.id)::int AS tickets_generated,
COUNT(t.check_in_at)::int AS tickets_checked_in,
p.transaction_id, p.juspay_order_id, p.gateway, p.status AS gateway_status, p.paid_at
FROM bookings b
JOIN users u ON u.id = b.user_id
JOIN events e ON e.id = b.event_id
LEFT JOIN tickets t ON t.booking_id = b.id
LEFT JOIN payments p ON p.booking_id = b.id
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| field: 'id', headerName: 'Order ID', width: 100, | ||
| renderCell: ({ row }) => ( | ||
| <Typography variant='body2' fontWeight={600} sx={{ fontFamily: 'monospace' }}> | ||
| #{row.id} | ||
| </Typography> |
There was a problem hiding this comment.
This column labels row.id as "Order ID", but in this grid id is the booking id (while the actual payment/order identifier is shown as juspay_order_id). Renaming the header to something like "Booking ID" (or "Booking #") would reduce confusion in the UI and CSV export.
| COALESCE((SELECT SUM(p.amount - COALESCE(p.refund_amount, 0)) FROM payments p JOIN bookings bk ON bk.id = p.booking_id WHERE ${paymentWhere}), 0)::numeric AS total_revenue, | ||
| COALESCE((SELECT AVG(p.amount - COALESCE(p.refund_amount, 0)) FROM payments p JOIN bookings bk ON bk.id = p.booking_id WHERE ${paymentWhere} AND (p.amount - COALESCE(p.refund_amount, 0)) > 0), 0)::numeric AS avg_order_value, |
There was a problem hiding this comment.
getPaymentStats now computes revenue/AOV from payments joined via p.booking_id. But payments are created as a single record linked to the first booking in a multi-event order (see payment-service comment), so manager-scoped stats can incorrectly include/exclude revenue depending on which booking was first, and AOV becomes per-payment rather than per-booking. Consider deriving manager revenue from bookings (as before) or expanding the payments subquery to associate a payment to all bookingIds stored in payments.raw_payload so manager/date filters apply correctly.
| COALESCE((SELECT SUM(p.amount - COALESCE(p.refund_amount, 0)) FROM payments p JOIN bookings bk ON bk.id = p.booking_id WHERE ${paymentWhere}), 0)::numeric AS total_revenue, | |
| COALESCE((SELECT AVG(p.amount - COALESCE(p.refund_amount, 0)) FROM payments p JOIN bookings bk ON bk.id = p.booking_id WHERE ${paymentWhere} AND (p.amount - COALESCE(p.refund_amount, 0)) > 0), 0)::numeric AS avg_order_value, | |
| COALESCE(( | |
| SELECT SUM(p.amount - COALESCE(p.refund_amount, 0)) | |
| FROM payments p | |
| JOIN LATERAL jsonb_array_elements_text( | |
| COALESCE( | |
| (p.raw_payload->'bookingIds')::jsonb, | |
| to_jsonb(ARRAY[p.booking_id]::int[]) | |
| ) | |
| ) AS bid(booking_id_text) ON TRUE | |
| JOIN bookings bk ON bk.id = bid.booking_id_text::int | |
| WHERE ${paymentWhere} | |
| ), 0)::numeric AS total_revenue, | |
| COALESCE(( | |
| SELECT AVG(p.amount - COALESCE(p.refund_amount, 0)) | |
| FROM payments p | |
| JOIN LATERAL jsonb_array_elements_text( | |
| COALESCE( | |
| (p.raw_payload->'bookingIds')::jsonb, | |
| to_jsonb(ARRAY[p.booking_id]::int[]) | |
| ) | |
| ) AS bid(booking_id_text) ON TRUE | |
| JOIN bookings bk ON bk.id = bid.booking_id_text::int | |
| WHERE ${paymentWhere} AND (p.amount - COALESCE(p.refund_amount, 0)) > 0 | |
| ), 0)::numeric AS avg_order_value, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/admin-service.js (1)
625-637:⚠️ Potential issue | 🟠 MajorKeep the KPI sources consistent.
totalRevenueandavgOrderValueare now payment-derived, buttotal_paymentsand the status buckets are still counted frombookings. That means this payload can include bookings with no payment row, and it still collapses multiple payment attempts into one “payment”. The same object is rendered as payment KPIs insrc/views/admin/payments/PaymentAnalysisView.js, Lines 123-127 and 349-362, so the dashboard can still disagree with the transaction list and gateway totals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/admin-service.js` around lines 625 - 637, The KPI counts are inconsistent: total_payments and status buckets are being counted from bookings (COUNT(*) and FILTER on b.status) while total_revenue and avg_order_value use the payments table/paymentWhere. Update the query so payment-derived KPIs are all sourced from the payments table—e.g., replace total_payments and the COUNT FILTERs on b.status with counts/selects from payments (alias p) joined to bookings (bk) using the same paymentWhere filter (or move the FROM to payments p JOIN bookings b and use paymentWhere for those aggregates), ensuring total_revenue, avg_order_value, total_payments, and payment status buckets all use the same paymentWhere and payment-derived rows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/admin-service.js`:
- Around line 548-549: The SQL SELECT is missing the payment row identifier
(p.id) which causes multiple payments for the same booking to produce duplicate
b.id rows and collide with the frontend DataGrid that uses getRowId={row =>
row.id}; update the query that builds the payment rows to include p.id (or alias
it as payment_id) in the SELECT and GROUP BY so each payment row has a unique
id, and ensure the frontend PaymentAnalysisView's grid uses that payment_id (or
the new unique alias) as the row key instead of booking id.
---
Outside diff comments:
In `@src/services/admin-service.js`:
- Around line 625-637: The KPI counts are inconsistent: total_payments and
status buckets are being counted from bookings (COUNT(*) and FILTER on b.status)
while total_revenue and avg_order_value use the payments table/paymentWhere.
Update the query so payment-derived KPIs are all sourced from the payments
table—e.g., replace total_payments and the COUNT FILTERs on b.status with
counts/selects from payments (alias p) joined to bookings (bk) using the same
paymentWhere filter (or move the FROM to payments p JOIN bookings b and use
paymentWhere for those aggregates), ensuring total_revenue, avg_order_value,
total_payments, and payment status buckets all use the same paymentWhere and
payment-derived rows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c7e1543-1583-4518-8596-e538a61b6e14
📒 Files selected for processing (2)
src/services/admin-service.jssrc/views/admin/payments/PaymentAnalysisView.js
| p.transaction_id, p.juspay_order_id, p.gateway, p.status AS gateway_status, p.paid_at | ||
| FROM bookings b |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== getPaymentsWithTickets row shape ==\n'
sed -n '539,586p' src/services/admin-service.js
printf '\n== PaymentAnalysisView row key ==\n'
sed -n '505,518p' src/views/admin/payments/PaymentAnalysisView.js
printf '\n== payments table schema ==\n'
sed -n '133,160p' schema.sqlRepository: NexEvent/Citronics_2k26
Length of output: 3748
Add a payment-scoped row ID to prevent DataGrid collisions.
The query groups by p.juspay_order_id and other payment fields without grouping by p.id. This means a single booking with multiple payments will produce multiple rows in the result—all with the same b.id. The frontend grid at src/views/admin/payments/PaymentAnalysisView.js, Line 516 uses getRowId={row => row.id} (booking ID), causing duplicate row IDs in the DataGrid whenever a booking has more than one payment.
Suggested change
- COUNT(t.check_in_at)::int AS tickets_checked_in,
- p.transaction_id, p.juspay_order_id, p.gateway, p.status AS gateway_status, p.paid_at
+ COUNT(t.check_in_at)::int AS tickets_checked_in,
+ p.id AS payment_id, p.transaction_id, p.juspay_order_id, p.gateway, p.status AS gateway_status, p.paid_at
...
- p.transaction_id, p.juspay_order_id, p.gateway, p.status, p.paid_at`
+ p.id, p.transaction_id, p.juspay_order_id, p.gateway, p.status, p.paid_at`Update the grid row key:
- getRowId={row => row.id}
+ getRowId={row => row.payment_id ?? row.id}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/admin-service.js` around lines 548 - 549, The SQL SELECT is
missing the payment row identifier (p.id) which causes multiple payments for the
same booking to produce duplicate b.id rows and collide with the frontend
DataGrid that uses getRowId={row => row.id}; update the query that builds the
payment rows to include p.id (or alias it as payment_id) in the SELECT and GROUP
BY so each payment row has a unique id, and ensure the frontend
PaymentAnalysisView's grid uses that payment_id (or the new unique alias) as the
row key instead of booking id.
| const subqueryDateCond = [`bk.booked_at >= $${fromParamPosition}`] | ||
| if (toParamPosition) subqueryDateCond.push(`bk.booked_at <= $${toParamPosition}`) | ||
| const ticketWhere = [subqueryManagerCond, ...subqueryDateCond, 'bk.status = \'confirmed\''].filter(Boolean).join(' AND ') | ||
| const paymentWhere = [subqueryManagerCond, ...subqueryDateCond, "p.status IN ('success', 'refunded')"].filter(Boolean).join(' AND ') |
There was a problem hiding this comment.
Don't window refunded revenue by bk.booked_at.
The new net-revenue formula still uses the booking timestamp for date filtering. That makes historical ranges unstable: a refund posted later changes revenue for the original booking window because refund_amount is subtracted regardless of refund_at. If this report is meant to reconcile with payment data, the windowing needs to use payment/refund timestamps, not the booking date.
Also applies to: 631-632
…tion, add order/juspay/txn IDs to export
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist:
Summary by CodeRabbit