Page MenuHomePhabricator
Files F14028659

D8451.diff
No OneTemporary
Actions

D8451.diff
View Options

diff --git a/src/applications/differential/conduit/ConduitAPI_differential_updaterevision_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_updaterevision_Method.php
--- a/src/applications/differential/conduit/ConduitAPI_differential_updaterevision_Method.php
+++ b/src/applications/differential/conduit/ConduitAPI_differential_updaterevision_Method.php
@@ -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 @@
}
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 @@
$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);
+
+ $field_list
+ ->setViewer($viewer)
+ ->readFieldsFromStorage($revision);
+ $field_map = mpull($field_list->getFields(), null, 'getFieldKeyForConduit');
+
+ $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 = new DifferentialRevisionEditor(
- $revision);
- $editor->setActor($request->getUser());
- $editor->setContentSource($content_source);
- $fields = $request->getValue('fields');
- $editor->addDiff($diff, $request->getValue('message'));
- $editor->copyFieldsFromConduit($fields);
+ $editor = id(new DifferentialTransactionEditor())
+ ->setActor($viewer)
+ ->setContentSourceFromConduitRequest($request)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true);
- $editor->save();
+ $editor->applyTransactions($revision, $xactions);
return array(
'revisionid' => $revision->getID(),
diff --git a/src/applications/differential/customfield/DifferentialManiphestTasksField.php b/src/applications/differential/customfield/DifferentialManiphestTasksField.php
--- a/src/applications/differential/customfield/DifferentialManiphestTasksField.php
+++ b/src/applications/differential/customfield/DifferentialManiphestTasksField.php
@@ -1,7 +1,7 @@
<?php
final class DifferentialManiphestTasksField
- extends DifferentialCustomField {
+ extends DifferentialCoreCustomField {
public function getFieldKey() {
return 'differential:maniphest-tasks';
@@ -15,6 +15,10 @@
return false;
}
+ public function shouldAppearInEditView() {
+ return false;
+ }
+
public function getFieldName() {
return pht('Maniphest Tasks');
}
@@ -31,16 +35,39 @@
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);
}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1496,6 +1496,7 @@
$field_list = PhabricatorCustomField::getObjectFields(
$object,
PhabricatorCustomField::ROLE_EDIT);
+ $field_list->setViewer($this->getActor());
$role_xactions = PhabricatorCustomField::ROLE_APPLICATIONTRANSACTIONS;
foreach ($field_list->getFields() as $field) {

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 9, 3:05 PM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6710525
Default Alt Text
D8451.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility