bernard/RosterChief
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: season-scope club memberships and convert to UniqueConstraint

Browse Source 
ClubMembership is now tied to a Season (plus status / fee_status / sign-up
dates) and unique per (club, member, season). The CSV importer attaches
each membership to the club's current season (Season.get_current), skipping
the row with a clear error when none exists; the now-unreachable
"clubs created" bookkeeping is removed.

Register SeasonAdmin and rebuild ClubMembershipAdmin for the new fields, and
add an admin smoke test that asserts every model in authentication/club/
members/teams is registered and its changelist + add pages load.

Convert every unique_together to a Meta UniqueConstraint (Season,
ClubMembership, FamilyMembership) per Django's guidance. Regenerate
migrations. club, members, and importer stay at 100% coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Bernard Siebens
2026-07-12 16:47:39 +02:00
parent 3914765f90
commit dd2e4b2169
11 changed files with 316 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
club/admin.py
View File

@@ -1,24 +1,33 @@
from django.contrib import admin
from django.utils.translation import gettext_lazy as _
from .models import Club, ClubMembership
from .models import Club, ClubMembership, Season
# Register your models here.
@admin.register(Club)
class ClubAdmin(admin.ModelAdmin):
list_display = ["name"]
search_fields = ["name"]
list_display = ["name", "slug"]
search_fields = ["name", "slug"]
prepopulated_fields = {"slug": ["name"]}
ordering = ["name"]
@admin.register(Season)
class SeasonAdmin(admin.ModelAdmin):
list_display = ["__str__", "club", "start_date", "end_date"]
list_filter = ["club"]
search_fields = ["club__name"]
ordering = ["club", "-start_date"]
@admin.register(ClubMembership)
class ClubMembershipAdmin(admin.ModelAdmin):
list_display = ["club__name", "member__last_name", "member__first_name", "license"]
search_fields = ["club__name", "member__last_name", "member__first_name"]
list_filter = ["club"]
list_display = ["club__name", "member__last_name", "member__first_name", "season", "status", "fee_status", "license"]
search_fields = ["club__name", "member__last_name", "member__first_name", "license"]
list_filter = ["club", "season", "status", "fee_status"]
raw_id_fields = ["member"]
fieldsets = [
[None, {"fields": ["club", "member"]}],
[_("Member informaton"), {"fields": ["license"]}],
[None, {"fields": ["club", "season", "member"]}],
[_("Membership"), {"fields": ["license", "status", "fee_status"]}],
[_("Dates"), {"fields": ["signed_up_at", "activated_at"]}],
]

25
club/migrations/0007_clubmembership_season_alter_clubmembership_club.py Normal file
View File

@@ -0,0 +1,25 @@
# Generated by Django 6.0.6 on 2026-07-12 13:43
import django.db.models.deletion
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('club', '0006_club_slug_season'),
]
operations = [
migrations.AddField(
model_name='clubmembership',
name='season',
field=models.ForeignKey(default=1, on_delete=django.db.models.deletion.PROTECT, related_name='memberships', to='club.season', verbose_name='season'),
preserve_default=False,
),
migrations.AlterField(
model_name='clubmembership',
name='club',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='club.club'),
),
]

46
club/migrations/0008_alter_clubmembership_unique_together_and_more.py Normal file
View File

