From dee7bd8d64d35e1def1f90c6b216b8bc2f1c30b9 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 4 Jun 2026 05:22:02 +0530 Subject: [PATCH] fix(subscription): correct billing/deferred bugs and tighten guards (backport #55554) (#55610) fix(subscription): correct billing/deferred bugs and tighten guards (#55554) (cherry picked from commit d54db2e0ca74fc8d420df9e44c1cb68a84cb170f) Co-authored-by: Jatin3128 <140256508+Jatin3128@users.noreply.github.com> --- .../doctype/subscription/subscription.py | 59 +++++++++++-------- .../doctype/subscription/test_subscription.py | 58 +++++++++++++++++- 2 files changed, 92 insertions(+), 25 deletions(-) diff --git a/erpnext/accounts/doctype/subscription/subscription.py b/erpnext/accounts/doctype/subscription/subscription.py index 642f918c3b1..f95056a5390 100644 --- a/erpnext/accounts/doctype/subscription/subscription.py +++ b/erpnext/accounts/doctype/subscription/subscription.py @@ -269,7 +269,7 @@ class Subscription(Document): Returns `True` if the grace period for the `Subscription` has passed """ if not self.current_invoice_is_past_due(): - return + return False grace_period = cint(frappe.get_value("Subscription Settings", None, "grace_period")) return getdate(posting_date) >= getdate(add_days(self.current_invoice.due_date, grace_period)) @@ -281,6 +281,9 @@ class Subscription(Document): if not self.current_invoice or self.is_paid(self.current_invoice): return False + if not self.current_invoice.due_date: + return False + return getdate(posting_date) >= getdate(self.current_invoice.due_date) @property @@ -345,7 +348,13 @@ class Subscription(Document): frappe.throw(_("Trial Period Start date cannot be after Subscription Start Date")) def validate_end_date(self) -> None: + if not self.plans: + return + billing_cycle_info = self.get_billing_cycle_data() + if not billing_cycle_info: + return + end_date = add_to_date(self.start_date, **billing_cycle_info) if self.end_date and getdate(self.end_date) <= getdate(end_date): @@ -514,7 +523,7 @@ class Subscription(Document): item_code = plan_doc.item - if self.party == "Customer": + if self.party_type == "Customer": deferred_field = "enable_deferred_revenue" else: deferred_field = "enable_deferred_expense" @@ -598,19 +607,28 @@ class Subscription(Document): if self.has_outstanding_invoice() and not self.generate_new_invoices_past_due_date: return False - if self.generate_invoice_at == "Beginning of the current subscription period" and ( - getdate(posting_date) == getdate(self.current_invoice_start) - ): - return True - elif self.generate_invoice_at == "Days before the current subscription period" and ( - getdate(posting_date) == getdate(add_days(self.current_invoice_start, -1 * self.number_of_days)) - ): - return True - elif getdate(posting_date) == getdate(self.current_invoice_end): - return True + posting = getdate(posting_date) + + if self.generate_invoice_at == "Beginning of the current subscription period": + trigger = getdate(self.current_invoice_start) + elif self.generate_invoice_at == "Days before the current subscription period": + trigger = getdate(add_days(self.current_invoice_start, -1 * self.number_of_days)) else: + trigger = getdate(self.current_invoice_end) + + if posting < trigger: return False + # Cap the late-fire window at one billing cycle past the period end so a + # multi-year gap doesn't retroactively bill cycle after cycle in one call. + billing_cycle_info = self.get_billing_cycle_data() + if billing_cycle_info: + upper = getdate(add_to_date(self.current_invoice_end, **billing_cycle_info)) + else: + upper = getdate(self.current_invoice_end) + + return posting <= upper + def is_current_invoice_generated( self, _current_start_date: DateTimeLikeObject | None = None, @@ -650,13 +668,6 @@ class Subscription(Document): if invoice: return frappe.get_doc(self.invoice_document_type, invoice[0]) - def cancel_subscription_at_period_end(self) -> None: - """ - Called when `Subscription.cancel_at_period_end` is truthy - """ - self.status = "Cancelled" - self.cancelation_date = nowdate() - @property def invoices(self) -> list[dict]: return frappe.get_all( @@ -703,7 +714,7 @@ class Subscription(Document): self.status = "Cancelled" self.cancelation_date = nowdate() - if to_generate_invoice and self.cancelation_date >= self.current_invoice_start: + if to_generate_invoice and getdate(self.cancelation_date) >= getdate(self.current_invoice_start): self.generate_invoice(self.current_invoice_start, self.cancelation_date) self.save() @@ -731,7 +742,7 @@ class Subscription(Document): """ # Don't process future subscriptions - if nowdate() < self.current_invoice_start: + if getdate(nowdate()) < getdate(self.current_invoice_start): frappe.msgprint(_("Subscription for Future dates cannot be processed.")) return @@ -770,10 +781,10 @@ def process_all(subscription: list, posting_date: DateTimeLikeObject | None = No for subscription_name in subscription: try: - subscription = frappe.get_doc("Subscription", subscription_name) - subscription.process(posting_date) + sub = frappe.get_doc("Subscription", subscription_name) + sub.process(posting_date) if not frappe.in_test: frappe.db.commit() except frappe.ValidationError: frappe.db.rollback() - subscription.log_error("Subscription failed") + sub.log_error("Subscription failed") diff --git a/erpnext/accounts/doctype/subscription/test_subscription.py b/erpnext/accounts/doctype/subscription/test_subscription.py index dbec8be72c6..158733bd404 100644 --- a/erpnext/accounts/doctype/subscription/test_subscription.py +++ b/erpnext/accounts/doctype/subscription/test_subscription.py @@ -17,7 +17,7 @@ from frappe.utils.data import ( ) from erpnext.accounts.doctype.payment_entry.payment_entry import get_payment_entry -from erpnext.accounts.doctype.subscription.subscription import get_prorata_factor +from erpnext.accounts.doctype.subscription.subscription import Subscription, get_prorata_factor, process_all from erpnext.tests.utils import ERPNextTestSuite @@ -638,6 +638,62 @@ class TestSubscription(ERPNextTestSuite): self.assertRaises(frappe.ValidationError, subscription.process, posting_date=add_days(start_date, 7)) + def test_invoice_generated_when_scheduler_runs_one_day_late(self): + subscription = create_subscription(start_date="2018-01-01") + self.assertEqual(subscription.current_invoice_end, "2018-01-31") + + subscription.process(posting_date="2018-02-01") + self.assertEqual(len(subscription.invoices), 1) + + def test_deferred_revenue_applied_for_customer_subscription(self): + item_code = "_Test Non Stock Item" + frappe.db.set_value("Item", item_code, "enable_deferred_revenue", 1) + try: + subscription = create_subscription(start_date="2018-01-01") + items = subscription.get_items_from_plans(subscription.plans) + self.assertEqual(items[0].get("enable_deferred_revenue"), 1) + self.assertEqual(getdate(items[0]["service_start_date"]), getdate("2018-01-01")) + self.assertEqual(getdate(items[0]["service_end_date"]), getdate("2018-01-31")) + finally: + frappe.db.set_value("Item", item_code, "enable_deferred_revenue", 0) + + def test_validate_end_date_with_no_plans_does_not_crash(self): + sub = frappe.new_doc("Subscription") + sub.party_type = "Customer" + sub.party = "_Test Customer" + sub.company = "_Test Company" + sub.start_date = "2018-01-01" + sub.end_date = "2018-03-01" + try: + sub.validate_end_date() + except TypeError as e: + self.fail(f"validate_end_date crashed with no plans: {e}") + + def test_process_all_logs_error_when_first_subscription_fails(self): + sub1 = create_subscription(start_date="2018-01-01") + sub2 = create_subscription(start_date="2018-01-02") + + processed = [] + original_process = Subscription.process + original_rollback = frappe.db.rollback + + def patched(self, posting_date=None): + processed.append(self.name) + if self.name == sub1.name: + raise frappe.ValidationError("forced failure") + + Subscription.process = patched + # process_all calls frappe.db.rollback() on error which would otherwise wipe + # the test transaction; stub it so we can observe the iteration in isolation. + frappe.db.rollback = lambda *a, **kw: None + try: + process_all([sub1.name, sub2.name]) + finally: + Subscription.process = original_process + frappe.db.rollback = original_rollback + + self.assertEqual(processed, [sub1.name, sub2.name]) + def test_subscription_auto_completion(self): create_plan( plan_name="_Test Plan 3 Day",