From 8c415ed46a17129034d564993d9fcdaf7be5c08b Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Sun, 23 Aug 2026 17:59:18 +0200 Subject: [PATCH] Line-up: show each player's turnout rate, notify on changes after publish A "yes" to this game doesn't say how reliably a player actually shows up -- each row on the line-up screen now shows this season's turnout rate (events.services.attendance.player_attendance_rankings, one query for the whole team), color-coded, omitted for anyone with too little history to mean anything. Also: editing a published line-up and hitting "Save" never notified anyone (it only ever wrote LineupSelection, never touched Attendance.status) -- "Publish"/"Publish changes" now stays reachable after the first publish, and publish_lineup diffs against who was SELECTED before this run so only players whose status actually changed get notified, not everyone currently selected. Co-Authored-By: Claude Sonnet 5 --- events/services/lineup.py | 22 ++++++++-- events/tests.py | 37 +++++++++++++++++ mobile/coach_views.py | 38 ++++++++++++++--- mobile/templates/mobile/coach/lineup.html | 13 +++++- mobile/tests.py | 50 +++++++++++++++++++++++ 5 files changed, 149 insertions(+), 11 deletions(-) diff --git a/events/services/lineup.py b/events/services/lineup.py index ac71f35..bdb0ada 100644 --- a/events/services/lineup.py +++ b/events/services/lineup.py @@ -36,12 +36,21 @@ def publish_lineup(lineup): previously unused anywhere. Every selected member becomes SELECTED; every other member with an Attendance row for this event who was actually available (not out/silent, see UNAVAILABLE_STATUSES) becomes - NOT_SELECTED. Notifies only the selected players. + NOT_SELECTED. + Also doubles as "publish changes" for an already-published lineup a + coach keeps editing (CoachLineupView's own "Save line-up" only writes + LineupSelection -- Attendance.status, and any notification, only ever + updates here). Notifies by comparing against who was SELECTED *before* + this run, not by re-notifying everyone currently selected -- a player + unaffected by the edit shouldn't get a second "you're in" ping, and one + dropped since the last publish should hear about that specifically. Clears scheduled_publish_at regardless of whether this run came from a coach tapping Publish directly or from the scheduled sweep (events.tasks. publish_scheduled_lineups) catching a due one -- once published, there's nothing left pending either way.""" + previously_selected_ids = set(Attendance.objects.filter(event=lineup.event, status=Attendance.AttendanceStatus.SELECTED).values_list("member_id", flat=True)) + lineup.published_at = timezone.now() lineup.scheduled_publish_at = None lineup.save(update_fields=["published_at", "scheduled_publish_at"]) @@ -51,10 +60,15 @@ def publish_lineup(lineup): Attendance.objects.filter(event=lineup.event, member_id__in=selected_member_ids).update(status=Attendance.AttendanceStatus.SELECTED) Attendance.objects.filter(event=lineup.event).exclude(member_id__in=selected_member_ids).exclude(status__in=UNAVAILABLE_STATUSES).update(status=Attendance.AttendanceStatus.NOT_SELECTED) - selected_members = Member.objects.filter(pk__in=selected_member_ids) - if selected_members: + newly_selected = Member.objects.filter(pk__in=selected_member_ids - previously_selected_ids) + if newly_selected: body = _("You're in the line-up for %(event)s.") % {"event": lineup.event.title} - notify_members(selected_members, club=lineup.event.club, title=_("Line-up published"), body=body, source=lineup.event) + notify_members(newly_selected, club=lineup.event.club, title=_("Line-up published"), body=body, source=lineup.event) + + newly_dropped = Member.objects.filter(pk__in=previously_selected_ids - selected_member_ids) + if newly_dropped: + body = _("The line-up for %(event)s has changed -- you're not in it this time.") % {"event": lineup.event.title} + notify_members(newly_dropped, club=lineup.event.club, title=_("Line-up updated"), body=body, source=lineup.event) return lineup diff --git a/events/tests.py b/events/tests.py index 706c44e..ca2e468 100644 --- a/events/tests.py +++ b/events/tests.py @@ -573,6 +573,43 @@ class LineupServiceTests(EventsTestBase): notified_member_ids = set(Notification.objects.filter(member__in=[self.alice, self.bob]).values_list("member_id", flat=True)) self.assertEqual(notified_member_ids, {self.alice.pk}) + def test_republish_does_not_renotify_a_player_still_selected(self): + event, lineup = self.make_game_with_lineup() + LineupSelection.objects.create(lineup=lineup, member=self.alice) + Attendance.objects.update_or_create(event=event, member=self.alice, defaults={"status": Attendance.AttendanceStatus.PRESENT}) + publish_lineup(lineup) + Notification.objects.filter(member=self.alice).delete() + + publish_lineup(lineup) + + self.assertFalse(Notification.objects.filter(member=self.alice).exists()) + + def test_republish_notifies_a_newly_added_player(self): + event, lineup = self.make_game_with_lineup() + Attendance.objects.update_or_create(event=event, member=self.alice, defaults={"status": Attendance.AttendanceStatus.PRESENT}) + publish_lineup(lineup) + Notification.objects.filter(member=self.alice).delete() + + LineupSelection.objects.create(lineup=lineup, member=self.alice) + publish_lineup(lineup) + + notification = Notification.objects.get(member=self.alice) + self.assertIn("in the line-up", notification.body) + + def test_republish_notifies_a_dropped_player_with_a_different_message(self): + event, lineup = self.make_game_with_lineup() + LineupSelection.objects.create(lineup=lineup, member=self.alice) + Attendance.objects.update_or_create(event=event, member=self.alice, defaults={"status": Attendance.AttendanceStatus.PRESENT}) + publish_lineup(lineup) + Notification.objects.filter(member=self.alice).delete() + + LineupSelection.objects.filter(lineup=lineup, member=self.alice).delete() + publish_lineup(lineup) + + notification = Notification.objects.get(member=self.alice) + self.assertIn("not in it this time", notification.body) + self.assertEqual(Attendance.objects.get(event=event, member=self.alice).status, Attendance.AttendanceStatus.NOT_SELECTED) + def test_notify_dropout_notifies_the_teams_managers(self): event, _lineup = self.make_game_with_lineup() manager = Member.objects.create(first_name="Cara", last_name="Coach") diff --git a/mobile/coach_views.py b/mobile/coach_views.py index bb9b81b..df4ea09 100644 --- a/mobile/coach_views.py +++ b/mobile/coach_views.py @@ -25,7 +25,7 @@ from club.services.access import can_add_news, current_season from controlpanel.messages import notify from events.models import Attendance, Event, EventSeries, Lineup, LineupSelection, Location, Opponent from events.services import generate_occurrences -from events.services.attendance import member_attendance_counts, record_check_in +from events.services.attendance import member_attendance_counts, player_attendance_rankings, record_check_in from events.services.calendar import agenda_groups from events.services.lineup import UNAVAILABLE_STATUSES, cancel_scheduled_publish, publish_lineup, schedule_lineup_publish, toggle_selection from events.tasks import notify_new_event @@ -898,6 +898,15 @@ class CoachLineupView(CoachScopeMixin, LoginRequiredMixin, TemplateView): separate "start a line-up" step. Viewing is open to anyone staffing the team; saving/publishing is gated on can_manage_active_team, hidden in the template and 403'd here regardless. + + Saving here only ever writes LineupSelection -- it never touches + Attendance.status or sends a notification, before or after the first + publish. A coach can keep editing a published lineup freely without + pinging anyone; CoachLineupPublishView's "Publish"/"Publish changes" + button (see the template) is the only thing that syncs Attendance and + notifies -- and events.services.lineup.publish_lineup only notifies + whoever's status actually changed since the last publish, not everyone + currently selected. """ template_name = "mobile/coach/lineup.html" @@ -914,11 +923,21 @@ class CoachLineupView(CoachScopeMixin, LoginRequiredMixin, TemplateView): the member-side published view groups by (mobile/views.py's EventDetailView). A player with no TeamMembership for this team/ season (a guest call-up) lands in a catch-all "No position set" - bucket rather than being dropped.""" + bucket rather than being dropped. + + Each row also carries this season's turnout rate (``events.services. + attendance.player_attendance_rankings``, one query for the whole + team rather than one per row) -- a player saying "yes" to this game + doesn't tell a coach how reliably they actually show up, and that's + exactly the judgment call a line-up screen exists for. Riders with + too little history (rankings' own ``minimum_responses`` floor) get + no rate rather than a misleading 0%/100% from one data point.""" season = current_season(self.request.club) memberships_by_member = {} + rates_by_member = {} if season is not None: memberships_by_member = {tm.member_id: tm for tm in TeamMembership.objects.filter(team=self.active_team, season=season).select_related("position")} + rates_by_member = {row["member"].pk: row["rate"] for row in player_attendance_rankings(self.active_team, season)} selected_ids = set(LineupSelection.objects.filter(lineup=lineup).values_list("member_id", flat=True)) available = Attendance.objects.filter(event=event).exclude(status__in=UNAVAILABLE_STATUSES).select_related("member").order_by("member__last_name", "member__first_name") @@ -929,7 +948,7 @@ class CoachLineupView(CoachScopeMixin, LoginRequiredMixin, TemplateView): position = membership.position if membership else None key = position.pk if position else None bucket = buckets.setdefault(key, {"label": position.name if position else _("No position set"), "ordering": position.ordering if position else 9999, "rows": []}) - bucket["rows"].append({"member": attendance.member, "membership": membership, "selected": attendance.member_id in selected_ids}) + bucket["rows"].append({"member": attendance.member, "membership": membership, "selected": attendance.member_id in selected_ids, "attendance_rate": rates_by_member.get(attendance.member_id)}) return sorted(buckets.values(), key=lambda bucket: (bucket["ordering"], bucket["label"])) @@ -975,7 +994,12 @@ class CoachLineupPublishView(CoachScopeMixin, LoginRequiredMixin, View): cancel_scheduled_publish do the actual work; events.tasks. publish_scheduled_lineups is the periodic sweep that catches a schedule once its time arrives. ``action`` picks which (default "publish_now", - so the plain "Publish" button posts with no extra fields).""" + so the plain "Publish"/"Publish changes" button posts with no extra + fields). Reachable, and does the right thing, whether this is the first + publish or a republish of an already-published lineup a coach kept + editing -- publish_lineup itself is what limits the notification to + whoever's status actually changed. + """ def post(self, request, *args, **kwargs): if not self.can_manage_active_team: @@ -998,8 +1022,12 @@ class CoachLineupPublishView(CoachScopeMixin, LoginRequiredMixin, View): cancel_scheduled_publish(lineup) notify(request, f"s|{_('Schedule cancelled')}|{_('Publish it manually whenever you are ready.')}") else: + was_already_published = lineup.published_at is not None publish_lineup(lineup) - notify(request, f"s|{_('Line-up published')}|{_('Selected players have been notified.')}") + if was_already_published: + notify(request, f"s|{_('Line-up updated')}|{_('Anyone whose status changed has been notified.')}") + else: + notify(request, f"s|{_('Line-up published')}|{_('Selected players have been notified.')}") return HttpResponseRedirect(reverse("mobile:coach_lineup", kwargs={"event_id": event.pk})) diff --git a/mobile/templates/mobile/coach/lineup.html b/mobile/templates/mobile/coach/lineup.html index f3954c1..b7987de 100644 --- a/mobile/templates/mobile/coach/lineup.html +++ b/mobile/templates/mobile/coach/lineup.html @@ -16,6 +16,12 @@ html's own top-of-file comment) -- each row's Yes/No pair is Alpine-owned (x-data toggling a hidden input), and this codebase hasn't established an htmx interaction pattern to layer on top of that safely. + + The Publish section below stays visible after the first publish too (not + just while draft) -- editing selections and hitting "Save line-up" never + notifies anyone by itself (see CoachLineupView's own docstring), so a + coach who changes an already-published lineup needs an explicit + "Publish changes" step to sync it and notify whoever's status changed. {% endcomment %} {% block header_extra %} @@ -43,6 +49,9 @@
{{ row.member.get_full_name }} {% if row.membership.jersey_number %}· #{{ row.membership.jersey_number }}{% endif %} + {% if row.attendance_rate is not None %} + · {% blocktrans with rate=row.attendance_rate %}{{ rate }}% turnout{% endblocktrans %} + {% endif %}
{% if can_manage_active_team %} @@ -78,7 +87,7 @@ {% endif %} - {% if can_manage_active_team and not lineup.published_at %} + {% if can_manage_active_team %} {% if lineup.scheduled_publish_at %}

