diff --git a/erpnext/accounts/doctype/subscription/subscription.py b/erpnext/accounts/doctype/subscription/subscription.py index 8cf237ba475..11dfac33d72 100644 --- a/erpnext/accounts/doctype/subscription/subscription.py +++ b/erpnext/accounts/doctype/subscription/subscription.py @@ -254,6 +254,9 @@ class Subscription(Document): """ Sets the status of the `Subscription` """ + if self.status == "Cancelled": + return + if self.is_trialling(): self.status = "Trialing" elif ( @@ -605,6 +608,11 @@ class Subscription(Document): 1. `process_for_active` 2. `process_for_past_due` """ + # Snapshot before update_subscription_period() below can roll this forward, + # so the cancel_at_period_end check further down still targets the period + # that just ended, not the next one. + current_period_end = self.current_invoice_end + if not self.is_current_invoice_generated( self.current_invoice_start, self.current_invoice_end ) and self.can_generate_new_invoice(posting_date): @@ -625,8 +633,8 @@ class Subscription(Document): self.update_subscription_period() if self.cancel_at_period_end and ( - getdate(posting_date) >= getdate(self.current_invoice_end) - or getdate(posting_date) >= getdate(self.end_date) + getdate(posting_date) >= getdate(current_period_end) + or (self.end_date and getdate(posting_date) >= getdate(self.end_date)) ): self.cancel_subscription() diff --git a/erpnext/accounts/doctype/subscription/test_subscription.py b/erpnext/accounts/doctype/subscription/test_subscription.py index 7df77a839be..87cc0a31c39 100644 --- a/erpnext/accounts/doctype/subscription/test_subscription.py +++ b/erpnext/accounts/doctype/subscription/test_subscription.py @@ -614,6 +614,32 @@ class TestSubscription(ERPNextTestSuite): self.assertRaises(frappe.ValidationError, subscription.process, posting_date=add_days(start_date, 7)) + def test_subscription_cancels_at_period_end_without_end_date(self): + # https://github.com/frappe/erpnext/issues/57761 -- generate_invoice() rolls + # current_invoice_end forward to the next period before this check runs, so + # with no end_date to fall back on, cancel_at_period_end must compare + # against the period that just ended, not the (already advanced) next one. + create_plan( + plan_name="_Test plan name 11", + cost=80, + currency="INR", + billing_interval="Day", + billing_interval_count=3, + ) + subscription = create_subscription( + start_date=nowdate(), + cancel_at_period_end=1, + generate_invoice_at="End of the current subscription period", + plans=[{"plan": "_Test plan name 11", "qty": 1}], + ) + self.assertEqual(len(subscription.invoices), 0) + period_end = subscription.current_invoice_end + + subscription.process(posting_date=period_end) + + self.assertEqual(subscription.status, "Cancelled") + self.assertEqual(len(subscription.invoices), 1) + 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`). @@ -774,6 +800,38 @@ class TestSubscription(ERPNextTestSuite): subscription.reload() self.assertEqual(subscription.status, "Active") + def test_cancelled_subscription_stays_cancelled_after_payment_and_reprocess(self): + # https://github.com/frappe/erpnext/issues/57761 + subscription = create_subscription( + start_date=nowdate(), + generate_invoice_at="Beginning of the current subscription period", + submit_invoice=1, + cancel_at_period_end=1, + ) + subscription.process(posting_date=nowdate()) + invoice = subscription.get_current_invoice() + self.assertGreater(invoice.outstanding_amount, 0) + + subscription.cancel_subscription() + self.assertEqual(subscription.status, "Cancelled") + cancelation_date = getdate(subscription.cancelation_date) + self.assertIsNotNone(cancelation_date) + + 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.reload() + self.assertEqual(subscription.status, "Cancelled") + self.assertEqual(getdate(subscription.cancelation_date), cancelation_date) + + invoice_count = len(subscription.invoices) + subscription.process() + subscription.reload() + self.assertEqual(subscription.status, "Cancelled") + self.assertEqual(len(subscription.invoices), invoice_count) + def test_first_invoice_generated_on_create_for_prepaid(self): subscription = create_subscription( start_date=nowdate(), diff --git a/erpnext/manufacturing/doctype/bom/test_bom.py b/erpnext/manufacturing/doctype/bom/test_bom.py index 77e0ecddb1d..f40f6bc499e 100644 --- a/erpnext/manufacturing/doctype/bom/test_bom.py +++ b/erpnext/manufacturing/doctype/bom/test_bom.py @@ -881,10 +881,15 @@ def reset_item_valuation_rate(item_code, warehouse_list=None, qty=None, rate=Non warehouse_list = [warehouse_list] if not warehouse_list: + # Reconcile every warehouse the item has a non-zero balance in -- including + # negative balances left by other tests. get_valuation_rate averages + # Sum(stock_value)/Sum(actual_qty) across all bins, so a leftover negative + # balance in one warehouse can cancel the reset qty elsewhere and make the + # average collapse to 0, which is a source of flaky BOM-cost failures. warehouse_list = frappe.db.sql_list( """ select warehouse from `tabBin` - where item_code=%s and actual_qty > 0 + where item_code=%s and actual_qty != 0 """, item_code, )