From 49eaa9f8fe4cb7ed0976293745d65749f6e891df Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 7 Mar 2014 08:10:18 -0800 Subject: [PATCH] Use TransactionEditor in Differential mail handling Summary: Ref T2222. Moves this instance of CommentEditor to TransactionEditor. Test Plan: Used `bin/mail receive-test` to test receiving comment mail and action mail. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2222 Differential Revision: https://secure.phabricator.com/D8427 --- .../mail/DifferentialReplyHandler.php | 62 ++++++++++++------- .../mail/DifferentialRevisionMailReceiver.php | 1 + 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/applications/differential/mail/DifferentialReplyHandler.php b/src/applications/differential/mail/DifferentialReplyHandler.php index 2e3b47327c..a8ed6fcb30 100644 --- a/src/applications/differential/mail/DifferentialReplyHandler.php +++ b/src/applications/differential/mail/DifferentialReplyHandler.php @@ -129,28 +129,48 @@ class DifferentialReplyHandler extends PhabricatorMailReplyHandler { $body = $this->enhanceBodyWithAttachments($body, $attachments); - try { - $editor = new DifferentialCommentEditor( - $this->getMailReceiver(), - $command); - $editor->setActor($actor); - $editor->setExcludeMailRecipientPHIDs( - $this->getExcludeMailRecipientPHIDs()); + $xactions = array(); - // NOTE: We have to be careful about this because Facebook's - // implementation jumps straight into handleAction() and will not have - // a PhabricatorMetaMTAReceivedMail object. - if ($this->receivedMail) { - $content_source = PhabricatorContentSource::newForSource( - PhabricatorContentSource::SOURCE_EMAIL, - array( - 'id' => $this->receivedMail->getID(), - )); - $editor->setContentSource($content_source); - $editor->setParentMessageID($this->receivedMail->getMessageID()); - } - $editor->setMessage($body); - $editor->save(); + if ($command && ($command != DifferentialAction::ACTION_COMMENT)) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType(DifferentialTransaction::TYPE_ACTION) + ->setNewValue($command); + } + + if (strlen($body)) { + $xactions[] = id(new DifferentialTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new DifferentialTransactionComment()) + ->setContent($body)); + } + + $editor = id(new DifferentialTransactionEditor()) + ->setActor($actor) + ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) + ->setContinueOnMissingFields(true) + ->setContinueOnNoEffect(true); + + // NOTE: We have to be careful about this because Facebook's + // implementation jumps straight into handleAction() and will not have + // a PhabricatorMetaMTAReceivedMail object. + if ($this->receivedMail) { + $content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_EMAIL, + array( + 'id' => $this->receivedMail->getID(), + )); + $editor->setContentSource($content_source); + $editor->setParentMessageID($this->receivedMail->getMessageID()); + } else { + $content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_LEGACY, + array()); + $editor->setContentSource($content_source); + } + + try { + $editor->applyTransactions($this->getMailReceiver(), $xactions); } catch (Exception $ex) { if ($this->receivedMail) { $error_body = $this->receivedMail->getRawTextBody(); diff --git a/src/applications/differential/mail/DifferentialRevisionMailReceiver.php b/src/applications/differential/mail/DifferentialRevisionMailReceiver.php index 87f63a615e..0568cf186c 100644 --- a/src/applications/differential/mail/DifferentialRevisionMailReceiver.php +++ b/src/applications/differential/mail/DifferentialRevisionMailReceiver.php @@ -18,6 +18,7 @@ final class DifferentialRevisionMailReceiver $results = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withIDs(array($id)) + ->needReviewerStatus(true) ->execute(); return head($results);