From 7ed28dacb585416858a2af29b37c2606c39420fe Mon Sep 17 00:00:00 2001
From: Bob Trahan <btrahan@phacility.com>
Date: Tue, 29 Apr 2014 15:07:00 -0700
Subject: [PATCH] Diffusion + Herald - warn users if importing repository

Summary: 'cuz things fail a bunch until importing is done. Fixes T4094.

Test Plan: set isImporting to return true. Browsed Diffusion and saw helpful warnings everywhere. Browse Herald transcript and saw a helpful warning

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4094

Differential Revision: https://secure.phabricator.com/D8903
---
 .../controller/DiffusionController.php        | 54 +++++++++++++++----
 .../controller/HeraldTranscriptController.php | 51 ++++++++++++++++--
 2 files changed, 92 insertions(+), 13 deletions(-)

diff --git a/src/applications/diffusion/controller/DiffusionController.php b/src/applications/diffusion/controller/DiffusionController.php
index 14c819614e..25cc6513a4 100644
--- a/src/applications/diffusion/controller/DiffusionController.php
+++ b/src/applications/diffusion/controller/DiffusionController.php
@@ -4,6 +4,18 @@ abstract class DiffusionController extends PhabricatorController {
 
   protected $diffusionRequest;
 
+  public function setDiffusionRequest(DiffusionRequest $request) {
+    $this->diffusionRequest = $request;
+    return $this;
+  }
+
+  protected function getDiffusionRequest() {
+    if (!$this->diffusionRequest) {
+      throw new Exception("No Diffusion request object!");
+    }
+    return $this->diffusionRequest;
+  }
+
   public function willBeginExecution() {
     $request = $this->getRequest();
 
@@ -24,20 +36,26 @@ abstract class DiffusionController extends PhabricatorController {
       $drequest = DiffusionRequest::newFromAphrontRequestDictionary(
         $data,
         $this->getRequest());
-      $this->diffusionRequest = $drequest;
+      $this->setDiffusionRequest($drequest);
     }
   }
 
-  public function setDiffusionRequest(DiffusionRequest $request) {
-    $this->diffusionRequest = $request;
-    return $this;
-  }
+  public function buildApplicationPage($view, array $options) {
+    $drequest = $this->getDiffusionRequest();
+    $repository = $drequest->getRepository();
+    $error_view = $this->buildRepositoryWarning($repository);
 
-  protected function getDiffusionRequest() {
-    if (!$this->diffusionRequest) {
-      throw new Exception("No Diffusion request object!");
+    $views = array();
+    $not_inserted = true;
+    foreach ($view as $view_object_or_array) {
+      $views[] = $view_object_or_array;
+      if ($not_inserted &&
+          $view_object_or_array instanceof PhabricatorCrumbsView) {
+        $views[] = $error_view;
+        $not_inserted = false;
+      }
     }
-    return $this->diffusionRequest;
+    return parent::buildApplicationPage($views, $options);
   }
 
   public function buildCrumbs(array $spec = array()) {
@@ -235,4 +253,22 @@ abstract class DiffusionController extends PhabricatorController {
       ->appendChild($body);
   }
 
+  private function buildRepositoryWarning(PhabricatorRepository $repository) {
+    $error_view = null;
+    if ($repository->isImporting()) {
+      $title = pht('This repository is still importing.');
+      $body = pht('Things may not work properly until the import finishes.');
+    } else if (!$repository->isTracked()) {
+      $title = pht('This repository is not tracked.');
+      $body = pht(
+        'Things may not work properly until tracking is enabled and '.
+        'importing finishes.');
+    }
+
+    if ($title) {
+      $error_view = $this->renderStatusMessage($title, $body);
+    }
+
+    return $error_view;
+  }
 }
diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php
index 14a2d04cd3..8a44face99 100644
--- a/src/applications/herald/controller/HeraldTranscriptController.php
+++ b/src/applications/herald/controller/HeraldTranscriptController.php
@@ -83,6 +83,9 @@ final class HeraldTranscriptController extends HeraldController {
         $nav->appendChild($notice);
       }
 
+      $warning_panel = $this->buildWarningPanel($xscript);
+      $nav->appendChild($warning_panel);
+
       $apply_xscript_panel = $this->buildApplyTranscriptPanel(
         $xscript);
       $nav->appendChild($apply_xscript_panel);
@@ -287,7 +290,47 @@ final class HeraldTranscriptController extends HeraldController {
         $keep_rule_xscripts));
   }
 
-  private function buildApplyTranscriptPanel($xscript) {
+  private function buildWarningPanel(HeraldTranscript $xscript) {
+    $request = $this->getRequest();
+    $panel = null;
+    if ($xscript->getObjectTranscript()) {
+      $handles = $this->handles;
+      $object_xscript = $xscript->getObjectTranscript();
+      $handle = $handles[$object_xscript->getPHID()];
+      if ($handle->getType() ==
+          PhabricatorRepositoryPHIDTypeCommit::TYPECONST) {
+        $commit = id(new DiffusionCommitQuery())
+          ->setViewer($request->getUser())
+          ->withPHIDs(array($handle->getPHID()))
+          ->executeOne();
+        if ($commit) {
+          $repository = $commit->getRepository();
+          if ($repository->isImporting()) {
+            $title = pht(
+              'The %s repository is still importing.',
+              $repository->getMonogram());
+            $body = pht(
+              'Herald rules will not trigger until import completes.');
+          } else if (!$repository->isTracked()) {
+            $title = pht(
+              'The %s repository is not tracked.',
+              $repository->getMonogram());
+            $body = pht(
+              'Herald rules will not trigger until tracking is enabled.');
+          } else {
+            return $panel;
+          }
+          $panel = id(new AphrontErrorView())
+            ->setSeverity(AphrontErrorView::SEVERITY_WARNING)
+            ->setTitle($title)
+            ->appendChild($body);
+        }
+      }
+    }
+    return $panel;
+  }
+
+  private function buildApplyTranscriptPanel(HeraldTranscript $xscript) {
     $handles = $this->handles;
     $adapter = $this->getAdapter();
 
@@ -350,7 +393,7 @@ final class HeraldTranscriptController extends HeraldController {
     return $box;
   }
 
-  private function buildActionTranscriptPanel($xscript) {
+  private function buildActionTranscriptPanel(HeraldTranscript $xscript) {
     $action_xscript = mgroup($xscript->getApplyTranscripts(), 'getRuleID');
 
     $adapter = $this->getAdapter();
@@ -442,7 +485,7 @@ final class HeraldTranscriptController extends HeraldController {
     return $box;
   }
 
-  private function buildObjectTranscriptPanel($xscript) {
+  private function buildObjectTranscriptPanel(HeraldTranscript $xscript) {
 
     $adapter = $this->getAdapter();
     $field_names = $adapter->getFieldNameMap();
@@ -452,7 +495,7 @@ final class HeraldTranscriptController extends HeraldController {
     $data = array();
     if ($object_xscript) {
       $phid = $object_xscript->getPHID();
-      $handles = $this->loadViewerHandles(array($phid));
+      $handles = $this->handles;
 
       $data += array(
         pht('Object Name') => $object_xscript->getName(),