Uh oh!
There was an error while loading. Please reload this page.
feat(refunds): refund a booking from the Desk (backport #328) - #329
Conversation
* feat(refunds): add refund fields to Event Booking Event Booking Refund holds one row per refund raised against a booking: the gateway refund id, its amount, whether the gateway has settled it, the tickets it covers and the cancellation request queued for them. The booking itself carries the summary — refund status and the total refunded so far — in a section that appears only once there is a refund to show. Every field is allow_on_submit, on the child doctype as well as the parent, since refunds always land after the booking is submitted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(refunds): refund a booking and apply the gateway's answer `EventBooking.refund` is System Manager only. It resolves the received Event Payment, refuses anything that is not Razorpay with a message saying so, and hands the amount to the payments app, which owns the validation against the live payment. `get_refundable_tickets` offers each ticket with the share of the total the buyer actually paid for it. Attendee amounts are pre-tax and pre-discount, so they are scaled by what was charged; the ratio is taken over current attendee amounts rather than net_amount, because a Free Tickets coupon zeroes attendee rows after net_amount is computed and using it would count that discount twice. Tickets picked for a refund get a Ticket Cancellation Request in the usual review queue rather than being cancelled outright. A custom amount cancels nothing, because it maps to no particular ticket. The gateway confirms refunds separately, over the webhook the payments app dispatches. That handler is keyed on the refund id, since the same event arrives more than once, and it returns nothing so that other apps subscribed to the hook still run. A refund the gateway reports as failed is left out of the total and takes its queued cancellation request out of the queue with it — nobody should accept a cancellation for money that never moved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(refunds): add the Refund dialog to the booking form System Managers get a Refund button on a paid, submitted booking. The dialog lists the booking's tickets with what each one cost; ticking them fills in the amount, and the confirmation says exactly what is about to happen — whether tickets are being queued for cancellation or only money is moving. Toasts cover the send and the result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(refunds): make Event Booking Refund a document of its own A child table could not answer the question that matters — what is still refundable — without the booking loading every row, and it could not be opened, filtered or linked to on its own. Refunds are now documents, named REF-#####, carrying the booking and payment they belong to, with the tickets as links rather than newline-joined text. The booking keeps its summary — refund status and refunded total — and shows the refunds in an HTML table where the refund id links through to the document and the queued cancellation links to its request. Two holes closed with it: `get_refund_summary` replaces `get_refundable_tickets` and answers with the amount left and the tickets not already spoken for. Both the dialog and `refund` read it, so what an operator is offered cannot drift from what the server accepts. Tickets held by a refund the gateway has not refused are no longer offered a second time; a failed refund releases them again. `validate_refund_amount` refuses anything above what is left. A refund still waiting on the gateway holds its money, so a booking of 2000+3000 cannot give back 3000 twice — previously nothing local stopped it, and the gateway cannot be relied on to reject the second one while the first is unsettled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(refunds): link refunds from the booking's Connections Drops the column break in the refund section, and picks up the type hints Frappe regenerates for the new fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(refunds): cancel the tickets once the gateway settles the refund The cancellation request was raised when the refund went out and has been sitting in review since. Nobody needs to approve what the gateway already settled, so a processed refund accepts and submits it, and a failed one rejects it. Cancelling a ticket emails the attendee, which can fail for reasons that have nothing to do with the money. The attempt runs inside a savepoint, so a cancellation that cannot go through is undone whole and left in review, with the refund still recorded. Ticket Cancellation Request now passes its own ignore_permissions down to the documents it cancels, since the webhook job that submits it runs as Guest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(refunds): move the refund button under an Actions group "Refund" alone read like a status next to the booking's own fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(refunds): refuse tickets that belong to another booking refund() forwarded whatever ticket names it was handed straight into a cancellation request. The dialog only ever offers this booking's tickets, but the method is whitelisted, so a direct call could cancel a stranger's ticket and record the refund here. Tickets are now checked against what the booking actually has left to refund, which also refuses one an earlier refund already claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(refunds): cancel tickets only once the gateway has settled The cancellation request was raised the moment a refund went out and sat in review while Razorpay took its time, which can be days. Anyone working that queue could accept it before the answer arrived, and if the refund then failed the tickets were already gone: the failure path skipped a submitted request entirely, so the customer kept neither the money nor the seat. The refund now records which tickets it covers and cancels nothing. The request is raised, and accepted, when the gateway reports the refund processed. A failed refund leaves no request behind at all, so there is nothing for anyone to accept by mistake. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(refunds): say what the refund helpers return The docstrings on the refund helpers had grown into prose that explained the reasoning but never said what the functions hand back. They now open with a one-line summary like the rest of the file, list the keys of the dicts `get_refund_summary` and `get_refundable_tickets` return, and keep the reasoning as comments on the lines it belongs to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(refunds): parse the refund webhook into a model The handler dug the refund out of the webhook body with a chain of `get` calls and then read it as a dict, so every field was a string literal and a typo would have read as a missing value. A pydantic model pulls the four fields we care about straight out of `payload.refund.entity`, and owns the conversion from the gateway's minor unit. A payload that carries no refund now raises instead of returning quietly. Only `refund.processed` and `refund.failed` reach this handler, so such a payload is a defect, and the payments app already logs what we raise and marks the Integration Request failed. It used to be marked completed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(refunds): keep a refunded ticket out of the event A refund that settled could leave its ticket active: cancelling the ticket can fail, and the savepoint around it then rolls the cancellation back and leaves the refund standing. Check-in only turned away a cancelled ticket, so the attendee had their money back and a ticket that still scanned. Refunds now refuse a ticket the front desk has already seen, rather than finding out afterwards when cancelling it raises `LinkExistsError`, and the dialog stops offering those tickets. Check-in turns away any ticket a refund holds, settled or not, so a cancellation that never goes through no longer opens the door. A failed refund releases the ticket again. Cancelling a ticket no longer sends its email inline, so a mail server having a bad day cannot roll back the cancellation. Also fixes an unrelated crash it uncovered: Event Payment is named by autoincrement, so `name` reached `PaymentDetails.name` as an int and check-in raised for every ticket with a received payment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit f4990fe)
Greptile SummaryThis PR adds Desk-initiated Razorpay refunds for event bookings, including webhook reconciliation and ticket cancellation after settlement.
Confidence Score: 4/5The refund initiation path should be made atomic or idempotent before merging because concurrent requests or a post-gateway failure can refund more than the booking total. The balance check only observes persisted refund rows, while the irreversible Razorpay call happens before the new row is recorded, leaving both a concurrency window and an external-success/local-failure window. Files Needing Attention: buzz/ticketing/doctype/event_booking/event_booking.py
|
| Filename | Overview |
|---|---|
| buzz/ticketing/doctype/event_booking/event_booking.py | Implements refund calculation and initiation, but the external refund occurs without atomic balance reservation or idempotency protection. |
| buzz/payments.py | Parses refund notifications, resolves their booking payment, deduplicates records by gateway refund ID, and applies gateway state. |
| buzz/ticketing/doctype/event_booking_refund/event_booking_refund.py | Tracks committed refunds, updates aggregate booking status, and initiates ticket cancellation after successful settlement. |
| buzz/ticketing/doctype/event_booking/event_booking.js | Adds the Desk refund workflow and renders refund history with constrained or escaped persisted values. |
| buzz/api/checkin/services.py | Rejects tickets held by committed refunds and normalizes numeric payment names for the response schema. |
| buzz/ticketing/doctype/ticket_cancellation_request/ticket_cancellation_request.py | Propagates permission bypass to document cancellation so authenticated webhook jobs can complete settled-refund cancellations. |
Prompt To Fix All With AI
### Issue 1
buzz/ticketing/doctype/event_booking/event_booking.py:437
**Refund balance is not reserved**
If two refund requests for one booking overlap, both validate against the same persisted balance and call Razorpay before either refund is recorded, causing the customer to receive more than the booking total. A successful gateway call followed by a failed local insert similarly leaves the balance available for another refund.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(refunds): refund a booking from the..." | Re-trigger Greptile
| tickets = frappe.parse_json(tickets) if isinstance(tickets, str) else tickets | ||
| self.validate_refund(flt(amount), tickets) | ||
| refund = get_controller(payment.payment_gateway).refund_payment(payment.payment_id, flt(amount)) |
There was a problem hiding this comment.
Refund balance is not reserved
If two refund requests for one booking overlap, both validate against the same persisted balance and call Razorpay before either refund is recorded, causing the customer to receive more than the booking total. A successful gateway call followed by a failed local insert similarly leaves the balance available for another refund.
Prompt To Fix With AI
This is a comment left during a code review.
Path: buzz/ticketing/doctype/event_booking/event_booking.py
Line: 437
Comment:
**Refund balance is not reserved**
If two refund requests for one booking overlap, both validate against the same persisted balance and call Razorpay before either refund is recorded, causing the customer to receive more than the booking total. A successful gateway call followed by a failed local insert similarly leaves the balance available for another refund.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Backport of #328. Loom: https://www.loom.com/share/a0bfd08d12c648d3b55adb4144179ac6
The label-driven backport failed to cherry-pick
f4990fe, so this is the same commit applied by hand.Both conflicts were import blocks, and neither was about refunds — they came from teams (#312) being on develop and not here:
buzz/api/checkin/services.py— develop importsNotPermittedandhas_team_access. Kept main's imports and added onlyget_committed_tickets; main'sCheckinServicehas no team gate andbuzz/permissions.pydoes not exist on this branch.buzz/ticketing/doctype/event_booking/event_booking.py— develop importsget_event_team_settings. Dropped it and keptget_controller, the two refund helpers andRAZORPAY; main'ssend_booking_confirmation_emailnever used team settings.Everything else applied clean: both doctype JSONs,
event_booking.js,payments.py,hooks.py, the new doctype folders, and the tests.Before this is any use on a deployed site
frappe/paymentscarrying feat(razorpay): refunds and a signature-verified refund webhook frappe/payments#248 —refund_paymentand the refund webhook. Without it the first refund raisesAttributeErroronget_controller(...).refund_payment.bench migratefor the two new doctypes and the twoallow_on_submitfields on Event Booking.refund.processedandrefund.failed.Verification
Compile and
ruff checkclean on this branch, with no develop-only references left behind. The suites were not re-run locally — the dev site carries develop's schema — so CI is the first real run here. The identical code passes 29 refund / 9 check-in / 40 booking / 28 ticket-API tests on develop.