From c0816a1addc0d66203951423d4904280e00cb951 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Mon, 13 Jul 2026 14:42:06 +0200 Subject: [PATCH] feat(teams): management positions and cross-club validation Position gains management_position, distinguishing a coach/manager from other staff (physio, kit manager). A CheckConstraint enforces that a management position is always a staff position. TeamMembership and StaffAssignment validate that their season and position belong to the same club as the team. Co-Authored-By: Claude Opus 4.8 --- .../0002_position_management_position.py | 18 ++++++++ ...agement_position_implies_staff_position.py | 18 ++++++++ teams/models.py | 14 +++++- teams/tests.py | 43 +++++++++++++++++++ 4 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 teams/migrations/0002_position_management_position.py create mode 100644 teams/migrations/0003_position_management_position_implies_staff_position.py diff --git a/teams/migrations/0002_position_management_position.py b/teams/migrations/0002_position_management_position.py new file mode 100644 index 0000000..ebf1f0e --- /dev/null +++ b/teams/migrations/0002_position_management_position.py @@ -0,0 +1,18 @@ +# Generated by Django 6.0.6 on 2026-07-13 07:45 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('teams', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='position', + name='management_position', + field=models.BooleanField(default=False, verbose_name='management position'), + ), + ] diff --git a/teams/migrations/0003_position_management_position_implies_staff_position.py b/teams/migrations/0003_position_management_position_implies_staff_position.py new file mode 100644 index 0000000..617266e --- /dev/null +++ b/teams/migrations/0003_position_management_position_implies_staff_position.py @@ -0,0 +1,18 @@ +# Generated by Django 6.0.6 on 2026-07-13 07:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('club', '0009_clubrole'), + ('teams', '0002_position_management_position'), + ] + + operations = [ + migrations.AddConstraint( + model_name='position', + constraint=models.CheckConstraint(condition=models.Q(('management_position', False), ('staff_position', True), _connector='OR'), name='management_position_implies_staff_position'), + ), + ] diff --git a/teams/models.py b/teams/models.py index 2b41c16..ce43855 100644 --- a/teams/models.py +++ b/teams/models.py @@ -1,8 +1,9 @@ from django.db import models +from django.db.models import Q from django.utils.translation import gettext_lazy as _ from club.models import Season -from clubmanager.base import ClubScopedModel, UUIDModel +from clubmanager.base import ClubScopedModel, UUIDModel, validate_club_scope from members.models import Member @@ -27,12 +28,15 @@ class Position(ClubScopedModel): ordering = models.PositiveSmallIntegerField(_("ordering"), default=0) staff_position = models.BooleanField(_("staff position"), default=False) + management_position = models.BooleanField(_("management position"), default=False) class Meta: verbose_name = _("position") verbose_name_plural = _("positions") constraints = [ models.UniqueConstraint(fields=["club", "name"], name="unique_position_name_per_club"), + # A management position is always a staff position. + models.CheckConstraint(condition=Q(management_position=False) | Q(staff_position=True), name="management_position_implies_staff_position"), ] ordering = ["ordering", "name"] @@ -62,6 +66,10 @@ class TeamMembership(UUIDModel): def __str__(self): return f"{self.team} - {self.member}" + def clean(self): + club_id = self.team.club_id if self.team_id else None + validate_club_scope(self, club_id, same_club_fields=("season", "position")) + class StaffAssignment(UUIDModel): team = models.ForeignKey(Team, on_delete=models.CASCADE, related_name="staff_assignments", verbose_name=_("team")) @@ -79,3 +87,7 @@ class StaffAssignment(UUIDModel): def __str__(self): return f"{self.team} - {self.member}" + + def clean(self): + club_id = self.team.club_id if self.team_id else None + validate_club_scope(self, club_id, same_club_fields=("season", "position")) diff --git a/teams/tests.py b/teams/tests.py index 542163b..a16100f 100644 --- a/teams/tests.py +++ b/teams/tests.py @@ -1,5 +1,6 @@ import datetime +from django.core.exceptions import ValidationError from django.db import IntegrityError from django.db.models import ProtectedError from django.test import TestCase @@ -59,6 +60,15 @@ class PositionModelTests(TeamsTestCase): with self.assertRaises(IntegrityError): Position.objects.create(club=self.club, name="Forward", short_name="dup") + def test_management_position_must_also_be_a_staff_position(self): + with self.assertRaises(IntegrityError): + Position.objects.create(club=self.club, name="Bogus", short_name="BG", staff_position=False, management_position=True) + + def test_management_staff_position_is_allowed(self): + position = Position.objects.create(club=self.club, name="Manager", short_name="MG", staff_position=True, management_position=True) + + self.assertTrue(position.management_position) + class TeamMembershipModelTests(TeamsTestCase): def test_can_create_roster_entry(self): @@ -110,3 +120,36 @@ class StaffAssignmentModelTests(TeamsTestCase): self.assertEqual(self.forward.team_memberships.count(), 1) self.assertEqual(self.coach.staff_assignments.count(), 1) + + +class RosterCleanTests(TeamsTestCase): + def setUp(self): + super().setUp() + self.other = Club.objects.create(name="Rival FC", slug="rival-fc") + self.other_season = Season.objects.create(club=self.other, start_date=datetime.date(2026, 8, 1), end_date=datetime.date(2027, 5, 31)) + self.other_position = Position.objects.create(club=self.other, name="Forward", short_name="FW") + self.other_coach = Position.objects.create(club=self.other, name="Coach", short_name="C", staff_position=True) + + def test_teammembership_rejects_cross_club_season(self): + entry = TeamMembership(team=self.team, member=self.member, season=self.other_season, position=self.forward) + with self.assertRaises(ValidationError) as ctx: + entry.full_clean() + self.assertIn("season", ctx.exception.error_dict) + + def test_teammembership_rejects_cross_club_position(self): + entry = TeamMembership(team=self.team, member=self.member, season=self.season, position=self.other_position) + with self.assertRaises(ValidationError) as ctx: + entry.full_clean() + self.assertIn("position", ctx.exception.error_dict) + + def test_teammembership_accepts_same_club(self): + TeamMembership(team=self.team, member=self.member, season=self.season, position=self.forward).full_clean() + + def test_staffassignment_rejects_cross_club_season(self): + assignment = StaffAssignment(team=self.team, member=self.member, season=self.other_season, position=self.coach) + with self.assertRaises(ValidationError) as ctx: + assignment.full_clean() + self.assertIn("season", ctx.exception.error_dict) + + def test_staffassignment_accepts_same_club(self): + StaffAssignment(team=self.team, member=self.member, season=self.season, position=self.coach).full_clean()