When a revision has more than 7 reviewers, render only the first 7 in the list view

Summary:
See PHI489. Ref T13110. At least for now, this just shows "..." at the end since you can click the revision to see the whole list anyway.

Also remove the older-style external Handle passing in favor of lazy construction via HandlePool.

Test Plan: Viewed revisions, fiddled with the 7 limit, got sensible-seeming "..." behavior.

Maniphest Tasks: T13110

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

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -869,17 +869,11 @@ final class DifferentialRevisionViewController extends DifferentialController {
     $header = id(new PHUIHeaderView())
       ->setHeader(pht('Recent Similar Revisions'));
 
-    $view = id(new DifferentialRevisionListView())
+    return id(new DifferentialRevisionListView())
+      ->setViewer($viewer)
       ->setRevisions($revisions)
       ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
-      ->setNoBox(true)
+      ->setNoBox(true);
-      ->setUser($viewer);
-
-    $phids = $view->getRequiredHandlePHIDs();
-    $handles = $this->loadViewerHandles($phids);
-    $view->setHandles($handles);
-
-    return $view;
   }
 
 

src/applications/differential/query/DifferentialRevisionSearchEngine.php

@@ -148,7 +148,7 @@ final class DifferentialRevisionSearchEngine
 
     $viewer = $this->requireViewer();
     $template = id(new DifferentialRevisionListView())
-      ->setUser($viewer)
+      ->setViewer($viewer)
       ->setNoBox($this->isPanelContext());
 
     $bucket = $this->getResultBucket($query);
@@ -176,28 +176,16 @@ final class DifferentialRevisionSearchEngine
       }
     } else {
       $views[] = id(clone $template)
-        ->setRevisions($revisions)
+        ->setRevisions($revisions);
-        ->setHandles(array());
     }
 
     if (!$views) {
       $views[] = id(new DifferentialRevisionListView())
-        ->setUser($viewer)
+        ->setViewer($viewer)
         ->setNoDataString(pht('No revisions found.'));
     }
 
-    $phids = array_mergev(mpull($views, 'getRequiredHandlePHIDs'));
-    if ($phids) {
-      $handles = id(new PhabricatorHandleQuery())
-        ->setViewer($viewer)
-        ->withPHIDs($phids)
-        ->execute();
-    } else {
-      $handles = array();
-    }
-
     foreach ($views as $view) {
-      $view->setHandles($handles);
       $view->setUnlandedDependencies($unlanded);
     }
 

src/applications/differential/view/DifferentialRevisionListView.php

@@ -6,7 +6,6 @@
 final class DifferentialRevisionListView extends AphrontView {
 
   private $revisions = array();
-  private $handles;
   private $header;
   private $noDataString;
   private $noBox;
@@ -48,32 +47,43 @@ final class DifferentialRevisionListView extends AphrontView {
     return $this;
   }
 
-  public function getRequiredHandlePHIDs() {
-    $phids = array();
-    foreach ($this->revisions as $revision) {
-      $phids[] = array($revision->getAuthorPHID());
-      $phids[] = $revision->getReviewerPHIDs();
-    }
-    return array_mergev($phids);
-  }
-
-  public function setHandles(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $this->handles = $handles;
-    return $this;
-  }
-
   public function render() {
     $viewer = $this->getViewer();
 
     $this->initBehavior('phabricator-tooltips', array());
     $this->requireResource('aphront-tooltip-css');
 
-    $list = new PHUIObjectItemListView();
+    $reviewer_limit = 7;
 
-    foreach ($this->revisions as $revision) {
+    $reviewer_phids = array();
+    $reviewer_more = array();
+    $handle_phids = array();
+    foreach ($this->revisions as $key => $revision) {
+      $reviewers = $revision->getReviewers();
+      if (count($reviewers) > $reviewer_limit) {
+        $reviewers = array_slice($reviewers, 0, $reviewer_limit);
+        $reviewer_more[$key] = true;
+      } else {
+        $reviewer_more[$key] = false;
+      }
+
+      $phids = mpull($reviewers, 'getReviewerPHID');
+
+      $reviewer_phids[$key] = $phids;
+      foreach ($phids as $phid) {
+        $handle_phids[$phid] = $phid;
+      }
+
+      $author_phid = $revision->getAuthorPHID();
+      $handle_phids[$author_phid] = $author_phid;
+    }
+
+    $handles = $viewer->loadHandles($handle_phids);
+
+    $list = new PHUIObjectItemListView();
+    foreach ($this->revisions as $key => $revision) {
       $item = id(new PHUIObjectItemView())
-        ->setUser($viewer);
+        ->setViewer($viewer);
 
       $icons = array();
 
@@ -89,21 +99,17 @@ final class DifferentialRevisionListView extends AphrontView {
           '');
       }
 
-      if ($revision->getHasDraft($viewer)) {
-        $icons['draft'] = true;
-      }
-
       $modified = $revision->getDateModified();
 
       if (isset($icons['flag'])) {
         $item->addHeadIcon($icons['flag']);
       }
 
-      $item->setObjectName('D'.$revision->getID());
+      $item->setObjectName($revision->getMonogram());
       $item->setHeader($revision->getTitle());
-      $item->setHref('/D'.$revision->getID());
+      $item->setHref($revision->getURI());
 
-      if (isset($icons['draft'])) {
+      if ($revision->getHasDraft($viewer)) {
         $draft = id(new PHUIIconView())
           ->setIcon('fa-comment yellow')
           ->addSigil('has-tooltip')
@@ -114,8 +120,7 @@ final class DifferentialRevisionListView extends AphrontView {
         $item->addAttribute($draft);
       }
 
-      // Author
+      $author_handle = $handles[$revision->getAuthorPHID()];
-      $author_handle = $this->handles[$revision->getAuthorPHID()];
       $item->addByline(pht('Author: %s', $author_handle->renderLink()));
 
       $unlanded = idx($this->unlandedDependencies, $phid);
@@ -128,17 +133,26 @@ final class DifferentialRevisionListView extends AphrontView {
           ));
       }
 
-      $reviewers = array();
+      $more = null;
-      foreach ($revision->getReviewerPHIDs() as $reviewer) {
+      if ($reviewer_more[$key]) {
-        $reviewers[] = $this->handles[$reviewer]->renderLink();
+        $more = pht(', ...');
-      }
-      if (!$reviewers) {
-        $reviewers = phutil_tag('em', array(), pht('None'));
       } else {
-        $reviewers = phutil_implode_html(', ', $reviewers);
+        $more = null;
+      }
+
+      if ($reviewer_phids[$key]) {
+        $item->addAttribute(
+          array(
+            pht('Reviewers:'),
+            ' ',
+            $viewer->renderHandleList($reviewer_phids[$key])
+              ->setAsInline(true),
+            $more,
+          ));
+      } else {
+        $item->addAttribute(phutil_tag('em', array(), pht('No Reviewers')));
       }
 
-      $item->addAttribute(pht('Reviewers: %s', $reviewers));
       $item->setEpoch($revision->getDateModified());
 
       if ($revision->isClosed()) {

src/applications/diffusion/controller/DiffusionBrowseController.php

@@ -1754,14 +1754,10 @@ final class DiffusionBrowseController extends DiffusionController {
       ->setHeader(pht('Recently Open Revisions'));
 
     $list = id(new DifferentialRevisionListView())
+      ->setViewer($viewer)
       ->setRevisions($revisions)
-      ->setUser($viewer)
       ->setNoBox(true);
 
-    $phids = $list->getRequiredHandlePHIDs();
-    $handles = $this->loadViewerHandles($phids);
-    $list->setHandles($handles);
-
     $view = id(new PHUIObjectBoxView())
       ->setHeader($header)
       ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)

src/applications/people/controller/PhabricatorPeopleProfileRevisionsController.php

@@ -63,15 +63,11 @@ final class PhabricatorPeopleProfileRevisionsController
       ->execute();
 
     $list = id(new DifferentialRevisionListView())
-      ->setUser($viewer)
+      ->setViewer($viewer)
       ->setNoBox(true)
       ->setRevisions($revisions)
       ->setNoDataString(pht('No recent revisions.'));
 
-    $object_phids = $list->getRequiredHandlePHIDs();
-    $handles = $this->loadViewerHandles($object_phids);
-    $list->setHandles($handles);
-
     $view = id(new PHUIObjectBoxView())
       ->setHeaderText(pht('Recent Revisions'))
       ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)