diff --git a/events/tasks.py b/events/tasks.py index c400126..5441341 100644 --- a/events/tasks.py +++ b/events/tasks.py @@ -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)." diff --git a/management/tests.py b/management/tests.py index 9374f4f..e4c1bcc 100644 --- a/management/tests.py +++ b/management/tests.py @@ -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 diff --git a/management/views.py b/management/views.py index 8ba96bb..b597c89 100644 --- a/management/views.py +++ b/management/views.py @@ -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): diff --git a/mobile/templates/mobile/_notification_row.html b/mobile/templates/mobile/_notification_row.html index b9a2116..d9c93d6 100644 --- a/mobile/templates/mobile/_notification_row.html +++ b/mobile/templates/mobile/_notification_row.html @@ -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 @@ {{ row.notification.title }} {{ row.notification.body|truncatechars:120 }} - {% if row.news_item %}{% trans "Club news" %}{% endif %} + {% if row.source_label %}{{ row.source_label }}{% endif %} {% blocktrans with time=row.notification.created|timesince %}{{ time }} ago{% endblocktrans %} diff --git a/mobile/tests.py b/mobile/tests.py index 448ae59..4cf77b9 100644 --- a/mobile/tests.py +++ b/mobile/tests.py @@ -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) diff --git a/mobile/views.py b/mobile/views.py index 0e40b5c..8119a7a 100644 --- a/mobile/views.py +++ b/mobile/views.py @@ -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."))