From c82fe8f00d5afd01cd7de188d455ad7dcdfbcc1d Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Thu, 24 Sep 2026 00:57:40 +0530 Subject: [PATCH 01/10] feat: integrate PayPal checkout in Commera --- README.md | 14 +++ commera/api/payments.py | 40 ++++++- commera/jobs.py | 16 ++- commera/tests/test_paypal_checkout.py | 161 ++++++++++++++++++++++++++ commera/www/cart/checkout.py | 4 +- 5 files changed, 226 insertions(+), 9 deletions(-) create mode 100644 commera/tests/test_paypal_checkout.py diff --git a/README.md b/README.md index a38c147c..c35ca72a 100644 --- a/README.md +++ b/README.md @@ -50,10 +50,24 @@ Providers are configured from the dashboard rather than the desk. Each keeps its - **Stripe** — Cards and wallets, worldwide. - **Telr** — Cards and local methods across the GCC. - **Tabby** — Buy now, pay later in four instalments. MENA. +- **PayPal** — Wallet payments in supported currencies; unavailable for INR and Gulf currencies. - **Cash on delivery** — With its own fee, a threshold above which the fee is waived, and the account it books to. Gateway callbacks are idempotent, so a replayed webhook racing a shopper's return cannot bill twice. +#### PayPal setup + +PayPal uses `bwh_payments`' hosted checkout. In Desk, configure **PayPal Gateway Settings** with Sandbox +or Live credentials, redirect URLs, and the webhook ID returned by PayPal. Enable a **Payment Gateway +Profile** named `PayPal` pointing to those settings, and give the `PayPal` Mode of Payment a default +account for each selling company. If the separate `payments` app created a `Payment Gateway` named +`PayPal`, check that it points to `PayPal Gateway Settings` rather than its legacy `PayPal Settings`. + +Register a public HTTPS webhook at +`/api/method/bwh_payments.bwh_payments.webhook.handle?gateway=PayPal` for +`CHECKOUT.ORDER.APPROVED` and `PAYMENT.CAPTURE.COMPLETED`. Checkout offers PayPal only when the cart's +currency is supported; a pending PayPal order keeps its cart until the payment is resolved. +
Payment providers and cash on delivery settings
diff --git a/commera/api/payments.py b/commera/api/payments.py index 4c6a4d59..d4345387 100644 --- a/commera/api/payments.py +++ b/commera/api/payments.py @@ -1,7 +1,8 @@ from contextlib import contextmanager import frappe -from bwh_payments.bwh_payments.utils import get_available_payment_modes, resolve_payment_mode +from bwh_payments.bwh_payments.utils import get_payment_modes_for_currency, resolve_payment_mode +from bwh_payments.currency import to_minor_units from erpnext.accounts.doctype.journal_entry.journal_entry import get_default_bank_cash_account from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry from erpnext.accounts.doctype.pricing_rule.utils import validate_coupon_code @@ -42,6 +43,7 @@ class CheckoutPriceChangedError(frappe.ValidationError): from erpnext.selling.doctype.sales_order.sales_order import make_sales_invoice COD_PAYMENT_MODE = "COD" +PAYPAL_PAYMENT_MODE = "PayPal" def is_cod(payment_mode: str | None) -> bool: @@ -61,6 +63,30 @@ def get_open_gateway_payment_request(quotation_name: str) -> str | None: return None +def get_reusable_paypal_url(quotation, payment_mode: str, expected_total: float | None) -> str | None: + request_name = get_open_gateway_payment_request(quotation.name) + if not request_name: + return None + + payment_request = frappe.get_doc("Gateway Payment Request", request_name) + if payment_request.status != "Pending" or payment_request.gateway != PAYPAL_PAYMENT_MODE: + return None + + payment_request.sync_status() + if payment_request.status == "Paid": + return get_confirmation_url(payment_request.name) + if payment_request.status != "Pending" or payment_request.currency_code != quotation.currency: + return None + + if to_minor_units(payment_request.amount, quotation.currency) != to_minor_units( + get_charge_amount(quotation), quotation.currency + ): + return None + + validate_expected_total(quotation, payment_mode, expected_total) + return payment_request.order_url + + def validate_cart_is_not_in_checkout(quotation_name: str): if not quotation_name: return @@ -130,6 +156,11 @@ def open_checkout( quotation, payment_mode: str, delivery_option: str | None = None, expected_total: float | None = None ): validate_delivery_option(quotation, delivery_option) + if (payment_mode or "").strip().casefold() == PAYPAL_PAYMENT_MODE.casefold(): + if PAYPAL_PAYMENT_MODE in get_payment_modes_for_currency(quotation.currency): + if order_url := get_reusable_paypal_url(quotation, payment_mode, expected_total): + return {"order_url": order_url} + validate_cart_is_not_in_checkout(quotation.name) update_delivery_charges(quotation) validate_expected_total(quotation, payment_mode, expected_total) @@ -140,12 +171,13 @@ def open_checkout( return {"order_url": get_confirmation_url(quotation.name, payment_mode=COD_PAYMENT_MODE)} gateway = resolve_payment_mode(payment_mode) - if not gateway: + available_gateways = get_payment_modes_for_currency(quotation.currency) + if not gateway or gateway not in available_gateways: refuse_payment( - _("Please select a valid payment mode."), + _("Please select a payment mode available for this currency."), quotation.name, requested=payment_mode, - available=get_available_payment_modes(), + available=available_gateways, ) customer_contact = ( diff --git a/commera/jobs.py b/commera/jobs.py index 85db7d5f..f6f7146e 100644 --- a/commera/jobs.py +++ b/commera/jobs.py @@ -188,6 +188,17 @@ def delete_old_draft_quotations(): pluck="name", ) for quotation in old_drafts: + # A PayPal approval URL can remain payable after the shopper closes the browser. + if frappe.db.exists( + "Gateway Payment Request", + { + "ref_doctype": "Quotation", + "ref_docname": quotation, + "gateway": "PayPal", + "status": "Pending", + }, + ): + continue try: frappe.delete_doc("Quotation", quotation) except Exception as e: @@ -196,9 +207,8 @@ def delete_old_draft_quotations(): def sync_pending_gateway_payments(): """Settle checkouts the shopper's browser never confirmed, so captured money always reaches an order.""" - # ponytail: a request older than the lookback is left to manual reconciliation - delete_old_draft_ - # quotations already removes the draft cart it points at after 6 hours, so the order can no longer - # be placed from it. Widen both windows together, never this one alone. + # Requests older than the lookback need a webhook or manual reconciliation. PayPal carts are kept + # while their request is pending, so a late webhook can still place the order. now = now_datetime() pending_requests = frappe.get_all( "Gateway Payment Request", diff --git a/commera/tests/test_paypal_checkout.py b/commera/tests/test_paypal_checkout.py new file mode 100644 index 00000000..26a4ab90 --- /dev/null +++ b/commera/tests/test_paypal_checkout.py @@ -0,0 +1,161 @@ +from datetime import datetime +from types import SimpleNamespace +from unittest.mock import Mock, patch + +import frappe +from frappe.tests import UnitTestCase + +from commera.api.payments import get_reusable_paypal_url, open_checkout +from commera.jobs import delete_old_draft_quotations +from commera.www.cart.checkout import get_context + + +def make_quotation(amount=20): + return SimpleNamespace( + name="PAYPAL-CART-1", + currency="USD", + rounded_total=amount, + grand_total=amount, + custom_delivery_option=None, + ) + + +class TestPayPalCheckout(UnitTestCase): + def test_checkout_offers_gateways_for_the_cart_currency(self): + quotation = SimpleNamespace(currency="USD", items=[object()]) + context = frappe._dict() + with ( + patch("commera.www.cart.checkout._get_cart_quotation", return_value=quotation), + patch("commera.www.cart.checkout.frappe.get_cached_doc") as get_settings, + patch( + "commera.www.cart.checkout.get_payment_modes_for_currency", return_value=["PayPal"] + ) as available, + patch("commera.www.cart.checkout.get_checkout_summary", side_effect=StopIteration), + ): + get_settings.return_value.get.return_value = None + with self.assertRaises(StopIteration): + get_context(context) + + available.assert_called_once_with("USD") + self.assertEqual(context.payment_gateways, ["PayPal"]) + + def test_an_unsupported_currency_is_rejected_before_a_payment_request_is_created(self): + quotation = make_quotation() + with ( + patch("commera.api.payments.get_payment_modes_for_currency", return_value=[]) as available, + patch("commera.api.payments.resolve_payment_mode", return_value="PayPal"), + patch("commera.api.payments.validate_cart_is_not_in_checkout"), + patch("commera.api.payments.update_delivery_charges"), + patch("commera.api.payments._", side_effect=lambda message: message), + patch("commera.api.payments.refuse_payment", side_effect=frappe.ValidationError), + patch("commera.api.payments.frappe.get_doc") as get_doc, + ): + with self.assertRaises(frappe.ValidationError): + open_checkout(quotation, "PayPal") + + available.assert_called_with("USD") + get_doc.assert_not_called() + + def test_a_repeat_checkout_uses_the_existing_paypal_url(self): + quotation = make_quotation() + with ( + patch("commera.api.payments.get_payment_modes_for_currency", return_value=["PayPal"]), + patch("commera.api.payments.get_reusable_paypal_url", return_value="https://paypal.test/old"), + patch("commera.api.payments.validate_cart_is_not_in_checkout") as validate_cart, + patch("commera.api.payments.frappe.get_doc") as get_doc, + ): + result = open_checkout(quotation, "PayPal") + + self.assertEqual(result, {"order_url": "https://paypal.test/old"}) + validate_cart.assert_not_called() + get_doc.assert_not_called() + + def test_a_changed_cart_cannot_reuse_its_old_paypal_url(self): + quotation = make_quotation(amount=30) + payment_request = SimpleNamespace( + status="Pending", + gateway="PayPal", + currency_code="USD", + amount=20, + order_url="https://paypal.test/old", + sync_status=Mock(), + ) + with ( + patch("commera.api.payments.get_open_gateway_payment_request", return_value="GPR-1"), + patch("commera.api.payments.frappe.get_doc", return_value=payment_request), + patch( + "commera.api.payments.to_minor_units", side_effect=lambda amount, currency: int(amount * 100) + ), + ): + self.assertIsNone(get_reusable_paypal_url(quotation, "PayPal", None)) + + payment_request.sync_status.assert_called_once_with() + + def test_a_reused_paypal_url_must_match_the_total_the_shopper_saw(self): + quotation = make_quotation() + payment_request = SimpleNamespace( + status="Pending", + gateway="PayPal", + currency_code="USD", + amount=20, + order_url="https://paypal.test/old", + sync_status=Mock(), + ) + with ( + patch("commera.api.payments.get_open_gateway_payment_request", return_value="GPR-1"), + patch("commera.api.payments.frappe.get_doc", return_value=payment_request), + patch( + "commera.api.payments.to_minor_units", side_effect=lambda amount, currency: int(amount * 100) + ), + patch( + "commera.api.payments.validate_expected_total", side_effect=frappe.ValidationError + ) as validate, + ): + with self.assertRaises(frappe.ValidationError): + get_reusable_paypal_url(quotation, "PayPal", 25) + + validate.assert_called_once_with(quotation, "PayPal", 25) + + def test_a_paid_paypal_request_goes_to_confirmation(self): + quotation = make_quotation() + payment_request = SimpleNamespace(name="GPR-1", status="Pending", gateway="PayPal") + + def mark_paid(): + payment_request.status = "Paid" + + payment_request.sync_status = mark_paid + with ( + patch("commera.api.payments.get_open_gateway_payment_request", return_value="GPR-1"), + patch("commera.api.payments.frappe.get_doc", return_value=payment_request), + patch( + "commera.api.payments.get_confirmation_url", return_value="/orders/confirmation" + ) as confirm, + ): + self.assertEqual(get_reusable_paypal_url(quotation, "PayPal", None), "/orders/confirmation") + + confirm.assert_called_once_with("GPR-1") + + def test_cleanup_keeps_a_cart_with_pending_paypal_payment(self): + exists = Mock(return_value=True) + delete_doc = Mock() + fake_frappe = SimpleNamespace( + get_all=Mock(return_value=["PAYPAL-CART-1"]), + db=SimpleNamespace(exists=exists), + delete_doc=delete_doc, + ) + with ( + patch("commera.jobs.frappe", fake_frappe), + patch("commera.jobs.now_datetime", return_value=datetime(2026, 9, 24)), + ): + delete_old_draft_quotations() + + exists.assert_called_once_with( + "Gateway Payment Request", + { + "ref_doctype": "Quotation", + "ref_docname": "PAYPAL-CART-1", + "gateway": "PayPal", + "status": "Pending", + }, + ) + delete_doc.assert_not_called() diff --git a/commera/www/cart/checkout.py b/commera/www/cart/checkout.py index 60486485..e48ff7dd 100644 --- a/commera/www/cart/checkout.py +++ b/commera/www/cart/checkout.py @@ -1,5 +1,5 @@ import frappe -from bwh_payments.bwh_payments.utils import get_available_payment_modes +from bwh_payments.bwh_payments.utils import get_payment_modes_for_currency from frappe.query_builder import DocType from commera.api.shipping import get_checkout_summary @@ -30,7 +30,7 @@ def get_context(context): frappe.redirect(f"/{frappe.local.lang}/cart") commera_settings = frappe.get_cached_doc("Commera Settings") default_price_list = commera_settings.get("default_price_list") - context.payment_gateways = get_available_payment_modes() + context.payment_gateways = get_payment_modes_for_currency(cart_quotation.currency) context.show_cod = commera_settings.get("cod_enabled", 0) context.cart_quotation = cart_quotation context.checkout_summary = get_checkout_summary(cart_quotation) From cf6dfe8ef489e492fbced3f2efc77af73316d2a0 Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Sun, 11 Oct 2026 17:11:07 +0530 Subject: [PATCH 02/10] fix: keep the gateway reference on language redirects /en-US/ and /en-GB/ redirected to /en/ without the query string, so a shopper whose browser language is US or UK English came back from the gateway to a confirmation page with no reference_id and saw "could not confirm". The redirects a return URL can hit now forward the query. Co-Authored-By: Claude Opus 5.5 --- commera/hooks.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/commera/hooks.py b/commera/hooks.py index 7fd6606b..1cc96216 100644 --- a/commera/hooks.py +++ b/commera/hooks.py @@ -13,14 +13,15 @@ {"source": "/products", "target": "/en/products"}, {"source": "/products/(.*)", "target": r"/en/products/\1"}, {"source": "/", "target": "/en"}, - {"source": "/cart", "target": "/en/cart"}, - {"source": "/cart/checkout", "target": "/en/cart/checkout"}, + # Gateway return URLs carry `?reference_id=`, so these redirects must keep the query string. + {"source": "/cart", "target": "/en/cart", "forward_query_parameters": True}, + {"source": "/cart/checkout", "target": "/en/cart/checkout", "forward_query_parameters": True}, {"source": "/account", "target": "en/account/dashboard"}, - {"source": "/account/(.*)", "target": r"/en/account/\1"}, + {"source": "/account/(.*)", "target": r"/en/account/\1", "forward_query_parameters": True}, {"source": "/en/account", "target": "/account/dashboard"}, {"source": "/ar/account", "target": "/account/dashboard"}, - {"source": "/en-US/(.*)", "target": r"/en/\1"}, - {"source": "/en-GB/(.*)", "target": r"/en/\1"}, + {"source": "/en-US/(.*)", "target": r"/en/\1", "forward_query_parameters": True}, + {"source": "/en-GB/(.*)", "target": r"/en/\1", "forward_query_parameters": True}, ] # website_path_resolver = "commera.utils.resolve_bilingual_path" From c0d76773ca0a4d159d3fba597688b207de551e12 Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Sun, 11 Oct 2026 17:14:01 +0530 Subject: [PATCH 03/10] fix: give a webhook-placed order to the shopper A gateway webhook runs as Administrator, so place_order stamped Administrator as the Sales Order owner and the order never appeared in the shopper's account. The shopper is now read from the cart's contact_email, which the storefront sets to their login, whenever the order is placed as Administrator. Co-Authored-By: Claude Opus 5.5 --- commera/api/payments.py | 11 ++++++++++- commera/tests/test_payment_hooks.py | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/commera/api/payments.py b/commera/api/payments.py index d4345387..dd5ed935 100644 --- a/commera/api/payments.py +++ b/commera/api/payments.py @@ -318,9 +318,18 @@ def stamp_order_owner(sales_order, shopper: str) -> None: sales_order.db_set("owner", shopper, update_modified=False) +def get_shopper(quotation) -> str: + """A gateway webhook places the order as Administrator; the cart's contact_email is the shopper's login.""" + if frappe.session.user != "Administrator": + return frappe.session.user + if quotation.contact_email and frappe.db.exists("User", quotation.contact_email): + return quotation.contact_email + return frappe.session.user + + def place_order(quotation, payment_mode: str, gateway_amount=None, gateway_reference=None): """Submit the cart and bill it. Called once per payment; the Quotation docstatus enforces that.""" - shopper = frappe.session.user + shopper = get_shopper(quotation) with system_user_session(): fix_payment_schedule_dates(quotation) quotation.flags.ignore_permissions = True diff --git a/commera/tests/test_payment_hooks.py b/commera/tests/test_payment_hooks.py index 6809de63..c688102b 100644 --- a/commera/tests/test_payment_hooks.py +++ b/commera/tests/test_payment_hooks.py @@ -287,6 +287,15 @@ def test_a_paid_payment_creates_one_sales_order_and_one_sales_invoice(self): ) self.assertEqual(frappe.db.get_value("Payment Entry", payment_entries[0], "mode_of_payment"), GATEWAY) + def test_an_order_placed_by_a_webhook_belongs_to_the_shopper(self): + """A webhook runs as Administrator, and the account pages list only orders the shopper owns.""" + frappe.set_user("Administrator") + + on_payment_request_update(self.payment_request) + + sales_order = self.submitted_sales_orders()[0] + self.assertEqual(frappe.db.get_value("Sales Order", sales_order, "owner"), self.shopper) + def test_a_duplicate_callback_does_not_create_a_second_order(self): """A replayed webhook racing a confirm_payment poll must not bill the shopper twice.""" on_payment_request_update(self.payment_request) From ba9fcd9f577474d2ec7d578760e45c2915857edb Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Sun, 11 Oct 2026 17:35:33 +0530 Subject: [PATCH 04/10] refactor: read payment gateways from the commera_payment_gateways hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commera no longer names a gateway outside its built-in registry. Provider apps register a card through the commera_payment_gateways hook (spec 01 §3.1), and Commera acts on the card's capabilities: with reusable_checkout, a returning shopper is sent back to the still-open checkout link and the cart is kept while that payment is pending. This replaces the PayPal-specific checkout reuse and cart cleanup check. Co-Authored-By: Claude Opus 5.5 --- README.md | 19 ++-- commera/api/admin/payments.py | 24 ++++- commera/api/payments.py | 27 ++++-- commera/jobs.py | 13 ++- commera/tests/test_admin_integrations.py | 49 +++++++++- ..._checkout.py => test_reusable_checkout.py} | 94 +++++++++++++------ 6 files changed, 170 insertions(+), 56 deletions(-) rename commera/tests/{test_paypal_checkout.py => test_reusable_checkout.py} (59%) diff --git a/README.md b/README.md index c35ca72a..ecf686de 100644 --- a/README.md +++ b/README.md @@ -50,23 +50,18 @@ Providers are configured from the dashboard rather than the desk. Each keeps its - **Stripe** — Cards and wallets, worldwide. - **Telr** — Cards and local methods across the GCC. - **Tabby** — Buy now, pay later in four instalments. MENA. -- **PayPal** — Wallet payments in supported currencies; unavailable for INR and Gulf currencies. - **Cash on delivery** — With its own fee, a threshold above which the fee is waived, and the account it books to. Gateway callbacks are idempotent, so a replayed webhook racing a shopper's return cannot bill twice. -#### PayPal setup +#### Gateways from other apps -PayPal uses `bwh_payments`' hosted checkout. In Desk, configure **PayPal Gateway Settings** with Sandbox -or Live credentials, redirect URLs, and the webhook ID returned by PayPal. Enable a **Payment Gateway -Profile** named `PayPal` pointing to those settings, and give the `PayPal` Mode of Payment a default -account for each selling company. If the separate `payments` app created a `Payment Gateway` named -`PayPal`, check that it points to `PayPal Gateway Settings` rather than its legacy `PayPal Settings`. - -Register a public HTTPS webhook at -`/api/method/bwh_payments.bwh_payments.webhook.handle?gateway=PayPal` for -`CHECKOUT.ORDER.APPROVED` and `PAYMENT.CAPTURE.COMPLETED`. Checkout offers PayPal only when the cart's -currency is supported; a pending PayPal order keeps its cart until the payment is resolved. +A payment provider app adds its own card by declaring `commera_payment_gateways` in its `hooks.py`: a +list of dotted paths, each returning `{slug, label, blurb, settings_doctype, docs_url, capabilities}`, +where `label` is the Payment Gateway Profile's name. Commera never names a provider in code; it acts on +`capabilities` instead. With `reusable_checkout`, a shopper who returns to checkout is sent back to the +still-open checkout link, and the cart is kept while that payment is pending. Checkout only offers +gateways that can take the cart's currency.
Payment providers and cash on delivery settings diff --git a/commera/api/admin/payments.py b/commera/api/admin/payments.py index 29ae0b60..8279b3e7 100644 --- a/commera/api/admin/payments.py +++ b/commera/api/admin/payments.py @@ -48,17 +48,39 @@ def add_gateway_payment_mode(integration): add_payment_mode(integration["label"], "Bank") +def get_payment_gateways(): + """Built-ins, then `commera_payment_gateways` hook results; a hook cannot replace a built-in slug.""" + gateways = list(PAYMENT_GATEWAYS) + slugs = {gateway["slug"] for gateway in gateways} + for method in frappe.get_hooks("commera_payment_gateways"): + gateway = frappe.get_attr(method)() + if gateway["slug"] not in slugs: + gateways.append(gateway) + slugs.add(gateway["slug"]) + return gateways + + def get_payment_registry(): """The gateway registry as integration entries, with what every payment gateway shares filled in.""" return [ { + "capabilities": [], **gateway, "profile_doctype": PROFILE_DOCTYPE, "profile_settings_fieldname": PROFILE_SETTINGS_FIELDNAME, "webhook_path": WEBHOOK_PATH, "on_enable": add_gateway_payment_mode, } - for gateway in PAYMENT_GATEWAYS + for gateway in get_payment_gateways() + ] + + +def get_gateways_with_capability(capability: str) -> list[str]: + """Payment Gateway Profile names whose provider declares `capability`.""" + return [ + gateway["label"] + for gateway in get_payment_gateways() + if capability in gateway.get("capabilities", ()) ] diff --git a/commera/api/payments.py b/commera/api/payments.py index dd5ed935..956debb6 100644 --- a/commera/api/payments.py +++ b/commera/api/payments.py @@ -43,7 +43,8 @@ class CheckoutPriceChangedError(frappe.ValidationError): from erpnext.selling.doctype.sales_order.sales_order import make_sales_invoice COD_PAYMENT_MODE = "COD" -PAYPAL_PAYMENT_MODE = "PayPal" +# A gateway capability: its checkout link stays payable after the shopper closes the tab. +REUSABLE_CHECKOUT = "reusable_checkout" def is_cod(payment_mode: str | None) -> bool: @@ -63,13 +64,22 @@ def get_open_gateway_payment_request(quotation_name: str) -> str | None: return None -def get_reusable_paypal_url(quotation, payment_mode: str, expected_total: float | None) -> str | None: +def get_reusable_checkout_gateways() -> list[str]: + # Imported here: commera.api.admin.payments imports commera.migrate, which imports this module. + from commera.api.admin.payments import get_gateways_with_capability + + return get_gateways_with_capability(REUSABLE_CHECKOUT) + + +def get_reusable_checkout_url( + quotation, gateway: str, payment_mode: str, expected_total: float | None +) -> str | None: request_name = get_open_gateway_payment_request(quotation.name) if not request_name: return None payment_request = frappe.get_doc("Gateway Payment Request", request_name) - if payment_request.status != "Pending" or payment_request.gateway != PAYPAL_PAYMENT_MODE: + if payment_request.status != "Pending" or payment_request.gateway != gateway: return None payment_request.sync_status() @@ -156,10 +166,12 @@ def open_checkout( quotation, payment_mode: str, delivery_option: str | None = None, expected_total: float | None = None ): validate_delivery_option(quotation, delivery_option) - if (payment_mode or "").strip().casefold() == PAYPAL_PAYMENT_MODE.casefold(): - if PAYPAL_PAYMENT_MODE in get_payment_modes_for_currency(quotation.currency): - if order_url := get_reusable_paypal_url(quotation, payment_mode, expected_total): - return {"order_url": order_url} + gateway = resolve_payment_mode(payment_mode) + if gateway in get_reusable_checkout_gateways() and gateway in get_payment_modes_for_currency( + quotation.currency + ): + if order_url := get_reusable_checkout_url(quotation, gateway, payment_mode, expected_total): + return {"order_url": order_url} validate_cart_is_not_in_checkout(quotation.name) update_delivery_charges(quotation) @@ -170,7 +182,6 @@ def open_checkout( refuse_payment(_("Cash on delivery is not available."), quotation.name) return {"order_url": get_confirmation_url(quotation.name, payment_mode=COD_PAYMENT_MODE)} - gateway = resolve_payment_mode(payment_mode) available_gateways = get_payment_modes_for_currency(quotation.currency) if not gateway or gateway not in available_gateways: refuse_payment( diff --git a/commera/jobs.py b/commera/jobs.py index f6f7146e..d0b237e0 100644 --- a/commera/jobs.py +++ b/commera/jobs.py @@ -4,6 +4,8 @@ ) from frappe.utils import add_to_date, get_url, now_datetime +from commera.api.payments import get_reusable_checkout_gateways + def get_cc_email(): return frappe.get_cached_value("Commera Settings", "Commera Settings", "cc_email") @@ -187,14 +189,15 @@ def delete_old_draft_quotations(): }, pluck="name", ) + reusable_gateways = get_reusable_checkout_gateways() for quotation in old_drafts: - # A PayPal approval URL can remain payable after the shopper closes the browser. - if frappe.db.exists( + # The gateway's checkout link is still payable, so the cart must outlive the shopper's tab. + if reusable_gateways and frappe.db.exists( "Gateway Payment Request", { "ref_doctype": "Quotation", "ref_docname": quotation, - "gateway": "PayPal", + "gateway": ["in", reusable_gateways], "status": "Pending", }, ): @@ -207,8 +210,8 @@ def delete_old_draft_quotations(): def sync_pending_gateway_payments(): """Settle checkouts the shopper's browser never confirmed, so captured money always reaches an order.""" - # Requests older than the lookback need a webhook or manual reconciliation. PayPal carts are kept - # while their request is pending, so a late webhook can still place the order. + # Requests older than the lookback need a webhook or manual reconciliation. Carts of reusable-checkout + # gateways are kept while their request is pending, so a late webhook can still place the order. now = now_datetime() pending_requests = frappe.get_all( "Gateway Payment Request", diff --git a/commera/tests/test_admin_integrations.py b/commera/tests/test_admin_integrations.py index 5ecc1bff..49498283 100644 --- a/commera/tests/test_admin_integrations.py +++ b/commera/tests/test_admin_integrations.py @@ -1,5 +1,7 @@ # Copyright (c) 2026, company@bwhstudios.com and Contributors +from unittest.mock import patch + import frappe from bwh_payments.bwh_payments.utils import get_available_payment_modes from frappe.tests import IntegrationTestCase @@ -10,7 +12,12 @@ get_integration_linked_doctypes, get_link_options, ) -from commera.api.admin.payments import get_payment_integrations, save_payment_integration +from commera.api.admin.payments import ( + get_gateways_with_capability, + get_payment_gateways, + get_payment_integrations, + save_payment_integration, +) SLUG = "razorpay" GATEWAY = "Razorpay" @@ -127,7 +134,10 @@ def test_disabling_switches_both_off_without_deleting(self): def test_every_registered_gateway_is_described(self): cards = get_payment_integrations() - self.assertEqual([card["slug"] for card in cards], ["razorpay", "stripe", "telr", "tabby"]) + self.assertEqual( + [card["slug"] for card in cards], [gateway["slug"] for gateway in get_payment_gateways()] + ) + self.assertEqual([card["slug"] for card in cards][:4], ["razorpay", "stripe", "telr", "tabby"]) for card in cards: self.assertTrue(card["available"], f"{card['slug']} settings doctype is missing") self.assertTrue(card["webhook_url"].endswith(f"gateway={card['label']}")) @@ -137,6 +147,26 @@ def test_every_registered_gateway_is_described(self): [field["fieldname"] for group in card["groups"] for field in group["fields"]], ) + def test_a_provider_app_registers_its_gateway_through_the_hook(self): + with patch( + "commera.api.admin.payments.frappe.get_hooks", return_value=[f"{__name__}.wallet_gateway"] + ): + gateways = get_payment_gateways() + reusable = get_gateways_with_capability("reusable_checkout") + + self.assertEqual(gateways[-1]["slug"], "test-wallet") + self.assertEqual(reusable, ["Test Wallet"]) + + def test_a_hook_cannot_replace_a_built_in_gateway(self): + with patch( + "commera.api.admin.payments.frappe.get_hooks", return_value=[f"{__name__}.stripe_impostor"] + ): + gateways = get_payment_gateways() + + stripe = [gateway for gateway in gateways if gateway["slug"] == "stripe"] + self.assertEqual(len(stripe), 1) + self.assertEqual(stripe[0]["settings_doctype"], "Stripe Gateway Settings") + def test_a_gateway_whose_app_is_gone_is_reported_unavailable(self): card = describe_integration( { @@ -173,3 +203,18 @@ def test_the_picker_searches_a_doctype_a_provider_links_to(self): def test_the_picker_refuses_a_doctype_no_provider_links_to(self): # Otherwise the whitelisted picker reads any doctype the session can see. self.assertRaises(frappe.ValidationError, get_link_options, "User") + + +def wallet_gateway(): + return { + "slug": "test-wallet", + "label": "Test Wallet", + "blurb": "A wallet registered by another app.", + "settings_doctype": "Test Wallet Settings", + "docs_url": "https://wallet.test/docs", + "capabilities": ["reusable_checkout"], + } + + +def stripe_impostor(): + return {**wallet_gateway(), "slug": "stripe", "settings_doctype": "Impostor Settings"} diff --git a/commera/tests/test_paypal_checkout.py b/commera/tests/test_reusable_checkout.py similarity index 59% rename from commera/tests/test_paypal_checkout.py rename to commera/tests/test_reusable_checkout.py index 26a4ab90..e72ba1ee 100644 --- a/commera/tests/test_paypal_checkout.py +++ b/commera/tests/test_reusable_checkout.py @@ -5,14 +5,16 @@ import frappe from frappe.tests import UnitTestCase -from commera.api.payments import get_reusable_paypal_url, open_checkout +from commera.api.payments import get_reusable_checkout_url, open_checkout from commera.jobs import delete_old_draft_quotations from commera.www.cart.checkout import get_context +GATEWAY = "Test Wallet" + def make_quotation(amount=20): return SimpleNamespace( - name="PAYPAL-CART-1", + name="WALLET-CART-1", currency="USD", rounded_total=amount, grand_total=amount, @@ -20,7 +22,7 @@ def make_quotation(amount=20): ) -class TestPayPalCheckout(UnitTestCase): +class TestReusableCheckout(UnitTestCase): def test_checkout_offers_gateways_for_the_cart_currency(self): quotation = SimpleNamespace(currency="USD", items=[object()]) context = frappe._dict() @@ -28,7 +30,7 @@ def test_checkout_offers_gateways_for_the_cart_currency(self): patch("commera.www.cart.checkout._get_cart_quotation", return_value=quotation), patch("commera.www.cart.checkout.frappe.get_cached_doc") as get_settings, patch( - "commera.www.cart.checkout.get_payment_modes_for_currency", return_value=["PayPal"] + "commera.www.cart.checkout.get_payment_modes_for_currency", return_value=[GATEWAY] ) as available, patch("commera.www.cart.checkout.get_checkout_summary", side_effect=StopIteration), ): @@ -37,13 +39,14 @@ def test_checkout_offers_gateways_for_the_cart_currency(self): get_context(context) available.assert_called_once_with("USD") - self.assertEqual(context.payment_gateways, ["PayPal"]) + self.assertEqual(context.payment_gateways, [GATEWAY]) def test_an_unsupported_currency_is_rejected_before_a_payment_request_is_created(self): quotation = make_quotation() with ( patch("commera.api.payments.get_payment_modes_for_currency", return_value=[]) as available, - patch("commera.api.payments.resolve_payment_mode", return_value="PayPal"), + patch("commera.api.payments.resolve_payment_mode", return_value=GATEWAY), + patch("commera.api.payments.get_reusable_checkout_gateways", return_value=[GATEWAY]), patch("commera.api.payments.validate_cart_is_not_in_checkout"), patch("commera.api.payments.update_delivery_charges"), patch("commera.api.payments._", side_effect=lambda message: message), @@ -51,33 +54,35 @@ def test_an_unsupported_currency_is_rejected_before_a_payment_request_is_created patch("commera.api.payments.frappe.get_doc") as get_doc, ): with self.assertRaises(frappe.ValidationError): - open_checkout(quotation, "PayPal") + open_checkout(quotation, GATEWAY) available.assert_called_with("USD") get_doc.assert_not_called() - def test_a_repeat_checkout_uses_the_existing_paypal_url(self): + def test_a_repeat_checkout_reuses_the_open_link_of_a_reusable_gateway(self): quotation = make_quotation() with ( - patch("commera.api.payments.get_payment_modes_for_currency", return_value=["PayPal"]), - patch("commera.api.payments.get_reusable_paypal_url", return_value="https://paypal.test/old"), + patch("commera.api.payments.resolve_payment_mode", return_value=GATEWAY), + patch("commera.api.payments.get_reusable_checkout_gateways", return_value=[GATEWAY]), + patch("commera.api.payments.get_payment_modes_for_currency", return_value=[GATEWAY]), + patch("commera.api.payments.get_reusable_checkout_url", return_value="https://wallet.test/old"), patch("commera.api.payments.validate_cart_is_not_in_checkout") as validate_cart, patch("commera.api.payments.frappe.get_doc") as get_doc, ): - result = open_checkout(quotation, "PayPal") + result = open_checkout(quotation, GATEWAY) - self.assertEqual(result, {"order_url": "https://paypal.test/old"}) + self.assertEqual(result, {"order_url": "https://wallet.test/old"}) validate_cart.assert_not_called() get_doc.assert_not_called() - def test_a_changed_cart_cannot_reuse_its_old_paypal_url(self): + def test_a_changed_cart_cannot_reuse_its_old_link(self): quotation = make_quotation(amount=30) payment_request = SimpleNamespace( status="Pending", - gateway="PayPal", + gateway=GATEWAY, currency_code="USD", amount=20, - order_url="https://paypal.test/old", + order_url="https://wallet.test/old", sync_status=Mock(), ) with ( @@ -87,18 +92,18 @@ def test_a_changed_cart_cannot_reuse_its_old_paypal_url(self): "commera.api.payments.to_minor_units", side_effect=lambda amount, currency: int(amount * 100) ), ): - self.assertIsNone(get_reusable_paypal_url(quotation, "PayPal", None)) + self.assertIsNone(get_reusable_checkout_url(quotation, GATEWAY, GATEWAY, None)) payment_request.sync_status.assert_called_once_with() - def test_a_reused_paypal_url_must_match_the_total_the_shopper_saw(self): + def test_a_reused_link_must_match_the_total_the_shopper_saw(self): quotation = make_quotation() payment_request = SimpleNamespace( status="Pending", - gateway="PayPal", + gateway=GATEWAY, currency_code="USD", amount=20, - order_url="https://paypal.test/old", + order_url="https://wallet.test/old", sync_status=Mock(), ) with ( @@ -112,13 +117,13 @@ def test_a_reused_paypal_url_must_match_the_total_the_shopper_saw(self): ) as validate, ): with self.assertRaises(frappe.ValidationError): - get_reusable_paypal_url(quotation, "PayPal", 25) + get_reusable_checkout_url(quotation, GATEWAY, GATEWAY, 25) - validate.assert_called_once_with(quotation, "PayPal", 25) + validate.assert_called_once_with(quotation, GATEWAY, 25) - def test_a_paid_paypal_request_goes_to_confirmation(self): + def test_a_paid_request_goes_to_confirmation(self): quotation = make_quotation() - payment_request = SimpleNamespace(name="GPR-1", status="Pending", gateway="PayPal") + payment_request = SimpleNamespace(name="GPR-1", status="Pending", gateway=GATEWAY) def mark_paid(): payment_request.status = "Paid" @@ -131,21 +136,24 @@ def mark_paid(): "commera.api.payments.get_confirmation_url", return_value="/orders/confirmation" ) as confirm, ): - self.assertEqual(get_reusable_paypal_url(quotation, "PayPal", None), "/orders/confirmation") + self.assertEqual( + get_reusable_checkout_url(quotation, GATEWAY, GATEWAY, None), "/orders/confirmation" + ) confirm.assert_called_once_with("GPR-1") - def test_cleanup_keeps_a_cart_with_pending_paypal_payment(self): + def test_cleanup_keeps_a_cart_with_a_pending_reusable_checkout(self): exists = Mock(return_value=True) delete_doc = Mock() fake_frappe = SimpleNamespace( - get_all=Mock(return_value=["PAYPAL-CART-1"]), + get_all=Mock(return_value=["WALLET-CART-1"]), db=SimpleNamespace(exists=exists), delete_doc=delete_doc, ) with ( patch("commera.jobs.frappe", fake_frappe), patch("commera.jobs.now_datetime", return_value=datetime(2026, 9, 24)), + patch("commera.jobs.get_reusable_checkout_gateways", return_value=[GATEWAY]), ): delete_old_draft_quotations() @@ -153,9 +161,39 @@ def test_cleanup_keeps_a_cart_with_pending_paypal_payment(self): "Gateway Payment Request", { "ref_doctype": "Quotation", - "ref_docname": "PAYPAL-CART-1", - "gateway": "PayPal", + "ref_docname": "WALLET-CART-1", + "gateway": ["in", [GATEWAY]], "status": "Pending", }, ) delete_doc.assert_not_called() + + def test_cleanup_deletes_a_stale_cart_when_no_gateway_keeps_checkouts_open(self): + delete_doc = Mock() + fake_frappe = SimpleNamespace( + get_all=Mock(return_value=["WALLET-CART-1"]), + db=SimpleNamespace(exists=Mock(return_value=True)), + delete_doc=delete_doc, + ) + with ( + patch("commera.jobs.frappe", fake_frappe), + patch("commera.jobs.now_datetime", return_value=datetime(2026, 9, 24)), + patch("commera.jobs.get_reusable_checkout_gateways", return_value=[]), + ): + delete_old_draft_quotations() + + delete_doc.assert_called_once_with("Quotation", "WALLET-CART-1") + + def test_a_gateway_without_the_capability_never_reuses_a_link(self): + quotation = make_quotation() + with ( + patch("commera.api.payments.resolve_payment_mode", return_value=GATEWAY), + patch("commera.api.payments.get_reusable_checkout_gateways", return_value=[]), + patch("commera.api.payments.get_payment_modes_for_currency", return_value=[GATEWAY]), + patch("commera.api.payments.get_reusable_checkout_url") as reuse, + patch("commera.api.payments.validate_cart_is_not_in_checkout", side_effect=StopIteration), + ): + with self.assertRaises(StopIteration): + open_checkout(quotation, GATEWAY) + + reuse.assert_not_called() From 2bc9ff9d25faf495360f286f58ce984e29654cc6 Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Sun, 11 Oct 2026 17:48:29 +0530 Subject: [PATCH 05/10] refactor: register payment gateways through commera_hooks develop's plugin system reads keyed handlers from commera_hooks, checks each app's commera_api_version, and warns about unknown keys. Payment gateways now use that: a payment_gateways key in commera_hooks (added to HANDLER_HOOKS) instead of a separate top-level hook. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 ++-- commera/api/admin/payments.py | 5 +++-- commera/sdk/events.py | 8 +++++++- commera/tests/test_admin_integrations.py | 8 ++------ 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index ecf686de..5f85f5ad 100644 --- a/README.md +++ b/README.md @@ -56,8 +56,8 @@ Gateway callbacks are idempotent, so a replayed webhook racing a shopper's retur #### Gateways from other apps -A payment provider app adds its own card by declaring `commera_payment_gateways` in its `hooks.py`: a -list of dotted paths, each returning `{slug, label, blurb, settings_doctype, docs_url, capabilities}`, +A payment provider app adds its own card through `commera_hooks = {"payment_gateways": [...]}` in its +`hooks.py`: dotted paths, each returning `{slug, label, blurb, settings_doctype, docs_url, capabilities}`, where `label` is the Payment Gateway Profile's name. Commera never names a provider in code; it acts on `capabilities` instead. With `reusable_checkout`, a shopper who returns to checkout is sent back to the still-open checkout link, and the cart is kept while that payment is pending. Checkout only offers diff --git a/commera/api/admin/payments.py b/commera/api/admin/payments.py index 8279b3e7..c67d1941 100644 --- a/commera/api/admin/payments.py +++ b/commera/api/admin/payments.py @@ -5,6 +5,7 @@ from commera.api.admin.integrations import describe_integration, save_integration from commera.migrate import add_payment_mode +from commera.plugin_events import get_handlers PROFILE_DOCTYPE = "Payment Gateway Profile" PROFILE_SETTINGS_FIELDNAME = "gateway_settings" @@ -49,10 +50,10 @@ def add_gateway_payment_mode(integration): def get_payment_gateways(): - """Built-ins, then `commera_payment_gateways` hook results; a hook cannot replace a built-in slug.""" + """Built-ins, then `commera_hooks["payment_gateways"]` results; a hook cannot replace a built-in slug.""" gateways = list(PAYMENT_GATEWAYS) slugs = {gateway["slug"] for gateway in gateways} - for method in frappe.get_hooks("commera_payment_gateways"): + for method in get_handlers("commera_hooks", "payment_gateways"): gateway = frappe.get_attr(method)() if gateway["slug"] not in slugs: gateways.append(gateway) diff --git a/commera/sdk/events.py b/commera/sdk/events.py index 1fd0c911..daba2a1a 100644 --- a/commera/sdk/events.py +++ b/commera/sdk/events.py @@ -13,7 +13,13 @@ "product_updated", "inventory_changed", ), - "commera_hooks": ("validate_cart", "cart_fees", "delivery_options", "payment_methods"), + "commera_hooks": ( + "validate_cart", + "cart_fees", + "delivery_options", + "payment_methods", + "payment_gateways", + ), } diff --git a/commera/tests/test_admin_integrations.py b/commera/tests/test_admin_integrations.py index 49498283..51f1d2e1 100644 --- a/commera/tests/test_admin_integrations.py +++ b/commera/tests/test_admin_integrations.py @@ -148,9 +148,7 @@ def test_every_registered_gateway_is_described(self): ) def test_a_provider_app_registers_its_gateway_through_the_hook(self): - with patch( - "commera.api.admin.payments.frappe.get_hooks", return_value=[f"{__name__}.wallet_gateway"] - ): + with patch("commera.api.admin.payments.get_handlers", return_value=[f"{__name__}.wallet_gateway"]): gateways = get_payment_gateways() reusable = get_gateways_with_capability("reusable_checkout") @@ -158,9 +156,7 @@ def test_a_provider_app_registers_its_gateway_through_the_hook(self): self.assertEqual(reusable, ["Test Wallet"]) def test_a_hook_cannot_replace_a_built_in_gateway(self): - with patch( - "commera.api.admin.payments.frappe.get_hooks", return_value=[f"{__name__}.stripe_impostor"] - ): + with patch("commera.api.admin.payments.get_handlers", return_value=[f"{__name__}.stripe_impostor"]): gateways = get_payment_gateways() stripe = [gateway for gateway in gateways if gateway["slug"] == "stripe"] From b743e7e76b006663a9c059f0b639a612fab9b304 Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Sun, 11 Oct 2026 17:51:23 +0530 Subject: [PATCH 06/10] fix: only gateway orders placed as Administrator go to the cart's shopper bwh_payments applies a webhook's payment as Administrator, so a gateway order placed that way belongs to the cart's shopper. A COD order is placed by whoever is signed in, as on develop. Co-Authored-By: Claude Opus 5.5 --- commera/api/payments.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/commera/api/payments.py b/commera/api/payments.py index ba714ed2..f574f7c5 100644 --- a/commera/api/payments.py +++ b/commera/api/payments.py @@ -381,18 +381,18 @@ def stamp_order_owner(sales_order, shopper: str) -> None: sales_order.db_set("owner", shopper, update_modified=False) -def get_order_shopper(quotation) -> str | None: - # A guest cart has no account to hand the order to. bwh_payments places webhook orders as Administrator. +def get_order_shopper(quotation, by_gateway: bool = False) -> str | None: + # A guest cart has no account to hand the order to. bwh_payments applies a webhook's payment as Administrator. if quotation.custom_guest_cart_key: return None - if not is_guest() and frappe.session.user != "Administrator": + if not is_guest() and not (by_gateway and frappe.session.user == "Administrator"): return frappe.session.user return frappe.db.exists("User", quotation.contact_email) if quotation.contact_email else None def place_order(quotation, payment_mode: str, gateway_amount=None, gateway_reference=None): """Submit the cart and bill it. Called once per payment; the Quotation docstatus enforces that.""" - shopper = get_order_shopper(quotation) + shopper = get_order_shopper(quotation, by_gateway=True) with system_user_session(): fix_payment_schedule_dates(quotation) quotation.flags.ignore_permissions = True From adf9a52bc050adb4adf09f689b363eaf60d869ff Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Sun, 11 Oct 2026 18:04:42 +0530 Subject: [PATCH 07/10] refactor: register payment gateways through commera_payment_gateways A gateway card is a registration, like commera_storefront_blocks, not a checkout handler that takes the cart like the commera_hooks keys. It moves to its own top-level hook, as the extensibility spec names it, and keeps the commera_api_version check through is_supported_app. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 +++--- commera/api/admin/payments.py | 14 +++++++++++--- commera/sdk/events.py | 8 +------- commera/tests/test_admin_integrations.py | 17 +++++++++++++++-- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 5f85f5ad..4dac342c 100644 --- a/README.md +++ b/README.md @@ -56,9 +56,9 @@ Gateway callbacks are idempotent, so a replayed webhook racing a shopper's retur #### Gateways from other apps -A payment provider app adds its own card through `commera_hooks = {"payment_gateways": [...]}` in its -`hooks.py`: dotted paths, each returning `{slug, label, blurb, settings_doctype, docs_url, capabilities}`, -where `label` is the Payment Gateway Profile's name. Commera never names a provider in code; it acts on +A payment provider app adds its own card with `commera_payment_gateways = [...]` in its `hooks.py`, +next to `commera_api_version`: dotted paths, each returning `{slug, label, blurb, settings_doctype, docs_url, +capabilities}`, where `label` is the Payment Gateway Profile's name. Commera never names a provider in code; it acts on `capabilities` instead. With `reusable_checkout`, a shopper who returns to checkout is sent back to the still-open checkout link, and the cart is kept while that payment is pending. Checkout only offers gateways that can take the cart's currency. diff --git a/commera/api/admin/payments.py b/commera/api/admin/payments.py index c67d1941..bea0ae3d 100644 --- a/commera/api/admin/payments.py +++ b/commera/api/admin/payments.py @@ -5,7 +5,7 @@ from commera.api.admin.integrations import describe_integration, save_integration from commera.migrate import add_payment_mode -from commera.plugin_events import get_handlers +from commera.plugin_events import get_handler_app, is_supported_app PROFILE_DOCTYPE = "Payment Gateway Profile" PROFILE_SETTINGS_FIELDNAME = "gateway_settings" @@ -50,10 +50,10 @@ def add_gateway_payment_mode(integration): def get_payment_gateways(): - """Built-ins, then `commera_hooks["payment_gateways"]` results; a hook cannot replace a built-in slug.""" + """Built-ins, then each provider app's `commera_payment_gateways` card; a card cannot replace a built-in.""" gateways = list(PAYMENT_GATEWAYS) slugs = {gateway["slug"] for gateway in gateways} - for method in get_handlers("commera_hooks", "payment_gateways"): + for method in get_gateway_hooks(): gateway = frappe.get_attr(method)() if gateway["slug"] not in slugs: gateways.append(gateway) @@ -109,3 +109,11 @@ def save_payment_integration(slug: str, enabled: int | str, values: dict | str | return save_integration( get_payment_integration(slug), enabled, frappe.parse_json(values) if values else {} ) + + +def get_gateway_hooks() -> list[str]: + return [ + method + for method in frappe.get_hooks("commera_payment_gateways") + if is_supported_app(get_handler_app(method)) + ] diff --git a/commera/sdk/events.py b/commera/sdk/events.py index daba2a1a..1fd0c911 100644 --- a/commera/sdk/events.py +++ b/commera/sdk/events.py @@ -13,13 +13,7 @@ "product_updated", "inventory_changed", ), - "commera_hooks": ( - "validate_cart", - "cart_fees", - "delivery_options", - "payment_methods", - "payment_gateways", - ), + "commera_hooks": ("validate_cart", "cart_fees", "delivery_options", "payment_methods"), } diff --git a/commera/tests/test_admin_integrations.py b/commera/tests/test_admin_integrations.py index 51f1d2e1..dd5bf734 100644 --- a/commera/tests/test_admin_integrations.py +++ b/commera/tests/test_admin_integrations.py @@ -148,15 +148,28 @@ def test_every_registered_gateway_is_described(self): ) def test_a_provider_app_registers_its_gateway_through_the_hook(self): - with patch("commera.api.admin.payments.get_handlers", return_value=[f"{__name__}.wallet_gateway"]): + with patch( + "commera.api.admin.payments.get_gateway_hooks", return_value=[f"{__name__}.wallet_gateway"] + ): gateways = get_payment_gateways() reusable = get_gateways_with_capability("reusable_checkout") self.assertEqual(gateways[-1]["slug"], "test-wallet") self.assertEqual(reusable, ["Test Wallet"]) + def test_a_card_from_an_app_built_for_another_commera_api_is_skipped(self): + with ( + patch("commera.api.admin.payments.frappe.get_hooks", return_value=[f"{__name__}.wallet_gateway"]), + patch("commera.api.admin.payments.is_supported_app", return_value=False), + ): + gateways = get_payment_gateways() + + self.assertNotIn("test-wallet", [gateway["slug"] for gateway in gateways]) + def test_a_hook_cannot_replace_a_built_in_gateway(self): - with patch("commera.api.admin.payments.get_handlers", return_value=[f"{__name__}.stripe_impostor"]): + with patch( + "commera.api.admin.payments.get_gateway_hooks", return_value=[f"{__name__}.stripe_impostor"] + ): gateways = get_payment_gateways() stripe = [gateway for gateway in gateways if gateway["slug"] == "stripe"] From 32dd3c6bb8f51347b205953d2591c921cf02b828 Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Sun, 11 Oct 2026 18:43:22 +0530 Subject: [PATCH 08/10] fix: guard gateway cards and recheck plugins before reusing a checkout - A provider app's card that raises or lacks slug, label or settings_doctype is logged and skipped, like develop's other plugin hooks, instead of failing every checkout, the cart cleanup and the payments settings. - Reusing an open checkout link first runs the checks a new checkout runs: the gateway must still be offered after plugin payment_methods filters, and validate_cart must pass. Co-Authored-By: Claude Opus 5.5 --- commera/api/admin/payments.py | 17 +++++++++-- commera/api/payments.py | 8 ++++-- commera/tests/test_admin_integrations.py | 21 ++++++++++++++ commera/tests/test_reusable_checkout.py | 36 ++++++++++++++++++++++-- 4 files changed, 75 insertions(+), 7 deletions(-) diff --git a/commera/api/admin/payments.py b/commera/api/admin/payments.py index bea0ae3d..6b30b14a 100644 --- a/commera/api/admin/payments.py +++ b/commera/api/admin/payments.py @@ -8,6 +8,7 @@ from commera.plugin_events import get_handler_app, is_supported_app PROFILE_DOCTYPE = "Payment Gateway Profile" +REQUIRED_GATEWAY_FIELDS = {"slug", "label", "settings_doctype"} PROFILE_SETTINGS_FIELDNAME = "gateway_settings" WEBHOOK_PATH = "/api/method/bwh_payments.bwh_payments.webhook.handle?gateway={profile}" @@ -54,8 +55,8 @@ def get_payment_gateways(): gateways = list(PAYMENT_GATEWAYS) slugs = {gateway["slug"] for gateway in gateways} for method in get_gateway_hooks(): - gateway = frappe.get_attr(method)() - if gateway["slug"] not in slugs: + gateway = get_hook_gateway(method) + if gateway and gateway["slug"] not in slugs: gateways.append(gateway) slugs.add(gateway["slug"]) return gateways @@ -117,3 +118,15 @@ def get_gateway_hooks() -> list[str]: for method in frappe.get_hooks("commera_payment_gateways") if is_supported_app(get_handler_app(method)) ] + + +def get_hook_gateway(method: str) -> dict | None: + # Read on every checkout, so a broken provider app is logged and skipped, never raised. + try: + gateway = frappe.get_attr(method)() + if missing := REQUIRED_GATEWAY_FIELDS - set(gateway): + raise ValueError(f"Gateway card is missing {', '.join(sorted(missing))}") + return gateway + except Exception: + frappe.log_error(title=f"commera_payment_gateways hook failed: {method}"[:140], defer_insert=True) + return None diff --git a/commera/api/payments.py b/commera/api/payments.py index f574f7c5..7f40aedc 100644 --- a/commera/api/payments.py +++ b/commera/api/payments.py @@ -108,6 +108,10 @@ def get_reusable_checkout_url( ): return None + # The checks a new checkout runs, so a plugin that has since dropped the gateway or refuses the cart wins. + if gateway not in get_checkout_payment_methods(quotation, strict=True): + return None + validate_cart(quotation) validate_expected_total(quotation, payment_mode, expected_total) return payment_request.order_url @@ -184,9 +188,7 @@ def open_checkout( ): validate_delivery_option(quotation, delivery_option) gateway = resolve_payment_mode(payment_mode) - if gateway in get_reusable_checkout_gateways() and gateway in get_payment_modes_for_currency( - quotation.currency - ): + if gateway in get_reusable_checkout_gateways(): if order_url := get_reusable_checkout_url(quotation, gateway, payment_mode, expected_total): return {"order_url": order_url} diff --git a/commera/tests/test_admin_integrations.py b/commera/tests/test_admin_integrations.py index dd5bf734..276220d4 100644 --- a/commera/tests/test_admin_integrations.py +++ b/commera/tests/test_admin_integrations.py @@ -13,6 +13,7 @@ get_link_options, ) from commera.api.admin.payments import ( + PAYMENT_GATEWAYS, get_gateways_with_capability, get_payment_gateways, get_payment_integrations, @@ -166,6 +167,18 @@ def test_a_card_from_an_app_built_for_another_commera_api_is_skipped(self): self.assertNotIn("test-wallet", [gateway["slug"] for gateway in gateways]) + def test_a_failing_card_is_logged_and_skipped_instead_of_breaking_the_registry(self): + hooks = [f"{__name__}.broken_gateway", f"{__name__}.card_without_slug", f"{__name__}.wallet_gateway"] + with ( + patch("commera.api.admin.payments.get_gateway_hooks", return_value=hooks), + patch("commera.api.admin.payments.frappe.log_error") as log_error, + ): + gateways = get_payment_gateways() + + self.assertEqual(gateways[-1]["slug"], "test-wallet") + self.assertEqual(len(gateways), len(PAYMENT_GATEWAYS) + 1) + self.assertEqual(log_error.call_count, 2) + def test_a_hook_cannot_replace_a_built_in_gateway(self): with patch( "commera.api.admin.payments.get_gateway_hooks", return_value=[f"{__name__}.stripe_impostor"] @@ -227,3 +240,11 @@ def wallet_gateway(): def stripe_impostor(): return {**wallet_gateway(), "slug": "stripe", "settings_doctype": "Impostor Settings"} + + +def broken_gateway(): + raise RuntimeError("provider app bug") + + +def card_without_slug(): + return {"label": "No Slug", "settings_doctype": "No Slug Settings"} diff --git a/commera/tests/test_reusable_checkout.py b/commera/tests/test_reusable_checkout.py index e5f4bd35..a9a52166 100644 --- a/commera/tests/test_reusable_checkout.py +++ b/commera/tests/test_reusable_checkout.py @@ -63,7 +63,6 @@ def test_a_repeat_checkout_reuses_the_open_link_of_a_reusable_gateway(self): with ( patch("commera.api.payments.resolve_payment_mode", return_value=GATEWAY), patch("commera.api.payments.get_reusable_checkout_gateways", return_value=[GATEWAY]), - patch("commera.api.payments.get_payment_modes_for_currency", return_value=[GATEWAY]), patch("commera.api.payments.get_reusable_checkout_url", return_value="https://wallet.test/old"), patch("commera.api.payments.validate_cart_is_not_in_checkout") as validate_cart, patch("commera.api.payments.frappe.get_doc") as get_doc, @@ -108,6 +107,8 @@ def test_a_reused_link_must_match_the_total_the_shopper_saw(self): with ( patch("commera.api.payments.get_open_gateway_payment_request", return_value="GPR-1"), patch("commera.api.payments.frappe.get_doc", return_value=payment_request), + patch("commera.api.payments.get_checkout_payment_methods", return_value=[GATEWAY]), + patch("commera.api.payments.validate_cart"), patch( "commera.api.payments.to_minor_units", side_effect=lambda amount, currency: int(amount * 100) ), @@ -188,7 +189,6 @@ def test_a_gateway_without_the_capability_never_reuses_a_link(self): with ( patch("commera.api.payments.resolve_payment_mode", return_value=GATEWAY), patch("commera.api.payments.get_reusable_checkout_gateways", return_value=[]), - patch("commera.api.payments.get_payment_modes_for_currency", return_value=[GATEWAY]), patch("commera.api.payments.get_reusable_checkout_url") as reuse, patch("commera.api.payments.validate_cart_is_not_in_checkout", side_effect=StopIteration), ): @@ -196,3 +196,35 @@ def test_a_gateway_without_the_capability_never_reuses_a_link(self): open_checkout(quotation, GATEWAY) reuse.assert_not_called() + + def test_a_link_is_not_reused_once_a_plugin_stops_offering_its_gateway(self): + with self.patch_open_request(): + with patch("commera.api.payments.get_checkout_payment_methods", return_value=["Stripe"]): + self.assertIsNone(get_reusable_checkout_url(make_quotation(), GATEWAY, GATEWAY, None)) + + def test_a_link_is_not_reused_for_a_cart_a_plugin_refuses(self): + with self.patch_open_request(): + with ( + patch("commera.api.payments.get_checkout_payment_methods", return_value=[GATEWAY]), + patch("commera.api.payments.validate_cart", side_effect=frappe.ValidationError) as validate, + ): + with self.assertRaises(frappe.ValidationError): + get_reusable_checkout_url(make_quotation(), GATEWAY, GATEWAY, None) + + validate.assert_called_once() + + def patch_open_request(self): + payment_request = SimpleNamespace( + status="Pending", + gateway=GATEWAY, + currency_code="USD", + amount=20, + order_url="https://wallet.test/old", + sync_status=Mock(), + ) + return patch.multiple( + "commera.api.payments", + get_open_gateway_payment_request=Mock(return_value="GPR-1"), + to_minor_units=Mock(side_effect=lambda amount, currency: int(amount * 100)), + frappe=SimpleNamespace(get_doc=Mock(return_value=payment_request)), + ) From a2abc18a7803080f202d08007a63a8be45d38b75 Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Mon, 12 Oct 2026 12:23:23 +0530 Subject: [PATCH 09/10] refactor: filter checkout gateways through bwh_payments' public API bwh_payments v1-stable provides the per-currency gateway list as api.get_gateway_options, built on each gateway's supported_currencies(). Commera reads that instead of a utils helper that no longer exists. Co-Authored-By: Claude Opus 5.5 --- commera/api/payments.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/commera/api/payments.py b/commera/api/payments.py index 7f40aedc..89e63e7a 100644 --- a/commera/api/payments.py +++ b/commera/api/payments.py @@ -1,11 +1,8 @@ from contextlib import contextmanager import frappe -from bwh_payments.bwh_payments.utils import ( - get_available_payment_modes, - get_payment_modes_for_currency, - resolve_payment_mode, -) +from bwh_payments.api import get_gateway_options +from bwh_payments.bwh_payments.utils import get_available_payment_modes, resolve_payment_mode from bwh_payments.currency import to_minor_units from erpnext.accounts.doctype.journal_entry.journal_entry import get_default_bank_cash_account from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry @@ -62,6 +59,10 @@ class CheckoutPriceChangedError(frappe.ValidationError): REUSABLE_CHECKOUT = "reusable_checkout" +def get_payment_modes_for_currency(currency: str) -> list[str]: + return [option.name for option in get_gateway_options(currency)] + + def is_cod(payment_mode: str | None) -> bool: return (payment_mode or "").strip().casefold() == COD_PAYMENT_MODE.casefold() From f3ef321649f8b449379b0bacc046a5e1d92c333d Mon Sep 17 00:00:00 2001 From: Isha Dewangan Date: Mon, 12 Oct 2026 21:15:00 +0530 Subject: [PATCH 10/10] refactor: read payment gateway cards from bwh_payments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings → Payments kept its own list of Razorpay, Stripe, Telr and Tabby, and bwh_payments registered PayPal and Braintree through commera_payment_gateways, so a gateway had to be named in Commera or bwh_payments had to know about Commera. Cards now come from bwh_payments.api.get_gateways(): every gateway it ships, with its description, docs link, settings DocType and capabilities. commera_payment_gateways stays for provider apps outside bwh_payments. Cards are listed in bwh_payments' order (by name). Co-Authored-By: Claude Opus 5.5 --- README.md | 5 ++- commera/api/admin/payments.py | 48 +++++++----------------- commera/tests/test_admin_integrations.py | 47 ++++++++++++++++++++--- 3 files changed, 60 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 4dac342c..128ffbeb 100644 --- a/README.md +++ b/README.md @@ -50,13 +50,16 @@ Providers are configured from the dashboard rather than the desk. Each keeps its - **Stripe** — Cards and wallets, worldwide. - **Telr** — Cards and local methods across the GCC. - **Tabby** — Buy now, pay later in four instalments. MENA. +- **PayPal** — PayPal wallet payments. Not for INR or Gulf currencies. +- **Braintree** — Cards with 3-D Secure, worldwide. Needs a merchant account per currency. - **Cash on delivery** — With its own fee, a threshold above which the fee is waived, and the account it books to. Gateway callbacks are idempotent, so a replayed webhook racing a shopper's return cannot bill twice. #### Gateways from other apps -A payment provider app adds its own card with `commera_payment_gateways = [...]` in its `hooks.py`, +Every gateway bwh_payments ships gets a card on its own, from `bwh_payments.api.get_gateways()`. A payment +provider app outside bwh_payments adds its own card with `commera_payment_gateways = [...]` in its `hooks.py`, next to `commera_api_version`: dotted paths, each returning `{slug, label, blurb, settings_doctype, docs_url, capabilities}`, where `label` is the Payment Gateway Profile's name. Commera never names a provider in code; it acts on `capabilities` instead. With `reusable_checkout`, a shopper who returns to checkout is sent back to the diff --git a/commera/api/admin/payments.py b/commera/api/admin/payments.py index 6b30b14a..0c317345 100644 --- a/commera/api/admin/payments.py +++ b/commera/api/admin/payments.py @@ -2,6 +2,7 @@ # For license information, please see license.txt import frappe +from bwh_payments.api import GatewayInfo, get_gateways from commera.api.admin.integrations import describe_integration, save_integration from commera.migrate import add_payment_mode @@ -12,38 +13,6 @@ PROFILE_SETTINGS_FIELDNAME = "gateway_settings" WEBHOOK_PATH = "/api/method/bwh_payments.bwh_payments.webhook.handle?gateway={profile}" -# Order here is the order the cards render in. -PAYMENT_GATEWAYS = ( - { - "slug": "razorpay", - "label": "Razorpay", - "blurb": "Cards, UPI, netbanking and wallets. India.", - "settings_doctype": "Razorpay Gateway Settings", - "docs_url": "https://dashboard.razorpay.com/app/website-app-settings/webhooks", - }, - { - "slug": "stripe", - "label": "Stripe", - "blurb": "Cards and wallets, worldwide.", - "settings_doctype": "Stripe Gateway Settings", - "docs_url": "https://dashboard.stripe.com/apikeys", - }, - { - "slug": "telr", - "label": "Telr", - "blurb": "Cards and local methods across the GCC.", - "settings_doctype": "Telr Gateway Settings", - "docs_url": "https://telr.com/support/knowledge-base/hosted-payment-page-integration-guide/", - }, - { - "slug": "tabby", - "label": "Tabby", - "blurb": "Buy now, pay later in four instalments. MENA.", - "settings_doctype": "Tabby Gateway Settings", - "docs_url": "https://docs.tabby.ai/", - }, -) - def add_gateway_payment_mode(integration): """ERPNext refuses to post a Payment Entry for a gateway with no matching Mode of Payment.""" @@ -51,8 +20,8 @@ def add_gateway_payment_mode(integration): def get_payment_gateways(): - """Built-ins, then each provider app's `commera_payment_gateways` card; a card cannot replace a built-in.""" - gateways = list(PAYMENT_GATEWAYS) + """bwh_payments' gateways, then each provider app's `commera_payment_gateways` card; a card cannot replace one.""" + gateways = [get_bwh_payments_card(gateway) for gateway in get_gateways()] slugs = {gateway["slug"] for gateway in gateways} for method in get_gateway_hooks(): gateway = get_hook_gateway(method) @@ -112,6 +81,17 @@ def save_payment_integration(slug: str, enabled: int | str, values: dict | str | ) +def get_bwh_payments_card(gateway: GatewayInfo) -> dict: + return { + "slug": frappe.scrub(gateway.name), + "label": gateway.name, + "blurb": gateway.description, + "settings_doctype": gateway.settings_doctype, + "docs_url": gateway.docs_url, + "capabilities": gateway.capabilities, + } + + def get_gateway_hooks() -> list[str]: return [ method diff --git a/commera/tests/test_admin_integrations.py b/commera/tests/test_admin_integrations.py index 276220d4..e33b5d93 100644 --- a/commera/tests/test_admin_integrations.py +++ b/commera/tests/test_admin_integrations.py @@ -3,6 +3,7 @@ from unittest.mock import patch import frappe +from bwh_payments.api import GatewayInfo, get_gateways from bwh_payments.bwh_payments.utils import get_available_payment_modes from frappe.tests import IntegrationTestCase from frappe.utils.password import get_decrypted_password @@ -13,7 +14,6 @@ get_link_options, ) from commera.api.admin.payments import ( - PAYMENT_GATEWAYS, get_gateways_with_capability, get_payment_gateways, get_payment_integrations, @@ -138,7 +138,7 @@ def test_every_registered_gateway_is_described(self): self.assertEqual( [card["slug"] for card in cards], [gateway["slug"] for gateway in get_payment_gateways()] ) - self.assertEqual([card["slug"] for card in cards][:4], ["razorpay", "stripe", "telr", "tabby"]) + self.assertTrue({"razorpay", "stripe", "telr", "tabby"} <= {card["slug"] for card in cards}) for card in cards: self.assertTrue(card["available"], f"{card['slug']} settings doctype is missing") self.assertTrue(card["webhook_url"].endswith(f"gateway={card['label']}")) @@ -148,9 +148,35 @@ def test_every_registered_gateway_is_described(self): [field["fieldname"] for group in card["groups"] for field in group["fields"]], ) + def test_bwh_payments_gateways_need_no_registration_in_commera(self): + with ( + patch("commera.api.admin.payments.get_gateways", return_value=[wallet_gateway_info()]), + patch("commera.api.admin.payments.get_gateway_hooks", return_value=[]), + ): + gateways = get_payment_gateways() + reusable = get_gateways_with_capability("reusable_checkout") + + self.assertEqual( + gateways, + [ + { + "slug": "test_wallet", + "label": "Test Wallet", + "blurb": "A wallet bwh_payments describes.", + "settings_doctype": "Test Wallet Settings", + "docs_url": "https://wallet.test/docs", + "capabilities": ["reusable_checkout"], + } + ], + ) + self.assertEqual(reusable, ["Test Wallet"]) + def test_a_provider_app_registers_its_gateway_through_the_hook(self): - with patch( - "commera.api.admin.payments.get_gateway_hooks", return_value=[f"{__name__}.wallet_gateway"] + with ( + patch("commera.api.admin.payments.get_gateways", return_value=[]), + patch( + "commera.api.admin.payments.get_gateway_hooks", return_value=[f"{__name__}.wallet_gateway"] + ), ): gateways = get_payment_gateways() reusable = get_gateways_with_capability("reusable_checkout") @@ -160,6 +186,7 @@ def test_a_provider_app_registers_its_gateway_through_the_hook(self): def test_a_card_from_an_app_built_for_another_commera_api_is_skipped(self): with ( + patch("commera.api.admin.payments.get_gateways", return_value=[]), patch("commera.api.admin.payments.frappe.get_hooks", return_value=[f"{__name__}.wallet_gateway"]), patch("commera.api.admin.payments.is_supported_app", return_value=False), ): @@ -176,7 +203,7 @@ def test_a_failing_card_is_logged_and_skipped_instead_of_breaking_the_registry(s gateways = get_payment_gateways() self.assertEqual(gateways[-1]["slug"], "test-wallet") - self.assertEqual(len(gateways), len(PAYMENT_GATEWAYS) + 1) + self.assertEqual(len(gateways), len(get_gateways()) + 1) self.assertEqual(log_error.call_count, 2) def test_a_hook_cannot_replace_a_built_in_gateway(self): @@ -238,6 +265,16 @@ def wallet_gateway(): } +def wallet_gateway_info(): + return GatewayInfo( + name="Test Wallet", + settings_doctype="Test Wallet Settings", + description="A wallet bwh_payments describes.", + docs_url="https://wallet.test/docs", + capabilities=["reusable_checkout"], + ) + + def stripe_impostor(): return {**wallet_gateway(), "slug": "stripe", "settings_doctype": "Impostor Settings"}