Implement a "Reviewers" CustomField

Summary:
Ref T3886:

  - Adds a "Reviewers" field as a modern CustomField.

Ref T418:

  - Allows CustomFields to emit transaction metadata.

Test Plan: {F116254}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T418, T3886

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

src/__phutil_library_map__.php

@@ -441,6 +441,7 @@ phutil_register_library_map(array(
     'DifferentialReviewedByFieldSpecification' => 'applications/differential/field/specification/DifferentialReviewedByFieldSpecification.php',
     'DifferentialReviewer' => 'applications/differential/storage/DifferentialReviewer.php',
     'DifferentialReviewerStatus' => 'applications/differential/constants/DifferentialReviewerStatus.php',
+    'DifferentialReviewersField' => 'applications/differential/customfield/DifferentialReviewersField.php',
     'DifferentialReviewersFieldSpecification' => 'applications/differential/field/specification/DifferentialReviewersFieldSpecification.php',
     'DifferentialReviewersView' => 'applications/differential/view/DifferentialReviewersView.php',
     'DifferentialRevision' => 'applications/differential/storage/DifferentialRevision.php',
@@ -2979,6 +2980,7 @@ phutil_register_library_map(array(
     'DifferentialRevertPlanFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialReviewRequestMail' => 'DifferentialMail',
     'DifferentialReviewedByFieldSpecification' => 'DifferentialFieldSpecification',
+    'DifferentialReviewersField' => 'DifferentialCoreCustomField',
     'DifferentialReviewersFieldSpecification' => 'DifferentialFieldSpecification',
     'DifferentialReviewersView' => 'AphrontView',
     'DifferentialRevision' =>

src/applications/differential/customfield/DifferentialReviewersField.php

@@ -0,0 +1,84 @@
+<?php
+
+final class DifferentialReviewersField
+  extends DifferentialCoreCustomField {
+
+  public function getFieldKey() {
+    return 'differential:reviewers';
+  }
+
+  public function getFieldName() {
+    return pht('Reviewers');
+  }
+
+  public function getFieldDescription() {
+    return pht('Manage reviewers.');
+  }
+
+  protected function readValueFromRevision(
+    DifferentialRevision $revision) {
+    return $revision->getReviewerStatus();
+  }
+
+  public function getNewValueForApplicationTransactions() {
+    $specs = array();
+    foreach ($this->getValue() as $reviewer) {
+      $specs[$reviewer->getReviewerPHID()] = array(
+        'data' => $reviewer->getEdgeData(),
+      );
+    }
+
+    return array('=' => $specs);
+  }
+
+  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.
+
+    $old_status = $this->getValue();
+    $old_status = mpull($old_status, null, 'getReviewerPHID');
+
+    $new_phids = $request->getArr($this->getFieldKey());
+    $new_phids = array_fuse($new_phids);
+
+    $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];
+      }
+    }
+
+    $this->setValue($new_status);
+  }
+
+  public function getRequiredHandlePHIDsForEdit() {
+    return mpull($this->getValue(), 'getReviewerPHID');
+  }
+
+  public function renderEditControl(array $handles) {
+    return id(new AphrontFormTokenizerControl())
+      ->setName($this->getFieldKey())
+      ->setDatasource('/typeahead/common/usersorprojects/')
+      ->setValue($handles)
+      ->setError($this->getFieldError())
+      ->setLabel($this->getFieldName());
+  }
+
+  public function getApplicationTransactionType() {
+    return PhabricatorTransactions::TYPE_EDGE;
+  }
+
+  public function getApplicationTransactionMetadata() {
+    return array(
+      'edge:type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER,
+    );
+  }
+
+}

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -6,11 +6,11 @@ final class DifferentialTransactionEditor
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
 
