diff --git a/.env.production.example b/.env.production.example index 5a86d9b..b9cbd2d 100644 --- a/.env.production.example +++ b/.env.production.example @@ -36,3 +36,15 @@ DJANGO_STATICFILES_BACKEND=whitenoise.storage.CompressedManifestStaticFilesStora # AWS_S3_REGION_NAME=fsn1 # AWS_ACCESS_KEY_ID= # AWS_SECRET_ACCESS_KEY= + +# --- Email: any SMTP provider. Left unset, mail is PRINTED TO THE LOG and never delivered, +# which means send_billing_reminders will look like it worked while no club hears from you. +DJANGO_EMAIL_BACKEND=django.core.mail.backends.smtp.EmailBackend +DJANGO_EMAIL_HOST=smtp.example.com +DJANGO_EMAIL_PORT=587 +DJANGO_EMAIL_HOST_USER= +DJANGO_EMAIL_HOST_PASSWORD= +DJANGO_EMAIL_USE_TLS=True +DJANGO_DEFAULT_FROM_EMAIL=RosterChief +# Where a club is told to reply with a billing question. +ROSTERCHIEF_BILLING_CONTACT_EMAIL=billing@rosterchief.app diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index c76be18..ddbf454 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -307,6 +307,44 @@ season-scoped** (§5.1): it gains a `season` FK and sign-up / fee-status fields, is one member's affiliation for one season (`unique_together (club, member, season)`). This is the record the `MEMBER` role and shop fulfilment key off of (§3.4, §5.7). +### `billing` — what the platform charges a club + +**Deliberately NOT club-scoped, and the only app that isn't.** `shop` (§5.7) is a club charging +its *members* — tenant data, owned by the club. `billing` is RosterChief charging the *club*: +platform-owned, never visible to a club user except as the one notice described below. Nothing +here inherits `ClubScopedModel` — these rows reference a `Club`, they are not owned by one, and a +tenant-scoped manager would be exactly the wrong default. + +**`Plan`** — a duration and three clocks, named for what they measure *from*, which is the easy +thing to get wrong: `duration_months` (period length, from its start), `renewal_lead_days` (how far +*before* a period starts its invoice is raised), `grace_days` (how long *after* a period starts it +may stay unpaid). Two `CheckConstraint`s keep them coherent. `is_trial` marks a plan offered as a +trial; a trial's length is simply its own `duration_months`. + +**`PlanPrice`** — a dated price (`active_from`). A rate change is a new row, never an edit, so +every period already opened keeps what it was billed at. + +**`Subscription`** — one per club (`OneToOneField`): its current `plan`, `auto_renew`, +`auto_archive`, and the trial pair (`trial_ends_at` + `post_trial_plan`, constrained to be set +together or not at all). + +**`Due`** — one billing period for one club, and **the snapshot boundary**. `plan`, `amount`, +`period_end` and `grace_until` are all frozen when the period opens and never read back through +the plan at display time: raise a price or edit a plan's grace and last year's invoice must still +say what was actually charged. Storing the computed *dates* rather than the plan's *numbers* is +what buys that. + +**`DuePayment`** / **`Invoice`** — money received against a due (several may land on one), and the +gapless per-year invoice number. The PDF itself is rendered on demand from the `Due` snapshot; +only the number is stored. + +All lifecycle changes go through `billing/services/` — `dues.py` (open, renew, pay, waive, +archive), `notices.py` (the one club-facing warning), `reminders.py` (its email). Never through +the models directly: a `Due` whose `amount_paid` disagrees with its payments is a wrong invoice. + +**`BILLING.md` is the authoritative document for this app** — the lifecycle, the worked timelines, +and the migration hazards live there rather than here. + --- ## 5. Planned models (design) diff --git a/BILLING.md b/BILLING.md index 2d38b7d..7ee6279 100644 --- a/BILLING.md +++ b/BILLING.md @@ -8,7 +8,8 @@ and has never been documented there. Once this design is implemented, §4 (the m into `ARCHITECTURE.md` and this file keeps the lifecycle and operational detail, the same split `DEPLOYMENT.md` already has with the rest of the docs. -Status: **proposed, not implemented.** Nothing in §4–§9 exists on disk yet. +Status: **implemented.** The four decisions left open in §10 have been taken and are recorded +there. --- @@ -375,18 +376,31 @@ which is expected — but the annotations themselves need renaming, not just re- - `post_trial_plan` stays on `Subscription`, not on `Plan` — the same trial can convert to different paid plans for different clubs. -**Still open — worth deciding before implementation:** +**Resolved during implementation:** -1. **Does full payment auto-restore an archived club?** Today `reactivate()` is explicit. Automatic - restoration is friendlier ("they paid, let them back in") but requires knowing the club was - archived *for non-payment* rather than by hand — which means an `archived_reason` field. The - cheap version is to leave it explicit and surface a prominent "Reactivate" action on any archived - club whose dues are now settled. Recommend the cheap version. -2. **Banner placement.** `management/home.html` only, or persistent in `management/base.html`? Home - matches the request; base is harder to ignore in the final week. -3. **Email reminders.** Out of scope here — the request is for an in-app warning — but a - `send_billing_reminders` command is the obvious follow-up, and the `BillingNotice` service in §6 - is deliberately shaped so it could feed one without rework. -4. **Online payment.** Entirely out of scope. Every payment is still recorded by hand by a platform - admin (`record_payment`). Worth knowing that the warning tells a club admin money is due while - giving them no way to pay it in-app. +1. **Full payment does NOT auto-restore an archived club.** `reactivate()` stays an explicit + platform-admin action — a club can also be archived by hand for reasons that have nothing to do + with money, and an automatic restore would silently reverse that the next time a stray payment + was recorded. Instead, `_club_billing_card.html` shows a prominent prompt on any archived club + whose dues are settled, so the deliberate act is one click away. No `archived_reason` field was + needed. +2. **The banner escalates.** `management/home.html` renders it at every level; `management/base.html` + repeats it on every *other* management page only once it reaches `error` (≤7 days, or overdue). + Shown from the moment anything is owed it would sit on every screen for weeks and train people to + ignore the one week that matters. +3. **Email reminders are in.** `send_billing_reminders` (dry-run by default, `--commit` to send) + plus provider-agnostic SMTP settings read from the environment. Reminders go **once per + escalation level**, tracked on `Due.last_reminder_level`, because the command is on a daily cron + and a club that owes money for a month must not get thirty identical emails. +4. **Online payment stays out of scope.** Every payment is still recorded by hand by a platform + admin (`record_payment`). The consequence is real and worth stating: the banner and the reminder + email both tell a club admin money is due while giving them no way to pay it in-app. They pay by + transfer; you record it. + +### The email default that will catch you out + +`EMAIL_BACKEND` defaults to the **console backend**, not SMTP. That is deliberate — Django's own +default tries localhost:25 and raises `ConnectionRefused` on a box with no MTA — but it means a +deployment that forgets `DJANGO_EMAIL_HOST` will watch `send_billing_reminders --commit` report +success while no club hears anything. Set the mail variables in `.env.production` (see +`.env.production.example`) before trusting the job. diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index e9526ae..c52bfbd 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -117,15 +117,22 @@ docker compose up -d --no-deps web ## Scheduled jobs -Three commands need to run on a schedule. Put them on the **host**, not in a container, and on +Four commands need to run on a schedule. Put them on the **host**, not in a container, and on **exactly one node** when you have several — three nodes archiving the same club is three emails to the same club. ```cron +# Bill: remind club admins about outstanding platform fees. Dry-run by default, same as the +# archive job below — this one mails paying customers, so --commit is opt-in. Reminders go +# once per escalation level, not once per run, so a daily cron is not a daily email. +0 5 * * * cd /srv/rosterchief && docker compose run --rm web python manage.py send_billing_reminders --commit + # Bill: archive clubs unpaid past their grace period. # Run it WITHOUT --commit for the first week and read the output. The flag exists because # this switches off paying customers: a bad clock or a bad cron should cost you an email, -# not a morning of angry clubs. +# not a morning of angry clubs. Since grace now runs from the period START rather than its +# end (see BILLING.md §3), this job is load-bearing in a way it never used to be — a club +# is archivable ~60 days after being invoiced, not ~410. Re-do the dry-run week. 0 6 * * * cd /srv/rosterchief && docker compose run --rm web python manage.py archive_overdue_clubs --commit # Events: extend recurring series so the calendar never runs dry. diff --git a/billing/admin.py b/billing/admin.py index 82be11f..8760464 100644 --- a/billing/admin.py +++ b/billing/admin.py @@ -1,32 +1,32 @@ from django.contrib import admin -from .models import Due, DuePayment, Subscription, Tier, TierPrice +from .models import Due, DuePayment, Plan, PlanPrice, Subscription -class TierPriceInline(admin.TabularInline): - model = TierPrice +class PlanPriceInline(admin.TabularInline): + model = PlanPrice extra = 0 -@admin.register(Tier) -class TierAdmin(admin.ModelAdmin): - list_display = ["name", "is_active"] - list_filter = ["is_active"] +@admin.register(Plan) +class PlanAdmin(admin.ModelAdmin): + list_display = ["name", "duration_months", "renewal_lead_days", "grace_days", "is_trial", "is_active"] + list_filter = ["is_active", "is_trial"] search_fields = ["name"] prepopulated_fields = {"slug": ["name"]} - inlines = [TierPriceInline] + inlines = [PlanPriceInline] -@admin.register(TierPrice) -class TierPriceAdmin(admin.ModelAdmin): - list_display = ["tier", "amount", "active_from"] - list_filter = ["tier"] +@admin.register(PlanPrice) +class PlanPriceAdmin(admin.ModelAdmin): + list_display = ["plan", "amount", "active_from"] + list_filter = ["plan"] @admin.register(Subscription) class SubscriptionAdmin(admin.ModelAdmin): - list_display = ["club", "tier", "auto_renew", "auto_archive"] - list_filter = ["tier", "auto_renew", "auto_archive"] + list_display = ["club", "plan", "auto_renew", "auto_archive"] + list_filter = ["plan", "auto_renew", "auto_archive"] search_fields = ["club__name"] @@ -38,11 +38,13 @@ class DuePaymentInline(admin.TabularInline): @admin.register(Due) class DueAdmin(admin.ModelAdmin): - list_display = ["club", "tier", "period_start", "period_end", "amount", "amount_paid", "status"] - list_filter = ["status", "tier"] + list_display = ["club", "plan", "period_start", "period_end", "grace_until", "amount", "amount_paid", "status"] + list_filter = ["status", "plan"] search_fields = ["club__name"] # Money is settled by the billing service, which re-derives these from the payments. - readonly_fields = ["amount_paid", "status", "paid_at"] + # period_end/grace_until are snapshots taken when the period opened -- editing a plan + # afterwards must not move them, and neither should a hand edit here. + readonly_fields = ["amount_paid", "status", "paid_at", "period_end", "grace_until"] inlines = [DuePaymentInline] diff --git a/billing/management/commands/archive_overdue_clubs.py b/billing/management/commands/archive_overdue_clubs.py index 113c57a..531a2f3 100644 --- a/billing/management/commands/archive_overdue_clubs.py +++ b/billing/management/commands/archive_overdue_clubs.py @@ -27,7 +27,7 @@ class Command(MaintenanceAwareCommand): for due in overdue: days = (today - due.grace_until).days - self.stdout.write(f"{due.club} — {due.tier}, {due.balance} owed, grace ended {due.grace_until} ({days} day{'s'[: days != 1]} ago)") + self.stdout.write(f"{due.club} — {due.plan}, {due.balance} owed, grace ended {due.grace_until} ({days} day{'s'[: days != 1]} ago)") if not options["commit"]: self.stdout.write(self.style.WARNING(f"\nDry run: {len(overdue)} club(s) would be archived. Re-run with --commit to do it.")) diff --git a/billing/management/commands/renew_subscriptions.py b/billing/management/commands/renew_subscriptions.py index d642f57..c9a0645 100644 --- a/billing/management/commands/renew_subscriptions.py +++ b/billing/management/commands/renew_subscriptions.py @@ -9,7 +9,6 @@ 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 @@ -20,7 +19,7 @@ class Command(MaintenanceAwareCommand): 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}).") + parser.add_argument("--lead-days", type=int, default=None, help="Override every plan's own renewal lead. Left off, each plan uses its own.") def handle(self, *args, **options): due_for_renewal = subscriptions_due_for_renewal(lead_days=options["lead_days"]) @@ -34,13 +33,13 @@ class Command(MaintenanceAwareCommand): 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'}") + self.stdout.write(f"would renew {club} — {subscription.plan}, 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. + # One unpriced plan must not stop every other club from being billed. failures.append(f"{club}: {error}") self.stdout.write(self.style.ERROR(f"{club} — {error}")) continue diff --git a/billing/management/commands/send_billing_reminders.py b/billing/management/commands/send_billing_reminders.py new file mode 100644 index 0000000..a3484e2 --- /dev/null +++ b/billing/management/commands/send_billing_reminders.py @@ -0,0 +1,64 @@ +"""Email club admins about platform fees they owe. + +Reports by default and only sends with --commit, the same posture as archive_overdue_clubs +and for a related reason: this one mails paying customers, and a bad clock, a bad import or a +rehearsal against production data should cost you a confusing dry-run listing rather than a +mailshot you cannot recall. + +Idempotent across runs by design -- one reminder per due per escalation level, tracked on +Due.last_reminder_level -- so a daily cron does not produce a daily email. +""" + +from django.core.management.base import CommandError + +from billing.services.reminders import reminders_to_send, send_reminder +from club.models import Club +from features.commands import MaintenanceAwareCommand + + +class Command(MaintenanceAwareCommand): + help = "Email club admins about outstanding platform fees (dry run unless --commit)." + + def add_arguments(self, parser): + parser.add_argument("--commit", action="store_true", help="Actually send. Without this the command only reports.") + parser.add_argument("--force", action="store_true", help="Re-send even where a reminder already went out at this level.") + + def handle(self, *args, **options): + clubs = Club.objects.active().select_related("subscription", "subscription__plan").order_by("name") + results = reminders_to_send(clubs, force=options["force"]) + + sendable = [result for result in results if result.sent] + skipped = [result for result in results if not result.sent] + + if not results: + self.stdout.write(self.style.SUCCESS("Nothing owing. No reminders to send.")) + return + + for result in skipped: + style = self.style.WARNING if result.recipients else self.style.ERROR + self.stdout.write(style(f"{result.club} — skipped: {result.skipped_reason}")) + + for result in sendable: + self.stdout.write(f"{result.notice.level:>7} · {result.club} — €{result.notice.amount_outstanding} owed, {result.notice.days_until_archive}d to archive → {', '.join(result.recipients)}") + + if not options["commit"]: + self.stdout.write(self.style.WARNING(f"\nDry run: {len(sendable)} reminder(s) would be sent. Re-run with --commit to send them.")) + return + + failures = [] + for result in sendable: + try: + send_reminder(result.club, result.notice, recipients=result.recipients) + except OSError as error: + # One bad address or a momentary SMTP failure must not stop the rest of the + # run: the clubs further down the list are the ones closest to being archived. + failures.append(f"{result.club}: {error}") + self.stdout.write(self.style.ERROR(f"{result.club} — {error}")) + + sent = len(sendable) - len(failures) + self.stdout.write(self.style.SUCCESS(f"\nSent {sent} reminder(s).")) + + if failures: + # Non-zero so cron mails you: a club that could not be warned is a club that gets + # archived without notice. + raise CommandError(f"{len(failures)} reminder(s) could not be sent:\n " + "\n ".join(failures)) diff --git a/billing/migrations/0004_rename_tier_to_plan.py b/billing/migrations/0004_rename_tier_to_plan.py new file mode 100644 index 0000000..bc26e48 --- /dev/null +++ b/billing/migrations/0004_rename_tier_to_plan.py @@ -0,0 +1,34 @@ +"""Rename Tier -> Plan, and every field that pointed at it. + +Hand-written rather than generated. `makemigrations` only detects a rename by asking +interactively; run non-interactively it emits DeleteModel + CreateModel instead, which drops +every price, subscription and due in the table. RenameModel/RenameField preserve the data. + +The two RemoveConstraints have to come FIRST. Both constraints name fields this migration is +about to rename (`tier`, `post_trial_tier`), and SQLite implements a rename by rebuilding the +table -- which re-renders every constraint on it. Left in place, the rebuild tries to emit a +constraint over a column that no longer exists under that name and dies with +FieldDoesNotExist. 0005 adds them back under the new field names. + +Split from the field additions (0005) so this migration is pure renaming and can be read -- +and if necessary reversed -- without any other change mixed into it. +""" + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("billing", "0003_due_is_trial_subscription_post_trial_tier_and_more"), + ] + + operations = [ + migrations.RemoveConstraint(model_name="tierprice", name="unique_tier_price_per_start_date"), + migrations.RemoveConstraint(model_name="subscription", name="trial_fields_set_together"), + migrations.RenameModel(old_name="Tier", new_name="Plan"), + migrations.RenameModel(old_name="TierPrice", new_name="PlanPrice"), + migrations.RenameField(model_name="planprice", old_name="tier", new_name="plan"), + migrations.RenameField(model_name="subscription", old_name="tier", new_name="plan"), + migrations.RenameField(model_name="subscription", old_name="post_trial_tier", new_name="post_trial_plan"), + migrations.RenameField(model_name="due", old_name="tier", new_name="plan"), + ] diff --git a/billing/migrations/0005_alter_plan_options_alter_planprice_options_and_more.py b/billing/migrations/0005_alter_plan_options_alter_planprice_options_and_more.py new file mode 100644 index 0000000..ce520f4 --- /dev/null +++ b/billing/migrations/0005_alter_plan_options_alter_planprice_options_and_more.py @@ -0,0 +1,96 @@ +# Generated by Django 6.0.6 on 2026-08-08 16:30 + +import django.core.validators +import django.db.models.deletion +import django.db.models.expressions +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('billing', '0004_rename_tier_to_plan'), + ('club', '0020_sponsor_logo_height_sponsor_logo_width'), + ] + + operations = [ + migrations.AlterModelOptions( + name='plan', + options={'ordering': ['name'], 'verbose_name': 'plan', 'verbose_name_plural': 'plans'}, + ), + migrations.AlterModelOptions( + name='planprice', + options={'ordering': ['plan__name', '-active_from'], 'verbose_name': 'plan price', 'verbose_name_plural': 'plan prices'}, + ), + migrations.AddField( + model_name='plan', + name='duration_months', + field=models.PositiveSmallIntegerField(default=12, help_text='How long one billing period runs.', validators=[django.core.validators.MinValueValidator(1)], verbose_name='duration (months)'), + ), + migrations.AddField( + model_name='plan', + name='grace_days', + field=models.PositiveSmallIntegerField(default=30, help_text='Days after a period starts before an unpaid club is archived.', verbose_name='grace (days)'), + ), + migrations.AddField( + model_name='plan', + name='is_trial', + field=models.BooleanField(default=False, 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.', verbose_name='trial plan'), + ), + migrations.AddField( + model_name='plan', + name='renewal_lead_days', + field=models.PositiveSmallIntegerField(default=30, help_text="Raise the next period's invoice this many days before that period starts.", verbose_name='renewal lead (days)'), + ), + migrations.AlterField( + model_name='due', + name='grace_until', + field=models.DateField(blank=True, help_text='Past this date an unpaid club is archived. Measured from the period start, not its end.', verbose_name='grace until'), + ), + migrations.AlterField( + model_name='due', + name='plan', + field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='dues', to='billing.plan', verbose_name='plan'), + ), + migrations.AlterField( + model_name='plan', + name='is_active', + field=models.BooleanField(default=True, help_text='Inactive plans keep billing existing subscriptions but cannot be chosen for new ones.', verbose_name='active'), + ), + migrations.AlterField( + model_name='planprice', + name='plan', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='prices', to='billing.plan', verbose_name='plan'), + ), + migrations.AlterField( + model_name='subscription', + name='plan', + field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='subscriptions', to='billing.plan', verbose_name='plan'), + ), + migrations.AlterField( + model_name='subscription', + name='post_trial_plan', + field=models.ForeignKey(blank=True, help_text='The plan this club switches to automatically once its trial ends.', null=True, on_delete=django.db.models.deletion.PROTECT, related_name='+', to='billing.plan', verbose_name='post-trial plan'), + ), + migrations.AlterField( + model_name='subscription', + name='trial_ends_at', + field=models.DateField(blank=True, help_text='Set while this club is on a trial. The plan switches to the post-trial plan the next time a period is opened after this date.', null=True, verbose_name='trial ends at'), + ), + migrations.AddConstraint( + model_name='plan', + constraint=models.CheckConstraint(condition=models.Q(('renewal_lead_days__lt', django.db.models.expressions.CombinedExpression(models.F('duration_months'), '*', models.Value(28)))), name='renewal_lead_shorter_than_duration'), + ), + migrations.AddConstraint( + model_name='plan', + constraint=models.CheckConstraint(condition=models.Q(('grace_days__lte', django.db.models.expressions.CombinedExpression(models.F('duration_months'), '*', models.Value(28)))), name='grace_no_longer_than_duration'), + ), + migrations.AddConstraint( + model_name='planprice', + constraint=models.UniqueConstraint(fields=('plan', 'active_from'), name='unique_plan_price_per_start_date'), + ), + migrations.AddConstraint( + model_name='subscription', + constraint=models.CheckConstraint(condition=models.Q(models.Q(('post_trial_plan__isnull', True), ('trial_ends_at__isnull', True)), models.Q(('post_trial_plan__isnull', False), ('trial_ends_at__isnull', False)), _connector='OR'), name='trial_fields_set_together'), + ), + ] diff --git a/billing/migrations/0006_due_last_reminder_level_due_last_reminder_sent_at.py b/billing/migrations/0006_due_last_reminder_level_due_last_reminder_sent_at.py new file mode 100644 index 0000000..d3a4ade --- /dev/null +++ b/billing/migrations/0006_due_last_reminder_level_due_last_reminder_sent_at.py @@ -0,0 +1,23 @@ +# Generated by Django 6.0.6 on 2026-08-08 16:33 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('billing', '0005_alter_plan_options_alter_planprice_options_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='due', + name='last_reminder_level', + field=models.CharField(blank=True, editable=False, max_length=20, verbose_name='last reminder level'), + ), + migrations.AddField( + model_name='due', + name='last_reminder_sent_at', + field=models.DateTimeField(blank=True, editable=False, null=True, verbose_name='last reminder sent at'), + ), + ] diff --git a/billing/models.py b/billing/models.py index b00866d..31edd8d 100644 --- a/billing/models.py +++ b/billing/models.py @@ -11,9 +11,10 @@ from decimal import Decimal from dateutil import relativedelta from django.conf import settings +from django.core.exceptions import ValidationError from django.core.validators import MinValueValidator from django.db import models -from django.db.models import Q +from django.db.models import F, Q from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -21,39 +22,89 @@ from rosterchief.base import UUIDModel, unique_slugify 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 +#: Conservative lower bound on the number of days in a month, used to express the plan's +#: clock invariants as CheckConstraints — month arithmetic is not available in SQL, and +#: under-counting is the safe direction for a guard rail. +DAYS_PER_MONTH_FLOOR = 28 -# 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: - return day + relativedelta.relativedelta(years=1) +# Defaults for a new plan, chosen to reproduce the annual billing the platform started with. +DEFAULT_DURATION_MONTHS = 12 +DEFAULT_RENEWAL_LEAD_DAYS = 30 +DEFAULT_GRACE_DAYS = 30 def add_months(day: date, months: int) -> date: return day + relativedelta.relativedelta(months=months) -class Tier(UUIDModel): - """A price band. The price itself lives in TierPrice, which is dated.""" +class Plan(UUIDModel): + """What a club is billed on: a duration, a set of clocks, and a dated price. + + The price itself lives in PlanPrice, which is dated. The three day/month numbers here + are the plan's *clocks*, and they are named for what they measure from — see BILLING.md + §3, because confusing them is the easy mistake: + + * ``duration_months`` — how long a period runs, from its start. + * ``renewal_lead_days`` — how far BEFORE a period starts its invoice is raised. + * ``grace_days`` — how long AFTER a period starts it may remain unpaid. + """ name = models.CharField(_("name"), max_length=255) slug = models.SlugField(_("slug"), max_length=255, unique=True, blank=True) description = models.TextField(_("description"), blank=True) - is_active = models.BooleanField(_("active"), default=True, help_text=_("Inactive tiers keep billing existing subscriptions but cannot be chosen for new ones.")) + is_active = models.BooleanField(_("active"), default=True, help_text=_("Inactive plans keep billing existing subscriptions but cannot be chosen for new ones.")) + + duration_months = models.PositiveSmallIntegerField(_("duration (months)"), default=DEFAULT_DURATION_MONTHS, validators=[MinValueValidator(1)], help_text=_("How long one billing period runs.")) + renewal_lead_days = models.PositiveSmallIntegerField(_("renewal lead (days)"), default=DEFAULT_RENEWAL_LEAD_DAYS, help_text=_("Raise the next period's invoice this many days before that period starts.")) + grace_days = models.PositiveSmallIntegerField(_("grace (days)"), default=DEFAULT_GRACE_DAYS, help_text=_("Days after a period starts before an unpaid club is archived.")) + + is_trial = models.BooleanField( + _("trial plan"), + default=False, + 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."), + ) class Meta: - verbose_name = _("tier") - verbose_name_plural = _("tiers") + verbose_name = _("plan") + verbose_name_plural = _("plans") ordering = ["name"] + constraints = [ + # Lead longer than the period itself would raise the next invoice before the + # current period had even started, and periods would run away from the calendar. + models.CheckConstraint( + condition=Q(renewal_lead_days__lt=F("duration_months") * DAYS_PER_MONTH_FLOOR), + name="renewal_lead_shorter_than_duration", + ), + # Grace longer than the period means the next period is issued while this one is + # still in grace: unpaid periods stack and the club is never archived. + models.CheckConstraint( + condition=Q(grace_days__lte=F("duration_months") * DAYS_PER_MONTH_FLOOR), + name="grace_no_longer_than_duration", + ), + ] def __str__(self): return self.name + def clean(self): + """The same two invariants the CheckConstraints enforce, as form errors. + + Without this a form would hand the database an impossible plan and get back an + IntegrityError -- a 500 rather than "that lead is longer than the period". + """ + if not self.duration_months: + return + + period_days = self.duration_months * DAYS_PER_MONTH_FLOOR + errors = {} + if self.renewal_lead_days is not None and self.renewal_lead_days >= period_days: + errors["renewal_lead_days"] = _("Must be shorter than the period itself (under %(days)s days for this duration), or the next invoice would be raised before the current period starts.") % {"days": period_days} + if self.grace_days is not None and self.grace_days > period_days: + errors["grace_days"] = _("Must not be longer than the period itself (at most %(days)s days for this duration), or unpaid periods stack up and the club is never archived.") % {"days": period_days} + + if errors: + raise ValidationError(errors) + def save(self, *args, **kwargs): if not self.slug: self.slug = unique_slugify(self, self.name) @@ -62,7 +113,7 @@ class Tier(UUIDModel): def price_on(self, day: date | None = None) -> Decimal | None: """The price in force on ``day`` — the latest one that had started by then. - None means the tier had no price yet on that date. Callers must treat that as + None means the plan had no price yet on that date. Callers must treat that as "cannot bill", never as free. """ day = day or timezone.localdate() @@ -71,40 +122,40 @@ class Tier(UUIDModel): return price.amount if price else None -class TierPrice(UUIDModel): - """A dated price for a tier. +class PlanPrice(UUIDModel): + """A dated price for a plan. Dated rather than keyed by year: a rate change is one new row with a future ``active_from``, and every period already opened keeps the amount it was billed at. """ - tier = models.ForeignKey(Tier, on_delete=models.CASCADE, related_name="prices", verbose_name=_("tier")) + plan = models.ForeignKey(Plan, on_delete=models.CASCADE, related_name="prices", verbose_name=_("plan")) active_from = models.DateField(_("active from"), help_text=_("Periods opening on or after this date are billed at this amount.")) amount = models.DecimalField(_("amount"), max_digits=10, decimal_places=2, validators=[MinValueValidator(ZERO)]) class Meta: - verbose_name = _("tier price") - verbose_name_plural = _("tier prices") - ordering = ["tier__name", "-active_from"] + verbose_name = _("plan price") + verbose_name_plural = _("plan prices") + ordering = ["plan__name", "-active_from"] constraints = [ - models.UniqueConstraint(fields=["tier", "active_from"], name="unique_tier_price_per_start_date"), + models.UniqueConstraint(fields=["plan", "active_from"], name="unique_plan_price_per_start_date"), ] def __str__(self): - return f"{self.tier} — {self.amount} from {self.active_from}" + return f"{self.plan} — {self.amount} from {self.active_from}" class Subscription(UUIDModel): """A club's current plan. The periods it is billed for are Dues.""" 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")) + plan = models.ForeignKey(Plan, on_delete=models.PROTECT, related_name="subscriptions", verbose_name=_("plan")) 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) - trial_ends_at = models.DateField(_("trial ends at"), null=True, blank=True, help_text=_("Set while this club is on a trial. The tier switches to post_trial_tier the next time a period is opened after this date.")) - post_trial_tier = models.ForeignKey(Tier, on_delete=models.PROTECT, null=True, blank=True, related_name="+", verbose_name=_("post-trial tier"), help_text=_("The plan this club switches to automatically once its trial ends.")) + trial_ends_at = models.DateField(_("trial ends at"), null=True, blank=True, help_text=_("Set while this club is on a trial. The plan switches to the post-trial plan the next time a period is opened after this date.")) + post_trial_plan = models.ForeignKey(Plan, on_delete=models.PROTECT, null=True, blank=True, related_name="+", verbose_name=_("post-trial plan"), help_text=_("The plan this club switches to automatically once its trial ends.")) class Meta: verbose_name = _("subscription") @@ -114,21 +165,25 @@ class Subscription(UUIDModel): # Both set together or neither -- a trial with no target plan (or a target # plan with no trial end date) is a half-configured state nothing should read. models.CheckConstraint( - condition=Q(trial_ends_at__isnull=True, post_trial_tier__isnull=True) | Q(trial_ends_at__isnull=False, post_trial_tier__isnull=False), + condition=Q(trial_ends_at__isnull=True, post_trial_plan__isnull=True) | Q(trial_ends_at__isnull=False, post_trial_plan__isnull=False), name="trial_fields_set_together", ), ] def __str__(self): - return f"{self.club} — {self.tier}" + return f"{self.club} — {self.plan}" class Due(UUIDModel): """One billing period for one club. - ``tier`` and ``amount`` are snapshots taken when the period opens, never read back - through the tier at display time: raise the price and last year's period must still say + ``plan`` and ``amount`` are snapshots taken when the period opens, never read back + through the plan at display time: raise the price and last year's period must still say what was actually charged. A live lookup would rewrite financial history. + + ``period_end`` and ``grace_until`` are snapshots for the same reason. They are stored as + *dates* rather than as the plan's duration/grace *numbers*, which is what makes editing a + plan afterwards leave every period already running exactly where it was. """ class Status(models.TextChoices): @@ -142,20 +197,27 @@ class Due(UUIDModel): OWING = (Status.UNPAID, Status.PARTIAL) club = models.ForeignKey("club.Club", on_delete=models.CASCADE, related_name="dues", verbose_name=_("club")) - tier = models.ForeignKey(Tier, on_delete=models.PROTECT, related_name="dues", verbose_name=_("tier")) + plan = models.ForeignKey(Plan, on_delete=models.PROTECT, related_name="dues", verbose_name=_("plan")) amount = models.DecimalField(_("amount"), max_digits=10, decimal_places=2, validators=[MinValueValidator(ZERO)]) amount_paid = models.DecimalField(_("amount paid"), max_digits=10, decimal_places=2, default=ZERO, help_text=_("Kept in step with the payments by the billing service.")) period_start = models.DateField(_("period start")) period_end = models.DateField(_("period end"), blank=True) - grace_until = models.DateField(_("grace until"), blank=True, help_text=_("Past this date an unpaid club is archived.")) + grace_until = models.DateField(_("grace until"), blank=True, help_text=_("Past this date an unpaid club is archived. Measured from the period start, not its end.")) status = models.CharField(_("status"), max_length=20, choices=Status.choices, default=Status.UNPAID) paid_at = models.DateTimeField(_("paid at"), null=True, blank=True) is_trial = models.BooleanField(_("trial period"), default=False, help_text=_("This period was opened as a trial. A durable marker on the row itself -- the subscription's own trial fields are cleared once it converts.")) + # Reminders are sent once per escalation level, not once per run: the cron job runs daily, + # and a club that owes money for a month must not get thirty identical emails. Storing the + # level last sent (rather than a date) means an escalation always gets through, and nothing + # else does. See billing/services/reminders.py. + last_reminder_level = models.CharField(_("last reminder level"), max_length=20, blank=True, editable=False) + last_reminder_sent_at = models.DateTimeField(_("last reminder sent at"), null=True, blank=True, editable=False) + class Meta: verbose_name = _("due") verbose_name_plural = _("dues") @@ -168,12 +230,14 @@ class Due(UUIDModel): return f"{self.club} — {self.period_start} to {self.period_end}" def save(self, *args, **kwargs): - # A period runs a rolling year from its start and the grace hangs off its end. - # Derived here so no caller can open a period without them. + # A period runs for the plan's duration from its start, and the grace runs from that + # same start -- NOT from the period end. Measured from the end, a club would get the + # whole unpaid period plus the grace on top (~410 days on an annual plan) before + # anything switched it off. Derived here so no caller can open a period without them. if not self.period_end: - self.period_end = add_one_year(self.period_start) - timedelta(days=1) + self.period_end = add_months(self.period_start, self.plan.duration_months) - timedelta(days=1) if not self.grace_until: - self.grace_until = self.period_end + timedelta(days=GRACE_DAYS) + self.grace_until = self.period_start + timedelta(days=self.plan.grace_days) super().save(*args, **kwargs) @property @@ -184,11 +248,21 @@ class Due(UUIDModel): def is_owing(self) -> bool: return self.status in self.OWING - def is_in_grace(self, today: date | None = None) -> bool: - """The period has ended unpaid, but the club is not archivable yet.""" + def is_issued_ahead(self, today: date | None = None) -> bool: + """Billed and owing, but the period it covers has not started yet. + + The gentlest of the three owing states: the invoice was raised during the plan's + renewal lead window, and nothing is late yet. + """ today = today or timezone.localdate() - return self.is_owing and self.period_end < today <= self.grace_until + return self.is_owing and today < self.period_start + + def is_in_grace(self, today: date | None = None) -> bool: + """The period has started and is still unpaid, but is not archivable yet.""" + today = today or timezone.localdate() + + return self.is_owing and self.period_start <= today <= self.grace_until def is_overdue(self, today: date | None = None) -> bool: """Unpaid past grace — this is what makes a club archivable.""" @@ -196,6 +270,12 @@ class Due(UUIDModel): return self.is_owing and self.grace_until < today + def days_until_archive(self, today: date | None = None) -> int: + """Days left before this period makes the club archivable. Negative once past.""" + today = today or timezone.localdate() + + return (self.grace_until - today).days + class DuePayment(UUIDModel): """Money received against a due. @@ -230,7 +310,7 @@ class DuePayment(UUIDModel): class Invoice(UUIDModel): """The bill for one period. - Only the number and the issue date are stored: the money, the tier and the dates are + Only the number and the issue date are stored: the money, the plan and the dates are already frozen on the Due, so the PDF is rendered from those snapshots on demand. The number, though, must be stable and gapless — it is the thing an accountant reconciles against, so it is allocated once and never recomputed. diff --git a/billing/services/dues.py b/billing/services/dues.py index d60367e..8bca4e3 100644 --- a/billing/services/dues.py +++ b/billing/services/dues.py @@ -9,36 +9,37 @@ from django.db import transaction from django.db.models import DateField, OuterRef, Subquery, Sum from django.utils import timezone -from billing.models import RENEWAL_LEAD_DAYS, ZERO, Due, DuePayment, Subscription, Tier, add_months +from billing.models import ZERO, Due, DuePayment, Plan, Subscription, add_months 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, 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, "auto_renew": auto_renew}) +def subscribe(club, plan: Plan, *, start: date | None = None, auto_archive: bool = True, auto_renew: bool = True) -> Subscription: + """Put a club on a plan and open its first period.""" + subscription, _created = Subscription.objects.update_or_create(club=club, defaults={"plan": plan, "auto_archive": auto_archive, "auto_renew": auto_renew}) open_period(club, start=start) return subscription @transaction.atomic -def start_trial(club, trial_tier: Tier, *, post_trial_tier: Tier, trial_months: int, start: date | None = None, auto_renew: bool = True, auto_archive: bool = True) -> Due: - """Put a club on a short trial that switches itself to ``post_trial_tier`` the moment - the trial period is renewed -- see open_period()'s trial-conversion check. +def start_trial(club, trial_plan: Plan, *, post_trial_plan: Plan, start: date | None = None, auto_renew: bool = True, auto_archive: bool = True) -> Due: + """Put a club on a trial that switches itself to ``post_trial_plan`` the moment the trial + period is renewed -- see open_period()'s trial-conversion check. + + The trial's length is the trial plan's own ``duration_months``: a 1-month and a 3-month + trial are two plans, not one plan plus a number passed at the call site. Only for a club with no subscription yet -- converting an existing paying subscription into a trial is a different, deliberately unsupported operation for now. """ - if trial_months <= 0: - raise BillingError("Trial length must be at least 1 month.") if getattr(club, "subscription", None) is not None: raise BillingError(f"{club} is already subscribed -- use Change plan instead.") start = start or next_period_start(club) - trial_end = add_months(start, trial_months) - timedelta(days=1) + trial_end = add_months(start, trial_plan.duration_months) - timedelta(days=1) - Subscription.objects.create(club=club, tier=trial_tier, trial_ends_at=trial_end, post_trial_tier=post_trial_tier, auto_renew=auto_renew, auto_archive=auto_archive) + Subscription.objects.create(club=club, plan=trial_plan, trial_ends_at=trial_end, post_trial_plan=post_trial_plan, auto_renew=auto_renew, auto_archive=auto_archive) return open_period(club, start=start, period_end=trial_end, is_trial=True) @@ -57,34 +58,34 @@ def next_period_start(club, today: date | None = None) -> date: @transaction.atomic -def open_period(club, *, start: date | None = None, tier: Tier | None = None, period_end: date | None = None, is_trial: bool = False) -> Due: - """Issue the next due for a club, snapshotting the tier and the price of the day.""" +def open_period(club, *, start: date | None = None, plan: Plan | None = None, period_end: date | None = None, is_trial: bool = False) -> Due: + """Issue the next due for a club, snapshotting the plan and the price of the day.""" subscription = getattr(club, "subscription", None) - if tier is None: + if plan is None: if subscription is None: - raise BillingError(f"{club} has no tier: put it on a subscription before billing it.") + raise BillingError(f"{club} has no plan: put it on a subscription before billing it.") # A trial that has run its course: swap onto the pre-selected plan before billing - # the next period, rather than silently renewing the trial tier forever. Checked + # the next period, rather than silently renewing the trial plan forever. Checked # here (not in renew()) so it fires whether this period was opened by the renewal # command or by a platform admin clicking "Open period"/"Reactivate" by hand -- # both call open_period() directly. if subscription.trial_ends_at is not None and (start or next_period_start(club)) > subscription.trial_ends_at: - subscription.tier = subscription.post_trial_tier + subscription.plan = subscription.post_trial_plan subscription.trial_ends_at = None - subscription.post_trial_tier = None - subscription.save(update_fields=["tier", "trial_ends_at", "post_trial_tier"]) - tier = subscription.tier + subscription.post_trial_plan = None + subscription.save(update_fields=["plan", "trial_ends_at", "post_trial_plan"]) + plan = subscription.plan start = start or next_period_start(club) - amount = tier.price_on(start) + amount = plan.price_on(start) if amount is None: - raise BillingError(f"{tier} has no price in force on {start:%d %b %Y}. Add one before opening the period.") + raise BillingError(f"{plan} has no price in force on {start:%d %b %Y}. Add one before opening the period.") if club.dues.filter(period_start=start).exists(): raise BillingError(f"{club} is already billed for a period starting {start:%d %b %Y}.") - due = Due.objects.create(club=club, tier=tier, amount=amount, period_start=start, period_end=period_end, is_trial=is_trial) + due = Due.objects.create(club=club, plan=plan, amount=amount, period_start=start, period_end=period_end, is_trial=is_trial) if amount == ZERO: # Nothing is actually owed -- left at the default UNPAID, this would eventually # trip is_overdue() and get a free club archived for non-payment of nothing. @@ -157,10 +158,14 @@ def owing_dues(): def dues_in_grace(today: date | None = None): - """Period over, unpaid, not yet archivable.""" + """Period started, unpaid, not yet archivable. + + Bounded below by ``period_start``, not ``period_end``: grace now runs from the start of + the period, so a due is in grace *during* the period it covers, not after it. + """ today = today or timezone.localdate() - return owing_dues().filter(period_end__lt=today, grace_until__gte=today) + return owing_dues().filter(period_start__lte=today, grace_until__gte=today) def dues_overdue(today: date | None = None): @@ -176,7 +181,7 @@ def archivable_clubs(today: date | None = None): A club with auto_archive off is deliberately spared — that flag is how you keep a club you are negotiating with from being switched off overnight. """ - return dues_overdue(today).filter(club__archived_at__isnull=True, club__subscription__auto_archive=True).select_related("club", "tier").order_by("club__name") + return dues_overdue(today).filter(club__archived_at__isnull=True, club__subscription__auto_archive=True).select_related("club", "plan").order_by("club__name") @transaction.atomic @@ -191,27 +196,43 @@ def reactivate(club, *, start: date | None = None) -> Due: return open_period(club, start=start) -def subscriptions_due_for_renewal(today: date | None = None, lead_days: int = RENEWAL_LEAD_DAYS): +def subscriptions_due_for_renewal(today: date | None = None, lead_days: int | None = None): """Clubs whose next period should be issued now. + Each plan sets its own ``renewal_lead_days``: a single global lead is silently annual-only, + and on a 1-month plan a 30-day lead would issue the next period before the current one had + started. ``lead_days`` overrides every plan's own value — that is what makes a rehearsal or + a backfill possible, and it is what the command's --lead-days flag passes. + + The per-plan comparison is done in Python rather than SQL. The function already + materialised its result as a list, and date arithmetic against a field value is not + portably expressible across SQLite and Postgres; at platform scale (tens to low hundreds of + clubs) this is one query plus a list walk. + 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. + full duration out, which is past its 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") + subscriptions = Subscription.objects.filter(auto_renew=True, club__archived_at__isnull=True).select_related("club", "plan").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 is_due(subscription) -> bool: + if subscription.latest_period_end is None: + return True + lead = subscription.plan.renewal_lead_days if lead_days is None else lead_days + + return subscription.latest_period_end <= today + timedelta(days=lead) + + return [subscription for subscription in subscriptions if is_due(subscription)] def renew(subscription: Subscription) -> Due: diff --git a/billing/services/invoices.py b/billing/services/invoices.py index 18cde7a..0875b1f 100644 --- a/billing/services/invoices.py +++ b/billing/services/invoices.py @@ -1,6 +1,6 @@ """Invoice PDFs. -The PDF is rendered on demand from the Due's frozen snapshot (tier, amount, dates), so it +The PDF is rendered on demand from the Due's frozen snapshot (plan, amount, dates), so it carries no state of its own beyond the number. Only the number is stored — an accountant reconciles against it, so it is allocated once, never recomputed. """ diff --git a/billing/services/notices.py b/billing/services/notices.py new file mode 100644 index 0000000..1a9c516 --- /dev/null +++ b/billing/services/notices.py @@ -0,0 +1,77 @@ +"""What a club's own admins are told about money they owe the platform. + +Separate from dues.py because the audience is different: everything in dues.py is read by +platform staff in the control panel, and this is the one piece of billing a *club* sees. It +returns data, never rendered text — the wording lives in the template so it can be translated, +and the same notice feeds both the on-screen banner and the reminder email. +""" + +from dataclasses import dataclass +from datetime import date +from decimal import Decimal + +from django.utils import timezone + +from billing.models import Due + +#: Inside this many days of being archived, the notice stops being a warning and becomes a +#: final one — which is also the point at which it follows the admin onto every page. +URGENT_DAYS = 7 + +INFO = "info" +WARNING = "warning" +ERROR = "error" + + +@dataclass(frozen=True) +class BillingNotice: + """The single most pressing thing a club owes, and how alarmed to be about it.""" + + level: str + due: Due + amount_outstanding: Decimal + period_start: date + grace_until: date + days_until_archive: int + #: False when the subscription has auto_archive off. Money is still owed and still worth + #: saying so, but the countdown must not claim an archiving that will never happen. + will_archive: bool + + @property + def is_urgent(self) -> bool: + return self.level == ERROR + + +def club_billing_notice(club, today: date | None = None) -> BillingNotice | None: + """The notice for ``club``, or None when it owes nothing. + + Picks the due with the earliest ``grace_until`` when several are owing: that is the one + that will archive the club first, so it is the one worth shouting about. + """ + today = today or timezone.localdate() + + due = club.dues.filter(status__in=Due.OWING).select_related("plan").order_by("grace_until").first() + if due is None: + return None + + subscription = getattr(club, "subscription", None) + will_archive = subscription.auto_archive if subscription is not None else False + days_left = due.days_until_archive(today) + + if due.is_overdue(today): + level = ERROR + elif due.is_in_grace(today): + level = ERROR if days_left <= URGENT_DAYS else WARNING + else: + # Issued during the plan's renewal lead window: billed, but nothing is late yet. + level = INFO + + return BillingNotice( + level=level, + due=due, + amount_outstanding=due.balance, + period_start=due.period_start, + grace_until=due.grace_until, + days_until_archive=days_left, + will_archive=will_archive, + ) diff --git a/billing/services/reminders.py b/billing/services/reminders.py new file mode 100644 index 0000000..c8f1c47 --- /dev/null +++ b/billing/services/reminders.py @@ -0,0 +1,98 @@ +"""Emailing a club's admins about money it owes the platform. + +Built on the same BillingNotice the on-screen banner uses (notices.py), so the email and the +banner can never disagree about how much is owed or how long is left. + +**Sent once per escalation level, not once per run.** The command is on a daily cron; a club +that owes money for a month must not receive thirty identical emails. ``Due.last_reminder_level`` +records the level last mailed, so an escalation (info -> warning -> error) always gets through +and a repeat of the same level never does. +""" + +from dataclasses import dataclass + +from django.conf import settings +from django.core.mail import EmailMultiAlternatives +from django.template.loader import render_to_string +from django.utils import timezone +from django.utils.translation import gettext as _ + +from billing.services.notices import BillingNotice, club_billing_notice +from club.models import ClubRole + + +@dataclass(frozen=True) +class ReminderResult: + club: object + notice: BillingNotice + recipients: list[str] + sent: bool + skipped_reason: str = "" + + +def admin_emails(club) -> list[str]: + """Every club admin we can actually reach, de-duplicated and order-stable. + + A club with admins but no email addresses returns empty — the caller reports that rather + than silently counting it as reminded. + """ + roles = ClubRole.objects.filter(club=club, role=ClubRole.Roles.ADMIN).select_related("member", "member__user").order_by("member__last_name", "member__first_name") + + seen, emails = set(), [] + for role in roles: + email = role.member.contact_email + if email and email not in seen: + seen.add(email) + emails.append(email) + + return emails + + +def needs_reminder(due, notice: BillingNotice) -> bool: + """True when this due has not yet been mailed at its current level.""" + return due.last_reminder_level != notice.level + + +def send_reminder(club, notice: BillingNotice, *, recipients: list[str]) -> None: + """Render and send one reminder, then record the level so it is not repeated.""" + context = { + "club": club, + "notice": notice, + "due": notice.due, + "billing_contact": settings.BILLING_CONTACT_EMAIL, + } + subject = render_to_string("billing/email/reminder_subject.txt", context).strip() + text_body = render_to_string("billing/email/reminder.txt", context) + + message = EmailMultiAlternatives(subject=subject, body=text_body, from_email=settings.DEFAULT_FROM_EMAIL, to=recipients) + message.send(fail_silently=False) + + notice.due.last_reminder_level = notice.level + notice.due.last_reminder_sent_at = timezone.now() + notice.due.save(update_fields=["last_reminder_level", "last_reminder_sent_at", "modified"]) + + +def reminders_to_send(clubs, today=None, *, force: bool = False) -> list[ReminderResult]: + """Work out who would be reminded, without sending anything. + + Returned whether or not each one is actually sendable, so the command can report a club + with no reachable admin instead of skipping it in silence — an unreachable club is exactly + the one that gets archived without ever having been told. + """ + results = [] + for club in clubs: + notice = club_billing_notice(club, today) + if notice is None: + continue + + recipients = admin_emails(club) + if not recipients: + results.append(ReminderResult(club=club, notice=notice, recipients=[], sent=False, skipped_reason=_("no club admin with an email address"))) + continue + if not force and not needs_reminder(notice.due, notice): + results.append(ReminderResult(club=club, notice=notice, recipients=recipients, sent=False, skipped_reason=_("already reminded at this level"))) + continue + + results.append(ReminderResult(club=club, notice=notice, recipients=recipients, sent=True)) + + return results diff --git a/billing/templates/billing/email/reminder.txt b/billing/templates/billing/email/reminder.txt new file mode 100644 index 0000000..571bef2 --- /dev/null +++ b/billing/templates/billing/email/reminder.txt @@ -0,0 +1,14 @@ +{% load i18n %}{% blocktrans with club=club.name %}Hello, + +This is a reminder about the RosterChief platform fees for {{ club }}.{% endblocktrans %} + +{% blocktrans with amount=notice.amount_outstanding %}Outstanding: EUR {{ amount }}{% endblocktrans %} +{% blocktrans with start=due.period_start|date:"j M Y" end=due.period_end|date:"j M Y" %}Period: {{ start }} to {{ end }}{% endblocktrans %} +{% if due.invoice %}{% blocktrans with number=due.invoice.number %}Invoice: {{ number }}{% endblocktrans %} +{% endif %} +{% if notice.will_archive %}{% if notice.days_until_archive < 0 %}{% trans "This club is now past its payment deadline and is due to be archived. While archived, nobody can sign in and the club's site stops resolving. No data is deleted, and access is restored as soon as payment is received." %}{% else %}{% blocktrans count days=notice.days_until_archive %}If we do not receive payment within {{ days }} day, this club will be archived. While archived, nobody can sign in and the club's site stops resolving. No data is deleted.{% plural %}If we do not receive payment within {{ days }} days, this club will be archived. While archived, nobody can sign in and the club's site stops resolving. No data is deleted.{% endblocktrans %}{% endif %}{% else %}{% trans "Please settle this to keep your account in good standing." %}{% endif %} + +{% blocktrans with contact=billing_contact %}If you have already paid, or you think this is a mistake, reply to {{ contact }} and we will sort it out.{% endblocktrans %} + +{% trans "Thank you," %} +{% trans "RosterChief" %} diff --git a/billing/templates/billing/email/reminder_subject.txt b/billing/templates/billing/email/reminder_subject.txt new file mode 100644 index 0000000..d21c442 --- /dev/null +++ b/billing/templates/billing/email/reminder_subject.txt @@ -0,0 +1 @@ +{% load i18n %}{% if notice.level == 'error' %}{% blocktrans with club=club.name %}Action required: {{ club }} is about to be archived{% endblocktrans %}{% else %}{% blocktrans with club=club.name %}Platform fees are due for {{ club }}{% endblocktrans %}{% endif %} diff --git a/billing/templates/billing/invoice.html b/billing/templates/billing/invoice.html index e4f98fe..fa10404 100644 --- a/billing/templates/billing/invoice.html +++ b/billing/templates/billing/invoice.html @@ -72,7 +72,7 @@ - {{ due.tier.name }} — platform subscription + {{ due.plan.name }} — platform subscription
{{ due.period_start|date:"j M Y" }} to {{ due.period_end|date:"j M Y" }}
€{{ due.amount|floatformat:2 }} diff --git a/billing/tests.py b/billing/tests.py index b00d53f..da73c63 100644 --- a/billing/tests.py +++ b/billing/tests.py @@ -4,64 +4,78 @@ from decimal import Decimal from io import StringIO from unittest import mock +from django.core import mail +from django.core.exceptions import ValidationError from django.core.management import call_command from django.core.management.base import CommandError +from django.db.utils import IntegrityError from django.test import TestCase from django.utils import timezone -from club.models import Club +from authentication.models import User +from club.models import Club, ClubRole +from members.models import Member -from .models import GRACE_DAYS, Due, Invoice, Subscription, Tier, TierPrice, add_one_year +from .models import DEFAULT_DURATION_MONTHS, DEFAULT_GRACE_DAYS, DEFAULT_RENEWAL_LEAD_DAYS, Due, Invoice, Plan, PlanPrice, Subscription, add_months 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.reminders import admin_emails, reminders_to_send, send_reminder class BillingTestBase(TestCase): def setUp(self): self.today = timezone.localdate() self.club = Club.objects.create(name="Ajax United") - self.tier = Tier.objects.create(name="Standard") + self.plan = Plan.objects.create(name="Standard") # Priced well back, so a backdated (lapsed) period still has a price in force — # opening one before any price existed is refused, and rightly so. - TierPrice.objects.create(tier=self.tier, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("500.00")) + PlanPrice.objects.create(plan=self.plan, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("500.00")) def bill(self, start=None, club=None): - return open_period(club or self.club, start=start, tier=self.tier) + return open_period(club or self.club, start=start, plan=self.plan) -class TierPriceTests(BillingTestBase): +class PlanPriceTests(BillingTestBase): def test_the_price_in_force_is_the_latest_one_that_has_started(self): - TierPrice.objects.create(tier=self.tier, active_from=self.today, amount=Decimal("600.00")) + PlanPrice.objects.create(plan=self.plan, active_from=self.today, amount=Decimal("600.00")) - self.assertEqual(self.tier.price_on(self.today - datetime.timedelta(days=1)), Decimal("500.00")) - self.assertEqual(self.tier.price_on(self.today), Decimal("600.00")) + self.assertEqual(self.plan.price_on(self.today - datetime.timedelta(days=1)), Decimal("500.00")) + self.assertEqual(self.plan.price_on(self.today), Decimal("600.00")) def test_a_future_price_does_not_apply_yet(self): - TierPrice.objects.create(tier=self.tier, active_from=self.today + datetime.timedelta(days=30), amount=Decimal("600.00")) + PlanPrice.objects.create(plan=self.plan, active_from=self.today + datetime.timedelta(days=30), amount=Decimal("600.00")) - self.assertEqual(self.tier.price_on(self.today), Decimal("500.00")) + self.assertEqual(self.plan.price_on(self.today), Decimal("500.00")) - def test_a_tier_with_no_price_yet_cannot_be_billed(self): + def test_a_plan_with_no_price_yet_cannot_be_billed(self): # None must never be read as free. - empty = Tier.objects.create(name="Enterprise") + empty = Plan.objects.create(name="Enterprise") self.assertIsNone(empty.price_on(self.today)) with self.assertRaises(BillingError): - open_period(self.club, tier=empty) + open_period(self.club, plan=empty) class PeriodTests(BillingTestBase): - def test_a_period_runs_a_rolling_year_with_a_grace_tail(self): + def test_a_period_runs_for_the_plans_duration(self): due = self.bill(start=datetime.date(2026, 3, 1)) self.assertEqual(due.period_end, datetime.date(2027, 2, 28)) - self.assertEqual(due.grace_until, due.period_end + datetime.timedelta(days=GRACE_DAYS)) + + def test_grace_is_measured_from_the_period_start_not_its_end(self): + # The whole point of the redesign: measured from the end, an annual club would get + # ~410 days of unpaid use before anything switched it off. + due = self.bill(start=datetime.date(2026, 3, 1)) + + self.assertEqual(due.grace_until, datetime.date(2026, 3, 1) + datetime.timedelta(days=DEFAULT_GRACE_DAYS)) + self.assertLess(due.grace_until, due.period_end) def test_a_leap_day_period_does_not_explode(self): # 29 February has no counterpart in a common year. - self.assertEqual(add_one_year(datetime.date(2028, 2, 29)), datetime.date(2029, 2, 28)) + self.assertEqual(add_months(datetime.date(2028, 2, 29), 12), datetime.date(2029, 2, 28)) def test_the_next_period_continues_from_the_last_one(self): # Not from today: a club that pays two months late has still used those two months, @@ -75,7 +89,7 @@ class PeriodTests(BillingTestBase): def test_the_amount_is_snapshotted_at_the_price_of_the_day(self): due = self.bill() - TierPrice.objects.create(tier=self.tier, active_from=self.today + datetime.timedelta(days=1), amount=Decimal("900.00")) + PlanPrice.objects.create(plan=self.plan, active_from=self.today + datetime.timedelta(days=1), amount=Decimal("900.00")) due.refresh_from_db() # Raising the rate must not rewrite what was already billed. @@ -87,16 +101,16 @@ class PeriodTests(BillingTestBase): with self.assertRaises(BillingError): self.bill(start=self.today) - def test_a_club_with_no_tier_cannot_be_billed(self): + def test_a_club_with_no_plan_cannot_be_billed(self): with self.assertRaises(BillingError): open_period(Club.objects.create(name="Feyenoord")) - def test_subscribing_puts_a_club_on_a_tier_and_opens_a_period(self): + def test_subscribing_puts_a_club_on_a_plan_and_opens_a_period(self): club = Club.objects.create(name="Feyenoord") - subscribe(club, self.tier) + subscribe(club, self.plan) - self.assertEqual(Subscription.objects.get(club=club).tier, self.tier) + self.assertEqual(Subscription.objects.get(club=club).plan, self.plan) self.assertEqual(club.dues.count(), 1) @@ -175,15 +189,24 @@ class PaymentTests(BillingTestBase): class GraceAndArchiveTests(BillingTestBase): - LAPSED = 365 + GRACE_DAYS + 10 + LAPSED = DEFAULT_GRACE_DAYS + 10 - def test_a_period_past_its_end_but_inside_grace_is_in_grace(self): - due = self.bill(start=self.today - datetime.timedelta(days=370)) + def test_a_started_but_unpaid_period_inside_grace_is_in_grace(self): + # Grace runs from the period START now, so this is a period that began a few days + # ago and has not been paid -- not one that has already run its full length. + due = self.bill(start=self.today - datetime.timedelta(days=5)) self.assertTrue(due.is_in_grace(self.today)) self.assertFalse(due.is_overdue(self.today)) self.assertIn(due, dues_in_grace(self.today)) + def test_a_period_issued_ahead_of_its_start_is_not_yet_in_grace(self): + due = self.bill(start=self.today + datetime.timedelta(days=10)) + + self.assertTrue(due.is_issued_ahead(self.today)) + self.assertFalse(due.is_in_grace(self.today)) + self.assertFalse(due.is_overdue(self.today)) + def test_a_period_past_grace_is_overdue(self): due = self.bill(start=self.today - datetime.timedelta(days=self.LAPSED)) @@ -200,19 +223,19 @@ class GraceAndArchiveTests(BillingTestBase): self.assertNotIn(due, dues_overdue(self.today)) def test_an_overdue_club_is_archivable(self): - subscribe(self.club, self.tier, start=self.today - datetime.timedelta(days=self.LAPSED)) + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=self.LAPSED)) self.assertEqual(archivable_clubs(self.today).count(), 1) def test_a_club_that_opted_out_is_never_archived(self): # auto_archive off is how you stop a club you are negotiating with from being # switched off overnight. - subscribe(self.club, self.tier, start=self.today - datetime.timedelta(days=self.LAPSED), auto_archive=False) + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=self.LAPSED), auto_archive=False) self.assertEqual(archivable_clubs(self.today).count(), 0) def test_an_already_archived_club_is_not_archived_again(self): - subscribe(self.club, self.tier, start=self.today - datetime.timedelta(days=self.LAPSED)) + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=self.LAPSED)) self.club.archive() self.assertEqual(archivable_clubs(self.today).count(), 0) @@ -221,7 +244,7 @@ class GraceAndArchiveTests(BillingTestBase): class ArchiveCommandTests(BillingTestBase): def setUp(self): super().setUp() - subscribe(self.club, self.tier, start=self.today - datetime.timedelta(days=365 + GRACE_DAYS + 10)) + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=DEFAULT_GRACE_DAYS + 10)) def run_command(self, *args): out = StringIO() @@ -253,9 +276,9 @@ class ArchiveCommandTests(BillingTestBase): class ReactivationTests(BillingTestBase): def setUp(self): super().setUp() - # Through subscribe(), not open_period(): reactivating reads the club's tier off its + # Through subscribe(), not open_period(): reactivating reads the club's plan off its # subscription, and a club billed without one cannot be re-billed later. - subscribe(self.club, self.tier, start=self.today - datetime.timedelta(days=400)) + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=400)) self.first = self.club.dues.first() self.club.archive() @@ -282,7 +305,7 @@ class InvoiceTests(BillingTestBase): # Unlike the shop's per-club order numbers: these are OUR invoices, and one sequence # covers every club we bill. first = self.bill(start=self.today).invoice - second = open_period(Club.objects.create(name="Feyenoord"), tier=self.tier).invoice + second = open_period(Club.objects.create(name="Feyenoord"), plan=self.plan).invoice year = timezone.now().year self.assertEqual(first.number, f"INV-{year}-00001") @@ -330,12 +353,12 @@ class ModelStringTests(BillingTestBase): due = self.bill() payment = record_payment(due, Decimal("10.00")) - self.assertEqual(str(self.tier), "Standard") - self.assertIn("500.00", str(self.tier.prices.first())) + self.assertEqual(str(self.plan), "Standard") + self.assertIn("500.00", str(self.plan.prices.first())) self.assertIn("Ajax United", str(due)) self.assertIn("10.00", str(payment)) self.assertIn("INV-", str(due.invoice)) - self.assertIn("Standard", str(subscribe(Club.objects.create(name="PSV"), self.tier))) + self.assertIn("Standard", str(subscribe(Club.objects.create(name="PSV"), self.plan))) class RenewalTests(BillingTestBase): @@ -346,7 +369,7 @@ class RenewalTests(BillingTestBase): 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) + subscribe(club, self.plan, start=self.today - datetime.timedelta(days=365 - days), **kwargs) return club def test_a_club_nearing_its_end_date_is_picked_up(self): @@ -395,7 +418,7 @@ class RenewalTests(BillingTestBase): 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")) + PlanPrice.objects.create(plan=self.plan, active_from=self.today, amount=Decimal("900.00")) due = renew(club.subscription) @@ -427,15 +450,15 @@ class RenewalTests(BillingTestBase): self.assertEqual(club.dues.count(), 2) - def test_an_unpriced_tier_fails_loudly_without_stopping_the_others(self): + def test_an_unpriced_plan_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)) + subscribe(broken, self.plan, 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 + PlanPrice.objects.all().delete() + cheap = Plan.objects.create(name="Cheap") + PlanPrice.objects.create(plan=cheap, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("100.00")) + priced.subscription.plan = cheap priced.subscription.save() with self.assertRaises(CommandError): @@ -446,7 +469,7 @@ class RenewalTests(BillingTestBase): 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) + Subscription.objects.create(club=club, plan=self.plan) self.assertIn(club, [s.club for s in subscriptions_due_for_renewal()]) @@ -469,7 +492,7 @@ class RenewedButUnpaidTests(BillingTestBase): """A club on its first, PAID period — far enough back that a renewal from its end is itself already past grace, so only the renewal's payment state decides the outcome.""" club = Club.objects.create(name="Renewed FC") - subscribe(club, self.tier, start=self.today - datetime.timedelta(days=800)) + subscribe(club, self.plan, start=self.today - datetime.timedelta(days=800)) first = club.dues.first() record_payment(first, first.amount) # the FIRST period is settled; only the renewal is in question return club @@ -498,66 +521,281 @@ class TrialTests(BillingTestBase): def setUp(self): super().setUp() - self.trial_tier = Tier.objects.create(name="Trial") - TierPrice.objects.create(tier=self.trial_tier, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("50.00")) + # A trial is a plan whose own duration_months IS the trial length -- there is no + # trial_months argument any more. + self.trial_plan = Plan.objects.create(name="Trial", duration_months=2, is_trial=True, grace_days=14, renewal_lead_days=7) + PlanPrice.objects.create(plan=self.trial_plan, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("50.00")) def test_start_trial_creates_a_short_trial_period(self): - due = start_trial(self.club, self.trial_tier, post_trial_tier=self.tier, trial_months=2) + due = start_trial(self.club, self.trial_plan, post_trial_plan=self.plan) subscription = self.club.subscription - self.assertEqual(subscription.tier, self.trial_tier) - self.assertEqual(subscription.post_trial_tier, self.tier) + self.assertEqual(subscription.plan, self.trial_plan) + self.assertEqual(subscription.post_trial_plan, self.plan) self.assertEqual(subscription.trial_ends_at, due.period_end) self.assertTrue(due.is_trial) # Roughly 2 months, nowhere near the standard ~1-year period. self.assertLess((due.period_end - due.period_start).days, 65) def test_start_trial_refuses_if_already_subscribed(self): - subscribe(self.club, self.tier) + subscribe(self.club, self.plan) with self.assertRaises(BillingError): - start_trial(self.club, self.trial_tier, post_trial_tier=self.tier, trial_months=2) + start_trial(self.club, self.trial_plan, post_trial_plan=self.plan) - def test_start_trial_refuses_a_non_positive_length(self): - with self.assertRaises(BillingError): - start_trial(self.club, self.trial_tier, post_trial_tier=self.tier, trial_months=0) + def test_the_trials_length_comes_from_its_plan(self): + due = start_trial(self.club, self.trial_plan, post_trial_plan=self.plan) - def test_renewing_after_the_trial_switches_to_the_post_trial_tier(self): - start_trial(self.club, self.trial_tier, post_trial_tier=self.tier, trial_months=2) + self.assertEqual(due.period_end, add_months(due.period_start, 2) - datetime.timedelta(days=1)) + + def test_renewing_after_the_trial_switches_to_the_post_trial_plan(self): + start_trial(self.club, self.trial_plan, post_trial_plan=self.plan) due = renew(self.club.subscription) self.club.refresh_from_db() - self.assertEqual(self.club.subscription.tier, self.tier) + self.assertEqual(self.club.subscription.plan, self.plan) self.assertIsNone(self.club.subscription.trial_ends_at) - self.assertIsNone(self.club.subscription.post_trial_tier) - self.assertEqual(due.tier, self.tier) + self.assertIsNone(self.club.subscription.post_trial_plan) + self.assertEqual(due.plan, self.plan) self.assertFalse(due.is_trial) self.assertEqual(due.amount, Decimal("500.00")) - def test_manually_opening_the_next_period_also_switches_tier(self): + def test_manually_opening_the_next_period_also_switches_plan(self): # Same conversion must fire via the control panel's "Open period" button, which # calls open_period() directly rather than renew(). - start_trial(self.club, self.trial_tier, post_trial_tier=self.tier, trial_months=2) + start_trial(self.club, self.trial_plan, post_trial_plan=self.plan) open_period(self.club) self.club.refresh_from_db() - self.assertEqual(self.club.subscription.tier, self.tier) + self.assertEqual(self.club.subscription.plan, self.plan) def test_a_trial_nearing_its_end_is_picked_up_for_renewal(self): - start_trial(self.club, self.trial_tier, post_trial_tier=self.tier, trial_months=2, start=self.today - datetime.timedelta(days=50)) + # Inside the TRIAL PLAN's own 7-day lead, not the 30-day one an annual plan uses: + # a 2-month trial renewed a month early would be renewed before it had begun. + start_trial(self.club, self.trial_plan, post_trial_plan=self.plan, start=self.today - datetime.timedelta(days=57)) self.assertIn(self.club, [s.club for s in subscriptions_due_for_renewal()]) - def test_a_zero_amount_trial_is_created_already_paid(self): - free_tier = Tier.objects.create(name="Free Trial") - TierPrice.objects.create(tier=free_tier, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("0.00")) + def test_a_trial_outside_its_own_lead_window_is_not_yet_renewed(self): + # Same trial 7 days earlier in its life: an annual plan's 30-day lead would have + # picked this up, and the per-plan lead is exactly what stops that. + start_trial(self.club, self.trial_plan, post_trial_plan=self.plan, start=self.today - datetime.timedelta(days=40)) - due = start_trial(self.club, free_tier, post_trial_tier=self.tier, trial_months=2) + self.assertNotIn(self.club, [s.club for s in subscriptions_due_for_renewal()]) + + def test_a_zero_amount_trial_is_created_already_paid(self): + free_plan = Plan.objects.create(name="Free Trial") + PlanPrice.objects.create(plan=free_plan, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("0.00")) + + due = start_trial(self.club, free_plan, post_trial_plan=self.plan) self.assertEqual(due.status, Due.Status.PAID) self.assertIsNotNone(due.paid_at) far_future = due.grace_until + datetime.timedelta(days=100) self.assertNotIn(due, dues_overdue(far_future)) self.assertNotIn(self.club, [d.club for d in archivable_clubs(far_future)]) + + +class PlanClockTests(BillingTestBase): + """The three per-plan clocks, and the constraints that keep them sane -- see BILLING.md §3.""" + + def make_plan(self, **kwargs): + # A short plan cannot keep the annual defaults -- 30 days' lead on a 1-month period is + # exactly what the constraints forbid, so scale them down with the duration. + months = kwargs.get("duration_months", DEFAULT_DURATION_MONTHS) + defaults = {"name": f"Plan {Plan.objects.count()}", "renewal_lead_days": min(DEFAULT_RENEWAL_LEAD_DAYS, months * 7), "grace_days": min(DEFAULT_GRACE_DAYS, months * 14)} + plan = Plan.objects.create(**defaults | kwargs) + PlanPrice.objects.create(plan=plan, active_from=self.today - datetime.timedelta(days=1200), amount=Decimal("10.00")) + return plan + + def test_a_monthly_plan_gets_a_one_month_period(self): + plan = self.make_plan(duration_months=1) + + due = open_period(self.club, plan=plan, start=datetime.date(2026, 3, 1)) + + self.assertEqual(due.period_end, datetime.date(2026, 3, 31)) + + def test_a_quarterly_plan_gets_a_three_month_period(self): + plan = self.make_plan(duration_months=3) + + due = open_period(self.club, plan=plan, start=datetime.date(2026, 3, 1)) + + self.assertEqual(due.period_end, datetime.date(2026, 5, 31)) + + def test_grace_days_are_per_plan(self): + plan = self.make_plan(duration_months=1, grace_days=14) + + due = open_period(self.club, plan=plan, start=datetime.date(2026, 3, 1)) + + self.assertEqual(due.grace_until, datetime.date(2026, 3, 15)) + + def test_editing_a_plans_grace_does_not_move_an_open_period(self): + # grace_until is a stored snapshot for the same reason `amount` is: repricing the + # plan must not silently re-date an archiving already in flight. + plan = self.make_plan(grace_days=30) + due = open_period(self.club, plan=plan, start=self.today) + original = due.grace_until + + plan.grace_days = 1 + plan.save(update_fields=["grace_days"]) + due.refresh_from_db() + + self.assertEqual(due.grace_until, original) + + def test_a_lead_longer_than_the_period_is_rejected(self): + with self.assertRaises(IntegrityError): + Plan.objects.create(name="Runaway", duration_months=1, renewal_lead_days=90) + + def test_grace_longer_than_the_period_is_rejected(self): + with self.assertRaises(IntegrityError): + Plan.objects.create(name="Never archives", duration_months=1, grace_days=90) + + def test_full_clean_reports_an_impossible_lead_as_a_form_error(self): + # Not an IntegrityError/500: a platform admin typing this into the plan form should + # be told which field is wrong. + plan = Plan(name="Runaway", duration_months=1, renewal_lead_days=90, grace_days=14) + + with self.assertRaises(ValidationError) as caught: + plan.full_clean() + + self.assertIn("renewal_lead_days", caught.exception.error_dict) + + def test_renewal_lead_is_read_from_each_plan(self): + monthly = self.make_plan(duration_months=1, renewal_lead_days=7, grace_days=14) + club = Club.objects.create(name="Monthly FC") + # Period ends in 3 days: inside a 7-day lead, well outside an annual plan's 30. + subscribe(club, monthly, start=self.today - datetime.timedelta(days=27)) + + self.assertIn(club, [s.club for s in subscriptions_due_for_renewal()]) + + def test_an_explicit_lead_days_overrides_every_plan(self): + monthly = self.make_plan(duration_months=1, renewal_lead_days=1, grace_days=14) + club = Club.objects.create(name="Override FC") + subscribe(club, monthly, start=self.today - datetime.timedelta(days=20)) + + self.assertNotIn(club, [s.club for s in subscriptions_due_for_renewal()]) + self.assertIn(club, [s.club for s in subscriptions_due_for_renewal(lead_days=30)]) + + +class BillingNoticeTests(BillingTestBase): + """What a club's own admins are told -- see billing/services/notices.py.""" + + def test_no_notice_when_nothing_is_owed(self): + due = self.bill() + record_payment(due, Decimal("500.00")) + + self.assertIsNone(club_billing_notice(self.club, self.today)) + + def test_no_notice_for_a_club_that_was_never_billed(self): + self.assertIsNone(club_billing_notice(self.club, self.today)) + + def test_a_period_issued_ahead_of_its_start_is_only_informational(self): + self.bill(start=self.today + datetime.timedelta(days=10)) + + self.assertEqual(club_billing_notice(self.club, self.today).level, "info") + + def test_an_unpaid_started_period_warns(self): + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=1)) + + notice = club_billing_notice(self.club, self.today) + + self.assertEqual(notice.level, "warning") + self.assertEqual(notice.amount_outstanding, Decimal("500.00")) + self.assertFalse(notice.is_urgent) + + def test_the_last_week_before_archiving_is_urgent(self): + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=DEFAULT_GRACE_DAYS - 2)) + + notice = club_billing_notice(self.club, self.today) + + self.assertEqual(notice.level, "error") + self.assertTrue(notice.is_urgent) + self.assertEqual(notice.days_until_archive, 2) + + def test_an_overdue_period_is_urgent_with_a_negative_countdown(self): + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=DEFAULT_GRACE_DAYS + 5)) + + notice = club_billing_notice(self.club, self.today) + + self.assertEqual(notice.level, "error") + self.assertLess(notice.days_until_archive, 0) + + def test_auto_archive_off_still_reports_the_debt_but_promises_no_archiving(self): + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=1), auto_archive=False) + + notice = club_billing_notice(self.club, self.today) + + self.assertEqual(notice.amount_outstanding, Decimal("500.00")) + self.assertFalse(notice.will_archive) + + def test_the_soonest_archiving_due_is_the_one_reported(self): + self.bill(start=self.today - datetime.timedelta(days=1)) + later = self.bill(start=self.today + datetime.timedelta(days=400)) + + self.assertNotEqual(club_billing_notice(self.club, self.today).due, later) + + +class BillingReminderTests(BillingTestBase): + """Reminder emails -- see billing/services/reminders.py. Sent once per escalation + level, because the command is on a daily cron.""" + + def setUp(self): + super().setUp() + user = User.objects.create_user(email="admin@ajax.example", password="pw-secret-123") + member = Member.objects.create(user=user, first_name="Ada", last_name="Admin") + ClubRole.objects.create(club=self.club, member=member, role=ClubRole.Roles.ADMIN) + subscribe(self.club, self.plan, start=self.today - datetime.timedelta(days=1)) + self.due = self.club.dues.first() + + def test_a_reminder_goes_to_the_club_admins(self): + self.assertEqual(admin_emails(self.club), ["admin@ajax.example"]) + + def test_sending_records_the_level_and_fills_the_outbox(self): + notice = club_billing_notice(self.club, self.today) + send_reminder(self.club, notice, recipients=["admin@ajax.example"]) + + self.due.refresh_from_db() + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(self.due.last_reminder_level, notice.level) + self.assertIsNotNone(self.due.last_reminder_sent_at) + + def test_a_second_run_at_the_same_level_sends_nothing(self): + notice = club_billing_notice(self.club, self.today) + send_reminder(self.club, notice, recipients=["admin@ajax.example"]) + self.due.refresh_from_db() + + results = reminders_to_send([self.club], self.today) + + self.assertFalse(results[0].sent) + self.assertIn("already reminded", results[0].skipped_reason) + + def test_an_escalation_gets_through(self): + send_reminder(self.club, club_billing_notice(self.club, self.today), recipients=["admin@ajax.example"]) + + # Far enough on that the same due is now urgent rather than merely a warning. + later = self.today + datetime.timedelta(days=DEFAULT_GRACE_DAYS) + results = reminders_to_send([self.club], later) + + self.assertTrue(results[0].sent) + self.assertEqual(results[0].notice.level, "error") + + def test_force_resends_at_the_same_level(self): + send_reminder(self.club, club_billing_notice(self.club, self.today), recipients=["admin@ajax.example"]) + self.due.refresh_from_db() + + self.assertTrue(reminders_to_send([self.club], self.today, force=True)[0].sent) + + def test_a_club_with_no_reachable_admin_is_reported_not_skipped_silently(self): + ClubRole.objects.all().delete() + + results = reminders_to_send([self.club], self.today) + + self.assertFalse(results[0].sent) + self.assertIn("no club admin", results[0].skipped_reason) + + def test_a_settled_club_produces_no_reminder(self): + record_payment(self.due, Decimal("500.00")) + + self.assertEqual(reminders_to_send([self.club], self.today), []) diff --git a/controlpanel/forms.py b/controlpanel/forms.py index c837a60..4ebabe7 100644 --- a/controlpanel/forms.py +++ b/controlpanel/forms.py @@ -4,7 +4,7 @@ from django import forms from django.utils.translation import gettext_lazy as _ from waffle import get_waffle_flag_model -from billing.models import DuePayment, Subscription, Tier, TierPrice +from billing.models import DuePayment, Plan, PlanPrice, Subscription from club.models import Club from events.models import Location @@ -82,51 +82,61 @@ class FlagForm(forms.ModelForm): } -class TierForm(forms.ModelForm): +class PlanForm(forms.ModelForm): class Meta: - model = Tier - fields = ["name", "description", "is_active"] + model = Plan + fields = ["name", "description", "duration_months", "renewal_lead_days", "grace_days", "is_trial", "is_active"] -class TierPriceForm(forms.ModelForm): +class PlanPriceForm(forms.ModelForm): class Meta: - model = TierPrice + model = PlanPrice fields = ["active_from", "amount"] widgets = {"active_from": forms.DateInput(attrs={"type": "date"})} - help_texts = {"active_from": _("Periods opening on or after this date are billed at this amount. Existing periods keep the amount they were billed at.")} + help_texts = { + "active_from": _( + "Periods opening on or after this date are billed at this amount. Existing periods keep the amount " + "they were billed at — including any already issued during a plan's renewal lead window, so enter a " + "price change before that window opens." + ) + } class SubscriptionForm(forms.ModelForm): - """Put a club on a tier. The first period opens when the subscription is created.""" + """Put a club on a plan. The first period opens when the subscription is created.""" start = forms.DateField(required=False, widget=forms.DateInput(attrs={"type": "date"}), label=_("First period starts"), help_text=_("Left blank, the period starts today.")) class Meta: model = Subscription - fields = ["tier", "auto_renew", "auto_archive", "notes"] + fields = ["plan", "auto_renew", "auto_archive", "notes"] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - # An inactive tier still bills its existing subscriptions, but must not be picked up - # by a new one — which is the whole point of retiring a tier. - self.fields["tier"].queryset = Tier.objects.filter(is_active=True) + # An inactive plan still bills its existing subscriptions, but must not be picked up + # by a new one — which is the whole point of retiring a plan. Trial plans are excluded + # too: they are reached through the trial form, which converts them properly. + self.fields["plan"].queryset = Plan.objects.filter(is_active=True, is_trial=False) class TrialForm(forms.Form): - """Put a club with no subscription yet on a short trial that switches itself to - ``post_trial_tier`` automatically once it ends -- see billing.services.dues.start_trial.""" + """Put a club with no subscription yet on a trial that switches itself to + ``post_trial_plan`` automatically once it ends -- see billing.services.dues.start_trial. - trial_tier = forms.ModelChoiceField(queryset=Tier.objects.none(), label=_("Trial tier"), help_text=_("What this club is billed on during the trial.")) - post_trial_tier = forms.ModelChoiceField(queryset=Tier.objects.none(), label=_("Then switch to"), help_text=_("The plan it lands on automatically once the trial ends.")) - trial_months = forms.IntegerField(min_value=1, initial=2, label=_("Trial length (months)")) + There is no length field: a trial's length is its plan's own ``duration_months``, so a + 1-month and a 3-month trial are two plans rather than one plan plus a number typed here. + """ + + trial_plan = forms.ModelChoiceField(queryset=Plan.objects.none(), label=_("Trial plan"), help_text=_("What this club is billed on during the trial. Its length is the plan's own duration.")) + post_trial_plan = forms.ModelChoiceField(queryset=Plan.objects.none(), label=_("Then switch to"), help_text=_("The plan it lands on automatically once the trial ends.")) start = forms.DateField(required=False, widget=forms.DateInput(attrs={"type": "date"}), label=_("Trial starts"), help_text=_("Left blank, the trial starts today.")) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - # Same reasoning as SubscriptionForm: a retired tier keeps billing whoever is + # Same reasoning as SubscriptionForm: a retired plan keeps billing whoever is # already on it, but must not be offered for a new trial or a new plan either. - self.fields["trial_tier"].queryset = Tier.objects.filter(is_active=True) - self.fields["post_trial_tier"].queryset = Tier.objects.filter(is_active=True) + self.fields["trial_plan"].queryset = Plan.objects.filter(is_active=True, is_trial=True) + self.fields["post_trial_plan"].queryset = Plan.objects.filter(is_active=True, is_trial=False) class DuePaymentForm(forms.Form): diff --git a/controlpanel/services/statistics.py b/controlpanel/services/statistics.py index 8fc84db..0c43d92 100644 --- a/controlpanel/services/statistics.py +++ b/controlpanel/services/statistics.py @@ -84,7 +84,7 @@ def clubs_with_health(queryset=None, today=None, now=None): team_count=_subquery(Team.objects.all(), Count("pk"), IntegerField()), teams_managed=_subquery(Team.objects.filter(managed_this_season), Count("pk", distinct=True), IntegerField()), admin_count=_subquery(ClubRole.objects.filter(role=ClubRole.Roles.ADMIN), Count("pk"), IntegerField()), - tier_name=Subquery(Subscription.objects.filter(club=OuterRef("pk")).values("tier__name")[:1]), + plan_name=Subquery(Subscription.objects.filter(club=OuterRef("pk")).values("plan__name")[:1]), dues_owed=_subquery(Due.objects.filter(status__in=Due.OWING), Sum(F("amount") - F("amount_paid")), DecimalField(max_digits=10, decimal_places=2)), dues_grace_until=Subquery(Due.objects.filter(club=OuterRef("pk"), status__in=Due.OWING).order_by("grace_until").values("grace_until")[:1]), dues_period_end=Subquery(Due.objects.filter(club=OuterRef("pk"), status__in=Due.OWING).order_by("period_end").values("period_end")[:1]), diff --git a/controlpanel/templates/controlpanel/_club_billing_card.html b/controlpanel/templates/controlpanel/_club_billing_card.html index 5fcbfe2..f230280 100644 --- a/controlpanel/templates/controlpanel/_club_billing_card.html +++ b/controlpanel/templates/controlpanel/_club_billing_card.html @@ -27,17 +27,31 @@ + {% comment %} + Paying up does not un-archive a club on its own -- restoring is a deliberate act, + because a club can also be archived by hand for reasons that have nothing to do + with money. This is the prompt that makes the deliberate act one click away + instead of something you have to remember to go and check. + {% endcomment %} + {% if club.is_archived and dues_settled %} +
+ {% lucide "circle-check" size=16 %} + This club is archived but owes nothing. Reactivating will restore access and open its next period. +
+ {% endif %} + {% if not subscription %} -

