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

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

Backport of #57774 to version-15-hotfix.

set_subscription_status() unconditionally set status to Active once
there was no outstanding invoice, with no check for whether the
subscription had been intentionally cancelled. 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 reprocess.

version-15-hotfix does not yet have the Payment Entry -> invoice ->
refresh_subscription_status() hook chain that #57761 reports (that
was added later), and the scheduler already excludes Cancelled
subscriptions, so the exact automatic repro in the issue does not
apply here. The underlying status logic is defective the same way
though, and reachable directly (e.g. any caller invoking the
whitelisted process()/set_subscription_status() on a cancelled
subscription), so fixing it here too rather than leaving the same
class of bug in place.

Related to #57761

* fix(test): avoid tripping the unrelated period-end cancel clause

The test backdated start_date to 2018-01-01, so by the time process()
ran, current_invoice_end was long past. That made the *other* half of
the cancel_at_period_end OR-condition (period end reached) true on
its own, so process() tried to cancel an already-cancelled
subscription and hit cancel_subscription()'s "already cancelled"
guard - unrelated to the empty end_date bug being tested.

Use nowdate() as start_date so current_invoice_end sits safely in the
future, isolating the assertion to the empty end_date guard. Also
drop the fragile intermediate "Unpaid" status assertion, which
wasn't part of what this test verifies.

* 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.

Fixes the P1 flagged by Greptile review on this PR.

---------

Co-authored-by: test <test@test.com>
This commit is contained in:
Jatin3128
2026-08-05 12:26:01 +05:30
committed by GitHub
parent e3af2f9302
commit edc8daae21
2 changed files with 63 additions and 2 deletions

View File

@@ -222,6 +222,9 @@ class Subscription(Document):
"""
Sets the status of the `Subscription`
"""
if self.status == "Cancelled":
return
if self.is_trialling():
self.status = "Trialling"
elif self.status == "Active" and self.end_date and getdate(posting_date) > getdate(self.end_date):
@@ -558,6 +561,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):
@@ -567,8 +575,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()

View File

@@ -280,6 +280,59 @@ class TestSubscription(FrappeTestCase):
settings.cancel_after_grace = default_grace_period_action
settings.save()
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"
)
subscription.process(posting_date=nowdate()) # generate first invoice
invoice = subscription.get_current_invoice()
self.assertIsNotNone(invoice)
invoice.db_set("outstanding_amount", 0)
invoice.db_set("status", "Paid")
subscription.cancel_subscription()
self.assertEqual(subscription.status, "Cancelled")
cancelation_date = getdate(subscription.cancelation_date)
subscription.set_subscription_status()
self.assertEqual(subscription.status, "Cancelled")
self.assertEqual(getdate(subscription.cancelation_date), cancelation_date)
subscription.cancel_at_period_end = 1
subscription.end_date = None
invoice_count = len(subscription.invoices)
subscription.process()
self.assertEqual(subscription.status, "Cancelled")
self.assertEqual(len(subscription.invoices), invoice_count)
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(),
generate_invoice_at="End of the current subscription period",
plans=[{"plan": "_Test plan name 11", "qty": 1}],
)
subscription.cancel_at_period_end = 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_subscription_restart_and_process(self):
settings = frappe.get_single("Subscription Settings")
default_grace_period_action = settings.cancel_after_grace