Page MenuHomePhabricator
Files F14808446

D20462.id48816.diff
No OneTemporary
Actions

D20462.id48816.diff
View Options

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
@@ -977,111 +977,126 @@
PhabricatorLiskDAO $object,
array $xactions) {
- $this->object = $object;
- $this->xactions = $xactions;
- $this->isNewObject = ($object->getPHID() === null);
+ $is_new = ($object->getPHID() === null);
+ $this->isNewObject = $is_new;
- $this->validateEditParameters($object, $xactions);
- $xactions = $this->newMFATransactions($object, $xactions);
+ $is_preview = $this->getIsPreview();
+ $read_locking = false;
+ $transaction_open = false;
- $actor = $this->requireActor();
+ // If we're attempting to apply transactions, lock and reload the object
+ // before we go anywhere. If we don't do this at the very beginning, we
+ // may be looking at an older version of the object when we populate and
+ // filter the transactions. See PHI1165 for an example.
- // NOTE: Some transaction expansion requires that the edited object be
- // attached.
- foreach ($xactions as $xaction) {
- $xaction->attachObject($object);
- $xaction->attachViewer($actor);
- }
+ if (!$is_preview) {
+ if (!$is_new) {
+ $this->buildOldRecipientLists($object, $xactions);
+
+ $object->openTransaction();
+ $transaction_open = true;
- $xactions = $this->expandTransactions($object, $xactions);
- $xactions = $this->expandSupportTransactions($object, $xactions);
- $xactions = $this->combineTransactions($xactions);
+ $object->beginReadLocking();
+ $read_locking = true;
- foreach ($xactions as $xaction) {
- $xaction = $this->populateTransaction($object, $xaction);
+ $object->reload();
+ }
}
- $is_preview = $this->getIsPreview();
- $read_locking = false;
- $transaction_open = false;
+ try {
+ $this->object = $object;
+ $this->xactions = $xactions;
- if (!$is_preview) {
- $errors = array();
- $type_map = mgroup($xactions, 'getTransactionType');
- foreach ($this->getTransactionTypes() as $type) {
- $type_xactions = idx($type_map, $type, array());
- $errors[] = $this->validateTransaction($object, $type, $type_xactions);
- }
+ $this->validateEditParameters($object, $xactions);
+ $xactions = $this->newMFATransactions($object, $xactions);
- $errors[] = $this->validateAllTransactions($object, $xactions);
- $errors[] = $this->validateTransactionsWithExtensions($object, $xactions);
- $errors = array_mergev($errors);
+ $actor = $this->requireActor();
- $continue_on_missing = $this->getContinueOnMissingFields();
- foreach ($errors as $key => $error) {
- if ($continue_on_missing && $error->getIsMissingFieldError()) {
- unset($errors[$key]);
- }
+ // NOTE: Some transaction expansion requires that the edited object be
+ // attached.
+ foreach ($xactions as $xaction) {
+ $xaction->attachObject($object);
+ $xaction->attachViewer($actor);
}
- if ($errors) {
- throw new PhabricatorApplicationTransactionValidationException($errors);
+ $xactions = $this->expandTransactions($object, $xactions);
+ $xactions = $this->expandSupportTransactions($object, $xactions);
+ $xactions = $this->combineTransactions($xactions);
+
+ foreach ($xactions as $xaction) {
+ $xaction = $this->populateTransaction($object, $xaction);
}
- if ($this->raiseWarnings) {
- $warnings = array();
- foreach ($xactions as $xaction) {
- if ($this->hasWarnings($object, $xaction)) {
- $warnings[] = $xaction;
- }
- }
- if ($warnings) {
- throw new PhabricatorApplicationTransactionWarningException(
- $warnings);
+ if (!$is_preview) {
+ $errors = array();
+ $type_map = mgroup($xactions, 'getTransactionType');
+ foreach ($this->getTransactionTypes() as $type) {
+ $type_xactions = idx($type_map, $type, array());
+ $errors[] = $this->validateTransaction(
+ $object,
+ $type,
+ $type_xactions);
}
- }
- }
- foreach ($xactions as $xaction) {
- $this->adjustTransactionValues($object, $xaction);
- }
+ $errors[] = $this->validateAllTransactions($object, $xactions);
+ $errors[] = $this->validateTransactionsWithExtensions(
+ $object,
+ $xactions);
+ $errors = array_mergev($errors);
- // Now that we've merged and combined transactions, check for required
- // capabilities. Note that we're doing this before filtering
- // transactions: if you try to apply an edit which you do not have
- // permission to apply, we want to give you a permissions error even
- // if the edit would have no effect.
- $this->applyCapabilityChecks($object, $xactions);
+ $continue_on_missing = $this->getContinueOnMissingFields();
+ foreach ($errors as $key => $error) {
+ if ($continue_on_missing && $error->getIsMissingFieldError()) {
+ unset($errors[$key]);
+ }
+ }
- $xactions = $this->filterTransactions($object, $xactions);
+ if ($errors) {
+ throw new PhabricatorApplicationTransactionValidationException(
+ $errors);
+ }
- if (!$is_preview) {
- $this->hasRequiredMFA = true;
- if ($this->getShouldRequireMFA()) {
- $this->requireMFA($object, $xactions);
+ if ($this->raiseWarnings) {
+ $warnings = array();
+ foreach ($xactions as $xaction) {
+ if ($this->hasWarnings($object, $xaction)) {
+ $warnings[] = $xaction;
+ }
+ }
+ if ($warnings) {
+ throw new PhabricatorApplicationTransactionWarningException(
+ $warnings);
+ }
+ }
}
- if ($object->getID()) {
- $this->buildOldRecipientLists($object, $xactions);
+ foreach ($xactions as $xaction) {
+ $this->adjustTransactionValues($object, $xaction);
+ }
- $object->openTransaction();
- $transaction_open = true;
+ // Now that we've merged and combined transactions, check for required
+ // capabilities. Note that we're doing this before filtering
+ // transactions: if you try to apply an edit which you do not have
+ // permission to apply, we want to give you a permissions error even
+ // if the edit would have no effect.
+ $this->applyCapabilityChecks($object, $xactions);
- $object->beginReadLocking();
- $read_locking = true;
+ $xactions = $this->filterTransactions($object, $xactions);
- $object->reload();
- }
+ if (!$is_preview) {
+ $this->hasRequiredMFA = true;
+ if ($this->getShouldRequireMFA()) {
+ $this->requireMFA($object, $xactions);
+ }
- if ($this->shouldApplyInitialEffects($object, $xactions)) {
- if (!$transaction_open) {
- $object->openTransaction();
- $transaction_open = true;
+ if ($this->shouldApplyInitialEffects($object, $xactions)) {
+ if (!$transaction_open) {
+ $object->openTransaction();
+ $transaction_open = true;
+ }
}
}
- }
- try {
if ($this->shouldApplyInitialEffects($object, $xactions)) {
$this->applyInitialEffects($object, $xactions);
}

File Metadata

Mime Type
text/plain
Expires
Mon, Jan 27, 3:21 PM (4 h, 12 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7060919
Default Alt Text
D20462.id48816.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility