Only notify on email if opted in and show warnings on page

Author: JelteF

pgcommitfest/commitfest/fixtures/commitfest_data.json

@@ -1351,5 +1351,63 @@
         "created": "2025-01-26T22:11:40.000",
         "modified": "2025-01-26T22:12:00.000"
     }
+},
+{
+    "model": "commitfest.patch",
+    "pk": 9,
+    "fields": {
+        "name": "Abandoned patch in closed commitfest",
+        "topic": 3,
+        "wikilink": "",
+        "gitlink": "",
+        "targetversion": null,
+        "committer": null,
+        "created": "2024-10-15T10:00:00",
+        "modified": "2024-10-15T10:00:00",
+        "lastmail": "2024-10-01T10:00:00",
+        "tags": [],
+        "authors": [],
+        "reviewers": [],
+        "subscribers": [],
+        "mailthread_set": [
+            9
+        ]
+    }
+},
+{
+    "model": "commitfest.mailthread",
+    "pk": 9,
+    "fields": {
+        "messageid": "abandoned@message-09",
+        "subject": "Abandoned patch in closed commitfest",
+        "firstmessage": "2024-10-01T10:00:00",
+        "firstauthor": "test@test.com",
+        "latestmessage": "2024-10-01T10:00:00",
+        "latestauthor": "test@test.com",
+        "latestsubject": "Abandoned patch in closed commitfest",
+        "latestmsgid": "abandoned@message-09"
+    }
+},
+{
+    "model": "commitfest.patchoncommitfest",
+    "pk": 10,
+    "fields": {
+        "patch": 9,
+        "commitfest": 1,
+        "enterdate": "2024-10-15T10:00:00",
+        "leavedate": null,
+        "status": 1
+    }
+},
+{
+    "model": "commitfest.patchhistory",
+    "pk": 23,
+    "fields": {
+        "patch": 9,
+        "date": "2024-10-15T10:00:00",
+        "by": 1,
+        "by_cfbot": false,
+        "what": "Created patch record"
+    }
 }
 ]

pgcommitfest/commitfest/models.py

@@ -239,11 +239,17 @@ def send_closure_notifications(self, moved_patch_ids=None):
             next_cf_url = "https://commitfest.postgresql.org/"
 
         # Collect unique authors and their patches
+        # Only include authors who have notify_all_author enabled
         authors_patches = {}
         for poc in open_pocs:
             for author in poc.patch.authors.all():
                 if not author.email:
                     continue
+                try:
+                    if not author.userprofile.notify_all_author:
+                        continue
+                except UserProfile.DoesNotExist:
+                    continue
                 if author not in authors_patches:
                     authors_patches[author] = []
                 authors_patches[author].append(poc)
@@ -252,11 +258,8 @@ def send_closure_notifications(self, moved_patch_ids=None):
         for author, patches in authors_patches.items():
             # Get user's notification email preference
             email = author.email
-            try:
-                if author.userprofile and author.userprofile.notifyemail:
-                    email = author.userprofile.notifyemail.email
-            except UserProfile.DoesNotExist:
-                pass
+            if author.userprofile.notifyemail:
+                email = author.userprofile.notifyemail.email
 
             send_template_mail(
                 settings.NOTIFICATION_FROM,

pgcommitfest/commitfest/templates/commitfest.html

@@ -2,6 +2,12 @@
 {%load commitfest %}
 {%block contents%}
 
+ {%if cf.is_closed%}
+  <div class="alert alert-warning" role="alert">
+   <strong>This commitfest is closed.</strong> Open patches in this commitfest are no longer being picked up by CI. If you want CI to run on your patch, move it to an open commitfest.
+  </div>
+ {%endif%}
+
  <button type="button" class="btn btn-secondary active" id="filterButton" onClick="togglePatchFilterButton('filterButton', 'collapseFilters')">Search/filter</button>
  <a class="btn btn-secondary{% if request.GET.reviewer == '-2' %} active{% endif %}" href="?reviewer=-2">No reviewers</a>
  <a class="btn btn-secondary{% if request.GET.author == '-3' %} active{% endif %}" href="?author=-3">My patches</a>

pgcommitfest/commitfest/templates/help.html

@@ -15,6 +15,11 @@ <h2>Commitfest</h2>
  </p>
  <p>There are 5 Commitfests per year. The first one is "In Progress" in <em>July</em> and starts the nine months feature development cycle of PostgreSQL. The next three are "In Progress" in <em>September</em>, <em>November</em> and <em>January</em>. The last Commitfest of the feature development cycle is "In Progress" in <em>March</em>, and ends a when the feature freeze starts. The exact date of the feature freeze depends on the year, but it's usually in early April.</p>
 
+ <h3>Commitfest closure</h3>
+ <p>
+  When a Commitfest closes, patches that have been active recently are automatically moved to the next Commitfest. A patch is considered "active" if it has had email activity in the past 30 days and has not been failing CI for more than 21 days. Patches that are not automatically moved will stay in the closed Commitfest, where they will no longer be picked up by CI. Authors of such patches that have enabled "Notify on all where author" in their profile settings will receive an email notification asking them to either move the patch to the next Commitfest or close it with an appropriate status.
+ </p>
+
  <h2>Patches</h2>
  <p>
   A "patch" is a bit of an overloaded term in the PostgreSQL community. Email threads on the mailing list often contain "patch files" as attachments, such a file is often referred to as a "patch". A single email can even contain multiple related "patch files", which are called a "patchset". However, in the context of a Commitfest app a "patch" usually means a "patch entry" in the Commitfest app. Such a "patch entry" is a reference to a mailinglist thread on which change to PostgreSQL has been proposed, by someone sending an email that contain one or more "patch files". The Commitfest app will automatically detect new versions of the patch files and update the "patch entry" accordingly.

pgcommitfest/commitfest/templates/patch.html

@@ -1,6 +1,13 @@
 {%extends "base.html"%}
 {%load commitfest%}
 {%block contents%}
+ {%with current_poc=patch_commitfests.0%}
+  {%if cf.is_closed and not current_poc.is_closed%}
+   <div class="alert alert-warning" role="alert">
+    <strong>This patch is part of a closed commitfest.</strong> It is no longer being picked up by CI. If you want CI to run on this patch, move it to an open commitfest.
+   </div>
+  {%endif%}
+ {%endwith%}
  {%include "patch_commands.inc"%}
  <table class="table table-bordered">
   <tbody>

pgcommitfest/commitfest/tests/test_closure_notifications.py

@@ -16,6 +16,7 @@
     Topic,
 )
 from pgcommitfest.mailqueue.models import QueuedMail
+from pgcommitfest.userprofile.models import UserProfile
 
 pytestmark = pytest.mark.django_db
 
@@ -129,7 +130,10 @@ def test_one_email_per_author_with_multiple_patches(alice, in_progress_cf, topic
 
 
 def test_multiple_authors_receive_separate_emails(alice, bob, in_progress_cf, topic):
-    """Each author of open patches should receive their own notification."""
+    """Each author of open patches should receive their own notification (if opted in)."""
+    # Bob also needs notify_all_author enabled to receive closure emails
+    UserProfile.objects.create(user=bob, notify_all_author=True)
+
     patch1 = Patch.objects.create(name="Alice Patch", topic=topic)
     patch1.authors.add(alice)
     PatchOnCommitFest.objects.create(
@@ -176,7 +180,10 @@ def test_notification_includes_next_commitfest_info(
 
 
 def test_coauthors_both_receive_notification(alice, bob, in_progress_cf, topic):
-    """Both co-authors of a patch should receive notifications."""
+    """Both co-authors of a patch should receive notifications (if opted in)."""
+    # Bob also needs notify_all_author enabled to receive closure emails
+    UserProfile.objects.create(user=bob, notify_all_author=True)
+
     patch = Patch.objects.create(name="Coauthored Patch", topic=topic)
     patch.authors.add(alice)
     patch.authors.add(bob)
@@ -195,7 +202,8 @@ def test_coauthors_both_receive_notification(alice, bob, in_progress_cf, topic):
 
 
 def test_no_notification_for_author_without_email(bob, in_progress_cf, topic):
-    """Authors without email addresses should be skipped."""
+    """Authors without email addresses should be skipped even if opted in."""
+    UserProfile.objects.create(user=bob, notify_all_author=True)
     bob.email = ""
     bob.save()
 
@@ -213,6 +221,25 @@ def test_no_notification_for_author_without_email(bob, in_progress_cf, topic):
     assert QueuedMail.objects.count() == 0
 
 
+def test_no_notification_for_author_without_notify_all_author(
+    bob, in_progress_cf, topic
+):
+    """Authors without notify_all_author enabled should not receive closure notifications."""
+    # bob has no UserProfile, so notify_all_author is not enabled
+    patch = Patch.objects.create(name="Test Patch", topic=topic)
+    patch.authors.add(bob)
+    PatchOnCommitFest.objects.create(
+        patch=patch,
+        commitfest=in_progress_cf,
+        enterdate=datetime.now(),
+        status=PatchOnCommitFest.STATUS_REVIEW,
+    )
+
+    in_progress_cf.send_closure_notifications()
+
+    assert QueuedMail.objects.count() == 0
+
+
 # Auto-move tests