Index: src/__phutil_library_map__.php =================================================================== --- src/__phutil_library_map__.php +++ src/__phutil_library_map__.php @@ -441,6 +441,7 @@ '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 @@ 'DifferentialRevertPlanFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialReviewRequestMail' => 'DifferentialMail', 'DifferentialReviewedByFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialReviewersField' => 'DifferentialCoreCustomField', 'DifferentialReviewersFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialReviewersView' => 'AphrontView', 'DifferentialRevision' => Index: src/applications/differential/customfield/DifferentialReviewersField.php =================================================================== --- /dev/null +++ src/applications/differential/customfield/DifferentialReviewersField.php @@ -0,0 +1,84 @@ +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, + ); + } + +} Index: src/applications/differential/editor/DifferentialTransactionEditor.php =================================================================== --- src/applications/differential/editor/DifferentialTransactionEditor.php +++ src/applications/differential/editor/DifferentialTransactionEditor.php @@ -6,11 +6,11 @@ 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 @@ $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 @@ case PhabricatorTransactions::TYPE_EDIT_POLICY: return; case PhabricatorTransactions::TYPE_SUBSCRIBERS: + case PhabricatorTransactions::TYPE_EDGE: return; } @@ -93,7 +97,6 @@ return $errors; } - protected function requireCapabilities( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { Index: src/applications/differential/storage/DifferentialReviewer.php =================================================================== --- src/applications/differential/storage/DifferentialReviewer.php +++ src/applications/differential/storage/DifferentialReviewer.php @@ -46,4 +46,11 @@ return $this->authority[$viewer_phid]; } + public function getEdgeData() { + return array( + 'status' => $this->status, + 'diffID' => $this->diffID, + ); + } + } Index: src/applications/differential/storage/DifferentialRevision.php =================================================================== --- src/applications/differential/storage/DifferentialRevision.php +++ src/applications/differential/storage/DifferentialRevision.php @@ -469,6 +469,7 @@ new DifferentialTitleField(), new DifferentialSummaryField(), new DifferentialTestPlanField(), + new DifferentialReviewersField(), new DifferentialSubscribersField(), new DifferentialRepositoryField(), new DifferentialViewPolicyField(), Index: src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php =================================================================== --- src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -990,21 +990,24 @@ } $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 @@ } } - protected function normalizeEdgeTransactionValue( + private function normalizeEdgeTransactionValue( PhabricatorApplicationTransaction $xaction, - $edge) { + $edge, + $dst_phid) { if (!is_array($edge)) { - $edge = array( - 'dst' => $edge, - ); + if ($edge != $dst_phid) { + 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 { Index: src/infrastructure/customfield/field/PhabricatorCustomField.php =================================================================== --- src/infrastructure/customfield/field/PhabricatorCustomField.php +++ src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -747,6 +747,17 @@ /** * @task appxaction */ + public function getApplicationTransactionMetadata() { + if ($this->proxy) { + return $this->proxy->getApplicationTransactionMetadata(); + } + return array(); + } + + + /** + * @task appxaction + */ public function getOldValueForApplicationTransactions() { if ($this->proxy) { return $this->proxy->getOldValueForApplicationTransactions(); Index: src/infrastructure/customfield/field/PhabricatorCustomFieldList.php =================================================================== --- src/infrastructure/customfield/field/PhabricatorCustomFieldList.php +++ src/infrastructure/customfield/field/PhabricatorCustomFieldList.php @@ -199,22 +199,32 @@ continue; } - $old_value = $field->getOldValueForApplicationTransactions(); - - $field->readValueFromRequest($request); $transaction_type = $field->getApplicationTransactionType(); - $xaction = id(clone $template) - ->setTransactionType($transaction_type) - ->setMetadataValue('customfield:key', $field->getFieldKey()) - ->setNewValue($field->getNewValueForApplicationTransactions()); + ->setTransactionType($transaction_type); 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; }