fix(subscription): don't reactivate a cancelled subscription (backport #57774)

* fix(subscription): don't reactivate a cancelled subscription

Backport of #57774 to version-16-hotfix.

set_subscription_status() unconditionally set status to Active once
there was no outstanding invoice, even if the subscription had been
intentionally cancelled. Paying off an invoice issued before
cancellation (directly, or via the Payment Entry -> refresh hook)
flipped a Cancelled subscription back to Active while cancelation_date
stayed set.

process()'s cancel_at_period_end check compared posting_date against
getdate(self.end_date), and getdate(None) returns today, so an empty
end_date was silently treated as "cancel now" on every scheduler run.
Combined with the reactivation bug, this let a cancelled subscription
toggle Cancelled -> Active on each run and generate another invoice at
the next period boundary.

Fixes #57761

* test: fix flaky test_update_bom_cost_in_all_boms via valuation reset

Backport of #56796 to version-16-hotfix.

reset_item_valuation_rate() only reconciled warehouses where the item
currently has positive stock (actual_qty > 0). get_valuation_rate()
averages Sum(stock_value)/Sum(actual_qty) across all of an item's
bins, so a negative balance left over in another warehouse by a prior
test can cancel out the reset qty and collapse the average to 0,
failing the assertion with 0.0 != 10.0.

This branch never got #56796 (it predates the frappe.get_all
refactor of this helper and still uses raw SQL), so applying the same
fix here: reconcile every warehouse with a non-zero balance, not just
positive ones.

* fix(subscription): don't let period rollover defeat cancel_at_period_end

process() can advance current_invoice_end to the next period (via
update_subscription_period(), when generating the current period's
invoice) before the cancel_at_period_end check further down runs. For
a subscription with no end_date, that check now compared posting_date
against the already-rolled-forward current_invoice_end, which is
always in the future, so cancel_at_period_end was silently never
honored.

Snapshot current_invoice_end before any rollover and use that in the
check instead, so it still targets the period that just ended.

Found via review on the version-15-hotfix backport (#57780).

---------

Co-authored-by: test <test@test.com>

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()
 

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(),

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,
 		)