From bf5437212cc85bf2079c4bcfd796af26a0bba6ae Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 16 May 2016 11:53:29 -0700
Subject: [PATCH] When a revision is accepted but has open dependencies, show a
 note in the list UI

Summary:
Ref T10939. I don't think this is hugely important, but it doesn't clutter things up much and it's nice as a hint.

T4055 was the original request specifically asking for this. It wanted a separate bucket, but I think this use case isn't common/strong enough to justify that.

I would like to improve Differential's "X depends on Y" feature in the long term. We don't tend to use/need it much, but it could easily do a better and more automatic job of supporting review of a group of revisions.

Test Plan: {F1426636}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10939

Differential Revision: https://secure.phabricator.com/D15930
---
 .../DifferentialRevisionSearchEngine.php      | 55 +++++++++++++++++++
 .../view/DifferentialRevisionListView.php     | 20 +++++++
 2 files changed, 75 insertions(+)

diff --git a/src/applications/differential/query/DifferentialRevisionSearchEngine.php b/src/applications/differential/query/DifferentialRevisionSearchEngine.php
index 8263b91dbb..5cf6f82198 100644
--- a/src/applications/differential/query/DifferentialRevisionSearchEngine.php
+++ b/src/applications/differential/query/DifferentialRevisionSearchEngine.php
@@ -153,6 +153,8 @@ final class DifferentialRevisionSearchEngine
 
     $bucket = $this->getResultBucket($query);
 
+    $unlanded = $this->loadUnlandedDependencies($revisions);
+
     $views = array();
     if ($bucket) {
       $bucket->setViewer($viewer);
@@ -187,6 +189,7 @@ final class DifferentialRevisionSearchEngine
 
     foreach ($views as $view) {
       $view->setHandles($handles);
+      $view->setUnlandedDependencies($unlanded);
     }
 
     if (count($views) == 1) {
@@ -223,4 +226,56 @@ final class DifferentialRevisionSearchEngine
       return $view;
   }
 
+  private function loadUnlandedDependencies(array $revisions) {
+    $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED;
+
+    $phids = array();
+    foreach ($revisions as $revision) {
+      if ($revision->getStatus() != $status_accepted) {
+        continue;
+      }
+
+      $phids[] = $revision->getPHID();
+    }
+
+    if (!$phids) {
+      return array();
+    }
+
+    $query = id(new PhabricatorEdgeQuery())
+      ->withSourcePHIDs($phids)
+      ->withEdgeTypes(
+        array(
+          DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST,
+        ));
+
+    $query->execute();
+
+    $revision_phids = $query->getDestinationPHIDs();
+    if (!$revision_phids) {
+      return array();
+    }
+
+    $viewer = $this->requireViewer();
+
+    $blocking_revisions = id(new DifferentialRevisionQuery())
+      ->setViewer($viewer)
+      ->withPHIDs($revision_phids)
+      ->withStatus(DifferentialRevisionQuery::STATUS_OPEN)
+      ->execute();
+    $blocking_revisions = mpull($blocking_revisions, null, 'getPHID');
+
+    $result = array();
+    foreach ($revisions as $revision) {
+      $revision_phid = $revision->getPHID();
+      $blocking_phids = $query->getDestinationPHIDs(array($revision_phid));
+      $blocking = array_select_keys($blocking_revisions, $blocking_phids);
+      if ($blocking) {
+        $result[$revision_phid] = $blocking;
+      }
+    }
+
+    return $result;
+  }
+
 }
diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php
index 826536355e..6451bd9d2a 100644
--- a/src/applications/differential/view/DifferentialRevisionListView.php
+++ b/src/applications/differential/view/DifferentialRevisionListView.php
@@ -12,6 +12,16 @@ final class DifferentialRevisionListView extends AphrontView {
   private $noDataString;
   private $noBox;
   private $background = null;
+  private $unlandedDependencies = array();
+
+  public function setUnlandedDependencies(array $unlanded_dependencies) {
+    $this->unlandedDependencies = $unlanded_dependencies;
+    return $this;
+  }
+
+  public function getUnlandedDependencies() {
+    return $this->unlandedDependencies;
+  }
 
   public function setNoDataString($no_data_string) {
     $this->noDataString = $no_data_string;
@@ -121,6 +131,16 @@ final class DifferentialRevisionListView extends AphrontView {
       $author_handle = $this->handles[$revision->getAuthorPHID()];
       $item->addByline(pht('Author: %s', $author_handle->renderLink()));
 
+      $unlanded = idx($this->unlandedDependencies, $phid);
+      if ($unlanded) {
+        $item->addAttribute(
+          array(
+            id(new PHUIIconView())->setIcon('fa-chain-broken', 'red'),
+            ' ',
+            pht('Open Dependencies'),
+          ));
+      }
+
       $reviewers = array();
       // TODO: As above, this should be based on `getReviewerStatus()`.
       foreach ($revision->getReviewers() as $reviewer) {