From 4cfa42921f32377c725cd22d3912b9ed140fbd9f Mon Sep 17 00:00:00 2001 From: Jatin3128 <140256508+Jatin3128@users.noreply.github.com> Date: Wed, 26 Aug 2026 16:16:40 +0530 Subject: [PATCH] fix(accounts): resolve subscription plans for any reference doctype in Payment Request (#58438) fix(accounts): resolve subscription plans for any reference doctype and require read permission get_subscription_details() was hardcoded to only resolve plans for Sales Invoice, but is_a_subscription in make_payment_request() was set for any reference doctype with a `subscription` field. Since Purchase Invoice also has this field (supplier-side subscriptions), creating a Payment Request against a subscription-linked Purchase Invoice set is_a_subscription=1 with an empty subscription_plans table. get_subscription_details() is also whitelisted with no permission check, letting any logged-in user query which Subscription/plan/qty is linked to an arbitrary Sales Invoice or Purchase Invoice. Make plan resolution generic (guarded by Meta.has_field so doctypes without a subscription field never hit a nonexistent column), derive is_a_subscription from the resolved plans so the two can't disagree, and add a frappe.has_permission read check before returning any data. --- .../payment_request/payment_request.py | 22 ++--- .../payment_request/test_payment_request.py | 91 ++++++++++++++++++- 2 files changed, 101 insertions(+), 12 deletions(-) diff --git a/erpnext/accounts/doctype/payment_request/payment_request.py b/erpnext/accounts/doctype/payment_request/payment_request.py index d0d176f20d7..6b23e6ba3e0 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.py +++ b/erpnext/accounts/doctype/payment_request/payment_request.py @@ -874,7 +874,8 @@ def make_payment_request(**args): if not party_account_currency: party_account = get_party_account(party_type, ref_doc.get(party_type.lower()), ref_doc.company) party_account_currency = get_account_currency(party_account) - is_a_subscription = 1 if ref_doc.get("subscription") else 0 + + subscription_plans = get_subscription_details(ref_doc.doctype, ref_doc.name) pr.update( { "payment_gateway_account": gateway_account.get("name"), @@ -906,15 +907,14 @@ def make_payment_request(**args): or gateway_account.get("payment_channel", "Email") != "Email" ), "phone_number": args.get("phone_number") if args.get("phone_number") else None, - "is_a_subscription": is_a_subscription, + "is_a_subscription": 1 if subscription_plans else 0, } ) if selected_payment_schedules: apply_payment_references(pr, payment_reference) - if is_a_subscription: - values = get_subscription_details(ref_doc.doctype, ref_doc.name) + if subscription_plans: pr.set( "subscription_plans", [ @@ -922,7 +922,7 @@ def make_payment_request(**args): "plan": row.plan, "qty": row.qty, } - for row in values + for row in subscription_plans ], ) # Dimensions @@ -1238,16 +1238,18 @@ def get_dummy_message(doc): @frappe.whitelist() -def get_subscription_details(reference_doctype: str, reference_name: str): - if reference_doctype != "Sales Invoice": +def get_subscription_details(reference_doctype: str, reference_name: str) -> list[dict]: + frappe.has_permission(reference_doctype, "read", reference_name, throw=True) + + if not frappe.get_meta(reference_doctype).has_field("subscription"): return [] - subscription = frappe.db.get_value("Sales Invoice", reference_name, "subscription") + subscription = frappe.db.get_value(reference_doctype, reference_name, "subscription") if not subscription: return [] - subscription_plan = frappe.get_all( + return frappe.get_all( "Subscription Plan Detail", filters={"parent": subscription, "parenttype": "Subscription", "parentfield": "plans"}, fields=[ @@ -1256,8 +1258,6 @@ def get_subscription_details(reference_doctype: str, reference_name: str): ], ) - return subscription_plan - @frappe.whitelist() def make_payment_order(source_name: str, target_doc: str | dict | Document | None = None): diff --git a/erpnext/accounts/doctype/payment_request/test_payment_request.py b/erpnext/accounts/doctype/payment_request/test_payment_request.py index 851f7d41e71..51bb1c0ce98 100644 --- a/erpnext/accounts/doctype/payment_request/test_payment_request.py +++ b/erpnext/accounts/doctype/payment_request/test_payment_request.py @@ -11,7 +11,10 @@ from frappe.utils import add_days, nowdate from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry from erpnext.accounts.doctype.payment_entry.test_payment_entry import create_payment_terms_template -from erpnext.accounts.doctype.payment_request.payment_request import make_payment_request +from erpnext.accounts.doctype.payment_request.payment_request import ( + get_subscription_details, + make_payment_request, +) from erpnext.accounts.doctype.purchase_invoice.test_purchase_invoice import make_purchase_invoice from erpnext.accounts.doctype.sales_invoice.test_sales_invoice import create_sales_invoice from erpnext.accounts.doctype.subscription.test_subscription import ( @@ -2066,3 +2069,89 @@ class TestPaymentRequestV2Gateway(ERPNextTestSuite): self.assertEqual(len(payment_request.subscription_plans), 0) self.assertEqual(payment_request.reference_doctype, "Sales Invoice") self.assertEqual(payment_request.reference_name, si.name) + + def test_payment_request_with_subscription_for_purchase_invoice(self): + make_plans() + + subscription_plan = frappe.get_doc("Subscription Plan", "_Test Plan Name") + subscription_plan.payment_gateway = "_Test Gateway - INR - _TC" + subscription_plan.save() + + subscription = create_subscription( + party_type="Supplier", + party="_Test Supplier", + plans=[{"plan": "_Test Plan Name", "qty": 1}], + start_date=nowdate(), + generate_invoice_at="Prepaid (bill at period start)", + submit_invoice=1, + ) + invoice_name = frappe.get_value( + "Purchase Invoice", + { + "subscription": subscription.name, + "docstatus": 1, + "is_return": 0, + }, + "name", + order_by="from_date asc", + ) + + payment_request = make_payment_request( + dt="Purchase Invoice", + dn=invoice_name, + party_type="Supplier", + party="_Test Supplier", + recipient_id="test@example.com", + ) + + self.assertEqual(payment_request.is_a_subscription, 1) + self.assertEqual(len(payment_request.subscription_plans), 1) + + subscription_plan = payment_request.subscription_plans[0] + self.assertEqual(subscription_plan.plan, "_Test Plan Name") + self.assertEqual(subscription_plan.qty, 1) + self.assertEqual(payment_request.reference_doctype, "Purchase Invoice") + self.assertEqual(payment_request.reference_name, invoice_name) + + def test_payment_request_without_subscription_for_purchase_invoice(self): + pi = make_purchase_invoice() + payment_request = make_payment_request( + dt="Purchase Invoice", + dn=pi.name, + party_type="Supplier", + party=pi.supplier, + recipient_id="test@example.com", + ) + self.assertEqual(payment_request.is_a_subscription, 0) + self.assertEqual(len(payment_request.subscription_plans), 0) + self.assertEqual(payment_request.reference_doctype, "Purchase Invoice") + self.assertEqual(payment_request.reference_name, pi.name) + + def test_get_subscription_details_returns_empty_for_doctype_without_subscription_field(self): + so = make_sales_order() + self.assertEqual(get_subscription_details("Sales Order", so.name), []) + + def test_get_subscription_details_requires_read_permission_on_reference(self): + si = create_sales_invoice() + + restricted_user = "no-roles@example.com" + if not frappe.db.exists("User", restricted_user): + user = frappe.new_doc("User") + user.email = restricted_user + user.first_name = "No Roles" + user.send_welcome_email = 0 + user.insert() + + accounts_user = "accounts-user@example.com" + if not frappe.db.exists("User", accounts_user): + user = frappe.new_doc("User") + user.email = accounts_user + user.first_name = "Accounts" + user.send_welcome_email = 0 + user.add_roles("Accounts User") + + with self.set_user(restricted_user): + self.assertRaises(frappe.PermissionError, get_subscription_details, "Sales Invoice", si.name) + + with self.set_user(accounts_user): + self.assertEqual(get_subscription_details("Sales Invoice", si.name), [])