Allow projects to review revisions

Summary:
Ref T1279. No actual logical changes, but:

  - You can now add projects as reviewers from the revision view typeahead ("Add Reviewers" action).
  - You can now add projects as reviewers from the revision detail typeahead.
  - You can now add projects as reviewers from the CLI (`#yoloswag`).
  - Generated commit messages now list project reviewers (`Reviewers: #yoloswag`).

I'll separate projects from users in the "Reviewers" tables in the next revision.

Test Plan:
  - Added projects as reviewers using the web UI and CLI.
  - Used `arc amend --show --revision Dnnn` to generate commit messages.
  - Viewed revision with project reviewers in web UI.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1279

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

src/applications/differential/field/specification/DifferentialFieldSpecification.php

@@ -779,6 +779,14 @@ abstract class DifferentialFieldSpecification {
     return $this->parseCommitMessageObjectList($value, $mailables = false);
   }
 
+  protected function parseCommitMessageUserOrProjectList($value) {
+    return $this->parseCommitMessageObjectList(
+      $value,
+      $mailables = false,
+      $allow_partial = false,
+      $projects = true);
+  }
+
   /**
    * Parse a list of mailable objects into a canonical PHID list.
    *
@@ -813,36 +821,66 @@ abstract class DifferentialFieldSpecification {
 
     $object_map = array();
 
-    $users = id(new PhabricatorUser())->loadAllWhere(
+    $project_names = array();
-      '(username IN (%Ls))',
+    $other_names = array();
-      $value);
+    foreach ($value as $item) {
-
+      if (preg_match('/^#/', $item)) {
-    $user_map = mpull($users, 'getPHID', 'getUsername');
+        $project_names[$item] = ltrim(phutil_utf8_strtolower($item), '#').'/';
-    foreach ($user_map as $username => $phid) {
+      } else {
-      // Usernames may have uppercase letters in them. Put both names in the
+        $other_names[] = $item;
-      // map so we can try the original case first, so that username *always*
+      }
-      // works in weird edge cases where some other mailable object collides.
-      $object_map[$username] = $phid;
-      $object_map[strtolower($username)] = $phid;
     }
 
-    if ($include_mailables) {
+    if ($project_names) {
-      $mailables = id(new PhabricatorMetaMTAMailingList())->loadAllWhere(
+      // TODO: (T603) This should probably be policy-aware, although maybe not,
-        '(email IN (%Ls)) OR (name IN (%Ls))',
+      // since we generally don't want to destroy data and it doesn't leak
-        $value,
+      // anything?
-        $value);
+      $projects = id(new PhabricatorProjectQuery())
-      $object_map += mpull($mailables, 'getPHID', 'getName');
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
-      $object_map += mpull($mailables, 'getPHID', 'getEmail');
+        ->withPhrictionSlugs($project_names)
+        ->execute();
+
+      $reverse_map = array_flip($project_names);
+      foreach ($projects as $project) {
+        $reverse_key = $project->getPhrictionSlug();
+        if (isset($reverse_map[$reverse_key])) {
+          $object_map[$reverse_map[$reverse_key]] = $project->getPHID();
+        }
+      }
+    }
+
+    if ($other_names) {
+      $users = id(new PhabricatorUser())->loadAllWhere(
+        '(username IN (%Ls))',
+        $other_names);
+
+      $user_map = mpull($users, 'getPHID', 'getUsername');
+      foreach ($user_map as $username => $phid) {
+        // Usernames may have uppercase letters in them. Put both names in the
+        // map so we can try the original case first, so that username *always*
+        // works in weird edge cases where some other mailable object collides.
+        $object_map[$username] = $phid;
+        $object_map[strtolower($username)] = $phid;
+      }
+
+      if ($include_mailables) {
+        $mailables = id(new PhabricatorMetaMTAMailingList())->loadAllWhere(
+          '(email IN (%Ls)) OR (name IN (%Ls))',
+          $other_names,
+          $other_names);
+        $object_map += mpull($mailables, 'getPHID', 'getName');
+        $object_map += mpull($mailables, 'getPHID', 'getEmail');
+      }
     }
 
     $invalid = array();
     $results = array();
     foreach ($value as $name) {
       if (empty($object_map[$name])) {
-        if (empty($object_map[strtolower($name)])) {
+        if (empty($object_map[phutil_utf8_strtolower($name)])) {
           $invalid[] = $name;
         } else {
-          $results[] = $object_map[strtolower($name)];
+          $results[] = $object_map[phutil_utf8_strtolower($name)];
         }
       } else {
         $results[] = $object_map[$name];

src/applications/differential/field/specification/DifferentialReviewersFieldSpecification.php

@@ -15,7 +15,7 @@ final class DifferentialReviewersFieldSpecification
   }
 
   public function renderLabelForRevisionView() {
-    return 'Reviewers:';
+    return pht('Reviewers');
   }
 
   public function renderValueForRevisionView() {
@@ -144,7 +144,7 @@ final class DifferentialReviewersFieldSpecification
       ->setLabel(pht('Reviewers'))
       ->setName('reviewers')
       ->setUser($this->getUser())
-      ->setDatasource('/typeahead/common/users/')
+      ->setDatasource('/typeahead/common/usersorprojects/')
       ->setValue($reviewer_map)
       ->setError($this->error);
   }
@@ -179,9 +179,11 @@ final class DifferentialReviewersFieldSpecification
       return null;
     }
 
+    $project_type = PhabricatorProjectPHIDTypeProject::TYPECONST;
+
     $names = array();
     foreach ($this->reviewers as $phid) {
-      $names[] = $this->getHandle($phid)->getName();
+      $names[] = $this->getHandle($phid)->getObjectName();
     }
 
     return implode(', ', $names);
@@ -195,7 +197,7 @@ final class DifferentialReviewersFieldSpecification
   }
 
   public function parseValueFromCommitMessage($value) {
-    return $this->parseCommitMessageUserList($value);
+    return $this->parseCommitMessageUserOrProjectList($value);
   }
 
   public function shouldAppearOnRevisionList() {
@@ -242,7 +244,8 @@ final class DifferentialReviewersFieldSpecification
     $handles = array_select_keys(
       $handles,
       array($this->getRevision()->getPrimaryReviewer())) + $handles;
-    $names = mpull($handles, 'getName');
+
+    $names = mpull($handles, 'getObjectName');
     return 'Reviewers: '.implode(', ', $names);
   }
 

src/applications/differential/query/DifferentialRevisionQuery.php

@@ -914,6 +914,7 @@ final class DifferentialRevisionQuery
     $edges = id(new PhabricatorEdgeQuery())
       ->withSourcePHIDs(mpull($revisions, 'getPHID'))
       ->withEdgeTypes(array($type_reviewer))
+      ->setOrder(PhabricatorEdgeQuery::ORDER_OLDEST_FIRST)
       ->execute();
 
     foreach ($revisions as $revision) {
@@ -923,6 +924,7 @@ final class DifferentialRevisionQuery
         $data[] = array(
           'relation' => DifferentialRevision::RELATION_REVIEWER,
           'objectPHID' => $dst_phid,
+          'reasonPHID' => null,
         );
       }
 
@@ -1024,11 +1026,11 @@ final class DifferentialRevisionQuery
       ->withSourcePHIDs(mpull($revisions, 'getPHID'))
       ->withEdgeTypes(array($edge_type))
       ->needEdgeData(true)
+      ->setOrder(PhabricatorEdgeQuery::ORDER_OLDEST_FIRST)
       ->execute();
 
     foreach ($revisions as $revision) {
       $revision_edges = $edges[$revision->getPHID()][$edge_type];
-
       $reviewers = array();
       foreach ($revision_edges as $user_phid => $edge) {
         $data = $edge['data'];

src/applications/differential/storage/DifferentialRevision.php

@@ -237,11 +237,13 @@ final class DifferentialRevision extends DifferentialDAO
     $reviewer_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
       $this->getPHID(),
       PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER);
+    $reviewer_phids = array_reverse($reviewer_phids);
 
     foreach ($reviewer_phids as $phid) {
       $data[] = array(
         'relation' => self::RELATION_REVIEWER,
         'objectPHID' => $phid,
+        'reasonPHID' => null,
       );
     }
 

src/applications/differential/view/DifferentialAddCommentView.php

@@ -118,12 +118,12 @@ final class DifferentialAddCommentView extends AphrontView {
               'add_reviewers' => 1,
               'resign' => 1,
             ),
-            'src' => '/typeahead/common/users/',
+            'src' => '/typeahead/common/usersorprojects/',
             'value' => $this->reviewers,
             'row' => 'add-reviewers',
             'ondemand' => PhabricatorEnv::getEnvConfig('tokenizer.ondemand'),
             'labels' => $add_reviewers_labels,
-            'placeholder' => pht('Type a user name...'),
+            'placeholder' => pht('Type a user or project name...'),
           ),
           'add-ccs-tokenizer' => array(
             'actions' => array('add_ccs' => 1),

src/applications/notification/PhabricatorNotificationQuery.php

@@ -107,4 +107,8 @@ final class PhabricatorNotificationQuery
     return $this->formatWhereClause($where);
   }
 
+  protected function getPagingValue($item) {
+    return $item->getChronologicalKey();
+  }
+
 }

src/applications/phid/PhabricatorObjectHandle.php

@@ -14,6 +14,19 @@ final class PhabricatorObjectHandle
   private $status = PhabricatorObjectHandleStatus::STATUS_OPEN;
   private $complete;
   private $disabled;
+  private $objectName;
+
+  public function setObjectName($object_name) {
+    $this->objectName = $object_name;
+    return $this;
+  }
+
+  public function getObjectName() {
+    if (!$this->objectName) {
+      return $this->getName();
+    }
+    return $this->objectName;
+  }
 
   public function setURI($uri) {
     $this->uri = $uri;

src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php

@@ -39,6 +39,7 @@ final class PhabricatorProjectPHIDTypeProject extends PhabricatorPHIDType {
       $id = $project->getID();
 
       $handle->setName($name);
+      $handle->setObjectName('#'.rtrim($project->getPhrictionSlug(), '/'));
       $handle->setURI("/project/view/{$id}/");
     }
   }

src/infrastructure/edges/query/PhabricatorEdgeQuery.php

@@ -26,6 +26,10 @@ final class PhabricatorEdgeQuery extends PhabricatorQuery {
   private $edgeTypes;
   private $resultSet;
 
+  const ORDER_OLDEST_FIRST = 'order:oldest';
+  const ORDER_NEWEST_FIRST = 'order:newest';
+  private $order = self::ORDER_NEWEST_FIRST;
+
   private $needEdgeData;
 
 
@@ -74,6 +78,20 @@ final class PhabricatorEdgeQuery extends PhabricatorQuery {
   }
 
 
+  /**
+   * Configure the order edge results are returned in.
+   *
+   * @param const Order constant.
+   * @return this
+   *
+   * @task config
+   */
+  public function setOrder($order) {
+    $this->order = $order;
+    return $this;
+  }
+
+
   /**
    * When loading edges, also load edge data.
    *
@@ -303,7 +321,11 @@ final class PhabricatorEdgeQuery extends PhabricatorQuery {
    * @task internal
    */
   private function buildOrderClause($conn_r) {
-    return 'ORDER BY edge.dateCreated DESC, edge.seq ASC';
+    if ($this->order == self::ORDER_NEWEST_FIRST) {
+      return 'ORDER BY edge.dateCreated DESC, edge.seq DESC';
+    } else {
+      return 'ORDER BY edge.dateCreated ASC, edge.seq ASC';
+    }
   }
 
 }