diff --git a/billing/admin.py b/billing/admin.py index a72326f..82be11f 100644 --- a/billing/admin.py +++ b/billing/admin.py @@ -25,8 +25,8 @@ class TierPriceAdmin(admin.ModelAdmin): @admin.register(Subscription) class SubscriptionAdmin(admin.ModelAdmin): - list_display = ["club", "tier", "auto_archive"] - list_filter = ["tier", "auto_archive"] + list_display = ["club", "tier", "auto_renew", "auto_archive"] + list_filter = ["tier", "auto_renew", "auto_archive"] search_fields = ["club__name"] diff --git a/billing/management/commands/renew_subscriptions.py b/billing/management/commands/renew_subscriptions.py new file mode 100644 index 0000000..d642f57 --- /dev/null +++ b/billing/management/commands/renew_subscriptions.py @@ -0,0 +1,57 @@ +"""Issue the next billing period for clubs whose current one is running out. + +Unlike archive_overdue_clubs, this ACTS by default and only previews with --dry-run. The +asymmetry is deliberate and runs the other way: archiving switches off a paying customer, so +not acting is the safe failure. Here, not acting means a club keeps using the platform for +free — and because nothing is owed, no dashboard number goes red and the archive job never +fires either. A missed renewal is silent, and silence is the expensive failure. +""" + +from django.core.management.base import CommandError + +from billing.models import RENEWAL_LEAD_DAYS +from billing.services import BillingError +from billing.services.dues import renew, subscriptions_due_for_renewal +from features.commands import MaintenanceAwareCommand + + +class Command(MaintenanceAwareCommand): + help = "Open the next billing period for clubs whose current period ends soon." + + def add_arguments(self, parser): + parser.add_argument("--dry-run", action="store_true", help="Report what would be issued, and issue nothing.") + parser.add_argument("--lead-days", type=int, default=RENEWAL_LEAD_DAYS, help=f"Issue this many days before the period ends (default {RENEWAL_LEAD_DAYS}).") + + def handle(self, *args, **options): + due_for_renewal = subscriptions_due_for_renewal(lead_days=options["lead_days"]) + + if not due_for_renewal: + self.stdout.write(self.style.SUCCESS("Nothing to renew.")) + return + + failures = [] + for subscription in due_for_renewal: + club = subscription.club + + if options["dry_run"]: + self.stdout.write(f"would renew {club} — {subscription.tier}, current period ends {subscription.latest_period_end or 'never opened'}") + continue + + try: + due = renew(subscription) + except BillingError as error: + # One unpriced tier must not stop every other club from being billed. + failures.append(f"{club}: {error}") + self.stdout.write(self.style.ERROR(f"{club} — {error}")) + continue + + self.stdout.write(self.style.SUCCESS(f"{club} — {due.period_start} to {due.period_end}, {due.amount} ({due.invoice.number})")) + + if options["dry_run"]: + self.stdout.write(self.style.WARNING(f"\nDry run: {len(due_for_renewal)} club(s) would be renewed.")) + return + + if failures: + # Non-zero, so cron mails you: a club that could not be billed is revenue quietly + # not being collected. + raise CommandError(f"{len(failures)} club(s) could not be renewed:\n " + "\n ".join(failures)) diff --git a/billing/migrations/0002_subscription_auto_renew.py b/billing/migrations/0002_subscription_auto_renew.py new file mode 100644 index 0000000..4746ac6 --- /dev/null +++ b/billing/migrations/0002_subscription_auto_renew.py @@ -0,0 +1,18 @@ +# Generated by Django 6.0.6 on 2026-07-14 22:35 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('billing', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='subscription', + name='auto_renew', + field=models.BooleanField(default=True, help_text='Issue the next period automatically before this one ends. Off means you invoice this club by hand.', verbose_name='auto renew'), + ), + ] diff --git a/billing/models.py b/billing/models.py index eacb9b7..56fc53f 100644 --- a/billing/models.py +++ b/billing/models.py @@ -22,6 +22,11 @@ ZERO = Decimal("0.00") #: A club stays live for six weeks past the end of an unpaid period before it is archived. GRACE_DAYS = 45 +#: The next period is issued this long before the current one ends, so the invoice reaches the +#: club — and can be paid — before the old period lapses. Grace then only matters for genuine +#: non-payers, rather than for everyone who takes a fortnight to pay a bank transfer. +RENEWAL_LEAD_DAYS = 30 + def add_one_year(day: date) -> date: """The day one year on. 29 February has no counterpart in a common year, so it falls @@ -93,6 +98,7 @@ class Subscription(UUIDModel): club = models.OneToOneField("club.Club", on_delete=models.CASCADE, related_name="subscription", verbose_name=_("club")) tier = models.ForeignKey(Tier, on_delete=models.PROTECT, related_name="subscriptions", verbose_name=_("tier")) + auto_renew = models.BooleanField(_("auto renew"), default=True, help_text=_("Issue the next period automatically before this one ends. Off means you invoice this club by hand.")) auto_archive = models.BooleanField(_("auto archive"), default=True, help_text=_("Archive this club when a period goes unpaid past its grace period.")) notes = models.TextField(_("notes"), blank=True) diff --git a/billing/services/dues.py b/billing/services/dues.py index 13cb9e7..c8c151a 100644 --- a/billing/services/dues.py +++ b/billing/services/dues.py @@ -6,17 +6,17 @@ from datetime import date, timedelta from decimal import Decimal from django.db import transaction -from django.db.models import Sum +from django.db.models import DateField, OuterRef, Subquery, Sum from django.utils import timezone -from billing.models import ZERO, Due, DuePayment, Subscription, Tier +from billing.models import RENEWAL_LEAD_DAYS, ZERO, Due, DuePayment, Subscription, Tier from billing.services import BillingError from billing.services.invoices import issue_invoice -def subscribe(club, tier: Tier, *, start: date | None = None, auto_archive: bool = True) -> Subscription: +def subscribe(club, tier: Tier, *, start: date | None = None, auto_archive: bool = True, auto_renew: bool = True) -> Subscription: """Put a club on a tier and open its first period.""" - subscription, _created = Subscription.objects.update_or_create(club=club, defaults={"tier": tier, "auto_archive": auto_archive}) + subscription, _created = Subscription.objects.update_or_create(club=club, defaults={"tier": tier, "auto_archive": auto_archive, "auto_renew": auto_renew}) open_period(club, start=start) return subscription @@ -151,3 +151,36 @@ def reactivate(club, *, start: date | None = None) -> Due: club.restore() return open_period(club, start=start) + + +def subscriptions_due_for_renewal(today: date | None = None, lead_days: int = RENEWAL_LEAD_DAYS): + """Clubs whose next period should be issued now. + + Idempotent by construction: a club that has just been renewed has a latest period ending a + year out, which is past the horizon, so it cannot be picked up twice. Running the job twice + a day is harmless. + + A subscription with no period at all (its only due was cancelled) counts too — a club on a + plan and billed for nothing is the leak this whole job exists to close. + """ + today = today or timezone.localdate() + horizon = today + timedelta(days=lead_days) + + latest_period_end = Subquery( + Due.objects.filter(club=OuterRef("club")).exclude(status=Due.Status.CANCELLED).order_by("-period_end").values("period_end")[:1], + output_field=DateField(), + ) + + subscriptions = ( + Subscription.objects.filter(auto_renew=True, club__archived_at__isnull=True) + .select_related("club", "tier") + .annotate(latest_period_end=latest_period_end) + .order_by("club__name") + ) + + return [subscription for subscription in subscriptions if subscription.latest_period_end is None or subscription.latest_period_end <= horizon] + + +def renew(subscription: Subscription) -> Due: + """Open the club's next period, continuing from the last one.""" + return open_period(subscription.club) diff --git a/billing/tests.py b/billing/tests.py index 5808b26..ea45beb 100644 --- a/billing/tests.py +++ b/billing/tests.py @@ -5,6 +5,7 @@ from io import StringIO from unittest import mock from django.core.management import call_command +from django.core.management.base import CommandError from django.test import TestCase from django.utils import timezone @@ -12,7 +13,7 @@ from club.models import Club from .models import GRACE_DAYS, Due, Invoice, Subscription, Tier, TierPrice, add_one_year 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, subscribe, waive +from .services.dues import archivable_clubs, dues_in_grace, dues_overdue, next_period_start, open_period, reactivate, record_payment, remove_payment, renew, subscribe, subscriptions_due_for_renewal, waive from .services.invoices import invoice_pdf, issue_invoice, render_pdf @@ -335,3 +336,124 @@ class ModelStringTests(BillingTestBase): self.assertIn("10.00", str(payment)) self.assertIn("INV-", str(due.invoice)) self.assertIn("Standard", str(subscribe(Club.objects.create(name="PSV"), self.tier))) + + +class RenewalTests(BillingTestBase): + """The leak this closes: a club whose period lapses with its last due PAID owes nothing, + so dues_overdue() is empty, so archive_overdue_clubs never fires — and the club keeps + using the platform for free while every number on the dashboard stays green.""" + + def ending_in(self, days, **kwargs): + """A club whose current period ends `days` from now.""" + club = Club.objects.create(name=f"Club {days}") + subscribe(club, self.tier, start=self.today - datetime.timedelta(days=365 - days), **kwargs) + return club + + def test_a_club_nearing_its_end_date_is_picked_up(self): + club = self.ending_in(20) + + due = [s.club for s in subscriptions_due_for_renewal()] + + self.assertIn(club, due) + + def test_a_club_with_a_period_beyond_the_horizon_is_left_alone(self): + club = self.ending_in(200) + + self.assertNotIn(club, [s.club for s in subscriptions_due_for_renewal()]) + + def test_renewing_continues_from_the_last_period(self): + club = self.ending_in(20) + first = club.dues.first() + + renew(club.subscription) + + latest = club.dues.order_by("-period_start").first() + self.assertEqual(latest.period_start, first.period_end + datetime.timedelta(days=1)) + self.assertEqual(club.dues.count(), 2) + + def test_running_twice_does_not_bill_twice(self): + # Idempotent by construction: once renewed, the club's latest period ends a year out, + # which is past the horizon. + club = self.ending_in(20) + + call_command("renew_subscriptions", stdout=StringIO()) + call_command("renew_subscriptions", stdout=StringIO()) + + self.assertEqual(club.dues.count(), 2) + + def test_a_club_that_opted_out_is_not_renewed(self): + club = self.ending_in(20, auto_renew=False) + + self.assertNotIn(club, [s.club for s in subscriptions_due_for_renewal()]) + + def test_an_archived_club_is_not_renewed(self): + # Reactivation is the way back, and it opens a period of its own. + club = self.ending_in(20) + club.archive() + + self.assertNotIn(club, [s.club for s in subscriptions_due_for_renewal()]) + + def test_the_new_period_is_billed_at_the_price_in_force_then(self): + club = self.ending_in(20) + TierPrice.objects.create(tier=self.tier, active_from=self.today, amount=Decimal("900.00")) + + due = renew(club.subscription) + + self.assertEqual(due.amount, Decimal("900.00")) # the new rate + self.assertEqual(club.dues.order_by("period_start").first().amount, Decimal("500.00")) # the old one, untouched + + def test_the_new_period_is_invoiced(self): + club = self.ending_in(20) + + due = renew(club.subscription) + + self.assertTrue(due.invoice.number.startswith("INV-")) + + def test_a_dry_run_issues_nothing(self): + club = self.ending_in(20) + out = StringIO() + + call_command("renew_subscriptions", "--dry-run", stdout=out) + + self.assertEqual(club.dues.count(), 1) + self.assertIn("would renew", out.getvalue()) + + def test_the_command_issues_by_default(self): + # The opposite asymmetry to archiving: NOT acting is the expensive failure here, + # because a club that is never billed is never chased either. + club = self.ending_in(20) + + call_command("renew_subscriptions", stdout=StringIO()) + + self.assertEqual(club.dues.count(), 2) + + def test_an_unpriced_tier_fails_loudly_without_stopping_the_others(self): + priced = self.ending_in(20) + broken = Club.objects.create(name="Unpriced FC") + subscribe(broken, self.tier, start=self.today - datetime.timedelta(days=350)) + # Its next period starts beyond the last price... by removing every price, it cannot bill. + TierPrice.objects.all().delete() + cheap = Tier.objects.create(name="Cheap") + TierPrice.objects.create(tier=cheap, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("100.00")) + priced.subscription.tier = cheap + priced.subscription.save() + + with self.assertRaises(CommandError): + call_command("renew_subscriptions", stdout=StringIO(), stderr=StringIO()) + + # ...and the club that COULD be billed still was. + self.assertEqual(priced.dues.count(), 2) + + def test_a_subscription_with_no_period_at_all_is_renewed(self): + club = Club.objects.create(name="Orphan FC") + Subscription.objects.create(club=club, tier=self.tier) + + self.assertIn(club, [s.club for s in subscriptions_due_for_renewal()]) + + def test_it_says_so_when_there_is_nothing_to_renew(self): + self.assertIn("Nothing to renew", self.run_renewal()) + + def run_renewal(self, *args): + out = StringIO() + call_command("renew_subscriptions", *args, stdout=out) + return out.getvalue() diff --git a/controlpanel/forms.py b/controlpanel/forms.py index e8054d2..1c5c4b8 100644 --- a/controlpanel/forms.py +++ b/controlpanel/forms.py @@ -82,7 +82,7 @@ class SubscriptionForm(forms.ModelForm): class Meta: model = Subscription - fields = ["tier", "auto_archive", "notes"] + fields = ["tier", "auto_renew", "auto_archive", "notes"] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/controlpanel/services/statistics.py b/controlpanel/services/statistics.py index ea0fe45..aef48ca 100644 --- a/controlpanel/services/statistics.py +++ b/controlpanel/services/statistics.py @@ -18,7 +18,7 @@ from waffle import get_waffle_flag_model from authentication.middleware import ELEVATED_ROLES from billing.models import Due, DuePayment, Subscription -from billing.services.dues import dues_in_grace, dues_overdue +from billing.services.dues import dues_in_grace, dues_overdue, subscriptions_due_for_renewal from club.models import Club, ClubMembership, ClubRole, Season from events.models import Attendance, Event from members.models import Member @@ -141,7 +141,7 @@ def onboarding_funnel(): return [ {"label": "Clubs", "count": total, "icon": "building-2"}, {"label": "With members", "count": sum(1 for club in clubs if club.member_count), "icon": "users"}, - {"label": "With a team", "count": sum(1 for club in clubs if club.team_count), "icon": "shield"}, + {"label": "With a team", "count": sum(1 for club in clubs if club.team_count), "icon": "trophy"}, {"label": "With events", "count": sum(1 for club in clubs if club.event_count), "icon": "calendar-days"}, ] @@ -172,6 +172,10 @@ def platform_attention(): "dues_in_grace": dues_in_grace().count(), "dues_overdue": dues_overdue().count(), "clubs_unbilled": Club.objects.active().filter(subscription__isnull=True).count(), + # Normally ~0: the renewal job keeps it there. A number that sits here means cron is + # dead, and a club is about to use the platform for free — silently, because nothing is + # owed, so no other number on this page would go red. + "renewals_pending": len(subscriptions_due_for_renewal()), } diff --git a/controlpanel/templates/controlpanel/club_detail.html b/controlpanel/templates/controlpanel/club_detail.html index 9d2c674..7b93823 100644 --- a/controlpanel/templates/controlpanel/club_detail.html +++ b/controlpanel/templates/controlpanel/club_detail.html @@ -234,6 +234,11 @@ {% else %}

On {{ subscription.tier.name }}. + {% if subscription.auto_renew %} + Renews automatically 30 days before the period ends. + {% else %} + Auto-renew off — you must open each period by hand, or this club uses the platform for free. + {% endif %} {% if subscription.auto_archive %} Archived automatically when a period goes unpaid past its grace period. {% else %} diff --git a/controlpanel/views.py b/controlpanel/views.py index add35dd..ebc37b1 100644 --- a/controlpanel/views.py +++ b/controlpanel/views.py @@ -414,7 +414,7 @@ class SubscribeClubView(PlatformStaffRequiredMixin, FormView): subscription.save() messages.success(self.request, f"{club} is now on {subscription.tier}. The current period keeps the amount it was billed at.") else: - subscribe(club, form.cleaned_data["tier"], start=form.cleaned_data.get("start"), auto_archive=form.cleaned_data["auto_archive"]) + subscribe(club, form.cleaned_data["tier"], start=form.cleaned_data.get("start"), auto_archive=form.cleaned_data["auto_archive"], auto_renew=form.cleaned_data["auto_renew"]) messages.success(self.request, f"{club} is on {form.cleaned_data['tier']}. Its first period is open.") except BillingError as error: messages.error(self.request, str(error))