Show more detailed hints about draft revisions in the UI

Summary: Ref T2543. When revisions are in the draft state, tell the user what we're waiting for or why they aren't moving forward.

Test Plan: {F5228840}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T2543

Differential Revision: https://secure.phabricator.com/D18714

src/__phutil_library_map__.php

@@ -444,6 +444,7 @@ phutil_register_library_map(array(
     'DifferentialDiffTransactionQuery' => 'applications/differential/query/DifferentialDiffTransactionQuery.php',
     'DifferentialDiffViewController' => 'applications/differential/controller/DifferentialDiffViewController.php',
     'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php',
+    'DifferentialDraftField' => 'applications/differential/customfield/DifferentialDraftField.php',
     'DifferentialExactUserFunctionDatasource' => 'applications/differential/typeahead/DifferentialExactUserFunctionDatasource.php',
     'DifferentialFieldParseException' => 'applications/differential/exception/DifferentialFieldParseException.php',
     'DifferentialFieldValidationException' => 'applications/differential/exception/DifferentialFieldValidationException.php',
@@ -5451,6 +5452,7 @@ phutil_register_library_map(array(
     'DifferentialDiffTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
     'DifferentialDiffViewController' => 'DifferentialController',
     'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher',
+    'DifferentialDraftField' => 'DifferentialCoreCustomField',
     'DifferentialExactUserFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'DifferentialFieldParseException' => 'Exception',
     'DifferentialFieldValidationException' => 'Exception',

src/applications/differential/customfield/DifferentialDraftField.php

@@ -0,0 +1,90 @@
+<?php
+
+final class DifferentialDraftField
+  extends DifferentialCoreCustomField {
+
+  public function getFieldKey() {
+    return 'differential:draft';
+  }
+
+  public function getFieldName() {
+    return pht('Draft');
+  }
+
+  public function getFieldDescription() {
+    return pht('Show a warning about draft revisions.');
+  }
+
+  protected function readValueFromRevision(
+    DifferentialRevision $revision) {
+    return null;
+  }
+
+  public function shouldAppearInPropertyView() {
+    return true;
+  }
+
+  public function renderPropertyViewValue() {
+    return null;
+  }
+
+  public function getWarningsForRevisionHeader(array $handles) {
+    $viewer = $this->getViewer();
+    $revision = $this->getObject();
+
+    if (!$revision->isDraft()) {
+      return array();
+    }
+
+    $warnings = array();
+
+    $blocking_map = array(
+      HarbormasterBuildStatus::STATUS_FAILED,
+      HarbormasterBuildStatus::STATUS_ABORTED,
+      HarbormasterBuildStatus::STATUS_ERROR,
+      HarbormasterBuildStatus::STATUS_PAUSED,
+      HarbormasterBuildStatus::STATUS_DEADLOCKED,
+    );
+    $blocking_map = array_fuse($blocking_map);
+
+    $builds = $revision->loadActiveBuilds($viewer);
+
+    $waiting = array();
+    $blocking = array();
+    foreach ($builds as $build) {
+      if (isset($blocking_map[$build->getBuildStatus()])) {
+        $blocking[] = $build;
+      } else {
+        $waiting[] = $build;
+      }
+    }
+
+    $blocking_list = $viewer->renderHandleList(mpull($blocking, 'getPHID'))
+      ->setAsInline(true);
+    $waiting_list = $viewer->renderHandleList(mpull($waiting, 'getPHID'))
+      ->setAsInline(true);
+
+    if ($blocking) {
+      $warnings[] = pht(
+        'This draft revision will not be submitted for review because %s '.
+        'build(s) failed: %s.',
+        phutil_count($blocking),
+        $blocking_list);
+      $warnings[] = pht(
+        'Fix build failures and update the revision.');
+    } else if ($waiting) {
+      $warnings[] = pht(
+        'This draft revision will be sent for review once %s '.
+        'build(s) pass: %s.',
+        phutil_count($waiting),
+        $waiting_list);
+    } else {
+      $warnings[] = pht(
+        'This is a draft revision that has not yet been submitted for '.
+        'review.');
+    }
+
+    return $warnings;
+  }
+
+}

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -1570,58 +1570,12 @@ final class DifferentialTransactionEditor
 
   private function hasActiveBuilds($object) {
     $viewer = $this->requireActor();
-    $diff = $object->getActiveDiff();
 
-    $buildables = id(new HarbormasterBuildableQuery())
+    $builds = $object->loadActiveBuilds($viewer);
-      ->setViewer($viewer)
-      ->withContainerPHIDs(array($object->getPHID()))
-      ->withBuildablePHIDs(array($diff->getPHID()))
-      ->withManualBuildables(false)
-      ->execute();
-    if (!$buildables) {
-      return false;
-    }
-
-    $builds = id(new HarbormasterBuildQuery())
-      ->setViewer($viewer)
-      ->withBuildablePHIDs(mpull($buildables, 'getPHID'))
-      ->withBuildStatuses(
-        array(
-          HarbormasterBuildStatus::STATUS_INACTIVE,
-          HarbormasterBuildStatus::STATUS_PENDING,
-          HarbormasterBuildStatus::STATUS_BUILDING,
-          HarbormasterBuildStatus::STATUS_FAILED,
-          HarbormasterBuildStatus::STATUS_ABORTED,
-          HarbormasterBuildStatus::STATUS_ERROR,
-          HarbormasterBuildStatus::STATUS_PAUSED,
-          HarbormasterBuildStatus::STATUS_DEADLOCKED,
-        ))
-      ->needBuildTargets(true)
-      ->execute();
     if (!$builds) {
       return false;
     }
 
-    $active = array();
-    foreach ($builds as $key => $build) {
-      foreach ($build->getBuildTargets() as $target) {
-        if ($target->isAutotarget()) {
-          // Ignore autotargets when looking for active of failed builds. If
-          // local tests fail and you continue anyway, you don't need to
-          // double-confirm them.
-          continue;
-        }
-
-        // This build has at least one real target that's doing something.
-        $active[$key] = $build;
-        break;
-      }
-    }
-
-    if (!$active) {
-      return false;
-    }
-
     return true;
   }
 

src/applications/differential/storage/DifferentialRevision.php

@@ -708,6 +708,58 @@ final class DifferentialRevision extends DifferentialDAO
     return false;
   }
 
+  public function loadActiveBuilds(PhabricatorUser $viewer) {
+    $diff = $this->getActiveDiff();
+
+    $buildables = id(new HarbormasterBuildableQuery())
+      ->setViewer($viewer)
+      ->withContainerPHIDs(array($this->getPHID()))
+      ->withBuildablePHIDs(array($diff->getPHID()))
+      ->withManualBuildables(false)
+      ->execute();
+    if (!$buildables) {
+      return array();
+    }
+
+    $builds = id(new HarbormasterBuildQuery())
+      ->setViewer($viewer)
+      ->withBuildablePHIDs(mpull($buildables, 'getPHID'))
+      ->withBuildStatuses(
+        array(
+          HarbormasterBuildStatus::STATUS_INACTIVE,
+          HarbormasterBuildStatus::STATUS_PENDING,
+          HarbormasterBuildStatus::STATUS_BUILDING,
+          HarbormasterBuildStatus::STATUS_FAILED,
+          HarbormasterBuildStatus::STATUS_ABORTED,
+          HarbormasterBuildStatus::STATUS_ERROR,
+          HarbormasterBuildStatus::STATUS_PAUSED,
+          HarbormasterBuildStatus::STATUS_DEADLOCKED,
+        ))
+      ->needBuildTargets(true)
+      ->execute();
+    if (!$builds) {
+      return array();
+    }
+
+    $active = array();
+    foreach ($builds as $key => $build) {
+      foreach ($build->getBuildTargets() as $target) {
+        if ($target->isAutotarget()) {
+          // Ignore autotargets when looking for active of failed builds. If
+          // local tests fail and you continue anyway, you don't need to
+          // double-confirm them.
+          continue;
+        }
+
+        // This build has at least one real target that's doing something.
+        $active[$key] = $build;
+        break;
+      }
+    }
+
+    return $active;
+  }
+
 
 /* -(  HarbormasterBuildableInterface  )------------------------------------- */