Separate revision list rendering logic into a RevisionListView

Summary: I want to throw this in Diffusion as part of T262, but it's embedded in
the controller right now. Split it out.

Test Plan: Looked at various revision list views, no changes.

Reviewers: jungejason, nh, tuomaspelkonen, aran

Reviewed By: jungejason

CC: aran, jungejason

Differential Revision: 977

src/__phutil_library_map__.php

@@ -221,6 +221,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionIDFieldSpecification' => 'applications/differential/field/specification/revisionid',
     'DifferentialRevisionListController' => 'applications/differential/controller/revisionlist',
     'DifferentialRevisionListData' => 'applications/differential/data/revisionlist',
+    'DifferentialRevisionListView' => 'applications/differential/view/revisionlist',
     'DifferentialRevisionStatus' => 'applications/differential/constants/revisionstatus',
     'DifferentialRevisionStatusFieldSpecification' => 'applications/differential/field/specification/revisionstatus',
     'DifferentialRevisionUpdateHistoryView' => 'applications/differential/view/revisionupdatehistory',
@@ -914,6 +915,7 @@ phutil_register_library_map(array(
     'DifferentialRevisionEditController' => 'DifferentialController',
     'DifferentialRevisionIDFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialRevisionListController' => 'DifferentialController',
+    'DifferentialRevisionListView' => 'AphrontView',
     'DifferentialRevisionStatusFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialRevisionUpdateHistoryView' => 'AphrontView',
     'DifferentialRevisionViewController' => 'DifferentialController',

src/applications/differential/controller/revisionlist/DifferentialRevisionListController.php

@@ -172,38 +172,42 @@ class DifferentialRevisionListController extends DifferentialController {
       $queries[$key]['revisions'] = $revisions;
     }
 
-    $rev = new DifferentialRevision();
     if ($rev_ids) {
-      $rev_ids = array_keys($rev_ids);
+      $rev = new DifferentialRevision();
-      $reviewers = queryfx_all(
+      $relationships = queryfx_all(
         $rev->establishConnection('r'),
-        'SELECT revisionID, objectPHID FROM %T revision JOIN %T relationship
+        'SELECT * FROM %T WHERE revisionID IN (%Ld) ORDER BY sequence',
-          ON revision.id = relationship.revisionID
-          WHERE revision.id IN (%Ld)
-            AND relationship.relation = %s
-          ORDER BY sequence',
-        $rev->getTableName(),
         DifferentialRevision::RELATIONSHIP_TABLE,
-        $rev_ids,
+        array_keys($rev_ids));
-        DifferentialRevision::RELATION_REVIEWER);
+      $relationships = igroup($relationships, 'revisionID');
-
-      $reviewer_map = array();
-      foreach ($reviewers as $reviewer) {
-        $reviewer_map[$reviewer['revisionID']][] = $reviewer['objectPHID'];
-      }
-      foreach ($reviewer_map as $revision_id => $reviewer_ids) {
-        $phids[reset($reviewer_ids)] = true;
-      }
     } else {
-      $reviewer_map = array();
+      $relationships = array();
     }
 
-    if ($phids) {
+    foreach ($queries as $query) {
-      $phids = array_keys($phids);
+      foreach ($query['revisions'] as $revision) {
-      $handles = id(new PhabricatorObjectHandleData($phids))
+        $revision->attachRelationships(
-        ->loadHandles();
+          idx(
-    } else {
+            $relationships,
-      $handles = array();
+            $revision->getID(),
+            array()));
+      }
+    }
+
+    $phids = array();
+    foreach ($queries as $key => $query) {
+      $view = id(new DifferentialRevisionListView())
+        ->setRevisions($query['revisions'])
+        ->setUser($user)
+        ->setNoDataString(idx($query, 'nodata'));
+      $phids[] = $view->getRequiredHandlePHIDs();
+      $queries[$key]['view'] = $view;
+    }
+    $phids = array_mergev($phids);
+    $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles();
+
+    foreach ($queries as $query) {
+      $query['view']->setHandles($handles);
     }
 
     if (empty($filters[$this->filter]['nofilter'])) {
@@ -226,13 +230,13 @@ class DifferentialRevisionListController extends DifferentialController {
     }
 
     foreach ($queries as $query) {
-      $table = $this->renderRevisionTable(
+      $table = $query['view']->render();
-        $query['revisions'],
+
-        $query['header'],
+      $panel = new AphrontPanelView();
-        idx($query, 'nodata'),
+      $panel->setHeader($query['header']);
-        $handles,
+      $panel->appendChild($table);
-        $reviewer_map);
+
-      $side_nav->appendChild($table);
+      $side_nav->appendChild($panel);
     }
 
     return $this->buildStandardPageResponse(
@@ -243,81 +247,4 @@ class DifferentialRevisionListController extends DifferentialController {
       ));
   }
 
-  private function renderRevisionTable(
-    array $revisions,
-    $header,
-    $nodata,
-    array $handles,
-    array $reviewer_map) {
-
-    $rows = array();
-    foreach ($revisions as $revision) {
-      $status = DifferentialRevisionStatus::getNameForRevisionStatus(
-        $revision->getStatus());
-
-      $reviewers = idx($reviewer_map, $revision->getID(), array());
-      if ($reviewers) {
-        $first = reset($reviewers);
-        if (count($reviewers) > 1) {
-          $suffix = ' (+'.(count($reviewers) - 1).')';
-        } else {
-          $suffix = null;
-        }
-        $reviewers = $handles[$first]->renderLink().$suffix;
-      } else {
-        $reviewers = '<em>None</em>';
-      }
-
-      $rows[] = array(
-        'D'.$revision->getID(),
-        '<strong>'.phutil_render_tag(
-          'a',
-          array(
-            'href' => '/D'.$revision->getID(),
-          ),
-          phutil_escape_html($revision->getTitle())).'</strong>',
-        phutil_escape_html($status),
-        number_format($revision->getLineCount()),
-        $handles[$revision->getAuthorPHID()]->renderLink(),
-        $reviewers,
-        phabricator_format_timestamp($revision->getDateModified()),
-        phabricator_format_timestamp($revision->getDateCreated()),
-      );
-    }
-
-    $table = new AphrontTableView($rows);
-    $table->setHeaders(
-      array(
-        'ID',
-        'Revision',
-        'Status',
-        'Lines',
-        'Author',
-        'Reviewers',
-        'Updated',
-        'Created',
-      ));
-    $table->setColumnClasses(
-      array(
-        null,
-        'wide',
-        null,
-        null,
-        null,
-        null,
-        null,
-        null,
-      ));
-    if ($nodata !== null) {
-      $table->setNoDataString($nodata);
-    }
-
-
-    $panel = new AphrontPanelView();
-    $panel->setHeader($header);
-    $panel->appendChild($table);
-
-    return $panel;
-  }
-
 }

src/applications/differential/controller/revisionlist/__init__.php

@@ -7,19 +7,17 @@
 
 
 phutil_require_module('phabricator', 'aphront/response/redirect');
-phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus');
 phutil_require_module('phabricator', 'applications/differential/controller/base');
 phutil_require_module('phabricator', 'applications/differential/data/revisionlist');
 phutil_require_module('phabricator', 'applications/differential/storage/revision');
+phutil_require_module('phabricator', 'applications/differential/view/revisionlist');
 phutil_require_module('phabricator', 'applications/phid/handle/data');
 phutil_require_module('phabricator', 'storage/queryfx');
-phutil_require_module('phabricator', 'view/control/table');
 phutil_require_module('phabricator', 'view/form/base');
 phutil_require_module('phabricator', 'view/form/control/tokenizer');
 phutil_require_module('phabricator', 'view/layout/listfilter');
 phutil_require_module('phabricator', 'view/layout/panel');
 phutil_require_module('phabricator', 'view/layout/sidenav');
-phutil_require_module('phabricator', 'view/utils');
 
 phutil_require_module('phutil', 'markup');
 phutil_require_module('phutil', 'utils');

src/applications/differential/storage/revision/DifferentialRevision.php

@@ -134,8 +134,11 @@ class DifferentialRevision extends DifferentialDAO {
       self::RELATIONSHIP_TABLE,
       $this->getID());
 
-    $this->relationships = igroup($data, 'relation');
+    return $this->attachRelationships($data);
+  }
 
+  public function attachRelationships(array $relationships) {
+    $this->relationships = igroup($relationships, 'relation');
     return $this;
   }
 

src/applications/differential/view/revisionlist/DifferentialRevisionListView.php

@@ -0,0 +1,136 @@
+<?php
+
+/*
+ * Copyright 2011 Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Render a table of Differential revisions.
+ */
+final class DifferentialRevisionListView extends AphrontView {
+
+  private $revisions;
+  private $handles;
+  private $user;
+  private $noDataString;
+
+  public function setRevisions(array $revisions) {
+    $this->revisions = $revisions;
+    return $this;
+  }
+
+  public function getRequiredHandlePHIDs() {
+    $phids = array();
+    foreach ($this->revisions as $revision) {
+      $phids[] = $revision->getAuthorPHID();
+      $reviewers = $revision->getReviewers();
+      if ($reviewers) {
+        $phids[] = head($reviewers);
+      }
+    }
+    return $phids;
+  }
+
+  public function setHandles(array $handles) {
+    $this->handles = $handles;
+    return $this;
+  }
+
+  public function setUser(PhabricatorUser $user) {
+    $this->user = $user;
+    return $this;
+  }
+
+  public function setNoDataString($no_data_string) {
+    $this->noDataString = $no_data_string;
+    return $this;
+  }
+
+  public function render() {
+
+    $user = $this->user;
+    if (!$user) {
+      throw new Exception("Call setUser() before render()!");
+    }
+
+    $rows = array();
+    foreach ($this->revisions as $revision) {
+      $status = $revision->getStatus();
+      $status = DifferentialRevisionStatus::getNameForRevisionStatus($status);
+
+      $reviewer_phids = $revision->getReviewers();
+      if ($reviewer_phids) {
+        $first = reset($reviewer_phids);
+        if (count($reviewer_phids) > 1) {
+          $suffix = ' (+'.(count($reviewer_phids) - 1).')';
+        } else {
+          $suffix = null;
+        }
+        $reviewers = $this->handles[$first]->renderLink().$suffix;
+      } else {
+        $reviewers = '<em>None</em>';
+      }
+
+      $link = phutil_render_tag(
+        'a',
+        array(
+          'href' => '/D'.$revision->getID(),
+        ),
+        phutil_escape_html($revision->getTitle()));
+
+      $rows[] = array(
+        'D'.$revision->getID(),
+        $link,
+        phutil_escape_html($status),
+        number_format($revision->getLineCount()),
+        $this->handles[$revision->getAuthorPHID()]->renderLink(),
+        $reviewers,
+        phabricator_datetime($revision->getDateModified(), $user),
+        phabricator_date($revision->getDateCreated(), $user),
+      );
+    }
+
+    $table = new AphrontTableView($rows);
+    $table->setHeaders(
+      array(
+        'ID',
+        'Revision',
+        'Status',
+        'Lines',
+        'Author',
+        'Reviewers',
+        'Updated',
+        'Created',
+      ));
+    $table->setColumnClasses(
+      array(
+        null,
+        'wide pri',
+        null,
+        'n',
+        null,
+        null,
+        'right',
+        'right',
+      ));
+
+    if ($this->noDataString) {
+      $table->setNoDataString($this->noDataString);
+    }
+
+    return $table->render();
+  }
+
+}

src/applications/differential/view/revisionlist/__init__.php

@@ -0,0 +1,18 @@
+<?php
+/**
+ * This file is automatically generated. Lint this module to rebuild it.
+ * @generated
+ */
+
+
+
+phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus');
+phutil_require_module('phabricator', 'view/base');
+phutil_require_module('phabricator', 'view/control/table');
+phutil_require_module('phabricator', 'view/utils');
+
+phutil_require_module('phutil', 'markup');
+phutil_require_module('phutil', 'utils');
+
+
+phutil_require_source('DifferentialRevisionListView.php');