From 2b6a4d21bb884a1b6eea3979fd57b0c2c82464f3 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Sat, 22 Aug 2026 16:09:36 +0200 Subject: [PATCH] Rationalize news notifications and the Home news teaser for multi-child families A family with several kids on a news item's audience now gets one email/one Notification, not one per child -- deduped on resolved recipient emails, so overlapping (not just identical) guardian sets still collapse correctly. Event notifications are untouched: each child still needs their own reply. Home's "Club news" teaser is also decoupled from the person-scope switcher -- a parent with no team of their own still sees their kids' team news when they've picked their own "Me" chip, not just when "All" is selected. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9 --- mobile/tests.py | 22 ++++++++++++++++++++++ mobile/views.py | 11 ++++++++--- news/tasks.py | 26 ++++++++++++++++++++++++-- news/tests.py | 41 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 94 insertions(+), 6 deletions(-) diff --git a/mobile/tests.py b/mobile/tests.py index 56da9ed..c262052 100644 --- a/mobile/tests.py +++ b/mobile/tests.py @@ -381,6 +381,28 @@ class HomeViewTests(TestCase): self.assertEqual(list(response.context["news_items"]), []) + def test_news_teaser_includes_a_managed_childs_team_news_even_when_scoped_to_me(self): + # A parent with no team of their own, scoped to their own "Me" chip + # (no team membership -> empty team_ids for `people`) should still see + # news about a child's team -- news isn't a per-person action like + # RSVP/dues, so it's keyed off every managed person, not just scope. + 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) + team = Team.objects.create(club=self.club, name="U12", short_name="U12") + position = Position.objects.create(club=self.club, name="Forward", short_name="F") + TeamMembership.objects.create(team=team, member=child, season=self.season, position=position) + team_news = News.objects.create(club=self.club, title="U12 news", body="Body.", status=News.Status.PUBLISHED, published_at=timezone.now()) + team_news.teams.add(team) + self.client.force_login(self.user) + + response = self._get(url=reverse("mobile:home") + f"?as={self.member.pk}") + + self.assertEqual(response.context["scope_person"], self.member) + self.assertIn(team_news, response.context["news_items"]) + def test_news_card_shows_an_empty_state_with_a_link_to_all_news_when_there_is_none(self): self.client.force_login(self.user) diff --git a/mobile/views.py b/mobile/views.py index 3046603..0243991 100644 --- a/mobile/views.py +++ b/mobile/views.py @@ -240,9 +240,14 @@ class HomeView(PersonScopeMixin, LoginRequiredMixin, TemplateView): season = current_season(self.request.club) if season is not None: dues_rows = open_dues_rows(self.request.club, people, season) - team_ids = list(TeamMembership.objects.filter(member__in=people, season=season).values_list("team_id", flat=True)) + # Deliberately self.managed_people, not the scoped `people` above -- + # news isn't a per-person action like RSVP/dues, it's "things this + # account should know about", so a parent picking their own "Me" + # chip (no team of their own) still sees their kids' team news + # rather than only club-wide items. + news_team_ids = list(TeamMembership.objects.filter(member__in=self.managed_people, season=season).values_list("team_id", flat=True)) else: - team_ids = [] + news_team_ids = [] news_items = list( News.objects.filter( @@ -251,7 +256,7 @@ class HomeView(PersonScopeMixin, LoginRequiredMixin, TemplateView): published_at__lte=now, visibility__in=[News.Visibility.INTERNAL, News.Visibility.BOTH], ) - .filter(Q(teams__isnull=True) | Q(teams__id__in=team_ids)) + .filter(Q(teams__isnull=True) | Q(teams__id__in=news_team_ids)) .prefetch_related("teams") .order_by("-published_at") .distinct()[: self.NEWS_LIMIT] diff --git a/news/tasks.py b/news/tasks.py index f46c0f4..4b40635 100644 --- a/news/tasks.py +++ b/news/tasks.py @@ -15,7 +15,7 @@ from django.utils.html import strip_tags from club.models import ClubMembership from club.services.access import current_season from members.models import Member -from notifications.services import notify_members +from notifications.services import notify_members, recipient_emails from .models import News from .services import render_body_html @@ -43,6 +43,28 @@ def _notify_audience(news_item): return members.distinct() +def _dedupe_by_recipients(members): + """Collapses siblings (or anyone else sharing a guardian) down to one + notification each, keyed on where the email would actually land -- not + family membership itself, since a blended family's kids don't + necessarily share the exact same guardian set, only some overlap. A + parent of three kids all on the news audience gets one email, not three; + each kid still gets their own row (and read state) via events.tasks. + notify_new_event, which is deliberately untouched by this -- an event + needs a reply per child, a news post doesn't. Members nobody's reachable + for (no login, no guardian) are never deduped against anything -- there's + no shared inbox to spare.""" + seen_emails = set() + representatives = [] + for member in members: + emails = recipient_emails(member) + if emails and any(email in seen_emails for email in emails): + continue + representatives.append(member) + seen_emails.update(emails) + return representatives + + def _plain_text_body(news_item) -> str: """The notification's own plain-text body: rendered from the same Markdown source the public site would use, then stripped back to plain @@ -61,7 +83,7 @@ def notify_news_published(news_id): # (this task isn't registered for that anyway -- see the module docstring). return "Skipped: not published." - members = list(_notify_audience(news_item)) + members = _dedupe_by_recipients(_notify_audience(news_item)) if not members: return "Skipped: no current-season active members to notify." diff --git a/news/tests.py b/news/tests.py index 46b68c3..bbe30a0 100644 --- a/news/tests.py +++ b/news/tests.py @@ -8,7 +8,7 @@ from django.test import TestCase from django.utils import timezone from club.models import Club, ClubMembership, ClubRole, Season -from members.models import Member +from members.models import Family, FamilyMembership, Member from notifications.models import Notification from teams.models import Position, Team, TeamMembership @@ -210,6 +210,45 @@ class NotifyNewsPublishedTests(TestCase): self.assertEqual(result, "Skipped: not published.") self.assertFalse(Notification.objects.exists()) + def test_siblings_sharing_a_guardian_are_notified_once(self): + # Two children, no login of their own, both reachable only through the + # same parent -- one Notification/one email for the family, not two. + parent_user = User.objects.create_user(email="parent@example.com", password="pw-secret-123") + parent = Member.objects.create(first_name="Pat", last_name="Parent", email="parent@example.com", user=parent_user) + family = Family.objects.create(name="Parent family") + FamilyMembership.objects.create(family=family, member=parent, role=FamilyMembership.FamilyRole.PARENT) + child_a = Member.objects.create(first_name="Ana", last_name="Parent") + child_b = Member.objects.create(first_name="Ben", last_name="Parent") + for child in (child_a, child_b): + FamilyMembership.objects.create(family=family, member=child, role=FamilyMembership.FamilyRole.CHILD) + ClubMembership.objects.create(club=self.club, member=child, season=self.season, status=ClubMembership.StatusChoices.ACTIVE) + news_item = News.objects.create(club=self.club, title="Club news", body="Body.", status=News.Status.PUBLISHED, published_at=timezone.now()) + + result = notify_news_published(news_item.pk) + + self.assertEqual(Notification.objects.filter(club=self.club, title="Club news").count(), 1) + self.assertIn("Notified 1", result) + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(mail.outbox[0].to, ["parent@example.com"]) + + def test_children_with_different_guardians_are_each_notified(self): + family_one = Family.objects.create(name="First family") + family_two = Family.objects.create(name="Second family") + for family_name, family in (("one", family_one), ("two", family_two)): + parent_user = User.objects.create_user(email=f"parent-{family_name}@example.com", password="pw-secret-123") + parent = Member.objects.create(first_name=f"Parent{family_name}", last_name="Adult", email=f"parent-{family_name}@example.com", user=parent_user) + FamilyMembership.objects.create(family=family, member=parent, role=FamilyMembership.FamilyRole.PARENT) + child = Member.objects.create(first_name=f"Child{family_name}", last_name="Kid") + FamilyMembership.objects.create(family=family, member=child, role=FamilyMembership.FamilyRole.CHILD) + ClubMembership.objects.create(club=self.club, member=child, season=self.season, status=ClubMembership.StatusChoices.ACTIVE) + news_item = News.objects.create(club=self.club, title="Club news", body="Body.", status=News.Status.PUBLISHED, published_at=timezone.now()) + + result = notify_news_published(news_item.pk) + + self.assertEqual(Notification.objects.filter(club=self.club, title="Club news").count(), 2) + self.assertIn("Notified 2", result) + self.assertEqual(len(mail.outbox), 2) + def test_the_body_is_plain_text_not_markdown(self): member = self.make_member("Jamie", email="jamie@example.com") news_item = News.objects.create(club=self.club, title="News", body="**Bold** text.", status=News.Status.PUBLISHED, published_at=timezone.now())