Modernize "Author" and "Committer" rendering for commits

Summary:
Ref T13552. Give "Commit" objects a more modern, identity-aware way to render author and committer information.

This uses handles in a more modern way and gives us a single read callsite for raw author and committer names.

Test Plan:
  - Grepped for callers to the old methods, found none. (There are a lot of "renderAuthor()" callers in transactions, but this call takes no parameters.)
  - Viewed some commits, saw sensible lists of authors and committers.

Maniphest Tasks: T13552

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

src/applications/audit/view/PhabricatorAuditListView.php

@@ -90,11 +90,6 @@ final class PhabricatorAuditListView extends AphrontView {
       foreach ($commit->getAudits() as $audit) {
         $phids[] = $audit->getAuditorPHID();
       }
-
-      $author_phid = $commit->getAuthorPHID();
-      if ($author_phid) {
-        $phids[] = $author_phid;
-      }
     }
 
     $handles = $viewer->loadHandles($phids);
@@ -126,21 +121,18 @@ final class PhabricatorAuditListView extends AphrontView {
       $status_color = $status->getColor();
       $status_icon = $status->getIcon();
 
-      $author_phid = $commit->getAuthorPHID();
-      if ($author_phid) {
-        $author_name = $handles[$author_phid]->renderLink();
-      } else {
-        $author_name = $commit->getCommitData()->getAuthorName();
-      }
-
       $item = id(new PHUIObjectItemView())
         ->setObjectName($commit_name)
         ->setHeader($commit_desc)
         ->setHref($commit_link)
         ->setDisabled($commit->isUnreachable())
-        ->addByline(pht('Author: %s', $author_name))
         ->addIcon('none', $committed);
 
+      $author_name = $commit->newCommitAuthorView($viewer);
+      if ($author_name) {
+        $item->addByline(pht('Author: %s', $author_name));
+      }
+
       if ($show_drafts) {
         if ($commit->getHasDraft($viewer)) {
           $item->addAttribute($draft_icon);

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -625,33 +625,50 @@ final class DiffusionCommitController extends DiffusionController {
       }
     }
 
-    $author_epoch = $data->getCommitDetail('authorEpoch');
+    $provenance_list = new PHUIStatusListView();
 
-    $committed_info = id(new PHUIStatusItemView())
-      ->setNote(phabricator_datetime($commit->getEpoch(), $viewer))
-      ->setTarget($commit->renderAnyCommitter($viewer, $handles));
+    $author_view = $commit->newCommitAuthorView($viewer);
+    if ($author_view) {
+      $author_date = $data->getCommitDetail('authorEpoch');
+      $author_date = phabricator_datetime($author_date, $viewer);
 
-    $committed_list = new PHUIStatusListView();
-    $committed_list->addItem($committed_info);
-    $view->addProperty(
-      pht('Committed'),
-      $committed_list);
+      $provenance_list->addItem(
+        id(new PHUIStatusItemView())
+          ->setTarget($author_view)
+          ->setNote(pht('Authored on %s', $author_date)));
+    }
+
+    if (!$commit->isAuthorSameAsCommitter()) {
+      $committer_view = $commit->newCommitCommitterView($viewer);
+      if ($committer_view) {
+        $committer_date = $commit->getEpoch();
+        $committer_date = phabricator_datetime($committer_date, $viewer);
+
+        $provenance_list->addItem(
+          id(new PHUIStatusItemView())
+            ->setTarget($committer_view)
+            ->setNote(pht('Committed on %s', $committer_date)));
+      }
+    }
 
     if ($push_logs) {
       $pushed_list = new PHUIStatusListView();
 
       foreach ($push_logs as $push_log) {
-        $pushed_item = id(new PHUIStatusItemView())
-          ->setTarget($handles[$push_log->getPusherPHID()]->renderLink())
-          ->setNote(phabricator_datetime($push_log->getEpoch(), $viewer));
-        $pushed_list->addItem($pushed_item);
-      }
+        $pusher_date = $push_log->getEpoch();
+        $pusher_date = phabricator_datetime($pusher_date, $viewer);
 
-      $view->addProperty(
-        pht('Pushed'),
-        $pushed_list);
+        $pusher_view = $handles[$push_log->getPusherPHID()]->renderLink();
+
+        $provenance_list->addItem(
+          id(new PHUIStatusItemView())
+            ->setTarget($pusher_view)
+            ->setNote(pht('Pushed on %s', $pusher_date)));
+      }
     }
 
+    $view->addProperty(pht('Provenance'), $provenance_list);
+
     $reviewer_phid = $data->getCommitDetail('reviewerPHID');
     if ($reviewer_phid) {
       $view->addProperty(

src/applications/repository/storage/PhabricatorRepositoryCommit.php

@@ -377,52 +377,6 @@ final class PhabricatorRepositoryCommit
     return $repository->formatCommitName($identifier, $local = true);
   }
 
-  /**
-   * Make a strong effort to find a way to render this commit's committer.
-   * This currently attempts to use @{PhabricatorRepositoryIdentity}, and
-   * falls back to examining the commit detail information. After we force
-   * the migration to using identities, update this method to remove the
-   * fallback. See T12164 for details.
-   */
-  public function renderAnyCommitter(PhabricatorUser $viewer, $handles) {
-    $committer = $this->renderCommitter($viewer, $handles);
-    if ($committer) {
-      return $committer;
-    }
-
-    return $this->renderAuthor($viewer, $handles);
-  }
-
-  public function renderCommitter(PhabricatorUser $viewer, $handles) {
-    $committer_phid = $this->getCommitterDisplayPHID();
-    if ($committer_phid) {
-      return $handles[$committer_phid]->renderLink();
-    }
-
-    $data = $this->getCommitData();
-    $committer_name = $data->getCommitDetail('committer');
-    if (strlen($committer_name)) {
-      return DiffusionView::renderName($committer_name);
-    }
-
-    return null;
-  }
-
-  public function renderAuthor(PhabricatorUser $viewer, $handles) {
-    $author_phid = $this->getAuthorDisplayPHID();
-    if ($author_phid) {
-      return $handles[$author_phid]->renderLink();
-    }
-
-    $data = $this->getCommitData();
-    $author_name = $data->getAuthorName();
-    if (strlen($author_name)) {
-      return DiffusionView::renderName($author_name);
-    }
-
-    return null;
-  }
-
   public function loadIdentities(PhabricatorUser $viewer) {
     if ($this->authorIdentity !== self::ATTACHABLE) {
       return $this;
@@ -511,6 +465,65 @@ final class PhabricatorRepositoryCommit
     return (bool)$this->isPartiallyImported(self::IMPORTED_CLOSEABLE);
   }
 
+  public function newCommitAuthorView(PhabricatorUser $viewer) {
+    $author_phid = $this->getAuthorDisplayPHID();
+    if ($author_phid) {
+      $handles = $viewer->loadHandles(array($author_phid));
+      return $handles[$author_phid]->renderLink();
+    }
+
+    $author = $this->getRawAuthorStringForDisplay();
+    if (strlen($author)) {
+      return DiffusionView::renderName($author);
+    }
+
+    return null;
+  }
+
+  public function newCommitCommitterView(PhabricatorUser $viewer) {
+    $committer_phid = $this->getCommitterDisplayPHID();
+    if ($committer_phid) {
+      $handles = $viewer->loadHandles(array($committer_phid));
+      return $handles[$committer_phid]->renderLink();
+    }
+
+    $committer = $this->getRawCommitterStringForDisplay();
+    if (strlen($committer)) {
+      return DiffusionView::renderName($committer);
+    }
+
+    return null;
+  }
+
+  public function isAuthorSameAsCommitter() {
+    $author_phid = $this->getAuthorDisplayPHID();
+    $committer_phid = $this->getCommitterDisplayPHID();
+
+    if ($author_phid && $committer_phid) {
+      return ($author_phid === $committer_phid);
+    }
+
+    if ($author_phid || $committer_phid) {
+      return false;
+    }
+
+    $author = $this->getRawAuthorStringForDisplay();
+    $committer = $this->getRawCommitterStringForDisplay();
+
+    return ($author === $committer);
+  }
+
+  private function getRawAuthorStringForDisplay() {
+    $data = $this->getCommitData();
+    return $data->getAuthorName();
+  }
+
+  private function getRawCommitterStringForDisplay() {
+    $data = $this->getCommitData();
+    return $data->getCommitDetail('committer');
+  }
+
+
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 
   public function getCapabilities() {