@@ -0,0 +1,46 @@
# Generated by Django 6.0.6 on 2026-07-12 14:12
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('club', '0007_clubmembership_season_alter_clubmembership_club'),
('members', '0002_alter_familymembership_unique_together_and_more'),
]
operations = [
migrations.AlterUniqueTogether(
name='clubmembership',
unique_together=set(),
),
migrations.AddField(
model_name='clubmembership',
name='activated_at',
field=models.DateField(blank=True, null=True, verbose_name='activated at'),
),
migrations.AddField(
model_name='clubmembership',
name='fee_status',
field=models.CharField(choices=[('unpaid', 'unpaid'), ('paid', 'paid'), ('partially_paid', 'partially paid'), ('waived', 'waived')], default='unpaid', max_length=250, verbose_name='fee status'),
),
migrations.AddField(
model_name='clubmembership',
name='signed_up_at',
field=models.DateField(blank=True, null=True, verbose_name='signed up at'),
),
migrations.AddField(
model_name='clubmembership',
name='status',
field=models.CharField(choices=[('active', 'active'), ('pending', 'pending'), ('lapsed', 'lapsed'), ('cancelled', 'cancelled')], default='pending', max_length=250, verbose_name='status'),
),
migrations.AddConstraint(
model_name='clubmembership',
constraint=models.UniqueConstraint(fields=('club', 'member', 'season'), name='unique_member_per_club_per_season'),
),
migrations.AddConstraint(
model_name='season',
constraint=models.UniqueConstraint(fields=('club', 'start_date', 'end_date'), name='unique_season_dates_per_club'),
),
]

54
club/models.py
View File

@@ -47,22 +47,6 @@ class Club(UUIDModel):
return slug
class ClubMembership(UUIDModel):
member = models.ForeignKey(Member, on_delete=models.CASCADE, related_name="member_of", verbose_name=_("member"))
club = models.ForeignKey(Club, on_delete=models.CASCADE, related_name="members", verbose_name=_("club"))
license = models.CharField(_("license"), max_length=250, blank=True)
class Meta:
verbose_name = _("club membership")
verbose_name_plural = _("club memberships")
ordering = ["club", "member__last_name", "member__first_name"]
unique_together = ("club", "member")
def __str__(self):
return f"{self.club} - {self.member}"
class Season(ClubScopedModel):
start_date = models.DateField(_("start date"))
end_date = models.DateField(_("end date"))
@@ -73,6 +57,9 @@ class Season(ClubScopedModel):
class Meta:
verbose_name = _("season")
verbose_name_plural = _("seasons")
constraints = [
models.UniqueConstraint(fields=["club", "start_date", "end_date"], name="unique_season_dates_per_club"),
]
@property
def name(self):
@@ -86,3 +73,38 @@ class Season(ClubScopedModel):
date = timezone.now().date()
return cls.objects.current_club().filter(start_date__lte=date, end_date__gte=date).first()
class ClubMembership(ClubScopedModel):
class StatusChoices(models.TextChoices):
ACTIVE = "active", _("active")
PENDING = "pending", _("pending")
LAPSED = "lapsed", _("lapsed")
CANCELLED = "cancelled", _("cancelled")
class FeeStatus(models.TextChoices):
UNPAID = "unpaid", _("unpaid")
PAID = "paid", _("paid")
PARTIALLY_PAID = "partially_paid", _("partially paid")
WAIVED = "waived", _("waived")
member = models.ForeignKey(Member, on_delete=models.CASCADE, related_name="member_of", verbose_name=_("member"))
season = models.ForeignKey(Season, on_delete=models.PROTECT, related_name="memberships", verbose_name=_("season"))
license = models.CharField(_("license"), max_length=250, blank=True)
status = models.CharField(_("status"), max_length=250, choices=StatusChoices.choices, default=StatusChoices.PENDING)
fee_status = models.CharField(_("fee status"), max_length=250, choices=FeeStatus.choices, default=FeeStatus.UNPAID)
signed_up_at = models.DateField(_("signed up at"), blank=True, null=True)
activated_at = models.DateField(_("activated at"), blank=True, null=True)
class Meta:
verbose_name = _("club membership")
verbose_name_plural = _("club memberships")
ordering = ["club", "member__last_name", "member__first_name"]
constraints = [
models.UniqueConstraint(fields=["club", "member", "season"], name="unique_member_per_club_per_season"),
]
def __str__(self):
return f"{self.club} - {self.member}"

171
club/tests.py
View File

@@ -2,8 +2,12 @@ import datetime
import uuid
from contextlib import contextmanager
from django.contrib import admin as django_admin
from django.contrib.auth import get_user_model
from django.db import IntegrityError
from django.db.models import ProtectedError
from django.test import RequestFactory, TestCase, override_settings
from django.urls import reverse
from django.utils import timezone
from members.models import Member
@@ -54,9 +58,18 @@ class ClubModelTests(TestCase):
self.assertEqual(Club._meta.verbose_name_plural, "clubs")
def make_season(club, start_year=2026):
return Season.objects.create(
club=club,
start_date=datetime.date(start_year, 8, 1),
end_date=datetime.date(start_year + 1, 5, 31),
)
class ClubMembershipModelTests(TestCase):
def setUp(self):
self.club = Club.objects.create(name="City Swim Club")
self.season = make_season(self.club)
self.member = Member.objects.create(
first_name="Jane",
last_name="Doe",
@@ -64,98 +77,97 @@ class ClubMembershipModelTests(TestCase):
)
def test_str_returns_club_and_member(self):
membership = ClubMembership.objects.create(
club=self.club,
member=self.member,
license="LIC-001",
)
membership = ClubMembership.objects.create(club=self.club, season=self.season, member=self.member, license="LIC-001")
self.assertEqual(str(membership), "City Swim Club - Jane Doe")
def test_license_is_optional(self):
membership = ClubMembership.objects.create(
club=self.club,
member=self.member,
)
def test_defaults_are_pending_and_unpaid(self):
membership = ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
self.assertEqual(membership.status, ClubMembership.StatusChoices.PENDING)
self.assertEqual(membership.fee_status, ClubMembership.FeeStatus.UNPAID)
self.assertEqual(membership.license, "")
self.assertIsNone(membership.signed_up_at)
def test_pk_is_uuid(self):
membership = ClubMembership.objects.create(
club=self.club,
member=self.member,
)
membership = ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
self.assertIsInstance(membership.pk, uuid.UUID)
def test_same_member_can_join_different_clubs(self):
other_club = Club.objects.create(name="Other Swim Club")
def test_club_is_filled_from_active_tenant(self):
with with_club(self.club):
membership = ClubMembership.objects.create(season=self.season, member=self.member)
first_membership = ClubMembership.objects.create(
club=self.club,
member=self.member,
license="LIC-001",
)
second_membership = ClubMembership.objects.create(
club=other_club,
member=self.member,
license="LIC-002",
)
self.assertEqual(membership.club, self.club)
self.assertEqual(first_membership.member, self.member)
self.assertEqual(second_membership.member, self.member)
self.assertEqual(self.member.member_of.count(), 2)
def test_member_is_unique_per_club(self):
ClubMembership.objects.create(
club=self.club,
member=self.member,
license="LIC-001",
)
def test_member_is_unique_per_club_and_season(self):
ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
with self.assertRaises(IntegrityError):
ClubMembership.objects.create(
club=self.club,
member=self.member,
license="LIC-002",
)
ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
def test_deleting_club_deletes_membership_but_keeps_member(self):
ClubMembership.objects.create(
club=self.club,
member=self.member,
license="LIC-001",
)
def test_same_member_can_join_consecutive_seasons(self):
next_season = make_season(self.club, start_year=2027)
first = ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
second = ClubMembership.objects.create(club=self.club, season=next_season, member=self.member)
self.assertEqual(self.member.member_of.count(), 2)
self.assertNotEqual(first.season, second.season)
def test_same_member_can_join_different_clubs(self):
other_club = Club.objects.create(name="Other Swim Club")
other_season = make_season(other_club)
ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
ClubMembership.objects.create(club=other_club, season=other_season, member=self.member)
self.assertEqual(self.member.member_of.count(), 2)
def test_deleting_club_is_blocked_while_a_season_has_memberships(self):
# Club -> Season is CASCADE, but ClubMembership -> Season is PROTECT, so
# the club can't be deleted while one of its seasons is still referenced.
ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
with self.assertRaises(ProtectedError):
self.club.delete()
self.assertTrue(ClubMembership.objects.exists())
def test_deleting_empty_club_cascades_to_its_seasons(self):
self.club.delete()
self.assertFalse(ClubMembership.objects.exists())
self.assertTrue(Member.objects.filter(pk=self.member.pk).exists())
self.assertFalse(Club.objects.filter(pk=self.club.pk).exists())
self.assertFalse(Season.objects.filter(pk=self.season.pk).exists())
def test_deleting_member_deletes_membership_but_keeps_club(self):
ClubMembership.objects.create(
club=self.club,
member=self.member,
license="LIC-001",
)
ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
self.member.delete()
self.assertFalse(ClubMembership.objects.exists())
self.assertTrue(Club.objects.filter(pk=self.club.pk).exists())
def test_season_is_protected_while_referenced(self):
ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
with self.assertRaises(ProtectedError):
self.season.delete()
def test_memberships_are_ordered_by_club_then_member_name(self):
alpha_club = Club.objects.create(name="Alpha Club")
zulu_club = Club.objects.create(name="Zulu Club")
alpha_season = make_season(alpha_club)
zulu_season = make_season(zulu_club)
jane = Member.objects.create(first_name="Jane", last_name="Doe")
alice = Member.objects.create(first_name="Alice", last_name="Smith")
bob = Member.objects.create(first_name="Bob", last_name="Smith")
ClubMembership.objects.create(club=zulu_club, member=bob)
ClubMembership.objects.create(club=alpha_club, member=bob)
ClubMembership.objects.create(club=alpha_club, member=alice)
ClubMembership.objects.create(club=alpha_club, member=jane)
ClubMembership.objects.create(club=zulu_club, season=zulu_season, member=bob)
ClubMembership.objects.create(club=alpha_club, season=alpha_season, member=bob)
ClubMembership.objects.create(club=alpha_club, season=alpha_season, member=alice)
ClubMembership.objects.create(club=alpha_club, season=alpha_season, member=jane)
self.assertEqual(
[(membership.club.name, membership.member.last_name, membership.member.first_name) for membership in ClubMembership.objects.all()],
@@ -168,14 +180,11 @@ class ClubMembershipModelTests(TestCase):
)
def test_reverse_relations(self):
membership = ClubMembership.objects.create(
club=self.club,
member=self.member,
license="LIC-001",
)
membership = ClubMembership.objects.create(club=self.club, season=self.season, member=self.member)
self.assertEqual(list(self.club.members.all()), [membership])
self.assertEqual(list(self.club.clubmemberships.all()), [membership])
self.assertEqual(list(self.member.member_of.all()), [membership])
self.assertEqual(list(self.season.memberships.all()), [membership])
def test_verbose_names(self):
self.assertEqual(ClubMembership._meta.verbose_name, "club membership")
@@ -430,3 +439,37 @@ class SeasonGetCurrentTests(TestCase):
def test_requires_an_active_club(self):
with self.assertRaises(RuntimeError):
Season.get_current(datetime.date(2026, 12, 25))
class AdminRegistrationSmokeTests(TestCase):
"""Every registered model across all apps must have a working admin: load
each changelist and add page to catch bad list_display / search_fields /
fieldsets / autocomplete targets in any app's admin config."""
def setUp(self):
self.admin = get_user_model().objects.create_superuser(email="root@club.test", password="pw-secret-123")
self.client.force_login(self.admin)
def test_every_model_is_registered_in_admin(self):
from django.apps import apps
registered = set(django_admin.site._registry)
# Concrete, non-auto-created models in these apps should all be registered.
project_apps = {"authentication", "club", "members", "teams"}
for model in apps.get_models():
if model._meta.app_label not in project_apps or model._meta.auto_created:
continue
with self.subTest(model=model.__name__):
self.assertIn(model, registered, f"{model.__name__} is not registered in the admin")
def test_all_changelists_load(self):
for model in django_admin.site._registry:
url = reverse(f"admin:{model._meta.app_label}_{model._meta.model_name}_changelist")
with self.subTest(model=model.__name__):
self.assertEqual(self.client.get(url).status_code, 200)
def test_all_add_pages_load(self):
for model in django_admin.site._registry:
url = reverse(f"admin:{model._meta.app_label}_{model._meta.model_name}_add")
with self.subTest(model=model.__name__):
self.assertEqual(self.client.get(url).status_code, 200)

