Separate guardians from members: a parent is not automatically a member
members/services/family.py enrolled a parent exactly like the child they were registering, so every parent held a full membership: counted in the member list, in the club and platform KPIs, and in the fee roll, with a fee record of their own. ClubMembership.kind (member | guardian) separates the two. A guardian is attached to the club only through their child. They hold the login, can be contacted and can sit in a Group -- the stated exception -- but they are not a member: no fee (clean() refuses one), absent from the member list, the fee list and every member count, and not eligible for a roster or a staff spot. A parent who also plays or coaches is a member who happens to be a parent; the two facts are independent, which is why this is its own field rather than inferred from FamilyMembership.role. A field on ClubMembership rather than a separate model because everything that answers "is this person attached to this club" already reads through that table -- tenancy, groups, the club-wide event audience -- and a second kind of link would need a parallel path through all of it. What changes is only who counts. Two things that weren't obvious going in: Excluding guardians had to be a subtraction, not a narrower filter. The obvious move -- match only member-kind rows and drop the MEMBER-role branch, since an active membership of any kind grants that role -- also hides someone the club knows but hasn't signed up for a season yet, which is a real state the member edit page supports. Two existing tests caught it. _guardians_only() subtracts instead, so anyone who also plays, is on staff or runs the club stays visible. Their tie to the club isn't seasonal but rides on a per-season row, so it has to be carried forward or a parent silently drops off at the season boundary while their child stays enrolled. Copied from the immediately preceding season only, so a deliberate removal isn't resurrected from an older row. The data migration reclassifies existing parents, deliberately skipping anyone who plays, is on a team's staff or holds an elevated ClubRole -- demoting them would strip them from their own team's roster eligibility. Anything ambiguous stays a member, which an admin can flip; noticing someone quietly vanished is much harder. The import template gains a membership_kind column next to family_role (a child marked guardian is refused), and the review screen shows what each row will join as. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
23
club/migrations/0022_clubmembership_kind.py
Normal file
23
club/migrations/0022_clubmembership_kind.py
Normal file
@@ -0,0 +1,23 @@
|
||||
# Generated by Django 6.0.6 on 2026-08-11 13:42
|
||||
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
('club', '0021_club_legal_name'),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AddField(
|
||||
model_name='clubmembership',
|
||||
name='kind',
|
||||
field=models.CharField(choices=[('member', 'member'), ('guardian', 'guardian')], default='member', help_text="A guardian is attached to the club only as a parent of a member -- they hold the login, but don't count as a member themselves and owe no fee. A parent who also plays is a member.", max_length=20, verbose_name='kind'),
|
||||
),
|
||||
migrations.AlterField(
|
||||
model_name='club',
|
||||
name='legal_name',
|
||||
field=models.CharField(blank=True, help_text='Full registered name (e.g. including a legal form like VZW/ASBL), used on official documents. Falls back to club name if blank.', max_length=255, verbose_name='legal name'),
|
||||
),
|
||||
]
|
||||
58
club/migrations/0023_backfill_guardian_memberships.py
Normal file
58
club/migrations/0023_backfill_guardian_memberships.py
Normal file
@@ -0,0 +1,58 @@
|
||||
"""Reclassify existing family parents as guardians.
|
||||
|
||||
Before ``kind`` existed, members/services/family.py enrolled a parent exactly like
|
||||
the child they were registering, so every parent already in the database holds a
|
||||
full membership and is counted as a member.
|
||||
|
||||
The guard matters more than the rule: anyone who is *also* on a roster, on a
|
||||
team's staff, or holds an elevated ClubRole is left as a member. A parent who
|
||||
plays, coaches or runs the club is a member who happens to have children there,
|
||||
and silently demoting them would strip them out of the member list and their own
|
||||
team's roster eligibility. Anything ambiguous stays as it is -- an admin can flip
|
||||
a membership to guardian by hand, which is cheap; noticing that someone quietly
|
||||
vanished is not.
|
||||
"""
|
||||
|
||||
from django.db import migrations
|
||||
|
||||
|
||||
def backfill_guardians(apps, schema_editor):
|
||||
ClubMembership = apps.get_model("club", "ClubMembership")
|
||||
FamilyMembership = apps.get_model("members", "FamilyMembership")
|
||||
TeamMembership = apps.get_model("teams", "TeamMembership")
|
||||
StaffAssignment = apps.get_model("teams", "StaffAssignment")
|
||||
ClubRole = apps.get_model("club", "ClubRole")
|
||||
|
||||
parent_ids = set(FamilyMembership.objects.filter(role__in=["parent", "guardian"]).values_list("member_id", flat=True))
|
||||
if not parent_ids:
|
||||
return
|
||||
|
||||
for membership in ClubMembership.objects.filter(member_id__in=parent_ids).iterator():
|
||||
club_id, member_id = membership.club_id, membership.member_id
|
||||
|
||||
plays = TeamMembership.objects.filter(member_id=member_id, team__club_id=club_id).exists()
|
||||
on_staff = StaffAssignment.objects.filter(member_id=member_id, team__club_id=club_id).exists()
|
||||
runs_the_club = ClubRole.objects.filter(member_id=member_id, club_id=club_id, role__in=["admin", "editor"]).exists()
|
||||
if plays or on_staff or runs_the_club:
|
||||
continue
|
||||
|
||||
membership.kind = "guardian"
|
||||
# Guardians owe nothing; clear any fee the old parent-as-member flow left behind.
|
||||
membership.fee_amount = 0
|
||||
membership.save(update_fields=["kind", "fee_amount"])
|
||||
|
||||
|
||||
def restore_members(apps, schema_editor):
|
||||
"""Everything was a member before this migration ran."""
|
||||
ClubMembership = apps.get_model("club", "ClubMembership")
|
||||
ClubMembership.objects.filter(kind="guardian").update(kind="member")
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
dependencies = [
|
||||
("club", "0022_clubmembership_kind"),
|
||||
("members", "0004_group_groupmembership_and_more"),
|
||||
("teams", "0009_team_referee_management"),
|
||||
]
|
||||
|
||||
operations = [migrations.RunPython(backfill_guardians, restore_members)]
|
||||
@@ -253,6 +253,10 @@ class Season(ClubScopedModel):
|
||||
|
||||
|
||||
class ClubMembership(ClubScopedModel):
|
||||
class Kind(models.TextChoices):
|
||||
MEMBER = "member", _("member")
|
||||
GUARDIAN = "guardian", _("guardian")
|
||||
|
||||
class StatusChoices(models.TextChoices):
|
||||
ACTIVE = "active", _("active")
|
||||
PENDING = "pending", _("pending")
|
||||
@@ -268,6 +272,14 @@ class ClubMembership(ClubScopedModel):
|
||||
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"))
|
||||
|
||||
kind = models.CharField(
|
||||
_("kind"),
|
||||
max_length=20,
|
||||
choices=Kind.choices,
|
||||
default=Kind.MEMBER,
|
||||
help_text=_("A guardian is attached to the club only as a parent of a member -- they hold the login, but don't count as a member themselves and owe no fee. A parent who also plays is a member."),
|
||||
)
|
||||
|
||||
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)
|
||||
@@ -289,8 +301,25 @@ class ClubMembership(ClubScopedModel):
|
||||
def __str__(self):
|
||||
return f"{self.club} - {self.member}"
|
||||
|
||||
@property
|
||||
def is_guardian(self) -> bool:
|
||||
"""Attached to the club as a parent of a member, not as one themselves.
|
||||
|
||||
Guardians are deliberately kept as ClubMembership rows rather than given
|
||||
their own model: everything that answers "is this person attached to this
|
||||
club" (tenancy scoping, group membership, the event audience) already
|
||||
reads through this table, and a second kind of link would need a parallel
|
||||
path through all of it. What changes is only who *counts* -- the member
|
||||
list, the fee list and every member KPI filter on ``kind``.
|
||||
"""
|
||||
return self.kind == self.Kind.GUARDIAN
|
||||
|
||||
def clean(self):
|
||||
validate_club_scope(self, self.club_id, same_club_fields=("season",))
|
||||
# A guardian owes nothing -- they're not a member. Caught here rather than
|
||||
# silently zeroed on save so a mistaken import row says so out loud.
|
||||
if self.is_guardian and self.fee_amount:
|
||||
raise ValidationError({"fee_amount": _("A guardian doesn't hold a membership, so they can't owe a fee.")})
|
||||
|
||||
|
||||
class FeePayment(UUIDModel):
|
||||
|
||||
@@ -20,7 +20,7 @@ from django.db.models import Q, QuerySet
|
||||
from django.utils import timezone
|
||||
|
||||
from authentication.models import User
|
||||
from club.models import Club, ClubRole, Season
|
||||
from club.models import Club, ClubMembership, ClubRole, Season
|
||||
from events.models import Event
|
||||
from members.models import FamilyMembership, Group, Member
|
||||
from teams.models import StaffAssignment, Team
|
||||
@@ -114,15 +114,41 @@ def teams_staffed_by(user: User, club: Club) -> QuerySet[Team]:
|
||||
).distinct()
|
||||
|
||||
|
||||
def members_visible_to(user: User, club: Club) -> QuerySet[Member]:
|
||||
def _guardians_only(club: Club) -> QuerySet[Member]:
|
||||
"""People whose *only* tie to ``club`` is being a parent of a member.
|
||||
|
||||
Subtracted rather than filtered out at the source, because a bare MEMBER
|
||||
ClubRole with no ClubMembership is a real state -- someone the club knows
|
||||
but hasn't signed up for a season yet -- and narrowing the role branch to
|
||||
weed guardians out would take those people with it. Anyone who also holds a
|
||||
real membership, plays, is on a team's staff or runs the club is a member
|
||||
who happens to be a parent, and stays visible.
|
||||
"""
|
||||
return Member.objects.filter(member_of__club=club, member_of__kind=ClubMembership.Kind.GUARDIAN).exclude(
|
||||
Q(member_of__club=club, member_of__kind=ClubMembership.Kind.MEMBER)
|
||||
| Q(team_memberships__team__club=club)
|
||||
| Q(staff_assignments__team__club=club)
|
||||
| Q(roles__club=club, roles__role__in=[ClubRole.Roles.ADMIN, ClubRole.Roles.EDITOR])
|
||||
)
|
||||
|
||||
|
||||
def members_visible_to(user: User, club: Club, *, include_guardians: bool = False) -> QuerySet[Member]:
|
||||
"""Members the user may see.
|
||||
|
||||
ADMIN: everyone linked to the club (membership, roster, staff or role).
|
||||
Otherwise: themselves, their children, and the current-season players *and*
|
||||
staff of every team they're staffed on.
|
||||
|
||||
Guardians -- parents attached to the club only through a child, see
|
||||
``ClubMembership.Kind`` -- are **excluded by default**: they aren't members,
|
||||
so they don't belong in a member list or any member count. Pass
|
||||
``include_guardians=True`` where the page is about a *person* rather than
|
||||
about members: opening a guardian's own detail page, editing them, putting
|
||||
them in a group, or showing a family (whose parents are the whole point).
|
||||
"""
|
||||
if is_club_admin(user, club):
|
||||
return Member.objects.filter(Q(member_of__club=club) | Q(team_memberships__team__club=club) | Q(staff_assignments__team__club=club) | Q(roles__club=club)).distinct()
|
||||
attached = Member.objects.filter(Q(member_of__club=club) | Q(team_memberships__team__club=club) | Q(staff_assignments__team__club=club) | Q(roles__club=club)).distinct()
|
||||
return attached if include_guardians else attached.exclude(pk__in=_guardians_only(club))
|
||||
|
||||
me = Member.objects.filter(user=user).first()
|
||||
if me is None:
|
||||
|
||||
@@ -46,12 +46,39 @@ def generate_seasons(club, 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:
|
||||
_carry_guardians_into(club, season)
|
||||
created.append(season)
|
||||
start = end + datetime.timedelta(days=1)
|
||||
|
||||
return created
|
||||
|
||||
|
||||
def _carry_guardians_into(club, season):
|
||||
"""Copy the previous season's guardians into a season that has just been created.
|
||||
|
||||
The other half of members.services.family.carry_guardians_forward, which
|
||||
covers a guardian added *after* the later seasons already existed. Between
|
||||
them a parent keeps their tie to the club across every season boundary --
|
||||
without which they would quietly drop off the club while their child stayed
|
||||
enrolled, which is exactly the state a guardian exists to prevent.
|
||||
|
||||
Copied from the immediately preceding season, not from "any season ever", so
|
||||
a guardian an admin deliberately removed stays removed rather than being
|
||||
resurrected from an older row.
|
||||
"""
|
||||
from club.models import ClubMembership
|
||||
|
||||
previous = Season.objects.filter(club=club, start_date__lt=season.start_date).order_by("-start_date").first()
|
||||
if previous is None:
|
||||
return
|
||||
|
||||
guardians = ClubMembership.objects.filter(club=club, season=previous, kind=ClubMembership.Kind.GUARDIAN)
|
||||
ClubMembership.objects.bulk_create(
|
||||
[ClubMembership(club=club, member_id=guardian.member_id, season=season, kind=ClubMembership.Kind.GUARDIAN, status=guardian.status, signed_up_at=guardian.signed_up_at) for guardian in guardians],
|
||||
ignore_conflicts=True,
|
||||
)
|
||||
|
||||
|
||||
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
|
||||
|
||||
@@ -19,6 +19,7 @@ from django.utils import timezone
|
||||
from events.models import Event
|
||||
from members.models import Family, FamilyMembership, Member
|
||||
from teams.models import Position, StaffAssignment, Team, TeamMembership
|
||||
from teams.services import eligible_roster_members
|
||||
|
||||
from .models import Club, ClubMembership, ClubRole, FeePayment, Season, Sponsor, club_logo_path
|
||||
from .services.access import (
|
||||
@@ -675,6 +676,94 @@ class ClubMembershipCleanTests(TestCase):
|
||||
ClubMembership(club=self.club, member=self.member, season=self.season).full_clean()
|
||||
|
||||
|
||||
class GuardianMembershipTests(TestCase):
|
||||
"""A parent attached to the club only through their child -- see
|
||||
ClubMembership.Kind. They hold the login and can be reached, but they are not
|
||||
a member: no fee, absent from every member list and count, and not eligible
|
||||
for a roster or staff spot."""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
cls.club = Club.objects.create(name="Ajax United", slug="ajax-united")
|
||||
today = timezone.localdate()
|
||||
cls.season = Season.objects.create(club=cls.club, start_date=today, end_date=today + datetime.timedelta(days=300))
|
||||
cls.child = Member.objects.create(first_name="Jamie", last_name="Doe")
|
||||
cls.parent = Member.objects.create(first_name="Taylor", last_name="Doe")
|
||||
ClubMembership.objects.create(club=cls.club, member=cls.child, season=cls.season, status=ClubMembership.StatusChoices.ACTIVE)
|
||||
cls.guardianship = ClubMembership.objects.create(club=cls.club, member=cls.parent, season=cls.season, kind=ClubMembership.Kind.GUARDIAN, status=ClubMembership.StatusChoices.ACTIVE)
|
||||
|
||||
cls.admin_user = get_user_model().objects.create_user(email="guardian-admin@example.com", password="pw")
|
||||
admin_member = Member.objects.create(user=cls.admin_user, first_name="Ada", last_name="Admin")
|
||||
ClubRole.objects.create(club=cls.club, member=admin_member, role=ClubRole.Roles.ADMIN)
|
||||
|
||||
def test_a_guardian_is_not_a_visible_member(self):
|
||||
visible = members_visible_to(self.admin_user, self.club)
|
||||
|
||||
self.assertIn(self.child, visible)
|
||||
self.assertNotIn(self.parent, visible)
|
||||
|
||||
def test_a_guardian_is_visible_when_explicitly_asked_for(self):
|
||||
# The group pickers and the person's own detail page ask for this: they're
|
||||
# about a person, not about the member list.
|
||||
visible = members_visible_to(self.admin_user, self.club, include_guardians=True)
|
||||
|
||||
self.assertIn(self.parent, visible)
|
||||
|
||||
def test_the_derived_member_role_does_not_leak_a_guardian_back_in(self):
|
||||
# An active membership of any kind grants a MEMBER ClubRole (club/signals.py),
|
||||
# so matching on "has any role in this club" would undo the exclusion.
|
||||
self.assertTrue(ClubRole.objects.filter(club=self.club, member=self.parent, role=ClubRole.Roles.MEMBER).exists())
|
||||
self.assertNotIn(self.parent, members_visible_to(self.admin_user, self.club))
|
||||
|
||||
def test_a_parent_who_also_plays_stays_a_member(self):
|
||||
# Being a parent and being a member are independent; the family graph
|
||||
# records the first, `kind` the second.
|
||||
self.guardianship.kind = ClubMembership.Kind.MEMBER
|
||||
self.guardianship.save(update_fields=["kind"])
|
||||
|
||||
self.assertIn(self.parent, members_visible_to(self.admin_user, self.club))
|
||||
|
||||
def test_a_guardian_who_is_also_on_a_roster_stays_visible(self):
|
||||
# The guardian row alone would hide them; playing for a team must not be
|
||||
# undone by their also being someone's parent.
|
||||
team = Team.objects.create(club=self.club, name="First Team", short_name="1st")
|
||||
position = Position.objects.create(club=self.club, name="Forward", short_name="FW")
|
||||
TeamMembership.objects.create(team=team, member=self.parent, season=self.season, position=position)
|
||||
|
||||
self.assertIn(self.parent, members_visible_to(self.admin_user, self.club))
|
||||
|
||||
def test_a_guardian_is_not_eligible_for_a_roster_or_staff_spot(self):
|
||||
self.assertIn(self.child, eligible_roster_members(self.club))
|
||||
self.assertNotIn(self.parent, eligible_roster_members(self.club))
|
||||
|
||||
def test_a_guardian_cannot_owe_a_fee(self):
|
||||
self.guardianship.fee_amount = Decimal("250.00")
|
||||
|
||||
with self.assertRaises(ValidationError) as ctx:
|
||||
self.guardianship.full_clean()
|
||||
|
||||
self.assertIn("fee_amount", ctx.exception.error_dict)
|
||||
|
||||
def test_a_new_season_carries_guardians_forward(self):
|
||||
# Their tie to the club isn't seasonal -- without this a parent silently
|
||||
# drops off at the season boundary while their child stays enrolled.
|
||||
generate_seasons(self.club, until=self.season.end_date + datetime.timedelta(days=400))
|
||||
|
||||
next_season = Season.objects.filter(club=self.club, start_date__gt=self.season.start_date).order_by("start_date").first()
|
||||
self.assertIsNotNone(next_season)
|
||||
self.assertTrue(ClubMembership.objects.filter(club=self.club, member=self.parent, season=next_season, kind=ClubMembership.Kind.GUARDIAN).exists())
|
||||
|
||||
def test_a_guardian_removed_from_the_latest_season_is_not_resurrected(self):
|
||||
# Copied from the season immediately before, not from "any season ever",
|
||||
# so a deliberate removal stays removed.
|
||||
self.guardianship.delete()
|
||||
|
||||
generate_seasons(self.club, until=self.season.end_date + datetime.timedelta(days=400))
|
||||
|
||||
next_season = Season.objects.filter(club=self.club, start_date__gt=self.season.start_date).order_by("start_date").first()
|
||||
self.assertFalse(ClubMembership.objects.filter(club=self.club, member=self.parent, season=next_season).exists())
|
||||
|
||||
|
||||
class AccessServiceTests(TestCase):
|
||||
# The clubs, season, teams and positions are pure scaffolding here -- every test
|
||||
# builds its *own* people and assignments on top of them -- so they are created once
|
||||
|
||||
Reference in New Issue
Block a user