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/__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 194167aaf..5789548c8 100644 --- a/buzz/api/payments/test_payments.py +++ b/buzz/api/payments/test_payments.py @@ -1,16 +1,17 @@ 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.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) @@ -63,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/test_sponsorships.py b/buzz/api/sponsorships/test_sponsorships.py index 384418f45..22bb6e7eb 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}?success=true", + ) + 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/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..552dfe707 --- /dev/null +++ b/buzz/patches/move_payments_to_bwh_payments.py @@ -0,0 +1,79 @@ +import frappe +from bwh_payments.api import record_external_payment, record_external_refund +from bwh_payments.migrate.frappe_payments import import_gateways + + +def execute(): + if "payments" not in frappe.get_installed_apps(): + return + + 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) 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/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/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..5f8308a63 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,20 +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 }) - - 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."), - indicator: "green", - }) + const { message: refundStatus } = await frm.call("refund", { amount, tickets: selected }) + + let message = __("Refund initiated. The payment provider confirms it separately.") + if (refundStatus === "Processed") { + message = __("Refund settled.") + } else if (selected.length) { + message = __( + "Refund initiated. The tickets are cancelled once the payment provider confirms it.", + ) + } + frappe.show_alert({ message, 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..da56ac070 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 @@ -509,66 +508,46 @@ def get_refundable_tickets(self) -> list[dict]: @frappe.whitelist() def refund(self, amount: float, tickets: list[str] | None = None) -> str: - """Refund `amount` against this booking's payment.""" + """Refund `amount` against this booking's payment and return the refund's status.""" 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 Refund", refund.name, "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))) - + payments.sync_refunds(self.get_received_payment().payment_request) return { - "refunds": len(refunds), - "created": sum(self.record_refund(payment, refund) for refund in refunds), + "pending": frappe.db.count("Event Booking Refund", {"booking": self.name, "status": "Initiated"}) } - 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, - ) - def validate_refund(self, amount: float, tickets: list[str] | None = None) -> None: summary = self.get_refund_summary() @@ -619,12 +598,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..33cdb3180 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,56 @@ 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_refund_returns_its_own_status_not_the_bookings(self): + payment_testing.set_next_refund_status("Processed") + with patch("frappe.sendmail"): + self.booking.refund(amount=CHARGED_PER_TICKET) + + payment_testing.set_next_refund_status("Pending") + self.assertEqual(self.booking.refund(amount=CHARGED_PER_TICKET), "Initiated") + + 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 +98,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 +108,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 +118,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 +127,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 +139,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 +157,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 +180,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 +189,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..5c40a0657 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, {"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, {"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(), {"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/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 @@ 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..cecd5803f --- /dev/null +++ b/dashboard/src/data/paymentGatewayLogos.ts @@ -0,0 +1,13 @@ +import { createSharedComposable } from "@vueuse/core" +import { useCall } from "frappe-ui" + +import type { PaymentGatewayLogo } from "@/types" + +// Site-wide and the same for every user, so it is safe to persist. Shared, so a list of logos +// makes one request. +export const usePaymentGatewayLogos = createSharedComposable(() => + 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..0a0d07cda 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. +function isBookingConfirmed(data: Record | null | undefined) { + return Boolean(data?.event?.free_event || data?.booking?.payment_status === "Paid") +} +const isConfirmed = computed(() => isBookingConfirmed(confirmation.data)) + const showPaymentSummary = computed(() => { const booking = confirmation.data?.booking return booking && (booking.total_amount || 0) > 0 @@ -94,6 +113,8 @@ 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) => { + // useCall runs onSuccess before it sets confirmation.data, so read the response itself. + if (!isBookingConfirmed(data)) 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]] 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