diff --git a/events/services/referees.py b/events/services/referees.py index a371d25..659ffd7 100644 --- a/events/services/referees.py +++ b/events/services/referees.py @@ -30,7 +30,7 @@ from django.utils.translation import gettext_lazy as _ from events.models import ASSUMED_EVENT_DURATION, Event, EventReferee from events.services.attendance import effective_members from members.models import Member -from teams.models import Team +from teams.models import RefereeLevel, Team class RefereeAssignmentError(Exception): @@ -52,17 +52,24 @@ def needs_referee_management(event) -> bool: def eligible_referees(event): """Members who could referee `event`: their RefereeProfile has a level - qualifying for one of its club-managed teams, and is currently valid, - minus whoever is already assigned. Empty unless needs_referee_management.""" + qualifying for one of its club-managed teams (directly, or via whatever + that level inherits from), and is currently valid, minus whoever is + already assigned. Empty unless needs_referee_management. + + Levels resolved to ids first (RefereeLevel.eligible_team_ids can't be + expressed as a single ORM lookup once inheritance is transitive) rather + than the old flat `level__teams__id__in` filter -- a club has a handful + of levels, so this stays cheap.""" if not needs_referee_management(event): return Member.objects.none() - team_ids = list(event.teams.filter(referee_management=Team.RefereeManagement.CLUB).values_list("id", flat=True)) + team_ids = set(event.teams.filter(referee_management=Team.RefereeManagement.CLUB).values_list("id", flat=True)) + qualifying_level_ids = [level.pk for level in RefereeLevel.objects.filter(club=event.club) if level.eligible_team_ids() & team_ids] assigned_ids = event.referees.values_list("member_id", flat=True) today = timezone.localdate() return ( - Member.objects.filter(referee_profile__level__teams__id__in=team_ids, referee_profile__valid_until__gte=today) + Member.objects.filter(referee_profile__level_id__in=qualifying_level_ids, referee_profile__valid_until__gte=today) .exclude(pk__in=assigned_ids) .distinct() ) diff --git a/events/tests.py b/events/tests.py index 22cbbcb..6194332 100644 --- a/events/tests.py +++ b/events/tests.py @@ -1205,6 +1205,18 @@ class RefereeServiceTests(EventsTestBase): self.assertEqual(set(eligible_referees(game)), set()) + def test_eligible_referees_includes_a_higher_level_via_inheritance(self): + # self.level ("Regional") qualifies for self.team; a referee holding a + # higher level that inherits from Regional (without self.team added + # directly) must still show up -- see RefereeLevel.eligible_team_ids. + national = RefereeLevel.objects.create(club=self.club, name="National", inherits_from=self.level) + national_referee = Member.objects.create(first_name="Nat", last_name="Ional") + self.make_eligible_profile(national_referee, level=national) + + game = self.make_home_game() + + self.assertIn(national_referee, set(eligible_referees(game))) + def test_eligible_referees_ignores_an_expired_profile(self): expired_referee = Member.objects.create(first_name="Ex", last_name="Pired") RefereeProfile.objects.create(member=expired_referee, level=self.level, valid_until=timezone.localdate() - timedelta(days=1)) diff --git a/management/forms.py b/management/forms.py index 62930e8..69125cf 100644 --- a/management/forms.py +++ b/management/forms.py @@ -56,12 +56,17 @@ class GroupForm(forms.ModelForm): class RefereeLevelForm(forms.ModelForm): class Meta: model = RefereeLevel - fields = ["name", "ordering", "teams"] + fields = ["name", "ordering", "teams", "inherits_from"] widgets = {"teams": forms.SelectMultiple(attrs={"data-searchable": "true", "data-search-placeholder": _("Type a team to search...")})} def __init__(self, *args, club=None, **kwargs): super().__init__(*args, **kwargs) self.fields["teams"].queryset = Team.objects.filter(club=club) + # Excludes self from the dropdown -- self.instance always has a pk (UUIDModel + # gets its default at construction, not at save()), so this also works for a + # brand-new, unsaved level. clean() still catches an indirect loop (A -> B -> A). + self.fields["inherits_from"].queryset = RefereeLevel.objects.filter(club=club).exclude(pk=self.instance.pk) + self.fields["inherits_from"].empty_label = _("— none —") class MemberRefereeEligibilityForm(forms.ModelForm): diff --git a/management/templates/management/referee_level_list.html b/management/templates/management/referee_level_list.html index 25222fe..be62dc3 100644 --- a/management/templates/management/referee_level_list.html +++ b/management/templates/management/referee_level_list.html @@ -17,6 +17,7 @@ {% trans "Ordering" %} {% trans "Name" %} + {% trans "Inherits from" %} {% trans "Qualifies for" %} @@ -26,12 +27,22 @@ {{ level.ordering }} {{ level.name }} + + {% if level.inherits_from %} + {{ level.inherits_from.name }} + {% else %} + — + {% endif %} + {% for team in level.teams.all %} {{ team.name }} {% empty %} - — + {% if not level.inherits_from %}—{% endif %} {% endfor %} + {% if level.inherits_from %} +
{% blocktrans with name=level.inherits_from.name %}+ everything “{{ name }}” covers{% endblocktrans %}
+ {% endif %} {% if is_club_admin %} @@ -41,7 +52,7 @@ {% empty %} - {% trans "No referee levels yet." %} + {% trans "No referee levels yet." %} {% endfor %} diff --git a/management/tests.py b/management/tests.py index 2d7be39..9e32661 100644 --- a/management/tests.py +++ b/management/tests.py @@ -1318,6 +1318,46 @@ class RefereeLevelManagementTests(ManagementTestBase): self.assertNotContains(response, "Rival Level") + def test_admin_can_link_a_level_to_inherit_from_another(self): + regional = RefereeLevel.objects.create(club=self.club, name="Regional") + regional.teams.add(self.team) + self.client.force_login(self.admin_user) + + response = self.club_post("referee_level_create", {"name": "National", "ordering": 0, "teams": [str(self.other_team.pk)], "inherits_from": str(regional.pk)}) + + national = RefereeLevel.objects.get(club=self.club, name="National") + self.assertRedirects(response, reverse("management:referee_level_list")) + self.assertEqual(national.inherits_from, regional) + self.assertEqual(national.eligible_team_ids(), {self.team.pk, self.other_team.pk}) + + def test_a_level_cannot_be_set_to_inherit_from_itself(self): + level = RefereeLevel.objects.create(club=self.club, name="Regional") + self.client.force_login(self.admin_user) + + self.club_post("referee_level_update", {"name": "Regional", "ordering": 0, "teams": [], "inherits_from": str(level.pk)}, level.pk) + + level.refresh_from_db() + self.assertIsNone(level.inherits_from) + + def test_the_inherits_from_dropdown_only_offers_this_clubs_levels(self): + other_club = Club.objects.create(name="Rival FC", slug="rival-fc") + RefereeLevel.objects.create(club=other_club, name="Rival Level") + self.client.force_login(self.admin_user) + + response = self.club_get("referee_level_create") + + self.assertNotContains(response, "Rival Level") + + def test_the_list_shows_what_a_level_inherits(self): + regional = RefereeLevel.objects.create(club=self.club, name="Regional") + RefereeLevel.objects.create(club=self.club, name="National", inherits_from=regional) + self.client.force_login(self.admin_user) + + response = self.club_get("referee_level_list") + + self.assertContains(response, "Regional") + self.assertContains(response, "everything") + class RefereeListViewTests(ManagementTestBase): """The club-wide referee overview -- see management.views.RefereeListView.""" diff --git a/management/views.py b/management/views.py index 253417e..174169b 100644 --- a/management/views.py +++ b/management/views.py @@ -1133,7 +1133,16 @@ class TeamDetailView(ClubStaffRequiredMixin, DetailView): no_shows=no_shows, # None (not an empty queryset) signals "federation-managed" to the # template, distinct from "club-managed, nobody eligible yet". - eligible_referees=(Member.objects.filter(referee_profile__level__teams=team, referee_profile__valid_until__gte=timezone.localdate()).order_by("last_name", "first_name") if team.referee_management == Team.RefereeManagement.CLUB else None), + # Levels resolved to ids first, not a flat level__teams=team filter -- + # same reasoning as events.services.referees.eligible_referees, since + # a level may qualify for this team only via what it inherits from. + eligible_referees=( + Member.objects.filter( + referee_profile__level_id__in=[level.pk for level in RefereeLevel.objects.filter(club=club) if team.pk in level.eligible_team_ids()], referee_profile__valid_until__gte=timezone.localdate() + ).order_by("last_name", "first_name") + if team.referee_management == Team.RefereeManagement.CLUB + else None + ), **kwargs, ) @@ -1835,7 +1844,7 @@ class RefereeLevelListView(ClubStaffRequiredMixin, ListView): context_object_name = "levels" def get_queryset(self): - return RefereeLevel.objects.filter(club=self.request.club).prefetch_related("teams") + return RefereeLevel.objects.filter(club=self.request.club).select_related("inherits_from").prefetch_related("teams") class RefereeLevelCreateView(MemberAdminRequiredMixin, CreateView): @@ -1890,8 +1899,11 @@ class RefereeListView(ClubStaffRequiredMixin, ListView): context_object_name = "referees" def get_queryset(self): + # No prefetch for eligible_teams below select_related's level: it walks the + # level's own inherits_from chain (RefereeLevel.eligible_team_ids), which a + # single prefetch_related path can't cover anyway. members = members_visible_to(self.request.user, self.request.club, include_guardians=True).filter(referee_profile__isnull=False) - return members.select_related("referee_profile", "referee_profile__level").prefetch_related("referee_profile__level__teams").order_by("last_name", "first_name") + return members.select_related("referee_profile", "referee_profile__level").order_by("last_name", "first_name") # --- Groups: a generic named collection of members (all coaches, all team managers, diff --git a/teams/admin.py b/teams/admin.py index aeb86a3..92e041d 100644 --- a/teams/admin.py +++ b/teams/admin.py @@ -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")) diff --git a/teams/migrations/0011_refereelevel_inherits_from.py b/teams/migrations/0011_refereelevel_inherits_from.py new file mode 100644 index 0000000..3e018a7 --- /dev/null +++ b/teams/migrations/0011_refereelevel_inherits_from.py @@ -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'), + ), + ] diff --git a/teams/models.py b/teams/models.py index c465d14..4e35f15 100644 --- a/teams/models.py +++ b/teams/models.py @@ -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): diff --git a/teams/tests.py b/teams/tests.py index 8c1f186..e085f34 100644 --- a/teams/tests.py +++ b/teams/tests.py @@ -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())