Make CFbot add a history item when patch needs rebase

This is needed to send notifications to a patch author that it
subscribed. We don't send updates to other subscribers to the patch, as
this information is generally only useful for the author themselves.

By using the "history item" infrastructure it's also displayed on the
patch page in the "History" table. That seems quite useful too, because
it makes it clear how long the patch has needed a rebase.

Author: JelteF

pgcommitfest/commitfest/migrations/0007_needs_rebase_emails.py

@@ -0,0 +1,47 @@
+# Generated by Django 4.2.17 on 2024-12-25 11:17
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ("commitfest", "0006_cfbot_integration"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="cfbotbranch",
+            name="needs_rebase_since",
+            field=models.DateTimeField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name="patchhistory",
+            name="by_cfbot",
+            field=models.BooleanField(default=False),
+        ),
+        migrations.AlterField(
+            model_name="patchhistory",
+            name="by",
+            field=models.ForeignKey(
+                blank=True,
+                null=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                to=settings.AUTH_USER_MODEL,
+            ),
+        ),
+        migrations.AddConstraint(
+            model_name="patchhistory",
+            constraint=models.CheckConstraint(
+                check=models.Q(
+                    models.Q(("by_cfbot", True), ("by__isnull", True)),
+                    models.Q(("by_cfbot", False), ("by__isnull", False)),
+                    _connector="OR",
+                ),
+                name="check_by",
+            ),
+        ),
+    ]

pgcommitfest/commitfest/models.py

@@ -224,46 +224,61 @@ class Meta:
 class PatchHistory(models.Model):
     patch = models.ForeignKey(Patch, blank=False, null=False, on_delete=models.CASCADE)
     date = models.DateTimeField(blank=False, null=False, auto_now_add=True, db_index=True)
-    by = models.ForeignKey(User, blank=False, null=False, on_delete=models.CASCADE)
+    by = models.ForeignKey(User, blank=True, null=True, on_delete=models.CASCADE)
+    by_cfbot = models.BooleanField(null=False, blank=False, default=False)
     what = models.CharField(max_length=500, null=False, blank=False)
 
     @property
     def by_string(self):
+        if (self.by_cfbot):
+            return "CFbot"
+
         return "%s %s (%s)" % (self.by.first_name, self.by.last_name, self.by.username)
 
     def __str__(self):
         return "%s - %s" % (self.patch.name, self.date)
 
     class Meta:
         ordering = ('-date', )
+        constraints = [
+            models.CheckConstraint(
+                check=(
+                    models.Q(by_cfbot=True) & models.Q(by__isnull=True)
+                ) | (
+                    models.Q(by_cfbot=False) & models.Q(by__isnull=False)
+                ),
+                name='check_by',
+            ),
+        ]
 
     def save_and_notify(self, prevcommitter=None,
-                        prevreviewers=None, prevauthors=None):
+                        prevreviewers=None, prevauthors=None, authors_only=False):
         # Save this model, and then trigger notifications if there are any. There are
         # many different things that can trigger notifications, so try them all.
         self.save()
 
         recipients = []
-        recipients.extend(self.patch.subscribers.all())
-
-        # Current or previous committer wants all notifications
-        try:
-            if self.patch.committer and self.patch.committer.user.userprofile.notify_all_committer:
-                recipients.append(self.patch.committer.user)
-        except UserProfile.DoesNotExist:
-            pass
-
-        try:
-            if prevcommitter and prevcommitter.user.userprofile.notify_all_committer:
-                recipients.append(prevcommitter.user)
-        except UserProfile.DoesNotExist:
-            pass
-
-        # Current or previous reviewers wants all notifications
-        recipients.extend(self.patch.reviewers.filter(userprofile__notify_all_reviewer=True))
-        if prevreviewers:
-            # prevreviewers is a list
-            recipients.extend(User.objects.filter(id__in=[p.id for p in prevreviewers], userprofile__notify_all_reviewer=True))
+        if not authors_only:
+            recipients.extend(self.patch.subscribers.all())
+
+            # Current or previous committer wants all notifications
+            try:
+                if self.patch.committer and self.patch.committer.user.userprofile.notify_all_committer:
+                    recipients.append(self.patch.committer.user)
+            except UserProfile.DoesNotExist:
+                pass
+
+            try:
+                if prevcommitter and prevcommitter.user.userprofile.notify_all_committer:
+                    recipients.append(prevcommitter.user)
+            except UserProfile.DoesNotExist:
+                pass
+
+            # Current or previous reviewers wants all notifications
+            recipients.extend(self.patch.reviewers.filter(userprofile__notify_all_reviewer=True))
+            if prevreviewers:
+                # prevreviewers is a list
+                recipients.extend(User.objects.filter(id__in=[p.id for p in prevreviewers], userprofile__notify_all_reviewer=True))
 
         # Current or previous authors wants all notifications
         recipients.extend(self.patch.authors.filter(userprofile__notify_all_author=True))
@@ -358,6 +373,7 @@ class CfbotBranch(models.Model):
     apply_url = models.TextField(null=False)
     # Actually a postgres enum column
     status = models.TextField(choices=STATUS_CHOICES, null=False)
+    needs_rebase_since = models.DateTimeField(null=True, blank=True)
     created = models.DateTimeField(auto_now_add=True)
     modified = models.DateTimeField(auto_now=True)
 

pgcommitfest/commitfest/templates/mail/patch_notify.txt

@@ -5,7 +5,7 @@ have received updates in the PostgreSQL commitfest app:
 {{p.patch.name}}
 https://commitfest.postgresql.org/{{p.patch.patchoncommitfest_set.all.0.commitfest.id}}/{{p.patch.id}}/
 {%for h in p.entries%}
-* {{h.what}} ({{h.by}}){%endfor%}
+* {{h.what}} by {{h.by_string()}}{%endfor%}
 
 
 {%endfor%}

pgcommitfest/commitfest/views.py

@@ -823,7 +823,7 @@ def cfbot_ingest(message):
     branch_id = branch_status["branch_id"]
 
     try:
-        Patch.objects.get(pk=patch_id)
+        patch = Patch.objects.get(pk=patch_id)
     except Patch.DoesNotExist:
         # If the patch doesn't exist, there's nothing to do. This should never
         # happen in production, but on the test system it's possible because
@@ -910,6 +910,24 @@ def cfbot_ingest(message):
     # per patch.
     cursor.execute("DELETE FROM commitfest_cfbottask WHERE patch_id=%s AND branch_id != %s", (patch_id, branch_id))
 
+    # We change the needs_rebase field using a separate UPDATE because this way
+    # we can find out what the previous state of the field was (sadly INSERT ON
+    # CONFLICT does not allow us to return that). We need to know the previous
+    # state so we can skip sending notifications if the needs_rebase status did
+    # not change.
+    needs_rebase = branch_status['commit_id'] is None
+    if bool(branch_in_db.needs_rebase_since) is not needs_rebase:
+        if needs_rebase:
+            branch_in_db.needs_rebase_since = datetime.now()
+        else:
+            branch_in_db.needs_rebase_since = None
+        branch_in_db.save()
+
+        if needs_rebase:
+            PatchHistory(patch=patch, by=None, by_cfbot=True, what='Patch needs rebase').save_and_notify(authors_only=True)
+        else:
+            PatchHistory(patch=patch, by=None, by_cfbot=True, what='Patch does not need rebase anymore').save_and_notify(authors_only=True)
+
 
 @csrf_exempt
 def cfbot_notify(request):