From 0d225cdc2383d34fc1359581bf1e758964b4a54a Mon Sep 17 00:00:00 2001 From: Jack Bonatakis Date: Tue, 31 Mar 2026 22:08:15 -0400 Subject: [PATCH 1/6] Send emails 7 days before CF closure --- ...8_commitfest_preclosure_warning_sent_at.py | 17 +++ pgcommitfest/commitfest/models.py | 110 ++++++++++++++++++ .../mail/commitfest_preclosure_warning.txt | 19 +++ .../tests/test_closure_notifications.py | 46 ++++++++ .../tests/test_refresh_commitfests.py | 22 ++++ 5 files changed, 214 insertions(+) create mode 100644 pgcommitfest/commitfest/migrations/0018_commitfest_preclosure_warning_sent_at.py create mode 100644 pgcommitfest/commitfest/templates/mail/commitfest_preclosure_warning.txt diff --git a/pgcommitfest/commitfest/migrations/0018_commitfest_preclosure_warning_sent_at.py b/pgcommitfest/commitfest/migrations/0018_commitfest_preclosure_warning_sent_at.py new file mode 100644 index 00000000..4c6ae1a5 --- /dev/null +++ b/pgcommitfest/commitfest/migrations/0018_commitfest_preclosure_warning_sent_at.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.19 on 2026-03-31 00:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("commitfest", "0017_make_topic_optional"), + ] + + operations = [ + migrations.AddField( + model_name="commitfest", + name="preclosure_warning_sent_at", + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 6594e3c4..cc818dc1 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -57,6 +57,7 @@ class CommitFest(models.Model): startdate = models.DateField(blank=False, null=False) enddate = models.DateField(blank=False, null=False) draft = models.BooleanField(blank=False, null=False, default=False) + preclosure_warning_sent_at = models.DateTimeField(blank=True, null=True) @property def statusstring(self): @@ -225,12 +226,116 @@ def send_closure_notifications(self): }, ) + @staticmethod + def _notification_email_for_user(user): + try: + if user.userprofile and user.userprofile.notifyemail: + return user.userprofile.notifyemail.email + except UserProfile.DoesNotExist: + pass + return user.email + + @staticmethod + def _wants_role_notification(user, role): + try: + profile = user.userprofile + except UserProfile.DoesNotExist: + return role == "author" + + if role == "author": + return profile.notify_all_author + if role == "reviewer": + return profile.notify_all_reviewer + if role == "committer": + return profile.notify_all_committer + + return False + + def send_preclosure_notifications(self, days_remaining=7): + """Send pre-closure reminder notifications to involved users.""" + if self.preclosure_warning_sent_at: + return + + open_pocs = list( + self.patchoncommitfest_set.filter( + status__in=PatchOnCommitFest.OPEN_STATUSES + ) + .select_related("patch", "patch__committer__user") + .prefetch_related( + "patch__authors", + "patch__reviewers", + "patch__subscribers", + ) + ) + if not open_pocs: + return + + recipients_patches = {} + for poc in open_pocs: + patch = poc.patch + recipients = set() + + for author in patch.authors.all(): + if self._wants_role_notification(author, "author"): + recipients.add(author) + + for reviewer in patch.reviewers.all(): + if self._wants_role_notification(reviewer, "reviewer"): + recipients.add(reviewer) + + if patch.committer and self._wants_role_notification( + patch.committer.user, "committer" + ): + recipients.add(patch.committer.user) + + recipients.update(patch.subscribers.all()) + + for recipient in recipients: + if recipient.id not in recipients_patches: + recipients_patches[recipient.id] = { + "user": recipient, + "patches": {}, + } + recipients_patches[recipient.id]["patches"][patch.id] = poc + + if not recipients_patches: + return + + for payload in recipients_patches.values(): + user = payload["user"] + patches = list(payload["patches"].values()) + send_template_mail( + settings.NOTIFICATION_FROM, + None, + self._notification_email_for_user(user), + ( + f"Commitfest {self.name} closes in {days_remaining} days " + "and your patches need attention" + ), + "mail/commitfest_preclosure_warning.txt", + { + "user": user, + "commitfest": self, + "patches": patches, + "days_remaining": days_remaining, + }, + ) + + self.preclosure_warning_sent_at = datetime.now(timezone.utc) + self.save(update_fields=["preclosure_warning_sent_at"]) + @staticmethod def _are_relevant_commitfests_up_to_date(cfs, current_date): inprogress_cf = cfs["in_progress"] if inprogress_cf and inprogress_cf.enddate < current_date: return False + if ( + inprogress_cf + and inprogress_cf.enddate == current_date + timedelta(days=7) + and not inprogress_cf.preclosure_warning_sent_at + ): + return False if cfs["open"].startdate <= current_date: return False @@ -261,6 +366,11 @@ def _refresh_relevant_commitfests(cls, for_update): inprogress_cf.save() inprogress_cf.auto_move_active_patches() inprogress_cf.send_closure_notifications() + elif ( + inprogress_cf + and inprogress_cf.enddate == current_date + timedelta(days=7) + ): + inprogress_cf.send_preclosure_notifications(days_remaining=7) open_cf = cfs["open"] diff --git a/pgcommitfest/commitfest/templates/mail/commitfest_preclosure_warning.txt b/pgcommitfest/commitfest/templates/mail/commitfest_preclosure_warning.txt new file mode 100644 index 00000000..e4ca3d95 --- /dev/null +++ b/pgcommitfest/commitfest/templates/mail/commitfest_preclosure_warning.txt @@ -0,0 +1,19 @@ +Commitfest {{commitfest.name}} will auto-close in {{days_remaining}} days. + +You are involved in {{patches|length}} open patch{{patches|length|pluralize:"es"}}: + +{% for poc in patches %} + - {{poc.patch.name}} + https://commitfest.postgresql.org/patch/{{poc.patch.id}}/ +{% endfor %} + +Please take action on {{patches|length|pluralize:"these patches,this patch"}} before close: + +1. Move {{patches|length|pluralize:"them,it"}} to the next commitfest if work should continue. + +2. Or close {{patches|length|pluralize:"them,it"}} with an appropriate status (Withdrawn, Returned with feedback, etc.) + +-- +This is an automated message from the PostgreSQL Commitfest app. +To manage your notification preferences, visit: +https://commitfest.postgresql.org/userprofile/ diff --git a/pgcommitfest/commitfest/tests/test_closure_notifications.py b/pgcommitfest/commitfest/tests/test_closure_notifications.py index eb7cfc7a..522140ce 100644 --- a/pgcommitfest/commitfest/tests/test_closure_notifications.py +++ b/pgcommitfest/commitfest/tests/test_closure_notifications.py @@ -8,6 +8,7 @@ from pgcommitfest.commitfest.models import ( CfbotBranch, + Committer, CommitFest, Patch, PatchHistory, @@ -441,3 +442,48 @@ def test_draft_cf_moves_active_patches_to_next_draft(alice, bob): # No closure email for moved patches assert QueuedMail.objects.count() == 0 + + +def test_send_preclosure_notifications_to_all_involved_roles( + alice, bob, charlie, dave, in_progress_cf +): + """Pre-closure reminder should notify involved users based on role preferences.""" + UserProfile.objects.create(user=bob, notify_all_reviewer=True) + UserProfile.objects.create(user=charlie, notify_all_committer=True) + committer = Committer.objects.create(user=charlie) + + patch = Patch.objects.create(name="Preclose Patch", committer=committer) + patch.authors.add(alice) + patch.reviewers.add(bob) + patch.subscribers.add(dave) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.send_preclosure_notifications(days_remaining=7) + + assert QueuedMail.objects.count() == 4 + receivers = set(QueuedMail.objects.values_list("receiver", flat=True)) + assert receivers == {alice.email, bob.email, charlie.email, dave.email} + in_progress_cf.refresh_from_db() + assert in_progress_cf.preclosure_warning_sent_at is not None + + +def test_preclosure_notifications_are_sent_only_once(alice, in_progress_cf): + """Pre-closure reminder should be idempotent for a commitfest.""" + patch = Patch.objects.create(name="Single Reminder Patch") + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + in_progress_cf.send_preclosure_notifications(days_remaining=7) + in_progress_cf.send_preclosure_notifications(days_remaining=7) + + assert QueuedMail.objects.count() == 1 diff --git a/pgcommitfest/commitfest/tests/test_refresh_commitfests.py b/pgcommitfest/commitfest/tests/test_refresh_commitfests.py index dda1fc9d..f50d721f 100644 --- a/pgcommitfest/commitfest/tests/test_refresh_commitfests.py +++ b/pgcommitfest/commitfest/tests/test_refresh_commitfests.py @@ -8,6 +8,7 @@ Patch, PatchOnCommitFest, ) +from pgcommitfest.mailqueue.models import QueuedMail pytestmark = pytest.mark.django_db @@ -156,3 +157,24 @@ def test_no_changes_when_up_to_date(commitfests): assert open_cf.status == CommitFest.STATUS_OPEN assert draft_cf.status == CommitFest.STATUS_OPEN assert CommitFest.objects.count() == initial_cf_count + + +@freeze_time("2024-11-23") +def test_preclosure_notifications_trigger_7_days_before_close(commitfests, alice): + """In-progress CF should send pre-closure notifications exactly once.""" + in_progress_cf = commitfests["in_progress"] + patch = Patch.objects.create(name="Reminder Patch") + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + CommitFest._refresh_relevant_commitfests(for_update=False) + CommitFest._refresh_relevant_commitfests(for_update=False) + + assert QueuedMail.objects.count() == 1 + in_progress_cf.refresh_from_db() + assert in_progress_cf.preclosure_warning_sent_at is not None From ddb28183f3b763b7cceb1e790a814f2ed4b60c44 Mon Sep 17 00:00:00 2001 From: Jack Bonatakis Date: Tue, 31 Mar 2026 22:13:15 -0400 Subject: [PATCH 2/6] Don't hardcode 7 days, use settings --- pgcommitfest/commitfest/models.py | 13 +++++++++---- .../commitfest/tests/test_closure_notifications.py | 12 +++++++++--- pgcommitfest/settings.py | 2 ++ 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index cc818dc1..278ab210 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -251,8 +251,11 @@ def _wants_role_notification(user, role): return False - def send_preclosure_notifications(self, days_remaining=7): + def send_preclosure_notifications(self, days_remaining=None): """Send pre-closure reminder notifications to involved users.""" + if days_remaining is None: + days_remaining = settings.PRE_CLOSURE_NOTIFICATION_DAYS + if self.preclosure_warning_sent_at: return @@ -327,12 +330,13 @@ def send_preclosure_notifications(self, days_remaining=7): @staticmethod def _are_relevant_commitfests_up_to_date(cfs, current_date): inprogress_cf = cfs["in_progress"] + reminder_days = settings.PRE_CLOSURE_NOTIFICATION_DAYS if inprogress_cf and inprogress_cf.enddate < current_date: return False if ( inprogress_cf - and inprogress_cf.enddate == current_date + timedelta(days=7) + and inprogress_cf.enddate == current_date + timedelta(days=reminder_days) and not inprogress_cf.preclosure_warning_sent_at ): return False @@ -361,6 +365,7 @@ def _refresh_relevant_commitfests(cls, for_update): return cfs inprogress_cf = cfs["in_progress"] + reminder_days = settings.PRE_CLOSURE_NOTIFICATION_DAYS if inprogress_cf and inprogress_cf.enddate < current_date: inprogress_cf.status = CommitFest.STATUS_CLOSED inprogress_cf.save() @@ -368,9 +373,9 @@ def _refresh_relevant_commitfests(cls, for_update): inprogress_cf.send_closure_notifications() elif ( inprogress_cf - and inprogress_cf.enddate == current_date + timedelta(days=7) + and inprogress_cf.enddate == current_date + timedelta(days=reminder_days) ): - inprogress_cf.send_preclosure_notifications(days_remaining=7) + inprogress_cf.send_preclosure_notifications(days_remaining=reminder_days) open_cf = cfs["open"] diff --git a/pgcommitfest/commitfest/tests/test_closure_notifications.py b/pgcommitfest/commitfest/tests/test_closure_notifications.py index 522140ce..ef411164 100644 --- a/pgcommitfest/commitfest/tests/test_closure_notifications.py +++ b/pgcommitfest/commitfest/tests/test_closure_notifications.py @@ -463,7 +463,9 @@ def test_send_preclosure_notifications_to_all_involved_roles( status=PatchOnCommitFest.STATUS_REVIEW, ) - in_progress_cf.send_preclosure_notifications(days_remaining=7) + in_progress_cf.send_preclosure_notifications( + days_remaining=settings.PRE_CLOSURE_NOTIFICATION_DAYS + ) assert QueuedMail.objects.count() == 4 receivers = set(QueuedMail.objects.values_list("receiver", flat=True)) @@ -483,7 +485,11 @@ def test_preclosure_notifications_are_sent_only_once(alice, in_progress_cf): status=PatchOnCommitFest.STATUS_REVIEW, ) - in_progress_cf.send_preclosure_notifications(days_remaining=7) - in_progress_cf.send_preclosure_notifications(days_remaining=7) + in_progress_cf.send_preclosure_notifications( + days_remaining=settings.PRE_CLOSURE_NOTIFICATION_DAYS + ) + in_progress_cf.send_preclosure_notifications( + days_remaining=settings.PRE_CLOSURE_NOTIFICATION_DAYS + ) assert QueuedMail.objects.count() == 1 diff --git a/pgcommitfest/settings.py b/pgcommitfest/settings.py index 07e4adaf..dffad9d8 100644 --- a/pgcommitfest/settings.py +++ b/pgcommitfest/settings.py @@ -173,6 +173,8 @@ AUTO_MOVE_EMAIL_ACTIVITY_DAYS = 30 # Patches failing CI for longer than this many days will NOT be auto-moved AUTO_MOVE_MAX_FAILING_DAYS = 21 +# Send pre-closure reminders this many days before an in-progress CF ends +PRE_CLOSURE_NOTIFICATION_DAYS = 7 # Load local settings overrides try: From 3a8eca1b857593579cd7a7755d4d186113fc1ba0 Mon Sep 17 00:00:00 2001 From: Jack Bonatakis Date: Tue, 31 Mar 2026 22:29:34 -0400 Subject: [PATCH 3/6] Make date checking more robust --- pgcommitfest/commitfest/models.py | 43 +++++++++++++------ .../tests/test_refresh_commitfests.py | 24 +++++++++++ 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 278ab210..52d97dbc 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -102,6 +102,28 @@ def is_open_draft(self): def is_in_progress(self): return self.status == self.STATUS_INPROGRESS + @property + def preclosure_warning_date(self): + return self.enddate - timedelta(days=settings.PRE_CLOSURE_NOTIFICATION_DAYS) + + @staticmethod + def _current_date(): + return datetime.now(timezone.utc).date() + + def days_until_close(self, current_date=None): + if current_date is None: + current_date = self._current_date() + return (self.enddate - current_date).days + + def should_send_preclosure_warning(self, current_date=None): + if current_date is None: + current_date = self._current_date() + return ( + self.is_in_progress + and not self.preclosure_warning_sent_at + and self.preclosure_warning_date <= current_date <= self.enddate + ) + def to_json(self): return { "id": self.id, @@ -254,7 +276,7 @@ def _wants_role_notification(user, role): def send_preclosure_notifications(self, days_remaining=None): """Send pre-closure reminder notifications to involved users.""" if days_remaining is None: - days_remaining = settings.PRE_CLOSURE_NOTIFICATION_DAYS + days_remaining = self.days_until_close() if self.preclosure_warning_sent_at: return @@ -330,15 +352,10 @@ def send_preclosure_notifications(self, days_remaining=None): @staticmethod def _are_relevant_commitfests_up_to_date(cfs, current_date): inprogress_cf = cfs["in_progress"] - reminder_days = settings.PRE_CLOSURE_NOTIFICATION_DAYS if inprogress_cf and inprogress_cf.enddate < current_date: return False - if ( - inprogress_cf - and inprogress_cf.enddate == current_date + timedelta(days=reminder_days) - and not inprogress_cf.preclosure_warning_sent_at - ): + if inprogress_cf and inprogress_cf.should_send_preclosure_warning(): return False if cfs["open"].startdate <= current_date: @@ -352,7 +369,7 @@ def _are_relevant_commitfests_up_to_date(cfs, current_date): @classmethod def _refresh_relevant_commitfests(cls, for_update): cfs = CommitFest.relevant_commitfests(for_update=for_update, refresh=False) - current_date = datetime.now(timezone.utc).date() + current_date = cls._current_date() if cls._are_relevant_commitfests_up_to_date(cfs, current_date): return cfs @@ -365,17 +382,15 @@ def _refresh_relevant_commitfests(cls, for_update): return cfs inprogress_cf = cfs["in_progress"] - reminder_days = settings.PRE_CLOSURE_NOTIFICATION_DAYS if inprogress_cf and inprogress_cf.enddate < current_date: inprogress_cf.status = CommitFest.STATUS_CLOSED inprogress_cf.save() inprogress_cf.auto_move_active_patches() inprogress_cf.send_closure_notifications() - elif ( - inprogress_cf - and inprogress_cf.enddate == current_date + timedelta(days=reminder_days) - ): - inprogress_cf.send_preclosure_notifications(days_remaining=reminder_days) + elif inprogress_cf and inprogress_cf.should_send_preclosure_warning(): + inprogress_cf.send_preclosure_notifications( + days_remaining=inprogress_cf.days_until_close() + ) open_cf = cfs["open"] diff --git a/pgcommitfest/commitfest/tests/test_refresh_commitfests.py b/pgcommitfest/commitfest/tests/test_refresh_commitfests.py index f50d721f..b9f6fd1d 100644 --- a/pgcommitfest/commitfest/tests/test_refresh_commitfests.py +++ b/pgcommitfest/commitfest/tests/test_refresh_commitfests.py @@ -178,3 +178,27 @@ def test_preclosure_notifications_trigger_7_days_before_close(commitfests, alice assert QueuedMail.objects.count() == 1 in_progress_cf.refresh_from_db() assert in_progress_cf.preclosure_warning_sent_at is not None + + +@freeze_time("2024-11-24") +def test_preclosure_notifications_trigger_on_first_check_inside_window( + commitfests, alice +): + """The reminder should still be sent if the site is first hit after the window opens.""" + in_progress_cf = commitfests["in_progress"] + patch = Patch.objects.create(name="Late Reminder Patch") + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + CommitFest._refresh_relevant_commitfests(for_update=False) + + assert QueuedMail.objects.count() == 1 + mail = QueuedMail.objects.get() + assert "closes in 6 days" in mail.fullmsg + in_progress_cf.refresh_from_db() + assert in_progress_cf.preclosure_warning_sent_at is not None From ad9f3e0ef5663958590a2754042731403f02778b Mon Sep 17 00:00:00 2001 From: Jack Bonatakis Date: Tue, 31 Mar 2026 22:31:46 -0400 Subject: [PATCH 4/6] Fix template pluralization --- .../templates/mail/commitfest_preclosure_warning.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pgcommitfest/commitfest/templates/mail/commitfest_preclosure_warning.txt b/pgcommitfest/commitfest/templates/mail/commitfest_preclosure_warning.txt index e4ca3d95..e64a5325 100644 --- a/pgcommitfest/commitfest/templates/mail/commitfest_preclosure_warning.txt +++ b/pgcommitfest/commitfest/templates/mail/commitfest_preclosure_warning.txt @@ -7,11 +7,11 @@ You are involved in {{patches|length}} open patch{{patches|length|pluralize:"es" https://commitfest.postgresql.org/patch/{{poc.patch.id}}/ {% endfor %} -Please take action on {{patches|length|pluralize:"these patches,this patch"}} before close: +Please take action on {{patches|length|pluralize:"this patch,these patches"}} before close: -1. Move {{patches|length|pluralize:"them,it"}} to the next commitfest if work should continue. +1. Move {{patches|length|pluralize:"it,them"}} to the next commitfest if work should continue. -2. Or close {{patches|length|pluralize:"them,it"}} with an appropriate status (Withdrawn, Returned with feedback, etc.) +2. Or close {{patches|length|pluralize:"it,them"}} with an appropriate status (Withdrawn, Returned with feedback, etc.) -- This is an automated message from the PostgreSQL Commitfest app. From 23be8b1171338d4568a682254f8c8e981df2ed56 Mon Sep 17 00:00:00 2001 From: Jack Bonatakis Date: Tue, 31 Mar 2026 22:43:47 -0400 Subject: [PATCH 5/6] Add some helpful typing --- pgcommitfest/commitfest/models.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 52d97dbc..85776c77 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -4,7 +4,8 @@ from django.db.models import Q from django.shortcuts import get_object_or_404 -from datetime import datetime, timedelta, timezone +from datetime import date, datetime, timedelta, timezone +from typing import Literal from pgcommitfest.mailqueue.util import send_template_mail from pgcommitfest.userprofile.models import UserProfile @@ -103,19 +104,19 @@ def is_in_progress(self): return self.status == self.STATUS_INPROGRESS @property - def preclosure_warning_date(self): + def preclosure_warning_date(self) -> date: return self.enddate - timedelta(days=settings.PRE_CLOSURE_NOTIFICATION_DAYS) @staticmethod - def _current_date(): + def _current_date() -> date: return datetime.now(timezone.utc).date() - def days_until_close(self, current_date=None): + def days_until_close(self, current_date: date | None = None) -> int: if current_date is None: current_date = self._current_date() return (self.enddate - current_date).days - def should_send_preclosure_warning(self, current_date=None): + def should_send_preclosure_warning(self, current_date: date | None = None) -> bool: if current_date is None: current_date = self._current_date() return ( @@ -249,7 +250,7 @@ def send_closure_notifications(self): ) @staticmethod - def _notification_email_for_user(user): + def _notification_email_for_user(user: User) -> str: try: if user.userprofile and user.userprofile.notifyemail: return user.userprofile.notifyemail.email @@ -258,7 +259,9 @@ def _notification_email_for_user(user): return user.email @staticmethod - def _wants_role_notification(user, role): + def _wants_role_notification( + user: User, role: Literal["author", "reviewer", "committer"] + ) -> bool: try: profile = user.userprofile except UserProfile.DoesNotExist: @@ -273,7 +276,7 @@ def _wants_role_notification(user, role): return False - def send_preclosure_notifications(self, days_remaining=None): + def send_preclosure_notifications(self, days_remaining: int | None = None) -> None: """Send pre-closure reminder notifications to involved users.""" if days_remaining is None: days_remaining = self.days_until_close() From 8ac71c87605a8f73961e8730b621ec1a916cfef1 Mon Sep 17 00:00:00 2001 From: Jack Bonatakis Date: Tue, 31 Mar 2026 23:17:05 -0400 Subject: [PATCH 6/6] Clear preclosure_warning_sent_at if enddate is adjusted --- pgcommitfest/commitfest/models.py | 18 ++++++++ .../tests/test_refresh_commitfests.py | 41 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/pgcommitfest/commitfest/models.py b/pgcommitfest/commitfest/models.py index 85776c77..98fdac07 100644 --- a/pgcommitfest/commitfest/models.py +++ b/pgcommitfest/commitfest/models.py @@ -125,6 +125,24 @@ def should_send_preclosure_warning(self, current_date: date | None = None) -> bo and self.preclosure_warning_date <= current_date <= self.enddate ) + def save(self, *args, **kwargs): + update_fields = kwargs.get("update_fields") + update_fields_set = set(update_fields) if update_fields is not None else None + + enddate_is_updating = update_fields_set is None or "enddate" in update_fields_set + if self.pk and enddate_is_updating: + # I don't love adding a query here, but it seems like the best way to get the old enddate. + old_enddate = ( + type(self).objects.filter(pk=self.pk).values_list("enddate", flat=True).first() + ) + if old_enddate is not None and old_enddate != self.enddate: + self.preclosure_warning_sent_at = None + if update_fields_set is not None: + update_fields_set.add("preclosure_warning_sent_at") + kwargs["update_fields"] = update_fields_set + + super().save(*args, **kwargs) + def to_json(self): return { "id": self.id, diff --git a/pgcommitfest/commitfest/tests/test_refresh_commitfests.py b/pgcommitfest/commitfest/tests/test_refresh_commitfests.py index b9f6fd1d..aae4b07d 100644 --- a/pgcommitfest/commitfest/tests/test_refresh_commitfests.py +++ b/pgcommitfest/commitfest/tests/test_refresh_commitfests.py @@ -202,3 +202,44 @@ def test_preclosure_notifications_trigger_on_first_check_inside_window( assert "closes in 6 days" in mail.fullmsg in_progress_cf.refresh_from_db() assert in_progress_cf.preclosure_warning_sent_at is not None + + +def test_changing_enddate_clears_preclosure_warning_and_allows_resend( + commitfests, alice +): + """Moving the close date should clear the reminder timestamp and reopen the flow.""" + in_progress_cf = commitfests["in_progress"] + patch = Patch.objects.create(name="Rescheduled Reminder Patch") + patch.authors.add(alice) + PatchOnCommitFest.objects.create( + patch=patch, + commitfest=in_progress_cf, + enterdate=datetime.now(), + status=PatchOnCommitFest.STATUS_REVIEW, + ) + + with freeze_time("2024-11-24"): + CommitFest._refresh_relevant_commitfests(for_update=False) + + assert QueuedMail.objects.count() == 1 + in_progress_cf.refresh_from_db() + assert in_progress_cf.preclosure_warning_sent_at is not None + + in_progress_cf.enddate = datetime(2024, 12, 7).date() + in_progress_cf.save(update_fields=["enddate"]) + in_progress_cf.refresh_from_db() + assert in_progress_cf.preclosure_warning_sent_at is None + + with freeze_time("2024-11-24"): + CommitFest._refresh_relevant_commitfests(for_update=False) + + assert QueuedMail.objects.count() == 1 + + with freeze_time("2024-11-30"): + CommitFest._refresh_relevant_commitfests(for_update=False) + + assert QueuedMail.objects.count() == 2 + mail = QueuedMail.objects.order_by("-id").first() + assert "closes in 7 days" in mail.fullmsg + in_progress_cf.refresh_from_db() + assert in_progress_cf.preclosure_warning_sent_at is not None