Skip to content

Commit 515613b

Browse files
committed
Implement daily reminder of trips needing approval
This is mostly at the request of MIT's auditors. During winter, the WSC is quite disciplined about reviewing all trips and approving them (or talking to leaders about the planned itinerary).
1 parent 7f9bd5e commit 515613b

File tree

15 files changed

+657
-18
lines changed

15 files changed

+657
-18
lines changed

ws/email/approval.py

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
import logging
2+
from datetime import timedelta
3+
from typing import NamedTuple
4+
5+
from django.core.mail import EmailMultiAlternatives
6+
from django.template.loader import get_template
7+
from typing_extensions import assert_never
8+
9+
from ws import enums, models
10+
from ws.utils import dates as date_utils
11+
12+
logger = logging.getLogger(__name__)
13+
14+
15+
class ReminderIsSent(NamedTuple):
16+
trip_id: int
17+
activity: enums.Activity
18+
has_trip_info: bool
19+
20+
21+
def _upcoming_trips_lacking_approval(trips: list[models.Trip]) -> list[models.Trip]:
22+
today = date_utils.local_date()
23+
already_approved_trip_ids = {
24+
approval.trip_id
25+
for approval in models.ChairApproval.objects.filter(trip_id__in=trips)
26+
}
27+
return [
28+
trip
29+
for trip in trips
30+
# It's pointless to remind about trips in the past
31+
if trip.trip_date >= today
32+
# Obviously, approved trips ought be excluded
33+
and trip.id not in already_approved_trip_ids
34+
# Circuses & trips that have no activity should never need reminding
35+
# (we assume they got in by nature of a race condition)
36+
and trip.required_activity_enum() is not None
37+
]
38+
39+
40+
def _notified_chairs_too_recently(trips: list[models.Trip]) -> bool:
41+
now = date_utils.local_now()
42+
43+
# Wait, why are multiple activities possible? (it's a weird edge case)
44+
# This method answers "should we notify one activity chair about these trips?"
45+
# We call the method with a collection of trip IDs meant for one chair;
46+
# they **all shared the same activity at the time they were inspected.**
47+
# However, if one trip changed activity in the small window between
48+
# fanning out trips to chairs based on activity & calling this method...
49+
# we still want to notify at least one chair about the trip, even
50+
# if the activity chair that we notify is the wrong one!
51+
# (we'll notify the new activity chair on the next pass)
52+
trip_activities: list[str] = []
53+
for trip in trips:
54+
activity_enum = trip.required_activity_enum()
55+
if activity_enum is not None:
56+
trip_activities.append(activity_enum.value)
57+
58+
try:
59+
last_reminder_sent = models.ChairApprovalReminder.objects.filter(
60+
activity__in=trip_activities
61+
).latest("pk")
62+
except models.ChairApprovalReminder.DoesNotExist:
63+
pass
64+
else:
65+
if last_reminder_sent.time_created > (now - timedelta(minutes=55)):
66+
logger.error(
67+
"Trying to send another reminder at %s, less than an hour since last reminder email at %s",
68+
now,
69+
last_reminder_sent.time_created,
70+
)
71+
return True
72+
return False
73+
74+
75+
def _trips_without_similar_reminders(trips: list[models.Trip]) -> list[models.Trip]:
76+
"""Return any trips which have not already been sent to chairs in their current state."""
77+
trip_ids_having_itinerary = set(
78+
models.TripInfo.objects.filter(trip__in=trips).values_list("trip", flat=True)
79+
)
80+
81+
def _get_reminder_key(trip: models.Trip) -> ReminderIsSent:
82+
activity_enum = trip.required_activity_enum()
83+
assert activity_enum is not None, f"Trip #{trip.id} somehow has no activity?"
84+
return ReminderIsSent(
85+
trip.pk,
86+
activity_enum,
87+
trip.pk in trip_ids_having_itinerary,
88+
)
89+
90+
# We can regard a trip as having been notified if both:
91+
# 1. An email was sent containing that trip in the message.
92+
# 2. The activity & itinerary (at the time the email was sent) match current values.
93+
already_reminded_keys = {
94+
ReminderIsSent(
95+
reminder.trip_id,
96+
enums.Activity(reminder.activity),
97+
reminder.trip_id in trip_ids_having_itinerary,
98+
)
99+
for reminder in models.ChairApprovalReminder.objects.filter(trip_id__in=trips)
100+
}
101+
102+
# Obviously, we can notify for *any* trip that's never had a reminder.
103+
could_notify_trip_ids = {trip.id for trip in trips} - {
104+
key.trip_id for key in already_reminded_keys
105+
}
106+
# Trips that were notified with a different activity *or* itinerary status
107+
# are eligible for re-notification!
108+
could_notify_trip_ids.update(
109+
key.trip_id
110+
for key in already_reminded_keys.difference(
111+
_get_reminder_key(trip) for trip in trips
112+
)
113+
)
114+
return [
115+
trip
116+
for trip in trips
117+
if trip.id in could_notify_trip_ids
118+
# We do *not* want to prompt for "hey, approve these trips!" before itineraries are available.
119+
# 1. It encourages the wrong behavior (approving trips too early to silence emails).
120+
# 2. Trips are ideally meant to be approved once an itinerary is posted.
121+
and trip.info_editable
122+
]
123+
124+
125+
def at_least_one_trip_merits_reminder_email(trips: list[models.Trip]) -> list[str]:
126+
"""Avoid sending reminder emails until actually necessary.
127+
128+
This will return a non-empty list *only* if:
129+
1. We should notify the chair in the first place about one or more trips.
130+
2. There would be something new in the email body were we to notify chairs
131+
about the trips needing approval.
132+
133+
If we're already sending an email, we might as well notify them about
134+
*all* trips currently pending approval.
135+
"""
136+
now = date_utils.local_now()
137+
if _notified_chairs_too_recently(trips):
138+
return []
139+
trips_lacking_approval = _upcoming_trips_lacking_approval(trips)
140+
141+
# If *any* trips leave tomorrow & don't have an approval, remind!
142+
tomorrow = now.date() + timedelta(days=1)
143+
trips_leaving_soon = [
144+
f"Trip #{trip.id} starts very soon (on {trip.trip_date}) but has no approval!"
145+
for trip in trips_lacking_approval
146+
if trip.trip_date <= tomorrow
147+
]
148+
if trips_leaving_soon:
149+
return trips_leaving_soon
150+
151+
return [
152+
(
153+
f"Trip #{trip.id} could complete an itinerary, "
154+
f"has{'' if trip.info else ' not'} done so, "
155+
"and chairs have not been emailed about the trip yet."
156+
)
157+
for trip in _trips_without_similar_reminders(trips_lacking_approval)
158+
]
159+
160+
161+
def emails_for_activity_chair(activity: enums.Activity) -> list[str]:
162+
if activity == enums.Activity.BIKING:
163+
return ["[email protected]"]
164+
if activity == enums.Activity.BOATING:
165+
return ["[email protected]"]
166+
if activity == enums.Activity.CABIN:
167+
# 1) Cabin trips don't require approval
168+
# 2) We don't provide any way to indicate *which* cabin is being used.
169+
170+
if activity == enums.Activity.CLIMBING:
171+
return ["[email protected]"]
172+
if activity == enums.Activity.HIKING:
173+
# Include 3-season chair for now, 3SSC is new
174+
175+
if activity == enums.Activity.WINTER_SCHOOL:
176+
# Exclude WS chairs, they don't approve trips.
177+
return ["[email protected]"]
178+
assert_never(activity)
179+
180+
181+
def notify_activity_chair(
182+
activity_enum: enums.Activity,
183+
trips: list[models.Trip],
184+
) -> None:
185+
context = {"activity_enum": activity_enum, "trips": trips}
186+
187+
text_content = get_template("email/sole/trips_needing_approval.txt").render(context)
188+
html_content = get_template("email/sole/trips_needing_approval.html").render(
189+
context
190+
)
191+
msg = EmailMultiAlternatives(
192+
f"{len(trips)} {activity_enum.label} trip{'' if len(trips) == 1 else 's'} need approval",
193+
text_content,
194+
to=emails_for_activity_chair(activity_enum),
195+
# TEMPORARY while we make sure this feature works as expected.
196+
197+
)
198+
msg.attach_alternative(html_content, "text/html")
199+
msg.send()
200+
201+
# Creating a record of the reminders is the mechanism by which we don't spam chairs.
202+
models.ChairApprovalReminder.objects.bulk_create(
203+
[
204+
models.ChairApprovalReminder(
205+
trip=trip,
206+
# Importantly, we log the receiving chair activity, *NOT* trip activity.
207+
# (this has ramifications for an edge case on trips changing activity)
208+
activity=activity_enum.value,
209+
had_trip_info=trip.info is not None,
210+
)
211+
for trip in trips
212+
]
213+
)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Generated by Django 4.2.23 on 2025-10-25 18:04
2+
3+
import django.core.validators
4+
import django.db.models.deletion
5+
from django.db import migrations, models
6+
7+
8+
class Migration(migrations.Migration):
9+
dependencies = [
10+
("ws", "0014_paddleboarding"),
11+
]
12+
13+
operations = [
14+
migrations.CreateModel(
15+
name="ChairApprovalReminder",
16+
fields=[
17+
(
18+
"id",
19+
models.AutoField(
20+
auto_created=True,
21+
primary_key=True,
22+
serialize=False,
23+
verbose_name="ID",
24+
),
25+
),
26+
("time_created", models.DateTimeField(auto_now_add=True)),
27+
(
28+
"activity",
29+
models.CharField(
30+
choices=[
31+
("biking", "Biking"),
32+
("boating", "Boating"),
33+
("cabin", "Cabin"),
34+
("climbing", "Climbing"),
35+
("hiking", "Hiking"),
36+
("winter_school", "Winter School"),
37+
],
38+
max_length=31,
39+
),
40+
),
41+
("had_trip_info", models.BooleanField()),
42+
(
43+
"trip",
44+
models.ForeignKey(
45+
on_delete=django.db.models.deletion.PROTECT, to="ws.trip"
46+
),
47+
),
48+
],
49+
),
50+
]

