Replace "DiffusionCommitListView" with "DiffusionCommitGraphView"

Summary:
Ref T13552. This older view mostly duplicates other code and has only two callsites:

  - The "Commits" section of user profile pages.
  - The "Ambiguous Hash" page when you visit a commit hash page which is an ambiguous prefix of two or more commit hashes.

Replace both with "DiffusionCommitGraphView".

Test Plan:
  - Visited profile page, clicked "Commits".
  - Visited an ambiguous hash page (`rPbd3c23`).

Maniphest Tasks: T13552

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

resources/celerity/map.php

@@ -73,7 +73,7 @@ return array(
     'rsrc/css/application/diffusion/diffusion-icons.css' => '23b31a1b',
     'rsrc/css/application/diffusion/diffusion-readme.css' => 'b68a76e4',
     'rsrc/css/application/diffusion/diffusion-repository.css' => 'b89e8c6c',
-    'rsrc/css/application/diffusion/diffusion.css' => 'a54bb336',
+    'rsrc/css/application/diffusion/diffusion.css' => 'db8bbf58',
     'rsrc/css/application/feed/feed.css' => 'd8b6e3f8',
     'rsrc/css/application/files/global-drag-and-drop.css' => '1d2713a4',
     'rsrc/css/application/flag/flag.css' => '2b77be8d',
@@ -567,7 +567,7 @@ return array(
     'differential-revision-history-css' => '8aa3eac5',
     'differential-revision-list-css' => '93d2df7d',
     'differential-table-of-contents-css' => 'bba788b9',
-    'diffusion-css' => 'a54bb336',
+    'diffusion-css' => 'db8bbf58',
     'diffusion-icons-css' => '23b31a1b',
     'diffusion-readme-css' => 'b68a76e4',
     'diffusion-repository-css' => 'b89e8c6c',

src/__phutil_library_map__.php

@@ -783,7 +783,6 @@ phutil_register_library_map(array(
     'DiffusionCommitHookEngine' => 'applications/diffusion/engine/DiffusionCommitHookEngine.php',
     'DiffusionCommitHookRejectException' => 'applications/diffusion/exception/DiffusionCommitHookRejectException.php',
     'DiffusionCommitListController' => 'applications/diffusion/controller/DiffusionCommitListController.php',
-    'DiffusionCommitListView' => 'applications/diffusion/view/DiffusionCommitListView.php',
     'DiffusionCommitMergeHeraldField' => 'applications/diffusion/herald/DiffusionCommitMergeHeraldField.php',
     'DiffusionCommitMessageHeraldField' => 'applications/diffusion/herald/DiffusionCommitMessageHeraldField.php',
     'DiffusionCommitPackageAuditHeraldField' => 'applications/diffusion/herald/DiffusionCommitPackageAuditHeraldField.php',
@@ -6873,7 +6872,6 @@ phutil_register_library_map(array(
     'DiffusionCommitHookEngine' => 'Phobject',
     'DiffusionCommitHookRejectException' => 'Exception',
     'DiffusionCommitListController' => 'DiffusionController',
-    'DiffusionCommitListView' => 'AphrontView',
     'DiffusionCommitMergeHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitMessageHeraldField' => 'DiffusionCommitHeraldField',
     'DiffusionCommitPackageAuditHeraldField' => 'DiffusionCommitHeraldField',

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -90,10 +90,9 @@ final class DiffusionCommitController extends DiffusionController {
         ->setSeverity(PHUIInfoView::SEVERITY_WARNING)
         ->appendChild($warning_message);
 
-      $list = id(new DiffusionCommitListView())
+      $list = id(new DiffusionCommitGraphView())
         ->setViewer($viewer)
-        ->setCommits($commits)
+        ->setCommits($commits);
-        ->setNoDataString(pht('No recent commits.'));
 
       $crumbs->addTextCrumb(pht('Ambiguous Commit'));
 

src/applications/diffusion/controller/DiffusionHistoryController.php

@@ -54,13 +54,28 @@ final class DiffusionHistoryController extends DiffusionController {
 
     $history_list = id(new DiffusionCommitGraphView())
       ->setViewer($viewer)
-      ->setParents($history_results['parents'])
-      ->setIsHead(!$pager->getOffset())
-      ->setIsTail(!$pager->getHasMorePages())
       ->setDiffusionRequest($drequest)
       ->setHistory($history)
       ->setCommits($commits);
 
+    // NOTE: If we have a path (like "src/"), many nodes in the graph are
+    // likely to be missing (since the path wasn't touched by those commits).
+
+    // If we draw the graph, commits will often appear to be unrelated because
+    // intermediate nodes are omitted. Just drop the graph.
+
+    // The ideal behavior would be to load the entire graph and then connect
+    // ancestors appropriately, but this would currrently be prohibitively
+    // expensive in the general case.
+
+    $show_graph = !strlen($drequest->getPath());
+    if ($show_graph) {
+      $history_list
+        ->setParents($history_results['parents'])
+        ->setIsHead(!$pager->getOffset())
+        ->setIsTail(!$pager->getHasMorePages());
+    }
+
     $header = $this->buildHeader($drequest);
 
     $crumbs = $this->buildCrumbs(

src/applications/diffusion/view/DiffusionCommitGraphView.php

@@ -332,14 +332,21 @@ final class DiffusionCommitGraphView
   }
 
   private function newBrowseButton($hash) {
-    $commit = $this->getCommit($hash);
+    $repository = $this->getRepository();
 
-    return $this->linkBrowse(
+    if ($repository) {
-      '/',
+      $drequest = $this->getDiffusionRequest();
-      array(
+
-        'commit' => $hash,
+      return $this->linkBrowse(
-      ),
+        $drequest->getPath(),
-      $as_button = true);
+        array(
+          'commit' => $hash,
+          'branch' => $drequest->getBranch(),
+        ),
+        $as_button = true);
+    }
+
+    return null;
   }
 
   private function getCommit($hash) {

src/applications/diffusion/view/DiffusionCommitListView.php

@@ -1,177 +0,0 @@
-<?php
-
-final class DiffusionCommitListView extends AphrontView {
-
-  private $commits = array();
-  private $noDataString;
-
-  public function setNoDataString($no_data_string) {
-    $this->noDataString = $no_data_string;
-    return $this;
-  }
-
-  public function setHeader($header) {
-    $this->header = $header;
-    return $this;
-  }
-
-  public function setCommits(array $commits) {
-    assert_instances_of($commits, 'PhabricatorRepositoryCommit');
-    $this->commits = mpull($commits, null, 'getPHID');
-    return $this;
-  }
-
-  public function getCommits() {
-    return $this->commits;
-  }
-
-  public function setHandles(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $this->handles = $handles;
-    return $this;
-  }
-
-  private function getRequiredHandlePHIDs() {
-    $phids = array();
-    foreach ($this->history as $item) {
-      $data = $item->getCommitData();
-      if ($data) {
-        if ($data->getCommitDetail('authorPHID')) {
-          $phids[$data->getCommitDetail('authorPHID')] = true;
-        }
-        if ($data->getCommitDetail('committerPHID')) {
-          $phids[$data->getCommitDetail('committerPHID')] = true;
-        }
-      }
-    }
-    return array_keys($phids);
-  }
-
-  private function getCommitDescription($phid) {
-    if ($this->commits === null) {
-      return pht('(Unknown Commit)');
-    }
-
-    $commit = idx($this->commits, $phid);
-    if (!$commit) {
-      return pht('(Unknown Commit)');
-    }
-
-    $summary = $commit->getCommitData()->getSummary();
-    if (strlen($summary)) {
-      return $summary;
-    }
-
-    // No summary, so either this is still importing or just has an empty
-    // commit message.
-
-    if (!$commit->isImported()) {
-      return pht('(Importing Commit...)');
-    } else {
-      return pht('(Untitled Commit)');
-    }
-  }
-
-  public function render() {
-    require_celerity_resource('diffusion-css');
-    return $this->buildList();
-  }
-
-  public function buildList() {
-    $viewer = $this->getViewer();
-    $rowc = array();
-
-    $phids = array();
-    foreach ($this->getCommits() as $commit) {
-      $phids[] = $commit->getPHID();
-
-      $author_phid = $commit->getAuthorPHID();
-      if ($author_phid) {
-        $phids[] = $author_phid;
-      }
-    }
-
-    $handles = $viewer->loadHandles($phids);
-
-    $cur_date = 0;
-    $view = array();
-    foreach ($this->commits as $commit) {
-      $new_date = phabricator_date($commit->getEpoch(), $viewer);
-      if ($cur_date !== $new_date) {
-        $date = ucfirst(
-          phabricator_relative_date($commit->getEpoch(), $viewer));
-        $header = id(new PHUIHeaderView())
-          ->setHeader($date);
-        $list = id(new PHUIObjectItemListView())
-          ->setFlush(true)
-          ->addClass('diffusion-history-list');
-
-        $view[] = id(new PHUIObjectBoxView())
-          ->setHeader($header)
-          ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
-          ->setObjectList($list);
-      }
-
-      $commit_phid = $commit->getPHID();
-      $commit_handle = $handles[$commit_phid];
-      $committed = null;
-
-      $commit_name = $commit_handle->getName();
-      $commit_link = $commit_handle->getURI();
-      $commit_desc = $this->getCommitDescription($commit_phid);
-      $committed = phabricator_datetime($commit->getEpoch(), $viewer);
-
-      $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine();
-      $engine->setConfig('viewer', $viewer);
-      $commit_data = $commit->getCommitData();
-      $message = $commit_data->getCommitMessage();
-      $message = $engine->markupText($message);
-      $message = phutil_tag_div(
-        'diffusion-history-message phabricator-remarkup', $message);
-
-      $author_phid = $commit->getAuthorPHID();
-      if ($author_phid) {
-        $author_name = $handles[$author_phid]->renderLink();
-        $author_image_uri = $handles[$author_phid]->getImageURI();
-      } else {
-        $author_name = $commit->getCommitData()->getAuthorName();
-        $author_image_uri =
-          celerity_get_resource_uri('/rsrc/image/people/user0.png');
-      }
-
-      $commit_tag = id(new PHUITagView())
-        ->setName($commit_name)
-        ->setType(PHUITagView::TYPE_SHADE)
-        ->setColor(PHUITagView::COLOR_INDIGO)
-        ->setBorder(PHUITagView::BORDER_NONE)
-        ->setSlimShady(true);
-
-      $item = id(new PHUIObjectItemView())
-        ->setHeader($commit_desc)
-        ->setHref($commit_link)
-        ->setDisabled($commit->isUnreachable())
-        ->setDescription($message)
-        ->setImageURI($author_image_uri)
-        ->addByline(pht('Author: %s', $author_name))
-        ->addIcon('none', $committed)
-        ->addAttribute($commit_tag);
-
-      $list->addItem($item);
-      $cur_date = $new_date;
-    }
-
-    if (!$view) {
-      $list = id(new PHUIObjectItemListView())
-        ->setFlush(true)
-        ->setNoDataString($this->noDataString);
-
-      $view = id(new PHUIObjectBoxView())
-        ->setHeaderText(pht('Recent Commits'))
-        ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
-        ->setObjectList($list);
-    }
-
-    return $view;
-  }
-
-}

src/applications/people/controller/PhabricatorPeopleProfileCommitsController.php

@@ -58,13 +58,13 @@ final class PhabricatorPeopleProfileCommitsController
       ->setViewer($viewer)
       ->withAuthorPHIDs(array($user->getPHID()))
       ->needCommitData(true)
+      ->needIdentities(true)
       ->setLimit(100)
       ->execute();
 
-    $list = id(new DiffusionCommitListView())
+    $list = id(new DiffusionCommitGraphView())
       ->setViewer($viewer)
-      ->setCommits($commits)
+      ->setCommits($commits);
-      ->setNoDataString(pht('No recent commits.'));
 
     return $list;
   }

webroot/rsrc/css/application/diffusion/diffusion.css

@@ -93,14 +93,6 @@
   font-size: {$biggerfontsize};
 }
 
-.diffusion-history-message {
-  background-color: {$bluebackground};
-  padding: 16px;
-  margin: 4px 0;
-  border-radius: 5px;
-  color: {$darkbluetext};
-}
-
 .diffusion-history-list .phui-oi-attribute {
   font-size: {$smallerfontsize};
   letter-spacing: 0.01em;