Allow blocking reviewers to be added via the web UI

Summary:
Ref T10939. Adds a `blocking(...)` token.

This code is pretty iffy and going to get worse before it gets better, but the fix (T10967 + EditEngine) is going to be a fair chunk of work down the road.

Test Plan: {F1426966}

Reviewers: chad

Reviewed By: chad

Subscribers: scode

Maniphest Tasks: T10939

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

src/__phutil_library_map__.php

@@ -361,6 +361,7 @@ phutil_register_library_map(array(
     'DifferentialAuthorField' => 'applications/differential/customfield/DifferentialAuthorField.php',
     'DifferentialBlameRevisionField' => 'applications/differential/customfield/DifferentialBlameRevisionField.php',
     'DifferentialBlockHeraldAction' => 'applications/differential/herald/DifferentialBlockHeraldAction.php',
+    'DifferentialBlockingReviewerDatasource' => 'applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php',
     'DifferentialBranchField' => 'applications/differential/customfield/DifferentialBranchField.php',
     'DifferentialChangeDetailMailView' => 'applications/differential/mail/DifferentialChangeDetailMailView.php',
     'DifferentialChangeHeraldFieldGroup' => 'applications/differential/herald/DifferentialChangeHeraldFieldGroup.php',
@@ -497,6 +498,7 @@ phutil_register_library_map(array(
     'DifferentialRevertPlanField' => 'applications/differential/customfield/DifferentialRevertPlanField.php',
     'DifferentialReviewedByField' => 'applications/differential/customfield/DifferentialReviewedByField.php',
     'DifferentialReviewer' => 'applications/differential/storage/DifferentialReviewer.php',
+    'DifferentialReviewerDatasource' => 'applications/differential/typeahead/DifferentialReviewerDatasource.php',
     'DifferentialReviewerForRevisionEdgeType' => 'applications/differential/edge/DifferentialReviewerForRevisionEdgeType.php',
     'DifferentialReviewerStatus' => 'applications/differential/constants/DifferentialReviewerStatus.php',
     'DifferentialReviewersAddBlockingReviewersHeraldAction' => 'applications/differential/herald/DifferentialReviewersAddBlockingReviewersHeraldAction.php',
@@ -4561,6 +4563,7 @@ phutil_register_library_map(array(
     'DifferentialAuthorField' => 'DifferentialCustomField',
     'DifferentialBlameRevisionField' => 'DifferentialStoredCustomField',
     'DifferentialBlockHeraldAction' => 'HeraldAction',
+    'DifferentialBlockingReviewerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'DifferentialBranchField' => 'DifferentialCustomField',
     'DifferentialChangeDetailMailView' => 'DifferentialMailView',
     'DifferentialChangeHeraldFieldGroup' => 'HeraldFieldGroup',
@@ -4713,6 +4716,7 @@ phutil_register_library_map(array(
     'DifferentialRevertPlanField' => 'DifferentialStoredCustomField',
     'DifferentialReviewedByField' => 'DifferentialCoreCustomField',
     'DifferentialReviewer' => 'Phobject',
+    'DifferentialReviewerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'DifferentialReviewerForRevisionEdgeType' => 'PhabricatorEdgeType',
     'DifferentialReviewerStatus' => 'Phobject',
     'DifferentialReviewersAddBlockingReviewersHeraldAction' => 'DifferentialReviewersHeraldAction',

src/applications/differential/customfield/DifferentialReviewersField.php

@@ -36,43 +36,83 @@ final class DifferentialReviewersField
   }
 
   public function readValueFromRequest(AphrontRequest $request) {
-    // Compute a new set of reviewer objects. For reviewers who haven't been
-    // added or removed, retain their existing status. Also, respect the new
-    // order.
+    // Compute a new set of reviewer objects. We're going to respect the new
+    // reviewer order, add or remove any missing or new reviewers, and respect
+    // any blocking or unblocking changes. For reviewers who were there before
+    // and are still there, we're going to keep the current value because it
+    // may be something like "Accept", "Reject", etc.
 
     $old_status = $this->getValue();
-    $old_status = mpull($old_status, null, 'getReviewerPHID');
+    $old_status = mpull($old_status, 'getStatus', 'getReviewerPHID');
+
+    $datasource = id(new DifferentialBlockingReviewerDatasource())
+      ->setViewer($request->getViewer());
 
     $new_phids = $request->getArr($this->getFieldKey());
-    $new_phids = array_fuse($new_phids);
+    $new_phids = $datasource->evaluateTokens($new_phids);
+
+    $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
+
+    $specs = array();
+    foreach ($new_phids as $spec) {
+      if (!is_array($spec)) {
+        $spec = array(
+          'type' => DifferentialReviewerStatus::STATUS_ADDED,
+          'phid' => $spec,
+        );
+      }
+      $specs[$spec['phid']] = $spec;
+    }
 
     $new_status = array();
-    foreach ($new_phids as $new_phid) {
-      if (empty($old_status[$new_phid])) {
-        $new_status[$new_phid] = new DifferentialReviewer(
-          $new_phid,
-          array(
-            'status' => DifferentialReviewerStatus::STATUS_ADDED,
-          ));
-      } else {
-        $new_status[$new_phid] = $old_status[$new_phid];
+    foreach ($specs as $phid => $spec) {
+      $new = $spec['type'];
+      $old = idx($old_status, $phid);
+
+      // If we have an old status and this didn't make the reviewer blocking
+      // or nonblocking, just retain the old status. This makes sure we don't
+      // throw away rejects, accepts, etc.
+      if ($old) {
+        $is_block = ($old !== $status_blocking && $new === $status_blocking);
+        $is_unblock = ($old === $status_blocking && $new !== $status_blocking);
+        if (!$is_block && !$is_unblock) {
+          $new_status[$phid] = $old;
+          continue;
+        }
       }
+
+      $new_status[$phid] = $new;
+    }
+
+    foreach ($new_status as $phid => $status) {
+      $new_status[$phid] = new DifferentialReviewer(
+        $phid,
+        array(
+          'status' => $status,
+        ));
     }
 
     $this->setValue($new_status);
   }
 
   public function renderEditControl(array $handles) {
-    $phids = array();
-    if ($this->getValue()) {
-      $phids = mpull($this->getValue(), 'getReviewerPHID');
+    $status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
+
+    $value = array();
+    foreach ($this->getValue() as $reviewer) {
+      $phid = $reviewer->getReviewerPHID();
+      if ($reviewer->getStatus() == $status_blocking) {
+        $value[] = 'blocking('.$phid.')';
+      } else {
+        $value[] = $phid;
+      }
     }
 
     return id(new AphrontFormTokenizerControl())
       ->setUser($this->getViewer())
       ->setName($this->getFieldKey())
-      ->setDatasource(new DiffusionAuditorDatasource())
-      ->setValue($phids)
+      ->setDatasource(new DifferentialReviewerDatasource())
+      ->setValue($value)
       ->setError($this->getFieldError())
       ->setLabel($this->getFieldName());
   }

src/applications/differential/typeahead/DifferentialBlockingReviewerDatasource.php

@@ -0,0 +1,128 @@
+<?php
+
+final class DifferentialBlockingReviewerDatasource
+  extends PhabricatorTypeaheadCompositeDatasource {
+
+  public function getBrowseTitle() {
+    return pht('Browse Blocking Reviewers');
+  }
+
+  public function getPlaceholderText() {
+    return pht('Type a user, project, or package name...');
+  }
+
+  public function getDatasourceApplicationClass() {
+    return 'PhabricatorDifferentialApplication';
+  }
+
+  public function getComponentDatasources() {
+    return array(
+      new PhabricatorPeopleDatasource(),
+      new PhabricatorProjectDatasource(),
+      new PhabricatorOwnersPackageDatasource(),
+    );
+  }
+
+  public function getDatasourceFunctions() {
+    return array(
+      'blocking' => array(
+        'name' => pht('Blocking: ...'),
+        'arguments' => pht('reviewer'),
+        'summary' => pht('Select a blocking reviewer.'),
+        'description' => pht(
+          "This function allows you to add a reviewer as a blocking ".
+          "reviewer. For example, this will add `%s` as a blocking reviewer: ".
+          "\n\n%s\n\n",
+          'alincoln',
+          '> blocking(alincoln)'),
+      ),
+    );
+  }
+
+
+  protected function didLoadResults(array $results) {
+    foreach ($results as $result) {
+      $result
+        ->setColor('red')
+        ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION)
+        ->setIcon('fa-asterisk')
+        ->setPHID('blocking('.$result->getPHID().')')
+        ->setDisplayName(pht('Blocking: %s', $result->getDisplayName()))
+        ->setName($result->getName().' blocking');
+    }
+
+    return $results;
+  }
+
+  protected function evaluateFunction($function, array $argv_list) {
+    $phids = array();
+    foreach ($argv_list as $argv) {
+      $phids[] = head($argv);
+    }
+
+    $phids = $this->resolvePHIDs($phids);
+
+    $results = array();
+    foreach ($phids as $phid) {
+      $results[] = array(
+        'type' => DifferentialReviewerStatus::STATUS_BLOCKING,
+        'phid' => $phid,
+      );
+    }
+
+    return $results;
+  }
+
+  public function renderFunctionTokens($function, array $argv_list) {
+    $phids = array();
+    foreach ($argv_list as $argv) {
+      $phids[] = head($argv);
+    }
+
+    $phids = $this->resolvePHIDs($phids);
+
+    $tokens = $this->renderTokens($phids);
+    foreach ($tokens as $token) {
+      $token->setColor(null);
+      if ($token->isInvalid()) {
+        $token
+          ->setValue(pht('Blocking: Invalid Reviewer'));
+      } else {
+        $token
+          ->setIcon('fa-asterisk')
+          ->setTokenType(PhabricatorTypeaheadTokenView::TYPE_FUNCTION)
+          ->setColor('red')
+          ->setKey('blocking('.$token->getKey().')')
+          ->setValue(pht('Blocking: %s', $token->getValue()));
+      }
+    }
+
+    return $tokens;
+  }
+
+  private function resolvePHIDs(array $phids) {
+    $usernames = array();
+    foreach ($phids as $key => $phid) {
+      if (phid_get_type($phid) != PhabricatorPeopleUserPHIDType::TYPECONST) {
+        $usernames[$key] = $phid;
+      }
+    }
+
+    if ($usernames) {
+      $users = id(new PhabricatorPeopleQuery())
+        ->setViewer($this->getViewer())
+        ->withUsernames($usernames)
+        ->execute();
+      $users = mpull($users, null, 'getUsername');
+      foreach ($usernames as $key => $username) {
+        $user = idx($users, $username);
+        if ($user) {
+          $phids[$key] = $user->getPHID();
+        }
+      }
+    }
+
+    return $phids;
+  }
+
+}

src/applications/differential/typeahead/DifferentialReviewerDatasource.php

@@ -0,0 +1,27 @@
+<?php
+
+final class DifferentialReviewerDatasource
+  extends PhabricatorTypeaheadCompositeDatasource {
+
+  public function getBrowseTitle() {
+    return pht('Browse Reviewers');
+  }
+
+  public function getPlaceholderText() {
+    return pht('Type a user, project, or package name...');
+  }
+
+  public function getDatasourceApplicationClass() {
+    return 'PhabricatorDifferentialApplication';
+  }
+
+  public function getComponentDatasources() {
+    return array(
+      new PhabricatorPeopleDatasource(),
+      new PhabricatorProjectDatasource(),
+      new PhabricatorOwnersPackageDatasource(),
+      new DifferentialBlockingReviewerDatasource(),
+    );
+  }
+
+}

src/applications/differential/view/DifferentialAddCommentView.php

@@ -69,7 +69,9 @@ final class DifferentialAddCommentView extends AphrontView {
     );
 
     $mailable_source = new PhabricatorMetaMTAMailableDatasource();
-    $reviewer_source = new PhabricatorProjectOrUserDatasource();
+
+    // TODO: This should be a reviewers datasource, but it's a mess.
+    $reviewer_source = new PhabricatorMetaMTAMailableDatasource();
 
     $form = new AphrontFormView();
     $form