diff --git a/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php b/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php --- a/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php +++ b/src/applications/maniphest/mail/ManiphestTaskMailReceiver.php @@ -14,14 +14,12 @@ protected function loadObject($pattern, PhabricatorUser $viewer) { $id = (int)trim($pattern, 'T'); - $results = id(new ManiphestTaskQuery()) + return id(new ManiphestTaskQuery()) ->setViewer($viewer) ->withIDs(array($id)) ->needSubscriberPHIDs(true) ->needProjectPHIDs(true) - ->execute(); - - return head($results); + ->executeOne(); } protected function getTransactionReplyHandler() { diff --git a/src/applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php b/src/applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php --- a/src/applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php +++ b/src/applications/metamta/receiver/__tests__/PhabricatorObjectMailReceiverTestCase.php @@ -23,14 +23,16 @@ $mail->getStatus()); } -/* - - TODO: Tasks don't support policies yet. Implement this once they do. - public function testDropPolicyViolationMail() { - list($task, $user, $mail) = $this->buildMail('public'); + list($task, $user, $mail) = $this->buildMail('policy'); - // TODO: Set task policy to "no one" here. + $task + ->setViewPolicy(PhabricatorPolicies::POLICY_NOONE) + ->setOwnerPHID(null) + ->save(); + + $env = PhabricatorEnv::beginScopedEnv(); + $env->overrideEnvConfig('metamta.public-replies', true); $mail->save(); $mail->processReceivedMail(); @@ -40,8 +42,6 @@ $mail->getStatus()); } -*/ - public function testDropInvalidObjectMail() { list($task, $user, $mail) = $this->buildMail('404'); @@ -122,6 +122,11 @@ 'To' => $to, )); + $mail->setBodies( + array( + 'text' => 'test', + )); + return array($task, $user, $mail); } diff --git a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php --- a/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php +++ b/src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php @@ -67,40 +67,9 @@ PhabricatorMetaMTAReceivedMail $mail); public function processEmail(PhabricatorMetaMTAReceivedMail $mail) { - $this->dropEmptyMail($mail); - return $this->receiveEmail($mail); } - private function dropEmptyMail(PhabricatorMetaMTAReceivedMail $mail) { - $body = $mail->getCleanTextBody(); - $attachments = $mail->getAttachments(); - - if (strlen($body) || $attachments) { - return; - } - - // Only send an error email if the user is talking to just Phabricator. - // We can assume if there is only one "To" address it is a Phabricator - // address since this code is running and everything. - $is_direct_mail = (count($mail->getToAddresses()) == 1) && - (count($mail->getCCAddresses()) == 0); - - if ($is_direct_mail) { - $status_code = MetaMTAReceivedMailStatus::STATUS_EMPTY; - } else { - $status_code = MetaMTAReceivedMailStatus::STATUS_EMPTY_IGNORED; - } - - throw new PhabricatorMetaMTAReceivedMailProcessingException( - $status_code, - pht( - 'Your message does not contain any body text or attachments, so '. - 'Phabricator can not do anything useful with it. Make sure comment '. - 'text appears at the top of your message: quoted replies, inline '. - 'text, and signatures are discarded and ignored.')); - } - public function supportsPrivateReplies() { return (bool)$this->getReplyHandlerDomain() && !$this->supportsPublicReplies(); diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php @@ -109,6 +109,7 @@ try { $this->dropMailFromPhabricator(); $this->dropMailAlreadyReceived(); + $this->dropEmptyMail(); $receiver = $this->loadReceiver(); $sender = $receiver->loadSender($this); @@ -260,6 +261,34 @@ $message); } + private function dropEmptyMail() { + $body = $this->getCleanTextBody(); + $attachments = $this->getAttachments(); + + if (strlen($body) || $attachments) { + return; + } + + // Only send an error email if the user is talking to just Phabricator. + // We can assume if there is only one "To" address it is a Phabricator + // address since this code is running and everything. + $is_direct_mail = (count($this->getToAddresses()) == 1) && + (count($this->getCCAddresses()) == 0); + + if ($is_direct_mail) { + $status_code = MetaMTAReceivedMailStatus::STATUS_EMPTY; + } else { + $status_code = MetaMTAReceivedMailStatus::STATUS_EMPTY_IGNORED; + } + + throw new PhabricatorMetaMTAReceivedMailProcessingException( + $status_code, + pht( + 'Your message does not contain any body text or attachments, so '. + 'Phabricator can not do anything useful with it. Make sure comment '. + 'text appears at the top of your message: quoted replies, inline '. + 'text, and signatures are discarded and ignored.')); + } /** * Load a concrete instance of the @{class:PhabricatorMailReceiver} which diff --git a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAReceivedMailTestCase.php b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAReceivedMailTestCase.php --- a/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAReceivedMailTestCase.php +++ b/src/applications/metamta/storage/__tests__/PhabricatorMetaMTAReceivedMailTestCase.php @@ -54,6 +54,10 @@ 'Message-ID' => 'test@example.com', 'To' => 'does+not+exist@example.com', )); + $mail->setBodies( + array( + 'text' => 'test', + )); $mail->save(); $mail->processReceivedMail(); @@ -77,6 +81,10 @@ 'To' => 'bugs@example.com', 'From' => 'does+not+exist@example.com', )); + $mail->setBodies( + array( + 'text' => 'test', + )); $mail->save(); $mail->processReceivedMail(); @@ -101,6 +109,10 @@ 'From' => $user->loadPrimaryEmail()->getAddress(), 'To' => 'bugs@example.com', )); + $mail->setBodies( + array( + 'text' => 'test', + )); $mail->save(); $mail->processReceivedMail();