fix: exclude sandbox payments, export all with filters, add compare s… - #67
Conversation
…cript - Exclude sandbox (hdfcuat) payments from payment list and stats queries - Lift export limit to 10000 when export=true flag is set - Export respects current filters (status, search, date range) - Add compare-payments.js script to cross-check Juspay HDFC CSV vs admin export
📝 WalkthroughWalkthroughAdded a CSV comparison script, increased export pagination via an API flag, excluded sandbox payments in service queries, and added a client-side "Export All" CSV feature with progress/error handling. Changes
Sequence DiagramsequenceDiagram
actor User as User
participant View as PaymentAnalysisView
participant API as /api/admin/payments
participant Service as adminService
participant DB as Database
participant Browser as Browser
User->>View: Click "Export All"
View->>View: Build query params (filters, export=true, limit=10000)
View->>API: GET /api/admin/payments?export=true&...
API->>Service: Request payments (apply export limit)
Service->>DB: Query payments (exclude sandbox, apply filters)
DB-->>Service: Return payment rows
Service-->>API: Return payment data
API-->>View: JSON payment records
View->>View: Convert rows to CSV (escape fields, format dates)
View->>Browser: Create Blob & trigger download
Browser->>Browser: Save CSV
View->>View: Update exporting state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
Updates the admin payments tooling to better align reported payments with production data by excluding sandbox (hdfcuat) transactions, expanding export capabilities, and adding a local reconciliation script for comparing Juspay CSV exports with the admin export.
Changes:
- Exclude sandbox (hdfcuat) payments from admin payment list and payment stats queries.
- Add “Export All” flow that exports up to 10,000 rows and respects current filters (status/search/date range).
- Add a
compare-payments.jsscript to compare Juspay HDFC CSV vs admin export CSV.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/views/admin/payments/PaymentAnalysisView.js | Adds “Export All” button and client-side CSV export using current filters. |
| src/services/admin-service.js | Applies sandbox-payment exclusion to payments list query and payment stats query. |
| src/pages/api/admin/payments/index.js | Raises maximum limit to 10,000 when export=true. |
| scripts/compare-payments.js | Adds a Node script to compare Juspay CSV vs admin export and print investigation SQL. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FROM bookings b | ||
| WHERE ${whereClause} | ||
| LEFT JOIN payments p ON p.booking_id = b.id | ||
| WHERE ${whereClause} AND ${sandboxExclude} |
There was a problem hiding this comment.
In getPaymentStats, adding LEFT JOIN payments p can inflate COUNT(*)-based metrics when a booking has multiple payment rows (the schema allows multiple payments per booking). This will overcount total_payments and the booking-status counts. Prefer filtering sandbox bookings via NOT EXISTS (...) (or count DISTINCT b.id) to keep counts based on bookings while still excluding sandbox payments.
| function parseCSV(content) { | ||
| const lines = content.trim().split('\n'); | ||
| const headers = parseCSVLine(lines[0]); | ||
| const rows = []; | ||
| for (let i = 1; i < lines.length; i++) { |
There was a problem hiding this comment.
parseCSV() splits lines using split('\n'), which will leave trailing \r characters when the CSV uses Windows line endings (\r\n)—this can corrupt the last column/header and break comparisons. Consider splitting with /\r?\n/ (and/or trimming a trailing \r in parseCSVLine).
| const headers = ['Order ID', 'Juspay ID', 'Transaction ID', 'User', 'Email', 'Phone', 'Event', 'Amount', 'Qty', 'Status', 'Gateway Status', 'Date'] | ||
| const csvContent = [ | ||
| headers.join(','), | ||
| ...allPayments.map(row => [ | ||
| row.id, | ||
| row.juspay_order_id || '', | ||
| row.transaction_id || '', | ||
| `"${(row.user_name || '').replace(/"/g, '""')}"`, | ||
| row.user_email || '', | ||
| row.user_phone || '', | ||
| `"${(row.event_name || '').replace(/"/g, '""')}"`, | ||
| row.total_amount || 0, | ||
| row.quantity || 1, | ||
| row.status || '', | ||
| row.gateway_status || '', | ||
| row.booked_at ? format(new Date(row.booked_at), 'yyyy-MM-dd HH:mm:ss') : '' | ||
| ].join(',')) |
There was a problem hiding this comment.
The CSV generation here doesn’t consistently escape fields (e.g., email/phone/transaction_id/status) that may contain commas, quotes, or newlines, which can produce a malformed CSV. Consider reusing the escaping logic used in src/components/customComponent/CustomDatagrid.js (quote when needed + escape quotes) so exports are reliably parseable.
| ...allPayments.map(row => [ | ||
| row.id, | ||
| row.juspay_order_id || '', | ||
| row.transaction_id || '', | ||
| `"${(row.user_name || '').replace(/"/g, '""')}"`, | ||
| row.user_email || '', | ||
| row.user_phone || '', | ||
| `"${(row.event_name || '').replace(/"/g, '""')}"`, | ||
| row.total_amount || 0, | ||
| row.quantity || 1, | ||
| row.status || '', | ||
| row.gateway_status || '', | ||
| row.booked_at ? format(new Date(row.booked_at), 'yyyy-MM-dd HH:mm:ss') : '' |
There was a problem hiding this comment.
CSV exports can be vulnerable to spreadsheet formula injection (cells beginning with =, +, -, or @). Since user_name / event_name and other fields come from user-controlled data, sanitize values for CSV export (e.g., prefix a single quote) before writing them to the file.
| const handleExportAll = async () => { | ||
| setExporting(true) | ||
| try { | ||
| const params = new URLSearchParams() | ||
| if (statusFilter) params.set('status', statusFilter) | ||
| if (search) params.set('search', search) | ||
| params.set('limit', '10000') | ||
| params.set('export', 'true') | ||
|
|
There was a problem hiding this comment.
handleExportAll sets errors on failure/empty export but never clears a previous error when starting a new export. This can leave a stale error banner visible even after a successful export. Consider calling setError('') at the start of the export flow (before the request).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/services/admin-service.js (1)
578-579: Consider using a more precise JSONB path check for sandbox detection.The text-based
LIKE '%hdfcuat%'check on the entiresdk_payload::textis functional but fragile—it could match if "hdfcuat" appears anywhere in the JSON structure (e.g., in a URL, error message, or unrelated field). If the sandbox domain is stored in a specific JSONB key, consider using a path-based check likep.sdk_payload->>'endpoint' NOT LIKE '%hdfcuat%'for more precision.🤖 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 578 - 579, The current condition uses a text-based LIKE on p.sdk_payload::text which is fragile; update the predicate to check the specific JSONB key that holds the domain (e.g., use p.sdk_payload->>'endpoint' or the correct key name instead of casting whole JSON to text) so sandbox detection is precise; modify the conditions.push call that currently references p.sdk_payload to use a JSONB path extraction (p.sdk_payload->>'<key>') and then apply NOT LIKE '%hdfcuat%' (or an exact match) to that extracted value.scripts/compare-payments.js (1)
227-230: Hardcoded order IDs should be configurable or removed.These specific order IDs appear to be from a debugging session. Consider making them command-line arguments or removing this section for a cleaner utility script.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/compare-payments.js` around lines 227 - 230, The hardcoded array imageOrderIds should be removed or made configurable; replace the literal const imageOrderIds with a configurable source (accept a CLI flag like --imageOrderIds or an env var such as IMAGE_ORDER_IDS containing a comma/JSON list) and parse it into an array before use (validate it's an array of strings and provide a sensible default or show a helpful error if missing). Update any code that references imageOrderIds (the imageOrderIds symbol) to use the parsed value, and ensure argument parsing/validation is added (e.g., process.argv or a lightweight arg parser) so the script no longer contains debug-specific hardcoded order IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/compare-payments.js`:
- Line 345: Update the logged SQL fragment that references a non-existent column
by replacing "e.title as event_name" with "e.name as event_name" in the
console.log inside scripts/compare-payments.js (the logged query string that
currently contains e.title) so the column matches the events table schema.
- Around line 341-351: The code builds SQL by interpolating order IDs
(problemOrderIds) directly from allProblematic, exposing SQL injection risk;
update the mapping that builds problemOrderIds to escape single quotes in each
orderId (replace each ' with ''), and apply the same escaping wherever
juspay_txn_id or other CSV-derived values are interpolated into SQL strings;
also add a short console.warn before printing the query warning that these
queries are for inspection only and should not be executed against production
without parameterization or proper escaping.
In `@src/services/admin-service.js`:
- Around line 639-641: The LEFT JOIN to payments (the fragment using "LEFT JOIN
payments p ON p.booking_id = b.id") causes duplicate booking rows because
payments.booking_id is not unique; instead remove that join and enforce the
sandbox exclusion by adding a NOT EXISTS(...) subquery (or a CTE that
pre-filters payments) that checks for matching payments for the booking that
meet the sandbox criteria, using the same sandboxExclude expression, so the main
FROM bookings b remains one row per booking; update the SQL assembly where
${whereClause} and ${sandboxExclude} are used to reference the NOT EXISTS or CTE
and keep COUNT(*) and FILTER aggregates unchanged.
In `@src/views/admin/payments/PaymentAnalysisView.js`:
- Around line 131-148: CSV generation in csvContent currently leaves user_email
and user_phone unquoted which can break parsing if they contain commas; update
the mapping in the allPayments -> row transformation to quote and escape
user_email and user_phone (similar to how user_name and event_name are handled)
so each value is wrapped in quotes and internal quotes are doubled, ensuring
fields like user_email, user_phone (and any other unquoted string fields like
transaction_id/gateway_status if desired) are consistently escaped before
joining into the CSV line.
---
Nitpick comments:
In `@scripts/compare-payments.js`:
- Around line 227-230: The hardcoded array imageOrderIds should be removed or
made configurable; replace the literal const imageOrderIds with a configurable
source (accept a CLI flag like --imageOrderIds or an env var such as
IMAGE_ORDER_IDS containing a comma/JSON list) and parse it into an array before
use (validate it's an array of strings and provide a sensible default or show a
helpful error if missing). Update any code that references imageOrderIds (the
imageOrderIds symbol) to use the parsed value, and ensure argument
parsing/validation is added (e.g., process.argv or a lightweight arg parser) so
the script no longer contains debug-specific hardcoded order IDs.
In `@src/services/admin-service.js`:
- Around line 578-579: The current condition uses a text-based LIKE on
p.sdk_payload::text which is fragile; update the predicate to check the specific
JSONB key that holds the domain (e.g., use p.sdk_payload->>'endpoint' or the
correct key name instead of casting whole JSON to text) so sandbox detection is
precise; modify the conditions.push call that currently references p.sdk_payload
to use a JSONB path extraction (p.sdk_payload->>'<key>') and then apply NOT LIKE
'%hdfcuat%' (or an exact match) to that extracted value.
🪄 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: 020d0c2a-8fda-420e-99d4-92dce5d076ec
📒 Files selected for processing (4)
scripts/compare-payments.jssrc/pages/api/admin/payments/index.jssrc/services/admin-service.jssrc/views/admin/payments/PaymentAnalysisView.js
| const problemOrderIds = allProblematic.map(p => `'${p.orderId}'`).join(', '); | ||
| console.log(`SELECT b.id, b.status, b.juspay_order_id, b.total_amount, b.created_at,`); | ||
| console.log(` p.status as payment_status, p.gateway_status, p.transaction_id, p.paid_at,`); | ||
| console.log(` u.name, u.email, u.phone,`); | ||
| console.log(` e.title as event_name`); | ||
| console.log(`FROM bookings b`); | ||
| console.log(`LEFT JOIN payments p ON p.booking_id = b.id`); | ||
| console.log(`LEFT JOIN users u ON u.id = b.user_id`); | ||
| console.log(`LEFT JOIN booking_items bi ON bi.booking_id = b.id`); | ||
| console.log(`LEFT JOIN events e ON e.id = bi.event_id`); | ||
| console.log(`WHERE b.juspay_order_id IN (${problemOrderIds});`); |
There was a problem hiding this comment.
SQL injection risk in generated queries if CSV contains malicious data.
The order IDs are directly interpolated into SQL strings without escaping. If someone crafts a malicious CSV with an order_id like '; DROP TABLE bookings; --, the generated SQL could be dangerous when copy-pasted. Consider escaping single quotes or adding a warning comment.
🛡️ Proposed fix to escape single quotes
+// Helper to escape single quotes for SQL literals
+function escapeSql(str) {
+ return String(str).replace(/'/g, "''");
+}
+
if (allProblematic.length > 0) {
console.log('-- Query to check the problematic bookings:');
- const problemOrderIds = allProblematic.map(p => `'${p.orderId}'`).join(', ');
+ const problemOrderIds = allProblematic.map(p => `'${escapeSql(p.orderId)}'`).join(', ');Apply similar escaping to all orderId and juspay_txn_id interpolations in SQL strings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/compare-payments.js` around lines 341 - 351, The code builds SQL by
interpolating order IDs (problemOrderIds) directly from allProblematic, exposing
SQL injection risk; update the mapping that builds problemOrderIds to escape
single quotes in each orderId (replace each ' with ''), and apply the same
escaping wherever juspay_txn_id or other CSV-derived values are interpolated
into SQL strings; also add a short console.warn before printing the query
warning that these queries are for inspection only and should not be executed
against production without parameterization or proper escaping.
| console.log(`SELECT b.id, b.status, b.juspay_order_id, b.total_amount, b.created_at,`); | ||
| console.log(` p.status as payment_status, p.gateway_status, p.transaction_id, p.paid_at,`); | ||
| console.log(` u.name, u.email, u.phone,`); | ||
| console.log(` e.title as event_name`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the events table column name
rg -n "e\.name\s+AS\s+(event_name|event)" src/services/admin-service.js | head -5
# Check schema for events table structure
rg -n "CREATE TABLE events" -A 20 schema.sql 2>/dev/null || rg -n "CREATE TABLE events" -A 20 . --type sqlRepository: NexEvent/Citronics_2k26
Length of output: 1328
🏁 Script executed:
cat -n scripts/compare-payments.js | sed -n '340,350p'Repository: NexEvent/Citronics_2k26
Length of output: 853
Column name mismatch: e.title should be e.name.
The events table schema defines the event name column as name, not title. The query being logged at line 345 references a non-existent column and would fail if executed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/compare-payments.js` at line 345, Update the logged SQL fragment that
references a non-existent column by replacing "e.title as event_name" with
"e.name as event_name" in the console.log inside scripts/compare-payments.js
(the logged query string that currently contains e.title) so the column matches
the events table schema.
| LEFT JOIN payments p ON p.booking_id = b.id | ||
| WHERE ${whereClause} AND ${sandboxExclude} | ||
| `, params) |
There was a problem hiding this comment.
Critical: LEFT JOIN causes duplicate booking counts when multiple payment attempts exist.
The schema shows payments.booking_id has no UNIQUE constraint (see schema.sql:133-163), meaning a single booking can have multiple payment rows (e.g., retry attempts). The added LEFT JOIN payments p ON p.booking_id = b.id will produce multiple rows per booking, causing COUNT(*), COUNT(*) FILTER (WHERE b.status = 'confirmed'), etc. to return inflated values.
To fix this, apply the sandbox exclusion within a NOT EXISTS subquery or a CTE that pre-filters payments, rather than joining directly to the outer FROM bookings row set.
🐛 Proposed fix using NOT EXISTS
const row = await dbOneOrNone(`
SELECT
COUNT(*)::int AS total_payments,
COUNT(*) FILTER (WHERE b.status = 'confirmed')::int AS successful_payments,
COUNT(*) FILTER (WHERE b.status = 'pending')::int AS pending_payments,
COUNT(*) FILTER (WHERE b.status = 'cancelled')::int AS failed_payments,
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,
(SELECT COUNT(*)::int FROM tickets t JOIN bookings bk ON bk.id = t.booking_id WHERE ${ticketWhere}) AS total_tickets,
(SELECT COUNT(*)::int FROM tickets t JOIN bookings bk ON bk.id = t.booking_id WHERE t.check_in_at IS NOT NULL AND ${ticketWhere}) AS checked_in_tickets
FROM bookings b
- LEFT JOIN payments p ON p.booking_id = b.id
- WHERE ${whereClause} AND ${sandboxExclude}
+ WHERE ${whereClause}
+ AND NOT EXISTS (
+ SELECT 1 FROM payments px
+ WHERE px.booking_id = b.id
+ AND px.sdk_payload IS NOT NULL
+ AND px.sdk_payload::text LIKE '%hdfcuat%'
+ )
`, params)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LEFT JOIN payments p ON p.booking_id = b.id | |
| WHERE ${whereClause} AND ${sandboxExclude} | |
| `, params) | |
| LEFT JOIN payments p ON p.booking_id = b.id | |
| WHERE ${whereClause} | |
| AND NOT EXISTS ( | |
| SELECT 1 FROM payments px | |
| WHERE px.booking_id = b.id | |
| AND px.sdk_payload IS NOT NULL | |
| AND px.sdk_payload::text LIKE '%hdfcuat%' | |
| ) | |
| `, params) |
🤖 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 639 - 641, The LEFT JOIN to
payments (the fragment using "LEFT JOIN payments p ON p.booking_id = b.id")
causes duplicate booking rows because payments.booking_id is not unique; instead
remove that join and enforce the sandbox exclusion by adding a NOT EXISTS(...)
subquery (or a CTE that pre-filters payments) that checks for matching payments
for the booking that meet the sandbox criteria, using the same sandboxExclude
expression, so the main FROM bookings b remains one row per booking; update the
SQL assembly where ${whereClause} and ${sandboxExclude} are used to reference
the NOT EXISTS or CTE and keep COUNT(*) and FILTER aggregates unchanged.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
scripts/compare-payments.js (2)
345-350:⚠️ Potential issue | 🟡 MinorUse
e.namein the generated SQL.Line 345 and Line 379 still reference
e.title, butsrc/services/admin-service.js:1-30reads the event label fromevents.name. Copy-pasting these queries will fail against the current schema.🛠️ Suggested fix
- console.log(` e.title as event_name`); + console.log(` e.name as event_name`); ... - console.log(` u.name, u.email, u.phone, e.title as event`); + console.log(` u.name, u.email, u.phone, e.name as event`);Also applies to: 379-384
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/compare-payments.js` around lines 345 - 350, The generated SQL in scripts/compare-payments.js uses the wrong event column `e.title`; update every occurrence to `e.name` (e.g., the console.log lines that emit "e.title as event_name" and the later block around lines referenced 379-384) so the query matches the schema used by src/services/admin-service.js which reads events.name; search for the literal "e.title" in the file and replace with "e.name" to ensure both generated queries use the correct column.
341-367:⚠️ Potential issue | 🟡 MinorEscape CSV-derived values before embedding them in SQL.
Line 341, Line 362, Line 365, Line 367, and Line 385 interpolate
orderId/juspay_txn_iddirectly from the CSV. A single quote in either export turns these inspection queries into invalid or dangerous SQL when copy-pasted.🔒 Suggested helper
+function escapeSqlLiteral(value) { + return String(value).replace(/'/g, "''"); +} ... - const problemOrderIds = allProblematic.map(p => `'${p.orderId}'`).join(', '); + const problemOrderIds = allProblematic.map(p => `'${escapeSqlLiteral(p.orderId)}'`).join(', ');Apply the same helper to the remaining
orderIdandjuspay_txn_idinterpolations in Section 4.Also applies to: 375-385
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/compare-payments.js` around lines 341 - 367, CSV-derived values like mm.orderId and successTxn.juspay_txn_id are interpolated directly into SQL strings (e.g., problemOrderIds creation, the -- Fix lines, UPDATE bookings WHERE juspay_order_id = '${mm.orderId}', the subquery SELECT id, and the UPDATE payments transaction_id), which breaks/opens SQL when values contain quotes; add a small helper function (e.g., escapeSql or sqlEscape) that escapes single quotes (replace ' with '') and use it everywhere you currently interpolate mm.orderId and successTxn.juspay_txn_id (including the Juspay Txn log and problemOrderIds mapping, the UPDATE bookings, the SELECT id subquery, and the UPDATE payments transaction_id) so generated SQL is safe to copy/paste.
🧹 Nitpick comments (1)
scripts/compare-payments.js (1)
62-73: Prefer explicit CSV paths over “latest file in Downloads”.Now that admin exports can differ by filters, auto-selecting the newest
payments_all*.csvmakes the comparison easy to run against the wrong dataset. Takingprocess.argvpaths first and falling back to~/Downloadswould make reruns reproducible.♻️ Suggested change
-const juspayFile = findFile('44207_tab_performance'); -const adminFile = findFile('payments_all'); +const [juspayArg, adminArg] = process.argv.slice(2); +const juspayFile = juspayArg || findFile('44207_tab_performance'); +const adminFile = adminArg || findFile('payments_all');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/compare-payments.js` around lines 62 - 73, The script currently auto-selects the newest CSV in Downloads via findFile (used for juspayFile and adminFile), which can pick the wrong export; change it to first accept explicit paths from process.argv (e.g., process.argv[2] and process.argv[3]) for the admin and juspay files and only call findFile(prefix) as a fallback when those argv entries are absent or not readable, validating the provided paths exist and end with .csv before using them; update any code that references juspayFile/adminFile to use the resolved path variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/compare-payments.js`:
- Around line 343-367: The export uses payments.status as gateway_status, but
the fix SQL sets payments.status = 'completed' while the CSV checks for
'success', and also overwrites paid_at with NOW(); update the UPDATE logic in
the loop that builds SQL (around variables mm, a, successTxn and the generated
UPDATE statements) to set the same field/value the export reads (use
payments.status = 'success' or whatever exact token the export expects instead
of 'completed') and set gateway_status consistently, and do NOT set paid_at =
NOW() — instead use the original Juspay settlement timestamp from successTxn
(e.g. successTxn.settled_at or successTxn.settlement_time) or omit paid_at so
the original value is preserved; make the same correction for the second
occurrence that the comment notes (the other UPDATE block around lines 377-379).
- Around line 89-129: Add explicit header validation immediately after parseCSV
results for both juspayData and adminData: check that each row contains the
expected keys used later (e.g., for Juspay rows: order_id, payment_status,
amount, customer_id, platform; for Admin rows: Juspay ID, Order ID, Gateway
Status). Implement this validation in the same scope where juspayData/adminData
are created (before building juspayByOrder or adminByJuspayId) and if any
required header is missing, throw or exit with a clear error referencing the
missing header(s) and the source file variable (juspayData/adminData) so the
script fails fast instead of producing empty lookups like juspayByOrder or
adminByJuspayId and generating misleading SQL.
In `@src/views/admin/payments/PaymentAnalysisView.js`:
- Around line 130-131: The export flow currently forces
params.set('limit','10000') while the UI shows "Export All" (in
PaymentAnalysisView), which can silently truncate data; fix by making the export
limit reflect the real total or change the label: either (A) remove the
hardcoded params.set('limit','10000') and set params.set('limit',
String(totalCount)) using the component's totalCount/rowsCount before sending
the export request (or fetch the full count then set limit), or (B) if you must
keep a safety cap, update the export button/label and tooltip (the "Export All"
action handler and UI text around lines referencing the export action) to
indicate "Export (first 10,000 rows)" and surface a warning when totalCount >
10000; update all uses around params.set('limit','10000') (lines ~352-356 and
130) and the export handler function in PaymentAnalysisView accordingly.
- Line 160: Validate row.booked_at before calling format to avoid "Invalid Date"
in the CSV: construct a Date from row.booked_at, check its validity (e.g., using
date-fns isValid(date) or testing !Number.isNaN(date.getTime())), and only call
format(date, 'yyyy-MM-dd HH:mm:ss') when valid; otherwise return an empty
string. Update the CSV export code that currently does format(new
Date(row.booked_at), ...) (in PaymentAnalysisView) to perform this check and use
the empty-string fallback consistent with the existing try-catch guards.
---
Duplicate comments:
In `@scripts/compare-payments.js`:
- Around line 345-350: The generated SQL in scripts/compare-payments.js uses the
wrong event column `e.title`; update every occurrence to `e.name` (e.g., the
console.log lines that emit "e.title as event_name" and the later block around
lines referenced 379-384) so the query matches the schema used by
src/services/admin-service.js which reads events.name; search for the literal
"e.title" in the file and replace with "e.name" to ensure both generated queries
use the correct column.
- Around line 341-367: CSV-derived values like mm.orderId and
successTxn.juspay_txn_id are interpolated directly into SQL strings (e.g.,
problemOrderIds creation, the -- Fix lines, UPDATE bookings WHERE
juspay_order_id = '${mm.orderId}', the subquery SELECT id, and the UPDATE
payments transaction_id), which breaks/opens SQL when values contain quotes; add
a small helper function (e.g., escapeSql or sqlEscape) that escapes single
quotes (replace ' with '') and use it everywhere you currently interpolate
mm.orderId and successTxn.juspay_txn_id (including the Juspay Txn log and
problemOrderIds mapping, the UPDATE bookings, the SELECT id subquery, and the
UPDATE payments transaction_id) so generated SQL is safe to copy/paste.
---
Nitpick comments:
In `@scripts/compare-payments.js`:
- Around line 62-73: The script currently auto-selects the newest CSV in
Downloads via findFile (used for juspayFile and adminFile), which can pick the
wrong export; change it to first accept explicit paths from process.argv (e.g.,
process.argv[2] and process.argv[3]) for the admin and juspay files and only
call findFile(prefix) as a fallback when those argv entries are absent or not
readable, validating the provided paths exist and end with .csv before using
them; update any code that references juspayFile/adminFile to use the resolved
path variables.
🪄 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: 4883fc54-895a-4f1f-83be-4c159acc9165
📒 Files selected for processing (3)
scripts/compare-payments.jssrc/services/admin-service.jssrc/views/admin/payments/PaymentAnalysisView.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/admin-service.js
| const juspayData = parseCSV(fs.readFileSync(juspayFile, 'utf-8')); | ||
| const adminData = parseCSV(fs.readFileSync(adminFile, 'utf-8')); | ||
|
|
||
| // ─── Build lookup maps ─── | ||
|
|
||
| // Juspay: Group by order_id, track all txn attempts and final status | ||
| // For each unique order_id, find the last transaction attempt's status | ||
| const juspayByOrder = {}; | ||
| for (const row of juspayData) { | ||
| const orderId = row.order_id; | ||
| if (!juspayByOrder[orderId]) { | ||
| juspayByOrder[orderId] = []; | ||
| } | ||
| juspayByOrder[orderId].push(row); | ||
| } | ||
|
|
||
| // Determine final Juspay status for each order (SUCCESS if any attempt was SUCCESS) | ||
| const juspayFinalStatus = {}; | ||
| for (const [orderId, attempts] of Object.entries(juspayByOrder)) { | ||
| const hasSuccess = attempts.some(a => a.payment_status === 'SUCCESS'); | ||
| juspayFinalStatus[orderId] = { | ||
| status: hasSuccess ? 'SUCCESS' : 'FAILURE', | ||
| amount: parseFloat(attempts[0].amount), | ||
| attempts: attempts.length, | ||
| successTxn: attempts.find(a => a.payment_status === 'SUCCESS'), | ||
| allAttempts: attempts, | ||
| customerId: attempts[0].customer_id, | ||
| platform: attempts[0].platform, | ||
| }; | ||
| } | ||
|
|
||
| // Admin: Map by Juspay ID (which is the order_id from Juspay) | ||
| const adminByJuspayId = {}; | ||
| const adminByOrderId = {}; // by admin's internal Order ID | ||
| for (const row of adminData) { | ||
| const juspayId = row['Juspay ID']; | ||
| if (juspayId) { | ||
| adminByJuspayId[juspayId] = row; | ||
| } | ||
| adminByOrderId[row['Order ID']] = row; | ||
| } |
There was a problem hiding this comment.
Fail fast when the CSV headers do not match the expected export shape.
This script is keyed off exact columns like order_id, payment_status, Order ID, Juspay ID, and Gateway Status. If someone grabs the wrong file or the admin headers drift from src/views/admin/payments/PaymentAnalysisView.js:145-160, adminByJuspayId goes empty and Section 4 can emit misleading fix SQL for every successful Juspay order.
🧪 Suggested guardrail
function parseCSV(content) {
const lines = content.trim().split(/\r?\n/);
- const headers = parseCSVLine(lines[0]);
+ const headers = parseCSVLine(lines[0]).map(h => h.trim());
const rows = [];
for (let i = 1; i < lines.length; i++) {
const line = lines[i].trim();
if (!line) continue;
const values = parseCSVLine(line);
const row = {};
headers.forEach((h, idx) => {
- row[h.trim()] = (values[idx] || '').trim();
+ row[h] = (values[idx] || '').trim();
});
rows.push(row);
}
- return rows;
+ return { headers, rows };
+}
+
+function assertRequiredColumns(headers, required, label) {
+ const present = new Set(headers);
+ const missing = required.filter(column => !present.has(column));
+ if (missing.length > 0) {
+ throw new Error(`${label} CSV is missing required columns: ${missing.join(', ')}`);
+ }
}
-const juspayData = parseCSV(fs.readFileSync(juspayFile, 'utf-8'));
-const adminData = parseCSV(fs.readFileSync(adminFile, 'utf-8'));
+const { headers: juspayHeaders, rows: juspayData } = parseCSV(fs.readFileSync(juspayFile, 'utf-8'));
+const { headers: adminHeaders, rows: adminData } = parseCSV(fs.readFileSync(adminFile, 'utf-8'));
+
+assertRequiredColumns(juspayHeaders, ['order_id', 'payment_status', 'amount'], 'Juspay');
+assertRequiredColumns(adminHeaders, ['Order ID', 'Juspay ID', 'Gateway Status', 'Status', 'Amount'], 'Admin');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/compare-payments.js` around lines 89 - 129, Add explicit header
validation immediately after parseCSV results for both juspayData and adminData:
check that each row contains the expected keys used later (e.g., for Juspay
rows: order_id, payment_status, amount, customer_id, platform; for Admin rows:
Juspay ID, Order ID, Gateway Status). Implement this validation in the same
scope where juspayData/adminData are created (before building juspayByOrder or
adminByJuspayId) and if any required header is missing, throw or exit with a
clear error referencing the missing header(s) and the source file variable
(juspayData/adminData) so the script fails fast instead of producing empty
lookups like juspayByOrder or adminByJuspayId and generating misleading SQL.
| console.log(` p.status as payment_status, p.gateway_status, p.transaction_id, p.paid_at,`); | ||
| console.log(` u.name, u.email, u.phone,`); | ||
| console.log(` e.title as event_name`); | ||
| console.log(`FROM bookings b`); | ||
| console.log(`LEFT JOIN payments p ON p.booking_id = b.id`); | ||
| console.log(`LEFT JOIN users u ON u.id = b.user_id`); | ||
| console.log(`LEFT JOIN booking_items bi ON bi.booking_id = b.id`); | ||
| console.log(`LEFT JOIN events e ON e.id = bi.event_id`); | ||
| console.log(`WHERE b.juspay_order_id IN (${problemOrderIds});`); | ||
| console.log(''); | ||
|
|
||
| console.log('-- Fix queries (UPDATE status for confirmed payments):'); | ||
| console.log('-- ⚠️ VERIFY EACH ONE BEFORE RUNNING! Check Juspay dashboard first.'); | ||
| console.log(''); | ||
| for (const mm of allProblematic) { | ||
| const a = mm.adminRow; | ||
| const successTxn = mm.juspayInfo.successTxn; | ||
| console.log(`-- Fix: ${a['User']} - ${a['Event']} - ₹${a['Amount']}`); | ||
| console.log(`-- Juspay Txn: ${successTxn ? successTxn.juspay_txn_id : 'N/A'}`); | ||
| console.log(`UPDATE bookings SET status = 'confirmed' WHERE juspay_order_id = '${mm.orderId}';`); | ||
| if (successTxn) { | ||
| console.log(`UPDATE payments SET status = 'completed', gateway_status = 'success',`); | ||
| console.log(` transaction_id = '${successTxn.juspay_txn_id}',`); | ||
| console.log(` paid_at = NOW()`); | ||
| console.log(` WHERE booking_id = (SELECT id FROM bookings WHERE juspay_order_id = '${mm.orderId}');`); |
There was a problem hiding this comment.
Update the same payment status field/value that the export reads.
src/services/admin-service.js:1-30 exposes the admin CSV’s “Gateway Status” from p.status AS gateway_status, and this script only treats 'success' as successful on Line 156 and Line 307. The generated fix SQL sets payments.status = 'completed' and separately updates gateway_status, so rerunning the export can still leave the row non-success. paid_at = NOW() also rewrites the original settlement time.
🛠️ Suggested fix
- console.log(` p.status as payment_status, p.gateway_status, p.transaction_id, p.paid_at,`);
+ console.log(` p.status as payment_status, p.transaction_id, p.paid_at,`);
...
- console.log(`UPDATE payments SET status = 'completed', gateway_status = 'success',`);
- console.log(` transaction_id = '${successTxn.juspay_txn_id}',`);
- console.log(` paid_at = NOW()`);
+ console.log(`UPDATE payments SET status = 'success',`);
+ console.log(` transaction_id = '${successTxn.juspay_txn_id}'`);
...
- console.log(` p.status as pay_status, p.gateway_status, p.transaction_id,`);
+ console.log(` p.status as payment_status, p.transaction_id,`);Also applies to: 377-379
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/compare-payments.js` around lines 343 - 367, The export uses
payments.status as gateway_status, but the fix SQL sets payments.status =
'completed' while the CSV checks for 'success', and also overwrites paid_at with
NOW(); update the UPDATE logic in the loop that builds SQL (around variables mm,
a, successTxn and the generated UPDATE statements) to set the same field/value
the export reads (use payments.status = 'success' or whatever exact token the
export expects instead of 'completed') and set gateway_status consistently, and
do NOT set paid_at = NOW() — instead use the original Juspay settlement
timestamp from successTxn (e.g. successTxn.settled_at or
successTxn.settlement_time) or omit paid_at so the original value is preserved;
make the same correction for the second occurrence that the comment notes (the
other UPDATE block around lines 377-379).
| params.set('limit', '10000') | ||
| params.set('export', 'true') |
There was a problem hiding this comment.
“Export All” is misleading with a hard 10,000-row cap.
At Line 130 you hard-cap export to 10000, but Line 355 labels it as Export All. This can silently produce incomplete reconciliation files when total rows exceed 10,000.
💡 Minimal UX-safe patch
- params.set('limit', '10000')
+ params.set('limit', '10000')
params.set('export', 'true')
...
- {exporting ? 'Exporting...' : 'Export All'}
+ {exporting ? 'Exporting...' : 'Export (max 10,000)'}Also applies to: 352-356
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/views/admin/payments/PaymentAnalysisView.js` around lines 130 - 131, The
export flow currently forces params.set('limit','10000') while the UI shows
"Export All" (in PaymentAnalysisView), which can silently truncate data; fix by
making the export limit reflect the real total or change the label: either (A)
remove the hardcoded params.set('limit','10000') and set params.set('limit',
String(totalCount)) using the component's totalCount/rowsCount before sending
the export request (or fetch the full count then set limit), or (B) if you must
keep a safety cap, update the export button/label and tooltip (the "Export All"
action handler and UI text around lines referencing the export action) to
indicate "Export (first 10,000 rows)" and surface a warning when totalCount >
10000; update all uses around params.set('limit','10000') (lines ~352-356 and
130) and the export handler function in PaymentAnalysisView accordingly.
| row.quantity || 1, | ||
| escapeCSVField(row.status), | ||
| escapeCSVField(row.gateway_status), | ||
| row.booked_at ? format(new Date(row.booked_at), 'yyyy-MM-dd HH:mm:ss') : '' |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In date-fns, does format(new Date('invalid'), 'yyyy-MM-dd HH:mm:ss')throw a RangeError, and isisValid/timestamp validation the recommended guard?
💡 Result:
No, format(new Date('invalid'), 'yyyy-MM-dd HH:mm:ss') does not throw a RangeError in date-fns. It returns 'Invalid Date' string. Yes, isValid is the recommended guard to check if a date is valid before formatting, as it returns false for Invalid Dates (where date.getTime is NaN). For timestamps, check if they are finite numbers.
Citations:
- 1: https://docs.w3cub.com/date_fns/isValid.html
- 2: https://github.com/date-fns/date-fns/blob/main/src/isValid/index.ts
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Invalid_date
- 4: format() method shows
Invalid Dateon Safari macOS and iOS date-fns/date-fns#1275
🏁 Script executed:
# Find the PaymentAnalysisView.js file
find . -name "PaymentAnalysisView.js" -type fRepository: NexEvent/Citronics_2k26
Length of output: 116
🏁 Script executed:
# Check the structure of the repository
ls -la src/views/admin/payments/ 2>/dev/null || echo "Path not found, searching for payment-related files"
find . -path "*/payments*" -type f | head -20Repository: NexEvent/Citronics_2k26
Length of output: 474
🏁 Script executed:
# Read the PaymentAnalysisView.js file to see line 160 and surrounding context
cat -n src/views/admin/payments/PaymentAnalysisView.js | sed -n '150,170p'Repository: NexEvent/Citronics_2k26
Length of output: 1072
🏁 Script executed:
# Check file size to see if we need to read selectively
wc -l src/views/admin/payments/PaymentAnalysisView.jsRepository: NexEvent/Citronics_2k26
Length of output: 118
🏁 Script executed:
# Look for date-fns imports and format usage
grep -n "import.*format\|from.*date-fns" src/views/admin/payments/PaymentAnalysisView.js | head -10Repository: NexEvent/Citronics_2k26
Length of output: 103
🏁 Script executed:
# Search for other date formatting in the same file
grep -n "format(new Date" src/views/admin/payments/PaymentAnalysisView.jsRepository: NexEvent/Citronics_2k26
Length of output: 396
Guard against invalid dates in CSV export to prevent malformed output.
Line 160 doesn't handle invalid date strings. If row.booked_at is malformed, format(new Date(row.booked_at), ...) will produce 'Invalid Date' in the CSV. This is inconsistent with the try-catch guards on lines 39–42 in the same file. Add validation before formatting.
+ const safeExportDate = value => {
+ if (!value) return ''
+ const d = new Date(value)
+ if (Number.isNaN(d.getTime())) return ''
+ return format(d, 'yyyy-MM-dd HH:mm:ss')
+ }
...
- row.booked_at ? format(new Date(row.booked_at), 'yyyy-MM-dd HH:mm:ss') : ''
+ escapeCSVField(safeExportDate(row.booked_at))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/views/admin/payments/PaymentAnalysisView.js` at line 160, Validate
row.booked_at before calling format to avoid "Invalid Date" in the CSV:
construct a Date from row.booked_at, check its validity (e.g., using date-fns
isValid(date) or testing !Number.isNaN(date.getTime())), and only call
format(date, 'yyyy-MM-dd HH:mm:ss') when valid; otherwise return an empty
string. Update the CSV export code that currently does format(new
Date(row.booked_at), ...) (in PaymentAnalysisView) to perform this check and use
the empty-string fallback consistent with the existing try-catch guards.
…cript
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
New Features
Bug Fixes
Chores