Add Coach mode C6: bulk-add players to the active team's roster
management.forms.TeamMembershipForm is shaped for one member at a time (per-row jersey number/position/captain flags), which doesn't fit a "tap a few names, add them" flow -- the design mock itself shows plain checkboxes, no inline position picker. Reuses the same two eligibility rules the form applies internally (teams.services.eligible_roster_members, minus whoever's already on this team+season) directly instead, and lets a coach fill in jersey number/position afterward on the desktop -- the model's own help_text already documents a blank position as a normal, expected state. "Suggested" (on this team last season) is real, computed data via club.models.Season.before. "Age eligible" from the mock isn't built -- neither Club nor Team carries an age-group field to compare a birth date against, so faking that filter would just mean it silently matched nothing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9
This commit is contained in:
@@ -12,8 +12,10 @@ from django.shortcuts import get_object_or_404
|
||||
from django.urls import reverse
|
||||
from django.utils import timezone
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
from django.utils.translation import ngettext
|
||||
from django.views.generic import TemplateView
|
||||
|
||||
from club.models import Season
|
||||
from club.services.access import can_add_news, current_season
|
||||
from controlpanel.messages import notify
|
||||
from events.models import Attendance, Event
|
||||
@@ -23,6 +25,7 @@ from management.forms import EventForm, NewsForm
|
||||
from news.models import News
|
||||
from news.services import notify_editors_of_pending_review
|
||||
from teams.models import Team, TeamMembership
|
||||
from teams.services import eligible_roster_members
|
||||
|
||||
from .coach_mixins import CoachScopeMixin
|
||||
from .forms import _INPUT_CLASSES
|
||||
@@ -330,3 +333,89 @@ class CoachCreateNewsView(CoachScopeMixin, LoginRequiredMixin, TemplateView):
|
||||
body = _("“%(news)s” is ready for review.") % {"news": news_item}
|
||||
notify(request, f"s|{_('Sent for review')}|{body}")
|
||||
return HttpResponseRedirect(reverse("mobile:coach_today"))
|
||||
|
||||
|
||||
class CoachAddPlayerView(CoachScopeMixin, LoginRequiredMixin, TemplateView):
|
||||
"""C6 -- bulk-add players to the active team's roster: a checkbox per
|
||||
candidate rather than management.forms.TeamMembershipForm's one-member-
|
||||
at-a-time shape, which doesn't fit a "tap a few names, add them" flow
|
||||
anyway (the mock itself shows plain checkboxes, no inline position
|
||||
picker). A coach sets jersey number/position afterward on the desktop --
|
||||
same as any roster spot added blank via the Sign-up page today
|
||||
(TeamMembership.position's own help_text already documents this as a
|
||||
normal, expected state, not a shortcut this screen invents).
|
||||
|
||||
The pool is teams.services.eligible_roster_members(club) minus whoever's
|
||||
already on this team+season -- the same two rules TeamMembershipForm
|
||||
applies internally, just reused directly rather than through the form.
|
||||
"Suggested" (on this team last season) is real, computed data. "Age
|
||||
eligible" from the mock isn't built -- neither Club nor Team carries an
|
||||
age-group field to compare a birth date against, so faking that filter
|
||||
would just mean it silently matched nothing.
|
||||
"""
|
||||
|
||||
template_name = "mobile/coach/add_player.html"
|
||||
screen_title = _("Add players")
|
||||
active_tab = "coach_today"
|
||||
|
||||
#: ?filter= values this screen understands -- anything else (including no
|
||||
#: param at all) means "All".
|
||||
FILTERS = {"suggested", "no_team"}
|
||||
|
||||
def get(self, request, *args, **kwargs):
|
||||
if not self.can_manage_active_team:
|
||||
return HttpResponseRedirect(reverse("mobile:coach_today"))
|
||||
return super().get(request, *args, **kwargs)
|
||||
|
||||
def _candidate_pool(self, season):
|
||||
taken = TeamMembership.objects.filter(team=self.active_team, season=season).values_list("member_id", flat=True)
|
||||
return eligible_roster_members(self.request.club).exclude(pk__in=taken)
|
||||
|
||||
def get_context_data(self, **kwargs):
|
||||
season = current_season(self.request.club)
|
||||
candidates = []
|
||||
squad_count = 0
|
||||
|
||||
filter_param = self.request.GET.get("filter")
|
||||
if filter_param not in self.FILTERS:
|
||||
filter_param = ""
|
||||
|
||||
if self.active_team is not None and season is not None:
|
||||
squad_count = TeamMembership.objects.filter(team=self.active_team, season=season).count()
|
||||
pool = self._candidate_pool(season)
|
||||
|
||||
if filter_param == "no_team":
|
||||
pool = pool.exclude(team_memberships__season=season)
|
||||
elif filter_param == "suggested":
|
||||
previous_season = Season.before(self.request.club, season)
|
||||
pool = pool.filter(team_memberships__team=self.active_team, team_memberships__season=previous_season) if previous_season is not None else pool.none()
|
||||
|
||||
candidates = list(pool.distinct().order_by("last_name", "first_name"))
|
||||
|
||||
return super().get_context_data(
|
||||
candidates=candidates,
|
||||
squad_count=squad_count,
|
||||
filter_param=filter_param,
|
||||
**kwargs,
|
||||
)
|
||||
|
||||
def post(self, request, *args, **kwargs):
|
||||
if not self.can_manage_active_team:
|
||||
return HttpResponseForbidden()
|
||||
|
||||
season = current_season(request.club)
|
||||
if self.active_team is None or season is None:
|
||||
return HttpResponseForbidden()
|
||||
|
||||
pool_ids = {str(pk) for pk in self._candidate_pool(season).values_list("pk", flat=True)}
|
||||
added = 0
|
||||
for member_id in request.POST.getlist("member"):
|
||||
if member_id not in pool_ids:
|
||||
continue
|
||||
TeamMembership.objects.get_or_create(team=self.active_team, season=season, member_id=member_id)
|
||||
added += 1
|
||||
|
||||
if added:
|
||||
body = ngettext("%(count)d player added to the roster.", "%(count)d players added to the roster.", added) % {"count": added}
|
||||
notify(request, f"s|{_('Roster updated')}|{body}")
|
||||
return HttpResponseRedirect(reverse("mobile:coach_today"))
|
||||
|
||||
50
mobile/templates/mobile/coach/add_player.html
Normal file
50
mobile/templates/mobile/coach/add_player.html
Normal file
@@ -0,0 +1,50 @@
|
||||
{% extends "mobile/coach/base.html" %}
|
||||
{% load i18n %}
|
||||
|
||||
{% comment %}
|
||||
C6 -- design_handoff_rosterchief_platform/README.md's C6 section: a
|
||||
checkbox per eligible candidate rather than one member at a time (see
|
||||
CoachAddPlayerView's own docstring for why, and what's scoped down from
|
||||
the mock -- no "Age eligible" filter, no fixed footer).
|
||||
{% endcomment %}
|
||||
|
||||
{% block header_extra %}
|
||||
<div class="mt-3 flex items-center justify-between">
|
||||
<span class="font-display text-lg font-extrabold text-white uppercase">{% blocktrans with team=active_team.name %}Add to {{ team }}{% endblocktrans %}</span>
|
||||
<span class="font-mono text-xs text-on-dark">{% blocktrans count counter=squad_count %}{{ counter }} on squad{% plural %}{{ counter }} on squad{% endblocktrans %}</span>
|
||||
</div>
|
||||
{% endblock header_extra %}
|
||||
|
||||
{% block content %}
|
||||
<div class="flex gap-2">
|
||||
<a class="flex h-9 flex-1 items-center justify-center rounded-full font-display text-xs font-extrabold tracking-wide uppercase {% if not filter_param %}bg-ink text-white{% else %}border border-line bg-white text-muted{% endif %}" href="?">
|
||||
{% trans "All" %}
|
||||
</a>
|
||||
<a class="flex h-9 flex-1 items-center justify-center rounded-full font-display text-xs font-extrabold tracking-wide uppercase {% if filter_param == "suggested" %}bg-ink text-white{% else %}border border-line bg-white text-muted{% endif %}" href="?filter=suggested">
|
||||
{% trans "Suggested" %}
|
||||
</a>
|
||||
<a class="flex h-9 flex-1 items-center justify-center rounded-full font-display text-xs font-extrabold tracking-wide uppercase {% if filter_param == "no_team" %}bg-ink text-white{% else %}border border-line bg-white text-muted{% endif %}" href="?filter=no_team">
|
||||
{% trans "No team" %}
|
||||
</a>
|
||||
</div>
|
||||
|
||||
<form method="post" action="{% url "mobile:coach_add_player" %}" x-data="{ count: 0 }">
|
||||
{% csrf_token %}
|
||||
<div class="m-card flex flex-col overflow-hidden">
|
||||
{% for candidate in candidates %}
|
||||
<label class="flex items-center gap-3 px-4 py-2.5 {% if not forloop.last %}border-b border-rule{% endif %}">
|
||||
{% include "mobile/_avatar.html" with person=candidate size_class="h-9 w-9" text_class="text-xs" %}
|
||||
<span class="min-w-0 flex-1 text-sm font-semibold text-ink">{{ candidate.get_full_name }}</span>
|
||||
<input class="h-5 w-5 shrink-0 accent-ink" type="checkbox" name="member" value="{{ candidate.pk }}" @change="count = $el.form.querySelectorAll('input[name=member]:checked').length">
|
||||
</label>
|
||||
{% empty %}
|
||||
<div class="px-4 py-6 text-center text-sm text-muted">{% trans "No one matches this filter." %}</div>
|
||||
{% endfor %}
|
||||
</div>
|
||||
|
||||
<button class="btn btn-dark mt-4 w-full" type="submit" :disabled="count === 0">
|
||||
<span x-show="count === 0">{% trans "Select players to add" %}</span>
|
||||
<span x-show="count > 0" x-cloak>{% trans "Add" %} (<span x-text="count"></span>)</span>
|
||||
</button>
|
||||
</form>
|
||||
{% endblock content %}
|
||||
@@ -20,6 +20,7 @@
|
||||
<div class="flex gap-2">
|
||||
<a class="btn btn-dark flex-1" href="{% url "mobile:coach_create_event" %}">{% trans "New event" %}</a>
|
||||
<a class="btn btn-secondary flex-1" href="{% url "mobile:coach_create_news" %}">{% trans "New post" %}</a>
|
||||
<a class="btn btn-secondary flex-1" href="{% url "mobile:coach_add_player" %}">{% trans "Add player" %}</a>
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
|
||||
110
mobile/tests.py
110
mobile/tests.py
@@ -2132,3 +2132,113 @@ class CoachCreateNewsViewTests(TestCase):
|
||||
|
||||
self.assertEqual(response.status_code, 403)
|
||||
self.assertFalse(News.objects.filter(title="Blocked post").exists())
|
||||
|
||||
|
||||
@override_settings(ROSTERCHIEF_BASE_DOMAIN="rosterchief.app", ALLOWED_HOSTS=["rosterchief.app", "ajax-united.rosterchief.app", "testserver"])
|
||||
class CoachAddPlayerViewTests(TestCase):
|
||||
"""C6 -- bulk-add players to the active team's roster; see
|
||||
CoachAddPlayerView's own docstring for what's scoped down from the
|
||||
design mock."""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
cls.club = make_club()
|
||||
today = timezone.localdate()
|
||||
cls.season = Season.objects.create(club=cls.club, start_date=today - datetime.timedelta(days=30), end_date=today + datetime.timedelta(days=300))
|
||||
cls.user = User.objects.create_user(email="coach@example.com", password="pw-secret-123")
|
||||
cls.member = Member.objects.create(first_name="Sam", last_name="Coach", email="coach@example.com", user=cls.user)
|
||||
cls.team = Team.objects.create(club=cls.club, name="U16", short_name="U16")
|
||||
cls.position = Position.objects.create(club=cls.club, name="Head coach", short_name="HC", staff_position=True, management_position=True)
|
||||
StaffAssignment.objects.create(team=cls.team, member=cls.member, season=cls.season, position=cls.position)
|
||||
|
||||
def make_eligible_member(self, first_name="Anna", last_name="Player"):
|
||||
member = Member.objects.create(first_name=first_name, last_name=last_name)
|
||||
ClubMembership.objects.create(club=self.club, member=member, season=self.season, status=ClubMembership.StatusChoices.ACTIVE, kind=ClubMembership.Kind.MEMBER)
|
||||
return member
|
||||
|
||||
def test_requires_login(self):
|
||||
response = self.client.get(reverse("mobile:coach_add_player"), HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
self.assertEqual(response.status_code, 302)
|
||||
|
||||
def test_get_redirects_a_non_managing_staffer(self):
|
||||
physio_position = Position.objects.create(club=self.club, name="Physio", short_name="PHY", staff_position=True, management_position=False)
|
||||
physio_user = User.objects.create_user(email="physio@example.com", password="pw-secret-123")
|
||||
physio_member = Member.objects.create(first_name="Pat", last_name="Physio", user=physio_user)
|
||||
StaffAssignment.objects.create(team=self.team, member=physio_member, season=self.season, position=physio_position)
|
||||
self.client.force_login(physio_user)
|
||||
|
||||
response = self.client.get(reverse("mobile:coach_add_player"), HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
self.assertRedirects(response, reverse("mobile:coach_today"), fetch_redirect_response=False)
|
||||
|
||||
def test_lists_eligible_members_not_already_on_the_roster(self):
|
||||
eligible = self.make_eligible_member()
|
||||
already_on_roster = self.make_eligible_member(first_name="On", last_name="Roster")
|
||||
TeamMembership.objects.create(team=self.team, member=already_on_roster, season=self.season)
|
||||
self.client.force_login(self.user)
|
||||
|
||||
response = self.client.get(reverse("mobile:coach_add_player"), HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
candidates = response.context["candidates"]
|
||||
self.assertIn(eligible, candidates)
|
||||
self.assertNotIn(already_on_roster, candidates)
|
||||
|
||||
def test_no_team_filter_excludes_members_on_any_team_this_season(self):
|
||||
on_other_team = self.make_eligible_member(first_name="Other", last_name="Team")
|
||||
other_team = Team.objects.create(club=self.club, name="U14", short_name="U14")
|
||||
TeamMembership.objects.create(team=other_team, member=on_other_team, season=self.season)
|
||||
unrostered = self.make_eligible_member(first_name="No", last_name="Team")
|
||||
self.client.force_login(self.user)
|
||||
|
||||
response = self.client.get(reverse("mobile:coach_add_player") + "?filter=no_team", HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
candidates = response.context["candidates"]
|
||||
self.assertIn(unrostered, candidates)
|
||||
self.assertNotIn(on_other_team, candidates)
|
||||
|
||||
def test_suggested_filter_matches_last_seasons_roster(self):
|
||||
previous_season = Season.objects.create(club=self.club, start_date=self.season.start_date - datetime.timedelta(days=365), end_date=self.season.start_date - datetime.timedelta(days=1))
|
||||
returning = self.make_eligible_member(first_name="Returning", last_name="Player")
|
||||
TeamMembership.objects.create(team=self.team, member=returning, season=previous_season)
|
||||
new_signup = self.make_eligible_member(first_name="New", last_name="Signup")
|
||||
self.client.force_login(self.user)
|
||||
|
||||
response = self.client.get(reverse("mobile:coach_add_player") + "?filter=suggested", HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
candidates = response.context["candidates"]
|
||||
self.assertEqual(list(candidates), [returning])
|
||||
self.assertNotIn(new_signup, candidates)
|
||||
|
||||
def test_post_adds_selected_members_to_the_roster(self):
|
||||
first = self.make_eligible_member(first_name="First", last_name="Pick")
|
||||
second = self.make_eligible_member(first_name="Second", last_name="Pick")
|
||||
self.client.force_login(self.user)
|
||||
|
||||
response = self.client.post(reverse("mobile:coach_add_player"), {"member": [str(first.pk), str(second.pk)]}, HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
self.assertRedirects(response, reverse("mobile:coach_today"), fetch_redirect_response=False)
|
||||
self.assertTrue(TeamMembership.objects.filter(team=self.team, season=self.season, member=first).exists())
|
||||
self.assertTrue(TeamMembership.objects.filter(team=self.team, season=self.season, member=second).exists())
|
||||
|
||||
def test_post_ignores_a_member_id_outside_the_eligible_pool(self):
|
||||
ineligible = Member.objects.create(first_name="Not", last_name="Eligible")
|
||||
self.client.force_login(self.user)
|
||||
|
||||
response = self.client.post(reverse("mobile:coach_add_player"), {"member": [str(ineligible.pk)]}, HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
self.assertRedirects(response, reverse("mobile:coach_today"), fetch_redirect_response=False)
|
||||
self.assertFalse(TeamMembership.objects.filter(team=self.team, member=ineligible).exists())
|
||||
|
||||
def test_non_managing_staff_cannot_post(self):
|
||||
physio_position = Position.objects.create(club=self.club, name="Physio", short_name="PHY", staff_position=True, management_position=False)
|
||||
physio_user = User.objects.create_user(email="physio@example.com", password="pw-secret-123")
|
||||
physio_member = Member.objects.create(first_name="Pat", last_name="Physio", user=physio_user)
|
||||
StaffAssignment.objects.create(team=self.team, member=physio_member, season=self.season, position=physio_position)
|
||||
candidate = self.make_eligible_member()
|
||||
self.client.force_login(physio_user)
|
||||
|
||||
response = self.client.post(reverse("mobile:coach_add_player"), {"member": [str(candidate.pk)]}, HTTP_HOST="ajax-united.rosterchief.app")
|
||||
|
||||
self.assertEqual(response.status_code, 403)
|
||||
self.assertFalse(TeamMembership.objects.filter(team=self.team, member=candidate).exists())
|
||||
|
||||
@@ -27,4 +27,5 @@ urlpatterns = [
|
||||
path("coach/attendance/<uuid:event_id>/", coach_views.CoachAttendanceView.as_view(), name="coach_attendance"),
|
||||
path("coach/events/new/", coach_views.CoachCreateEventView.as_view(), name="coach_create_event"),
|
||||
path("coach/news/new/", coach_views.CoachCreateNewsView.as_view(), name="coach_create_news"),
|
||||
path("coach/roster/add/", coach_views.CoachAddPlayerView.as_view(), name="coach_add_player"),
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user