This club is not billed for anything. Put it on a tier to start.

+

This club is not billed for anything. Put it on a plan to start.

{% else %}

- On plan {{ subscription.tier.name }}. + On plan {{ subscription.plan.name }}. {% if subscription.trial_ends_at %} {% lucide "hourglass" size=12 %} Trial - On trial until {{ subscription.trial_ends_at|date:"j M Y" }}, then switches to {{ subscription.post_trial_tier.name }}. + On trial until {{ subscription.trial_ends_at|date:"j M Y" }}, then switches to {{ subscription.post_trial_plan.name }}. {% endif %} + {{ subscription.plan.duration_months }}-month periods, archived {{ subscription.plan.grace_days }} days after a period starts if unpaid. {% if subscription.auto_renew %} - Renews automatically 30 days before the period ends. + Renews automatically {{ subscription.plan.renewal_lead_days }} 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 %} @@ -64,7 +78,7 @@ {{ due.period_start|date:"j M Y" }} — {{ due.period_end|date:"j M Y" }} -

{{ due.tier.name }} · {{ due.invoice.number }} · grace to {{ due.grace_until|date:"j M Y" }}
+
{{ due.plan.name }} · {{ due.invoice.number }} · grace to {{ due.grace_until|date:"j M Y" }}
€{{ due.amount|floatformat:2 }} €{{ due.amount_paid|floatformat:2 }} @@ -125,11 +139,11 @@ {% url 'controlpanel:club_subscribe' club.pk as subscribe_url %} -{% include "controlpanel/_modal_form.html" with modal_id="subscription_modal" title=subscription|yesno:"Change plan,Start billing" form=subscription_form action_url=subscribe_url submit_label="Save plan" submit_icon="layers" blurb="Changing tier does not re-bill: the current period keeps the amount it was issued at, and the new rate applies from the next one." %} +{% include "controlpanel/_modal_form.html" with modal_id="subscription_modal" title=subscription|yesno:"Change plan,Start billing" form=subscription_form action_url=subscribe_url submit_label="Save plan" submit_icon="layers" blurb="Changing plan does not re-bill: the current period keeps the amount it was issued at, and the new rate applies from the next one." %} {% if not subscription %} {% url 'controlpanel:club_trial_start' club.pk as trial_start_url %} - {% include "controlpanel/_modal_form.html" with modal_id="trial_modal" title="Start trial" form=trial_form action_url=trial_start_url submit_label="Start trial" submit_icon="hourglass" blurb="The trial period is billed like any other, on the trial tier you pick. It switches to the plan you pick here automatically the next time a period is opened after it ends -- no follow-up needed." %} + {% include "controlpanel/_modal_form.html" with modal_id="trial_modal" title="Start trial" form=trial_form action_url=trial_start_url submit_label="Start trial" submit_icon="hourglass" blurb="The trial period is billed like any other, on the trial plan you pick. It switches to the plan you pick here automatically the next time a period is opened after it ends -- no follow-up needed." %} {% endif %} {% if subscription %} diff --git a/controlpanel/templates/controlpanel/_club_health_table.html b/controlpanel/templates/controlpanel/_club_health_table.html index 3613f14..eada2ec 100644 --- a/controlpanel/templates/controlpanel/_club_health_table.html +++ b/controlpanel/templates/controlpanel/_club_health_table.html @@ -77,8 +77,8 @@ {{ club.upcoming_events }} - {% if club.tier_name %} - {{ club.tier_name|lower }} + {% if club.plan_name %} + {{ club.plan_name|lower }} {% else %} - {% endif %} @@ -87,7 +87,7 @@
{% if not club.dues_owed %} - {% if club.tier_name %} + {% if club.plan_name %} {% comment %} Not owing and on a plan. covered_until is the settled period's end — the day grace would start if nothing renews — shown on its own row under the badge, diff --git a/controlpanel/templates/controlpanel/billing.html b/controlpanel/templates/controlpanel/billing.html index 2487280..248bb59 100644 --- a/controlpanel/templates/controlpanel/billing.html +++ b/controlpanel/templates/controlpanel/billing.html @@ -4,12 +4,12 @@ {% block heading %}Billing{% endblock heading %} {% block actions %} - + {% endblock actions %} {% block panel %} - {% url 'controlpanel:tier_create' as tier_create_url %} - {% include "controlpanel/_modal_form.html" with modal_id="tier_create_modal" title="New plan" form=tier_form action_url=tier_create_url submit_label="Create plan" submit_icon="plus" %} + {% url 'controlpanel:plan_create' as plan_create_url %} + {% include "controlpanel/_modal_form.html" with modal_id="plan_create_modal" title="New plan" form=plan_form action_url=plan_create_url submit_label="Create plan" submit_icon="plus" %}
@@ -25,23 +25,34 @@ Plan + Clocks Clubs Prices - {% for tier in tiers %} + {% for plan in plans %} -
{{ tier.name }}
- {% if not tier.is_active %}Retired{% endif %} - {% if tier.description %} -
{{ tier.description }}
{% endif %} +
{{ plan.name }}
+ {% if plan.is_trial %}Trial{% endif %} + {% if not plan.is_active %}Retired{% endif %} + {% if plan.description %} +
{{ plan.description }}
{% endif %} - {{ tier.club_count }} + {% comment %} + Named for what each measures from, because that is the easy thing to + get wrong: grace runs from the period START, not its end. + {% endcomment %} + +
{{ plan.duration_months }} month{{ plan.duration_months|pluralize }} long
+
billed {{ plan.renewal_lead_days }}d before it starts
+
archived {{ plan.grace_days }}d after it starts
+ + {{ plan.club_count }} - {% for price in tier.prices.all %} + {% for price in plan.prices.all %}
€{{ price.amount|floatformat:2 }} from {{ price.active_from|date:"j M Y" }} @@ -52,13 +63,13 @@ {% endfor %} - - + + {% empty %} - No plans yet. + No plans yet. {% endfor %} @@ -68,12 +79,12 @@
{% comment %} Dialogs live outside the table: may only contain elements. {% endcomment %} - {% for tier in tiers %} - {% url 'controlpanel:tier_price_create' tier.pk as tier_price_url %} - {% include "controlpanel/_modal_form.html" with modal_id=tier.pk|dom_id:"tier_price_modal" title="New price — "|add:tier.name form=tier.price_form action_url=tier_price_url submit_label="Add price" submit_icon="euro" %} + {% for plan in plans %} + {% url 'controlpanel:plan_price_create' plan.pk as plan_price_url %} + {% include "controlpanel/_modal_form.html" with modal_id=plan.pk|dom_id:"plan_price_modal" title="New price — "|add:plan.name form=plan.price_form action_url=plan_price_url submit_label="Add price" submit_icon="euro" %} - {% url 'controlpanel:tier_update' tier.pk as tier_update_url %} - {% include "controlpanel/_modal_form.html" with modal_id=tier.pk|dom_id:"tier_edit_modal" title="Edit "|add:tier.name form=tier.edit_form action_url=tier_update_url submit_label="Save" submit_icon="check" %} + {% url 'controlpanel:plan_update' plan.pk as plan_update_url %} + {% include "controlpanel/_modal_form.html" with modal_id=plan.pk|dom_id:"plan_edit_modal" title="Edit "|add:plan.name form=plan.edit_form action_url=plan_update_url submit_label="Save" submit_icon="check" %} {% endfor %}
@@ -95,7 +106,7 @@ {{ due.club.name }} -
{{ due.tier.name }}
+
{{ due.plan.name }}
{{ due.period_start|date:"j M Y" }} — {{ due.period_end|date:"j M Y" }} diff --git a/controlpanel/tests.py b/controlpanel/tests.py index d8be903..5995cdd 100644 --- a/controlpanel/tests.py +++ b/controlpanel/tests.py @@ -16,7 +16,7 @@ from django.urls import reverse from django.utils import timezone from waffle import get_waffle_flag_model, get_waffle_switch_model -from billing.models import GRACE_DAYS, Due, Tier, TierPrice +from billing.models import DEFAULT_GRACE_DAYS, Due, Plan, PlanPrice from billing.services import BillingError from billing.services.dues import record_payment, subscribe, waive from club.models import Club, ClubMembership, ClubRole, Season @@ -278,7 +278,7 @@ class ClubHomeLocationTests(ControlPanelTestBase): self.assertNotContains(response, 'type="lazyselect"') self.assertContains(response, "Belgium") - self.assertContains(response, '/toggle/", views.SwitchToggleView.as_view(), name="switch_toggle"), # Billing (platform charging the clubs) path("billing/", views.BillingView.as_view(), name="billing"), - path("billing/tiers/new/", views.TierCreateView.as_view(), name="tier_create"), - path("billing/tiers//edit/", views.TierUpdateView.as_view(), name="tier_update"), - path("billing/tiers//prices/new/", views.TierPriceCreateView.as_view(), name="tier_price_create"), + path("billing/plans/new/", views.PlanCreateView.as_view(), name="plan_create"), + path("billing/plans//edit/", views.PlanUpdateView.as_view(), name="plan_update"), + path("billing/plans//prices/new/", views.PlanPriceCreateView.as_view(), name="plan_price_create"), path("billing/dues//pay/", views.RecordPaymentView.as_view(), name="due_pay"), path("billing/dues//waive/", views.WaiveDueView.as_view(), name="due_waive"), path("billing/dues//invoice.pdf", views.InvoicePdfView.as_view(), name="due_invoice"), diff --git a/controlpanel/views.py b/controlpanel/views.py index c810a7e..1342c2e 100644 --- a/controlpanel/views.py +++ b/controlpanel/views.py @@ -10,7 +10,7 @@ from django.utils.formats import date_format from django.views.generic import CreateView, DetailView, FormView, ListView, TemplateView, UpdateView, View from waffle import get_waffle_flag_model, get_waffle_switch_model -from billing.models import Due, Tier, TierPrice +from billing.models import Due, Plan, PlanPrice from billing.services import BillingError from billing.services.dues import next_period_start, open_period, reactivate, record_payment, start_trial, subscribe, waive from billing.services.invoices import invoice_pdf, issue_invoice @@ -18,7 +18,7 @@ from club.models import Club, ClubRole from events.models import Location from features.models import Maintenance -from .forms import ClubAdminForm, ClubForm, DuePaymentForm, FlagForm, HomeLocationForm, MaintenanceForm, OpenPeriodForm, PlatformAdminForm, SubscriptionForm, TierForm, TierPriceForm, TrialForm +from .forms import ClubAdminForm, ClubForm, DuePaymentForm, FlagForm, HomeLocationForm, MaintenanceForm, OpenPeriodForm, PlanForm, PlanPriceForm, PlatformAdminForm, SubscriptionForm, TrialForm from .messages import notify from .mixins import PlatformStaffRequiredMixin, PlatformSuperuserRequiredMixin, RedirectOnInvalidMixin from .services.admins import grant_club_admin, revoke_club_admin @@ -130,7 +130,7 @@ class ClubDetailView(PlatformStaffRequiredMixin, DetailView): # Bound per-row so each due's "Add payment" modal can render its own form without # the template calling DuePaymentForm(initial=...) itself. - dues = list(self.object.dues.select_related("tier", "invoice").prefetch_related("payments")) + dues = list(self.object.dues.select_related("plan", "invoice").prefetch_related("payments")) for due in dues: if due.is_owing: due.payment_form = DuePaymentForm(initial={"amount": due.balance}) @@ -138,6 +138,9 @@ class ClubDetailView(PlatformStaffRequiredMixin, DetailView): home_location = Location.objects.filter(club=self.object, is_home=True).first() return super().get_context_data( + # Drives the "archived but owes nothing -- reactivate?" prompt. Computed from the + # dues already fetched above rather than re-querying. + dues_settled=not any(due.is_owing for due in dues), home_location=home_location, home_location_form=HomeLocationForm(instance=home_location), nav="clubs", @@ -388,7 +391,7 @@ class PlatformAdminRevokeView(PlatformSuperuserRequiredMixin, View): class BillingView(PlatformStaffRequiredMixin, TemplateView): - """Tiers and their prices, plus every period we are owed money for.""" + """Plans and their prices, plus every period we are owed money for.""" template_name = "controlpanel/billing.html" @@ -396,76 +399,76 @@ class BillingView(PlatformStaffRequiredMixin, TemplateView): today = timezone.localdate() # Bound per-row so each "Edit" / "New price" modal can render its own form: the - # template can't call TierForm(instance=tier) itself, so the form rides along on + # template can't call PlanForm(instance=plan) itself, so the form rides along on # the object it belongs to. - tiers = list(Tier.objects.prefetch_related("prices").annotate(club_count=Count("subscriptions"))) - for tier in tiers: - tier.edit_form = TierForm(instance=tier) - tier.price_form = TierPriceForm() + plans = list(Plan.objects.prefetch_related("prices").annotate(club_count=Count("subscriptions"))) + for plan in plans: + plan.edit_form = PlanForm(instance=plan) + plan.price_form = PlanPriceForm() - owing = list(Due.objects.filter(status__in=Due.OWING).select_related("club", "tier").order_by("grace_until")) + owing = list(Due.objects.filter(status__in=Due.OWING).select_related("club", "plan").order_by("grace_until")) for due in owing: due.payment_form = DuePaymentForm(initial={"amount": due.balance}) return super().get_context_data( nav="billing", - tiers=tiers, - tier_form=TierForm(), + plans=plans, + plan_form=PlanForm(), owing=owing, today=today, **kwargs, ) -class TierCreateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, CreateView): +class PlanCreateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, CreateView): """Reachable only via the "New plan" modal on the billing page — POST-only, and there is no standalone template to render on GET or on a rejected submission.""" - model = Tier - form_class = TierForm + model = Plan + form_class = PlanForm http_method_names = ["post"] invalid_redirect_url_name = "controlpanel:billing" def get_success_url(self): - notify(self.request, f"s|Plan created|Tier “{self.object}” created. Give it a price before billing anyone.") + notify(self.request, f"s|Plan created|Plan “{self.object}” created. Give it a price before billing anyone.") return reverse("controlpanel:billing") -class TierUpdateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, UpdateView): - """Reachable only via a tier's "Edit" modal on the billing page — POST-only, and there +class PlanUpdateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, UpdateView): + """Reachable only via a plan's "Edit" modal on the billing page — POST-only, and there is no standalone template to render on GET or on a rejected submission.""" - model = Tier - form_class = TierForm + model = Plan + form_class = PlanForm http_method_names = ["post"] invalid_redirect_url_name = "controlpanel:billing" def get_success_url(self): - notify(self.request, f"s|Plan updated|Tier “{self.object}” updated.") + notify(self.request, f"s|Plan updated|Plan “{self.object}” updated.") return reverse("controlpanel:billing") -class TierPriceCreateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, CreateView): +class PlanPriceCreateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, CreateView): """A rate change is a new dated price, never an edit of the old one — periods already billed keep the amount they were billed at. - Reachable only via a tier's "New price" modal on the billing page — POST-only, and + Reachable only via a plan's "New price" modal on the billing page — POST-only, and there is no standalone template to render on GET or on a rejected submission. """ - model = TierPrice - form_class = TierPriceForm + model = PlanPrice + form_class = PlanPriceForm http_method_names = ["post"] invalid_redirect_url_name = "controlpanel:billing" @property - def tier(self): - return get_object_or_404(Tier, pk=self.kwargs["pk"]) + def plan(self): + return get_object_or_404(Plan, pk=self.kwargs["pk"]) def form_valid(self, form): - form.instance.tier = self.tier + form.instance.plan = self.plan response = super().form_valid(form) - notify(self.request, f"s|Price added|{self.tier} is €{self.object.amount} for periods opening from {self.object.active_from}.") + notify(self.request, f"s|Price added|{self.plan} is €{self.object.amount} for periods opening from {self.object.active_from}.") return response def get_success_url(self): @@ -473,7 +476,7 @@ class TierPriceCreateView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, Cr class SubscribeClubView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, FormView): - """Put a club on a tier, which opens its first period. + """Put a club on a plan, which opens its first period. Reachable only via the "Change plan" modal on the club detail page — POST-only, and there is no standalone template to render on GET or on a rejected submission. @@ -502,22 +505,22 @@ class SubscribeClubView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, Form existing = getattr(club, "subscription", None) with suppress_billing_errors(self.request, title="Couldn't change plan"): if existing: - # Changing tier does not re-bill: the current period keeps the amount it was + # Changing plan does not re-bill: the current period keeps the amount it was # issued at, and the new rate applies from the next one. was_on_trial = existing.trial_ends_at is not None subscription = form.save(commit=False) subscription.club = club if was_on_trial: - # A manual tier change while on a trial is a deliberate override -- - # left in place, the trial fields would silently swap the tier again + # A manual plan change while on a trial is a deliberate override -- + # left in place, the trial fields would silently swap the plan again # later, onto a plan the admin didn't just choose. subscription.trial_ends_at = None - subscription.post_trial_tier = None + subscription.post_trial_plan = None subscription.save() - notify(self.request, f"s|Plan changed|{club} is now on {subscription.tier}. The current period keeps the amount it was billed at.") + notify(self.request, f"s|Plan changed|{club} is now on {subscription.plan}. 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"], auto_renew=form.cleaned_data["auto_renew"]) - notify(self.request, f"s|Billing started|{club} is on {form.cleaned_data['tier']}. Its first period is open.") + subscribe(club, form.cleaned_data["plan"], start=form.cleaned_data.get("start"), auto_archive=form.cleaned_data["auto_archive"], auto_renew=form.cleaned_data["auto_renew"]) + notify(self.request, f"s|Billing started|{club} is on {form.cleaned_data['plan']}. Its first period is open.") return redirect("controlpanel:club_detail", pk=club.pk) @@ -541,14 +544,14 @@ class ClubStartTrialView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, For def form_valid(self, form): club = self.club with suppress_billing_errors(self.request, title="Couldn't start trial"): + trial_plan = form.cleaned_data["trial_plan"] start_trial( club, - form.cleaned_data["trial_tier"], - post_trial_tier=form.cleaned_data["post_trial_tier"], - trial_months=form.cleaned_data["trial_months"], + trial_plan, + post_trial_plan=form.cleaned_data["post_trial_plan"], start=form.cleaned_data.get("start"), ) - notify(self.request, f"s|Trial started|{club} is on a {form.cleaned_data['trial_months']}-month trial of {form.cleaned_data['trial_tier']}, then switches to {form.cleaned_data['post_trial_tier']}.") + notify(self.request, f"s|Trial started|{club} is on a {trial_plan.duration_months}-month trial of {trial_plan}, then switches to {form.cleaned_data['post_trial_plan']}.") return redirect("controlpanel:club_detail", pk=club.pk) @@ -564,7 +567,7 @@ class RecordPaymentView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, Form @property def due(self): - return get_object_or_404(Due.objects.select_related("club", "tier"), pk=self.kwargs["pk"]) + return get_object_or_404(Due.objects.select_related("club", "plan"), pk=self.kwargs["pk"]) def get_invalid_redirect_kwargs(self): return {"pk": self.due.club_id} @@ -627,7 +630,7 @@ class OpenPeriodView(PlatformStaffRequiredMixin, RedirectOnInvalidMixin, FormVie class InvoicePdfView(PlatformStaffRequiredMixin, View): def get(self, request, pk): - due = get_object_or_404(Due.objects.select_related("club", "tier", "invoice"), pk=pk) + due = get_object_or_404(Due.objects.select_related("club", "plan", "invoice"), pk=pk) invoice = issue_invoice(due) try: pdf = invoice_pdf(invoice) diff --git a/formbuilder/tests.py b/formbuilder/tests.py index 0305e3c..3a9adc6 100644 --- a/formbuilder/tests.py +++ b/formbuilder/tests.py @@ -127,7 +127,7 @@ class FieldChoicesTests(FormbuilderTestBase): self.assertEqual(field_choices(self.size), [("S", "S"), ("M", "M"), ("L", "L")]) def test_dict_options(self): - field = Field.objects.create(form=self.form, key="tier", label="Tier", field_type=Field.FieldType.CHOICE, order=3, options=[{"value": "a", "label": "Gold"}, {"value": "b", "label": "Silver"}]) + field = Field.objects.create(form=self.form, key="plan", label="Plan", field_type=Field.FieldType.CHOICE, order=3, options=[{"value": "a", "label": "Gold"}, {"value": "b", "label": "Silver"}]) self.assertEqual(field_choices(field), [("a", "Gold"), ("b", "Silver")]) diff --git a/management/context_processors.py b/management/context_processors.py index 9275e30..7201b70 100644 --- a/management/context_processors.py +++ b/management/context_processors.py @@ -9,6 +9,7 @@ action rather than the whole section (``NewsAuthorRequiredMixin``/``can_add_news from waffle import flag_is_active +from billing.services.notices import club_billing_notice from club.services.access import can_add_news, has_management_access, is_club_admin, is_coach_manager #: Every management URL name, mapped to the nav item it should light up -- @@ -123,6 +124,23 @@ def is_admin(request): return {"is_club_admin": is_club_admin(request.user, club)} +def billing_notice(request): + """What this club owes the platform, for the club's own admins. + + A context processor rather than view context because the notice has to be able to follow + an admin onto every management page once it turns urgent -- billing/base.html renders it + at error level only, and the home page renders it at every level. + + Admins only: platform billing is none of an ordinary member's business, and the query is + skipped entirely for everyone else rather than fetched and hidden in the template. + """ + club = getattr(request, "club", None) + if club is None or not request.user.is_authenticated or not is_club_admin(request.user, club): + return {"billing_notice": None} + + return {"billing_notice": club_billing_notice(club)} + + def management_position(request): """Whether the signed-in user holds a management position (or is ADMIN) -- gates the nav's Locations/Opponents links, which ``ManagementPositionRequiredMixin`` diff --git a/management/templates/management/_billing_notice.html b/management/templates/management/_billing_notice.html new file mode 100644 index 0000000..3107eec --- /dev/null +++ b/management/templates/management/_billing_notice.html @@ -0,0 +1,35 @@ +{% comment %} + What this club owes the platform, for its own admins. + + `billing_notice` comes from management.context_processors.billing_notice, which returns + None for anyone who is not a club admin -- so this partial never needs to check that + itself. Included unconditionally by home.html, and by base.html only when the notice has + reached error level, so a final notice follows an admin onto every management page while + an early one stays on the dashboard. +{% endcomment %} +{% load i18n lucide %} + +{% if billing_notice %} +
+ {% if billing_notice.level == 'error' %} + {% lucide "octagon-alert" size=20 %} + {% elif billing_notice.level == 'warning' %} + {% lucide "triangle-alert" size=20 %} + {% else %} + {% lucide "receipt-euro" size=20 %} + {% endif %} + + {% blocktrans with amount=billing_notice.amount_outstanding %}Platform fees of €{{ amount }} are outstanding.{% endblocktrans %} + + {% if billing_notice.will_archive %} + {% if billing_notice.days_until_archive < 0 %} + {% trans "This club is now due to be archived. Pay to keep access." %} + {% else %} + {% blocktrans count days=billing_notice.days_until_archive %}This club will be archived in {{ days }} day unless payment is received.{% plural %}This club will be archived in {{ days }} days unless payment is received.{% endblocktrans %} + {% endif %} + {% else %} + {% trans "Please settle it to keep your account in good standing." %} + {% endif %} + +
+{% endif %} diff --git a/management/templates/management/base.html b/management/templates/management/base.html index 9560dd1..caf8474 100644 --- a/management/templates/management/base.html +++ b/management/templates/management/base.html @@ -38,5 +38,16 @@ {% include "management/_nav_items.html" %} + {% comment %} + A final billing notice follows the admin onto every management page -- but only at + error level (inside 7 days of archiving, or already past it). Shown from the moment + anything is owed it would sit on every screen for weeks and train people to ignore + the one week it matters. home.html includes the same partial at every level, hence + the guard here rather than inside the partial. + {% endcomment %} + {% if billing_notice.is_urgent and nav != "home" %} + {% include "management/_billing_notice.html" %} + {% endif %} + {% block panel %}{% endblock panel %} {% endblock main %} diff --git a/management/templates/management/home.html b/management/templates/management/home.html index ca214ce..33273ca 100644 --- a/management/templates/management/home.html +++ b/management/templates/management/home.html @@ -5,6 +5,9 @@ {% block subheading %}{% trans "Management" %}{% endblock subheading %} {% block panel %} + {# Every level here; base.html repeats it on other pages only once it turns urgent. #} + {% include "management/_billing_notice.html" %} + {% if attention.no_season %}
{% lucide "calendar-x" size=20 %} diff --git a/management/tests.py b/management/tests.py index d61d7c1..d5dc34b 100644 --- a/management/tests.py +++ b/management/tests.py @@ -15,8 +15,8 @@ from django.urls import NoReverseMatch, reverse from django.utils import timezone from waffle import get_waffle_flag_model -from billing.models import Tier, TierPrice -from billing.services.dues import subscribe +from billing.models import Plan, PlanPrice +from billing.services.dues import record_payment, subscribe from club.models import Club, ClubMembership, ClubRole, FeePayment, Season, Sponsor from events.models import Attendance, Competition, Event, EventSeries, Location, Opponent from events.services.rbihf_import import RBIHFImportError @@ -1407,9 +1407,7 @@ class FamilyMembershipRoleUpdateTests(ManagementTestBase): self.assertRedirects(response, next_url) def test_ignores_an_unsafe_next_url(self): - response = self.club_post( - "family_membership_role_update", {"role": FamilyMembership.FamilyRole.GUARDIAN, "next": "https://evil.example.com/steal"}, self.family.pk, self.member.pk - ) + response = self.club_post("family_membership_role_update", {"role": FamilyMembership.FamilyRole.GUARDIAN, "next": "https://evil.example.com/steal"}, self.family.pk, self.member.pk) self.assertRedirects(response, reverse("management:family_detail", args=[self.family.pk])) @@ -1682,9 +1680,7 @@ class MembershipRecordPaymentTests(ManagementTestBase): super().setUp() self.client.force_login(self.admin_user) self.member = Member.objects.create(first_name="Owed", last_name="Fee") - self.membership = ClubMembership.objects.create( - club=self.club, member=self.member, season=self.season, status=ClubMembership.StatusChoices.PENDING, fee_status=ClubMembership.FeeStatus.UNPAID, fee_amount=Decimal("150.00") - ) + self.membership = ClubMembership.objects.create(club=self.club, member=self.member, season=self.season, status=ClubMembership.StatusChoices.PENDING, fee_status=ClubMembership.FeeStatus.UNPAID, fee_amount=Decimal("150.00")) def test_recording_a_partial_payment(self): response = self.club_post("membership_record_payment", {"amount": "50.00", "method": FeePayment.Method.CASH, "reference": "R1"}, self.membership.pk) @@ -1731,9 +1727,7 @@ class MembershipMarkFullyPaidTests(ManagementTestBase): super().setUp() self.client.force_login(self.admin_user) self.member = Member.objects.create(first_name="Owed", last_name="Fee") - self.membership = ClubMembership.objects.create( - club=self.club, member=self.member, season=self.season, status=ClubMembership.StatusChoices.PENDING, fee_status=ClubMembership.FeeStatus.UNPAID, fee_amount=Decimal("150.00") - ) + self.membership = ClubMembership.objects.create(club=self.club, member=self.member, season=self.season, status=ClubMembership.StatusChoices.PENDING, fee_status=ClubMembership.FeeStatus.UNPAID, fee_amount=Decimal("150.00")) def test_settles_the_remaining_balance_in_one_click(self): response = self.club_post("membership_mark_fully_paid", {}, self.membership.pk) @@ -2563,7 +2557,7 @@ class LocationOpponentManagementTests(ManagementTestBase): self.assertNotContains(response, 'type="lazyselect"') self.assertContains(response, "Belgium") - self.assertContains(response, '