From 744b6234038fdf67c4583e211d1ef1c049f947a5 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Tue, 11 Aug 2026 16:11:41 +0200 Subject: [PATCH] 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 --- ARCHITECTURE.md | 28 ++++ club/migrations/0022_clubmembership_kind.py | 23 +++ .../0023_backfill_guardian_memberships.py | 58 ++++++++ club/models.py | 29 ++++ club/services/access.py | 32 +++- club/services/seasons.py | 27 ++++ club/tests.py | 89 +++++++++++ controlpanel/services/statistics.py | 27 ++-- management/bulk_import.py | 34 ++++- management/forms.py | 13 +- .../templates/management/family_form.html | 1 + .../templates/management/member_detail.html | 26 +++- .../management/member_import_preview.html | 15 +- management/tests.py | 140 +++++++++++++++++- management/views.py | 31 ++-- members/services/family.py | 57 +++++-- teams/services.py | 4 + 17 files changed, 581 insertions(+), 53 deletions(-) create mode 100644 club/migrations/0022_clubmembership_kind.py create mode 100644 club/migrations/0023_backfill_guardian_memberships.py diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 383e966..7570243 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -404,6 +404,7 @@ Season(ClubScopedModel) # -> carries `club` ClubMembership(ClubScopedModel) # -> carries `club` member FK Member (CASCADE, related_name="club_memberships") season FK Season (PROTECT, related_name="memberships") + kind CharField (TextChoices: member | guardian) # default member license CharField (blank) # federation license for that season status CharField (TextChoices: pending | active | lapsed | cancelled) fee_status CharField (TextChoices: unpaid | partial | paid | waived) @@ -412,6 +413,33 @@ ClubMembership(ClubScopedModel) # -> carries `club` Meta: unique_together (club, member, season); ordering = ["-season__start_date", ...] ``` +- **`kind` separates a member from a guardian** *(built)*. A guardian is a parent attached + to the club only through their child: they hold the login, can be contacted and can sit in + a Group, but they are **not a member** — no fee, absent from the member list, the fee list + and every member KPI (club + platform), 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 being inferred from + `members.FamilyMembership.role`. Before this existed, `members/services/family.py` enrolled + a parent exactly like the child, so every parent counted as a member — a data migration + reclassifies them, deliberately skipping anyone who plays, is on a team's staff, or holds + an elevated ClubRole. +- **Why a field and not a separate model.** Everything that answers "is this person attached + to this club" already reads through `ClubMembership` — tenancy scoping, group membership, + 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*. +- **Guardians are carried forward across seasons.** Their tie to the club isn't really + seasonal (it lasts as long as the child is there) but it rides on a per-season row, so + `club/services/seasons.py::_carry_guardians_into` copies them when a season is created and + `members/services/family.py::carry_guardians_forward` covers a guardian added after later + seasons already existed. Copied from the *immediately preceding* season only, so a guardian + an admin deliberately removed stays removed rather than being resurrected from an old row. +- **Excluding guardians is a subtraction, not a narrower filter.** + `club/services/access.py::members_visible_to` subtracts `_guardians_only(club)` rather than + matching only member-kind rows: a bare MEMBER `ClubRole` with no `ClubMembership` is a real + state (someone the club knows but hasn't signed up yet), and narrowing the role branch to + weed guardians out would take those people with it. Pass `include_guardians=True` where the + page is about a *person* rather than about members — a guardian's own detail page, editing + them, the group pickers, or a family page (whose parents are the whole point). - One row per member **per season** — sign-up and fee payment are tracked independently each season. `unique_together` moves from `(club, member)` → `(club, member, season)` (a data migration must backfill existing rows with the current season). diff --git a/club/migrations/0022_clubmembership_kind.py b/club/migrations/0022_clubmembership_kind.py new file mode 100644 index 0000000..c82b290 --- /dev/null +++ b/club/migrations/0022_clubmembership_kind.py @@ -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'), + ), + ] diff --git a/club/migrations/0023_backfill_guardian_memberships.py b/club/migrations/0023_backfill_guardian_memberships.py new file mode 100644 index 0000000..ca00275 --- /dev/null +++ b/club/migrations/0023_backfill_guardian_memberships.py @@ -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)] diff --git a/club/models.py b/club/models.py index 8002828..99f32d5 100644 --- a/club/models.py +++ b/club/models.py @@ -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): diff --git a/club/services/access.py b/club/services/access.py index 403f0d8..95faa68 100644 --- a/club/services/access.py +++ b/club/services/access.py @@ -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: diff --git a/club/services/seasons.py b/club/services/seasons.py index fb31fa1..03863ce 100644 --- a/club/services/seasons.py +++ b/club/services/seasons.py @@ -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 diff --git a/club/tests.py b/club/tests.py index 4d4018d..806c98a 100644 --- a/club/tests.py +++ b/club/tests.py @@ -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 diff --git a/controlpanel/services/statistics.py b/controlpanel/services/statistics.py index 0c43d92..b71b515 100644 --- a/controlpanel/services/statistics.py +++ b/controlpanel/services/statistics.py @@ -41,7 +41,10 @@ def clubs_with_totals(queryset=None): """Clubs annotated with headline counts (one query, no N+1).""" clubs = Club.objects.all() if queryset is None else queryset return clubs.annotate( - member_count=Count("clubmemberships__member", distinct=True), + # Members only: a guardian is attached to the club as a parent of a member, + # not as one, so counting them would overstate every club's size (and the + # onboarding funnel's "with members" step). + member_count=Count("clubmemberships__member", filter=Q(clubmemberships__kind=ClubMembership.Kind.MEMBER), distinct=True), team_count=Count("teams", distinct=True), event_count=Count("events", distinct=True), admin_count=Count("clubroles", filter=Q(clubroles__role=ClubRole.Roles.ADMIN), distinct=True), @@ -77,8 +80,8 @@ def clubs_with_health(queryset=None, today=None, now=None): return ( clubs.annotate( has_season=Exists(Season.objects.filter(club=OuterRef("pk"), start_date__lte=today, end_date__gte=today)), - active_members=_subquery(ClubMembership.objects.filter(in_season, status=ClubMembership.StatusChoices.ACTIVE), Count("pk"), IntegerField()), - unpaid_members=_subquery(ClubMembership.objects.filter(in_season, fee_status=ClubMembership.FeeStatus.UNPAID), Count("pk"), IntegerField()), + active_members=_subquery(ClubMembership.objects.filter(in_season, kind=ClubMembership.Kind.MEMBER, status=ClubMembership.StatusChoices.ACTIVE), Count("pk"), IntegerField()), + unpaid_members=_subquery(ClubMembership.objects.filter(in_season, kind=ClubMembership.Kind.MEMBER, fee_status=ClubMembership.FeeStatus.UNPAID), Count("pk"), IntegerField()), outstanding=_subquery(Order.objects.filter(status__in=OWED_STATUSES), Sum("total"), DecimalField(max_digits=10, decimal_places=2)), upcoming_events=_subquery(Event.objects.filter(start__gte=now, start__lte=now + timedelta(days=DORMANT_DAYS)), Count("pk"), IntegerField()), team_count=_subquery(Team.objects.all(), Count("pk"), IntegerField()), @@ -105,7 +108,7 @@ def platform_totals(): return { "clubs": Club.objects.active().count(), "archived_clubs": Club.objects.archived().count(), - "members": Member.objects.count(), + "members": Member.objects.filter(member_of__kind=ClubMembership.Kind.MEMBER).distinct().count(), "admins": ClubRole.objects.filter(role=ClubRole.Roles.ADMIN).count(), } @@ -264,12 +267,12 @@ def renewal_rate(club, season): if previous is None: return None - was_active = ClubMembership.objects.filter(club=club, season=previous, status=ClubMembership.StatusChoices.ACTIVE) + was_active = ClubMembership.objects.filter(club=club, season=previous, kind=ClubMembership.Kind.MEMBER, status=ClubMembership.StatusChoices.ACTIVE) total = was_active.count() if not total: return None - returned = ClubMembership.objects.filter(club=club, season=season, member__in=was_active.values("member")).count() + returned = ClubMembership.objects.filter(club=club, season=season, kind=ClubMembership.Kind.MEMBER, member__in=was_active.values("member")).count() return round(100 * returned / total) @@ -284,7 +287,7 @@ def new_members(club, season): if season is None: return Member.objects.none() - seen_before = ClubMembership.objects.filter(club=club, season__start_date__lt=season.start_date).values("member") + seen_before = ClubMembership.objects.filter(club=club, season__start_date__lt=season.start_date, kind=ClubMembership.Kind.MEMBER).values("member") return Member.objects.filter(member_of__club=club, member_of__season=season).exclude(pk__in=seen_before).distinct() @@ -301,7 +304,7 @@ def signup_split(club=None, months=MONTHS_OF_HISTORY): """ start = (timezone.now() - relativedelta(months=months)).replace(day=1, hour=0, minute=0, second=0, microsecond=0) - memberships = ClubMembership.objects.all() if club is None else ClubMembership.objects.filter(club=club) + memberships = ClubMembership.objects.filter(kind=ClubMembership.Kind.MEMBER) if club is None else ClubMembership.objects.filter(club=club, kind=ClubMembership.Kind.MEMBER) first_season = {} for club_id, member_id, season_start in memberships.values_list("club_id", "member_id", "season__start_date"): @@ -342,7 +345,7 @@ def unrostered_members(club, season): rostered = TeamMembership.objects.filter(team__club=club, season=season).values("member") - return Member.objects.filter(member_of__club=club, member_of__season=season, member_of__status=ClubMembership.StatusChoices.ACTIVE).exclude(pk__in=rostered).distinct() + return Member.objects.filter(member_of__club=club, member_of__season=season, member_of__kind=ClubMembership.Kind.MEMBER, member_of__status=ClubMembership.StatusChoices.ACTIVE).exclude(pk__in=rostered).distinct() def fee_aging(club): @@ -389,7 +392,7 @@ def attendance_rates(club, season): def club_attention(club): """A club's own numbers that are supposed to be zero.""" season = Season.covering(club, timezone.localdate()) - memberships = ClubMembership.objects.filter(club=club) + memberships = ClubMembership.objects.filter(club=club, kind=ClubMembership.Kind.MEMBER) return { "season": season, @@ -408,7 +411,7 @@ def club_attention(club): def club_charts(club): season = Season.covering(club, timezone.localdate()) - memberships = ClubMembership.objects.filter(club=club, season=season) if season else ClubMembership.objects.none() + memberships = ClubMembership.objects.filter(club=club, season=season, kind=ClubMembership.Kind.MEMBER) if season else ClubMembership.objects.none() return { "signups": signup_split(club), @@ -430,7 +433,7 @@ def club_statistics(club): season = Season.covering(club, timezone.localdate()) now = timezone.now() - memberships = ClubMembership.objects.filter(club=club) + memberships = ClubMembership.objects.filter(club=club, kind=ClubMembership.Kind.MEMBER) events = Event.objects.filter(club=club) orders = Order.objects.filter(club=club) diff --git a/management/bulk_import.py b/management/bulk_import.py index d0efc85..a042aa8 100644 --- a/management/bulk_import.py +++ b/management/bulk_import.py @@ -19,17 +19,23 @@ from members.models import FamilyMembership, Member from .forms import MemberForm -TEMPLATE_COLUMNS = ["first_name", "last_name", "date_of_birth", "email", "phone", "emergency_phone", "license", "status", "fee_status", "family_group", "family_role"] +TEMPLATE_COLUMNS = ["first_name", "last_name", "date_of_birth", "email", "phone", "emergency_phone", "license", "status", "fee_status", "family_group", "family_role", "membership_kind"] REQUIRED_HEADER_COLUMNS = {"first_name", "last_name"} TEMPLATE_EXAMPLE_ROWS = [ # A standalone member -- no family link. - ["Alex", "Morgan", date(2012, 5, 14), "alex.morgan@example.com", "+32470123456", "+32470654321", "", ClubMembership.StatusChoices.ACTIVE, ClubMembership.FeeStatus.UNPAID, "", ""], + ["Alex", "Morgan", date(2012, 5, 14), "alex.morgan@example.com", "+32470123456", "+32470654321", "", ClubMembership.StatusChoices.ACTIVE, ClubMembership.FeeStatus.UNPAID, "", "", ClubMembership.Kind.MEMBER], # A parent and child linked together: same family_group value, one row each. # The child has no email of its own -- it gets a login only if given one via # the "Grant login" action later, same as adding a family by hand. - ["Taylor", "Doe", "", "taylor.doe@example.com", "+32470654322", "", "", ClubMembership.StatusChoices.ACTIVE, ClubMembership.FeeStatus.UNPAID, "Doe family", FamilyMembership.FamilyRole.PARENT], - ["Jamie", "Doe", date(2014, 3, 2), "", "", "+32470654322", "", ClubMembership.StatusChoices.ACTIVE, ClubMembership.FeeStatus.UNPAID, "Doe family", FamilyMembership.FamilyRole.CHILD], + # + # membership_kind sits next to family_role because it answers the question + # family_role raises: this parent is a `guardian`, so they hold the login and + # can be contacted but don't count as a member and owe no fee. Put `member` + # there instead for a parent who also plays -- the two are independent, which + # is why it is its own column rather than inferred from family_role. + ["Taylor", "Doe", "", "taylor.doe@example.com", "+32470654322", "", "", ClubMembership.StatusChoices.ACTIVE, "", "Doe family", FamilyMembership.FamilyRole.PARENT, ClubMembership.Kind.GUARDIAN], + ["Jamie", "Doe", date(2014, 3, 2), "", "", "+32470654322", "", ClubMembership.StatusChoices.ACTIVE, ClubMembership.FeeStatus.UNPAID, "Doe family", FamilyMembership.FamilyRole.CHILD, ClubMembership.Kind.MEMBER], ] @@ -48,7 +54,7 @@ def build_member_import_template(): for example_row in TEMPLATE_EXAMPLE_ROWS: sheet.append(example_row) - for column_name, choices in (("status", ClubMembership.StatusChoices), ("fee_status", ClubMembership.FeeStatus), ("family_role", FamilyMembership.FamilyRole)): + for column_name, choices in (("membership_kind", ClubMembership.Kind), ("status", ClubMembership.StatusChoices), ("fee_status", ClubMembership.FeeStatus), ("family_role", FamilyMembership.FamilyRole)): column_index = TEMPLATE_COLUMNS.index(column_name) + 1 column_letter = sheet.cell(row=1, column=column_index).column_letter options = ",".join(choices.values) @@ -140,6 +146,12 @@ def parse_member_import_rows(rows, club): family_group, family_role, family_errors = _parse_family_fields(raw) errors.extend(family_errors) + # The two columns are otherwise independent -- a parent may or may not also + # be a member -- but a child is the member the guardian is attached *to*, + # so that particular combination is always a mistake. + if family_role == FamilyMembership.FamilyRole.CHILD and membership_kwargs["kind"] == ClubMembership.Kind.GUARDIAN: + errors.append(_("A child is always a member, so membership_kind cannot be 'guardian'.")) + results.append( { "line_number": line_number, @@ -159,6 +171,16 @@ def _parse_membership_fields(raw): errors = [] license_number = raw.get("license", "").strip() + kind = raw.get("membership_kind", "").strip() + if kind: + kind_value = _match_choice(kind, ClubMembership.Kind) + if kind_value is None: + errors.append(_("Invalid membership_kind '%(value)s'.") % {"value": kind}) + else: + # Blank means member: the overwhelmingly common row, and the value every + # file written before this column existed effectively carried. + kind_value = ClubMembership.Kind.MEMBER + status = raw.get("status", "").strip() if status: status_value = _match_choice(status, ClubMembership.StatusChoices) @@ -175,7 +197,7 @@ def _parse_membership_fields(raw): else: fee_status_value = ClubMembership.FeeStatus.UNPAID - return {"license": license_number, "status": status_value, "fee_status": fee_status_value}, errors + return {"kind": kind_value, "license": license_number, "status": status_value, "fee_status": fee_status_value}, errors def _parse_family_fields(raw): diff --git a/management/forms.py b/management/forms.py index 5bcfc90..35743c2 100644 --- a/management/forms.py +++ b/management/forms.py @@ -707,6 +707,12 @@ class FamilyCreateForm(forms.Form): parent_last_name = forms.CharField(label=_("Parent last name")) parent_email = forms.EmailField(label=_("Parent email"), help_text=_("If this email has no account yet, one is created and they set a password via the reset link.")) + parent_is_member = forms.BooleanField( + label=_("Parent is also a member"), + required=False, + help_text=_("Tick only if this parent belongs to the club in their own right (they play, or hold a membership). Left unticked they are a guardian: they hold the login and can be contacted, but owe no fee and are not counted as a member."), + ) + child_first_name = forms.CharField(label=_("Child first name")) child_last_name = forms.CharField(label=_("Child last name")) child_date_of_birth = forms.DateField(label=_("Child date of birth"), required=False, widget=forms.DateInput(attrs={"type": "date"})) @@ -728,6 +734,11 @@ class AddParentForm(forms.Form): email = forms.EmailField(label=_("Email address"), help_text=_("If this email has no account yet, one is created and they set a password via the reset link.")) first_name = forms.CharField(label=_("First name"), required=False) last_name = forms.CharField(label=_("Last name"), required=False) + parent_is_member = forms.BooleanField( + label=_("Also a member"), + required=False, + help_text=_("Tick only if this parent belongs to the club in their own right (they play, or hold a membership). Left unticked they are a guardian: they hold the login and can be contacted, but owe no fee and are not counted as a member."), + ) def clean(self): cleaned = super().clean() @@ -790,7 +801,7 @@ class ClubMembershipForm(forms.ModelForm): class Meta: model = ClubMembership - fields = ["license", "status", "fee_status", "fee_amount"] + fields = ["kind", "license", "status", "fee_status", "fee_amount"] class NewsForm(forms.ModelForm): diff --git a/management/templates/management/family_form.html b/management/templates/management/family_form.html index c6eb451..dc4e077 100644 --- a/management/templates/management/family_form.html +++ b/management/templates/management/family_form.html @@ -21,6 +21,7 @@ {% form_field form.parent_first_name %} {% form_field form.parent_last_name %} {% form_field form.parent_email %} + {% form_field form.parent_is_member %}
diff --git a/management/templates/management/member_detail.html b/management/templates/management/member_detail.html index 5ce7315..06ac8a3 100644 --- a/management/templates/management/member_detail.html +++ b/management/templates/management/member_detail.html @@ -79,7 +79,23 @@

