From 498fb331038cb286fceb32ab8010b398e423b818 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 24 Aug 2016 05:55:47 -0700
Subject: [PATCH] When a commit has a "rewritten" hint, show it in the UI
 instead of the generic "deleted" message

Summary:
Ref T11522. When a commit is no longer reachable from any branch/tag, we currently show a "this has been deleted" message.

Instead, go further: check if there is a "rewritten" hint pointing at a commit the current commit was rewritten into. If we find one, show a message about that instead.

(This isn't super pretty, just getting it working for now. I expect to revisit this UI in T9713 if we don't get to it before that.)

Test Plan: {F1780703}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11522

Differential Revision: https://secure.phabricator.com/D16436
---
 .../controller/DiffusionCommitController.php  | 100 ++++++++++++------
 .../PhabricatorRepositoryCommitHint.php       |   4 +
 2 files changed, 71 insertions(+), 33 deletions(-)

diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php
index 1346955f80..c27006350f 100644
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -73,6 +73,38 @@ final class DiffusionCommitController extends DiffusionController {
     $commit_data = $commit->getCommitData();
     $is_foreign = $commit_data->getCommitDetail('foreign-svn-stub');
     $error_panel = null;
+
+    $hard_limit = 1000;
+
+    if ($commit->isImported()) {
+      $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest(
+        $drequest);
+      $change_query->setLimit($hard_limit + 1);
+      $changes = $change_query->loadChanges();
+    } else {
+      $changes = array();
+    }
+
+    $was_limited = (count($changes) > $hard_limit);
+    if ($was_limited) {
+      $changes = array_slice($changes, 0, $hard_limit);
+    }
+
+    $count = count($changes);
+
+    $is_unreadable = false;
+    $hint = null;
+    if (!$count || $commit->isUnreachable()) {
+      $hint = id(new DiffusionCommitHintQuery())
+        ->setViewer($viewer)
+        ->withRepositoryPHIDs(array($repository->getPHID()))
+        ->withOldCommitIdentifiers(array($commit->getCommitIdentifier()))
+        ->executeOne();
+      if ($hint) {
+        $is_unreadable = $hint->isUnreadable();
+      }
+    }
+
     if ($is_foreign) {
       $subpath = $commit_data->getCommitDetail('svn-subpath');
 
@@ -130,9 +162,41 @@ final class DiffusionCommitController extends DiffusionController {
           $message));
 
       if ($commit->isUnreachable()) {
-        $this->commitErrors[] = pht(
-          'This commit has been deleted in the repository: it is no longer '.
-          'reachable from any branch, tag, or ref.');
+        $did_rewrite = false;
+        if ($hint) {
+          if ($hint->isRewritten()) {
+            $rewritten = id(new DiffusionCommitQuery())
+              ->setViewer($viewer)
+              ->withRepository($repository)
+              ->withIdentifiers(array($hint->getNewCommitIdentifier()))
+              ->executeOne();
+            if ($rewritten) {
+              $did_rewrite = true;
+              $rewritten_uri = $rewritten->getURI();
+              $rewritten_name = $rewritten->getLocalName();
+
+              $rewritten_link = phutil_tag(
+                'a',
+                array(
+                  'href' => $rewritten_uri,
+                ),
+                $rewritten_name);
+
+              $this->commitErrors[] = pht(
+                'This commit was rewritten after it was published, which '.
+                'changed the commit hash. This old version of the commit is '.
+                'no longer reachable from any branch, tag or ref. The new '.
+                'version of this commit is %s.',
+                $rewritten_link);
+            }
+          }
+        }
+
+        if (!$did_rewrite) {
+          $this->commitErrors[] = pht(
+            'This commit has been deleted in the repository: it is no longer '.
+            'reachable from any branch, tag, or ref.');
+        }
       }
 
       if ($this->getCommitErrors()) {
@@ -143,42 +207,12 @@ final class DiffusionCommitController extends DiffusionController {
     }
 
     $timeline = $this->buildComments($commit);
-    $hard_limit = 1000;
-
-    if ($commit->isImported()) {
-      $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest(
-        $drequest);
-      $change_query->setLimit($hard_limit + 1);
-      $changes = $change_query->loadChanges();
-    } else {
-      $changes = array();
-    }
-
-    $was_limited = (count($changes) > $hard_limit);
-    if ($was_limited) {
-      $changes = array_slice($changes, 0, $hard_limit);
-    }
-
     $merge_table = $this->buildMergesTable($commit);
 
     $highlighted_audits = $commit->getAuthorityAudits(
       $viewer,
       $this->auditAuthorityPHIDs);
 
-    $count = count($changes);
-
-    $is_unreadable = false;
-    if (!$count) {
-      $hint = id(new DiffusionCommitHintQuery())
-        ->setViewer($viewer)
-        ->withRepositoryPHIDs(array($repository->getPHID()))
-        ->withOldCommitIdentifiers(array($commit->getCommitIdentifier()))
-        ->executeOne();
-      if ($hint) {
-        $is_unreadable = $hint->isUnreadable();
-      }
-    }
-
     $show_changesets = false;
     $info_panel = null;
     $change_list = null;
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php
index 9ab8d2d056..e8594a1496 100644
--- a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php
@@ -105,6 +105,10 @@ final class PhabricatorRepositoryCommitHint
     return ($this->getHintType() == self::HINT_UNREADABLE);
   }
 
+  public function isRewritten() {
+    return ($this->getHintType() == self::HINT_REWRITTEN);
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */