Let referee levels inherit from a lower level instead of relying on ordering alone
RefereeLevel.inherits_from chains levels together so a higher tier is automatically eligible for everything a linked lower tier covers, transitively, without hand-duplicating teams onto every level. Kept the ordering field (still drives display order) but eligibility everywhere (RefereeProfile.eligible_teams, events.services.referees, the team detail page's eligible-referees list) now reads through RefereeLevel.eligible_team_ids, which walks the inherits_from chain. clean() rejects a loop, including an indirect one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9
This commit is contained in:
@@ -72,10 +72,10 @@ class StaffAssignmentAdmin(admin.ModelAdmin):
|
||||
|
||||
@admin.register(RefereeLevel)
|
||||
class RefereeLevelAdmin(admin.ModelAdmin):
|
||||
list_display = ["name", "club", "ordering", "team_list"]
|
||||
list_display = ["name", "club", "ordering", "inherits_from", "team_list"]
|
||||
list_filter = ["club"]
|
||||
search_fields = ["name"]
|
||||
autocomplete_fields = ["teams"]
|
||||
autocomplete_fields = ["teams", "inherits_from"]
|
||||
ordering = ["club", "ordering", "name"]
|
||||
|
||||
@admin.display(description=_("teams"))
|
||||
|
||||
19
teams/migrations/0011_refereelevel_inherits_from.py
Normal file
19
teams/migrations/0011_refereelevel_inherits_from.py
Normal file
@@ -0,0 +1,19 @@
|
||||
# Generated by Django 6.0.6 on 2026-08-20 20:39
|
||||
|
||||
import django.db.models.deletion
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('teams', '0010_alter_team_options_alter_teammembership_position'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='refereelevel',
|
||||
name='inherits_from',
|
||||
field=models.ForeignKey(blank=True, help_text='A referee holding this level is also eligible for everything the linked level covers (and, transitively, whatever that one inherits from).', null=True, on_delete=django.db.models.deletion.PROTECT, related_name='inherited_by', to='teams.refereelevel', verbose_name='inherits from'),
|
||||
),
|
||||
]
|
||||
@@ -1,3 +1,4 @@
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.db import models
|
||||
from django.db.models import Q
|
||||
from django.utils import timezone
|
||||
@@ -125,11 +126,27 @@ class RefereeLevel(ClubScopedModel):
|
||||
qualifies a referee for: eligibility is a property of the *level*, not of
|
||||
the individual referee -- a club typically has a handful of levels, each
|
||||
unlocking a tier of teams/competitions, rather than hand-picking teams per
|
||||
referee."""
|
||||
referee.
|
||||
|
||||
`inherits_from` chains levels together so a higher tier doesn't need every
|
||||
lower tier's team re-added by hand: a "National" referee is automatically
|
||||
eligible for everything "Regional" (its inherits_from) covers, and so on
|
||||
down the chain -- see eligible_team_ids, the single definition every
|
||||
consumer (RefereeProfile.eligible_teams, events.services.referees) reads
|
||||
through."""
|
||||
|
||||
name = models.CharField(_("name"), max_length=255)
|
||||
ordering = models.PositiveSmallIntegerField(_("ordering"), default=0, help_text=_("Lower numbers are listed first. Levels with the same number are ordered by name."))
|
||||
teams = models.ManyToManyField(Team, related_name="referee_levels", blank=True, verbose_name=_("qualifies for"), help_text=_("Members holding this level can be assigned to referee these teams' home games."))
|
||||
inherits_from = models.ForeignKey(
|
||||
"self",
|
||||
on_delete=models.PROTECT,
|
||||
null=True,
|
||||
blank=True,
|
||||
related_name="inherited_by",
|
||||
verbose_name=_("inherits from"),
|
||||
help_text=_("A referee holding this level is also eligible for everything the linked level covers (and, transitively, whatever that one inherits from)."),
|
||||
)
|
||||
|
||||
class Meta:
|
||||
verbose_name = _("referee level")
|
||||
@@ -142,6 +159,33 @@ class RefereeLevel(ClubScopedModel):
|
||||
def __str__(self):
|
||||
return self.name
|
||||
|
||||
def clean(self):
|
||||
validate_club_scope(self, self.club_id, same_club_fields=("inherits_from",))
|
||||
current = self.inherits_from
|
||||
seen = set()
|
||||
while current is not None:
|
||||
if current.pk == self.pk:
|
||||
raise ValidationError({"inherits_from": _("This would create a loop -- a level can't inherit from itself, even indirectly.")})
|
||||
if current.pk in seen:
|
||||
break # An already-broken chain elsewhere; not this field's problem to fix.
|
||||
seen.add(current.pk)
|
||||
current = current.inherits_from
|
||||
|
||||
def eligible_team_ids(self):
|
||||
"""This level's own qualifying teams, plus (transitively) every level
|
||||
it inherits from -- so a higher tier doesn't need a lower tier's teams
|
||||
duplicated onto it by hand, and stays correct if the lower tier's own
|
||||
teams change later. Cycle-guarded even though clean() already blocks
|
||||
creating one, in case of data written outside that path."""
|
||||
team_ids = set(self.teams.values_list("id", flat=True))
|
||||
seen = {self.pk}
|
||||
current = self.inherits_from
|
||||
while current is not None and current.pk not in seen:
|
||||
team_ids.update(current.teams.values_list("id", flat=True))
|
||||
seen.add(current.pk)
|
||||
current = current.inherits_from
|
||||
return team_ids
|
||||
|
||||
|
||||
class RefereeProfile(UUIDModel):
|
||||
"""Marks a member as a club referee: their level (which determines which
|
||||
@@ -184,10 +228,12 @@ class RefereeProfile(UUIDModel):
|
||||
@property
|
||||
def eligible_teams(self):
|
||||
"""Teams this profile currently qualifies for -- empty whenever it
|
||||
isn't currently eligible, regardless of what level is set."""
|
||||
isn't currently eligible, regardless of what level is set. Includes
|
||||
whatever the level inherits from, transitively -- see
|
||||
RefereeLevel.eligible_team_ids."""
|
||||
if not self.is_eligible:
|
||||
return Team.objects.none()
|
||||
return self.level.teams.all()
|
||||
return Team.objects.filter(id__in=self.level.eligible_team_ids())
|
||||
|
||||
|
||||
class StaffAssignment(UUIDModel):
|
||||
|
||||
@@ -246,6 +246,65 @@ class RefereeLevelModelTests(TeamsTestCase):
|
||||
|
||||
self.assertEqual(list(self.team.referee_levels.all()), [level])
|
||||
|
||||
def test_eligible_team_ids_with_no_inheritance_is_just_its_own_teams(self):
|
||||
level = RefereeLevel.objects.create(club=self.club, name="Regional")
|
||||
level.teams.add(self.team)
|
||||
|
||||
self.assertEqual(level.eligible_team_ids(), {self.team.pk})
|
||||
|
||||
def test_a_higher_level_inherits_its_lower_levels_teams(self):
|
||||
other_team = Team.objects.create(club=self.club, name="Second Team", short_name="2nd")
|
||||
regional = RefereeLevel.objects.create(club=self.club, name="Regional")
|
||||
regional.teams.add(self.team)
|
||||
national = RefereeLevel.objects.create(club=self.club, name="National", inherits_from=regional)
|
||||
national.teams.add(other_team)
|
||||
|
||||
self.assertEqual(national.eligible_team_ids(), {self.team.pk, other_team.pk})
|
||||
# Inheritance is one-directional -- Regional doesn't gain National's teams.
|
||||
self.assertEqual(regional.eligible_team_ids(), {self.team.pk})
|
||||
|
||||
def test_inheritance_is_transitive_through_a_chain(self):
|
||||
other_team = Team.objects.create(club=self.club, name="Second Team", short_name="2nd")
|
||||
third_team = Team.objects.create(club=self.club, name="Third Team", short_name="3rd")
|
||||
local = RefereeLevel.objects.create(club=self.club, name="Local")
|
||||
local.teams.add(self.team)
|
||||
regional = RefereeLevel.objects.create(club=self.club, name="Regional", inherits_from=local)
|
||||
regional.teams.add(other_team)
|
||||
national = RefereeLevel.objects.create(club=self.club, name="National", inherits_from=regional)
|
||||
national.teams.add(third_team)
|
||||
|
||||
self.assertEqual(national.eligible_team_ids(), {self.team.pk, other_team.pk, third_team.pk})
|
||||
|
||||
def test_a_level_cannot_inherit_from_itself(self):
|
||||
level = RefereeLevel.objects.create(club=self.club, name="Regional")
|
||||
level.inherits_from = level
|
||||
|
||||
with self.assertRaises(ValidationError):
|
||||
level.clean()
|
||||
|
||||
def test_a_level_cannot_indirectly_inherit_from_itself(self):
|
||||
regional = RefereeLevel.objects.create(club=self.club, name="Regional")
|
||||
national = RefereeLevel.objects.create(club=self.club, name="National", inherits_from=regional)
|
||||
regional.inherits_from = national
|
||||
|
||||
with self.assertRaises(ValidationError):
|
||||
regional.clean()
|
||||
|
||||
def test_inherits_from_must_be_the_same_club(self):
|
||||
other_club = Club.objects.create(name="Rival FC", slug="rival-fc")
|
||||
other_level = RefereeLevel.objects.create(club=other_club, name="Regional")
|
||||
level = RefereeLevel.objects.create(club=self.club, name="National", inherits_from=other_level)
|
||||
|
||||
with self.assertRaises(ValidationError):
|
||||
level.clean()
|
||||
|
||||
def test_deleting_an_inherited_from_level_is_protected(self):
|
||||
regional = RefereeLevel.objects.create(club=self.club, name="Regional")
|
||||
RefereeLevel.objects.create(club=self.club, name="National", inherits_from=regional)
|
||||
|
||||
with self.assertRaises(ProtectedError):
|
||||
regional.delete()
|
||||
|
||||
|
||||
class RefereeProfileModelTests(TeamsTestCase):
|
||||
@classmethod
|
||||
@@ -291,6 +350,14 @@ class RefereeProfileModelTests(TeamsTestCase):
|
||||
profile = RefereeProfile.objects.create(member=self.member, level=self.level, valid_until=timezone.localdate() + datetime.timedelta(days=1))
|
||||
self.assertEqual(list(profile.eligible_teams), [self.team])
|
||||
|
||||
def test_eligible_teams_include_what_the_level_inherits(self):
|
||||
other_team = Team.objects.create(club=self.club, name="Second Team", short_name="2nd")
|
||||
national = RefereeLevel.objects.create(club=self.club, name="National", inherits_from=self.level)
|
||||
national.teams.add(other_team)
|
||||
profile = RefereeProfile.objects.create(member=self.member, level=national, valid_until=timezone.localdate() + datetime.timedelta(days=1))
|
||||
|
||||
self.assertEqual(set(profile.eligible_teams), {self.team, other_team})
|
||||
|
||||
def test_deleting_a_referenced_level_is_protected(self):
|
||||
RefereeProfile.objects.create(member=self.member, level=self.level, valid_until=timezone.localdate())
|
||||
|
||||
|
||||
Reference in New Issue
Block a user