Page MenuHomePhabricator
Files F14056862

D8401.diff
No OneTemporary
Actions

D8401.diff
View Options

Index: src/__phutil_library_map__.php
===================================================================
--- src/__phutil_library_map__.php
+++ src/__phutil_library_map__.php
@@ -1196,6 +1196,7 @@
'PhabricatorApplicationTransactionPHIDTypeTransaction' => 'applications/transactions/phid/PhabricatorApplicationTransactionPHIDTypeTransaction.php',
'PhabricatorApplicationTransactionQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionQuery.php',
'PhabricatorApplicationTransactionResponse' => 'applications/transactions/response/PhabricatorApplicationTransactionResponse.php',
+ 'PhabricatorApplicationTransactionStructureException' => 'applications/transactions/exception/PhabricatorApplicationTransactionStructureException.php',
'PhabricatorApplicationTransactionTextDiffDetailView' => 'applications/transactions/view/PhabricatorApplicationTransactionTextDiffDetailView.php',
'PhabricatorApplicationTransactionValidationError' => 'applications/transactions/error/PhabricatorApplicationTransactionValidationError.php',
'PhabricatorApplicationTransactionValidationException' => 'applications/transactions/exception/PhabricatorApplicationTransactionValidationException.php',
@@ -3895,6 +3896,7 @@
'PhabricatorApplicationTransactionPHIDTypeTransaction' => 'PhabricatorPHIDType',
'PhabricatorApplicationTransactionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorApplicationTransactionResponse' => 'AphrontProxyResponse',
+ 'PhabricatorApplicationTransactionStructureException' => 'Exception',
'PhabricatorApplicationTransactionTextDiffDetailView' => 'AphrontView',
'PhabricatorApplicationTransactionValidationError' => 'Phobject',
'PhabricatorApplicationTransactionValidationException' => 'Exception',
Index: src/applications/maniphest/storage/ManiphestTransaction.php
===================================================================
--- src/applications/maniphest/storage/ManiphestTransaction.php
+++ src/applications/maniphest/storage/ManiphestTransaction.php
@@ -28,17 +28,13 @@
}
public function shouldGenerateOldValue() {
- $generate = true;
switch ($this->getTransactionType()) {
case self::TYPE_PROJECT_COLUMN:
case self::TYPE_EDGE:
- $generate = false;
- break;
- default:
- $generate = true;
- break;
+ return false;
}
- return $generate;
+
+ return parent::shouldGenerateOldValue();
}
public function getRequiredHandlePHIDs() {
Index: src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
===================================================================
--- src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -122,8 +122,11 @@
private function adjustTransactionValues(
PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) {
- $old = $this->getTransactionOldValue($object, $xaction);
- $xaction->setOldValue($old);
+
+ if ($xaction->shouldGenerateOldValue()) {
+ $old = $this->getTransactionOldValue($object, $xaction);
+ $xaction->setOldValue($old);
+ }
$new = $this->getTransactionNewValue($object, $xaction);
$xaction->setNewValue($new);
@@ -723,37 +726,61 @@
assert_instances_of($xactions, 'PhabricatorApplicationTransaction');
foreach ($xactions as $xaction) {
if ($xaction->getPHID() || $xaction->getID()) {
- throw new Exception(
- "You can not apply transactions which already have IDs/PHIDs!");
+ throw new PhabricatorApplicationTransactionStructureException(
+ $xaction,
+ pht(
+ "You can not apply transactions which already have IDs/PHIDs!"));
}
if ($xaction->getObjectPHID()) {
- throw new Exception(
- "You can not apply transactions which already have objectPHIDs!");
+ throw new PhabricatorApplicationTransactionStructureException(
+ $xaction,
+ pht(
+ "You can not apply transactions which already have objectPHIDs!"));
}
if ($xaction->getAuthorPHID()) {
- throw new Exception(
- "You can not apply transactions which already have authorPHIDs!");
+ throw new PhabricatorApplicationTransactionStructureException(
+ $xaction,
+ pht(
+ 'You can not apply transactions which already have authorPHIDs!'));
}
if ($xaction->getCommentPHID()) {
- throw new Exception(
- "You can not apply transactions which already have commentPHIDs!");
+ throw new PhabricatorApplicationTransactionStructureException(
+ $xaction,
+ pht(
+ 'You can not apply transactions which already have '.
+ 'commentPHIDs!'));
}
if ($xaction->getCommentVersion() !== 0) {
- throw new Exception(
- "You can not apply transactions which already have commentVersions!");
+ throw new PhabricatorApplicationTransactionStructureException(
+ $xaction,
+ pht(
+ 'You can not apply transactions which already have '.
+ 'commentVersions!'));
}
- if (!$xaction->shouldGenerateOldValue()) {
- if ($xaction->getOldValue() === null) {
- throw new Exception(
- 'You can not apply transactions which should already have '.
- 'oldValue but do not!');
- }
+ $expect_value = !$xaction->shouldGenerateOldValue();
+ $has_value = $xaction->hasOldValue();
+
+ if ($expect_value && !$has_value) {
+ throw new PhabricatorApplicationTransactionStructureException(
+ $xaction,
+ pht(
+ 'This transaction is supposed to have an oldValue set, but '.
+ 'it does not!'));
+ }
+
+ if ($has_value && !$expect_value) {
+ throw new PhabricatorApplicationTransactionStructureException(
+ $xaction,
+ pht(
+ 'This transaction should generate its oldValue automatically, '.
+ 'but has already had one set!'));
}
$type = $xaction->getTransactionType();
if (empty($types[$type])) {
- throw new Exception(
+ throw new PhabricatorApplicationTransactionStructureException(
+ $xaction,
pht(
'Transaction has type "%s", but that transaction type is not '.
'supported by this editor (%s).',
Index: src/applications/transactions/exception/PhabricatorApplicationTransactionStructureException.php
===================================================================
--- /dev/null
+++ src/applications/transactions/exception/PhabricatorApplicationTransactionStructureException.php
@@ -0,0 +1,20 @@
+<?php
+
+final class PhabricatorApplicationTransactionStructureException
+ extends Exception {
+
+ public function __construct(
+ PhabricatorApplicationTransaction $xaction,
+ $message) {
+
+ $full_message = pht(
+ 'Attempting to apply a transaction (of class "%s", with type "%s") '.
+ 'which has not been constructed correctly: %s',
+ get_class($xaction),
+ $xaction->getTransactionType(),
+ $message);
+
+ parent::__construct($full_message);
+ }
+
+}
Index: src/applications/transactions/storage/PhabricatorApplicationTransaction.php
===================================================================
--- src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -30,6 +30,7 @@
private $transactionGroup = array();
private $viewer = self::ATTACHABLE;
private $object = self::ATTACHABLE;
+ private $oldValueHasBeenSet = false;
private $ignoreOnNoEffect;
@@ -169,6 +170,16 @@
return $blocks;
}
+ public function setOldValue($value) {
+ $this->oldValueHasBeenSet = true;
+ $this->writeField('oldValue', $value);
+ return $this;
+ }
+
+ public function hasOldValue() {
+ return $this->oldValueHasBeenSet;
+ }
+
/* -( Rendering )---------------------------------------------------------- */

File Metadata

Mime Type
text/plain
Expires
Sun, Nov 17, 10:52 PM (1 d, 18 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6717552
Default Alt Text
D8401.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility