Allow sorting commitfest entries by "failing since" (#47)

This adds a new column to the `CfbotBranch` model: `failing_since`. This
tracks when a patch first started to fail the CI or started requiring a
rebase. This value its primary use is to allow people to sort by the
"CI status" column in the patch list. This allows people to quickly see
what patches are healthy, which require the author to take action, and
which might benefit from a reviewer pinging the author that the patch
has been broken for a while. Non-failing builds are sorted on their CI
build creation time, so patches that are currently running CI are shown
at the top, followed by patches for which CI succeeded most recently.

The values of needs_rebase_since and failing_since are also shown on
hover and on the patch pages.

Fixes #42

Author: JelteF

pgcommitfest/commitfest/migrations/0010_add_failing_since_column.py

@@ -0,0 +1,17 @@
+# Generated by Django 4.2.19 on 2025-03-01 13:53
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("commitfest", "0009_extra_branch_fields"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="cfbotbranch",
+            name="failing_since",
+            field=models.DateTimeField(blank=True, null=True),
+        ),
+    ]

pgcommitfest/commitfest/models.py

@@ -469,6 +469,7 @@ class CfbotBranch(models.Model):
     # Actually a postgres enum column
     status = models.TextField(choices=STATUS_CHOICES, null=False)
     needs_rebase_since = models.DateTimeField(null=True, blank=True)
+    failing_since = models.DateTimeField(null=True, blank=True)
     created = models.DateTimeField(auto_now_add=True)
     modified = models.DateTimeField(auto_now=True)
     version = models.TextField(null=True, blank=True)

pgcommitfest/commitfest/templates/commitfest.html

@@ -64,7 +64,7 @@ <h3>{{p.is_open|yesno:"Active patches,Closed patches"}}</h3>
    <th><a href="#" style="color:#333333;" onclick="return sortpatches(4);">ID</a>{%if sortkey == 4%}<div style="float:right;"><i class="glyphicon glyphicon-arrow-down"></i></div>{%elif sortkey == -4%}<div style="float:right;"><i class="glyphicon glyphicon-arrow-up"></i></div>{%endif%}</th>
    <th>Status</th>
    <th>Ver</th>
-   <th>CI status</th>
+   <th><a href="#" style="color:#333333;" onclick="return sortpatches(7);">CI status</a>{%if sortkey == 7%}<div style="float:right;"><i class="glyphicon glyphicon-arrow-down"></i></div>{%elif sortkey == -7%}<div style="float:right;"><i class="glyphicon glyphicon-arrow-up"></i></div>{%endif%}</th>
    <th><a href="#" style="color:#333333;" onclick="return sortpatches(6);">Stats</a>{%if sortkey == 6%}<div style="float:right;"><i class="glyphicon glyphicon-arrow-down"></i></div>{%elif sortkey == -6%}<div style="float:right;"><i class="glyphicon glyphicon-arrow-up"></i></div>{%endif%}</th>
    <th>Author</th>
    <th>Reviewers</th>
@@ -94,14 +94,14 @@ <h3>{{p.is_open|yesno:"Active patches,Closed patches"}}</h3>
     {%with p.cfbot_results as cfb%}
     {%if not cfb %}
       <span class="label label-default">Not processed</span>
-    {%elif cfb.needs_rebase %}
-    <a href="{{cfb.apply_url}}" title="View git apply logs">
+    {%elif p.needs_rebase_since %}
+    <a href="{{cfb.apply_url}}" title="View git apply logs. Needs rebase since {{p.needs_rebase_since|timesince}}. {%if p.failing_since and p.failing_since != p.needs_rebase_since %}Failing since {{p.failing_since|timesince}}.{%endif%}">
      <span class="label label-warning">Needs rebase!</span>
     </a>
     {%else%}
     <a href="https://github.com/postgresql-cfbot/postgresql/compare/cf/{{p.id}}~1...cf/{{p.id}}" title="View last patch set on GitHub"><img class="github-logo" src="/media/commitfest/github-mark.svg"/></a>
     <a href="https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F{{p.id}}"
-       title="View CI history{%if cfb.failed_task_names %}. Failed jobs: {{cfb.failed_task_names}}{%endif%}">
+       title="View CI history. {%if p.failing_since%}Failing since {{p.failing_since|timesince}}. {%endif%}{%if cfb.failed_task_names %}Failed jobs: {{cfb.failed_task_names}}{%endif%}">
       {%if cfb.failed > 0 or cfb.branch_status == 'failed' or cfb.branch_status == 'timeout' %}
       <img src="/media/commitfest/new_failure.svg"/>
       {%elif cfb.completed < cfb.total  %}

