From d8834377be0ce2f1d41ad5588bc8b6588b8ebcd8 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 1 Aug 2018 08:39:08 -0700
Subject: [PATCH] When a Herald rule blocks a push, show which rule fired in
 the push log UI

Summary:
Ref T13164. See PHI765. We currently show "Rejected: Herald" in the push log UI, but don't show which rule rejected a push.

We store this data, and it's potentially useful: either for hunting down a particular issue, or for getting a general sense of how often a reject rule is triggering (maybe because you want to tune how aggressive it is).

Show this data in the web UI, and include it in the data export payload.

Test Plan:
  - Pushed to a hosted repository so that I got blocked by a Herald rule.
  - Viewed the push logs in the web UI, now saw which rule triggered things.
  - Exported logs to CSV, saw Herald rule PHIDs in the data.

{F5776211}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13164

Differential Revision: https://secure.phabricator.com/D19555
---
 .../view/DiffusionPushLogListView.php         | 23 +++++++++++++++----
 ...abricatorRepositoryPushLogSearchEngine.php |  4 ++++
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/applications/diffusion/view/DiffusionPushLogListView.php b/src/applications/diffusion/view/DiffusionPushLogListView.php
index e101625752..12a5fa5494 100644
--- a/src/applications/diffusion/view/DiffusionPushLogListView.php
+++ b/src/applications/diffusion/view/DiffusionPushLogListView.php
@@ -14,6 +14,8 @@ final class DiffusionPushLogListView extends AphrontView {
     $logs = $this->logs;
     $viewer = $this->getViewer();
 
+    $reject_herald = PhabricatorRepositoryPushLog::REJECT_HERALD;
+
     $handle_phids = array();
     foreach ($logs as $log) {
       $handle_phids[] = $log->getPusherPHID();
@@ -21,9 +23,13 @@ final class DiffusionPushLogListView extends AphrontView {
       if ($device_phid) {
         $handle_phids[] = $device_phid;
       }
+
+      if ($log->getPushEvent()->getRejectCode() == $reject_herald) {
+        $handle_phids[] = $log->getPushEvent()->getRejectDetails();
+      }
     }
 
-    $handles = $viewer->loadHandles($handle_phids);
+    $viewer->loadHandles($handle_phids);
 
     // Only administrators can view remote addresses.
     $remotes_visible = $viewer->getIsAdmin();
@@ -74,10 +80,17 @@ final class DiffusionPushLogListView extends AphrontView {
         $flag_names);
 
       $reject_code = $log->getPushEvent()->getRejectCode();
-      $reject_label = idx(
-        $reject_map,
-        $reject_code,
-        pht('Unknown ("%s")', $reject_code));
+
+      if ($reject_code == $reject_herald) {
+        $rule_phid = $log->getPushEvent()->getRejectDetails();
+        $handle = $viewer->renderHandle($rule_phid);
+        $reject_label = pht('Blocked: %s', $handle);
+      } else {
+        $reject_label = idx(
+          $reject_map,
+          $reject_code,
+          pht('Unknown ("%s")', $reject_code));
+      }
 
       $rows[] = array(
         phutil_tag(
diff --git a/src/applications/repository/query/PhabricatorRepositoryPushLogSearchEngine.php b/src/applications/repository/query/PhabricatorRepositoryPushLogSearchEngine.php
index 7b626fff32..eb2bc6b45b 100644
--- a/src/applications/repository/query/PhabricatorRepositoryPushLogSearchEngine.php
+++ b/src/applications/repository/query/PhabricatorRepositoryPushLogSearchEngine.php
@@ -149,6 +149,9 @@ final class PhabricatorRepositoryPushLogSearchEngine
       id(new PhabricatorStringExportField())
         ->setKey('resultName')
         ->setLabel(pht('Result Name')),
+      id(new PhabricatorStringExportField())
+        ->setKey('resultDetails')
+        ->setLabel(pht('Result Details')),
       id(new PhabricatorIntExportField())
         ->setKey('writeWait')
         ->setLabel(pht('Write Wait (us)')),
@@ -237,6 +240,7 @@ final class PhabricatorRepositoryPushLogSearchEngine
         'flagNames' => $flag_names,
         'result' => $result,
         'resultName' => $result_name,
+        'resultDetails' => $event->getRejectDetails(),
         'writeWait' => $event->getWriteWait(),
         'readWait' => $event->getReadWait(),
         'hostWait' => $event->getHostWait(),