mirror of
https://github.com/frappe/erpnext.git
synced 2026-08-27 05:45:19 +00:00
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.
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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), [])
|
||||
|
||||
Reference in New Issue
Block a user