From 0f069e13da321a53581bca8e23f9a50758ada306 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 8 Jun 2026 07:33:08 +0530 Subject: [PATCH] fix(subscription): bill on creation and keep status in sync with invoices (backport #55615) (#55701) * fix(subscription): bill on creation and keep status in sync with invoices (#55615) (cherry picked from commit bb36e956ace763dc95c8389dc239c0ba1e7dddf1) # Conflicts: # erpnext/accounts/doctype/sales_invoice/sales_invoice.py # erpnext/accounts/utils.py * fix(subscription): resolve cherry-pick conflicts for version-16-hotfix backport --------- Co-authored-by: Jatin3128 <140256508+Jatin3128@users.noreply.github.com> Co-authored-by: Jatin3128 --- .../doctype/payment_entry/payment_entry.py | 15 ++ .../purchase_invoice/purchase_invoice.py | 11 +- .../doctype/sales_invoice/sales_invoice.py | 5 + .../doctype/subscription/subscription.py | 41 +++- .../doctype/subscription/test_subscription.py | 196 ++++++++++++------ erpnext/accounts/utils.py | 13 ++ erpnext/hooks.py | 2 +- 7 files changed, 211 insertions(+), 72 deletions(-) diff --git a/erpnext/accounts/doctype/payment_entry/payment_entry.py b/erpnext/accounts/doctype/payment_entry/payment_entry.py index 4b35ed9fa0e..68b54cd9d3a 100644 --- a/erpnext/accounts/doctype/payment_entry/payment_entry.py +++ b/erpnext/accounts/doctype/payment_entry/payment_entry.py @@ -208,6 +208,7 @@ class PaymentEntry(AccountsController): self.make_gl_entries() self.update_outstanding_amounts() self.set_status() + self.trigger_invoice_update_for_subscriptions() def validate_for_repost(self): validate_docs_for_voucher_types(["Payment Entry"]) @@ -314,6 +315,7 @@ class PaymentEntry(AccountsController): self.update_outstanding_amounts() self.delink_advance_entry_references() self.set_status() + self.trigger_invoice_update_for_subscriptions() def update_payment_requests(self, cancel=False): from erpnext.accounts.doctype.payment_request.payment_request import ( @@ -505,6 +507,19 @@ class PaymentEntry(AccountsController): doc = frappe.get_lazy_doc(reference.reference_doctype, reference.reference_name) doc.delink_advance_entries(self.name) + def trigger_invoice_update_for_subscriptions(self): + invoice_names = set() + for ref in self.references: + if ref.reference_doctype in ("Sales Invoice", "Purchase Invoice"): + invoice_names.add((ref.reference_doctype, ref.reference_name)) + + for doctype, name in invoice_names: + try: + doc = frappe.get_doc(doctype, name) + doc.refresh_subscription_status() + except Exception: + frappe.log_error(_("Failed to update subscription status for {0} {1}").format(doctype, name)) + def set_missing_values(self): if self.payment_type == "Internal Transfer": for field in ( diff --git a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py index 9161ab6a932..0ba5cd446c1 100644 --- a/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py +++ b/erpnext/accounts/doctype/purchase_invoice/purchase_invoice.py @@ -32,7 +32,12 @@ from erpnext.accounts.general_ledger import ( merge_similar_entries, ) from erpnext.accounts.party import get_due_date, get_party_account -from erpnext.accounts.utils import get_account_currency, get_fiscal_year, update_voucher_outstanding +from erpnext.accounts.utils import ( + get_account_currency, + get_fiscal_year, + refresh_subscription_status, + update_voucher_outstanding, +) from erpnext.assets.doctype.asset.asset import is_cwip_accounting_enabled from erpnext.assets.doctype.asset_category.asset_category import get_asset_category_account from erpnext.controllers.accounts_controller import merge_taxes, validate_account_head @@ -823,6 +828,10 @@ class PurchaseInvoice(BuyingController): self.validate_for_repost() self.repost_accounting_entries() + def refresh_subscription_status(self): + if self.get("subscription"): + refresh_subscription_status(self.subscription) + def make_gl_entries(self, gl_entries=None, from_repost=False): update_outstanding = "No" if (cint(self.is_paid) or self.write_off_account) else "Yes" if self.docstatus == 1: diff --git a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py index 862cdcd70b6..53e0647b4e6 100644 --- a/erpnext/accounts/doctype/sales_invoice/sales_invoice.py +++ b/erpnext/accounts/doctype/sales_invoice/sales_invoice.py @@ -31,6 +31,7 @@ from erpnext.accounts.general_ledger import get_round_off_account_and_cost_cente from erpnext.accounts.party import get_due_date, get_party_account, get_party_details from erpnext.accounts.utils import ( get_account_currency, + refresh_subscription_status, update_voucher_outstanding, ) from erpnext.assets.doctype.asset.asset import split_asset @@ -808,6 +809,10 @@ class SalesInvoice(SellingController): "set_default_payment": pos.get("set_grand_total_to_default_mop", 1), } + def refresh_subscription_status(self): + if self.get("subscription"): + refresh_subscription_status(self.subscription) + @frappe.whitelist() def reset_mode_of_payments(self): if self.pos_profile: diff --git a/erpnext/accounts/doctype/subscription/subscription.py b/erpnext/accounts/doctype/subscription/subscription.py index f95056a5390..570cfd847f9 100644 --- a/erpnext/accounts/doctype/subscription/subscription.py +++ b/erpnext/accounts/doctype/subscription/subscription.py @@ -86,6 +86,39 @@ class Subscription(Document): # update start just before the subscription doc is created self.update_subscription_period(self.start_date) + def after_insert(self) -> None: + if frappe.flags.in_import or frappe.flags.in_migrate: + return + + if getdate(self.start_date) > getdate(nowdate()): + return + + self.generate_invoices_till_date() + + def generate_invoices_till_date(self) -> None: + """ + Catch up a freshly created subscription by billing every elapsed period + from the start date up to today, then advancing the status (e.g. cancelling + if the end date has been crossed). Stops early when no further invoice is due + or an outstanding invoice blocks billing (per `generate_new_invoices_past_due_date`). + """ + while getdate(self._next_invoice_trigger_date()) <= getdate(nowdate()): + period_start = self.current_invoice_start + self.process(posting_date=self._next_invoice_trigger_date()) + + if self.status == "Cancelled" or getdate(self.current_invoice_start) == getdate(period_start): + break + + if not self.generate_new_invoices_past_due_date: + break + + def _next_invoice_trigger_date(self) -> DateTimeLikeObject: + if self.generate_invoice_at == "Beginning of the current subscription period": + return self.current_invoice_start + if self.generate_invoice_at == "Days before the current subscription period": + return add_days(self.current_invoice_start, -self.number_of_days) + return self.current_invoice_end + def update_subscription_period(self, date: DateTimeLikeObject | None = None): """ Subscription period is the period to be billed. This method updates the @@ -608,13 +641,7 @@ class Subscription(Document): return False 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) + trigger = getdate(self._next_invoice_trigger_date()) if posting < trigger: return False diff --git a/erpnext/accounts/doctype/subscription/test_subscription.py b/erpnext/accounts/doctype/subscription/test_subscription.py index 158733bd404..d86aa33bb9d 100644 --- a/erpnext/accounts/doctype/subscription/test_subscription.py +++ b/erpnext/accounts/doctype/subscription/test_subscription.py @@ -18,6 +18,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 Subscription, get_prorata_factor, process_all +from erpnext.accounts.utils import update_subscription_on_invoice_update from erpnext.tests.utils import ERPNextTestSuite @@ -61,16 +62,13 @@ class TestSubscription(ERPNextTestSuite): self.assertRaises(frappe.ValidationError, subscription.save) def test_invoice_is_generated_at_end_of_billing_period(self): + # Back-dated postpaid period has already ended, so catch-up bills it on creation + # and advances to the next period. subscription = create_subscription(start_date="2018-01-01") - self.assertEqual(subscription.status, "Active") - self.assertEqual(subscription.current_invoice_start, "2018-01-01") - self.assertEqual(subscription.current_invoice_end, "2018-01-31") - - subscription.process(posting_date="2018-01-31") self.assertEqual(len(subscription.invoices), 1) - self.assertEqual(subscription.current_invoice_start, "2018-02-01") - self.assertEqual(subscription.current_invoice_end, "2018-02-28") self.assertEqual(subscription.status, "Unpaid") + self.assertEqual(getdate(subscription.current_invoice_start), getdate("2018-02-01")) + self.assertEqual(getdate(subscription.current_invoice_end), getdate("2018-02-28")) def test_status_goes_back_to_active_after_invoice_is_paid(self): subscription = create_subscription( @@ -100,12 +98,10 @@ class TestSubscription(ERPNextTestSuite): settings.cancel_after_grace = 1 settings.save() + # Back-dated unpaid invoice is already past its (zero) grace period, so catch-up + # cancels the subscription on creation. subscription = create_subscription(start_date="2018-01-01") - self.assertEqual(subscription.status, "Active") - - subscription.process(posting_date="2018-01-31") # generate first invoice - # This should change status to Cancelled since grace period is 0 - # And is backdated subscription so subscription will be cancelled after processing + self.assertEqual(len(subscription.invoices), 1) self.assertEqual(subscription.status, "Cancelled") def test_subscription_unpaid_after_grace_period(self): @@ -257,18 +253,12 @@ class TestSubscription(ERPNextTestSuite): settings.cancel_after_grace = 1 settings.save() + # Back-dated unpaid invoice past grace -> cancelled with one invoice on creation. subscription = create_subscription(start_date="2018-01-01") - subscription.process() # generate first invoice - - # Generate an invoice for the cancelled period - subscription.cancel_subscription() - self.assertEqual(subscription.status, "Cancelled") - self.assertEqual(len(subscription.invoices), 1) - - subscription.process() self.assertEqual(subscription.status, "Cancelled") self.assertEqual(len(subscription.invoices), 1) + # Re-processing a cancelled subscription is a no-op. subscription.process() self.assertEqual(subscription.status, "Cancelled") self.assertEqual(len(subscription.invoices), 1) @@ -407,13 +397,21 @@ class TestSubscription(ERPNextTestSuite): subscription.append("plans", {"plan": "_Test Plan Name 4", "qty": 1}) subscription.save() - # even though subscription starts at "2018-01-15" and Billing interval is Month and count 3 - # First invoice will end at "2018-03-31" instead of "2018-04-14" - self.assertEqual(get_date_str(subscription.current_invoice_end), "2018-03-31") + # The first (prepaid) period is billed on creation. Even though the subscription + # starts at "2018-01-15" with a 3-month interval, follow_calendar_months ends the + # first invoice at "2018-03-31" instead of "2018-04-14". + self.assertEqual(len(subscription.invoices), 1) + self.assertEqual( + getdate(frappe.db.get_value("Purchase Invoice", subscription.invoices[0].name, "to_date")), + getdate("2018-03-31"), + ) def test_subscription_generate_invoice_past_due(self): + # With `generate_new_invoices_past_due_date` enabled, catch-up bills every elapsed + # 3-month period up to the end date on creation, even while previous ones are unpaid. subscription = create_subscription( start_date="2018-01-01", + end_date="2018-12-31", party_type="Supplier", party="_Test Supplier", generate_invoice_at="Beginning of the current subscription period", @@ -421,18 +419,9 @@ class TestSubscription(ERPNextTestSuite): plans=[{"plan": "_Test Plan Name 4", "qty": 1}], ) - # Process subscription and create first invoice - # Subscription status will be unpaid since due date has already passed - subscription.process(posting_date="2018-01-01") - self.assertEqual(len(subscription.invoices), 1) + self.assertEqual(len(subscription.invoices), 4) self.assertEqual(subscription.status, "Unpaid") - # Now the Subscription is unpaid - # Even then new invoice should be created as we have enabled `generate_new_invoices_past_due_date` in - # subscription and the interval between the subscriptions is 3 months - subscription.process(posting_date="2018-04-01") - self.assertEqual(len(subscription.invoices), 2) - def test_subscription_without_generate_invoice_past_due(self): subscription = create_subscription( start_date="2018-01-01", @@ -493,16 +482,13 @@ class TestSubscription(ERPNextTestSuite): """Test if Subscription recovers when start/end date run out of sync with created invoices.""" subscription = create_subscription( start_date="2021-01-01", + end_date="2021-02-28", submit_invoice=0, generate_new_invoices_past_due_date=1, party="_Test Subscription Customer John Doe", ) - # create invoices for the first two moths - subscription.process(posting_date="2021-01-31") - - subscription.process(posting_date="2021-02-28") - + # Catch-up bills both elapsed months on creation. self.assertEqual(len(subscription.invoices), 2) self.assertEqual( getdate(frappe.db.get_value("Sales Invoice", subscription.invoices[0].name, "from_date")), @@ -513,7 +499,7 @@ class TestSubscription(ERPNextTestSuite): getdate("2021-02-01"), ) - # recreate most recent invoice + # Re-processing much later must not duplicate the already-billed periods. subscription.process(posting_date="2022-01-31") self.assertEqual(len(subscription.invoices), 2) @@ -527,17 +513,16 @@ class TestSubscription(ERPNextTestSuite): ) def test_subscription_invoice_generation_before_days(self): + # "Days before" trigger fires 10 days ahead of each period; catch-up bills both + # elapsed periods (within the end date) on creation. subscription = create_subscription( start_date="2023-01-01", + end_date="2023-02-28", generate_invoice_at="Days before the current subscription period", number_of_days=10, generate_new_invoices_past_due_date=1, ) - subscription.process(posting_date="2022-12-22") - self.assertEqual(len(subscription.invoices), 1) - - subscription.process(posting_date="2023-01-22") self.assertEqual(len(subscription.invoices), 2) def test_future_subscription(self): @@ -596,13 +581,7 @@ class TestSubscription(ERPNextTestSuite): generate_invoice_at="Beginning of the current subscription period", plans=[{"plan": "_Test plan name 10", "qty": 1}], ) - subscription.process(posting_date=add_days(start_date, 2)) - self.assertEqual(len(subscription.invoices), 1) - - subscription.process(posting_date=add_days(start_date, 5)) - self.assertEqual(len(subscription.invoices), 2) - - subscription.process(posting_date=add_days(start_date, 8)) + # Catch-up billing on creation generates every elapsed period and cancels at end self.assertEqual(len(subscription.invoices), 3) self.assertEqual(subscription.status, "Cancelled") @@ -624,32 +603,27 @@ class TestSubscription(ERPNextTestSuite): plans=[{"plan": "_Test plan name 10", "qty": 1}], ) - subscription.process(posting_date=add_days(start_date, 2)) - self.assertEqual(len(subscription.invoices), 1) - - subscription.process(posting_date=add_days(start_date, 5)) - self.assertEqual(len(subscription.invoices), 2) - - # partial last cycle invoice - subscription.process(posting_date=add_days(start_date, 6)) + # Catch-up billing on creation incl. the partial last cycle, then cancels at end self.assertEqual(len(subscription.invoices), 3) - self.assertEqual(subscription.status, "Cancelled") self.assertRaises(frappe.ValidationError, subscription.process, posting_date=add_days(start_date, 7)) def test_invoice_generated_when_scheduler_runs_one_day_late(self): + # The trigger date (period end) is long past, yet catch-up still bills the period + # on creation (Bug 1: the check is `>= trigger`, not `== trigger`). 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") + # Build the period without saving, so on-create billing doesn't try to post an + # invoice (the deferred item has no account configured). This only exercises the + # item-mapping helper. + subscription = create_subscription(start_date="2018-01-01", do_not_save=True) + subscription.update_subscription_period("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")) @@ -730,10 +704,106 @@ class TestSubscription(ERPNextTestSuite): for invoice in invoices: pi = get_payment_entry("Sales Invoice", invoice.name) pi.submit() + # Paying the invoices refreshes the subscription via the Payment Entry hook, so + # reload before processing the stale in-memory copy. + subscription.reload() # After processing through all days, subscription should be completed subscription.process(posting_date=add_days(end_date, 1)) self.assertEqual(subscription.status, "Completed") + def test_status_updates_immediately_when_invoice_paid(self): + subscription = create_subscription( + start_date=nowdate(), + generate_invoice_at="Beginning of the current subscription period", + submit_invoice=1, + ) + subscription.process(posting_date=nowdate()) + self.assertEqual(subscription.status, "Unpaid") + + invoice = subscription.get_current_invoice() + payment = get_payment_entry("Sales Invoice", invoice.name) + payment.submit() + + subscription.reload() + self.assertEqual(subscription.status, "Active") + + def test_invoice_update_hook_refreshes_subscription_status(self): + subscription = create_subscription( + start_date=nowdate(), + generate_invoice_at="Beginning of the current subscription period", + submit_invoice=1, + ) + subscription.process(posting_date=nowdate()) + self.assertEqual(subscription.status, "Unpaid") + + invoice = subscription.get_current_invoice() + invoice.db_set("outstanding_amount", 0) + invoice.db_set("status", "Paid") + + update_subscription_on_invoice_update(invoice) + + subscription.reload() + self.assertEqual(subscription.status, "Active") + + def test_payment_entry_triggers_subscription_status_update(self): + # Test that payment entry → invoice → subscription status update chain works + subscription = create_subscription( + start_date=nowdate(), + generate_invoice_at="Beginning of the current subscription period", + submit_invoice=1, + ) + subscription.process(posting_date=nowdate()) + self.assertEqual(subscription.status, "Unpaid") + + invoice = subscription.get_current_invoice() + self.assertIsNotNone(invoice) + self.assertGreater(invoice.outstanding_amount, 0) + + # Create and submit payment entry + payment_entry = get_payment_entry(invoice.doctype, invoice.name, bank_account="_Test Bank - _TC") + payment_entry.reference_no = "12345" + payment_entry.reference_date = nowdate() + payment_entry.submit() + + # Subscription status should now be Active (via on_update_after_submit hook) + subscription.reload() + self.assertEqual(subscription.status, "Active") + + def test_first_invoice_generated_on_create_for_prepaid(self): + subscription = create_subscription( + start_date=nowdate(), + generate_invoice_at="Beginning of the current subscription period", + ) + self.assertEqual(len(subscription.invoices), 1) + + def test_first_invoice_not_generated_on_create_during_trial(self): + subscription = create_subscription( + start_date=nowdate(), + trial_period_start=nowdate(), + trial_period_end=add_days(nowdate(), 30), + generate_invoice_at="Beginning of the current subscription period", + ) + self.assertEqual(len(subscription.invoices), 0) + self.assertEqual(subscription.status, "Trialing") + + def test_first_invoice_not_generated_during_bulk_import(self): + frappe.flags.in_import = True + try: + subscription = create_subscription( + start_date=nowdate(), + generate_invoice_at="Beginning of the current subscription period", + ) + self.assertEqual(len(subscription.invoices), 0) + finally: + frappe.flags.in_import = False + + def test_first_invoice_not_generated_for_future_dated_subscription(self): + subscription = create_subscription( + start_date=add_days(nowdate(), 10), + generate_invoice_at="Beginning of the current subscription period", + ) + self.assertEqual(len(subscription.invoices), 0) + def make_plans(): create_plan(plan_name="_Test Plan Name", cost=900, currency="INR") diff --git a/erpnext/accounts/utils.py b/erpnext/accounts/utils.py index 2fd9bb30e34..e4930d46c10 100644 --- a/erpnext/accounts/utils.py +++ b/erpnext/accounts/utils.py @@ -43,6 +43,8 @@ from erpnext.stock import get_warehouse_account_map from erpnext.stock.utils import get_combine_datetime, get_stock_value_on if TYPE_CHECKING: + from frappe.model.document import Document + from erpnext.stock.doctype.repost_item_valuation.repost_item_valuation import RepostItemValuation @@ -2710,3 +2712,14 @@ def build_qb_match_conditions(doctype, user=None) -> list: def is_immutable_ledger_enabled(): return frappe.get_single_value("Accounts Settings", "enable_immutable_ledger") + + +def update_subscription_on_invoice_update(doc: "Document", method: str | None = None) -> None: + if doc.get("subscription"): + refresh_subscription_status(doc.subscription) + + +def refresh_subscription_status(name: str) -> None: + subscription = frappe.get_doc("Subscription", name) + subscription.set_subscription_status() + subscription.save(ignore_permissions=True) diff --git a/erpnext/hooks.py b/erpnext/hooks.py index a32c033c9ec..ef05f059e22 100644 --- a/erpnext/hooks.py +++ b/erpnext/hooks.py @@ -385,7 +385,7 @@ doc_events = { "validate": [ "erpnext.regional.united_arab_emirates.utils.update_grand_total_for_rcm", "erpnext.regional.united_arab_emirates.utils.validate_returns", - ] + ], }, "Payment Entry": { "on_trash": "erpnext.regional.check_deletion_permission",