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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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."
|
||||
|
||||
|
||||
@@ -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())
|
||||
|
||||
Reference in New Issue
Block a user