From 9a4a5dd624a1cedc7cdc40687815739b228e5c77 Mon Sep 17 00:00:00 2001
From: Pierre Bourdon <delroth@gmail.com>
Date: Sat, 20 Jul 2024 13:09:39 +0200
Subject: [PATCH] jobset-eval: fix actions not showing up sometimes for new
 jobs

New jobs have their "new" status take precedence over them being
"failed" or "queued", which means actions that can act on "failed" or
"queued" jobs weren't shown to the user when they could only act on
"new" jobs.
---
 src/lib/Hydra/Controller/JobsetEval.pm |  4 +++-
 src/lib/Hydra/Helper/BuildDiff.pm      | 18 +++++++++++++++---
 src/root/jobset-eval.tt                |  8 ++++----
 t/Hydra/Helper/BuildDiff.t             | 17 +++++++----------
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/src/lib/Hydra/Controller/JobsetEval.pm b/src/lib/Hydra/Controller/JobsetEval.pm
index 643a516c..77c01a84 100644
--- a/src/lib/Hydra/Controller/JobsetEval.pm
+++ b/src/lib/Hydra/Controller/JobsetEval.pm
@@ -76,7 +76,9 @@ sub view_GET {
     $c->stash->{removed} = $diff->{removed};
     $c->stash->{unfinished} = $diff->{unfinished};
     $c->stash->{aborted} = $diff->{aborted};
-    $c->stash->{failed} = $diff->{failed};
+    $c->stash->{totalAborted} = $diff->{totalAborted};
+    $c->stash->{totalFailed} = $diff->{totalFailed};
+    $c->stash->{totalQueued} = $diff->{totalQueued};
 
     $c->stash->{full} = ($c->req->params->{full} || "0") eq "1";
 
diff --git a/src/lib/Hydra/Helper/BuildDiff.pm b/src/lib/Hydra/Helper/BuildDiff.pm
index 65dad17c..be8525d6 100644
--- a/src/lib/Hydra/Helper/BuildDiff.pm
+++ b/src/lib/Hydra/Helper/BuildDiff.pm
@@ -32,7 +32,12 @@ sub buildDiff {
         removed => [],
         unfinished => [],
         aborted => [],
-        failed => [],
+
+        # These summary counters cut across the categories to determine whether
+        # actions such as "Restart all failed" or "Bump queue" are available.
+        totalAborted => 0,
+        totalFailed => 0,
+        totalQueued => 0,
     };
 
     my $n = 0;
@@ -80,8 +85,15 @@ sub buildDiff {
         } else {
             push @{$ret->{new}}, $build if !$found;
         }
-        if (defined $build->buildstatus && $build->buildstatus != 0) {
-            push @{$ret->{failed}}, $build;
+
+        if ($build->finished != 0 && $build->buildstatus != 0) {
+            if ($aborted) {
+                ++$ret->{totalAborted};
+            } else {
+                ++$ret->{totalFailed};
+            }
+        } elsif ($build->finished == 0) {
+            ++$ret->{totalQueued};
         }
     }
 
diff --git a/src/root/jobset-eval.tt b/src/root/jobset-eval.tt
index f0b92f97..12086d85 100644
--- a/src/root/jobset-eval.tt
+++ b/src/root/jobset-eval.tt
@@ -48,16 +48,16 @@ c.uri_for(c.controller('JobsetEval').action_for('view'),
       <a class="nav-link dropdown-toggle" data-toggle="dropdown" href="#">Actions</a>
       <div class="dropdown-menu">
         <a class="dropdown-item" href="[% c.uri_for(c.controller('JobsetEval').action_for('create_jobset'), [eval.id]) %]">Create a jobset from this evaluation</a>
-        [% IF unfinished.size > 0 %]
+        [% IF totalQueued > 0 %]
           <a class="dropdown-item" href="[% c.uri_for(c.controller('JobsetEval').action_for('cancel'), [eval.id]) %]">Cancel all scheduled builds</a>
         [% END %]
-        [% IF aborted.size > 0 || stillFail.size > 0 || nowFail.size > 0 || failed.size > 0 %]
+        [% IF totalFailed > 0 %]
           <a class="dropdown-item" href="[% c.uri_for(c.controller('JobsetEval').action_for('restart_failed'), [eval.id]) %]">Restart all failed builds</a>
         [% END %]
-        [% IF aborted.size > 0 %]
+        [% IF totalAborted > 0 %]
           <a class="dropdown-item" href="[% c.uri_for(c.controller('JobsetEval').action_for('restart_aborted'), [eval.id]) %]">Restart all aborted builds</a>
         [% END %]
-        [% IF unfinished.size > 0 %]
+        [% IF totalQueued > 0 %]
           <a class="dropdown-item" href="[% c.uri_for(c.controller('JobsetEval').action_for('bump'), [eval.id]) %]">Bump builds to front of queue</a>
         [% END %]
       </div>
diff --git a/t/Hydra/Helper/BuildDiff.t b/t/Hydra/Helper/BuildDiff.t
index 243bb596..eef25a0f 100644
--- a/t/Hydra/Helper/BuildDiff.t
+++ b/t/Hydra/Helper/BuildDiff.t
@@ -25,7 +25,10 @@ subtest "empty diff" => sub {
             removed => [],
             unfinished => [],
             aborted => [],
-            failed => [],
+
+            totalAborted => 0,
+            totalFailed => 0,
+            totalQueued => 0,
         },
         "empty list of jobs returns empty diff"
     );
@@ -48,12 +51,7 @@ subtest "2 different jobs" => sub {
         "succeed_with_failed is a new job"
     );
 
-    is(scalar(@{$ret->{failed}}), 1, "list of failed jobs is 1 element long");
-    is(
-        $ret->{failed}[0]->get_column('id'),
-        $builds->{"succeed_with_failed"}->get_column('id'),
-        "succeed_with_failed is a failed job"
-    );
+    is($ret->{totalFailed}, 1, "total failed jobs is 1");
 
     is(
         $ret->{removed},
@@ -70,9 +68,9 @@ subtest "2 different jobs" => sub {
 subtest "failed job with no previous history" => sub {
     my $ret = buildDiff([$builds->{"fails"}], []);
 
-    is(scalar(@{$ret->{failed}}), 1, "list of failed jobs is 1 element long");
+    is($ret->{totalFailed}, 1, "total failed jobs is 1");
     is(
-        $ret->{failed}[0]->get_column('id'),
+        $ret->{new}[0]->get_column('id'),
         $builds->{"fails"}->get_column('id'),
         "fails is a failed job"
     );
@@ -93,7 +91,6 @@ subtest "not-yet-built job with no previous history" => sub {
     is($ret->{removed}, [], "removed");
     is($ret->{unfinished}, [], "unfinished");
     is($ret->{aborted}, [], "aborted");
-    is($ret->{failed}, [], "failed");
 
     is(scalar(@{$ret->{new}}), 1, "list of new jobs is 1 element long");
     is(