Notify members when a new event is planned
New events.tasks.notify_new_event, scheduled from management.views.EventCreateView.form_valid -- the deliberate "a staff member planned one new event" action, not every Event row that happens to get created. A recurring series' rolling-horizon extension (extend_event_series) and bulk fixture imports are NOT wired to this on purpose: either would flood everyone with one push per occurrence instead of the single, deliberate action this is meant to catch. Notifies whoever is still NO_RESPONSE right after creation -- exactly "everyone who needs to respond", using the existing notify_members() -> Notification -> mobile.signals push chain already in place, so this is in-app (visible in the M7 inbox, mark read/mark-all-read) and a push notification both, with no new plumbing needed there. Generalized mobile.views.NotificationsView's News-only "tap to open the source" handling (previously isinstance(source, News)) into _notification_source_link, covering Event sources too -- tapping an "new event" notification now marks it read and opens the event's answer screen, the same way a news notification already opened the article. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ECGMEwrc2k4D8VQuwjstj9
This commit is contained in:
@@ -1,15 +1,17 @@
|
||||
"""Celery task behind the `extend-event-series` beat schedule entry (see
|
||||
rosterchief/settings.CELERY_BEAT_SCHEDULE and features/jobs.py).
|
||||
|
||||
Mirrors `manage.py extend_event_series` exactly -- that command still exists, unchanged, for
|
||||
manual use from a shell (see events/management/commands/extend_event_series.py).
|
||||
"""Celery tasks for events -- the `extend-event-series` beat schedule entry
|
||||
(see rosterchief/settings.CELERY_BEAT_SCHEDULE and features/jobs.py), and
|
||||
notifying members when a new event needs their RSVP.
|
||||
"""
|
||||
|
||||
from celery import shared_task
|
||||
from django.utils import timezone
|
||||
from django.utils.translation import gettext as _
|
||||
|
||||
from events.models import EventSeries
|
||||
from events.models import Attendance, Event, EventSeries
|
||||
from events.services import generate_occurrences, horizon
|
||||
from features.models import Maintenance
|
||||
from members.models import Member
|
||||
from notifications.services import notify_members
|
||||
|
||||
|
||||
@shared_task(name="events.tasks.extend_event_series")
|
||||
@@ -26,3 +28,32 @@ def extend_event_series():
|
||||
total += len(generate_occurrences(series, until))
|
||||
|
||||
return f"Generated {total} occurrence(s) across {EventSeries.objects.count()} series."
|
||||
|
||||
|
||||
@shared_task(name="events.tasks.notify_new_event")
|
||||
def notify_new_event(event_id):
|
||||
"""Scheduled from management.views.EventCreateView.form_valid -- a staff
|
||||
member deliberately planning one new event, not every Event row that
|
||||
happens to get created (a recurring series' rolling-horizon extension
|
||||
via extend_event_series above, or a bulk fixture import, would otherwise
|
||||
flood everyone with one push per occurrence; those are intentionally not
|
||||
wired to this).
|
||||
|
||||
Notifies only whoever is still NO_RESPONSE -- right after creation every
|
||||
invited member starts there (events.services.attendance.sync_event_attendances
|
||||
just ran via the Event post_save signal), so this is exactly "everyone
|
||||
who needs to respond", not the full invited list.
|
||||
"""
|
||||
event = Event.objects.filter(pk=event_id, cancelled=False).select_related("club").first()
|
||||
if event is None:
|
||||
return "Skipped: event no longer exists or was cancelled."
|
||||
|
||||
member_ids = Attendance.objects.filter(event=event, status=Attendance.AttendanceStatus.NO_RESPONSE).values_list("member_id", flat=True)
|
||||
members = Member.objects.filter(id__in=member_ids)
|
||||
if not members:
|
||||
return "Skipped: no one to notify."
|
||||
|
||||
when = timezone.localtime(event.start).strftime("%a %d %b, %H:%M")
|
||||
body = _("New %(kind)s: %(when)s. Let us know if you can make it.") % {"kind": event.get_kind_display(), "when": when}
|
||||
notifications = notify_members(members, club=event.club, title=event.title, body=body, source=event)
|
||||
return f"Notified {len(notifications)} member(s)."
|
||||
|
||||
@@ -5158,6 +5158,18 @@ class EventManagementTests(ManagementTestBase):
|
||||
self.assertRedirects(response, reverse("management:event_detail", args=[event.pk]))
|
||||
self.assertIn(self.own_team, event.teams.all())
|
||||
|
||||
def test_creating_an_event_notifies_the_invited_roster(self):
|
||||
position = Position.objects.create(club=self.club, name="Forward", short_name="F")
|
||||
player = Member.objects.create(first_name="Poul", last_name="Player")
|
||||
TeamMembership.objects.create(team=self.own_team, member=player, season=self.season, position=position)
|
||||
self.client.force_login(self.own_team_coach)
|
||||
|
||||
self.club_post("event_create", self.event_data())
|
||||
|
||||
event = Event.objects.get(title="Training")
|
||||
notification = Notification.objects.get(member=player, content_type__model="event", object_id=str(event.pk))
|
||||
self.assertEqual(notification.title, "Training")
|
||||
|
||||
def test_creating_an_event_with_a_same_club_location_does_not_raise_a_cross_club_error(self):
|
||||
# Regression: Event.clean() rejects a location from another club by
|
||||
# comparing against self.club_id, which was still None on a brand-new
|
||||
|
||||
@@ -42,6 +42,7 @@ from events.services.competitions import CompetitionFetchError, fetch_game_info
|
||||
from events.services.rbihf_import import RBIHFImportError, apply_plan, build_plan, extract_team_id, fetch_html
|
||||
from events.services.recurrence import cancel_occurrence, detach_occurrence, generate_occurrences, propagate_series
|
||||
from events.services.referees import RefereeAssignmentError, add_external_referee, assign_referee, conflicting_events, eligible_referees, needs_referee_management, remove_referee, set_referee_fee
|
||||
from events.tasks import notify_new_event
|
||||
from formbuilder.models import Form as FormBuilderForm
|
||||
from formbuilder.models import Submission
|
||||
from members.forms import ClaimRejectForm, ClaimReviewForm
|
||||
@@ -2795,6 +2796,10 @@ class EventCreateView(ClubStaffRequiredMixin, CreateView):
|
||||
response = super().form_valid(form)
|
||||
body = _("“%(event)s” created.") % {"event": self.object}
|
||||
notify(self.request, f"s|{_('Event created')}|{body}")
|
||||
# A deliberately-planned single event, not every Event row that ends up
|
||||
# created (see notify_new_event's own docstring for why a recurring
|
||||
# series' occurrences and bulk fixture imports aren't wired to this).
|
||||
notify_new_event.delay(str(self.object.pk))
|
||||
return response
|
||||
|
||||
def get_success_url(self):
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
{% load i18n %}
|
||||
{% comment %}
|
||||
One M7 notification row -- included from notifications.html once per day
|
||||
bucket. Expects ``row`` ({notification, news_item}) in scope. The whole
|
||||
row is the tap target (a submit button styled full-width, since a POST
|
||||
form can't wrap another form/link) -- tapping always marks it read, and
|
||||
also navigates to the linked News item when there is one (see
|
||||
bucket. Expects ``row`` ({notification, source_label}) in scope. The
|
||||
whole row is the tap target (a submit button styled full-width, since a
|
||||
POST form can't wrap another form/link) -- tapping always marks it read,
|
||||
and also navigates to the linked News/Event when its source resolves to
|
||||
one (see mobile.views._notification_source_link and
|
||||
NotificationsView.post). A club-coloured bar marks it unread, same
|
||||
treatment as management/templates/management/home.html's own
|
||||
notifications card.
|
||||
@@ -18,7 +19,7 @@
|
||||
<span class="min-w-0 flex-1">
|
||||
<span class="block text-sm font-semibold text-ink">{{ row.notification.title }}</span>
|
||||
<span class="block truncate text-xs text-muted">{{ row.notification.body|truncatechars:120 }}</span>
|
||||
{% if row.news_item %}<span class="block font-display text-[11px] font-extrabold text-club uppercase tracking-wide">{% trans "Club news" %}</span>{% endif %}
|
||||
{% if row.source_label %}<span class="block font-display text-[11px] font-extrabold text-club uppercase tracking-wide">{{ row.source_label }}</span>{% endif %}
|
||||
</span>
|
||||
<span class="shrink-0 font-mono text-[11px] text-dim">{% blocktrans with time=row.notification.created|timesince %}{{ time }} ago{% endblocktrans %}</span>
|
||||
</button>
|
||||
|
||||
@@ -897,6 +897,17 @@ class NotificationsViewTests(TestCase):
|
||||
redirect_response = self._post({"action": "mark_read", "notification_id": str(notification.pk)})
|
||||
self.assertRedirects(redirect_response, reverse("mobile:news_detail", kwargs={"slug": news_item.slug}), fetch_redirect_response=False)
|
||||
|
||||
def test_notification_with_an_event_source_is_labelled_and_mark_read_redirects_to_it(self):
|
||||
event = Event.objects.create(club=self.club, title="Practice", start=timezone.now() + datetime.timedelta(days=1))
|
||||
notification = Notification.objects.create(club=self.club, member=self.member, title="New event", body="Body.", source=event)
|
||||
self.client.force_login(self.user)
|
||||
|
||||
response = self._get()
|
||||
self.assertContains(response, "New event")
|
||||
|
||||
redirect_response = self._post({"action": "mark_read", "notification_id": str(notification.pk)})
|
||||
self.assertRedirects(redirect_response, reverse("mobile:event_detail", kwargs={"pk": event.pk}), fetch_redirect_response=False)
|
||||
|
||||
def test_empty_account_gets_a_graceful_empty_state(self):
|
||||
bare_user = User.objects.create_user(email="new@example.com", password="pw-secret-123")
|
||||
self.client.force_login(bare_user)
|
||||
|
||||
@@ -669,18 +669,32 @@ class EditProfileView(PersonScopeMixin, LoginRequiredMixin, TemplateView):
|
||||
)
|
||||
|
||||
|
||||
def _notification_source_link(source):
|
||||
"""(label, url) for a notification's ``source`` when it's something this
|
||||
app has a detail page for -- (None, None) otherwise. One place both
|
||||
NotificationsView.get_context_data (the row's own label) and .post (the
|
||||
tap-to-mark-read redirect target) read from, so a new source type only
|
||||
needs adding here."""
|
||||
if isinstance(source, News):
|
||||
return _("Club news"), reverse("mobile:news_detail", kwargs={"slug": source.slug})
|
||||
if isinstance(source, Event):
|
||||
return _("Event"), reverse("mobile:event_detail", kwargs={"pk": source.pk})
|
||||
return None, None
|
||||
|
||||
|
||||
class NotificationsView(PersonScopeMixin, LoginRequiredMixin, TemplateView):
|
||||
"""M7 -- design_handoff_rosterchief_platform/README.md's M7 section
|
||||
("Inbox"). The mockup shows rich per-type cards (RSVP-needed, medical
|
||||
form missing, invoice due, line-up published, ...) with inline quick
|
||||
actions, but notifications.models.Notification is generic -- title/body/
|
||||
created/read_at plus an optional ``source`` -- and the only thing that
|
||||
creates member-facing rows today is news.tasks.notify_news_published.
|
||||
There's no type/category field to key a richer layout or the mockup's
|
||||
"Action"/"Club" filter off, so this is deliberately a flat, generic list:
|
||||
day-grouped ("Today"/"Earlier this week"/"Older", echoing Calendar's own
|
||||
"This week"/"Next week" bucketing) with an unread treatment and a link to
|
||||
the underlying News item when ``source`` happens to resolve to one.
|
||||
created/read_at plus an optional ``source`` -- and the only things that
|
||||
create member-facing rows today are news.tasks.notify_news_published and
|
||||
events.tasks.notify_new_event. There's no type/category field to key a
|
||||
richer layout or the mockup's "Action"/"Club" filter off, so this is
|
||||
deliberately a flat, generic list: day-grouped ("Today"/"Earlier this
|
||||
week"/"Older", echoing Calendar's own "This week"/"Next week" bucketing)
|
||||
with an unread treatment and a link to the underlying News/Event when
|
||||
``source`` resolves to one (see _notification_source_link above).
|
||||
|
||||
Scoped to every one of ``self.managed_people`` (not just scope_person) --
|
||||
same scope PersonScopeMixin.get_context_data already uses for
|
||||
@@ -704,9 +718,8 @@ class NotificationsView(PersonScopeMixin, LoginRequiredMixin, TemplateView):
|
||||
notifications = Notification.objects.filter(club=self.request.club, member__in=self.managed_people).select_related("member").order_by("-created")
|
||||
|
||||
for notification in notifications:
|
||||
source = notification.source
|
||||
news_item = source if isinstance(source, News) else None
|
||||
row = {"notification": notification, "news_item": news_item}
|
||||
source_label, _url = _notification_source_link(notification.source)
|
||||
row = {"notification": notification, "source_label": source_label}
|
||||
|
||||
created_date = timezone.localtime(notification.created).date()
|
||||
if created_date == local_today:
|
||||
@@ -733,12 +746,12 @@ class NotificationsView(PersonScopeMixin, LoginRequiredMixin, TemplateView):
|
||||
notification.read_at = timezone.now()
|
||||
notification.save(update_fields=["read_at", "modified"])
|
||||
|
||||
# A row whose source resolves to a News item doubles as a link to
|
||||
# it (see the class docstring) -- the plain-POST tap that marks
|
||||
# it read also lands the member on the article, no separate
|
||||
# "next" field needed since the server already has the source.
|
||||
if isinstance(notification.source, News):
|
||||
return HttpResponseRedirect(reverse("mobile:news_detail", kwargs={"slug": notification.source.slug}))
|
||||
return HttpResponseRedirect(reverse("mobile:notifications"))
|
||||
# A row whose source resolves to something this app has a detail
|
||||
# page for doubles as a link to it (see _notification_source_link)
|
||||
# -- the plain-POST tap that marks it read also lands the member
|
||||
# there, no separate "next" field needed since the server already
|
||||
# has the source.
|
||||
_label, url = _notification_source_link(notification.source)
|
||||
return HttpResponseRedirect(url or reverse("mobile:notifications"))
|
||||
|
||||
return HttpResponseBadRequest(_("Unknown action."))
|
||||
|
||||
Reference in New Issue
Block a user