Page MenuHomePhabricator
Files F13993954

D8452.id20057.diff
No OneTemporary
Actions

D8452.id20057.diff
View Options

diff --git a/src/applications/differential/conduit/ConduitAPI_differential_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_Method.php
--- a/src/applications/differential/conduit/ConduitAPI_differential_Method.php
+++ b/src/applications/differential/conduit/ConduitAPI_differential_Method.php
@@ -43,5 +43,109 @@
);
}
+ protected function applyFieldEdit(
+ ConduitAPIRequest $request,
+ DifferentialRevision $revision,
+ DifferentialDiff $diff,
+ array $fields,
+ $message) {
+
+ $viewer = $request->getUser();
+
+ $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 = id(new DifferentialTransactionEditor())
+ ->setActor($viewer)
+ ->setContentSourceFromConduitRequest($request)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true);
+
+ $editor->applyTransactions($revision, $xactions);
+ }
}
diff --git a/src/applications/differential/conduit/ConduitAPI_differential_createrevision_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_createrevision_Method.php
--- a/src/applications/differential/conduit/ConduitAPI_differential_createrevision_Method.php
+++ b/src/applications/differential/conduit/ConduitAPI_differential_createrevision_Method.php
@@ -1,13 +1,10 @@
<?php
-/**
- * @group conduit
- */
final class ConduitAPI_differential_createrevision_Method
- extends ConduitAPIMethod {
+ extends ConduitAPI_differential_Method {
public function getMethodDescription() {
- return "Create a new Differential revision.";
+ return pht("Create a new Differential revision.");
}
public function defineParamTypes() {
@@ -31,20 +28,25 @@
}
protected function execute(ConduitAPIRequest $request) {
- $fields = $request->getValue('fields');
+ $viewer = $request->getUser();
$diff = id(new DifferentialDiffQuery())
- ->setViewer($request->getUser())
+ ->setViewer($viewer)
->withIDs(array($request->getValue('diffid')))
->executeOne();
if (!$diff) {
throw new ConduitException('ERR_BAD_DIFF');
}
- $revision = DifferentialRevisionEditor::newRevisionFromConduitWithDiff(
- $fields,
+ $revision = DifferentialRevision::initializeNewRevision($viewer);
+ $revision->attachReviewerStatus(array());
+
+ $this->applyFieldEdit(
+ $request,
+ $revision,
$diff,
- $request->getUser());
+ $request->getValue('fields', array()),
+ $message = null);
return array(
'revisionid' => $revision->getID(),
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,7 +1,7 @@
<?php
final class ConduitAPI_differential_updaterevision_Method
- extends ConduitAPIMethod {
+ extends ConduitAPI_differential_Method {
public function getMethodDescription() {
return pht("Update a Differential revision.");
@@ -59,100 +59,12 @@
throw new ConduitException('ERR_CLOSED');
}
- $field_list = PhabricatorCustomField::getObjectFields(
+ $this->applyFieldEdit(
+ $request,
$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 = id(new DifferentialTransactionEditor())
- ->setActor($viewer)
- ->setContentSourceFromConduitRequest($request)
- ->setContinueOnNoEffect(true)
- ->setContinueOnMissingFields(true);
-
- $editor->applyTransactions($revision, $xactions);
+ $diff,
+ $request->getValue('fields', array()),
+ $request->getValue('message'));
return array(
'revisionid' => $revision->getID(),
diff --git a/src/applications/differential/customfield/DifferentialSummaryField.php b/src/applications/differential/customfield/DifferentialSummaryField.php
--- a/src/applications/differential/customfield/DifferentialSummaryField.php
+++ b/src/applications/differential/customfield/DifferentialSummaryField.php
@@ -21,6 +21,9 @@
protected function readValueFromRevision(
DifferentialRevision $revision) {
+ if (!$revision->getID()) {
+ return null;
+ }
return $revision->getSummary();
}
@@ -82,6 +85,11 @@
$xaction->getNewValue());
}
+ public function shouldHideInApplicationTransactions(
+ PhabricatorApplicationTransaction $xaction) {
+ return ($xaction->getOldValue() === null);
+ }
+
public function shouldAppearInGlobalSearch() {
return true;
}
diff --git a/src/applications/differential/customfield/DifferentialTestPlanField.php b/src/applications/differential/customfield/DifferentialTestPlanField.php
--- a/src/applications/differential/customfield/DifferentialTestPlanField.php
+++ b/src/applications/differential/customfield/DifferentialTestPlanField.php
@@ -21,6 +21,9 @@
protected function readValueFromRevision(
DifferentialRevision $revision) {
+ if (!$revision->getID()) {
+ return null;
+ }
return $revision->getTestPlan();
}
@@ -96,6 +99,10 @@
$xaction->getNewValue());
}
+ public function shouldHideInApplicationTransactions(
+ PhabricatorApplicationTransaction $xaction) {
+ return ($xaction->getOldValue() === null);
+ }
public function shouldAppearInGlobalSearch() {
return true;
diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php
--- a/src/applications/differential/storage/DifferentialTransaction.php
+++ b/src/applications/differential/storage/DifferentialTransaction.php
@@ -20,10 +20,23 @@
}
public function shouldHide() {
+ $old = $this->getOldValue();
+ $new = $this->getNewValue();
+
switch ($this->getTransactionType()) {
+ case self::TYPE_UPDATE:
+ // Older versions of this transaction have an ID for the new value,
+ // and/or do not record the old value. Only hide the transaction if
+ // the new value is a PHID, indicating that this is a newer style
+ // transaction.
+ if ($old === null) {
+ if (phid_get_type($new) == DifferentialPHIDTypeDiff::TYPECONST) {
+ return true;
+ }
+ }
+ break;
+
case PhabricatorTransactions::TYPE_EDGE:
- $old = $this->getOldValue();
- $new = $this->getNewValue();
$add = array_diff_key($new, $old);
$rem = array_diff_key($old, $new);
@@ -38,7 +51,7 @@
break;
}
- return false;
+ return parent::shouldHide();
}
public function shouldHideForMail(array $xactions) {
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -356,6 +356,11 @@
return false;
}
break;
+ case PhabricatorTransactions::TYPE_CUSTOMFIELD:
+ $field = $this->getTransactionCustomField();
+ if ($field) {
+ return $field->shouldHideInApplicationTransactions($this);
+ }
}
return false;
diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php
--- a/src/infrastructure/customfield/field/PhabricatorCustomField.php
+++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php
@@ -991,6 +991,14 @@
return array();
}
+ public function shouldHideInApplicationTransactions(
+ PhabricatorApplicationTransaction $xaction) {
+ if ($this->proxy) {
+ return $this->proxy->shouldHideInApplicationTransactions($xaction);
+ }
+ return false;
+ }
+
/* -( Edit View )---------------------------------------------------------- */

File Metadata

Mime Type
text/plain
Expires
Thu, Oct 24, 2:38 AM (3 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6745693
Default Alt Text
D8452.id20057.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility