Add plan deletion, with a confirmation screen listing affected clubs
Due.plan is PROTECT -- a plan that has ever billed anyone can never truly be removed, on purpose: amount/period_end/grace_until are frozen on a Due precisely so a later change can't rewrite what was actually charged, and losing the plan link off an old Due would do exactly that to every historical invoice. "Delete" therefore means one of two things, chosen automatically (billing/services/plans.py): - never billed anyone -> the row is removed outright. - has billing history -> soft-deleted (Plan.deleted_at, is_active off): hidden from every picker/listing via the new opt-in Plan.objects.visible(), but the row survives so old invoices still show what they were billed under. Either way, every club currently on the plan is unsubscribed outright -- its Subscription row deleted, not just its plan field cleared. "No plan" was already a fully-understood state everywhere else in the app, so this reuses it instead of inventing a new one. Also handles the easy-to-miss second group: a club on a DIFFERENT plan, mid-trial, configured to convert to the plan being deleted (Subscription.post_trial_plan). Left alone that would try to convert onto a hidden/gone plan later; instead that club's trial is ended now (both trial fields cleared, per the CheckConstraint requiring them together) so it needs a new plan picked by hand. The confirmation screen is a real page, not a modal like every other billing action -- naming exactly which clubs are affected, in both groups, and that list can be long.
This commit is contained in:
18
billing/migrations/0007_plan_deleted_at.py
Normal file
18
billing/migrations/0007_plan_deleted_at.py
Normal file
@@ -0,0 +1,18 @@
|
||||
# Generated by Django 6.0.6 on 2026-08-08 17:54
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('billing', '0006_due_last_reminder_level_due_last_reminder_sent_at'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='plan',
|
||||
name='deleted_at',
|
||||
field=models.DateTimeField(blank=True, editable=False, null=True, verbose_name='deleted at'),
|
||||
),
|
||||
]
|
||||
@@ -37,6 +37,17 @@ def add_months(day: date, months: int) -> date:
|
||||
return day + relativedelta.relativedelta(months=months)
|
||||
|
||||
|
||||
class PlanQuerySet(models.QuerySet):
|
||||
def visible(self):
|
||||
"""Excludes soft-deleted plans -- see billing.services.plans.delete_plan.
|
||||
|
||||
Opt-in, same shape as club.models.ClubManager.active(): the default manager stays
|
||||
unfiltered (Django admin, and anything reading historical data, sees everything),
|
||||
and every picker/listing a platform admin actually chooses from calls this.
|
||||
"""
|
||||
return self.filter(deleted_at__isnull=True)
|
||||
|
||||
|
||||
class Plan(UUIDModel):
|
||||
"""What a club is billed on: a duration, a set of clocks, and a dated price.
|
||||
|
||||
@@ -64,6 +75,14 @@ class Plan(UUIDModel):
|
||||
help_text=_("Offered as a trial rather than as a paid plan. A trial converts to the plan chosen on the subscription once it runs out."),
|
||||
)
|
||||
|
||||
# Not user-editable: set by billing.services.plans.delete_plan. Due.plan is PROTECT, so
|
||||
# a plan that has ever billed anyone can never actually be removed -- deleting it hides
|
||||
# it (and clears every club currently on it) instead, so past invoices still say what
|
||||
# they were billed under. See that module's docstring for the full reasoning.
|
||||
deleted_at = models.DateTimeField(_("deleted at"), null=True, blank=True, editable=False)
|
||||
|
||||
objects = PlanQuerySet.as_manager()
|
||||
|
||||
class Meta:
|
||||
verbose_name = _("plan")
|
||||
verbose_name_plural = _("plans")
|
||||
@@ -86,6 +105,10 @@ class Plan(UUIDModel):
|
||||
def __str__(self):
|
||||
return self.name
|
||||
|
||||
@property
|
||||
def is_deleted(self) -> bool:
|
||||
return self.deleted_at is not None
|
||||
|
||||
def clean(self):
|
||||
"""The same two invariants the CheckConstraints enforce, as form errors.
|
||||
|
||||
|
||||
85
billing/services/plans.py
Normal file
85
billing/services/plans.py
Normal file
@@ -0,0 +1,85 @@
|
||||
"""Deleting a plan.
|
||||
|
||||
Due.plan is PROTECT -- a plan that has ever billed anyone can never truly be removed, and
|
||||
must not be: `amount`, `period_end` and `grace_until` are frozen on a Due precisely so a
|
||||
later change can't rewrite what was actually charged, and losing the plan link off an old
|
||||
Due would do exactly that to every historical invoice. So "delete" means one of two things,
|
||||
chosen automatically depending on whether the plan has billing history:
|
||||
|
||||
* No Due ever referenced it (created, then never actually used to bill anyone) -- the row
|
||||
itself is removed.
|
||||
* At least one Due references it -- soft-deleted instead (Plan.deleted_at set, is_active
|
||||
turned off): hidden from every picker and listing (Plan.objects.visible()), but the row
|
||||
survives so every old invoice still says what it was for.
|
||||
|
||||
Either way, every club CURRENTLY on the plan is unsubscribed outright -- its Subscription
|
||||
row deleted, not just its `plan` field cleared. "No plan" is already a state the rest of the
|
||||
app fully understands (every billing view already handles `getattr(club, "subscription",
|
||||
None)` being None), so there is no new state to teach it.
|
||||
|
||||
A second, easy-to-miss group: a club on a DIFFERENT plan, mid-trial, configured to convert
|
||||
to THIS plan once its trial ends (Subscription.post_trial_plan). Deleting the target plan
|
||||
out from under that trial can't be allowed to raise a stale IntegrityError days or weeks
|
||||
later when the trial tries to convert -- so it's handled now, at delete time: that club's
|
||||
trial is ended (trial_ends_at and post_trial_plan both cleared, per the CheckConstraint that
|
||||
requires them set together or not at all), leaving it on the trial plan with no scheduled
|
||||
conversion until a platform admin picks a new one.
|
||||
"""
|
||||
|
||||
from dataclasses import dataclass
|
||||
|
||||
from django.db import transaction
|
||||
from django.utils import timezone
|
||||
|
||||
from billing.models import Plan, Subscription
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class PlanDeletionImpact:
|
||||
plan: Plan
|
||||
#: Clubs currently ON this plan -- lose it entirely (Subscription row deleted).
|
||||
unsubscribed_clubs: list
|
||||
#: Clubs on a different plan, mid-trial, configured to convert to this one -- their
|
||||
#: trial's landing plan is cleared, leaving no scheduled conversion.
|
||||
broken_trial_clubs: list
|
||||
#: Whether the Plan row itself will be removed (True) or only hidden (False, because
|
||||
#: it has billing history).
|
||||
will_hard_delete: bool
|
||||
|
||||
@property
|
||||
def has_impact(self) -> bool:
|
||||
return bool(self.unsubscribed_clubs or self.broken_trial_clubs)
|
||||
|
||||
|
||||
def plan_deletion_impact(plan: Plan) -> PlanDeletionImpact:
|
||||
"""What deleting `plan` right now would do -- read-only, for the confirmation screen.
|
||||
|
||||
delete_plan() re-derives the same two lists itself rather than trust one computed here
|
||||
moments earlier and possibly stale by the time the platform admin actually confirms.
|
||||
"""
|
||||
unsubscribed = Subscription.objects.filter(plan=plan).select_related("club").order_by("club__name")
|
||||
broken_trial = Subscription.objects.filter(post_trial_plan=plan).exclude(plan=plan).select_related("club").order_by("club__name")
|
||||
|
||||
return PlanDeletionImpact(
|
||||
plan=plan,
|
||||
unsubscribed_clubs=[subscription.club for subscription in unsubscribed],
|
||||
broken_trial_clubs=[subscription.club for subscription in broken_trial],
|
||||
will_hard_delete=not plan.dues.exists(),
|
||||
)
|
||||
|
||||
|
||||
@transaction.atomic
|
||||
def delete_plan(plan: Plan) -> PlanDeletionImpact:
|
||||
impact = plan_deletion_impact(plan)
|
||||
|
||||
Subscription.objects.filter(plan=plan).delete()
|
||||
Subscription.objects.filter(post_trial_plan=plan).update(trial_ends_at=None, post_trial_plan=None)
|
||||
|
||||
if impact.will_hard_delete:
|
||||
plan.delete()
|
||||
else:
|
||||
plan.deleted_at = timezone.now()
|
||||
plan.is_active = False
|
||||
plan.save(update_fields=["deleted_at", "is_active", "modified"])
|
||||
|
||||
return impact
|
||||
122
billing/tests.py
122
billing/tests.py
@@ -21,6 +21,7 @@ from .services import BillingError
|
||||
from .services.dues import archivable_clubs, dues_in_grace, dues_overdue, next_period_start, open_period, reactivate, record_payment, remove_payment, renew, start_trial, subscribe, subscriptions_due_for_renewal, waive
|
||||
from .services.invoices import invoice_pdf, issue_invoice, render_pdf
|
||||
from .services.notices import club_billing_notice
|
||||
from .services.plans import delete_plan, plan_deletion_impact
|
||||
from .services.reminders import admin_emails, reminders_to_send, send_reminder
|
||||
|
||||
|
||||
@@ -835,3 +836,124 @@ class BillingReminderTests(BillingTestBase):
|
||||
record_payment(self.due, Decimal("500.00"))
|
||||
|
||||
self.assertEqual(reminders_to_send([self.club], self.today), [])
|
||||
|
||||
|
||||
class PlanVisibilityTests(BillingTestBase):
|
||||
"""Plan.objects.visible() -- see PlanQuerySet."""
|
||||
|
||||
def test_a_plain_plan_is_visible(self):
|
||||
self.assertIn(self.plan, Plan.objects.visible())
|
||||
|
||||
def test_a_soft_deleted_plan_is_excluded(self):
|
||||
self.plan.deleted_at = timezone.now()
|
||||
self.plan.save(update_fields=["deleted_at"])
|
||||
|
||||
self.assertNotIn(self.plan, Plan.objects.visible())
|
||||
|
||||
def test_the_default_manager_still_returns_a_soft_deleted_plan(self):
|
||||
# Django admin, and anything reading historical data, must still be able to find it.
|
||||
self.plan.deleted_at = timezone.now()
|
||||
self.plan.save(update_fields=["deleted_at"])
|
||||
|
||||
self.assertIn(self.plan, Plan.objects.all())
|
||||
|
||||
|
||||
class PlanDeletionTests(BillingTestBase):
|
||||
"""billing.services.plans -- see its module docstring for the full reasoning."""
|
||||
|
||||
def test_a_never_billed_plan_is_hard_deleted(self):
|
||||
unused = Plan.objects.create(name="Unused")
|
||||
|
||||
impact = delete_plan(unused)
|
||||
|
||||
self.assertTrue(impact.will_hard_delete)
|
||||
self.assertFalse(Plan.objects.filter(pk=unused.pk).exists())
|
||||
|
||||
def test_a_plan_with_a_due_cannot_be_hard_deleted(self):
|
||||
self.bill()
|
||||
|
||||
impact = delete_plan(self.plan)
|
||||
|
||||
self.assertFalse(impact.will_hard_delete)
|
||||
self.assertTrue(Plan.objects.filter(pk=self.plan.pk).exists())
|
||||
|
||||
def test_a_cancelled_due_still_protects_the_plan(self):
|
||||
# PROTECT does not care about the referencing row's own status -- a cancelled due is
|
||||
# still a row, and financial history includes rows nobody expects to see again.
|
||||
due = self.bill()
|
||||
due.status = Due.Status.CANCELLED
|
||||
due.save(update_fields=["status"])
|
||||
|
||||
impact = delete_plan(self.plan)
|
||||
|
||||
self.assertFalse(impact.will_hard_delete)
|
||||
|
||||
def test_soft_delete_marks_the_plan_inactive_and_deleted(self):
|
||||
subscribe(self.club, self.plan)
|
||||
|
||||
delete_plan(self.plan)
|
||||
self.plan.refresh_from_db()
|
||||
|
||||
self.assertTrue(self.plan.is_deleted)
|
||||
self.assertFalse(self.plan.is_active)
|
||||
|
||||
def test_deleting_unsubscribes_the_club_entirely_rather_than_nulling_a_field(self):
|
||||
subscribe(self.club, self.plan)
|
||||
|
||||
delete_plan(self.plan)
|
||||
|
||||
self.assertFalse(Subscription.objects.filter(club=self.club).exists())
|
||||
|
||||
def test_a_deleted_plans_historical_due_is_untouched(self):
|
||||
subscribe(self.club, self.plan)
|
||||
due = self.club.dues.first()
|
||||
amount, period_end, grace_until = due.amount, due.period_end, due.grace_until
|
||||
|
||||
delete_plan(self.plan)
|
||||
due.refresh_from_db()
|
||||
|
||||
self.assertEqual(due.plan_id, self.plan.pk)
|
||||
self.assertEqual(due.amount, amount)
|
||||
self.assertEqual(due.period_end, period_end)
|
||||
self.assertEqual(due.grace_until, grace_until)
|
||||
|
||||
def test_a_club_not_on_the_plan_is_unaffected(self):
|
||||
other_plan = Plan.objects.create(name="Other")
|
||||
PlanPrice.objects.create(plan=other_plan, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("100.00"))
|
||||
untouched = Club.objects.create(name="Untouched FC")
|
||||
subscribe(untouched, other_plan)
|
||||
|
||||
delete_plan(self.plan)
|
||||
|
||||
self.assertTrue(Subscription.objects.filter(club=untouched, plan=other_plan).exists())
|
||||
|
||||
def test_a_trial_scheduled_to_convert_to_the_deleted_plan_is_cleared(self):
|
||||
trial_plan = Plan.objects.create(name="Trial", is_trial=True, duration_months=2, renewal_lead_days=7, grace_days=14)
|
||||
PlanPrice.objects.create(plan=trial_plan, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("0.00"))
|
||||
club = Club.objects.create(name="Mid Trial FC")
|
||||
start_trial(club, trial_plan, post_trial_plan=self.plan)
|
||||
|
||||
impact = delete_plan(self.plan)
|
||||
|
||||
self.assertEqual([c.pk for c in impact.broken_trial_clubs], [club.pk])
|
||||
club.refresh_from_db()
|
||||
subscription = club.subscription
|
||||
self.assertEqual(subscription.plan, trial_plan)
|
||||
self.assertIsNone(subscription.trial_ends_at)
|
||||
self.assertIsNone(subscription.post_trial_plan)
|
||||
|
||||
def test_a_club_currently_on_the_plan_is_not_also_counted_as_a_broken_trial(self):
|
||||
subscribe(self.club, self.plan)
|
||||
|
||||
impact = delete_plan(self.plan)
|
||||
|
||||
self.assertEqual(impact.unsubscribed_clubs, [self.club])
|
||||
self.assertEqual(impact.broken_trial_clubs, [])
|
||||
|
||||
def test_plan_deletion_impact_is_read_only(self):
|
||||
subscribe(self.club, self.plan)
|
||||
|
||||
plan_deletion_impact(self.plan)
|
||||
|
||||
self.assertTrue(Subscription.objects.filter(club=self.club).exists())
|
||||
self.assertFalse(self.plan.is_deleted)
|
||||
|
||||
Reference in New Issue
Block a user