From 03c6bf0d0916b064bd691a4f273e1d27c05a6c7d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 21 Mar 2014 14:39:56 -0700
Subject: [PATCH] Make Herald less ambitious about resolving repositories for
 revisions

Summary:
Fixes T4636. If a user manually deletes a "repository" setting from a revision, Herald attempts to resolve it. Instead, Herald should now just trust Differential. Generally, the new logic is:

  - When diffs are created, figure out repository information.
  - When revisions are updated, copy info from diffs.
  - Everywhere else, just trust the revision field.

Test Plan:
  - Created revisions.
  - Used Herald to dry-run revisions before and after a manual edit to remove the repository setting.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4636

Differential Revision: https://secure.phabricator.com/D8576
---
 ...duitAPI_differential_creatediff_Method.php | 21 ++++++++++++++++--
 .../editor/DifferentialTransactionEditor.php  |  1 +
 .../HeraldDifferentialRevisionAdapter.php     | 22 ++-----------------
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/src/applications/differential/conduit/ConduitAPI_differential_creatediff_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_creatediff_Method.php
index 5ddfbb114d..f0b649b227 100644
--- a/src/applications/differential/conduit/ConduitAPI_differential_creatediff_Method.php
+++ b/src/applications/differential/conduit/ConduitAPI_differential_creatediff_Method.php
@@ -40,6 +40,7 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
   }
 
   protected function execute(ConduitAPIRequest $request) {
+    $viewer = $request->getUser();
     $change_data = $request->getValue('changes');
 
     $changes = array();
@@ -53,7 +54,7 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
 
     $diff->setBranch($request->getValue('branch'));
     $diff->setCreationMethod($request->getValue('creationMethod'));
-    $diff->setAuthorPHID($request->getUser()->getPHID());
+    $diff->setAuthorPHID($viewer->getPHID());
     $diff->setBookmark($request->getValue('bookmark'));
 
     // TODO: Remove this eventually; for now continue writing the UUID. Note
@@ -64,7 +65,7 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
     $repository_phid = $request->getValue('repositoryPHID');
     if ($repository_phid) {
       $repository = id(new PhabricatorRepositoryQuery())
-        ->setViewer($request->getUser())
+        ->setViewer($viewer)
         ->withPHIDs(array($repository_phid))
         ->executeOne();
       if ($repository) {
@@ -142,6 +143,22 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
 
     $diff->save();
 
+    // If we didn't get an explicit `repositoryPHID` (which means the client is
+    // old, or couldn't figure out which repository the working copy belongs
+    // to), apply heuristics to try to figure it out.
+
+    if (!$repository_phid) {
+      $repository = id(new DifferentialRepositoryLookup())
+        ->setDiff($diff)
+        ->setViewer($viewer)
+        ->lookupRepository();
+      if ($repository) {
+        $diff->setRepositoryPHID($repository->getPHID());
+        $diff->setRepositoryUUID($repository->getUUID());
+        $diff->save();
+      }
+    }
+
     $path = '/differential/diff/'.$diff->getID().'/';
     $uri = PhabricatorEnv::getURI($path);
 
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
index 23f77301b1..5f4483b7b3 100644
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -498,6 +498,7 @@ final class DifferentialTransactionEditor
 
         $object->setLineCount($diff->getLineCount());
         $object->setRepositoryPHID($diff->getRepositoryPHID());
+        $object->setArcanistProjectPHID($diff->getArcanistProjectPHID());
 
         return;
     }
diff --git a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
index a7f95e28e6..52f22f91a0 100644
--- a/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
+++ b/src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php
@@ -157,28 +157,10 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
   public function loadRepository() {
     if ($this->repository === null) {
       $this->repository = false;
-
-      $viewer = PhabricatorUser::getOmnipotentUser();
-      $repository_phid = null;
-
-      $revision = $this->revision;
-      if ($revision->getRepositoryPHID()) {
-        $repository_phid = $revision->getRepositoryPHID();
-      } else {
-        $repository = id(new DifferentialRepositoryLookup())
-          ->setViewer($viewer)
-          ->setDiff($this->diff)
-          ->lookupRepository();
-        if ($repository) {
-          // We want to get the projects for this repository too, so run a
-          // full query for it below.
-          $repository_phid = $repository->getPHID();
-        }
-      }
-
+      $repository_phid = $this->getObject()->getRepositoryPHID();
       if ($repository_phid) {
         $repository = id(new PhabricatorRepositoryQuery())
-          ->setViewer($viewer)
+          ->setViewer(PhabricatorUser::getOmnipotentUser())
           ->withPHIDs(array($repository_phid))
           ->needProjectPHIDs(true)
           ->executeOne();