{% lucide "id-card" size=18 %} {% trans "Club membership" %}

{% if current_membership %} + {% if current_membership.is_guardian %} + {% comment %} + Stated plainly rather than left to be inferred from an empty + fee: a guardian is deliberately missing from the member list + and every member count, and someone looking at this page needs + to know that is on purpose. See club.models.ClubMembership.Kind. + {% endcomment %} +
+ {% lucide "users" size=16 %} + {% trans "Guardian: attached to the club as a parent of a member. Holds the login, owes no fee, and is not counted as a member." %} +
+ {% endif %}
+
+
{% trans "Joined as" %}
+
{{ current_membership.get_kind_display|capfirst }}
+
{% trans "License" %}
{{ current_membership.license|default:"-" }}
@@ -88,10 +104,12 @@
{% trans "Status" %}
{{ current_membership.get_status_display }}
-
-
{% trans "Fee status" %}
-
{{ current_membership.get_fee_status_display }}
-
+ {% if not current_membership.is_guardian %} +
+
{% trans "Fee status" %}
+
{{ current_membership.get_fee_status_display }}
+
+ {% endif %}
{% else %}

{% trans "Not rostered for the current season." %}

diff --git a/management/templates/management/member_import_preview.html b/management/templates/management/member_import_preview.html index 1a6f4e1..18c9121 100644 --- a/management/templates/management/member_import_preview.html +++ b/management/templates/management/member_import_preview.html @@ -27,6 +27,7 @@ {% trans "First name" %} {% trans "Email" %} {% trans "Family" %} + {% trans "Joining as" %} {% trans "Outcome" %} @@ -45,6 +46,18 @@ - {% endif %} + + {% comment %} + A guardian holds the login for a child but is not a member: + no fee, and not counted in any member total. See + club.models.ClubMembership.Kind. + {% endcomment %} + {% if result.membership_kwargs.kind == "guardian" %} + {% trans "Guardian" %} + {% else %} + {% trans "Member" %} + {% endif %} + {% if result.member %} {% trans "Will create" %} @@ -56,7 +69,7 @@ {% empty %} - {% trans "No rows found in the uploaded file." %} + {% trans "No rows found in the uploaded file." %} {% endfor %} diff --git a/management/tests.py b/management/tests.py index b163e96..d9cae01 100644 --- a/management/tests.py +++ b/management/tests.py @@ -246,7 +246,7 @@ class MemberManagementTests(ManagementTestBase): # one combined submit, so its (required) fields must come along. self.club_post( "member_update", - {"first_name": "New", "last_name": "Name", "status": ClubMembership.StatusChoices.ACTIVE, "fee_status": ClubMembership.FeeStatus.UNPAID}, + {"first_name": "New", "last_name": "Name", "kind": ClubMembership.Kind.MEMBER, "status": ClubMembership.StatusChoices.ACTIVE, "fee_status": ClubMembership.FeeStatus.UNPAID}, member.pk, ) @@ -1363,6 +1363,95 @@ class FamilyManagementTests(ManagementTestBase): self.assertEqual(family.guardians.count(), 1) +class GuardianViewTests(ManagementTestBase): + """How a guardian -- a parent attached to the club only through their child -- + behaves across the management UI. See club.models.ClubMembership.Kind; the + model-level guarantees live in club.tests.GuardianMembershipTests.""" + + def family_payload(self, **overrides): + payload = { + "parent_first_name": "Pat", + "parent_last_name": "Parent", + "parent_email": "pat.parent@example.com", + "child_first_name": "Cody", + "child_last_name": "Child", + "child_date_of_birth": "2015-04-01", + } + payload.update(overrides) + return payload + + def test_registering_a_family_makes_the_parent_a_guardian_and_the_child_a_member(self): + self.client.force_login(self.admin_user) + + self.club_post("family_create", self.family_payload()) + + parent = Member.objects.get(first_name="Pat", last_name="Parent") + child = Member.objects.get(first_name="Cody", last_name="Child") + self.assertEqual(ClubMembership.objects.get(club=self.club, member=parent).kind, ClubMembership.Kind.GUARDIAN) + self.assertEqual(ClubMembership.objects.get(club=self.club, member=child).kind, ClubMembership.Kind.MEMBER) + + def test_ticking_also_a_member_enrols_the_parent_as_one(self): + self.client.force_login(self.admin_user) + + self.club_post("family_create", self.family_payload(parent_is_member="on")) + + parent = Member.objects.get(first_name="Pat", last_name="Parent") + self.assertEqual(ClubMembership.objects.get(club=self.club, member=parent).kind, ClubMembership.Kind.MEMBER) + + def test_a_guardian_is_absent_from_the_member_list(self): + self.client.force_login(self.admin_user) + self.club_post("family_create", self.family_payload()) + + response = self.club_get("member_list") + + self.assertContains(response, "Cody Child") + self.assertNotContains(response, "Pat Parent") + + def test_a_guardian_is_not_counted_in_the_membership_kpis(self): + # Measured as a delta, not an absolute: the base fixture's admin is a + # member too, and this is about what registering a family *adds*. + self.client.force_login(self.admin_user) + before = self.club_get("membership_list").context["kpi_total"] + + self.club_post("family_create", self.family_payload()) + + after = self.club_get("membership_list").context["kpi_total"] + # The child only. A guardian owes nothing, so counting them would + # overstate the roll and everything derived from it. + self.assertEqual(after - before, 1) + + def test_a_guardians_own_page_is_still_reachable(self): + # Excluded from the member *list*, not from the club: an admin has to be + # able to open them, edit them, and switch them to a member. + self.client.force_login(self.admin_user) + self.club_post("family_create", self.family_payload()) + parent = Member.objects.get(first_name="Pat", last_name="Parent") + + self.assertEqual(self.club_get("member_detail", parent.pk).status_code, 200) + self.assertEqual(self.club_get("member_update", parent.pk).status_code, 200) + + def test_a_guardian_can_still_be_put_in_a_group(self): + # The stated exception: a parent may well sit on a committee. + self.client.force_login(self.admin_user) + self.club_post("family_create", self.family_payload()) + parent = Member.objects.get(first_name="Pat", last_name="Parent") + group = Group.objects.create(club=self.club, name="Committee") + + response = self.club_get("group_bulk_add", group.pk) + + self.assertContains(response, str(parent.pk)) + + def test_a_guardian_is_not_offered_for_a_team_roster(self): + self.client.force_login(self.admin_user) + self.club_post("family_create", self.family_payload()) + parent = Member.objects.get(first_name="Pat", last_name="Parent") + team = Team.objects.create(club=self.club, name="First Team", short_name="1st") + + response = self.club_get("team_bulk_add", team.pk, self.season.pk) + + self.assertNotContains(response, str(parent.pk)) + + class MemberListFamilyColumnTests(ManagementTestBase): """The member list is one flat table -- family is a column (each member's family/ role attached in Python, management.views.MemberListView), not a grouping.""" @@ -1878,6 +1967,7 @@ class MemberClubMembershipFormTests(ManagementTestBase): { "first_name": "Fee", "last_name": "Payer", + "kind": ClubMembership.Kind.MEMBER, "license": "BE-9999", "status": ClubMembership.StatusChoices.ACTIVE, "fee_status": ClubMembership.FeeStatus.PAID, @@ -1912,7 +2002,7 @@ class MemberClubMembershipFormTests(ManagementTestBase): response = self.club_post( "member_update", - {"first_name": "Unrostered", "last_name": "Member", "status": ClubMembership.StatusChoices.ACTIVE, "fee_status": ClubMembership.FeeStatus.PAID}, + {"first_name": "Unrostered", "last_name": "Member", "kind": ClubMembership.Kind.MEMBER, "status": ClubMembership.StatusChoices.ACTIVE, "fee_status": ClubMembership.FeeStatus.PAID}, member.pk, ) @@ -2893,6 +2983,52 @@ class MemberBulkImportTests(ManagementTestBase): self.assertEqual(FamilyMembership.objects.get(family=family, member=parent).role, FamilyMembership.FamilyRole.PARENT) self.assertEqual(FamilyMembership.objects.get(family=family, member=child).role, FamilyMembership.FamilyRole.CHILD) + def test_membership_kind_guardian_creates_a_guardian_not_a_member(self): + # Columns are positional; membership_kind is the last one. + upload = make_import_workbook( + [ + ["Taylor", "Doe", "", "taylor.guardian@example.com", "", "", "", "", "", "Doe family", "parent", "guardian"], + ["Jamie", "Doe", "2014-03-02", "", "", "", "", "", "", "Doe family", "child", "member"], + ] + ) + self.club_post("member_import", {"file": upload}) + + self.club_post("member_import_confirm", {}) + + parent = Member.objects.get(email="taylor.guardian@example.com") + child = Member.objects.get(first_name="Jamie", last_name="Doe") + self.assertEqual(ClubMembership.objects.get(club=self.club, member=parent).kind, ClubMembership.Kind.GUARDIAN) + self.assertEqual(ClubMembership.objects.get(club=self.club, member=child).kind, ClubMembership.Kind.MEMBER) + + def test_a_blank_membership_kind_still_means_member(self): + # Every file written before the column existed carried this implicitly. + upload = make_import_workbook([["Solo", "Blankkind", "", "solo.blank@example.com", "", "", "", "", "", "", "", ""]]) + self.club_post("member_import", {"file": upload}) + + self.club_post("member_import_confirm", {}) + + member = Member.objects.get(email="solo.blank@example.com") + self.assertEqual(ClubMembership.objects.get(club=self.club, member=member).kind, ClubMembership.Kind.MEMBER) + + def test_a_child_marked_as_a_guardian_is_an_error(self): + # A child is the member the guardian is attached *to*. + upload = make_import_workbook([["Jamie", "Doe", "2014-03-02", "", "", "", "", "", "", "Doe family", "child", "guardian"]]) + + response = self.club_post("member_import", {"file": upload}) + + result = response.context["results"][0] + self.assertIsNone(result["member"]) + self.assertTrue(any("child is always a member" in error.lower() for error in result["errors"])) + + def test_an_invalid_membership_kind_is_reported(self): + upload = make_import_workbook([["Odd", "Kind", "", "odd.kind@example.com", "", "", "", "", "", "", "", "sponsor"]]) + + response = self.club_post("member_import", {"file": upload}) + + result = response.context["results"][0] + self.assertIsNone(result["member"]) + self.assertTrue(any("membership_kind" in error for error in result["errors"])) + def test_family_role_without_a_group_is_an_error(self): upload = make_import_workbook([["Odd", "Row", "", "odd.row@example.com", "", "", "", "", "", "", "parent"]]) diff --git a/management/views.py b/management/views.py index 4a20093..70eabc2 100644 --- a/management/views.py +++ b/management/views.py @@ -239,7 +239,9 @@ class MembershipListView(ClubAdminRequiredMixin, ListView): if season is None: return ClubMembership.objects.none() - memberships = ClubMembership.objects.filter(club=self.request.club, season=season).select_related("member").order_by("member__last_name", "member__first_name") + # kind=MEMBER: a guardian holds no membership and owes no fee, so they + # belong in neither this list nor the KPIs below it. + memberships = ClubMembership.objects.filter(club=self.request.club, season=season, kind=ClubMembership.Kind.MEMBER).select_related("member").order_by("member__last_name", "member__first_name") fee_status = self.request.GET.get("fee_status", "not_paid") if fee_status == "not_paid": @@ -279,7 +281,7 @@ class MembershipListView(ClubAdminRequiredMixin, ListView): counts = {} if current is not None: - counts = {row["fee_status"]: row["count"] for row in ClubMembership.objects.filter(club=club, season=current).values("fee_status").annotate(count=Count("id"))} + counts = {row["fee_status"]: row["count"] for row in ClubMembership.objects.filter(club=club, season=current, kind=ClubMembership.Kind.MEMBER).values("fee_status").annotate(count=Count("id"))} paid = counts.get(ClubMembership.FeeStatus.PAID, 0) partial = counts.get(ClubMembership.FeeStatus.PARTIALLY_PAID, 0) unpaid = counts.get(ClubMembership.FeeStatus.UNPAID, 0) @@ -569,7 +571,7 @@ class MemberUpdateView(ClubAdminRequiredMixin, View): template_name = "management/member_form.html" def get_member(self): - return get_object_or_404(members_visible_to(self.request.user, self.request.club), pk=self.kwargs["pk"]) + return get_object_or_404(members_visible_to(self.request.user, self.request.club, include_guardians=True), pk=self.kwargs["pk"]) def get_membership(self, member): season = current_season(self.request.club) @@ -607,10 +609,10 @@ class MemberDetailView(ClubStaffRequiredMixin, DetailView): context_object_name = "member" def get_queryset(self): - return members_visible_to(self.request.user, self.request.club) + return members_visible_to(self.request.user, self.request.club, include_guardians=True) def get_context_data(self, **kwargs): - visible = members_visible_to(self.request.user, self.request.club) + visible = members_visible_to(self.request.user, self.request.club, include_guardians=True) my_family_ids = FamilyMembership.objects.filter(member=self.object).values_list("family_id", flat=True) family_scoped_members = visible.filter(family_memberships__family_id__in=my_family_ids).distinct() family_groups, _ = group_by_family(family_scoped_members) @@ -647,11 +649,11 @@ class MemberAttachToFamilyView(ClubAdminRequiredMixin, RedirectOnInvalidMixin, F return {"pk": self.kwargs["pk"]} def get_form_kwargs(self): - member = get_object_or_404(members_visible_to(self.request.user, self.request.club), pk=self.kwargs["pk"]) + member = get_object_or_404(members_visible_to(self.request.user, self.request.club, include_guardians=True), pk=self.kwargs["pk"]) return super().get_form_kwargs() | {"club": self.request.club, "member": member} def form_valid(self, form): - member = get_object_or_404(members_visible_to(self.request.user, self.request.club), pk=self.kwargs["pk"]) + member = get_object_or_404(members_visible_to(self.request.user, self.request.club, include_guardians=True), pk=self.kwargs["pk"]) family = attach_to_family(member, role=form.cleaned_data["role"], family=form.cleaned_data["family"]) body = _("“%(member)s” is now part of %(family)s.") % {"member": member, "family": family} notify(self.request, f"s|{_('Added to family')}|{body}") @@ -671,7 +673,7 @@ class MemberRefereeEligibilityUpdateView(ClubAdminRequiredMixin, RedirectOnInval return {"pk": self.kwargs["pk"]} def get_member(self): - return get_object_or_404(members_visible_to(self.request.user, self.request.club), pk=self.kwargs["pk"]) + return get_object_or_404(members_visible_to(self.request.user, self.request.club, include_guardians=True), pk=self.kwargs["pk"]) def get_form_kwargs(self): return super().get_form_kwargs() | {"club": self.request.club, "member": self.get_member()} @@ -697,7 +699,7 @@ class MemberGrantLoginView(ClubAdminRequiredMixin, RedirectOnInvalidMixin, FormV return {"pk": self.kwargs["pk"]} def form_valid(self, form): - member = get_object_or_404(members_visible_to(self.request.user, self.request.club), pk=self.kwargs["pk"]) + member = get_object_or_404(members_visible_to(self.request.user, self.request.club, include_guardians=True), pk=self.kwargs["pk"]) if member.user_id is not None: # Already has one -- the row's button shouldn't have been there at all; # a direct POST replay (e.g. a resubmitted form) is the only way here. @@ -710,7 +712,7 @@ class MemberGrantLoginView(ClubAdminRequiredMixin, RedirectOnInvalidMixin, FormV class MemberDetachFromFamilyView(ClubAdminRequiredMixin, View): def post(self, request, pk, family_pk): - member = get_object_or_404(members_visible_to(request.user, request.club), pk=pk) + member = get_object_or_404(members_visible_to(request.user, request.club, include_guardians=True), pk=pk) family = get_object_or_404(Family, pk=family_pk, memberships__member=member) # detach_from_family may delete `family` itself (left empty) -- str() it first, # since Family.__str__ queries self.memberships, which needs a pk to still exist. @@ -752,7 +754,7 @@ class FamilyMembershipRoleUpdateView(ClubAdminRequiredMixin, View): class MemberDeleteView(ClubAdminRequiredMixin, View): def post(self, request, pk): - member = get_object_or_404(members_visible_to(request.user, request.club), pk=pk) + member = get_object_or_404(members_visible_to(request.user, request.club, include_guardians=True), pk=pk) name = str(member) # FamilyMembership cascades away with the member -- note which families # they were in before that happens, so an emptied one can be cleaned up @@ -1344,6 +1346,7 @@ class FamilyCreateView(ClubAdminRequiredMixin, FormView): child_first_name=cd["child_first_name"], child_last_name=cd["child_last_name"], child_date_of_birth=cd["child_date_of_birth"], + parent_is_member=cd["parent_is_member"], ) if season is not None: @@ -1372,7 +1375,7 @@ class FamilyDetailView(ClubStaffRequiredMixin, DetailView): return families_of_club(self.request.club) def get_context_data(self, **kwargs): - visible = members_visible_to(self.request.user, self.request.club) + visible = members_visible_to(self.request.user, self.request.club, include_guardians=True) members = visible.filter(family_memberships__family=self.object).distinct() # group_by_family scopes by member, not family -- a member visible here # because they're in *this* family can also belong to another one, in @@ -1541,7 +1544,7 @@ class RefereeListView(ClubStaffRequiredMixin, ListView): context_object_name = "referees" def get_queryset(self): - members = members_visible_to(self.request.user, self.request.club).filter(referee_profile__isnull=False) + 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") @@ -1629,7 +1632,7 @@ class GroupBulkAddView(ClubAdminRequiredMixin, View): def get_form_kwargs(self, group): existing_ids = set(GroupMembership.objects.filter(group=group).values_list("member_id", flat=True)) - members = members_visible_to(self.request.user, self.request.club).order_by("last_name", "first_name") + members = members_visible_to(self.request.user, self.request.club, include_guardians=True).order_by("last_name", "first_name") # One member query for the whole formset -- see TeamBulkAddView.get_form_kwargs. member_choices = [("", "---------")] + [(member.pk, _("%(member)s — already in this group") % {"member": member} if member.pk in existing_ids else str(member)) for member in members] diff --git a/members/services/family.py b/members/services/family.py index 416e367..de11ecf 100644 --- a/members/services/family.py +++ b/members/services/family.py @@ -7,7 +7,7 @@ from django.contrib.auth import get_user_model from django.db import transaction from django.utils import timezone -from club.models import ClubMembership +from club.models import ClubMembership, Season from members.models import Family, FamilyMembership, Member User = get_user_model() @@ -54,9 +54,16 @@ def grant_login(member, email): return user -def _enrol(club, season, member): +def _enrol(club, season, member, kind=ClubMembership.Kind.MEMBER): """Sign a member up for the club's current season, if there is one. The - implicit MEMBER role follows automatically (club/signals.py).""" + implicit MEMBER role follows automatically (club/signals.py). + + ``kind=GUARDIAN`` attaches a parent to the club *as a parent* -- they hold + the login and can be reached, but they aren't a member, owe no fee and are + left out of every member list and count. A parent who also plays is enrolled + as a MEMBER instead; the two are not exclusive of each other in the family + graph, which records the parent relationship separately. + """ if season is None: return None @@ -64,15 +71,44 @@ def _enrol(club, season, member): club=club, member=member, season=season, - defaults={"status": ClubMembership.StatusChoices.ACTIVE, "signed_up_at": timezone.localdate()}, + defaults={"kind": kind, "status": ClubMembership.StatusChoices.ACTIVE, "signed_up_at": timezone.localdate()}, ) + if kind == ClubMembership.Kind.GUARDIAN: + carry_guardians_forward(club, member, from_season=season) return membership +def carry_guardians_forward(club, member, *, from_season): + """Give ``member`` a guardian row in every season of ``club`` at or after + ``from_season``. + + A guardian's tie to the club isn't really seasonal -- it lasts as long as + their child is there -- but it rides on ClubMembership so that everything + already keying off that table (tenancy, groups, the event audience) keeps + working. The cost of that is a row per season, and without this a parent + would silently drop off the club at the next season boundary while their + child stayed enrolled. Seasons are generated well ahead of time + (club/services/seasons.py), so "every season from here on" is a real set, + not just the next one. Idempotent. + """ + later_seasons = Season.objects.filter(club=club, start_date__gte=from_season.start_date).exclude(pk=from_season.pk) + for season in later_seasons: + ClubMembership.objects.get_or_create( + club=club, + member=member, + season=season, + defaults={"kind": ClubMembership.Kind.GUARDIAN, "status": ClubMembership.StatusChoices.ACTIVE, "signed_up_at": timezone.localdate()}, + ) + + @transaction.atomic -def register_family(club, season, *, parent_email, parent_first_name, parent_last_name, child_first_name, child_last_name, child_date_of_birth=None): +def register_family(club, season, *, parent_email, parent_first_name, parent_last_name, child_first_name, child_last_name, child_date_of_birth=None, parent_is_member=False): """Create a new family in one go: a parent (with a login) and a child - (without one), linked to each other and signed up for ``season``.""" + (without one), linked to each other and signed up for ``season``. + + The child is always a member; the parent is a guardian unless + ``parent_is_member`` says they play (or otherwise belong) in their own right. + """ parent = get_or_create_login_member(parent_email, parent_first_name, parent_last_name) child = Member.objects.create(first_name=child_first_name, last_name=child_last_name, date_of_birth=child_date_of_birth) @@ -80,7 +116,7 @@ def register_family(club, season, *, parent_email, parent_first_name, parent_las FamilyMembership.objects.create(family=family, member=parent, role=FamilyMembership.FamilyRole.PARENT) FamilyMembership.objects.create(family=family, member=child, role=FamilyMembership.FamilyRole.CHILD) - _enrol(club, season, parent) + _enrol(club, season, parent, kind=ClubMembership.Kind.MEMBER if parent_is_member else ClubMembership.Kind.GUARDIAN) _enrol(club, season, child) return family @@ -97,13 +133,14 @@ def add_child_to_family(club, season, family, *, first_name, last_name, date_of_ @transaction.atomic -def add_parent_to_family(club, season, family, *, email, first_name="", last_name=""): - """A family that needs one more parent/guardian registered.""" +def add_parent_to_family(club, season, family, *, email, first_name="", last_name="", parent_is_member=False): + """A family that needs one more parent/guardian registered. A guardian + unless ``parent_is_member`` says they belong to the club in their own right.""" parent = get_or_create_login_member(email, first_name, last_name) # get_or_create, not create: re-adding an email already on this family (a typo'd # re-submit, say) must not trip unique_member_per_family. FamilyMembership.objects.get_or_create(family=family, member=parent, defaults={"role": FamilyMembership.FamilyRole.PARENT}) - _enrol(club, season, parent) + _enrol(club, season, parent, kind=ClubMembership.Kind.MEMBER if parent_is_member else ClubMembership.Kind.GUARDIAN) return parent diff --git a/teams/services.py b/teams/services.py index 0bb4d54..06910d0 100644 --- a/teams/services.py +++ b/teams/services.py @@ -14,4 +14,8 @@ def eligible_roster_members(club): member_of__club=club, member_of__season__in=eligible_seasons, member_of__status=ClubMembership.StatusChoices.ACTIVE, + # Guardians are attached to the club only as a parent of a member, so they + # are not eligible for a roster spot *or* a staff one. A parent who + # volunteers as a coach is a member of the club and marked as one. + member_of__kind=ClubMembership.Kind.MEMBER, ).distinct()