From 6fcbf6b0f8453e4a55633abd8e78769ac65dd46a Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Mon, 12 Oct 2026 01:51:05 +0530 Subject: [PATCH 1/8] feat(payments)!: take payments and refunds through bwh_payments Buzz now talks to bwh_payments' API instead of frappe/payments, and no longer names a payment provider anywhere in its code. - Payment links: bwh_payments.api.create_payment opens the checkout; the per-gateway branches (Razorpay/Paymob order creation, the payment id key per provider read from Integration Request data) are gone. A currency the gateway cannot charge is refused before a checkout opens, and a disabled gateway is never offered. Event Payment links the Gateway Payment Request. - Confirmation: on_payment_authorized marks the Event Payment received from the confirmed Gateway Payment Request (no more mid-request commit or in_test skip). - Refunds: work for every gateway bwh_payments runs, not only Razorpay. Event Booking Refund links the bwh_payments Gateway Refund; the new on_refund_update callback replaces the frappe/payments-fork handle_refund_notification hook and its Razorpay-shaped payload, and amounts are major units (no more /100). Refunds raised on a provider's dashboard are still recorded, with the tickets they cover. sync_refunds asks the provider about refunds still pending. - Desk refund messages no longer name Razorpay or assume rupees. - Links that pointed at frappe/payments' Payment Gateway now point at bwh_payments' Payment Gateway Profile. - Tests run the real payment and refund code on bwh_payments' in-memory test gateway instead of mocking a controller; the provider-bound factories are gone. - CI installs bwh_payments (v1-stable) instead of frappe/payments. BREAKING CHANGE: sites need bwh_payments installed; frappe/payments is no longer required. --- .github/actions/setup-bench/action.yml | 4 +- buzz/api/events/test_ticket_types.py | 4 +- buzz/api/payments/test_payments.py | 13 +- buzz/api/sponsorships/services.py | 2 +- buzz/api/sponsorships/test_sponsorships.py | 33 ++- .../doctype/buzz_settings/buzz_settings.json | 2 +- .../event_payment_gateway.json | 2 +- buzz/hooks.py | 8 +- buzz/payments.py | 172 ++++---------- buzz/tests/base_test_cases.py | 109 ++++----- buzz/tests/factories/__init__.py | 4 - buzz/tests/factories/event_payment_factory.py | 3 +- .../factories/integration_request_factory.py | 14 -- .../factories/payment_gateway_factory.py | 14 -- .../doctype/event_booking/event_booking.js | 28 +-- .../doctype/event_booking/event_booking.py | 54 ++--- .../test_event_booking_payment.py | 41 ++++ .../test_event_booking_refund.py | 93 ++++---- .../doctype/event_booking/test_refund_sync.py | 213 +++++------------- .../event_booking_refund.json | 9 + .../event_booking_refund.py | 37 ++- .../doctype/event_payment/event_payment.json | 11 +- .../doctype/event_payment/event_payment.py | 1 + dashboard/vite.config.ts | 2 +- pyproject.toml | 2 +- 25 files changed, 353 insertions(+), 522 deletions(-) delete mode 100644 buzz/tests/factories/integration_request_factory.py delete mode 100644 buzz/tests/factories/payment_gateway_factory.py create mode 100644 buzz/ticketing/doctype/event_booking/test_event_booking_payment.py diff --git a/.github/actions/setup-bench/action.yml b/.github/actions/setup-bench/action.yml index cea5c9d2b..708e7f705 100644 --- a/.github/actions/setup-bench/action.yml +++ b/.github/actions/setup-bench/action.yml @@ -81,8 +81,8 @@ runs: env: EXTRA_APP_URL: ${{ inputs.extra-app-url }} run: | - # buzz lists frappe/payments in required_apps, so it must precede install-app. - bench get-app --skip-assets https://github.com/frappe/payments + # buzz lists bwhtech/bwh_payments in required_apps, so it must precede install-app. + bench get-app --skip-assets --branch v1-stable https://github.com/bwhtech/bwh_payments # Test-only: buzz/tests/factories imports frappe_factory_bot. No install-app — # the app ships no DocTypes, and get-app already pip-installs it into the env. diff --git a/buzz/api/events/test_ticket_types.py b/buzz/api/events/test_ticket_types.py index ec2c86290..e5e46ef49 100644 --- a/buzz/api/events/test_ticket_types.py +++ b/buzz/api/events/test_ticket_types.py @@ -5,13 +5,13 @@ from buzz.api.events import get_event_ticket_types from buzz.api.events.exceptions import CannotManageEvent +from buzz.tests.base_test_cases import make_payment_gateway from buzz.tests.factories import ( BuzzEventFactory, BuzzTeamFactory, BuzzTeamMembershipFactory, EventBookingFactory, EventTicketTypeFactory, - PaymentGatewayFactory, UserFactory, ) from buzz.ticketing.doctype.event_ticket_type.event_ticket_type import PAID_EVENTS_FLAG @@ -98,7 +98,7 @@ def test_lists_sales_per_currency(self): self.assertEqual([price["tickets_sold"] for price in row["prices"]], [0, 1]) def test_lists_payment_providers_and_flags_the_default(self): - default = PaymentGatewayFactory.create().name + default = make_payment_gateway() with self.change_settings("Buzz Settings", default_payment_gateway=default): providers = self.payload_as(self.owner)["payment_providers"] diff --git a/buzz/api/payments/test_payments.py b/buzz/api/payments/test_payments.py index 194167aaf..fd4b88287 100644 --- a/buzz/api/payments/test_payments.py +++ b/buzz/api/payments/test_payments.py @@ -3,14 +3,15 @@ from buzz.api.payments import get_event_payment_gateways from buzz.payments import resolve_payment_gateway -from buzz.tests.factories import BuzzEventFactory, PaymentGatewayFactory +from buzz.tests.base_test_cases import make_payment_gateway +from buzz.tests.factories import BuzzEventFactory class TestGetEventPaymentGateways(IntegrationTestCase): def setUp(self): self.enterContext(self.change_settings("Buzz Settings", default_payment_gateway=None)) - self.gateway = PaymentGatewayFactory.create().name - self.other_gateway = PaymentGatewayFactory.create().name + self.gateway = make_payment_gateway() + self.other_gateway = make_payment_gateway() def test_configured_gateways_override_default(self): event = self.create_event(self.gateway) @@ -39,6 +40,12 @@ def test_new_event_keeps_gateways_it_was_created_with(self): self.assertEqual(self.gateways_on(event), [self.other_gateway]) + def test_a_disabled_gateway_is_not_offered(self): + event = self.create_event(self.gateway, self.other_gateway) + frappe.get_doc("Payment Gateway Profile", self.other_gateway).update({"enabled": 0}).save() + + self.assertEqual(get_event_payment_gateways(event), [self.gateway]) + def test_resolve_picks_the_first_gateway_when_none_is_given(self): self.assertEqual(resolve_payment_gateway(self.create_event(self.gateway), None), self.gateway) diff --git a/buzz/api/sponsorships/services.py b/buzz/api/sponsorships/services.py index 4f1806697..dfec650dd 100644 --- a/buzz/api/sponsorships/services.py +++ b/buzz/api/sponsorships/services.py @@ -62,7 +62,7 @@ def payment_link( return get_payment_link_for_sponsorship( self.enquiry_id, tier_id, - f"/b/account/sponsorships/{self.enquiry_id}?success=true", + f"/b/account/sponsorships/{self.enquiry_id}", payment_gateway=payment_gateway, currency=currency, ) diff --git a/buzz/api/sponsorships/test_sponsorships.py b/buzz/api/sponsorships/test_sponsorships.py index 384418f45..a0811aef2 100644 --- a/buzz/api/sponsorships/test_sponsorships.py +++ b/buzz/api/sponsorships/test_sponsorships.py @@ -1,6 +1,5 @@ -from unittest.mock import MagicMock, patch - import frappe +from bwh_payments import testing as payment_testing from buzz.api.sponsorships import ( create_sponsorship_payment_link, @@ -15,8 +14,7 @@ PaymentNotPermitted, WithdrawalNotPermitted, ) -from buzz.tests.base_test_cases import SponsorshipTestCase -from buzz.tests.factories import PaymentGatewayFactory +from buzz.tests.base_test_cases import SponsorshipTestCase, make_payment_gateway class TestGetSponsorshipDetails(SponsorshipTestCase): @@ -120,8 +118,7 @@ def test_link_charges_the_chosen_currency(self): self.tier.save() gateway = self.add_gateway_to_event() - with patch("buzz.payments.get_controller", return_value=MagicMock()): - create_sponsorship_payment_link(self.enquiry.name, self.tier.name, gateway, currency="USD") + create_sponsorship_payment_link(self.enquiry.name, self.tier.name, gateway, currency="USD") payment = frappe.get_last_doc("Event Payment", {"reference_docname": self.enquiry.name}) self.assertEqual((payment.currency, payment.amount), ("USD", 60)) @@ -132,8 +129,30 @@ def test_link_refuses_a_currency_the_tier_has_no_price_in(self): with self.assertRaises(frappe.ValidationError): create_sponsorship_payment_link(self.enquiry.name, self.tier.name, gateway, currency="EUR") + def test_paying_the_link_makes_the_enquirer_a_sponsor(self): + gateway = self.add_gateway_to_event() + create_sponsorship_payment_link(self.enquiry.name, self.tier.name, gateway) + payment = frappe.get_last_doc("Event Payment", {"reference_docname": self.enquiry.name}) + + payment_testing.pay(payment.payment_request) + + self.assertEqual(frappe.db.get_value("Sponsorship Enquiry", self.enquiry.name, "status"), "Paid") + self.assertTrue(frappe.db.exists("Event Sponsor", {"enquiry": self.enquiry.name})) + self.assertEqual(frappe.db.get_value("Event Payment", payment.name, "payment_received"), 1) + + def test_the_sponsor_comes_back_to_their_enquiry_after_paying(self): + gateway = self.add_gateway_to_event() + + create_sponsorship_payment_link(self.enquiry.name, self.tier.name, gateway) + + payment = frappe.get_last_doc("Event Payment", {"reference_docname": self.enquiry.name}) + self.assertEqual( + frappe.db.get_value("Gateway Payment Request", payment.payment_request, "redirect_to"), + f"/b/account/sponsorships/{self.enquiry.name}", + ) + def add_gateway_to_event(self) -> str: - gateway = PaymentGatewayFactory.create().name + gateway = make_payment_gateway() event = frappe.get_doc("Buzz Event", self.event) event.set("payment_gateways", [{"payment_gateway": gateway}]) event.save(ignore_permissions=True) diff --git a/buzz/events/doctype/buzz_settings/buzz_settings.json b/buzz/events/doctype/buzz_settings/buzz_settings.json index 15d8ceac4..2fc0e7b45 100644 --- a/buzz/events/doctype/buzz_settings/buzz_settings.json +++ b/buzz/events/doctype/buzz_settings/buzz_settings.json @@ -107,7 +107,7 @@ "fieldname": "default_payment_gateway", "fieldtype": "Link", "label": "Default Payment Gateway", - "options": "Payment Gateway" + "options": "Payment Gateway Profile" }, { "fieldname": "column_break_ehsr", diff --git a/buzz/events/doctype/event_payment_gateway/event_payment_gateway.json b/buzz/events/doctype/event_payment_gateway/event_payment_gateway.json index 531aaaa4a..f3d202ff4 100644 --- a/buzz/events/doctype/event_payment_gateway/event_payment_gateway.json +++ b/buzz/events/doctype/event_payment_gateway/event_payment_gateway.json @@ -14,7 +14,7 @@ "fieldtype": "Link", "in_list_view": 1, "label": "Payment Gateway", - "options": "Payment Gateway", + "options": "Payment Gateway Profile", "reqd": 1 } ], diff --git a/buzz/hooks.py b/buzz/hooks.py index 5e96b741a..7caae9d5d 100644 --- a/buzz/hooks.py +++ b/buzz/hooks.py @@ -8,7 +8,7 @@ # Apps # ------------------ -required_apps = ["frappe/payments"] +required_apps = ["bwhtech/bwh_payments"] export_python_type_annotations = True @@ -64,12 +64,6 @@ before_tests = "buzz.install.before_tests" -# Payments -# -------- - -# Fired by the payments app when a gateway settles a refund. -handle_refund_notification = "buzz.payments.handle_refund_notification" - doc_events = { "User": { diff --git a/buzz/payments.py b/buzz/payments.py index 0b51b7228..5c8fb49bd 100644 --- a/buzz/payments.py +++ b/buzz/payments.py @@ -1,41 +1,21 @@ import frappe +from bwh_payments import api as payments from frappe import _ -from frappe.utils import flt -from pydantic import AliasPath, BaseModel, ConfigDict, Field -from buzz.ticketing.doctype.event_booking_refund.event_booking_refund import record_gateway_refund +def get_payment_gateways_for_event(event: str) -> list[str]: + """Get the event's own payment gateways, or the site default when it has none. -class RefundNotification(BaseModel): - """The refund a Razorpay `refund.processed` or `refund.failed` webhook carries. - - The fields sit at `payload.refund.entity` in the webhook body, and every one - of them is mandatory: the payments app hands us refund events only, and a - refund event without them is broken rather than uninteresting. + Only enabled gateways are returned, so a gateway switched off in bwh_payments is never offered. """ - - model_config = ConfigDict(extra="ignore") - - refund_id: str = Field(validation_alias=AliasPath("payload", "refund", "entity", "id")) - payment_id: str = Field(validation_alias=AliasPath("payload", "refund", "entity", "payment_id")) - status: str = Field(validation_alias=AliasPath("payload", "refund", "entity", "status")) - amount_in_minor_unit: int = Field(validation_alias=AliasPath("payload", "refund", "entity", "amount")) - - @property - def amount(self) -> float: - """What the gateway refunded, in the major unit the booking is priced in.""" - return flt(self.amount_in_minor_unit) / 100 - - -def get_payment_gateways_for_event(event: str) -> list[str]: - """Get the event's own payment gateways, or the site default when it has none.""" gateways = frappe.get_all( "Event Payment Gateway", filters={"parent": event, "parenttype": "Buzz Event"}, pluck="payment_gateway", ) default = frappe.db.get_single_value("Buzz Settings", "default_payment_gateway") - return gateways or ([default] if default else []) + enabled = set(payments.get_payment_gateways()) + return [gateway for gateway in gateways or [default] if gateway in enabled] def resolve_payment_gateway(event: str, payment_gateway: str | None) -> str: @@ -48,17 +28,10 @@ def resolve_payment_gateway(event: str, payment_gateway: str | None) -> str: return payment_gateway or gateways[0] -def get_controller(payment_gateway): - from payments.utils import get_payment_gateway_controller - - return get_payment_gateway_controller(payment_gateway) - - def get_payment_link_for_booking( booking_id: str, redirect_to: str = "/events", payment_gateway: str | None = None ) -> str: booking_doc = frappe.get_cached_doc("Event Booking", booking_id) - event_title = frappe.get_cached_value("Buzz Event", booking_doc.event, "title") payment_gateway = resolve_payment_gateway(booking_doc.event, payment_gateway) return get_payment_link( "Event Booking", @@ -67,7 +40,6 @@ def get_payment_link_for_booking( booking_doc.currency, payment_gateway, redirect_to=redirect_to, - title=f"Payment for {event_title}", ) @@ -82,7 +54,6 @@ def get_payment_link_for_sponsorship( if not tier_doc.enabled: frappe.throw(_("This sponsorship tier is no longer available.")) payment_gateway = resolve_payment_gateway(tier_doc.event, payment_gateway) - event_title = frappe.get_cached_value("Buzz Event", tier_doc.event, "title") price = tier_doc.price_for(currency) frappe.db.set_value( "Sponsorship Enquiry", sponsorship_enquiry, "tier", sponsorship_tier @@ -95,7 +66,6 @@ def get_payment_link_for_sponsorship( price.currency, payment_gateway, redirect_to, - f"Payment for {tier_doc.title} Sponsorship at {event_title}", ) @@ -106,35 +76,32 @@ def get_payment_link( currency: str, payment_gateway: str, redirect_to: str = "/events", - title: str | None = None, ) -> str: - controller = get_controller(payment_gateway) - # Not every gateway controller implements this check. - if hasattr(controller, "validate_transaction_currency"): - controller.validate_transaction_currency(currency) + """Open a checkout and return its URL. bwh_payments brings the shopper back to `redirect_to`.""" + payments.validate_currency(payment_gateway, currency) payment = record_payment(reference_doctype, reference_docname, amount, currency, payment_gateway) - user_full_name = frappe.get_cached_value("User", frappe.session.user, "full_name") - - payment_details = { - "amount": amount, - "title": title or f"Payment for {reference_doctype}: {reference_docname}", - "description": f"{user_full_name}'s payment for {reference_doctype} (#{reference_docname})", - "reference_doctype": reference_doctype, - "reference_docname": reference_docname, - "payer_email": frappe.session.user, - "payer_name": user_full_name, - "currency": currency, - "payment_gateway": payment_gateway, - "redirect_to": redirect_to, - "payment": payment.name, + payment_request = payments.create_payment( + reference_doctype, + reference_docname, + amount, + currency, + payment_gateway, + redirect_to=redirect_to, + **get_customer_details(), + ) + payment.db_set({"payment_request": payment_request.name, "order_id": payment_request.order_ref}) + return payment_request.order_url + + +def get_customer_details() -> dict: + if frappe.session.user == "Guest": + return {} + first_name, last_name = frappe.get_cached_value("User", frappe.session.user, ["first_name", "last_name"]) + return { + "customer_email": frappe.session.user, + "customer_forenames": first_name, + "customer_surname": last_name, } - if payment_gateway == "Razorpay" or payment_gateway == "Paymob": - order = controller.create_order(**payment_details) - payment_details.update({"order_id": order.get("id")}) - - url = controller.get_payment_url(**payment_details) - - return url def record_payment( @@ -160,82 +127,21 @@ def record_payment( def mark_payment_as_received(reference_doctype: str, reference_docname: str): - if frappe.in_test: - return - - request = frappe.get_all( - "Integration Request", - { - "reference_doctype": reference_doctype, - "reference_docname": reference_docname, - }, - order_by="creation desc", - limit=1, - ) - - if len(request): - data = frappe.db.get_value("Integration Request", request[0].name, "data") - data = frappe.parse_json(data) - - payment_gateway = data.get("payment_gateway") - if payment_gateway == "Razorpay": - payment_id = "razorpay_payment_id" - - elif payment_gateway == "Paymob": - payment_id = "paymob_payment_id" - - elif payment_gateway == "PayPal": - payment_id = "transaction_id" - - elif "Stripe" in payment_gateway: - payment_id = "stripe_token_id" - else: - payment_id = "order_id" - - frappe.db.set_value( - "Event Payment", - data.payment, - { - "payment_received": 1, - "payment_id": data.get(payment_id), - "order_id": data.get("order_id"), - }, - ) - - frappe.db.commit() # nosemgrep: frappe-manual-commit - - -def handle_refund_notification(doctype: str, docname: str) -> None: - """Apply a gateway refund webhook to the booking whose payment it belongs to.""" - # Returns nothing on purpose: `call_hook_method` stops at the first handler - # that returns a value. - payload = frappe.parse_json(frappe.db.get_value(doctype, docname, "data")) - notification = RefundNotification.model_validate(payload) - - payment = frappe.db.get_value( - "Event Payment", - {"payment_id": notification.payment_id}, - ["name", "reference_doctype", "reference_docname"], + """Mark the Event Payment whose checkout bwh_payments has confirmed as received.""" + payment_request = frappe.db.get_value( + "Gateway Payment Request", + {"ref_doctype": reference_doctype, "ref_docname": reference_docname, "status": "Paid"}, + ["name", "gateway_transaction_ref"], as_dict=True, + order_by="creation desc", ) - - if not payment or payment.reference_doctype != "Event Booking": + if not payment_request: return - # The same event arrives more than once, so the refund is keyed on its id. - record_gateway_refund( - booking=payment.reference_docname, - payment=payment.name, - refund_id=notification.refund_id, - status=notification.status, - amount=notification.amount, - ) - frappe.db.set_value( - doctype, - docname, - {"reference_doctype": "Event Booking", "reference_docname": payment.reference_docname}, - update_modified=False, + "Event Payment", + {"payment_request": payment_request.name}, + {"payment_received": 1, "payment_id": payment_request.gateway_transaction_ref}, ) diff --git a/buzz/tests/base_test_cases.py b/buzz/tests/base_test_cases.py index d19d06927..1f0c90065 100644 --- a/buzz/tests/base_test_cases.py +++ b/buzz/tests/base_test_cases.py @@ -1,26 +1,23 @@ -from unittest.mock import MagicMock, patch +from unittest.mock import patch import frappe +from bwh_payments import testing as payment_testing from frappe.tests import IntegrationTestCase from buzz.api.booking.schemas import BookingRequest from buzz.api.booking.services import create_add_on_doc -from buzz.payments import handle_refund_notification +from buzz.payments import get_payment_link_for_booking from buzz.tests.factories import ( BuzzEventFactory, BuzzTeamFactory, EventBookingFactory, - EventPaymentFactory, EventSponsorFactory, EventTicketTypeFactory, - IntegrationRequestFactory, - PaymentGatewayFactory, SponsorshipEnquiryFactory, SponsorshipTierFactory, TicketAddOnFactory, UserFactory, ) -from buzz.ticketing.doctype.event_booking.event_booking import RAZORPAY BOOKER = "booking-owner@example.com" OUTSIDER = "booking-outsider@example.com" @@ -28,7 +25,6 @@ ADD_ON_PRICE = 200 # 500 per ticket with 10% tax on top. CHARGED_PER_TICKET = 550.0 -GATEWAY_CONTROLLER = "buzz.ticketing.doctype.event_booking.event_booking.get_controller" class BookingTestCase(IntegrationTestCase): @@ -123,7 +119,8 @@ def assert_amounts(self, booking, net: int, discount: int, total: int): class BookingRefundTestCase(IntegrationTestCase): - """A paid booking of one ticket per price in `ticket_prices`, on an event with 10% tax.""" + """A booking of one ticket per price in `ticket_prices`, paid through the test gateway, on an event + with 10% tax.""" ticket_prices = (TICKET_PRICE, TICKET_PRICE) attendee_names = ("John", "Jenny") @@ -131,79 +128,73 @@ class BookingRefundTestCase(IntegrationTestCase): @classmethod def setUpClass(cls): super().setUpClass() + # bwh_payments commits each refund before calling the gateway; held here so the class + # rollback still undoes the bookings and refunds. + cls.held_commits = payment_testing.without_commits() + cls.held_commits.__enter__() + cls.gateway = make_payment_gateway() cls.event = cls.create_event() cls.ticket_types = [ EventTicketTypeFactory.create(event=cls.event, prices=[{"currency": "INR", "price": price}]).name for price in cls.ticket_prices ] - cls.gateway = razorpay_gateway() + + @classmethod + def tearDownClass(cls): + cls.held_commits.__exit__(None, None, None) + super().tearDownClass() @classmethod def create_event(cls) -> str: - return BuzzEventFactory.create("with_tax", tax_percentage=10).name + return BuzzEventFactory.create( + "with_tax", tax_percentage=10, payment_gateways=[{"payment_gateway": cls.gateway}] + ).name def setUp(self): self.enterContext(self.set_user("Administrator")) + payment_testing.reset() self.booking = self.create_paid_booking(self.attendee_names, self.ticket_types) - def create_paid_booking(self, names, ticket_types): + def create_booking(self, names, ticket_types): attendees = [ {"ticket_type": ticket_type, "first_name": name, "email": f"{name.lower()}@example.com"} for name, ticket_type in zip(names, ticket_types, strict=True) ] - booking = EventBookingFactory.create(event=self.event, attendees=attendees, payment_status="Paid") - booking.submit() + return EventBookingFactory.create(event=self.event, attendees=attendees) + + def create_paid_booking(self, names, ticket_types): + booking = self.create_booking(names, ticket_types) + get_payment_link_for_booking(booking.name) + # Confirming the payment is what submits the booking. + payment_testing.pay(self.get_payment(booking.name).payment_request) + booking.reload() return booking - def make_payment(self, gateway: str | None = None): - self.payment = EventPaymentFactory.create( - "received", - user=self.booking.user, - amount=self.booking.total_amount, - currency=self.booking.currency, - reference_doctype="Event Booking", - reference_docname=self.booking.name, - payment_gateway=gateway or self.gateway, - ) + def get_payment(self, booking: str | None = None): + return frappe.get_last_doc("Event Payment", {"reference_docname": booking or self.booking.name}) - def initiate_refund(self, amount: float, tickets: list[str] | None = None, refund_id: str | None = None): - client = MagicMock() - client.refund_payment.return_value = { - "id": refund_id or self.refund_id(), - "status": "pending", - "amount": int(amount * 100), - } - with patch(GATEWAY_CONTROLLER, return_value=client): - self.booking.refund(amount=amount, tickets=tickets) - self.booking.reload() - return client - - def send_refund_webhook(self, refund_id: str, status: str, amount: float) -> str: - log = self.create_refund_log(self.refund_payload(refund_id, status, amount)) - self.handle_refund_log(log) - return log - - def refund_payload(self, refund_id: str, status: str, amount: float) -> dict: - entity = { - "id": refund_id, - "status": status, - "amount": int(amount * 100), - "payment_id": self.payment.payment_id, - } - return {"event": f"refund.{status}", "payload": {"refund": {"entity": entity}}} + @property + def payment(self): + return self.get_payment() - def create_refund_log(self, payload: dict) -> str: - return IntegrationRequestFactory.create(data=frappe.as_json(payload)).name + def initiate_refund(self, amount: float, tickets: list[str] | None = None): + """Refund through the gateway, which answers that the refund is pending.""" + payment_testing.set_next_refund_status("Pending") + self.booking.refund(amount=amount, tickets=tickets) + self.booking.reload() - def handle_refund_log(self, log: str) -> None: + def settle_refund(self, refund, status: str = "Processed") -> None: + """The gateway's webhook reports a refund raised from Buzz as Processed or Failed.""" # Cancelling a ticket emails the attendee, and delivery is not under test. with patch("frappe.sendmail"): - handle_refund_notification("Integration Request", log) + payment_testing.send_refund_webhook(refund.gateway_refund, status) self.booking.reload() - def refund_id(self, suffix: str = "1") -> str: - """Refund ids are unique in the database, and rollback is per class.""" - return f"rfnd_{self._testMethodName}_{suffix}" + def refund_outside_buzz(self, amount: float, status: str = "Processed") -> None: + """The gateway's webhook reports a refund raised from its own dashboard.""" + with patch("frappe.sendmail"): + payment_testing.send_external_refund_webhook(self.payment.payment_request, amount, status) + self.booking.reload() def refunds(self) -> list: names = frappe.get_all( @@ -266,8 +257,6 @@ def attendee_rows(ticket_type, count: int) -> list[dict]: ] -def razorpay_gateway() -> str: - """Refunds only go through a gateway named Razorpay, so this name is fixed.""" - if not frappe.db.exists("Payment Gateway", RAZORPAY): - PaymentGatewayFactory.create(gateway=RAZORPAY) - return RAZORPAY +def make_payment_gateway(name: str | None = None) -> str: + """An enabled payment gateway on bwh_payments' in-memory test gateway.""" + return payment_testing.configure_test_gateway(name or f"Gateway {frappe.generate_hash(length=8)}") diff --git a/buzz/tests/factories/__init__.py b/buzz/tests/factories/__init__.py index 29f2e4671..80230b3ce 100644 --- a/buzz/tests/factories/__init__.py +++ b/buzz/tests/factories/__init__.py @@ -21,9 +21,7 @@ from buzz.tests.factories.event_ticket_type_factory import EventTicketTypeFactory from buzz.tests.factories.event_venue_factory import EventVenueFactory from buzz.tests.factories.file_factory import FileFactory -from buzz.tests.factories.integration_request_factory import IntegrationRequestFactory from buzz.tests.factories.offline_payment_method_factory import OfflinePaymentMethodFactory -from buzz.tests.factories.payment_gateway_factory import PaymentGatewayFactory from buzz.tests.factories.speaker_profile_factory import SpeakerProfileFactory from buzz.tests.factories.sponsorship_enquiry_factory import SponsorshipEnquiryFactory from buzz.tests.factories.sponsorship_tier_factory import SponsorshipTierFactory @@ -59,9 +57,7 @@ "EventTicketTypeFactory", "EventVenueFactory", "FileFactory", - "IntegrationRequestFactory", "OfflinePaymentMethodFactory", - "PaymentGatewayFactory", "SpeakerProfileFactory", "SponsorshipEnquiryFactory", "SponsorshipTierFactory", diff --git a/buzz/tests/factories/event_payment_factory.py b/buzz/tests/factories/event_payment_factory.py index 52a9703d8..6bb0eec07 100644 --- a/buzz/tests/factories/event_payment_factory.py +++ b/buzz/tests/factories/event_payment_factory.py @@ -20,5 +20,4 @@ def default_attributes(self) -> dict[str, Any]: @property def received(self) -> dict[str, Any]: - # Webhooks find the payment by its id, so each one is unique. - return {"payment_received": 1, "payment_id": f"pay_{frappe.generate_hash(length=10)}"} + return {"payment_received": 1, "payment_id": f"payment-{frappe.generate_hash(length=10)}"} diff --git a/buzz/tests/factories/integration_request_factory.py b/buzz/tests/factories/integration_request_factory.py deleted file mode 100644 index 39eaaefe4..000000000 --- a/buzz/tests/factories/integration_request_factory.py +++ /dev/null @@ -1,14 +0,0 @@ -from typing import Any - -from frappe.integrations.doctype.integration_request.integration_request import IntegrationRequest -from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory - - -class IntegrationRequestFactory(BaseFactory[IntegrationRequest]): - """A queued Razorpay webhook. Pass the payload as JSON in `data`.""" - - doctype = "Integration Request" - - @property - def default_attributes(self) -> dict[str, Any]: - return {"integration_request_service": "Razorpay", "is_remote_request": 1, "status": "Queued"} diff --git a/buzz/tests/factories/payment_gateway_factory.py b/buzz/tests/factories/payment_gateway_factory.py deleted file mode 100644 index 258bed5eb..000000000 --- a/buzz/tests/factories/payment_gateway_factory.py +++ /dev/null @@ -1,14 +0,0 @@ -from typing import Any - -import frappe -from frappe_factory_bot.frappe_factory_bot.base_factory import BaseFactory -from payments.payments.doctype.payment_gateway.payment_gateway import PaymentGateway - - -class PaymentGatewayFactory(BaseFactory[PaymentGateway]): - doctype = "Payment Gateway" - - @property - def default_attributes(self) -> dict[str, Any]: - # Named after `gateway` and rows outlive a run, so a hash rather than Faker's `unique`. - return {"gateway": f"Gateway {frappe.generate_hash(length=8)}"} diff --git a/buzz/ticketing/doctype/event_booking/event_booking.js b/buzz/ticketing/doctype/event_booking/event_booking.js index c5c186d21..8b32e0715 100644 --- a/buzz/ticketing/doctype/event_booking/event_booking.js +++ b/buzz/ticketing/doctype/event_booking/event_booking.js @@ -124,12 +124,9 @@ async function syncRefunds(frm) { const { message } = await frm.call("sync_refunds") frappe.show_alert({ - message: message.refunds - ? __("Razorpay has {0} refund(s) on this payment, {1} new here.", [ - message.refunds, - message.created, - ]) - : __("Razorpay has no refunds on this payment."), + message: message.pending + ? __("{0} refund(s) are still waiting on the payment provider.", [message.pending]) + : __("Every refund on this booking is settled."), indicator: "blue", }) @@ -144,7 +141,7 @@ async function showRefundDialog(frm) { if (refundable <= 0) { frappe.msgprint({ title: __("Nothing Left to Refund"), - message: __("Every rupee on this booking is already refunded or waiting on the gateway."), + message: __("This booking is already refunded in full, or waiting on the payment provider."), indicator: "orange", }) return @@ -199,18 +196,21 @@ async function showRefundDialog(frm) { frappe.confirm(question, async () => { dialog.hide() frappe.show_alert({ - message: __("Sending the refund to Razorpay..."), + message: __("Sending the refund to the payment provider..."), indicator: "blue", }) - await frm.call("refund", { amount, tickets: selected }) + const { message: refundStatus } = await frm.call("refund", { amount, tickets: selected }) frappe.show_alert({ - message: selected.length - ? __( - "Refund initiated. Razorpay confirms it separately, and the tickets are queued for cancellation review.", - ) - : __("Refund initiated. Razorpay confirms it separately."), + message: + refundStatus !== "Refund Initiated" + ? __("Refund settled.") + : selected.length + ? __( + "Refund initiated. The tickets are cancelled once the payment provider confirms it.", + ) + : __("Refund initiated. The payment provider confirms it separately."), indicator: "green", }) frm.reload_doc() diff --git a/buzz/ticketing/doctype/event_booking/event_booking.py b/buzz/ticketing/doctype/event_booking/event_booking.py index b98bd12bd..74eb1e949 100644 --- a/buzz/ticketing/doctype/event_booking/event_booking.py +++ b/buzz/ticketing/doctype/event_booking/event_booking.py @@ -1,6 +1,7 @@ # Copyright (c) 2025, BWH Studios and contributors # For license information, please see license.txt import frappe +from bwh_payments import api as payments from frappe import _ from frappe.model.document import Document from frappe.utils import cstr, flt @@ -11,7 +12,7 @@ from buzz.emails import is_full_document from buzz.events.doctype.buzz_team_settings.buzz_team_settings import get_event_team_settings from buzz.events.online_meeting import OnlineMeeting -from buzz.payments import get_controller, mark_payment_as_received +from buzz.payments import mark_payment_as_received from buzz.permissions import has_team_access from buzz.ticketing.doctype.event_booking_refund.event_booking_refund import ( get_committed_refunds, @@ -20,8 +21,6 @@ ) from buzz.utils import render_email_template -RAZORPAY = "Razorpay" - class EventBooking(Document): # begin: auto-generated types @@ -513,61 +512,40 @@ def refund(self, amount: float, tickets: list[str] | None = None) -> str: frappe.only_for("System Manager") payment = self.get_received_payment() - if payment.payment_gateway != RAZORPAY: - frappe.throw(_("Refunds are only supported for Razorpay at the moment")) - 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)) - # The picked tickets are only recorded here. They are cancelled once the # gateway settles the refund, since a refund can still fail. A custom # amount maps to no ticket, so it cancels nothing. - frappe.get_doc( + refund = frappe.get_doc( { "doctype": "Event Booking Refund", "booking": self.name, "payment": payment.name, - "refund_id": refund.get("id"), "status": "Initiated", - "amount": flt(refund.get("amount")) / 100, + "amount": flt(amount), "currency": self.currency, "tickets": [{"ticket": ticket} for ticket in tickets or []], } ).insert() + gateway_refund = payments.refund(payment.payment_request, flt(amount), reference=refund.name) + frappe.db.set_value("Event Booking Refund", refund.name, "refund_id", gateway_refund.refund_id) - self.set_refund_status() + return frappe.db.get_value("Event Booking", self.name, "refund_status") - return self.refund_status + def on_refund_update(self, gateway_refund) -> None: + """bwh_payments reports a refund on this booking's payment, from Buzz or the gateway's dashboard.""" + record_gateway_refund(self.name, gateway_refund) @frappe.whitelist() def sync_refunds(self) -> dict: - """Reconcile this booking's refunds against what Razorpay actually holds.""" + """Ask the gateway about every refund on this booking that has not settled yet.""" frappe.only_for("System Manager") - payment = self.get_received_payment() - if payment.payment_gateway != RAZORPAY: - frappe.throw(_("Refunds are only supported for Razorpay at the moment")) - - # Razorpay lists the newest refund first, and each one recorded moves what - # is left to refund, so they are applied in the order they were raised. - refunds = list(reversed(get_controller(payment.payment_gateway).fetch_refunds(payment.payment_id))) - - return { - "refunds": len(refunds), - "created": sum(self.record_refund(payment, refund) for refund in refunds), - } - - def record_refund(self, payment, gateway_refund: dict) -> bool: - """Record one gateway refund against this booking. True when it was new.""" - return record_gateway_refund( - booking=self.name, - payment=payment.name, - refund_id=gateway_refund.get("id"), - status=gateway_refund.get("status"), - amount=flt(gateway_refund.get("amount")) / 100, - ) + payments.sync_refunds(self.get_received_payment().payment_request) + statuses = frappe.get_all("Event Booking Refund", filters={"booking": self.name}, pluck="status") + return {"refunds": len(statuses), "pending": statuses.count("Initiated")} def validate_refund(self, amount: float, tickets: list[str] | None = None) -> None: summary = self.get_refund_summary() @@ -619,12 +597,12 @@ def get_received_payment(self): "reference_docname": self.name, "payment_received": 1, }, - fields=["name", "payment_gateway", "payment_id", "amount"], + fields=["name", "payment_gateway", "payment_request", "amount"], order_by="creation desc", limit=1, ) - if not payment or not payment[0].payment_id: + if not payment or not payment[0].payment_request: frappe.throw(_("No received payment found for this booking")) return payment[0] diff --git a/buzz/ticketing/doctype/event_booking/test_event_booking_payment.py b/buzz/ticketing/doctype/event_booking/test_event_booking_payment.py new file mode 100644 index 000000000..89ae54290 --- /dev/null +++ b/buzz/ticketing/doctype/event_booking/test_event_booking_payment.py @@ -0,0 +1,41 @@ +import frappe + +from buzz.payments import get_payment_link_for_booking +from buzz.tests.base_test_cases import BookingRefundTestCase + + +class TestPayingForABooking(BookingRefundTestCase): + def test_a_confirmed_payment_submits_the_booking(self): + self.assertEqual( + (self.booking.docstatus, self.booking.status, self.booking.payment_status), + (1, "Confirmed", "Paid"), + ) + + def test_a_confirmed_payment_is_recorded_against_the_booking(self): + payment_request = frappe.get_doc("Gateway Payment Request", self.payment.payment_request) + + self.assertEqual(self.payment.payment_received, 1) + self.assertEqual( + (self.payment.payment_id, self.payment.order_id, self.payment.payment_gateway), + (payment_request.gateway_transaction_ref, payment_request.order_ref, self.gateway), + ) + + def test_an_unpaid_booking_stays_a_draft(self): + booking = self.create_booking(("Pending",), self.ticket_types[:1]) + + get_payment_link_for_booking(booking.name) + + booking.reload() + self.assertEqual((booking.docstatus, booking.payment_status), (0, "Unpaid")) + self.assertEqual(self.get_payment(booking.name).payment_received, 0) + + def test_the_shopper_comes_back_to_the_page_buzz_asked_for(self): + booking = self.create_booking(("Returning",), self.ticket_types[:1]) + + get_payment_link_for_booking(booking.name, redirect_to="/b/booking-success/x?token=t") + + payment_request = self.get_payment(booking.name).payment_request + self.assertEqual( + frappe.db.get_value("Gateway Payment Request", payment_request, "redirect_to"), + "/b/booking-success/x?token=t", + ) diff --git a/buzz/ticketing/doctype/event_booking/test_event_booking_refund.py b/buzz/ticketing/doctype/event_booking/test_event_booking_refund.py index 1a4590a6c..414130db5 100644 --- a/buzz/ticketing/doctype/event_booking/test_event_booking_refund.py +++ b/buzz/ticketing/doctype/event_booking/test_event_booking_refund.py @@ -4,11 +4,12 @@ from unittest.mock import patch import frappe +from bwh_payments import testing as payment_testing from buzz.api.checkin import checkin_ticket, validate_ticket_for_checkin from buzz.api.exceptions import Conflict from buzz.tests.base_test_cases import CHARGED_PER_TICKET, BookingRefundTestCase -from buzz.tests.factories import BuzzEventFactory, PaymentGatewayFactory +from buzz.tests.factories import BuzzEventFactory class TestRefundSummary(BookingRefundTestCase): @@ -38,32 +39,48 @@ def test_a_checked_in_ticket_is_not_offered(self): class TestBookingRefund(BookingRefundTestCase): - def test_refund_is_refused_when_the_gateway_is_not_razorpay(self): - self.make_payment(gateway=PaymentGatewayFactory.create().name) - - with self.assertRaises(frappe.ValidationError) as raised: - self.booking.refund(amount=100) - - self.assertIn("Razorpay", str(raised.exception)) - def test_refund_is_refused_without_a_received_payment(self): - self.assertRaises(frappe.ValidationError, self.booking.refund, amount=100) + booking = self.create_booking(("Unpaid",), self.ticket_types[:1]) + booking.submit() - def test_refunding_tickets_records_the_refund_and_marks_it_initiated(self): - self.make_payment() + self.assertRaises(frappe.ValidationError, booking.refund, amount=100) - client = self.initiate_refund(CHARGED_PER_TICKET, tickets=self.refundable_tickets()[:1]) + def test_refunding_tickets_records_the_refund_and_marks_it_initiated(self): + self.initiate_refund(CHARGED_PER_TICKET, tickets=self.refundable_tickets()[:1]) - client.refund_payment.assert_called_once_with(self.payment.payment_id, CHARGED_PER_TICKET) self.assertEqual(self.booking.refund_status, "Refund Initiated") refund = self.refunds()[0] self.assertEqual( - (refund.refund_id, refund.status, refund.amount), (self.refund_id(), "Initiated", 550) + (refund.status, refund.amount, refund.payment), ("Initiated", 550, str(self.payment.name)) ) + gateway_refund = frappe.get_doc("Gateway Refund", refund.gateway_refund) + self.assertEqual((gateway_refund.amount, gateway_refund.reference), (550, refund.name)) + self.assertEqual(refund.refund_id, gateway_refund.refund_id) + + def test_a_refund_the_gateway_settles_at_once_cancels_its_tickets_at_once(self): + ticket = self.refundable_tickets()[0] + payment_testing.set_next_refund_status("Processed") + + with patch("frappe.sendmail"): + self.booking.refund(amount=CHARGED_PER_TICKET, tickets=[ticket]) + + self.booking.reload() + self.assertEqual(self.refunds()[0].status, "Processed") + self.assertEqual(self.booking.refund_status, "Partially Refunded") + self.assertEqual(frappe.db.get_value("Event Ticket", ticket, "docstatus"), 2) + + def test_a_refund_the_gateway_refuses_is_recorded_as_failed(self): + payment_testing.set_next_refund_status("Failed") + + with self.assertRaises(frappe.ValidationError): + self.booking.refund(amount=CHARGED_PER_TICKET) + + self.booking.reload() + self.assertEqual([refund.status for refund in self.refunds()], ["Failed"]) + self.assertEqual(self.booking.get_refund_summary()["remaining"], 1100) def test_an_unsettled_refund_cancels_nothing_yet(self): # A refund can still fail, so tickets are only cancelled once it settles. - self.make_payment() ticket = self.refundable_tickets()[0] self.initiate_refund(CHARGED_PER_TICKET, tickets=[ticket]) @@ -73,11 +90,9 @@ def test_an_unsettled_refund_cancels_nothing_yet(self): self.assertEqual([row.ticket for row in self.refunds()[0].tickets], [ticket]) def test_a_ticket_from_another_booking_is_refused(self): - self.make_payment() other_booking = self.create_paid_booking(("Stranger",), self.ticket_types[:1]) foreign_ticket = frappe.db.get_value("Event Ticket", {"booking": other_booking.name}, "name") - # The gateway is patched, so only the ownership check can refuse this. with self.assertRaises(frappe.ValidationError): self.initiate_refund(CHARGED_PER_TICKET, tickets=[foreign_ticket]) @@ -85,7 +100,6 @@ def test_a_ticket_from_another_booking_is_refused(self): self.assertEqual(frappe.db.get_value("Event Ticket", foreign_ticket, "docstatus"), 1) def test_a_ticket_that_has_been_checked_in_is_refused(self): - self.make_payment() ticket = self.refundable_tickets()[0] checkin_ticket(ticket) @@ -96,7 +110,6 @@ def test_a_ticket_that_has_been_checked_in_is_refused(self): self.assertEqual(self.refunds(), []) def test_a_ticket_a_refund_holds_cannot_be_checked_in(self): - self.make_payment() ticket = self.refundable_tickets()[0] self.initiate_refund(CHARGED_PER_TICKET, tickets=[ticket]) @@ -106,8 +119,6 @@ def test_a_ticket_a_refund_holds_cannot_be_checked_in(self): self.assertEqual(frappe.local.message_log[-1]["title"], "Ticket Refunded") def test_a_custom_amount_refund_cancels_no_tickets(self): - self.make_payment() - self.initiate_refund(100) self.assertIsNone(self.refunds()[0].cancellation_request) @@ -120,21 +131,17 @@ class TestRefundCeiling(BookingRefundTestCase): @classmethod def create_event(cls) -> str: - return BuzzEventFactory.create().name - - def setUp(self): - super().setUp() - self.make_payment() + return BuzzEventFactory.create(payment_gateways=[{"payment_gateway": cls.gateway}]).name def test_the_booking_total_is_the_ceiling(self): self.assertEqual(self.booking.total_amount, 5000) def test_a_second_refund_cannot_exceed_what_is_left_after_a_settled_one(self): self.initiate_refund(3000) - self.settle_refund(self.refund_id(), 3000) + self.settle_refund(self.refunds()[0]) with self.assertRaises(frappe.ValidationError) as raised: - self.initiate_refund(3000, refund_id=self.refund_id("2")) + self.initiate_refund(3000) self.assertIn("2,000", str(raised.exception)) @@ -142,22 +149,22 @@ def test_a_second_refund_cannot_exceed_what_is_left_while_the_first_is_still_ini self.initiate_refund(3000) with self.assertRaises(frappe.ValidationError): - self.initiate_refund(3000, refund_id=self.refund_id("2")) + self.initiate_refund(3000) def test_a_failed_refund_frees_its_amount_again(self): self.initiate_refund(3000) - self.fail_refund(self.refund_id(), 3000) + self.settle_refund(self.refunds()[0], "Failed") - self.initiate_refund(3000, refund_id=self.refund_id("2")) + self.initiate_refund(3000) self.assertEqual(len(self.refunds()), 2) def test_refunding_exactly_what_is_left_is_allowed(self): self.initiate_refund(3000) - self.settle_refund(self.refund_id(), 3000) + self.settle_refund(self.refunds()[0]) - self.initiate_refund(2000, refund_id=self.refund_id("2")) - self.settle_refund(self.refund_id("2"), 2000) + self.initiate_refund(2000) + self.settle_refund(self.refunds()[1]) self.assertEqual(self.booking.refund_status, "Refunded") self.assertEqual(self.booking.refunded_amount, 5000) @@ -165,7 +172,7 @@ def test_refunding_exactly_what_is_left_is_allowed(self): def test_a_ticket_already_refunded_is_not_offered_again(self): low_price_ticket, high_price_ticket = self.refundable_tickets() self.initiate_refund(3000, tickets=[high_price_ticket]) - self.settle_refund(self.refund_id(), 3000) + self.settle_refund(self.refunds()[0]) self.assertEqual(self.refundable_tickets(), [low_price_ticket]) self.assertEqual(self.booking.get_refund_summary()["remaining"], 2000) @@ -174,18 +181,6 @@ def test_a_ticket_whose_refund_failed_is_offered_again(self): high_price_ticket = self.refundable_tickets()[1] self.initiate_refund(3000, tickets=[high_price_ticket]) - self.fail_refund(self.refund_id(), 3000) + self.settle_refund(self.refunds()[0], "Failed") self.assertEqual(len(self.refundable_tickets()), 2) - - def settle_refund(self, refund_id: str, amount: float) -> None: - with patch("frappe.sendmail"): - self.apply_gateway_status(refund_id, "processed", amount) - - def fail_refund(self, refund_id: str, amount: float) -> None: - self.apply_gateway_status(refund_id, "failed", amount) - - def apply_gateway_status(self, refund_id: str, status: str, amount: float) -> None: - refund = frappe.get_doc("Event Booking Refund", {"refund_id": refund_id}) - refund.apply_gateway_status(status, amount) - self.booking.reload() diff --git a/buzz/ticketing/doctype/event_booking/test_refund_sync.py b/buzz/ticketing/doctype/event_booking/test_refund_sync.py index 20fd919bc..c0ad18862 100644 --- a/buzz/ticketing/doctype/event_booking/test_refund_sync.py +++ b/buzz/ticketing/doctype/event_booking/test_refund_sync.py @@ -1,29 +1,19 @@ -from unittest.mock import MagicMock, patch +from unittest.mock import patch import frappe -from pydantic import ValidationError +from bwh_payments import testing as payment_testing from buzz.api.checkin import checkin_ticket -from buzz.payments import handle_refund_notification -from buzz.tests.base_test_cases import ( - CHARGED_PER_TICKET, - GATEWAY_CONTROLLER, - BookingRefundTestCase, -) -from buzz.tests.factories import PaymentGatewayFactory +from buzz.tests.base_test_cases import CHARGED_PER_TICKET, BookingRefundTestCase BOOKING_TOTAL = 1100 -class TestRefundNotification(BookingRefundTestCase): - def setUp(self): - super().setUp() - self.make_payment() - +class TestRefundWebhooks(BookingRefundTestCase): def test_a_processed_refund_covering_the_total_marks_the_booking_refunded(self): self.initiate_refund(BOOKING_TOTAL) - self.send_refund_webhook(self.refund_id(), "processed", BOOKING_TOTAL) + self.settle_refund(self.refunds()[0]) self.assertEqual(self.booking.refund_status, "Refunded") self.assertEqual(self.booking.refunded_amount, BOOKING_TOTAL) @@ -32,30 +22,37 @@ def test_a_processed_refund_covering_the_total_marks_the_booking_refunded(self): def test_a_processed_refund_below_the_total_marks_the_booking_partially_refunded(self): self.initiate_refund(CHARGED_PER_TICKET) - self.send_refund_webhook(self.refund_id(), "processed", CHARGED_PER_TICKET) + self.settle_refund(self.refunds()[0]) self.assertEqual(self.booking.refund_status, "Partially Refunded") self.assertEqual(self.booking.refunded_amount, CHARGED_PER_TICKET) def test_a_full_refund_buzz_never_raised_cancels_every_remaining_ticket(self): - # Raised on the Razorpay dashboard: the webhook carries no tickets. - self.send_refund_webhook(self.refund_id(), "processed", BOOKING_TOTAL) + # Raised on the gateway's dashboard: the webhook carries no tickets. + self.refund_outside_buzz(BOOKING_TOTAL) self.assertEqual(self.booking.refund_status, "Refunded") self.assertEqual(len(self.refunds()[0].tickets), 2) self.assertEqual([request.docstatus for request in self.cancellation_requests()], [1]) def test_a_partial_refund_buzz_never_raised_cancels_nothing(self): - self.send_refund_webhook(self.refund_id(), "processed", CHARGED_PER_TICKET) + self.refund_outside_buzz(CHARGED_PER_TICKET) self.assertEqual(self.refunds()[0].tickets, []) self.assertEqual(self.cancellation_requests(), []) - def test_the_same_event_arriving_twice_is_not_counted_twice(self): + def test_a_refund_buzz_never_raised_still_pending_is_initiated(self): + self.refund_outside_buzz(BOOKING_TOTAL, "Pending") + + self.assertEqual(self.refunds()[0].status, "Initiated") + self.assertEqual(self.booking.refund_status, "Refund Initiated") + self.assertEqual(self.cancellation_requests(), []) + + def test_the_same_outcome_reported_twice_is_not_counted_twice(self): self.initiate_refund(CHARGED_PER_TICKET) - self.send_refund_webhook(self.refund_id(), "processed", CHARGED_PER_TICKET) - self.send_refund_webhook(self.refund_id(), "processed", CHARGED_PER_TICKET) + self.settle_refund(self.refunds()[0]) + self.settle_refund(self.refunds()[0]) self.assertEqual(len(self.refunds()), 1) self.assertEqual(self.booking.refunded_amount, CHARGED_PER_TICKET) @@ -64,20 +61,19 @@ def test_a_failed_refund_is_not_counted_and_cancels_nothing(self): ticket = self.refundable_tickets()[0] self.initiate_refund(CHARGED_PER_TICKET, tickets=[ticket]) - self.send_refund_webhook(self.refund_id(), "failed", CHARGED_PER_TICKET) + self.settle_refund(self.refunds()[0], "Failed") self.assertEqual(self.refunds()[0].status, "Failed") self.assertEqual(self.booking.refunded_amount, 0) self.assertEqual(self.cancellation_requests(), []) self.assertEqual(frappe.db.get_value("Event Ticket", ticket, "docstatus"), 1) - def test_a_webhook_processed_as_guest_still_updates_the_booking(self): - # The webhook is an allow_guest endpoint, so its job runs as Guest. + def test_a_webhook_delivered_as_guest_still_updates_the_booking(self): + # The webhook is an allow_guest endpoint. self.initiate_refund(CHARGED_PER_TICKET, tickets=self.refundable_tickets()[:1]) - log = self.create_refund_log(self.refund_payload(self.refund_id(), "processed", CHARGED_PER_TICKET)) with self.set_user("Guest"): - self.handle_refund_log(log) + self.settle_refund(self.refunds()[0]) self.assertEqual(self.refunds()[0].status, "Processed") self.assertEqual(self.booking.refund_status, "Partially Refunded") @@ -87,7 +83,7 @@ def test_a_processed_refund_raises_and_submits_the_cancellation(self): ticket = self.refundable_tickets()[0] self.initiate_refund(CHARGED_PER_TICKET, tickets=[ticket]) - self.send_refund_webhook(self.refund_id(), "processed", CHARGED_PER_TICKET) + self.settle_refund(self.refunds()[0]) request = frappe.get_doc("Ticket Cancellation Request", self.refunds()[0].cancellation_request) self.assertEqual((request.status, request.docstatus), ("Accepted", 1)) @@ -98,7 +94,7 @@ def test_a_cancellation_that_cannot_go_through_leaves_the_refund_recorded(self): self.initiate_refund(CHARGED_PER_TICKET, tickets=[ticket]) with patch("frappe.sendmail", side_effect=Exception("no outgoing email account")): - self.refunds()[0].apply_gateway_status("processed", CHARGED_PER_TICKET) + payment_testing.send_refund_webhook(self.refunds()[0].gateway_refund, "Processed") self.booking.reload() self.assertEqual(self.refunds()[0].status, "Processed") @@ -107,159 +103,66 @@ def test_a_cancellation_that_cannot_go_through_leaves_the_refund_recorded(self): self.assertEqual((request.status, request.docstatus), ("In Review", 0)) self.assertEqual(frappe.db.get_value("Event Ticket", ticket, "docstatus"), 1) - def test_the_integration_request_is_linked_back_to_the_booking(self): - self.initiate_refund(CHARGED_PER_TICKET) - - log = self.send_refund_webhook(self.refund_id(), "processed", CHARGED_PER_TICKET) - - self.assertEqual( - frappe.db.get_value("Integration Request", log, "reference_docname"), self.booking.name - ) - - def test_a_refund_for_an_unknown_payment_is_ignored(self): - payload = self.refund_payload(self.refund_id(), "processed", CHARGED_PER_TICKET) - payload["payload"]["refund"]["entity"]["payment_id"] = "pay_unknown" - - self.assertIsNone(handle_refund_notification("Integration Request", self.create_refund_log(payload))) - self.assertEqual(self.refunds(), []) - - def test_a_payload_without_a_refund_is_refused(self): - log = self.create_refund_log({"event": "refund.processed", "payload": {}}) - - self.assertRaises(ValidationError, handle_refund_notification, "Integration Request", log) - - -class TestSyncRefunds(BookingRefundTestCase): - def setUp(self): - super().setUp() - self.make_payment() - - def test_a_refund_raised_outside_buzz_is_recorded(self): - result = self.sync(self.gateway_refund("processed", CHARGED_PER_TICKET)) - - self.assertEqual(result, {"refunds": 1, "created": 1}) - self.assertEqual([refund.status for refund in self.refunds()], ["Processed"]) - self.assertEqual(self.booking.refunded_amount, CHARGED_PER_TICKET) - self.assertEqual(self.booking.refund_status, "Partially Refunded") + def test_a_checked_in_ticket_is_never_cancelled_by_a_refund_buzz_never_raised(self): + checked_in_ticket, other_ticket = self.refundable_tickets() + checkin_ticket(checked_in_ticket) - def test_a_payment_with_no_refunds_records_nothing(self): - result = self.sync() + self.refund_outside_buzz(BOOKING_TOTAL) - self.assertEqual(result, {"refunds": 0, "created": 0}) - self.assertEqual(self.refunds(), []) - self.assertEqual(self.booking.refund_status, "") + self.assertEqual([row.ticket for row in self.refunds()[0].tickets], [other_ticket]) - def test_a_full_refund_raised_outside_buzz_cancels_every_remaining_ticket(self): - self.sync(self.gateway_refund("processed", BOOKING_TOTAL)) + def test_two_refunds_buzz_never_raised_cancel_the_tickets_once_they_cover_the_total(self): + # Only the second refund brings the total up to the booking amount. + self.refund_outside_buzz(CHARGED_PER_TICKET) + self.refund_outside_buzz(CHARGED_PER_TICKET) + first, second = self.refunds() + self.assertEqual((first.tickets, len(second.tickets)), ([], 2)) self.assertEqual(self.booking.refund_status, "Refunded") - self.assertEqual(len(self.refunds()[0].tickets), 2) self.assertEqual([request.docstatus for request in self.cancellation_requests()], [1]) - def test_a_partial_refund_raised_outside_buzz_cancels_nothing(self): - self.sync(self.gateway_refund("processed", CHARGED_PER_TICKET)) - - self.assertEqual(self.refunds()[0].tickets, []) - self.assertEqual(self.cancellation_requests(), []) - - def test_a_refund_the_gateway_still_calls_pending_is_initiated(self): - self.sync(self.gateway_refund("pending", BOOKING_TOTAL)) - - self.assertEqual(self.refunds()[0].status, "Initiated") - self.assertEqual(self.booking.refund_status, "Refund Initiated") - self.assertEqual(self.booking.refunded_amount, 0) - self.assertEqual(self.cancellation_requests(), []) - def test_a_refund_buzz_already_holds_is_updated_not_duplicated(self): +class TestSyncRefunds(BookingRefundTestCase): + def test_a_pending_refund_settles_once_the_gateway_reports_it(self): ticket = self.refundable_tickets()[0] self.initiate_refund(CHARGED_PER_TICKET, tickets=[ticket]) + payment_testing.set_refund_status(self.refunds()[0].gateway_refund, "Processed") - result = self.sync(self.gateway_refund("processed", CHARGED_PER_TICKET)) - - self.assertEqual(result, {"refunds": 1, "created": 0}) - self.assertEqual([refund.status for refund in self.refunds()], ["Processed"]) - self.assertEqual([row.ticket for row in self.refunds()[0].tickets], [ticket]) - self.assertEqual(self.booking.refunded_amount, CHARGED_PER_TICKET) - - def test_a_settled_refund_is_not_knocked_back_by_a_stale_pending(self): - self.sync(self.gateway_refund("processed", BOOKING_TOTAL)) - - self.sync(self.gateway_refund("pending", BOOKING_TOTAL)) + result = self.sync() + self.assertEqual(result, {"refunds": 1, "pending": 0}) self.assertEqual(self.refunds()[0].status, "Processed") - self.assertEqual(self.booking.refund_status, "Refunded") - self.assertEqual(self.booking.refunded_amount, BOOKING_TOTAL) - self.assertEqual(len(self.cancellation_requests()), 1) - - def test_a_failed_refund_is_not_revived_by_a_later_sighting(self): - self.sync(self.gateway_refund("failed", CHARGED_PER_TICKET)) - - self.sync(self.gateway_refund("processed", CHARGED_PER_TICKET)) - - self.assertEqual(self.refunds()[0].status, "Failed") - self.assertEqual(self.booking.refunded_amount, 0) - - def test_a_refund_the_webhook_recorded_is_not_duplicated_by_a_sync(self): - self.send_refund_webhook(self.refund_id(), "processed", CHARGED_PER_TICKET) - - result = self.sync(self.gateway_refund("processed", CHARGED_PER_TICKET)) - - self.assertEqual(result, {"refunds": 1, "created": 0}) - self.assertEqual(len(self.refunds()), 1) - self.assertEqual(self.booking.refunded_amount, CHARGED_PER_TICKET) - - def test_syncing_twice_does_not_count_the_same_refund_twice(self): - refund = self.gateway_refund("processed", CHARGED_PER_TICKET) - - self.sync(refund) - self.sync(refund) - - self.assertEqual(len(self.refunds()), 1) self.assertEqual(self.booking.refunded_amount, CHARGED_PER_TICKET) + self.assertEqual(frappe.db.get_value("Event Ticket", ticket, "docstatus"), 2) - def test_a_checked_in_ticket_is_never_cancelled_by_a_synced_refund(self): - checked_in_ticket, other_ticket = self.refundable_tickets() - checkin_ticket(checked_in_ticket) - - self.sync(self.gateway_refund("processed", BOOKING_TOTAL)) + def test_a_refund_the_gateway_failed_frees_its_tickets(self): + self.initiate_refund(CHARGED_PER_TICKET, tickets=self.refundable_tickets()[:1]) + payment_testing.set_refund_status(self.refunds()[0].gateway_refund, "Failed") - self.assertEqual([row.ticket for row in self.refunds()[0].tickets], [other_ticket]) + self.sync() - def test_two_refunds_in_one_batch_cancel_the_tickets_once_they_cover_the_total(self): - # Only the second refund brings the total up to the booking amount. - self.sync( - self.gateway_refund("processed", CHARGED_PER_TICKET, suffix="1"), - self.gateway_refund("processed", CHARGED_PER_TICKET, suffix="2"), - ) + self.assertEqual(self.refunds()[0].status, "Failed") + self.assertEqual(len(self.refundable_tickets()), 2) - first, second = self.refunds() - self.assertEqual((first.tickets, len(second.tickets)), ([], 2)) - self.assertEqual(self.booking.refund_status, "Refunded") - self.assertEqual([request.docstatus for request in self.cancellation_requests()], [1]) + def test_a_refund_the_gateway_still_holds_stays_initiated(self): + self.initiate_refund(CHARGED_PER_TICKET) - def test_sync_is_refused_when_the_gateway_is_not_razorpay(self): - other_gateway = PaymentGatewayFactory.create().name - frappe.db.set_value("Event Payment", self.payment.name, "payment_gateway", other_gateway) + result = self.sync() - with self.assertRaises(frappe.ValidationError) as raised: - self.booking.sync_refunds() + self.assertEqual(result, {"refunds": 1, "pending": 1}) + self.assertEqual(self.refunds()[0].status, "Initiated") - self.assertIn("Razorpay", str(raised.exception)) + def test_a_payment_with_no_refunds_changes_nothing(self): + self.assertEqual(self.sync(), {"refunds": 0, "pending": 0}) + self.assertEqual(self.booking.refund_status, "") def test_sync_is_refused_without_a_received_payment(self): frappe.db.set_value("Event Payment", self.payment.name, "payment_received", 0) self.assertRaises(frappe.ValidationError, self.booking.sync_refunds) - def gateway_refund(self, status: str, amount: float, suffix: str = "1") -> dict: - return {"id": self.refund_id(suffix), "status": status, "amount": int(amount * 100)} - - def sync(self, *gateway_refunds: dict) -> dict: - client = MagicMock() - client.fetch_refunds.return_value = list(gateway_refunds) - - with patch(GATEWAY_CONTROLLER, return_value=client), patch("frappe.sendmail"): + def sync(self) -> dict: + with patch("frappe.sendmail"): result = self.booking.sync_refunds() - self.booking.reload() return result diff --git a/buzz/ticketing/doctype/event_booking_refund/event_booking_refund.json b/buzz/ticketing/doctype/event_booking_refund/event_booking_refund.json index 029ff7d54..0d74f6a05 100644 --- a/buzz/ticketing/doctype/event_booking_refund/event_booking_refund.json +++ b/buzz/ticketing/doctype/event_booking_refund/event_booking_refund.json @@ -10,6 +10,7 @@ "booking", "payment", "column_break_refund", + "gateway_refund", "refund_id", "status", "amounts_section", @@ -42,6 +43,14 @@ "fieldname": "column_break_refund", "fieldtype": "Column Break" }, + { + "fieldname": "gateway_refund", + "fieldtype": "Link", + "label": "Gateway Refund", + "options": "Gateway Refund", + "read_only": 1, + "unique": 1 + }, { "description": "Identifier the payment gateway gave this refund.", "fieldname": "refund_id", diff --git a/buzz/ticketing/doctype/event_booking_refund/event_booking_refund.py b/buzz/ticketing/doctype/event_booking_refund/event_booking_refund.py index a3cf43563..6e36ccc04 100644 --- a/buzz/ticketing/doctype/event_booking_refund/event_booking_refund.py +++ b/buzz/ticketing/doctype/event_booking_refund/event_booking_refund.py @@ -14,7 +14,8 @@ # What the gateway settled on. Anything else is still in flight. FINAL_STATUSES = ("Processed", "Failed") -GATEWAY_STATUSES = {"processed": "Processed", "failed": "Failed"} +# bwh_payments refund statuses to this refund's. +GATEWAY_STATUSES = {"Pending": "Initiated", "Processed": "Processed", "Failed": "Failed"} class EventBookingRefund(Document): @@ -34,6 +35,7 @@ class EventBookingRefund(Document): booking: DF.Link cancellation_request: DF.Link | None currency: DF.Link | None + gateway_refund: DF.Link | None payment: DF.Link | None refund_id: DF.Data | None status: DF.Literal["Initiated", "Processed", "Failed"] @@ -100,20 +102,18 @@ def capture_processed(self, booking): ) -def record_gateway_refund( - booking: str, payment: str | None, refund_id: str, status: str, amount: float -) -> bool: - """Create or update the refund `refund_id` names. True when it was new. +def record_gateway_refund(booking: str, gateway_refund) -> bool: + """Create or update the refund behind a bwh_payments Gateway Refund. True when it was new. - Used for refunds the gateway reports, from the webhook or a sync — never for - one raised from the Desk, which records the tickets an operator picked. + A refund raised from the Desk comes back with its name as `reference`. One raised on the + gateway's dashboard has none, so the tickets it covers are worked out here. """ # The webhook job and a Desk sync can both be recording refunds on this # booking. Serialising them here is what lets the reads below see every # refund that came before, rather than a picture from an instant ago. frappe.db.get_value("Event Booking", booking, "name", for_update=True) - existing = frappe.db.exists("Event Booking Refund", {"refund_id": refund_id}) + existing = find_refund(booking, gateway_refund) if existing: refund = frappe.get_doc("Event Booking Refund", existing) else: @@ -121,18 +121,31 @@ def record_gateway_refund( { "doctype": "Event Booking Refund", "booking": booking, - "payment": payment, - "refund_id": refund_id, + "payment": frappe.db.get_value( + "Event Payment", {"payment_request": gateway_refund.payment_request}, "name" + ), "currency": frappe.db.get_value("Event Booking", booking, "currency"), - "tickets": [{"ticket": ticket} for ticket in tickets_a_refund_covers(booking, amount)], + "tickets": [ + {"ticket": ticket} for ticket in tickets_a_refund_covers(booking, gateway_refund.amount) + ], } ).insert(ignore_permissions=True) - refund.apply_gateway_status(gateway_status=status, amount=amount) + refund.gateway_refund = gateway_refund.name + refund.refund_id = gateway_refund.refund_id or refund.refund_id + refund.apply_gateway_status(gateway_status=gateway_refund.status, amount=gateway_refund.amount) return not existing +def find_refund(booking: str, gateway_refund) -> str | None: + if gateway_refund.reference and frappe.db.exists( + "Event Booking Refund", {"name": gateway_refund.reference, "booking": booking} + ): + return gateway_refund.reference + return frappe.db.exists("Event Booking Refund", {"gateway_refund": gateway_refund.name}) + + def tickets_a_refund_covers(booking: str, amount: float) -> list[str]: """The tickets a refund raised outside Buzz pays back for: what is left, or nothing.""" # Read before the refund is recorded: `remaining` is already net of every diff --git a/buzz/ticketing/doctype/event_payment/event_payment.json b/buzz/ticketing/doctype/event_payment/event_payment.json index 0cb46c009..ef4a02565 100644 --- a/buzz/ticketing/doctype/event_payment/event_payment.json +++ b/buzz/ticketing/doctype/event_payment/event_payment.json @@ -15,6 +15,7 @@ "payment_received", "section_break_mcjj", "payment_gateway", + "payment_request", "column_break_oauu", "payment_id", "order_id" @@ -87,7 +88,15 @@ "fieldname": "payment_gateway", "fieldtype": "Link", "label": "Payment Gateway", - "options": "Payment Gateway" + "options": "Payment Gateway Profile" + }, + { + "fieldname": "payment_request", + "fieldtype": "Link", + "label": "Payment Request", + "options": "Gateway Payment Request", + "read_only": 1, + "search_index": 1 } ], "grid_page_length": 50, diff --git a/buzz/ticketing/doctype/event_payment/event_payment.py b/buzz/ticketing/doctype/event_payment/event_payment.py index 200ed0ae8..914eb9cf0 100644 --- a/buzz/ticketing/doctype/event_payment/event_payment.py +++ b/buzz/ticketing/doctype/event_payment/event_payment.py @@ -19,6 +19,7 @@ class EventPayment(Document): name: DF.Int | None order_id: DF.Data | None payment_gateway: DF.Link | None + payment_request: DF.Link | None payment_id: DF.Data | None payment_received: DF.Check reference_docname: DF.DynamicLink | None diff --git a/dashboard/vite.config.ts b/dashboard/vite.config.ts index 47e549110..93779d2f8 100644 --- a/dashboard/vite.config.ts +++ b/dashboard/vite.config.ts @@ -9,7 +9,7 @@ export default defineConfig({ frappeui({ frappeProxy: { port: 8080, - source: "^/(app|login|api|assets|files|private|razorpay_checkout|events|community)", + source: "^/(app|login|api|assets|files|private|events|community)", }, jinjaBootData: true, lucideIcons: true, diff --git a/pyproject.toml b/pyproject.toml index 3c528f838..a85325211 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,4 +61,4 @@ indent-style = "tab" [tool.bench.frappe-dependencies] frappe = ">=16.0.0,<18.0.0" -payments = ">=0.0.1,<1.0.0" \ No newline at end of file +bwh_payments = ">=0.0.1,<1.0.0" \ No newline at end of file From 3f1a0422a8e666a671c73d4211d9abedde28bc17 Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Mon, 12 Oct 2026 01:52:48 +0530 Subject: [PATCH 2/8] feat(payments): move payment history from frappe/payments to bwh_payments A post-model-sync patch gives every existing payment and refund a bwh_payments record, so old bookings stay refundable and their history stays readable. It keeps every existing field; rows only gain a link. - Gateways: bwh_payments imports each frappe/payments Payment Gateway as a Payment Gateway Profile with the same name (copying Razorpay/Stripe keys), so the gateway links on events, templates, settings and payments stay valid. A gateway no longer listed anywhere gets a disabled profile. - Event Payment: a Gateway Payment Request per row, Paid when received and Expired otherwise, keyed on order_id/payment_id, with payment_id as the transaction ref (old Razorpay payments refund against it). - Event Booking Refund: a Gateway Refund per row with its refund id, status and amount, referencing the refund. Nothing is settled again: no booking is resubmitted and no ticket is cancelled. - Idempotent, and a row that cannot move (e.g. its booking was deleted) is logged and skipped without stopping the migration. frappe/payments must still be installed when this runs; uninstall it in a later update. --- buzz/patches.txt | 1 + buzz/patches/move_payments_to_bwh_payments.py | 76 ++++++++++ .../test_move_payments_to_bwh_payments.py | 137 ++++++++++++++++++ 3 files changed, 214 insertions(+) create mode 100644 buzz/patches/move_payments_to_bwh_payments.py create mode 100644 buzz/patches/test_move_payments_to_bwh_payments.py diff --git a/buzz/patches.txt b/buzz/patches.txt index 5564abf22..06bff81eb 100644 --- a/buzz/patches.txt +++ b/buzz/patches.txt @@ -32,3 +32,4 @@ buzz.patches.create_administrator_team buzz.patches.geocode_event_venues buzz.patches.rename_is_a_community_field buzz.patches.migrate_desk_tags_to_buzz_tags +buzz.patches.move_payments_to_bwh_payments diff --git a/buzz/patches/move_payments_to_bwh_payments.py b/buzz/patches/move_payments_to_bwh_payments.py new file mode 100644 index 000000000..d66e63ae7 --- /dev/null +++ b/buzz/patches/move_payments_to_bwh_payments.py @@ -0,0 +1,76 @@ +import frappe +from bwh_payments.api import record_external_payment, record_external_refund +from bwh_payments.migrate.frappe_payments import import_gateways + + +def execute(): + PaymentHistoryMigration().run() + + +class PaymentHistoryMigration: + """Give every payment and refund taken through frappe/payments a bwh_payments record. + + Event Payment and Event Booking Refund rows keep every field they had; they only gain a link. + """ + + def run(self): + import_gateways() + for payment in frappe.get_all( + "Event Payment", + filters={"payment_request": ("is", "not set"), "payment_gateway": ("is", "set")}, + fields=["*"], + order_by="creation asc", + ): + self.import_row(self.import_payment, payment) + + for refund in frappe.get_all( + "Event Booking Refund", + filters={"gateway_refund": ("is", "not set"), "refund_id": ("is", "set")}, + fields=["name", "payment", "refund_id", "status", "amount"], + order_by="creation asc", + ): + self.import_row(self.import_refund, refund) + + def import_row(self, importer, row): + # One unreadable row, such as a payment for a deleted booking, must not stop the site migrating. + frappe.db.savepoint("move_payment_row") + try: + importer(row) + except Exception: + frappe.db.rollback(save_point="move_payment_row") + frappe.log_error(title=f"Could not move payment record {row.name} to bwh_payments") + + def import_payment(self, payment): + self.keep_gateway(payment.payment_gateway) + payment_request = record_external_payment( + payment.reference_doctype, + payment.reference_docname, + payment.amount, + payment.currency, + payment.payment_gateway, + "Paid" if payment.payment_received else "Expired", + payment.order_id or payment.payment_id or f"legacy-{payment.name}", + transaction_ref=payment.payment_id, + ) + frappe.db.set_value( + "Event Payment", payment.name, "payment_request", payment_request.name, update_modified=False + ) + + def import_refund(self, refund): + payment_request = frappe.db.get_value("Event Payment", refund.payment, "payment_request") + if not payment_request: + return + + gateway_refund = record_external_refund( + payment_request, refund.refund_id, refund.status, refund.amount, reference=refund.name + ) + frappe.db.set_value( + "Event Booking Refund", refund.name, "gateway_refund", gateway_refund.name, update_modified=False + ) + + def keep_gateway(self, gateway: str): + """A gateway frappe/payments no longer lists is kept, disabled, so its payments still name it.""" + if not frappe.db.exists("Payment Gateway Profile", gateway): + frappe.get_doc({"doctype": "Payment Gateway Profile", "name": gateway, "enabled": 0}).insert( + ignore_permissions=True + ) diff --git a/buzz/patches/test_move_payments_to_bwh_payments.py b/buzz/patches/test_move_payments_to_bwh_payments.py new file mode 100644 index 000000000..6f361357e --- /dev/null +++ b/buzz/patches/test_move_payments_to_bwh_payments.py @@ -0,0 +1,137 @@ +import frappe +from frappe.tests import IntegrationTestCase + +from buzz.patches.move_payments_to_bwh_payments import PaymentHistoryMigration +from buzz.tests.base_test_cases import make_payment_gateway +from buzz.tests.factories import EventBookingFactory, EventBookingRefundFactory, EventPaymentFactory + + +class TestMovePaymentsToBwhPayments(IntegrationTestCase): + def setUp(self): + self.gateway = make_payment_gateway() + self.booking = EventBookingFactory.create() + self.booking.submit() + self.paid = self.make_legacy_payment( + "received", payment_id="pay_legacy_paid", order_id="order_legacy_paid", amount=1000 + ) + self.unpaid = self.make_legacy_payment(amount=1000) + self.refund = EventBookingRefundFactory.create( + booking=self.booking.name, + payment=self.paid.name, + refund_id="rfnd_legacy_1", + status="Processed", + amount=400, + currency="INR", + ) + + def tearDown(self): + # The migration reads every payment on the site, so one test's rows must not reach the next. + frappe.db.rollback() + + def make_legacy_payment(self, *traits, **values): + return EventPaymentFactory.create( + *traits, + reference_doctype="Event Booking", + reference_docname=self.booking.name, + payment_gateway=self.gateway, + currency="INR", + **values, + ) + + def get_payment_request(self, payment): + name = frappe.db.get_value("Event Payment", payment.name, "payment_request") + return frappe.get_doc("Gateway Payment Request", name) + + def test_a_received_payment_becomes_a_settled_payment_request_with_its_refunds(self): + PaymentHistoryMigration().run() + + payment_request = self.get_payment_request(self.paid) + self.assertEqual( + (payment_request.status, payment_request.amount, payment_request.refund_amount), + ("Partially Refunded", 1000, 400), + ) + self.assertEqual( + (payment_request.order_ref, payment_request.gateway_transaction_ref, payment_request.gateway), + ("order_legacy_paid", "pay_legacy_paid", self.gateway), + ) + self.assertEqual( + (payment_request.ref_doctype, payment_request.ref_docname), ("Event Booking", self.booking.name) + ) + + def test_an_unpaid_payment_is_kept_as_an_expired_payment_request(self): + PaymentHistoryMigration().run() + + payment_request = self.get_payment_request(self.unpaid) + self.assertEqual(payment_request.status, "Expired") + self.assertEqual(payment_request.order_ref, f"legacy-{self.unpaid.name}") + + def test_a_refund_is_linked_to_its_gateway_refund_and_keeps_its_own_record(self): + PaymentHistoryMigration().run() + + self.refund.reload() + gateway_refund = frappe.get_doc("Gateway Refund", self.refund.gateway_refund) + self.assertEqual( + ( + gateway_refund.refund_id, + gateway_refund.status, + gateway_refund.amount, + gateway_refund.reference, + ), + ("rfnd_legacy_1", "Processed", 400, self.refund.name), + ) + self.assertEqual((self.refund.refund_id, self.refund.status), ("rfnd_legacy_1", "Processed")) + + def test_the_legacy_payment_fields_are_left_as_they_were(self): + PaymentHistoryMigration().run() + + self.assertEqual( + frappe.db.get_value( + "Event Payment", self.paid.name, ["payment_received", "payment_id", "order_id"], as_dict=True + ), + {"payment_received": 1, "payment_id": "pay_legacy_paid", "order_id": "order_legacy_paid"}, + ) + + def test_running_it_again_creates_nothing_new(self): + PaymentHistoryMigration().run() + payment_requests = frappe.db.count("Gateway Payment Request") + gateway_refunds = frappe.db.count("Gateway Refund") + + PaymentHistoryMigration().run() + + self.assertEqual(frappe.db.count("Gateway Payment Request"), payment_requests) + self.assertEqual(frappe.db.count("Gateway Refund"), gateway_refunds) + + def test_a_gateway_that_no_longer_exists_is_kept_for_history_but_disabled(self): + retired = self.make_legacy_payment("received", amount=10) + # Set past the Link check: the gateway was deleted along with frappe/payments' record of it. + frappe.db.set_value("Event Payment", retired.name, "payment_gateway", "Retired Gateway") + + PaymentHistoryMigration().run() + + self.assertEqual(frappe.db.get_value("Payment Gateway Profile", "Retired Gateway", "enabled"), 0) + self.assertEqual(self.get_payment_request(retired).gateway, "Retired Gateway") + + def test_importing_settles_nothing_again(self): + PaymentHistoryMigration().run() + + self.assertEqual(frappe.db.count("Ticket Cancellation Request", {"booking": self.booking.name}), 0) + self.assertEqual(frappe.db.get_value("Event Booking", self.booking.name, "docstatus"), 1) + + def test_a_payment_for_a_deleted_booking_is_skipped_and_logged_without_stopping_the_rest(self): + orphan_booking = EventBookingFactory.create() + orphan = EventPaymentFactory.create( + "received", + reference_doctype="Event Booking", + reference_docname=orphan_booking.name, + payment_gateway=self.gateway, + currency="INR", + amount=10, + ) + frappe.db.delete("Event Booking", orphan_booking.name) + errors_before = frappe.db.count("Error Log") + + PaymentHistoryMigration().run() + + self.assertIsNone(frappe.db.get_value("Event Payment", orphan.name, "payment_request")) + self.assertTrue(frappe.db.get_value("Event Payment", self.paid.name, "payment_request")) + self.assertEqual(frappe.db.count("Error Log"), errors_before + 1) From 0860fbb679588c9c54b47adcd9cd2fa5c1cd3de3 Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Mon, 12 Oct 2026 01:54:40 +0530 Subject: [PATCH 3/8] feat(dashboard): payment logos from bwh_payments, and success only once paid - Gateway logos now come from bwh_payments through the new guest endpoint buzz.api.payments.get_payment_gateway_logos. The dashboard's provider-keyed logo map (and its "Stripe-Main" name parsing) and buzz's copy of the Razorpay logos are gone, so any gateway bwh_payments runs shows its logo. - The booking confirmation page said "Booking Confirmed" and fired confetti for anyone who landed on it. It now confirms only a paid (or free) booking; otherwise it shows "Payment Pending" with a "Check again" button. - The sponsorship page celebrates a return from payment only once the enquiry is actually Paid, instead of on ?success=true alone. --- buzz/api/payments/__init__.py | 13 ++++++++++- buzz/api/payments/schemas.py | 6 +++++ buzz/api/payments/test_payments.py | 18 ++++++++++++++- buzz/api/sponsorships/services.py | 2 +- buzz/api/sponsorships/test_sponsorships.py | 2 +- .../images/payment_gateways/razorpay-dark.svg | 21 ------------------ .../images/payment_gateways/razorpay.svg | 21 ------------------ .../src/components/PaymentGatewayLogo.vue | 9 ++++---- .../src/composables/usePaymentSuccess.ts | 12 ++++++++-- dashboard/src/data/paymentGatewayLogos.ts | 11 ++++++++++ dashboard/src/pages/BookingSuccess.vue | 22 ++++++++++++++++++- dashboard/src/pages/SponsorshipDetails.vue | 15 ++++++------- dashboard/src/types.ts | 6 +++++ dashboard/src/utils/paymentGateways.test.ts | 22 ------------------- dashboard/src/utils/paymentGateways.ts | 18 --------------- 15 files changed, 97 insertions(+), 101 deletions(-) delete mode 100644 buzz/public/images/payment_gateways/razorpay-dark.svg delete mode 100644 buzz/public/images/payment_gateways/razorpay.svg create mode 100644 dashboard/src/data/paymentGatewayLogos.ts delete mode 100644 dashboard/src/utils/paymentGateways.test.ts delete mode 100644 dashboard/src/utils/paymentGateways.ts diff --git a/buzz/api/payments/__init__.py b/buzz/api/payments/__init__.py index f8f27fcd5..56ed091a7 100644 --- a/buzz/api/payments/__init__.py +++ b/buzz/api/payments/__init__.py @@ -1,7 +1,8 @@ import frappe +from bwh_payments.api import get_gateway_options from buzz.api.payments.currencies import enabled_currencies -from buzz.api.payments.schemas import CurrencyItem +from buzz.api.payments.schemas import CurrencyItem, PaymentGatewayLogo from buzz.payments import get_payment_gateways_for_event @@ -14,3 +15,13 @@ def get_event_payment_gateways(event: str) -> list[str]: @frappe.whitelist(allow_guest=True) def get_enabled_currencies() -> list[CurrencyItem]: return [CurrencyItem(**currency) for currency in enabled_currencies()] + + +# nosemgrep: guest-whitelisted-method +@frappe.whitelist(allow_guest=True) +def get_payment_gateway_logos() -> list[PaymentGatewayLogo]: + return [ + PaymentGatewayLogo(name=option.name, logo=option.logo, logo_dark=option.logo_dark) + for option in get_gateway_options() + if option.logo + ] diff --git a/buzz/api/payments/schemas.py b/buzz/api/payments/schemas.py index 12f7ab376..d24874255 100644 --- a/buzz/api/payments/schemas.py +++ b/buzz/api/payments/schemas.py @@ -5,3 +5,9 @@ class CurrencyItem(APIResponse): name: str symbol: str | None number_format: str | None + + +class PaymentGatewayLogo(APIResponse): + name: str + logo: str + logo_dark: str diff --git a/buzz/api/payments/test_payments.py b/buzz/api/payments/test_payments.py index fd4b88287..5789548c8 100644 --- a/buzz/api/payments/test_payments.py +++ b/buzz/api/payments/test_payments.py @@ -1,7 +1,7 @@ import frappe from frappe.tests import IntegrationTestCase -from buzz.api.payments import get_event_payment_gateways +from buzz.api.payments import get_event_payment_gateways, get_payment_gateway_logos from buzz.payments import resolve_payment_gateway from buzz.tests.base_test_cases import make_payment_gateway from buzz.tests.factories import BuzzEventFactory @@ -70,3 +70,19 @@ def create_event(self, *gateways: str) -> str: def gateways_on(self, event: str) -> list[str]: return [row.payment_gateway for row in frappe.get_doc("Buzz Event", event).payment_gateways] + + +class TestGetPaymentGatewayLogos(IntegrationTestCase): + def test_every_enabled_gateway_comes_with_its_logo_for_both_themes(self): + gateway = make_payment_gateway() + + logos = {row.name: row for row in get_payment_gateway_logos()} + + self.assertTrue(logos[gateway].logo.startswith("/assets/bwh_payments/")) + self.assertTrue(logos[gateway].logo_dark.startswith("/assets/bwh_payments/")) + + def test_a_disabled_gateway_has_no_logo_listed(self): + gateway = make_payment_gateway() + frappe.get_doc("Payment Gateway Profile", gateway).update({"enabled": 0}).save() + + self.assertNotIn(gateway, [row.name for row in get_payment_gateway_logos()]) diff --git a/buzz/api/sponsorships/services.py b/buzz/api/sponsorships/services.py index dfec650dd..4f1806697 100644 --- a/buzz/api/sponsorships/services.py +++ b/buzz/api/sponsorships/services.py @@ -62,7 +62,7 @@ def payment_link( return get_payment_link_for_sponsorship( self.enquiry_id, tier_id, - f"/b/account/sponsorships/{self.enquiry_id}", + f"/b/account/sponsorships/{self.enquiry_id}?success=true", payment_gateway=payment_gateway, currency=currency, ) diff --git a/buzz/api/sponsorships/test_sponsorships.py b/buzz/api/sponsorships/test_sponsorships.py index a0811aef2..22bb6e7eb 100644 --- a/buzz/api/sponsorships/test_sponsorships.py +++ b/buzz/api/sponsorships/test_sponsorships.py @@ -148,7 +148,7 @@ def test_the_sponsor_comes_back_to_their_enquiry_after_paying(self): payment = frappe.get_last_doc("Event Payment", {"reference_docname": self.enquiry.name}) self.assertEqual( frappe.db.get_value("Gateway Payment Request", payment.payment_request, "redirect_to"), - f"/b/account/sponsorships/{self.enquiry.name}", + f"/b/account/sponsorships/{self.enquiry.name}?success=true", ) def add_gateway_to_event(self) -> str: diff --git a/buzz/public/images/payment_gateways/razorpay-dark.svg b/buzz/public/images/payment_gateways/razorpay-dark.svg deleted file mode 100644 index 3eb077483..000000000 --- a/buzz/public/images/payment_gateways/razorpay-dark.svg +++ /dev/null @@ -1,21 +0,0 @@ - - - - Group - Created with Sketch. - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/buzz/public/images/payment_gateways/razorpay.svg b/buzz/public/images/payment_gateways/razorpay.svg deleted file mode 100644 index 84153de77..000000000 --- a/buzz/public/images/payment_gateways/razorpay.svg +++ /dev/null @@ -1,21 +0,0 @@ - - - - Group - Created with Sketch. - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/dashboard/src/components/PaymentGatewayLogo.vue b/dashboard/src/components/PaymentGatewayLogo.vue index a4ec0b1a3..d1fef15f6 100644 --- a/dashboard/src/components/PaymentGatewayLogo.vue +++ b/dashboard/src/components/PaymentGatewayLogo.vue @@ -1,19 +1,20 @@ diff --git a/dashboard/src/composables/usePaymentSuccess.ts b/dashboard/src/composables/usePaymentSuccess.ts index 20b170ed7..481273534 100644 --- a/dashboard/src/composables/usePaymentSuccess.ts +++ b/dashboard/src/composables/usePaymentSuccess.ts @@ -8,6 +8,7 @@ interface PaymentSuccessOptions { messageDuration?: number enableConfetti?: boolean cleanupUrl?: boolean + checkOnMount?: boolean } interface PaymentSuccessReturn { @@ -27,10 +28,17 @@ interface PaymentSuccessReturn { * @param {number} options.messageDuration - How long to show success message in milliseconds (default: 10000) * @param {boolean} options.enableConfetti - Whether to trigger confetti animation (default: true) * @param {boolean} options.cleanupUrl - Whether to clean up success parameter from URL (default: true) + * @param {boolean} options.checkOnMount - Whether `?success=true` alone triggers the flow (default: true) * @returns {Object} - Returns reactive state and helper functions */ export function usePaymentSuccess(options: PaymentSuccessOptions = {}): PaymentSuccessReturn { - const { onSuccess, messageDuration = 10000, enableConfetti = true, cleanupUrl = true } = options + const { + onSuccess, + messageDuration = 10000, + enableConfetti = true, + cleanupUrl = true, + checkOnMount = true, + } = options const route = useRoute() const router = useRouter() @@ -95,7 +103,7 @@ export function usePaymentSuccess(options: PaymentSuccessOptions = {}): PaymentS // Auto-check for success on mount onMounted(() => { - checkForSuccess() + if (checkOnMount) checkForSuccess() }) return { diff --git a/dashboard/src/data/paymentGatewayLogos.ts b/dashboard/src/data/paymentGatewayLogos.ts new file mode 100644 index 000000000..e072443a9 --- /dev/null +++ b/dashboard/src/data/paymentGatewayLogos.ts @@ -0,0 +1,11 @@ +import { useCall } from "frappe-ui" + +import type { PaymentGatewayLogo } from "@/types" + +// Site-wide and the same for every user, so it is safe to persist. +export function usePaymentGatewayLogos() { + return useCall({ + url: "/api/v2/method/buzz.api.payments.get_payment_gateway_logos", + cacheKey: "payment-gateway-logos", + }) +} diff --git a/dashboard/src/pages/BookingSuccess.vue b/dashboard/src/pages/BookingSuccess.vue index 820d592d0..503714ed1 100644 --- a/dashboard/src/pages/BookingSuccess.vue +++ b/dashboard/src/pages/BookingSuccess.vue @@ -10,11 +10,24 @@ :is-registration="confirmation.data.event.free_event" /> -

+

{{ confirmation.data.event.free_event ? __("Registration Confirmed") : __("Booking Confirmed") }}

+
+

{{ __("Payment Pending") }}

+

+ {{ + __( + "We have not received your payment yet. If you have paid, it can take a minute to reach us.", + ) + }} +

+ +
@@ -85,6 +98,12 @@ const { showSuccessMessage, showSuccess } = usePaymentSuccess({ cleanupUrl: false, }) +// The shopper's return proves nothing; only a paid (or free) booking is confirmed. +const isConfirmed = computed(() => { + const data = confirmation.data + return Boolean(data?.event?.free_event || data?.booking?.payment_status === "Paid") +}) + const showPaymentSummary = computed(() => { const booking = confirmation.data?.booking return booking && (booking.total_amount || 0) > 0 @@ -94,6 +113,7 @@ const confirmation = useCall, Record>({ url: "/api/v2/method/buzz.api.booking.get_booking_confirmation", params: { booking_id: props.bookingId, token: route.query.token }, onSuccess: (data) => { + if (!isConfirmed.value) return showSuccess() // Clear any stored booking form data now that the booking is confirmed if (data?.event?.route) { diff --git a/dashboard/src/pages/SponsorshipDetails.vue b/dashboard/src/pages/SponsorshipDetails.vue index 7744af44e..8b9cb945e 100644 --- a/dashboard/src/pages/SponsorshipDetails.vue +++ b/dashboard/src/pages/SponsorshipDetails.vue @@ -316,7 +316,7 @@ import { Badge, Button, Dialog, ErrorMessage, FileUploader, Spinner, useCall } from "frappe-ui" import { toast } from "frappe-ui" import { dayjsLocal } from "frappe-ui" -import { computed, ref } from "vue" +import { computed, ref, watch } from "vue" import LucideCheckCircle from "~icons/lucide/check-circle" import LucideClock from "~icons/lucide/clock" import LucideXCircle from "~icons/lucide/x-circle" @@ -376,13 +376,12 @@ const updateLogoResource = useCall({ onError: (error) => console.error("Failed to update company logo:", error), }) -// Use the payment success composable -const { showSuccessMessage } = usePaymentSuccess({ - onSuccess: () => { - // Reload the enquiry details to get updated status - enquiryDetails.reload() - }, -}) +// Back from the payment page: celebrate only once the enquiry is actually paid. +const { showSuccessMessage, checkForSuccess } = usePaymentSuccess({ checkOnMount: false }) +watch( + () => enquiryDetails.data?.enquiry?.status, + (status) => status === "Paid" && checkForSuccess(), +) // Check if inquiry can be withdrawn (not paid and not already withdrawn) const canWithdraw = computed(() => { diff --git a/dashboard/src/types.ts b/dashboard/src/types.ts index 4015ae4d7..429b8d6d3 100644 --- a/dashboard/src/types.ts +++ b/dashboard/src/types.ts @@ -549,6 +549,12 @@ export interface CurrencyItem { number_format: string | null } +export interface PaymentGatewayLogo { + name: string + logo: string + logo_dark: string +} + export interface TierPrice { currency: string price: number diff --git a/dashboard/src/utils/paymentGateways.test.ts b/dashboard/src/utils/paymentGateways.test.ts deleted file mode 100644 index e2fc78f4e..000000000 --- a/dashboard/src/utils/paymentGateways.test.ts +++ /dev/null @@ -1,22 +0,0 @@ -import assert from "node:assert/strict" -import { test } from "node:test" - -import { paymentGatewayLogo } from "./paymentGateways.ts" - -test("matches a gateway by its exact name", () => { - assert.equal( - paymentGatewayLogo("Razorpay")?.light, - "/assets/buzz/images/payment_gateways/razorpay.svg", - ) -}) - -test("matches an account-suffixed gateway by its provider", () => { - assert.equal( - paymentGatewayLogo("Razorpay-Main")?.dark, - "/assets/buzz/images/payment_gateways/razorpay-dark.svg", - ) -}) - -test("returns nothing for an unknown gateway or offline method", () => { - assert.equal(paymentGatewayLogo("Bank Transfer"), undefined) -}) diff --git a/dashboard/src/utils/paymentGateways.ts b/dashboard/src/utils/paymentGateways.ts deleted file mode 100644 index d808778bf..000000000 --- a/dashboard/src/utils/paymentGateways.ts +++ /dev/null @@ -1,18 +0,0 @@ -const LOGO_DIRECTORY = "/assets/buzz/images/payment_gateways" - -// Brand marks are drawn for one background, so each gateway ships a version per theme. -interface PaymentGatewayLogoFiles { - light: string - dark: string -} - -const LOGOS: Record = { - Razorpay: { - light: `${LOGO_DIRECTORY}/razorpay.svg`, - dark: `${LOGO_DIRECTORY}/razorpay-dark.svg`, - }, -} - -// Gateways with per-account settings are named "-", e.g. "Stripe-Main". -export const paymentGatewayLogo = (gateway: string): PaymentGatewayLogoFiles | undefined => - LOGOS[gateway.split("-")[0]] From 55f1c1dc60022c405285932c80b00e5c589b9937 Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Mon, 12 Oct 2026 02:38:04 +0530 Subject: [PATCH 4/8] fix(payments): skip the payments move when frappe/payments is not installed --- buzz/patches/move_payments_to_bwh_payments.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/buzz/patches/move_payments_to_bwh_payments.py b/buzz/patches/move_payments_to_bwh_payments.py index d66e63ae7..552dfe707 100644 --- a/buzz/patches/move_payments_to_bwh_payments.py +++ b/buzz/patches/move_payments_to_bwh_payments.py @@ -4,6 +4,9 @@ def execute(): + if "payments" not in frappe.get_installed_apps(): + return + PaymentHistoryMigration().run() From 65a2ec37dc48213ee36db80f766fc4443b7e0d43 Mon Sep 17 00:00:00 2001 From: Harsh Tandiya Date: Mon, 12 Oct 2026 02:38:05 +0530 Subject: [PATCH 5/8] fix(booking): keep Pay & Book loading until the gateway page opens The booking call returns the checkout URL before the gateway page loads, so the button came back while the browser was still navigating. Reset on a back-forward cache restore so going back does not leave it stuck. --- dashboard/src/components/BookingForm.vue | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/dashboard/src/components/BookingForm.vue b/dashboard/src/components/BookingForm.vue index 7ca073bc8..c7bed2dd7 100644 --- a/dashboard/src/components/BookingForm.vue +++ b/dashboard/src/components/BookingForm.vue @@ -51,7 +51,7 @@