diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1588,11 +1588,8 @@ ->setAuthorPHID($author_phid) ->setTransactionType( DifferentialRevisionRequestReviewTransaction::TRANSACTIONTYPE) - ->setOldValue(false) ->setNewValue(true); - $xaction = $this->populateTransaction($object, $xaction); - // If we're creating this revision and immediately moving it out of // the draft state, mark this as a create transaction so it gets // hidden in the timeline and mail, since it isn't interesting: it @@ -1601,15 +1598,10 @@ $xaction->setIsCreateTransaction(true); } - $object->openTransaction(); - $object - ->setStatus(DifferentialRevisionStatus::NEEDS_REVIEW) - ->save(); - - $xaction->save(); - $object->saveTransaction(); - - $xactions[] = $xaction; + // Queue this transaction and apply it separately after the current + // batch of transactions finishes so that Herald can fire on the new + // revision state. See T13027 for discussion. + $this->queueTransaction($xaction); } } diff --git a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php --- a/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php @@ -57,11 +57,15 @@ 'revisions.')); } - if (!$this->isViewerRevisionAuthor($object, $viewer)) { - throw new Exception( - pht( - 'You can not request review of this revision because you are not '. - 'the author of the revision.')); + // When revisions automatically promote out of "Draft" after builds finish, + // the viewer may be acting as the Harbormaster application. + if (!$viewer->isOmnipotent()) { + if (!$this->isViewerRevisionAuthor($object, $viewer)) { + throw new Exception( + pht( + 'You can not request review of this revision because you are not '. + 'the author of the revision.')); + } } } 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 @@ -70,6 +70,8 @@ private $feedRelatedPHIDs = array(); private $modularTypes; + private $transactionQueue = array(); + const STORAGE_ENCODING_BINARY = 'binary'; /** @@ -1174,6 +1176,8 @@ 'priority' => PhabricatorWorker::PRIORITY_ALERTS, )); + $this->flushTransactionQueue($object); + return $xactions; } @@ -3864,4 +3868,39 @@ return pht('%s created an object: %s.', $author, $object); } +/* -( Queue )-------------------------------------------------------------- */ + + protected function queueTransaction( + PhabricatorApplicationTransaction $xaction) { + $this->transactionQueue[] = $xaction; + return $this; + } + + private function flushTransactionQueue($object) { + if (!$this->transactionQueue) { + return; + } + + $xactions = $this->transactionQueue; + $this->transactionQueue = array(); + + $editor = $this->newQueueEditor(); + + return $editor->applyTransactions($object, $xactions); + } + + private function newQueueEditor() { + $editor = id(newv(get_class($this), array())) + ->setActor($this->getActor()) + ->setContentSource($this->getContentSource()) + ->setContinueOnNoEffect($this->getContinueOnNoEffect()) + ->setContinueOnMissingFields($this->getContinueOnMissingFields()); + + if ($this->actingAsPHID !== null) { + $editor->setActingAsPHID($this->actingAsPHID); + } + + return $editor; + } + }