pgcommitfest/commitfest/templates/patch.html

@@ -17,10 +17,10 @@
       <td>
         {%if not cfbot_branch %}
         <span class="label label-default">Not processed</span></a>
-        {%elif not cfbot_branch.commit_id %}
+        {%elif cfbot_branch.needs_rebase_since %}
           <a href="{{cfbot_branch.apply_url}}">
           <span class="label label-warning" title="View git apply logs">Needs rebase!</span></a>
-          Additional links previous successfully applied patch (outdated):
+          Needs rebase since {{cfbot_branch.needs_rebase_since|timesince}}. {%if cfbot_branch.failing_since and cfbot_branch.failing_since != cfbot_branch.needs_rebase_since %}Failing since {{cfbot_branch.failing_since|timesince}}. {%endif%}<br>Additional links previous successfully applied patch (outdated):<br>
           <a href="https://github.com/postgresql-cfbot/postgresql/compare/cf/{{patch.id}}~1...cf/{{patch.id}}" title="View previous successfully applied patch set on GitHub"><img class="github-logo" src="/media/commitfest/github-mark.svg"/></a>
           <a href="https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F{{patch.id}}">
           <span class="label label-default">Summary</span></a>

pgcommitfest/commitfest/views.py

@@ -265,6 +265,10 @@ def commitfest(request, cfid):
             )
         elif sortkey == -6:
             orderby_str = "branch.all_additions + branch.all_deletions DESC NULLS LAST, created DESC"
+        elif sortkey == 7:
+            orderby_str = "branch.failing_since DESC NULLS FIRST, branch.created DESC"
+        elif sortkey == -7:
+            orderby_str = "branch.failing_since NULLS LAST, branch.created"
         else:
             orderby_str = "p.id"
             sortkey = 0
@@ -294,6 +298,8 @@ def commitfest(request, cfid):
 (SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_authors cpa ON cpa.user_id=auth_user.id WHERE cpa.patch_id=p.id) AS author_names,
 (SELECT string_agg(first_name || ' ' || last_name || ' (' || username || ')', ', ') FROM auth_user INNER JOIN commitfest_patch_reviewers cpr ON cpr.user_id=auth_user.id WHERE cpr.patch_id=p.id) AS reviewer_names,
 (SELECT count(1) FROM commitfest_patchoncommitfest pcf WHERE pcf.patch_id=p.id) AS num_cfs,
+branch.needs_rebase_since,
+branch.failing_since,
 (
     SELECT row_to_json(t) as cfbot_results
     from (
@@ -303,7 +309,6 @@ def commitfest(request, cfid):
             count(*) FILTER (WHERE task.status in ('ABORTED', 'ERRORED', 'FAILED')) failed,
             count(*) total,
             string_agg(task.task_name, ', ') FILTER (WHERE task.status in ('ABORTED', 'ERRORED', 'FAILED')) as failed_task_names,
-            branch.commit_id IS NULL as needs_rebase,
             branch.status as branch_status,
             branch.apply_url,
             branch.patch_count,
@@ -1250,13 +1255,14 @@ def cfbot_ingest(message):
     # 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_save = False
     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()
+        needs_save = True
 
         if needs_rebase:
             PatchHistory(
@@ -1270,6 +1276,27 @@ def cfbot_ingest(message):
                 what="Patch does not need rebase anymore",
             ).save_and_notify(authors_only=True)
 
+    # Similarly, we change the failing_since field using a separate UPDATE
+    failing = branch_status["status"] in ("failed", "timeout") or needs_rebase
+    finished = branch_status["status"] == "finished"
+
+    if "task_status" in message and message["task_status"]["status"] in (
+        "ABORTED",
+        "ERRORED",
+        "FAILED",
+    ):
+        failing = True
+
+    if (failing or finished) and bool(branch_in_db.failing_since) is not failing:
+        if failing:
+            branch_in_db.failing_since = datetime.now()
+        else:
+            branch_in_db.failing_since = None
+        needs_save = True
+
+    if needs_save:
+        branch_in_db.save()
+
 
 @csrf_exempt
 def cfbot_notify(request):