From 8598fd2b467bdb57e2de303c5e054022674b0e52 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Mon, 3 Aug 2026 17:03:03 +0200 Subject: [PATCH] Generate seasons per club instead of a hardcoded Aug-May window Club now carries its own season_start and season_duration_months, editable via controlpanel; generate_seasons chains each new season off the day after the club's last one ends (or its configured start, for a club with none yet) instead of assuming every club runs Aug 1 - May 31. Adds --resync to the generate_seasons command to clean up seasons left over from the old hardcoded rule -- removing any that don't match a club's current settings and aren't still referenced by real data. --- club/management/__init__.py | 0 club/management/commands/__init__.py | 0 club/management/commands/generate_seasons.py | 57 +++++ ...eason_duration_months_club_season_start.py | 25 ++ club/models.py | 14 +- club/services/seasons.py | 104 ++++++++ club/tests.py | 241 ++++++++++++++++++ controlpanel/forms.py | 3 +- controlpanel/tests.py | 7 +- 9 files changed, 447 insertions(+), 4 deletions(-) create mode 100644 club/management/__init__.py create mode 100644 club/management/commands/__init__.py create mode 100644 club/management/commands/generate_seasons.py create mode 100644 club/migrations/0017_club_season_duration_months_club_season_start.py create mode 100644 club/services/seasons.py diff --git a/club/management/__init__.py b/club/management/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/club/management/commands/__init__.py b/club/management/commands/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/club/management/commands/generate_seasons.py b/club/management/commands/generate_seasons.py new file mode 100644 index 0000000..0f679db --- /dev/null +++ b/club/management/commands/generate_seasons.py @@ -0,0 +1,57 @@ +"""Generate seasons ahead of time for every active club. + +Meant to run on a schedule (cron): safe to call repeatedly, since generate_seasons +skips whatever already exists. No --dry-run/--commit gate on generation itself -- +unlike archiving a club or billing it, creating a future season row is additive +and idempotent, same reasoning as extend_event_series (materialising occurrences). + +--resync is different: it can delete rows (any season that doesn't match a +club's *current* season_start/season_duration_months, and isn't referenced by a +membership), so it defaults to reporting only -- pass --commit alongside it to +actually remove anything. +""" + +from dateutil.relativedelta import relativedelta +from django.utils import timezone + +from club.models import Club +from club.services.seasons import generate_seasons, resync_seasons +from features.commands import MaintenanceAwareCommand + + +class Command(MaintenanceAwareCommand): + help = "Generate seasons up to N years ahead for every active club (default 2)." + + def add_arguments(self, parser): + parser.add_argument("--years", type=int, default=2, help="How many years ahead to generate seasons for (default 2).") + parser.add_argument("--resync", action="store_true", help="Before generating, remove any existing seasons that don't match the club's current settings (skips any still referenced by a membership).") + parser.add_argument("--commit", action="store_true", help="With --resync, actually delete the seasons found to be wrong. Without it, --resync only reports what it would remove.") + + def handle(self, *args, **options): + until = timezone.localdate() + relativedelta(years=options["years"]) + clubs = Club.objects.active() + + if options["resync"]: + total_removed, total_kept = 0, 0 + for club in clubs: + removed, kept = resync_seasons(club, until, commit=options["commit"]) + total_removed += len(removed) + total_kept += len(kept) + if removed: + verb = "Removed" if options["commit"] else "Would remove" + self.stdout.write(f"{club}: {verb} {len(removed)} season(s) that no longer match its settings.") + if kept: + self.stdout.write(self.style.WARNING(f"{club}: {len(kept)} season(s) don't match its settings but are still in use, left alone.")) + + if not options["commit"] and total_removed: + self.stdout.write(self.style.WARNING("Dry run -- pass --commit to actually delete these.")) + self.stdout.write(self.style.SUCCESS(f"Resync: {total_removed} season(s) {'removed' if options['commit'] else 'to remove'}, {total_kept} kept (in use).")) + + total = 0 + for club in clubs: + created = generate_seasons(club, until) + total += len(created) + if created: + self.stdout.write(f"{club}: generated {len(created)} season(s).") + + self.stdout.write(self.style.SUCCESS(f"Done. Generated {total} season(s) across {clubs.count()} club(s).")) diff --git a/club/migrations/0017_club_season_duration_months_club_season_start.py b/club/migrations/0017_club_season_duration_months_club_season_start.py new file mode 100644 index 0000000..3f17ec9 --- /dev/null +++ b/club/migrations/0017_club_season_duration_months_club_season_start.py @@ -0,0 +1,25 @@ +# Generated by Django 6.0.6 on 2026-08-03 14:30 + +import datetime +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('club', '0016_clubmembership_amount_paid_clubmembership_fee_amount_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='club', + name='season_duration_months', + field=models.PositiveSmallIntegerField(default=12, help_text='How many months a season lasts, counted from its start date.', validators=[django.core.validators.MinValueValidator(1), django.core.validators.MaxValueValidator(24)], verbose_name='season duration (months)'), + ), + migrations.AddField( + model_name='club', + name='season_start', + field=models.DateField(default=datetime.date(2000, 8, 1), help_text='Which day of the year a season begins — only the month and day are used, the year is ignored.', verbose_name='season start'), + ), + ] diff --git a/club/models.py b/club/models.py index f401560..2022ff2 100644 --- a/club/models.py +++ b/club/models.py @@ -2,7 +2,7 @@ import datetime from decimal import Decimal from django.conf import settings -from django.core.validators import FileExtensionValidator, MinValueValidator, RegexValidator +from django.core.validators import FileExtensionValidator, MaxValueValidator, MinValueValidator, RegexValidator from django.db import models from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -60,6 +60,18 @@ class Club(UUIDModel): archived_at = models.DateTimeField(_("archived at"), null=True, blank=True, help_text=_("Archived clubs stop resolving on their subdomain, but their data is retained.")) + season_start = models.DateField( + _("season start"), + default=datetime.date(2000, 8, 1), + help_text=_("Which day of the year a season begins — only the month and day are used, the year is ignored."), + ) + season_duration_months = models.PositiveSmallIntegerField( + _("season duration (months)"), + default=12, + validators=[MinValueValidator(1), MaxValueValidator(24)], + help_text=_("How many months a season lasts, counted from its start date."), + ) + objects = ClubManager() class Meta: diff --git a/club/services/seasons.py b/club/services/seasons.py new file mode 100644 index 0000000..fb31fa1 --- /dev/null +++ b/club/services/seasons.py @@ -0,0 +1,104 @@ +"""Generating a club's seasons ahead of time. + +Season (club/models.py) has no stored notion of "when a season starts" -- that +lives on Club instead (season_start, season_duration_months), since different +clubs run their year on different cycles. Same shape as +events/services/recurrence.py's generate_occurrences: materialise missing rows +up to a horizon, get_or_create per row, safe to call repeatedly. +""" + +import datetime + +from dateutil.relativedelta import relativedelta +from django.db.models import ProtectedError +from django.db.models.deletion import Collector +from django.utils import timezone + +from club.models import Season + + +def _initial_season_start(club, today): + """The most recent occurrence of the club's configured season_start that is + not later than ``today`` -- so a club with no seasons yet gets one covering + "now" (or the most recently completed one), not an arbitrary future year.""" + anchor = club.season_start + start = datetime.date(today.year, anchor.month, anchor.day) + if start > today: + start = datetime.date(today.year - 1, anchor.month, anchor.day) + return start + + +def _season_end(start, club): + return start + relativedelta(months=club.season_duration_months) - datetime.timedelta(days=1) + + +def generate_seasons(club, until): + """Materialise seasons for ``club`` from wherever it last left off -- the day + after its latest season's end_date, or its configured season_start if it has + none yet -- through ``until``. get_or_create per row (matches the + unique_season_dates_per_club constraint exactly), safe to call repeatedly. + """ + latest = Season.objects.filter(club=club).order_by("-end_date").first() + start = latest.end_date + datetime.timedelta(days=1) if latest else _initial_season_start(club, timezone.localdate()) + + created = [] + while start <= until: + end = _season_end(start, club) + season, was_created = Season.objects.get_or_create(club=club, start_date=start, end_date=end) + if was_created: + created.append(season) + start = end + datetime.timedelta(days=1) + + return created + + +def _expected_season_dates(club, until): + """The (start_date, end_date) pairs generate_seasons would produce for + ``club`` from scratch, ignoring whatever already exists -- used by + resync_seasons to tell "matches the club's current settings" from "doesn't".""" + start = _initial_season_start(club, timezone.localdate()) + expected = set() + while start <= until: + end = _season_end(start, club) + expected.add((start, end)) + start = end + datetime.timedelta(days=1) + return expected + + +def _is_referenced(season): + """Whether deleting ``season`` would hit a PROTECT on any of its relations + (ClubMembership, StaffAssignment, TeamMembership, Event, ...) without + actually deleting anything.""" + collector = Collector(using=season._state.db) + try: + collector.collect([season]) + except ProtectedError: + return True + return False + + +def resync_seasons(club, until, *, commit=False): + """Find seasons for ``club`` that don't match what its *current* + season_start/season_duration_months would produce (e.g. left over from a + different rule, or from before those settings were changed), within the + same horizon generate_seasons would cover. + + A season is only ever removed if nothing references it through a PROTECTed + relation -- a season already in use is reported as kept, never silently + dropped. With commit=False (the default) nothing is deleted; the caller + gets back what *would* happen. + """ + expected = _expected_season_dates(club, until) + + removed, kept = [], [] + for season in Season.objects.filter(club=club): + if (season.start_date, season.end_date) in expected: + continue + if _is_referenced(season): + kept.append(season) + else: + removed.append(season) + if commit: + season.delete() + + return removed, kept diff --git a/club/tests.py b/club/tests.py index 030aa1d..2ea83d2 100644 --- a/club/tests.py +++ b/club/tests.py @@ -2,11 +2,14 @@ import datetime import uuid from contextlib import contextmanager from decimal import Decimal +from io import StringIO from allauth.mfa.models import Authenticator +from dateutil.relativedelta import relativedelta from django.contrib import admin as django_admin from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError +from django.core.management import call_command from django.db import IntegrityError from django.db.models import ProtectedError from django.test import RequestFactory, TestCase, override_settings @@ -29,6 +32,7 @@ from .services.access import ( teams_staffed_by, ) from .services.fees import mark_as_paid, record_payment, remaining_balance +from .services.seasons import _initial_season_start, _season_end, generate_seasons, resync_seasons from .tenancy import ( ClubTenantMiddleware, get_current_club, @@ -1190,3 +1194,240 @@ class FeeServiceTests(TestCase): payment = record_payment(self.membership, amount=Decimal("50.00"), recorded_by=user) self.assertEqual(payment.recorded_by, user) + + +class SeasonStartEndTests(TestCase): + """club.services.seasons._initial_season_start / _season_end -- the + per-club rules generate_seasons chains off, now that a club's own + season_start/season_duration_months drive them instead of a fixed Aug-May + window.""" + + def setUp(self): + self.club = Club.objects.create(name="Ajax United", slug="ajax-united") + + def test_a_date_past_this_years_anchor_uses_this_year(self): + self.club.season_start = datetime.date(2000, 8, 1) + + start = _initial_season_start(self.club, datetime.date(2026, 8, 15)) + + self.assertEqual(start, datetime.date(2026, 8, 1)) + + def test_a_date_before_this_years_anchor_uses_last_year(self): + self.club.season_start = datetime.date(2000, 8, 1) + + start = _initial_season_start(self.club, datetime.date(2027, 2, 1)) + + self.assertEqual(start, datetime.date(2026, 8, 1)) + + def test_the_anchor_date_itself_uses_this_year(self): + self.club.season_start = datetime.date(2000, 8, 1) + + start = _initial_season_start(self.club, datetime.date(2026, 8, 1)) + + self.assertEqual(start, datetime.date(2026, 8, 1)) + + def test_season_end_is_the_day_before_the_start_plus_the_duration(self): + self.club.season_duration_months = 12 + + end = _season_end(datetime.date(2026, 8, 1), self.club) + + self.assertEqual(end, datetime.date(2027, 7, 31)) + + def test_a_shorter_duration_produces_a_shorter_season(self): + self.club.season_duration_months = 6 + + end = _season_end(datetime.date(2026, 8, 1), self.club) + + self.assertEqual(end, datetime.date(2027, 1, 31)) + + +class GenerateSeasonsTests(TestCase): + """club.services.seasons.generate_seasons -- the service behind the + generate_seasons management command.""" + + def setUp(self): + self.club = Club.objects.create(name="Ajax United", slug="ajax-united") + + def test_generates_a_season_covering_today(self): + today = timezone.localdate() + + generate_seasons(self.club, today) + + self.assertTrue(Season.objects.filter(club=self.club, start_date__lte=today, end_date__gte=today).exists()) + + def test_generates_every_window_through_the_horizon(self): + today = timezone.localdate() + until = today + relativedelta(years=2) + + created = generate_seasons(self.club, until) + + self.assertGreaterEqual(len(created), 2) + for season in created: + self.assertLessEqual(season.start_date, until) + + def test_is_idempotent_on_a_second_run(self): + until = timezone.localdate() + relativedelta(years=2) + generate_seasons(self.club, until) + count_after_first = Season.objects.filter(club=self.club).count() + + second_run = generate_seasons(self.club, until) + + self.assertEqual(second_run, []) + self.assertEqual(Season.objects.filter(club=self.club).count(), count_after_first) + + def test_a_new_season_starts_the_day_after_the_last_one_ends(self): + today = timezone.localdate() + generate_seasons(self.club, today) + latest = Season.objects.filter(club=self.club).order_by("-end_date").first() + + generate_seasons(self.club, latest.end_date + relativedelta(months=self.club.season_duration_months)) + + next_season = Season.objects.filter(club=self.club, start_date=latest.end_date + datetime.timedelta(days=1)).first() + self.assertIsNotNone(next_season) + + def test_changing_the_duration_only_affects_the_next_generated_season(self): + today = timezone.localdate() + generate_seasons(self.club, today) + first = Season.objects.filter(club=self.club).order_by("-end_date").first() + + self.club.season_duration_months = 6 + self.club.save() + generate_seasons(self.club, first.end_date + relativedelta(months=6)) + + first_end_before = first.end_date + first.refresh_from_db() + self.assertEqual(first.end_date, first_end_before) # existing season untouched + second = Season.objects.get(club=self.club, start_date=first.end_date + datetime.timedelta(days=1)) + self.assertEqual(second.end_date, second.start_date + relativedelta(months=6) - datetime.timedelta(days=1)) + + def test_does_not_disturb_a_pre_existing_irregular_season(self): + odd = Season.objects.create(club=self.club, start_date=datetime.date(2020, 3, 1), end_date=datetime.date(2020, 9, 1)) + + generate_seasons(self.club, timezone.localdate()) + + odd.refresh_from_db() + self.assertEqual(odd.start_date, datetime.date(2020, 3, 1)) + self.assertEqual(odd.end_date, datetime.date(2020, 9, 1)) + + +class ResyncSeasonsTests(TestCase): + """club.services.seasons.resync_seasons -- cleaning up seasons that don't + match a club's current settings (e.g. left over from a since-changed + season_start/season_duration_months).""" + + def setUp(self): + self.club = Club.objects.create(name="Ajax United", slug="ajax-united") + self.until = timezone.localdate() + relativedelta(years=2) + + def test_a_wrong_and_unreferenced_season_is_reported_as_removable(self): + wrong = Season.objects.create(club=self.club, start_date=datetime.date(2020, 3, 1), end_date=datetime.date(2020, 9, 1)) + + removed, kept = resync_seasons(self.club, self.until) + + self.assertIn(wrong, removed) + self.assertEqual(kept, []) + + def test_commit_actually_deletes_a_wrong_unreferenced_season(self): + wrong = Season.objects.create(club=self.club, start_date=datetime.date(2020, 3, 1), end_date=datetime.date(2020, 9, 1)) + + resync_seasons(self.club, self.until, commit=True) + + self.assertFalse(Season.objects.filter(pk=wrong.pk).exists()) + + def test_without_commit_nothing_is_actually_deleted(self): + wrong = Season.objects.create(club=self.club, start_date=datetime.date(2020, 3, 1), end_date=datetime.date(2020, 9, 1)) + + resync_seasons(self.club, self.until, commit=False) + + self.assertTrue(Season.objects.filter(pk=wrong.pk).exists()) + + def test_a_wrong_but_referenced_season_is_kept_not_removed(self): + wrong = Season.objects.create(club=self.club, start_date=datetime.date(2020, 3, 1), end_date=datetime.date(2020, 9, 1)) + member = Member.objects.create(first_name="Jane", last_name="Doe") + ClubMembership.objects.create(club=self.club, member=member, season=wrong) + + removed, kept = resync_seasons(self.club, self.until, commit=True) + + self.assertEqual(removed, []) + self.assertIn(wrong, kept) + self.assertTrue(Season.objects.filter(pk=wrong.pk).exists()) + + def test_a_season_matching_current_settings_is_left_alone(self): + generate_seasons(self.club, timezone.localdate()) + + removed, kept = resync_seasons(self.club, self.until) + + self.assertEqual(removed, []) + self.assertEqual(kept, []) + + def test_a_season_referenced_only_via_staff_assignment_is_kept_not_removed(self): + # Season is PROTECTed by more than just ClubMembership -- a season kept + # alive only through a StaffAssignment must not be silently deleted either. + wrong = Season.objects.create(club=self.club, start_date=datetime.date(2020, 3, 1), end_date=datetime.date(2020, 9, 1)) + team = Team.objects.create(club=self.club, name="First Team", short_name="1st") + position = Position.objects.create(club=self.club, name="Head Coach", short_name="HC", staff_position=True) + member = Member.objects.create(first_name="Jane", last_name="Doe") + StaffAssignment.objects.create(team=team, member=member, season=wrong, position=position) + + removed, kept = resync_seasons(self.club, self.until, commit=True) + + self.assertEqual(removed, []) + self.assertIn(wrong, kept) + self.assertTrue(Season.objects.filter(pk=wrong.pk).exists()) + + +class GenerateSeasonsCommandTests(TestCase): + def setUp(self): + self.club = Club.objects.create(name="Ajax United", slug="ajax-united") + + def test_default_years_is_two(self): + call_command("generate_seasons", stdout=StringIO()) + + today = timezone.localdate() + until = today + relativedelta(years=2) + seasons = Season.objects.filter(club=self.club).order_by("start_date") + self.assertTrue(seasons.exists()) + for season in seasons: + self.assertLessEqual(season.start_date, until) + self.assertTrue(seasons.filter(start_date__lte=today, end_date__gte=today).exists()) + + def test_years_argument_controls_the_horizon(self): + call_command("generate_seasons", "--years", "1", stdout=StringIO()) + + until = timezone.localdate() + relativedelta(years=1) + for season in Season.objects.filter(club=self.club): + self.assertLessEqual(season.start_date, until) + + def test_archived_clubs_are_skipped(self): + self.club.archive() + + call_command("generate_seasons", stdout=StringIO()) + + self.assertFalse(Season.objects.filter(club=self.club).exists()) + + def test_second_run_creates_nothing_new(self): + call_command("generate_seasons", stdout=StringIO()) + count_after_first = Season.objects.filter(club=self.club).count() + + out = StringIO() + call_command("generate_seasons", stdout=out) + + self.assertEqual(Season.objects.filter(club=self.club).count(), count_after_first) + self.assertIn("Generated 0 season", out.getvalue()) + + def test_resync_without_commit_reports_but_does_not_delete(self): + wrong = Season.objects.create(club=self.club, start_date=datetime.date(2020, 3, 1), end_date=datetime.date(2020, 9, 1)) + + out = StringIO() + call_command("generate_seasons", "--resync", stdout=out) + + self.assertTrue(Season.objects.filter(pk=wrong.pk).exists()) + self.assertIn("Would remove", out.getvalue()) + + def test_resync_with_commit_deletes_the_wrong_season(self): + wrong = Season.objects.create(club=self.club, start_date=datetime.date(2020, 3, 1), end_date=datetime.date(2020, 9, 1)) + + call_command("generate_seasons", "--resync", "--commit", stdout=StringIO()) + + self.assertFalse(Season.objects.filter(pk=wrong.pk).exists()) + diff --git a/controlpanel/forms.py b/controlpanel/forms.py index 8e188f8..7458513 100644 --- a/controlpanel/forms.py +++ b/controlpanel/forms.py @@ -13,7 +13,7 @@ from .services.admins import find_member_by_email class ClubForm(forms.ModelForm): class Meta: model = Club - fields = ["name", "slug", "logo", "primary_color", "secondary_color"] + fields = ["name", "slug", "logo", "primary_color", "secondary_color", "season_start", "season_duration_months"] help_texts = {"slug": _("Drives the club's subdomain. Left blank, it is derived from the name.")} # Deliberately a text input, not : a colour picker cannot # express "no colour" -- it would submit #000000 for every club that never @@ -22,6 +22,7 @@ class ClubForm(forms.ModelForm): "primary_color": forms.TextInput(attrs={"placeholder": "#1e40af"}), "secondary_color": forms.TextInput(attrs={"placeholder": "#be185d"}), "logo": forms.ClearableFileInput(attrs={"accept": "image/png,image/jpeg,image/gif,image/webp,image/svg+xml"}), + "season_start": forms.DateInput(attrs={"type": "date"}), } def __init__(self, *args, **kwargs): diff --git a/controlpanel/tests.py b/controlpanel/tests.py index 03f9914..417e585 100644 --- a/controlpanel/tests.py +++ b/controlpanel/tests.py @@ -117,14 +117,17 @@ class ClubManagementTests(ControlPanelTestBase): self.assertContains(self.client.get(reverse("controlpanel:dashboard")), "Ajax United") def test_create_club_derives_the_slug(self): - response = self.client.post(reverse("controlpanel:club_create"), {"name": "New Club", "slug": ""}) + response = self.client.post(reverse("controlpanel:club_create"), {"name": "New Club", "slug": "", "season_start": "2000-08-01", "season_duration_months": "12"}) club = Club.objects.get(name="New Club") self.assertEqual(club.slug, "new-club") self.assertRedirects(response, reverse("controlpanel:club_detail", args=[club.pk])) def test_update_club(self): - self.client.post(reverse("controlpanel:club_update", args=[self.club.pk]), {"name": "Renamed", "slug": self.club.slug}) + self.client.post( + reverse("controlpanel:club_update", args=[self.club.pk]), + {"name": "Renamed", "slug": self.club.slug, "season_start": "2000-08-01", "season_duration_months": "12"}, + ) self.club.refresh_from_db() self.assertEqual(self.club.name, "Renamed")