From 57a584e270a703113431068e77ffcea122fdeb6c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 28 Jul 2017 10:37:53 -0700
Subject: [PATCH] (stable) Don't fatal when viewing tags pointing at commits we
 haven't imported/parsed yet

Summary:
In Diffusion, the "Tags" view may read commits which haven't imported or parsed yet, and thus don't have loadable objects.

Most of this logic tests for `if ($commit)`, but the author part did not. Instead, don't render author information if `$commit` is not present.

Test Plan:
  - Loaded tags view with commits present.
  - Faked `$commit = null;`, loaded tag view, got this instead of a fatal:

{F5068432}

Reviewers: chad, amckinley

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D18290
---
 .../diffusion/view/DiffusionTagListView.php   | 47 ++++++++++++-------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/src/applications/diffusion/view/DiffusionTagListView.php b/src/applications/diffusion/view/DiffusionTagListView.php
index abab9003c3..0f8cbeae71 100644
--- a/src/applications/diffusion/view/DiffusionTagListView.php
+++ b/src/applications/diffusion/view/DiffusionTagListView.php
@@ -52,24 +52,12 @@ final class DiffusionTagListView extends DiffusionView {
           'commit' => $tag->getCommitIdentifier(),
         ));
 
-      $author = null;
-      if ($commit && $commit->getAuthorPHID()) {
-        $author = $this->handles[$commit->getAuthorPHID()]->renderLink();
-      } else if ($commit && $commit->getCommitData()) {
-        $author = self::renderName($commit->getCommitData()->getAuthorName());
+      if ($commit) {
+        $author = $this->renderAuthor($tag, $commit);
       } else {
-        $author = self::renderName($tag->getAuthor());
+        $author = null;
       }
 
-      $committed = phabricator_datetime($commit->getEpoch(), $viewer);
-      $author_name = phutil_tag(
-        'strong',
-        array(
-          'class' => 'diffusion-history-author-name',
-        ),
-        $author);
-      $authored = pht('%s on %s.', $author_name, $committed);
-
       $description = null;
       if ($tag->getType() == 'git/tag') {
         // In Git, a tag may be a "real" tag, or just a reference to a commit.
@@ -139,16 +127,43 @@ final class DiffusionTagListView extends DiffusionView {
         ->setHref($tag_href)
         ->addAttribute(array($commit_tag))
         ->addAttribute($description)
-        ->addAttribute($authored)
         ->setSideColumn(array(
           $build_view,
           $button_bar,
         ));
 
+      if ($author) {
+        $item->addAttribute($author);
+      }
+
       $list->addItem($item);
     }
 
     return $list;
   }
 
+  private function renderAuthor(
+    DiffusionRepositoryTag $tag,
+    PhabricatorRepositoryCommit $commit) {
+    $viewer = $this->getViewer();
+
+    if ($commit->getAuthorPHID()) {
+      $author = $this->handles[$commit->getAuthorPHID()]->renderLink();
+    } else if ($commit->getCommitData()) {
+      $author = self::renderName($commit->getCommitData()->getAuthorName());
+    } else {
+      $author = self::renderName($tag->getAuthor());
+    }
+
+    $committed = phabricator_datetime($commit->getEpoch(), $viewer);
+    $author_name = phutil_tag(
+      'strong',
+      array(
+        'class' => 'diffusion-history-author-name',
+      ),
+      $author);
+
+    return pht('%s on %s.', $author_name, $committed);
+  }
+
 }