1
members/management/commands/import_members_csv.py
View File

@@ -35,7 +35,6 @@ class Command(BaseCommand):
f"Members created: {result.created_members}. "
f"Members updated: {result.updated_members}. "
f"Users created: {result.created_users}. "
f"Clubs created: {result.created_clubs}. "
f"Memberships created: {result.created_memberships}. "
f"Memberships updated: {result.updated_memberships}. "
f"Rows skipped: {result.skipped_rows}."

21
members/migrations/0002_alter_familymembership_unique_together_and_more.py Normal file
View File

@@ -0,0 +1,21 @@
# Generated by Django 6.0.6 on 2026-07-12 14:12
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('members', '0001_initial'),
]
operations = [
migrations.AlterUniqueTogether(
name='familymembership',
unique_together=set(),
),
migrations.AddConstraint(
model_name='familymembership',
constraint=models.UniqueConstraint(fields=('family', 'member'), name='unique_member_per_family'),
),
]

4
members/models.py
View File

@@ -93,7 +93,9 @@ class FamilyMembership(models.Model):
verbose_name = _("family membership")
verbose_name_plural = _("family memberships")
ordering = ["family", "role", "member__last_name", "member__first_name"]
unique_together = ("family", "member")
constraints = [
models.UniqueConstraint(fields=["family", "member"], name="unique_member_per_family"),
]
def __str__(self):
return f"{self.family} - {self.member} ({self.get_role_display()})"