ws/models.py

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,7 @@ class MembershipReminder(models.Model):
736736
participant = models.ForeignKey(Participant, on_delete=models.CASCADE)
737737
reminder_sent_at = models.DateTimeField(
738738
verbose_name="Last time an email was sent reminding this participant to renew",
739-
# We allow (temporary) null rows participant to ensure we can lock something
739+
# We allow (temporary) null rows to ensure we can lock something
740740
null=True,
741741
blank=True,
742742
)
@@ -1396,15 +1396,19 @@ def last_of_priority(self):
13961396
return (last_signup.manual_order or 0) + 1
13971397

13981398
@property
1399-
def info_editable(self):
1399+
def itinerary_available_at(self) -> datetime:
1400+
return date_utils.itinerary_available_at(self.trip_date)
1401+
1402+
@property
1403+
def info_editable(self) -> bool:
14001404
now = date_utils.local_now()
14011405

14021406
# Past trips may not be edited!
14031407
if now.date() > self.trip_date:
14041408
return False
14051409

14061410
# Otherwise, info (including itinerary) should be editable after the cutoff has passed
1407-
return now > date_utils.itinerary_available_at(self.trip_date)
1411+
return now > self.itinerary_available_at
14081412

14091413
def make_fcfs(self, signups_open_at: datetime | None = None) -> None:
14101414
"""Set the algorithm to FCFS, adjust signup times appropriately."""
@@ -1469,6 +1473,33 @@ def __str__(self):
14691473
)
14701474

