diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d875e0a..383e966 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -456,6 +456,32 @@ number — modeled by `TeamMembership`, exactly matching the domain note. extra tenancy field is needed on the constraint. - `StaffAssignment` drives the coach/manager object-scope (§3.1–3.2) — it *is* the "is a coach of this team" fact; no `ClubRole` mirrors it. +- **Bulk add is a row formset, not a table of every member** (`TeamBulkAddView`, and the same + shape for groups in `GroupBulkAddView`). Each row picks one person from a searchable select + and one `Position`; the *position* decides what the row means — `Position.staff_position` + true ⇒ a `StaffAssignment`, otherwise a `TeamMembership` with an optional jersey number and + captain/alternate-captain flags — so there's no separate "player or staff?" control that + could disagree with the position picked. Jersey number and captaincy exist only on + `TeamMembership`, so a staff row rejects them (and the row script greys them out, keyed off + the `data-staff` marker `PositionSelect` stamps on staff options). Captain *and* alternate + captain on one row is refused as self-contradictory, but how many captains a team may have + is left alone: neither the model nor the single-add form constrains it, and inventing the + rule in one entry path only would be worse than not having it. + A playing coach is simply two rows. Rows are cloned client-side from the formset's + `empty_form` (`static/js/bulk-add-rows.js`); removing one deletes the node and deliberately + leaves `TOTAL_FORMS` alone, since Django reads a form whose fields are absent from the POST + as an unchanged extra and skips it — safe, unlike re-indexing live inputs. The earlier + design rendered *every* eligible member as a table row, which a club with a hundred-plus + members can't use, and its search was a GET round-trip that discarded anything already + ticked. **All-or-nothing on submit**: one bad row re-renders the page with every row still + filled in and the offending field flagged, rather than saving the good rows and losing the + rest (a partial save is far more costly when the rows were typed by hand). Cross-row checks + no single row can see — the same person twice, two rows claiming one jersey — live on the + formset's `clean()`; per-row checks (already assigned, jersey already taken by an existing + entry) live on the row form. Eligibility is never trusted from the POST: the member field's + queryset is `eligible_roster_members`, so an id that was never offered fails its own lookup. + The member `choices` are built once in the view and assigned onto each row's field — + a `ModelChoiceField` otherwise re-runs its queryset per form, i.e. once per row. **As built, `Team` also carries `referee_management`** (`TextChoices`: `club` | `federation`, default `club`) — whether the *club* arranges referees for this team's home games, or the diff --git a/management/forms.py b/management/forms.py index 53b6b18..5bcfc90 100644 --- a/management/forms.py +++ b/management/forms.py @@ -174,6 +174,230 @@ class StaffAssignmentForm(forms.ModelForm): self.fields["position"].queryset = Position.objects.filter(club=club, staff_position=True) +class PositionSelect(forms.Select): + """Marks staff positions in the DOM (``data-staff``) so a bulk-add row's + jersey-number input can disable itself for them -- a jersey number is + meaningless on a StaffAssignment, which has no such field.""" + + def create_option(self, name, value, *args, **kwargs): + option = super().create_option(name, value, *args, **kwargs) + position = getattr(value, "instance", None) + if position is not None and position.staff_position: + option["attrs"]["data-staff"] = "1" + return option + + +class GroupedPositionChoiceField(forms.ModelChoiceField): + """Positions split into "Player positions" / "Staff positions" optgroups. + + Which group a position sits in is exactly what tells the bulk-add view + whether a row means a ``TeamMembership`` or a ``StaffAssignment`` + (``Position.staff_position``) -- so the row needs no separate "player or + staff?" control that could drift out of step with the position picked. + """ + + widget = PositionSelect + + def _get_choices(self): + if hasattr(self, "_choices"): + return self._choices + + iterator = self.iterator(self) + player, staff = [], [] + for value, label in iterator: + if value == "": + continue # the empty label is yielded separately below + target = staff if getattr(value, "instance", None) is not None and value.instance.staff_position else player + target.append((value, label)) + + grouped = [] + if self.empty_label is not None: + grouped.append(("", self.empty_label)) + if player: + grouped.append((_("Player positions"), player)) + if staff: + grouped.append((_("Staff positions"), staff)) + return grouped + + choices = property(_get_choices, forms.ChoiceField.choices.fset) + + +def bulk_add_member_label(member, *, rostered_ids, staffed_ids): + """"Peter Player — on roster" for the bulk-add member picker. + + Already-assigned members stay selectable rather than being filtered out: + someone already on the roster as a player can still legitimately be added + as staff (a playing coach), so what's "taken" depends on the position the + row ends up picking. The label is the hint; the row's own clean() is what + actually refuses a true duplicate. + """ + marks = [] + if member.pk in rostered_ids: + marks.append(_("on roster")) + if member.pk in staffed_ids: + marks.append(_("on staff")) + if not marks: + return str(member) + # Not itself a translatable message -- just a separator between the member's + # name and the already-translated markers. + return f"{member} — {', '.join(str(mark) for mark in marks)}" + + +class TeamBulkAddRowForm(forms.Form): + """One row of the team bulk-add page: who, in what position, and (players + only) their jersey number and captaincy. + + Rows replace the old "every eligible member gets a table row" layout, which + doesn't survive a club with a hundred-plus members. `member` is a searchable + select (static ``choices``, not the field's own lazy queryset iterator, so a + twenty-row formset doesn't re-query the member list twenty times over). + """ + + member = forms.ModelChoiceField( + queryset=Member.objects.none(), + label=_("Member"), + widget=forms.Select(attrs={"class": "select select-bordered w-full", "data-searchable": "true", "data-search-placeholder": _("Type a name to search...")}), + ) + position = GroupedPositionChoiceField( + queryset=Position.objects.none(), + label=_("Position"), + widget=PositionSelect(attrs={"class": "select select-bordered w-full position-select"}), + ) + jersey_number = forms.IntegerField( + required=False, + min_value=0, + label=_("Jersey #"), + widget=forms.NumberInput(attrs={"class": "input input-bordered w-full player-only", "min": "0"}), + ) + # Player-only, same as jersey_number -- "player-only" marks them for the + # row script, which greys them out when a staff position is picked. + is_captain = forms.BooleanField(required=False, label=_("Captain"), widget=forms.CheckboxInput(attrs={"class": "checkbox player-only"})) + is_alternate_captain = forms.BooleanField(required=False, label=_("Alternate"), widget=forms.CheckboxInput(attrs={"class": "checkbox player-only"})) + + def __init__(self, *args, club=None, team=None, season=None, member_choices=None, position_queryset=None, rostered_ids=frozenset(), staffed_ids=frozenset(), **kwargs): + super().__init__(*args, **kwargs) + self.team = team + self.season = season + self.rostered_ids = rostered_ids + self.staffed_ids = staffed_ids + + self.fields["member"].queryset = eligible_roster_members(club) + if member_choices is not None: + self.fields["member"].choices = member_choices + + self.fields["position"].queryset = position_queryset if position_queryset is not None else Position.objects.filter(club=club) + + def clean(self): + cleaned = super().clean() + member, position = cleaned.get("member"), cleaned.get("position") + if member is None or position is None: + return cleaned + + if position.staff_position: + # A staff row becomes a StaffAssignment, which has no jersey number + # and no captaincy -- both belong to TeamMembership only. + if member.pk in self.staffed_ids: + self.add_error("member", _("%(member)s is already on this team's staff for this season.") % {"member": member}) + if cleaned.get("jersey_number") is not None: + self.add_error("jersey_number", _("A jersey number doesn't apply to a staff position.")) + if cleaned.get("is_captain"): + self.add_error("is_captain", _("Captaincy doesn't apply to a staff position.")) + if cleaned.get("is_alternate_captain"): + self.add_error("is_alternate_captain", _("Captaincy doesn't apply to a staff position.")) + return cleaned + + if member.pk in self.rostered_ids: + self.add_error("member", _("%(member)s is already on this team's roster for this season.") % {"member": member}) + + # Contradictory rather than a club policy: an alternate captain is by + # definition not the captain. How *many* captains a team may have isn't + # constrained anywhere (not by the model, not by the single-add form), + # so this row deliberately doesn't invent that rule either. + if cleaned.get("is_captain") and cleaned.get("is_alternate_captain"): + self.add_error("is_alternate_captain", _("Someone can be captain or alternate captain, not both.")) + + jersey_number = cleaned.get("jersey_number") + if jersey_number is not None and self.team is not None and self.season is not None: + if TeamMembership.objects.filter(team=self.team, season=self.season, jersey_number=jersey_number).exists(): + self.add_error("jersey_number", _("Jersey #%(number)s is already taken on this team this season.") % {"number": jersey_number}) + return cleaned + + +class BaseTeamBulkAddFormSet(forms.BaseFormSet): + """Cross-row checks the individual rows can't see: the same person twice, + or two rows claiming one jersey number. Per-row checks (already assigned, + jersey already taken by an *existing* roster entry) live on the row form.""" + + def clean(self): + super().clean() + if any(self.errors): + return + + seen_assignments, seen_jerseys = set(), set() + for form in self.forms: + member, position = form.cleaned_data.get("member"), form.cleaned_data.get("position") + if member is None or position is None: + continue + + assignment = (member.pk, bool(position.staff_position)) + if assignment in seen_assignments: + form.add_error("member", _("%(member)s is listed twice for the same kind of position.") % {"member": member}) + seen_assignments.add(assignment) + + jersey_number = form.cleaned_data.get("jersey_number") + if jersey_number is None or position.staff_position: + continue + if jersey_number in seen_jerseys: + form.add_error("jersey_number", _("Jersey #%(number)s is claimed by more than one row.") % {"number": jersey_number}) + seen_jerseys.add(jersey_number) + + +TeamBulkAddFormSet = forms.formset_factory(TeamBulkAddRowForm, formset=BaseTeamBulkAddFormSet, extra=3) + + +class GroupBulkAddRowForm(forms.Form): + """One row of the group bulk-add page -- just who. Group membership carries + no per-member attributes, so there's nothing else to fill in.""" + + member = forms.ModelChoiceField( + queryset=Member.objects.none(), + label=_("Member"), + widget=forms.Select(attrs={"class": "select select-bordered w-full", "data-searchable": "true", "data-search-placeholder": _("Type a name to search...")}), + ) + + def __init__(self, *args, member_queryset=None, member_choices=None, existing_ids=frozenset(), **kwargs): + super().__init__(*args, **kwargs) + self.existing_ids = existing_ids + self.fields["member"].queryset = member_queryset if member_queryset is not None else Member.objects.none() + if member_choices is not None: + self.fields["member"].choices = member_choices + + def clean_member(self): + member = self.cleaned_data["member"] + if member.pk in self.existing_ids: + raise forms.ValidationError(_("%(member)s is already in this group.") % {"member": member}) + return member + + +class BaseGroupBulkAddFormSet(forms.BaseFormSet): + def clean(self): + super().clean() + if any(self.errors): + return + + seen = set() + for form in self.forms: + member = form.cleaned_data.get("member") + if member is None: + continue + if member.pk in seen: + form.add_error("member", _("%(member)s is listed twice.") % {"member": member}) + seen.add(member.pk) + + +GroupBulkAddFormSet = forms.formset_factory(GroupBulkAddRowForm, formset=BaseGroupBulkAddFormSet, extra=3) + + class PositionForm(forms.ModelForm): class Meta: model = Position diff --git a/management/templates/management/_group_bulk_add_row.html b/management/templates/management/_group_bulk_add_row.html new file mode 100644 index 0000000..20f7e5f --- /dev/null +++ b/management/templates/management/_group_bulk_add_row.html @@ -0,0 +1,15 @@ +{% load i18n lucide %} +{% comment %} + One row of the group bulk-add formset -- see _team_bulk_add_row.html for why + this is a partial (rendered both for real rows and into the clone template). +{% endcomment %} + + + {{ form.member }} + {% for error in form.member.errors %}

{{ error }}

{% endfor %} + {% for error in form.non_field_errors %}

{{ error }}

{% endfor %} + + + + + diff --git a/management/templates/management/_team_bulk_add_row.html b/management/templates/management/_team_bulk_add_row.html new file mode 100644 index 0000000..542fc14 --- /dev/null +++ b/management/templates/management/_team_bulk_add_row.html @@ -0,0 +1,32 @@ +{% load i18n lucide %} +{% comment %} + One row of the team bulk-add formset. Rendered both for the rows already in + the formset and (with __prefix__ placeholders) into the