fix: applied credits were being removed on redundant api calls#291
fix: applied credits were being removed on redundant api calls#291vendz wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors travel booking and transaction management, specifically introducing a more comprehensive adjustAmount helper to handle credit refunds and transaction status updates. It also updates the Utsav booking logic to correctly track discounted amounts. Feedback identifies a logic gap in adjustAmount where increasing a transaction's total does not revert a "completed" status to "pending," which could result in unpaid balances. Further improvements were suggested to allow applying additional wallet credits when amounts increase and to restore the booking parameter for automatic status synchronization.
| let newStatus = transaction.status; | ||
| if (newNet === 0) { | ||
| newStatus = STATUS_PAYMENT_COMPLETED; | ||
| } |
There was a problem hiding this comment.
The logic for updating newStatus is incomplete. If a transaction was previously completed (e.g., fully paid) and the gross amount is increased such that a balance is now owed (newNet > 0), the status remains completed. This will lead to payment discrepancies where bookings appear fully paid despite having an outstanding balance.
let newStatus = transaction.status;
if (newNet === 0) {
newStatus = STATUS_PAYMENT_COMPLETED;
} else if (
[STATUS_PAYMENT_COMPLETED, STATUS_CASH_COMPLETED].includes(transaction.status)
) {
newStatus = STATUS_CASH_PENDING;
}|
|
||
| const bookingType = getBookingType(transaction); | ||
| const creditsAlreadyUsed = transaction.discount; | ||
| const newCreditsUsed = Math.min(amount, creditsAlreadyUsed); |
There was a problem hiding this comment.
This logic only re-applies credits that were already associated with this transaction. If the gross amount increases, it should ideally check the user's wallet (card.credits) to see if any additional credits can be applied to cover the new balance, similar to how createPendingTransaction works. Additionally, consider keeping the booking parameter to automatically update the booking status (e.g., to confirmed) if the transaction becomes fully paid, consistent with the behavior in useCredit.
There was a problem hiding this comment.
Pull request overview
Fixes incorrect handling of wallet credits when booking amounts are created/updated, ensuring Razorpay orders and transaction net amounts remain consistent with applied credits.
Changes:
- Utsav booking now totals the discounted (net) amount returned by
createPendingTransaction, instead of the package gross. - Introduces a rewritten
adjustAmounthelper to re-apply existing credits and refund excess credits on gross decreases. - Travel admin booking update now delegates credit/amount reconciliation to
adjustAmount(replacing inline credit math).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
helpers/utsavBooking.helper.js |
Uses discountedAmount from createPendingTransaction when accumulating total payable amount; minor formatting fix in date range logic. |
helpers/transactions.helper.js |
Reworks adjustAmount to compute new net/discount and refund excess credits to wallet. |
controllers/admin/travelManagement.controller.js |
Uses adjustAmount for admin booking amount updates and applies some formatting/cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let newStatus = transaction.status; | ||
| if (newNet === 0) { | ||
| newStatus = STATUS_PAYMENT_COMPLETED; | ||
| } | ||
|
|
||
| await transaction.update( | ||
| { | ||
| amount: newNet, | ||
| discount: newCreditsUsed, | ||
| description: newCreditsUsed > 0 | ||
| ? `credits used: ${newCreditsUsed}` | ||
| : `Transaction updated. New balance: ${newNet}.`, | ||
| status: newStatus, | ||
| updatedBy, |
| const card = await validateCard(transaction.cardno); | ||
| await adjustAmount(card, transaction, Number(amount), req.user.username, t); | ||
| updatedFields.push('amount'); |
There was a problem hiding this comment.
@copilot suggest changes based on this feedback
There was a problem hiding this comment.
Already addressed in 38d86eb — amount is now parsed with Number(), validated with Number.isFinite() && >= 0, and a 400 ApiError is thrown before adjustAmount is called on invalid input.
…controller Agent-Logs-Url: https://github.com/SRATRC/aashray-backend/sessions/827da12e-63c0-4d1f-8a78-85b5c9a48cba Co-authored-by: vendz <31917677+vendz@users.noreply.github.com>
Merged upstream changes (Number() casts for amount/discount) with stashed changes (guard against updating completed transactions). Both are needed: - Guard prevents invalid state changes - Number() casts ensure arithmetic works correctly with originalDiscount Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fix: Wallet credit not respected on booking amount updates
Problem
Two separate bugs caused customers to be overcharged when wallet credits were involved:
Bug 1 — Utsav booking (
utsavBooking.helper.js)bookUtsavForMumukshusaccumulatedpackage_info.amount(gross) into the Razorpay order total instead of thediscountedAmountreturned bycreatePendingTransaction. This meant a customer with ₹400 credit booking a ₹530 utsav package would have the wallet correctly debited (transaction row:amount=130, discount=400) but a Razorpay order created for the full ₹530.Bug 2 — Travel booking update (
travelManagement.controller.js)The
PUT /admin/travel/bookingupdateendpoint blindly overwrotetransaction.amountwith whatever the admin submitted, without accounting for an already-applied wallet discount. IfcreatePendingTransactionhad runuseCredit(reducingamount=530 → 130, discount=400), a subsequent admin edit resendingamount=530would restore the gross while leavingdiscount=400intact — causing the Razorpay order to be generated for ₹530 while the wallet credit was silently consumed and the transaction description falsely claimed "credits used: 400".This is the confirmed root cause of the payment discrepancy for booking
51cdddd9-203d-471a-88b9-26f24309c834(charged ₹530 instead of ₹130).Bug 3 —
adjustAmounthelper (transactions.helper.js)The existing
adjustAmountfunction referenced an undefineduservariable (parameter wasupdatedBy: string) and had incorrect logic for the amount-increase case (discount: originalAmountwas semantically wrong). It was unused due to this crash, so no live impact — but it was the right abstraction for the fix above.Evidence from logs
Changes
helpers/transactions.helper.jsadjustAmount: replaced undefineduserwith{ username: updatedBy }, rewrote both branches (gross decrease and gross increase) into a single unified credit re-application —newCreditsUsed = min(newGross, existingDiscount), refund excess to wallet if gross shrank, setSTATUS_PAYMENT_COMPLETEDif net hits zero. Removed unusedbookingparameter.helpers/utsavBooking.helper.jsbookUtsavForMumukshus: destructure and accumulatediscountedAmount(notpackage_info.amount) fromcreatePendingTransaction, matching the correct pattern already used in room and adhyayan booking helpers.controllers/admin/travelManagement.controller.jsupdateBooking: replace ~40 lines of inline (and partially incorrect) credit math with a call toadjustAmount. Credit re-application, wallet refund on gross decrease, and status transitions are now handled in one place.adjustAmountfromtransactions.helper.js.Behaviour after fix
amountamountamount