{% blocktrans with time=lineup.scheduled_publish_at|date:"D d M H:i" %}Scheduled to publish {{ time }}{% endblocktrans %}

@@ -99,7 +108,7 @@
{% csrf_token %} - +
{% trans "Or schedule for later" %} diff --git a/mobile/tests.py b/mobile/tests.py index 3be95dc..810378a 100644 --- a/mobile/tests.py +++ b/mobile/tests.py @@ -4300,6 +4300,29 @@ class CoachLineupViewTests(TestCase): self.assertEqual([row["member"] for row in categories[0]["rows"]], [self.player]) self.assertFalse(categories[0]["rows"][0]["selected"]) + def test_a_player_with_too_little_history_shows_no_turnout_rate(self): + # self.player has exactly one Attendance row (for the upcoming game + # itself, not even a past one) -- well under player_attendance_rankings' + # own minimum_responses floor, so no rate should be attached. + self.client.force_login(self.user) + + response = self.client.get(reverse("mobile:coach_lineup", kwargs={"event_id": self.event.pk}), HTTP_HOST="ajax-united.rosterchief.app") + + self.assertIsNone(response.context["categories"][0]["rows"][0]["attendance_rate"]) + self.assertNotContains(response, "turnout") + + def test_a_player_with_enough_history_shows_a_turnout_rate(self): + for index, status in enumerate([Attendance.AttendanceStatus.PRESENT, Attendance.AttendanceStatus.PRESENT, Attendance.AttendanceStatus.ABSENT]): + past_practice = Event.objects.create(club=self.club, title=f"Practice {index}", kind=Event.EventKind.TRAINING, season=self.season, start=timezone.now() - datetime.timedelta(days=index + 1)) + past_practice.teams.add(self.team) + Attendance.objects.update_or_create(event=past_practice, member=self.player, defaults={"status": status}) + self.client.force_login(self.user) + + response = self.client.get(reverse("mobile:coach_lineup", kwargs={"event_id": self.event.pk}), HTTP_HOST="ajax-united.rosterchief.app") + + self.assertEqual(response.context["categories"][0]["rows"][0]["attendance_rate"], 67) + self.assertContains(response, "67% turnout") + def test_save_selects_the_submitted_players(self): self.client.force_login(self.user) lineup = Lineup.objects.create(event=self.event, team=self.team) @@ -4346,6 +4369,33 @@ class CoachLineupViewTests(TestCase): lineup.refresh_from_db() self.assertIsNotNone(lineup.published_at) + def test_publish_button_still_offered_after_the_lineup_is_published(self): + # A coach can keep editing a published lineup (test_save_selects_the_ + # submitted_players above), and Save alone never notifies anyone -- so + # "Publish changes" has to stay reachable to actually push an edit out. + self.client.force_login(self.user) + Lineup.objects.create(event=self.event, team=self.team, published_at=timezone.now()) + + response = self.client.get(reverse("mobile:coach_lineup", kwargs={"event_id": self.event.pk}), HTTP_HOST="ajax-united.rosterchief.app") + + self.assertContains(response, "Publish changes") + + def test_republishing_only_notifies_players_whose_status_changed(self): + self.client.force_login(self.user) + lineup = Lineup.objects.create(event=self.event, team=self.team) + LineupSelection.objects.create(lineup=lineup, member=self.player) + self.client.post(reverse("mobile:coach_lineup_publish", kwargs={"event_id": self.event.pk}), HTTP_HOST="ajax-united.rosterchief.app") + Notification.objects.filter(member=self.player).delete() + + # Editing selections (Save) alone must not notify -- only a fresh Publish does. + self.client.post(reverse("mobile:coach_lineup", kwargs={"event_id": self.event.pk}), {}, HTTP_HOST="ajax-united.rosterchief.app") + self.assertFalse(Notification.objects.filter(member=self.player).exists()) + + self.client.post(reverse("mobile:coach_lineup_publish", kwargs={"event_id": self.event.pk}), HTTP_HOST="ajax-united.rosterchief.app") + + notification = Notification.objects.get(member=self.player) + self.assertIn("not in it this time", notification.body) + def test_schedule_sets_scheduled_publish_at_without_publishing(self): self.client.force_login(self.user) lineup = Lineup.objects.create(event=self.event, team=self.team)