27
members/services/member_csv_importer.py
View File

@@ -7,7 +7,8 @@ from typing import IO, Any
from django.contrib.auth import get_user_model
from django.db import transaction
from club.models import Club, ClubMembership
from club.models import Club, ClubMembership, Season
from club.tenancy import reset_current_club, set_current_club
from members.models import Member
REQUIRED_COLUMNS = {"first_name", "last_name", "email", "date_of_birth", "create_account", "club_name", "license_number"}
@@ -27,7 +28,6 @@ class MemberImportResult:
created_members: int = 0
updated_members: int = 0
created_users: int = 0
created_clubs: int = 0
created_memberships: int = 0
updated_memberships: int = 0
skipped_rows: int = 0
@@ -42,7 +42,6 @@ class MemberImportResult:
class ImportedMemberRowResult:
member_created: bool
user_created: bool
club_created: bool
membership_created: bool
@@ -91,9 +90,6 @@ class MemberCsvImporter:
if row_result.user_created:
result.created_users += 1
if row_result.club_created:
result.created_clubs += 1
if row_result.membership_created:
result.created_memberships += 1
else:
@@ -127,11 +123,13 @@ class MemberCsvImporter:
},
)
club, club_created = Club.objects.get_or_create(name=club_name)
club, _ = Club.objects.get_or_create(name=club_name)
season = self.get_current_season(club)
_, membership_created = ClubMembership.objects.update_or_create(
club=club,
member=member,
season=season,
defaults={
"license": license_number,
},
@@ -140,10 +138,23 @@ class MemberCsvImporter:
return ImportedMemberRowResult(
member_created=member_created,
user_created=user_created,
club_created=club_created,
membership_created=membership_created,
)
def get_current_season(self, club) -> Season:
# Season.get_current() is tenant-scoped, so bind the row's club as the
# active tenant for the lookup.
token = set_current_club(club)
try:
season = Season.get_current()
finally:
reset_current_club(token)
if season is None:
raise ValueError(f"No current season for club '{club.name}'.")
return season
def get_or_create_user(self, email) -> tuple[User, bool]:
user, created = User.objects.get_or_create(
email=email,

47
members/tests.py
View File

@@ -1,5 +1,5 @@
import tempfile
from datetime import date
from datetime import date, timedelta
from io import StringIO
from pathlib import Path
@@ -9,9 +9,10 @@ from django.core.management.base import CommandError
from django.db import IntegrityError
from django.db.models import SET_NULL
from django.test import TestCase
from django.utils import timezone
from authentication.models import User
from club.models import Club, ClubMembership
from club.models import Club, ClubMembership, Season
from members.admin import FamilyAdmin
from members.models import Family, FamilyMembership, Member
from members.services import MemberImportResult
@@ -292,6 +293,17 @@ class AdminSmokeTests(TestCase):
class ImportMembersCsvCommandTests(TestCase):
def setUp(self):
# Memberships are season-scoped: the importer attaches each one to the
# club's current season, so the target club needs one covering today.
self.club = Club.objects.create(name="City Swim Club")
today = timezone.localdate()
self.season = Season.objects.create(
club=self.club,
start_date=today - timedelta(days=90),
end_date=today + timedelta(days=275),
)
def write_csv(self, content):
temp_file = tempfile.NamedTemporaryFile(mode="w", suffix=".csv", delete=False, encoding="utf-8")
temp_file.write(content)
@@ -329,7 +341,6 @@ class ImportMembersCsvCommandTests(TestCase):
self.assertIn("Import complete.", stdout)
self.assertIn("Members created: 1.", stdout)
self.assertIn("Users created: 1.", stdout)
self.assertIn("Clubs created: 1.", stdout)
self.assertIn("Memberships created: 1.", stdout)
self.assertIn("Rows skipped: 0.", stdout)
@@ -341,9 +352,9 @@ class ImportMembersCsvCommandTests(TestCase):
self.assertEqual(member.user.email, "jane@example.com")
self.assertFalse(member.user.has_usable_password())
club = Club.objects.get(name="City Swim Club")
membership = ClubMembership.objects.get(club=club, member=member)
membership = ClubMembership.objects.get(club=self.club, member=member)
self.assertEqual(membership.license, "LIC-001")
self.assertEqual(membership.season, self.season)
def test_import_creates_member_without_user_when_create_account_is_false(self):
csv_path = self.write_csv(
@@ -366,7 +377,6 @@ class ImportMembersCsvCommandTests(TestCase):
self.assertFalse(User.objects.filter(email="john@example.com").exists())
def test_import_updates_existing_member_and_membership(self):
club = Club.objects.create(name="City Swim Club")
member = Member.objects.create(
first_name="Old",
last_name="Name",
@@ -374,7 +384,8 @@ class ImportMembersCsvCommandTests(TestCase):
date_of_birth=date(2010, 1, 1),
)
ClubMembership.objects.create(
club=club,
club=self.club,
season=self.season,
member=member,
license="OLD-LIC",
)
@@ -401,7 +412,7 @@ class ImportMembersCsvCommandTests(TestCase):
self.assertEqual(member.last_name, "Doe")
self.assertEqual(member.date_of_birth, date(2010, 4, 12))
membership = ClubMembership.objects.get(club=club, member=member)
membership = ClubMembership.objects.get(club=self.club, member=member)
self.assertEqual(membership.license, "LIC-NEW")
def test_import_links_existing_user_when_create_account_is_true(self):
@@ -539,6 +550,26 @@ class ImportMembersCsvCommandTests(TestCase):
self.assertIn("Rows skipped: 1.", stdout)
self.assertFalse(Member.objects.filter(email="nofirst@example.com").exists())
def test_import_skips_row_when_club_has_no_current_season(self):
# "New Club" has no season, so the row is skipped and — because the row
# is atomic — the member and club creation roll back too.
csv_path = self.write_csv(
"\n".join(
[
"first_name,last_name,email,date_of_birth,create_account,club_name,license_number",
"Jane,Doe,jane@example.com,2010-04-12,false,New Club,LIC-001",
]
)
)
stdout, stderr = self.call_import_command(csv_path)
self.assertIn("Row 2 skipped:", stderr)
self.assertIn("No current season for club 'New Club'.", stderr)
self.assertIn("Rows skipped: 1.", stdout)
self.assertFalse(Member.objects.filter(email="jane@example.com").exists())
self.assertFalse(Club.objects.filter(name="New Club").exists())
class MemberImportResultTests(TestCase):
def test_successful_rows_sums_created_and_updated(self):

1
members/views.py
View File

@@ -1,2 +1 @@
# Create your views here.