From 397b9cf687dc55e6b408cc717dcb92e424681907 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Fri, 21 Aug 2026 14:22:40 +0200 Subject: [PATCH] Add an "All" scope to the person switcher, default once there's a family A lone member (or a parent of exactly one child) still lands straight on their own record and never sees the switcher, same as before. The moment there's more than one managed person, an "All" chip appears first and is selected by default -- Home's cards (hero, needs-your-answer, dues) now aggregate across everyone in scope instead of just one person, and Calendar's own "My schedule" default does the same for its agenda. PersonScopeMixin gains ``scope_everyone`` (bool) and ``people_in_scope`` (the effective list to filter by, regardless of which mode is active) -- kept separate from Calendar's own, unrelated ``?scope=all`` toggle ("every club event" vs. this "every person I manage"). Every RSVP form now names its target member explicitly (``member_id``) rather than relying on the old implicit "whoever is currently scoped" fallback, which stops working the moment "All" -- not one person -- is the default. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9 --- mobile/mixins.py | 31 +++- mobile/templates/mobile/_calendar_row.html | 1 + mobile/templates/mobile/base.html | 8 +- mobile/templates/mobile/calendar.html | 2 +- mobile/templates/mobile/home.html | 53 ++++--- mobile/tests.py | 162 ++++++++++++++++++++- mobile/views.py | 66 +++++---- 7 files changed, 261 insertions(+), 62 deletions(-) diff --git a/mobile/mixins.py b/mobile/mixins.py index 3be406f..fd9d974 100644 --- a/mobile/mixins.py +++ b/mobile/mixins.py @@ -21,14 +21,24 @@ class PersonScopeMixin(ClubScopedPublicMixin): ``?as=`` re-scopes the current screen to one managed person, same as the design doc's horizontally-scrolling chip row -- it re-scopes in - place rather than navigating. Falls back to the account's own Member, then - to the first managed child (e.g. a parent with no Member record of their own). + place rather than navigating. ``?as=all`` (or, once there's more than one + managed person, no ``?as=`` at all) scopes to *every* managed person -- + that's the default the moment there's actually something to aggregate; a + member on their own, or a parent of exactly one child, has nothing to + aggregate and just lands on that one record, same as before this existed. + + Screens that only ever operate on one person at a time keep using + ``scope_person`` (``None`` in "everyone" mode); screens that can + meaningfully show several people at once (Home's cards, Calendar's own + "my schedule" listing) should use ``people_in_scope`` instead, which is + always the right list to filter by regardless of which mode is active. """ def dispatch(self, request, *args, **kwargs): self.me = Member.objects.filter(user=request.user).first() if request.user.is_authenticated else None self.managed_people = self._managed_people(request) - self.scope_person = self._resolve_scope_person(request) + self.scope_everyone, self.scope_person = self._resolve_scope(request) + self.people_in_scope = self.managed_people if self.scope_everyone else ([self.scope_person] if self.scope_person else []) return super().dispatch(request, *args, **kwargs) def _managed_people(self, request): @@ -44,13 +54,19 @@ class PersonScopeMixin(ClubScopedPublicMixin): ) return [self.me, *children] - def _resolve_scope_person(self, request): + def _resolve_scope(self, request) -> tuple[bool, Member | None]: + """Returns ``(scope_everyone, scope_person)`` -- exactly one of the + two is ever meaningful at a time (the other is ``False``/``None``).""" requested_id = request.GET.get("as") - if requested_id: + if requested_id and requested_id != "all": for person in self.managed_people: if str(person.pk) == requested_id: - return person - return self.managed_people[0] if self.managed_people else None + return False, person + if requested_id == "all": + return True, None + if len(self.managed_people) > 1: + return True, None + return False, (self.managed_people[0] if self.managed_people else None) def get_context_data(self, **kwargs): unread_notification_count = 0 @@ -61,6 +77,7 @@ class PersonScopeMixin(ClubScopedPublicMixin): me=self.me, managed_people=self.managed_people, scope_person=self.scope_person, + scope_everyone=self.scope_everyone, has_staff_access=self.me is not None and has_management_access(self.request.user, self.request.club), unread_notification_count=unread_notification_count, season=current_season(self.request.club), diff --git a/mobile/templates/mobile/_calendar_row.html b/mobile/templates/mobile/_calendar_row.html index 606cc70..ffe731a 100644 --- a/mobile/templates/mobile/_calendar_row.html +++ b/mobile/templates/mobile/_calendar_row.html @@ -18,6 +18,7 @@
{{ row.event.title }}
{{ row.event.start|date:"H:i" }} + {% if row.member %}· {{ row.member.first_name }}{% endif %} {% for team in row.event.teams.all %}· {{ team.name }}{% endfor %} {% if row.event.location %}· {{ row.event.location.name }}{% endif %}
diff --git a/mobile/templates/mobile/base.html b/mobile/templates/mobile/base.html index 7203342..2cf714b 100644 --- a/mobile/templates/mobile/base.html +++ b/mobile/templates/mobile/base.html @@ -73,8 +73,14 @@ {% if managed_people|length > 1 %}
+ + + + + {% trans "All" %} + {% for person in managed_people %} - + {{ person.first_name|slice:":1" }} {% if person == me %}{% trans "Me" %}{% else %}{{ person.first_name }}{% endif %} diff --git a/mobile/templates/mobile/calendar.html b/mobile/templates/mobile/calendar.html index c9e375c..3defe8f 100644 --- a/mobile/templates/mobile/calendar.html +++ b/mobile/templates/mobile/calendar.html @@ -22,7 +22,7 @@
- {% if not scope_all and not scope_person %} + {% if not scope_all and not managed_people %}

{% trans "No one to show yet" %}

{% trans "Once you're linked to a member record, their schedule will show up here." %}

diff --git a/mobile/templates/mobile/home.html b/mobile/templates/mobile/home.html index 99c93f4..2672ee1 100644 --- a/mobile/templates/mobile/home.html +++ b/mobile/templates/mobile/home.html @@ -3,13 +3,16 @@ {% comment %} M1 -- design_handoff_rosterchief_platform/README.md's M1 section. Four - independently-optional cards, all scoped to scope_person (mobile/mixins.py): - a hero for the soonest upcoming event with a quick In/Out RSVP, a "needs - your answer" list, a season-dues card, and a news teaser. + independently-optional cards, scoped to people_in_scope (mobile/mixins.py) + -- everyone the account manages once there's more than one (the header's + "All" chip, on by default), or just the one person picked from the chip + row. A hero for the soonest upcoming event with a quick In/Out RSVP, a + "needs your answer" list, a season-dues card per person who owes money, + and a news teaser. {% endcomment %} {% block content %} - {% if scope_person %} + {% if managed_people %} {% if hero_attendance %}
@@ -29,15 +32,17 @@

{% trans "Replies are closed for this event" %}

{{ hero_attendance.get_status_display }} {% else %} -

{% blocktrans with name=scope_person.first_name %}{{ name }} — are you in?{% endblocktrans %}

+

{% blocktrans with name=hero_attendance.member.first_name %}{{ name }} — are you in?{% endblocktrans %}

{% csrf_token %} +
{% csrf_token %} +
@@ -63,7 +68,7 @@
{{ attendance.event.title }}
-
{{ scope_person.first_name }} · {{ attendance.event.start|date:"H:i" }}
+
{{ attendance.member.first_name }} · {{ attendance.event.start|date:"H:i" }}
{% trans "Reply" %} @@ -72,22 +77,26 @@
{% endif %} - {% if dues_membership %} -
-
- € -
-
-
{% blocktrans with name=scope_person.first_name %}Season dues — {{ name }}{% endblocktrans %}
-
- {% if dues_invoice %} - {% blocktrans with amount=dues_balance|floatformat:2 due=dues_invoice.due_date|date:"d M" %}€ {{ amount }} · due {{ due }}{% endblocktrans %} - {% else %} - {% blocktrans with amount=dues_balance|floatformat:2 %}€ {{ amount }}{% endblocktrans %} - {% endif %} + {% if dues_rows %} +
+ {% for row in dues_rows %} +
+
+ € +
+
+
{% blocktrans with name=row.membership.member.first_name %}Season dues — {{ name }}{% endblocktrans %}
+
+ {% if row.invoice %} + {% blocktrans with amount=row.balance|floatformat:2 due=row.invoice.due_date|date:"d M" %}€ {{ amount }} · due {{ due }}{% endblocktrans %} + {% else %} + {% blocktrans with amount=row.balance|floatformat:2 %}€ {{ amount }}{% endblocktrans %} + {% endif %} +
+
+ {% trans "Pay" %}
-
- {% trans "Pay" %} + {% endfor %}
{% endif %} @@ -108,7 +117,7 @@
{% endif %} - {% if not hero_attendance and not needs_answer and not dues_membership and not news_item %} + {% if not hero_attendance and not needs_answer and not dues_rows and not news_item %}

{% trans "Nothing to show right now." %}

diff --git a/mobile/tests.py b/mobile/tests.py index 0b863ad..a995a49 100644 --- a/mobile/tests.py +++ b/mobile/tests.py @@ -95,6 +95,26 @@ class MobileShellTests(TestCase): self.assertEqual(response.context["scope_person"], child) + def test_all_chip_only_appears_once_theres_more_than_one_managed_person(self): + self.client.force_login(self.user) + + response = self._get("home") + + self.assertNotContains(response, 'href="?as=all"') + + def test_all_chip_appears_and_is_selected_by_default_with_a_child(self): + family = Family.objects.create(name="Bakker") + FamilyMembership.objects.create(family=family, member=self.member, role=FamilyMembership.FamilyRole.PARENT) + child = Member.objects.create(first_name="Noor", last_name="Bakker") + FamilyMembership.objects.create(family=family, member=child, role=FamilyMembership.FamilyRole.CHILD) + ClubMembership.objects.create(club=self.club, member=child, season=Season.objects.first()) + self.client.force_login(self.user) + + response = self._get("home") + + self.assertContains(response, 'href="?as=all"') + self.assertTrue(response.context["scope_everyone"]) + @override_settings( VAPID_PRIVATE_KEY="", @@ -231,7 +251,7 @@ class HomeViewTests(TestCase): response = self._get("home") - self.assertEqual(response.context["dues_balance"], Decimal("420.00")) + self.assertEqual(response.context["dues_rows"][0]["balance"], Decimal("420.00")) self.assertContains(response, "420") def test_dues_card_is_absent_when_nothing_is_owed(self): @@ -239,7 +259,7 @@ class HomeViewTests(TestCase): response = self._get("home") - self.assertIsNone(response.context["dues_membership"]) + self.assertEqual(response.context["dues_rows"], []) def test_news_teaser_shows_the_latest_published_item(self): News.objects.create(club=self.club, title="Old news", body="Body.", status=News.Status.PUBLISHED, published_at=timezone.now() - datetime.timedelta(days=5)) @@ -262,6 +282,83 @@ class HomeViewTests(TestCase): self.assertIsNone(response.context["scope_person"]) self.assertContains(response, "No one to show yet") + def add_child(self, first_name="Noor"): + family = Family.objects.create(name="Bakker") + FamilyMembership.objects.create(family=family, member=self.member, role=FamilyMembership.FamilyRole.PARENT) + child = Member.objects.create(first_name=first_name, last_name="Bakker") + FamilyMembership.objects.create(family=family, member=child, role=FamilyMembership.FamilyRole.CHILD) + ClubMembership.objects.create(club=self.club, member=child, season=self.season) + return child + + def test_all_is_the_default_scope_once_theres_more_than_one_managed_person(self): + self.add_child() + self.client.force_login(self.user) + + response = self._get("home") + + self.assertTrue(response.context["scope_everyone"]) + self.assertIsNone(response.context["scope_person"]) + + def test_a_lone_member_never_defaults_to_all(self): + # No child added -- managed_people is just [self.member]. + self.client.force_login(self.user) + + response = self._get("home") + + self.assertFalse(response.context["scope_everyone"]) + self.assertEqual(response.context["scope_person"], self.member) + + def test_all_scope_aggregates_the_hero_across_every_managed_person(self): + child = self.add_child() + mine = self.make_event(title="Lars's practice", start=self.future) + theirs = self.make_event(title="Noor's game", start=self.future - datetime.timedelta(hours=1)) + Attendance.objects.create(event=mine, member=self.member) + Attendance.objects.create(event=theirs, member=child) + self.client.force_login(self.user) + + response = self._get("home") + + self.assertEqual(response.context["hero_attendance"].event, theirs) + self.assertContains(response, "Noor") + + def test_all_scope_combines_needs_answer_and_dues_across_everyone(self): + child = self.add_child() + mine = self.make_event(title="Lars's practice", start=self.future) + theirs = self.make_event(title="Noor's game", start=self.future + datetime.timedelta(days=1)) + Attendance.objects.create(event=mine, member=self.member, status=Attendance.AttendanceStatus.NO_RESPONSE) + Attendance.objects.create(event=theirs, member=child, status=Attendance.AttendanceStatus.NO_RESPONSE) + child_membership = ClubMembership.objects.get(club=self.club, member=child, season=self.season) + child_membership.fee_amount = Decimal("100.00") + child_membership.save(update_fields=["fee_amount"]) + self.client.force_login(self.user) + + response = self._get("home") + + needs_answer_events = {a.event for a in response.context["needs_answer"]} + self.assertEqual(needs_answer_events, {theirs}) # mine is the hero, excluded from the list + self.assertEqual(len(response.context["dues_rows"]), 1) + self.assertEqual(response.context["dues_rows"][0]["membership"].member, child) + + def test_selecting_one_specific_person_narrows_back_to_just_them(self): + child = self.add_child() + theirs = self.make_event(title="Noor's game", start=self.future) + Attendance.objects.create(event=theirs, member=child, status=Attendance.AttendanceStatus.NO_RESPONSE) + self.client.force_login(self.user) + + response = self._get(url=reverse("mobile:home") + f"?as={self.member.pk}") + + self.assertFalse(response.context["scope_everyone"]) + self.assertEqual(response.context["scope_person"], self.member) + self.assertEqual(list(response.context["needs_answer"]), []) + + def test_as_all_explicitly_selects_everyone(self): + self.add_child() + self.client.force_login(self.user) + + response = self._get(url=reverse("mobile:home") + "?as=all") + + self.assertTrue(response.context["scope_everyone"]) + @override_settings(ROSTERCHIEF_BASE_DOMAIN="rosterchief.app", ALLOWED_HOSTS=["rosterchief.app", "ajax-united.rosterchief.app", "testserver"]) class EventDetailRsvpTests(TestCase): @@ -305,6 +402,25 @@ class EventDetailRsvpTests(TestCase): response = self._post(self.event, {"status": "present", "member_id": str(stranger.pk)}) self.assertEqual(response.status_code, 400) + + def test_explicit_member_id_still_works_once_all_is_the_default_scope(self): + # Home's hero form always sends an explicit member_id -- this must keep + # working once a second managed person makes "All" the default scope + # (scope_person is None in that case, so the old implicit fallback alone + # would no longer resolve who's RSVPing). + family = Family.objects.create(name="Bakker") + FamilyMembership.objects.create(family=family, member=self.member, role=FamilyMembership.FamilyRole.PARENT) + child = Member.objects.create(first_name="Noor", last_name="Bakker") + FamilyMembership.objects.create(family=family, member=child, role=FamilyMembership.FamilyRole.CHILD) + ClubMembership.objects.create(club=self.club, member=child, season=self.season) + child_attendance = Attendance.objects.create(event=self.event, member=child, status=Attendance.AttendanceStatus.NO_RESPONSE) + self.client.force_login(self.user) + + response = self._post(self.event, {"status": "present", "member_id": str(child.pk)}) + + self.assertRedirects(response, reverse("mobile:home"), fetch_redirect_response=False) + child_attendance.refresh_from_db() + self.assertEqual(child_attendance.status, Attendance.AttendanceStatus.PRESENT) self.attendance.refresh_from_db() self.assertEqual(self.attendance.status, Attendance.AttendanceStatus.NO_RESPONSE) @@ -414,6 +530,48 @@ class CalendarViewTests(TestCase): self.assertNotIn(cancelled, self._events_in_context(response)) + def add_child(self, first_name="Noor"): + family = Family.objects.create(name="Bakker") + FamilyMembership.objects.create(family=family, member=self.member, role=FamilyMembership.FamilyRole.PARENT) + child = Member.objects.create(first_name=first_name, last_name="Bakker") + FamilyMembership.objects.create(family=family, member=child, role=FamilyMembership.FamilyRole.CHILD) + ClubMembership.objects.create(club=self.club, member=child, season=self.season) + return child + + def test_my_schedule_aggregates_across_every_managed_person_once_all_is_the_default(self): + child = self.add_child() + mine = self.make_event(title="Lars's practice") + theirs = self.make_event(title="Noor's game") + Attendance.objects.create(event=mine, member=self.member) + Attendance.objects.create(event=theirs, member=child) + self.client.force_login(self.user) + + response = self._get() + + self.assertEqual(self._events_in_context(response), {mine, theirs}) + self.assertContains(response, "Noor") + + def test_selecting_one_person_narrows_my_schedule_back_to_just_them(self): + child = self.add_child() + mine = self.make_event(title="Lars's practice") + theirs = self.make_event(title="Noor's game") + Attendance.objects.create(event=mine, member=self.member) + Attendance.objects.create(event=theirs, member=child) + self.client.force_login(self.user) + + response = self._get(**{"as": self.member.pk}) + + self.assertEqual(self._events_in_context(response), {mine}) + + def test_my_schedule_with_no_managed_people_shows_the_empty_state_not_a_500(self): + bare_user = User.objects.create_user(email="new@example.com", password="pw-secret-123") + self.client.force_login(bare_user) + + response = self._get() + + self.assertEqual(response.status_code, 200) + self.assertContains(response, "No one to show yet") + def test_events_outside_the_two_week_window_are_excluded(self): far_future = self.make_event(title="Far future game", start=timezone.now() + datetime.timedelta(days=30)) past = self.make_event(title="Past practice", start=timezone.now() - datetime.timedelta(days=1)) diff --git a/mobile/views.py b/mobile/views.py index 447a189..9f57f3b 100644 --- a/mobile/views.py +++ b/mobile/views.py @@ -139,11 +139,18 @@ class _PlaceholderScreen(PersonScopeMixin, LoginRequiredMixin, TemplateView): class HomeView(PersonScopeMixin, LoginRequiredMixin, TemplateView): """M1 -- design_handoff_rosterchief_platform/README.md's M1 section: a - hero card for scope_person's soonest upcoming event (with a quick In/Out - RSVP -- see EventDetailView.post below), a "needs your answer" list of - upcoming events still NO_RESPONSE/MAYBE, a season-dues card when money is - owed, and a news teaser. Every card is independently optional -- an - empty-state screen is just the four `if`s below all being falsy. + hero card for the soonest upcoming event across everyone currently in + scope (with a quick In/Out RSVP -- see EventDetailView.post below), a + "needs your answer" list of upcoming events still NO_RESPONSE/MAYBE, a + season-dues card per person who owes money, and a news teaser. Every + card is independently optional -- an empty-state screen is just the + four `if`s below all being falsy. + + Scoped to ``self.people_in_scope`` (mobile/mixins.py), not just + ``scope_person`` -- with the header's "All" chip now the default the + moment there's more than one managed person, this screen aggregates + across everyone in that case rather than showing only one person's + cards. """ template_name = "mobile/home.html" @@ -151,24 +158,22 @@ class HomeView(PersonScopeMixin, LoginRequiredMixin, TemplateView): active_tab = "home" def get_context_data(self, **kwargs): - scope_person = self.scope_person + people = self.people_in_scope now = timezone.now() hero_attendance = None rsvp_closed = False needs_answer = [] - dues_membership = None - dues_balance = None - dues_invoice = None + dues_rows = [] news_item = None - if scope_person is not None: + if people: upcoming = Attendance.objects.filter( - member=scope_person, + member__in=people, event__club=self.request.club, event__cancelled=False, event__start__gte=now, - ).select_related("event", "event__location") + ).select_related("event", "event__location", "member") hero_attendance = upcoming.order_by("event__start").first() if hero_attendance is not None: @@ -182,20 +187,17 @@ class HomeView(PersonScopeMixin, LoginRequiredMixin, TemplateView): season = current_season(self.request.club) if season is not None: - membership = ( - ClubMembership.objects.filter(club=self.request.club, member=scope_person, season=season) + memberships = ( + ClubMembership.objects.filter(club=self.request.club, member__in=people, season=season) .exclude(fee_status=ClubMembership.FeeStatus.WAIVED) - .select_related("dues_invoice") - .first() + .select_related("dues_invoice", "member") ) - if membership is not None: + for membership in memberships: balance = remaining_balance(membership) if balance > 0: - dues_membership = membership - dues_balance = balance - dues_invoice = getattr(membership, "dues_invoice", None) + dues_rows.append({"membership": membership, "balance": balance, "invoice": getattr(membership, "dues_invoice", None)}) - team_ids = list(TeamMembership.objects.filter(member=scope_person, season=season).values_list("team_id", flat=True)) + team_ids = list(TeamMembership.objects.filter(member__in=people, season=season).values_list("team_id", flat=True)) else: team_ids = [] @@ -211,9 +213,7 @@ class HomeView(PersonScopeMixin, LoginRequiredMixin, TemplateView): hero_attendance=hero_attendance, rsvp_closed=rsvp_closed, needs_answer=needs_answer, - dues_membership=dues_membership, - dues_balance=dues_balance, - dues_invoice=dues_invoice, + dues_rows=dues_rows, news_item=news_item, news_team=news_item.teams.first() if news_item is not None else None, **kwargs, @@ -266,20 +266,25 @@ class CalendarView(PersonScopeMixin, LoginRequiredMixin, TemplateView): .order_by("start") ) rows = [{"event": event, "pill_class": "pill-info", "pill_label": event.get_kind_display()} for event in events] - elif self.scope_person is not None: + elif self.people_in_scope: attendances = ( Attendance.objects.filter( - member=self.scope_person, + member__in=self.people_in_scope, event__club=self.request.club, event__cancelled=False, event__start__gte=now, event__start__lte=window_end, ) - .select_related("event", "event__location", "event__opponent") + .select_related("event", "event__location", "event__opponent", "member") .prefetch_related("event__teams") .order_by("event__start") ) - rows = [{"event": attendance.event, "pill_class": self.STATUS_PILL_CLASSES.get(attendance.status, "pill-neutral"), "pill_label": attendance.get_status_display()} for attendance in attendances] + # Only worth naming whose row it is once "everyone" is aggregating more + # than one person -- a single scoped person's own agenda doesn't need it. + rows = [ + {"event": attendance.event, "pill_class": self.STATUS_PILL_CLASSES.get(attendance.status, "pill-neutral"), "pill_label": attendance.get_status_display(), "member": attendance.member if self.scope_everyone else None} + for attendance in attendances + ] this_week, next_week = [], [] for row in rows: @@ -363,7 +368,10 @@ class EventDetailView(PersonScopeMixin, LoginRequiredMixin, TemplateView): if status not in (Attendance.AttendanceStatus.PRESENT, Attendance.AttendanceStatus.ABSENT, Attendance.AttendanceStatus.MAYBE): return HttpResponseBadRequest(_("Unknown RSVP status.")) - member_id = request.POST.get("member_id") or (str(self.scope_person.pk) if self.scope_person else None) + # Every current caller (Home's hero, M2's per-person rows) always sends an + # explicit member_id -- this is just a defensive fallback for one that doesn't. + fallback_member = self.scope_person or self.me + member_id = request.POST.get("member_id") or (str(fallback_member.pk) if fallback_member else None) member = next((person for person in self.managed_people if str(person.pk) == member_id), None) if member_id else None if member is None: return HttpResponseBadRequest(_("You can't RSVP for that person."))