14711475

1476+
class ChairApprovalReminder(models.Model):
1477+
"""Log which trips we've already asked the activity chairs to review.
1478+
1479+
(If there are trips awaiting review *but* we've already notified chairs, we
1480+
don't want to inundandate their email and risk getting flagged as spammers)
1481+
"""
1482+
1483+
time_created = models.DateTimeField(auto_now_add=True)
1484+
1485+
trip = models.ForeignKey(Trip, on_delete=models.PROTECT)
1486+
1487+
# You can change a trip's activity once created.
1488+
# Log the activity *at the time we sent the reminder*
1489+
activity = models.CharField(
1490+
max_length=31,
1491+
choices=LeaderRating.CLOSED_ACTIVITY_CHOICES,
1492+
)
1493+
1494+
# If the trip *previously* had no itinerary, we can notify chairs again.
1495+
had_trip_info = models.BooleanField()
1496+
1497+
def __str__(self):
1498+
return (
1499+
f"{self.trip.name} (#{self.trip.pk}, Reminder sent on {self.time_created}"
1500+
)
1501+
1502+
14721503
class Feedback(models.Model):
14731504
"""Feedback given for a participant on one trip."""
14741505

ws/settings.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
1-
"""Django settings for ws project.
2-
3-
For more information on this file, see
4-
https://docs.djangoproject.com/en/3.2/topics/settings/
5-
"""
6-
71
# Build paths inside the project like this: os.path.join(BASE_DIR, ...)
82
import os
93
from typing import Any
@@ -160,6 +154,14 @@
160154
"task": "ws.tasks.run_ws_lottery",
161155
"schedule": crontab(minute=0, hour=14, month_of_year=[1, 2], day_of_week=3),
162156
},
157+
"email-all-activity-chairs-about-unapproved-trips": {
158+
"task": "ws.tasks.email_all_activity_chairs_about_unapproved_trips",
159+
"schedule": crontab(
160+
# Temporary testing -- don't automate until November!
161+
month_of_year=[11, 12],
162+
hour=17,
163+
),
164+
},
163165
}
164166

165167
CELERY_TIMEZONE = "UTC"

0 commit comments

Comments
 (0)