mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-19 13:22:42 +01:00
Use CustomField for differential.updaterevision
Summary: Ref T2222. Ref T3794. Medium term goal is to remove `DifferentialRevisionEditor`. This removes one of two callsites. Test Plan: Used `arc diff --edit` to repeatedly update a revision, making changes to various fields. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T3794, T2222 Differential Revision: https://secure.phabricator.com/D8451
This commit is contained in:
parent
6dd191a3c1
commit
d8968755e9
3 changed files with 135 additions and 24 deletions
|
@ -1,13 +1,10 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* @group conduit
|
||||
*/
|
||||
final class ConduitAPI_differential_updaterevision_Method
|
||||
extends ConduitAPIMethod {
|
||||
|
||||
public function getMethodDescription() {
|
||||
return "Update a Differential revision.";
|
||||
return pht("Update a Differential revision.");
|
||||
}
|
||||
|
||||
public function defineParamTypes() {
|
||||
|
@ -33,8 +30,10 @@ final class ConduitAPI_differential_updaterevision_Method
|
|||
}
|
||||
|
||||
protected function execute(ConduitAPIRequest $request) {
|
||||
$viewer = $request->getUser();
|
||||
|
||||
$diff = id(new DifferentialDiffQuery())
|
||||
->setViewer($request->getUser())
|
||||
->setViewer($viewer)
|
||||
->withIDs(array($request->getValue('diffid')))
|
||||
->executeOne();
|
||||
if (!$diff) {
|
||||
|
@ -44,32 +43,116 @@ final class ConduitAPI_differential_updaterevision_Method
|
|||
$revision = id(new DifferentialRevisionQuery())
|
||||
->setViewer($request->getUser())
|
||||
->withIDs(array($request->getValue('id')))
|
||||
->needReviewerStatus(true)
|
||||
->needActiveDiffs(true)
|
||||
->requireCapabilities(
|
||||
array(
|
||||
PhabricatorPolicyCapability::CAN_VIEW,
|
||||
PhabricatorPolicyCapability::CAN_EDIT,
|
||||
))
|
||||
->executeOne();
|
||||
if (!$revision) {
|
||||
throw new ConduitException('ERR_BAD_REVISION');
|
||||
}
|
||||
|
||||
if ($request->getUser()->getPHID() !== $revision->getAuthorPHID()) {
|
||||
throw new ConduitException('ERR_WRONG_USER');
|
||||
}
|
||||
|
||||
if ($revision->getStatus() == ArcanistDifferentialRevisionStatus::CLOSED) {
|
||||
throw new ConduitException('ERR_CLOSED');
|
||||
}
|
||||
|
||||
$content_source = PhabricatorContentSource::newForSource(
|
||||
PhabricatorContentSource::SOURCE_CONDUIT,
|
||||
array());
|
||||
$field_list = PhabricatorCustomField::getObjectFields(
|
||||
$revision,
|
||||
DifferentialCustomField::ROLE_COMMITMESSAGEEDIT);
|
||||
|
||||
$editor = new DifferentialRevisionEditor(
|
||||
$revision);
|
||||
$editor->setActor($request->getUser());
|
||||
$editor->setContentSource($content_source);
|
||||
$fields = $request->getValue('fields');
|
||||
$editor->addDiff($diff, $request->getValue('message'));
|
||||
$editor->copyFieldsFromConduit($fields);
|
||||
$field_list
|
||||
->setViewer($viewer)
|
||||
->readFieldsFromStorage($revision);
|
||||
$field_map = mpull($field_list->getFields(), null, 'getFieldKeyForConduit');
|
||||
|
||||
$editor->save();
|
||||
$xactions = array();
|
||||
|
||||
$xactions[] = id(new DifferentialTransaction())
|
||||
->setTransactionType(DifferentialTransaction::TYPE_UPDATE)
|
||||
->setNewValue($diff->getPHID());
|
||||
|
||||
$values = $request->getValue('fields', array());
|
||||
foreach ($values as $key => $value) {
|
||||
$field = idx($field_map, $key);
|
||||
if (!$field) {
|
||||
// NOTE: We're just ignoring fields we don't know about. This isn't
|
||||
// ideal, but the way the workflow currently works involves us getting
|
||||
// several read-only fields, like the revision ID field, which we should
|
||||
// just skip.
|
||||
continue;
|
||||
}
|
||||
|
||||
$role = PhabricatorCustomField::ROLE_APPLICATIONTRANSACTIONS;
|
||||
if (!$field->shouldEnableForRole($role)) {
|
||||
throw new Exception(
|
||||
pht(
|
||||
'Request attempts to update field "%s", but that field can not '.
|
||||
'perform transactional updates.',
|
||||
$key));
|
||||
}
|
||||
|
||||
// TODO: This is fairly similar to PhabricatorCustomField's
|
||||
// buildFieldTransactionsFromRequest() method, but that's currently not
|
||||
// easy to reuse.
|
||||
|
||||
$transaction_type = $field->getApplicationTransactionType();
|
||||
$xaction = id(new DifferentialTransaction())
|
||||
->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);
|
||||
}
|
||||
|
||||
// The transaction itself will be validated so this is somewhat
|
||||
// redundant, but this validator will sometimes give us a better error
|
||||
// message or a better reaction to a bad value type.
|
||||
$field->validateCommitMessageValue($value);
|
||||
$field->readValueFromCommitMessage($value);
|
||||
|
||||
$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 $meta_key => $meta_value) {
|
||||
$xaction->setMetadataValue($meta_key, $meta_value);
|
||||
}
|
||||
|
||||
$xactions[] = $xaction;
|
||||
}
|
||||
|
||||
$message = $request->getValue('message');
|
||||
if (strlen($message)) {
|
||||
// This is a little awkward, and should maybe move inside the transaction
|
||||
// editor. It largely exists for legacy reasons.
|
||||
$first_line = head(phutil_split_lines($message, false));
|
||||
$diff->setDescription($first_line);
|
||||
$diff->save();
|
||||
|
||||
$xactions[] = id(new DifferentialTransaction())
|
||||
->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
|
||||
->attachComment(
|
||||
id(new DifferentialTransactionComment())
|
||||
->setContent($message));
|
||||
}
|
||||
|
||||
$editor = id(new DifferentialTransactionEditor())
|
||||
->setActor($viewer)
|
||||
->setContentSourceFromConduitRequest($request)
|
||||
->setContinueOnNoEffect(true)
|
||||
->setContinueOnMissingFields(true);
|
||||
|
||||
$editor->applyTransactions($revision, $xactions);
|
||||
|
||||
return array(
|
||||
'revisionid' => $revision->getID(),
|
||||
|
|
|
@ -1,7 +1,7 @@
|
|||
<?php
|
||||
|
||||
final class DifferentialManiphestTasksField
|
||||
extends DifferentialCustomField {
|
||||
extends DifferentialCoreCustomField {
|
||||
|
||||
public function getFieldKey() {
|
||||
return 'differential:maniphest-tasks';
|
||||
|
@ -15,6 +15,10 @@ final class DifferentialManiphestTasksField
|
|||
return false;
|
||||
}
|
||||
|
||||
public function shouldAppearInEditView() {
|
||||
return false;
|
||||
}
|
||||
|
||||
public function getFieldName() {
|
||||
return pht('Maniphest Tasks');
|
||||
}
|
||||
|
@ -31,16 +35,39 @@ final class DifferentialManiphestTasksField
|
|||
return $this->getFieldName();
|
||||
}
|
||||
|
||||
public function getRequiredHandlePHIDsForPropertyView() {
|
||||
if (!$this->getObject()->getPHID()) {
|
||||
public function readValueFromRevision(DifferentialRevision $revision) {
|
||||
if (!$revision->getPHID()) {
|
||||
return array();
|
||||
}
|
||||
|
||||
return PhabricatorEdgeQuery::loadDestinationPHIDs(
|
||||
$this->getObject()->getPHID(),
|
||||
$revision->getPHID(),
|
||||
PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK);
|
||||
}
|
||||
|
||||
public function getApplicationTransactionType() {
|
||||
return PhabricatorTransactions::TYPE_EDGE;
|
||||
}
|
||||
|
||||
public function getApplicationTransactionMetadata() {
|
||||
return array(
|
||||
'edge:type' => PhabricatorEdgeConfig::TYPE_DREV_HAS_RELATED_TASK,
|
||||
);
|
||||
}
|
||||
|
||||
public function getNewValueForApplicationTransactions() {
|
||||
$edges = array();
|
||||
foreach ($this->getValue() as $phid) {
|
||||
$edges[$phid] = $phid;
|
||||
}
|
||||
|
||||
return array('=' => $edges);
|
||||
}
|
||||
|
||||
public function getRequiredHandlePHIDsForPropertyView() {
|
||||
return $this->getValue();
|
||||
}
|
||||
|
||||
public function renderPropertyViewValue(array $handles) {
|
||||
return $this->renderHandleList($handles);
|
||||
}
|
||||
|
|
|
@ -1496,6 +1496,7 @@ abstract class PhabricatorApplicationTransactionEditor
|
|||
$field_list = PhabricatorCustomField::getObjectFields(
|
||||
$object,
|
||||
PhabricatorCustomField::ROLE_EDIT);
|
||||
$field_list->setViewer($this->getActor());
|
||||
|
||||
$role_xactions = PhabricatorCustomField::ROLE_APPLICATIONTRANSACTIONS;
|
||||
foreach ($field_list->getFields() as $field) {
|
||||
|
|
Loading…
Reference in a new issue