+    $types[] = PhabricatorTransactions::TYPE_EDGE;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
 
 /*
-    $types[] = PhabricatorTransactions::TYPE_EDGE;
 
     $types[] = DifferentialTransaction::TYPE_INLINE;
     $types[] = DifferentialTransaction::TYPE_UPDATE;
@@ -59,6 +59,9 @@ final class DifferentialTransactionEditor
         $object->setEditPolicy($xaction->getNewValue());
         return;
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+      case PhabricatorTransactions::TYPE_EDGE:
+        // TODO: When removing reviewers, we may be able to move the revision
+        // to "Accepted".
         return;
     }
 
@@ -74,6 +77,7 @@ final class DifferentialTransactionEditor
       case PhabricatorTransactions::TYPE_EDIT_POLICY:
         return;
       case PhabricatorTransactions::TYPE_SUBSCRIBERS:
+      case PhabricatorTransactions::TYPE_EDGE:
         return;
     }
 
@@ -93,7 +97,6 @@ final class DifferentialTransactionEditor
     return $errors;
   }
 
-
   protected function requireCapabilities(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {

src/applications/differential/storage/DifferentialReviewer.php

@@ -46,4 +46,11 @@ final class DifferentialReviewer {
     return $this->authority[$viewer_phid];
   }
 
+  public function getEdgeData() {
+    return array(
+      'status' => $this->status,
+      'diffID' => $this->diffID,
+    );
+  }
+
 }

src/applications/differential/storage/DifferentialRevision.php

@@ -469,6 +469,7 @@ final class DifferentialRevision extends DifferentialDAO
       new DifferentialTitleField(),
       new DifferentialSummaryField(),
       new DifferentialTestPlanField(),
+      new DifferentialReviewersField(),
       new DifferentialSubscribersField(),
       new DifferentialRepositoryField(),
       new DifferentialViewPolicyField(),

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -990,21 +990,24 @@ abstract class PhabricatorApplicationTransactionEditor
       }
       $result[$dst_phid] = $this->normalizeEdgeTransactionValue(
         $xaction,
-        $edge);
+        $edge,
+        $dst_phid);
     }
 
     if ($new_set !== null) {
       foreach ($new_set as $dst_phid => $edge) {
         $result[$dst_phid] = $this->normalizeEdgeTransactionValue(
           $xaction,
-          $edge);
+          $edge,
+          $dst_phid);
       }
     }
 
     foreach ($new_add as $dst_phid => $edge) {
       $result[$dst_phid] = $this->normalizeEdgeTransactionValue(
         $xaction,
-        $edge);
+        $edge,
+        $dst_phid);
     }
 
     foreach ($new_rem as $dst_phid => $edge) {
@@ -1032,18 +1035,44 @@ abstract class PhabricatorApplicationTransactionEditor
     }
   }
 
-  protected function normalizeEdgeTransactionValue(
+  private function normalizeEdgeTransactionValue(
     PhabricatorApplicationTransaction $xaction,
-    $edge) {
+    $edge,
+    $dst_phid) {
 
     if (!is_array($edge)) {
-      $edge = array(
+      if ($edge != $dst_phid) {
-        'dst' => $edge,
+        throw new Exception(
-      );
+          pht(
+            'Transaction edge data must either be the edge PHID or an edge '.
+            'specification dictionary.'));
+      }
+      $edge = array();
+    } else {
+      foreach ($edge as $key => $value) {
+        switch ($key) {
+          case 'src':
+          case 'dst':
+          case 'type':
+          case 'data':
+          case 'dateCreated':
+          case 'dateModified':
+          case 'seq':
+          case 'dataID':
+            break;
+          default:
+            throw new Exception(
+              pht(
+                'Transaction edge specification contains unexpected key '.
+                '"%s".',
+                $key));
+        }
+      }
     }
 
-    $edge_type = $xaction->getMetadataValue('edge:type');
+    $edge['dst'] = $dst_phid;
 
+    $edge_type = $xaction->getMetadataValue('edge:type');
     if (empty($edge['type'])) {
       $edge['type'] = $edge_type;
     } else {

src/infrastructure/customfield/field/PhabricatorCustomField.php

@@ -744,6 +744,17 @@ abstract class PhabricatorCustomField {
   }
 
 
+  /**
+   * @task appxaction
+   */
+  public function getApplicationTransactionMetadata() {
+    if ($this->proxy) {
+      return $this->proxy->getApplicationTransactionMetadata();
+    }
+    return array();
+  }
+
+
   /**
    * @task appxaction
    */

src/infrastructure/customfield/field/PhabricatorCustomFieldList.php

@@ -199,22 +199,32 @@ final class PhabricatorCustomFieldList extends Phobject {
         continue;
       }
 
-      $old_value = $field->getOldValueForApplicationTransactions();
-
-      $field->readValueFromRequest($request);
       $transaction_type = $field->getApplicationTransactionType();
-
       $xaction = id(clone $template)
-        ->setTransactionType($transaction_type)
+        ->setTransactionType($transaction_type);
-        ->setMetadataValue('customfield:key', $field->getFieldKey())
-        ->setNewValue($field->getNewValueForApplicationTransactions());
 
       if ($transaction_type == PhabricatorTransactions::TYPE_CUSTOMFIELD) {
         // For TYPE_CUSTOMFIELD transactions only, we provide the old value
         // as an input.
+        $old_value = $field->getOldValueForApplicationTransactions();
         $xaction->setOldValue($old_value);
       }
 
+      $field->readValueFromRequest($request);
+
+      $xaction
+        ->setNewValue($field->getNewValueForApplicationTransactions());
+
+      if ($transaction_type == PhabricatorTransactions::TYPE_CUSTOMFIELD) {
+        // For TYPE_CUSTOMFIELD transactions, add the field key in metadata.
+        $xaction->setMetadataValue('customfield:key', $field->getFieldKey());
+      }
+
+      $metadata = $field->getApplicationTransactionMetadata();
+      foreach ($metadata as $key => $value) {
+        $xaction->setMetadataValue($key, $value);
+      }
+
       $xactions[] = $xaction;
     }