Make "new task" and "new conpherence" not-so-awful

Summary: Ref T1205. Moves the handling logic for these email types to reply handlers.

Test Plan: Used test form to send conpherence and maniphest mail.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1205

Differential Revision: https://secure.phabricator.com/D5945

src/applications/conpherence/mail/ConpherenceCreateThreadMailReceiver.php

@@ -9,22 +9,12 @@ final class ConpherenceCreateThreadMailReceiver
   }
 
   public function canAcceptMail(PhabricatorMetaMTAReceivedMail $mail) {
-    $usernames = array();
-    foreach ($mail->getToAddresses() as $to_address) {
-      $address = self::stripMailboxPrefix($to_address);
-      $usernames[] = id(new PhutilEmailAddress($address))->getLocalPart();
-    }
-
-    $usernames = array_unique($usernames);
-
+    $usernames = $this->getMailUsernames($mail);
     if (!$usernames) {
       return false;
     }
 
-    $users = id(new PhabricatorUser())->loadAllWhere(
-      'username in (%Ls)',
-      $usernames);
-
+    $users = $this->loadMailUsers($mail);
     if (count($users) != count($usernames)) {
       // At least some of the addresses are not users, so don't accept this as
       // a new Conpherence thread.
@@ -34,4 +24,42 @@ final class ConpherenceCreateThreadMailReceiver
     return true;
   }
 
+  private function getMailUsernames(PhabricatorMetaMTAReceivedMail $mail) {
+    $usernames = array();
+    foreach ($mail->getToAddresses() as $to_address) {
+      $address = self::stripMailboxPrefix($to_address);
+      $usernames[] = id(new PhutilEmailAddress($address))->getLocalPart();
+    }
+
+    return array_unique($usernames);
+  }
+
+  private function loadMailUsers(PhabricatorMetaMTAReceivedMail $mail) {
+    $usernames = $this->getMailUsernames($mail);
+    if (!$usernames) {
+      return array();
+    }
+
+    return id(new PhabricatorUser())->loadAllWhere(
+      'username in (%Ls)',
+      $usernames);
+  }
+
+  public function processReceivedMail(
+    PhabricatorMetaMTAReceivedMail $mail,
+    PhabricatorUser $sender) {
+
+    $users = $this->loadMailUsers($mail);
+    $phids = mpull($users, 'getPHID');
+
+    $conpherence = id(new ConpherenceReplyHandler())
+      ->setMailReceiver(new ConpherenceThread())
+      ->setMailAddedParticipantPHIDs($phids)
+      ->setActor($sender)
+      ->setExcludeMailRecipientPHIDs($mail->loadExcludeMailRecipientPHIDs())
+      ->processEmail($mail);
+
+    $mail->setRelatedPHID($conpherence->getPHID());
+  }
+
 }

src/applications/maniphest/mail/ManiphestCreateMailReceiver.php

@@ -56,4 +56,26 @@ final class ManiphestCreateMailReceiver extends PhabricatorMailReceiver {
     }
   }
 
+  public function processReceivedMail(
+    PhabricatorMetaMTAReceivedMail $mail,
+    PhabricatorUser $sender) {
+
+    $task = new ManiphestTask();
+
+    $task->setAuthorPHID($sender->getPHID());
+    $task->setOriginalEmailSource($mail->getHeader('From'));
+    $task->setPriority(ManiphestTaskPriority::PRIORITY_TRIAGE);
+
+    $editor = new ManiphestTransactionEditor();
+    $editor->setActor($sender);
+    $handler = $editor->buildReplyHandler($task);
+
+    $handler->setActor($sender);
+    $handler->setExcludeMailRecipientPHIDs(
+      $mail->loadExcludeMailRecipientPHIDs());
+    $handler->processEmail($mail);
+
+    $mail->setRelatedPHID($task->getPHID());
+  }
+
 }

src/applications/metamta/receiver/PhabricatorMailReceiver.php

@@ -2,10 +2,21 @@
 
 abstract class PhabricatorMailReceiver {
 
-
   abstract public function isEnabled();
   abstract public function canAcceptMail(PhabricatorMetaMTAReceivedMail $mail);
 
+  public function processReceivedMail(
+    PhabricatorMetaMTAReceivedMail $mail,
+    PhabricatorUser $sender) {
+    return;
+  }
+
+  final public function receiveMail(
+    PhabricatorMetaMTAReceivedMail $mail,
+    PhabricatorUser $sender) {
+    $this->processReceivedMail($mail, $sender);
+  }
+
   public function validateSender(
     PhabricatorMetaMTAReceivedMail $mail,
     PhabricatorUser $sender) {

src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php

@@ -61,7 +61,7 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
     return $this->getRawEmailAddresses(idx($this->headers, 'to'));
   }
 
-  private function loadExcludeMailRecipientPHIDs() {
+  public function loadExcludeMailRecipientPHIDs() {
     $addresses = array_merge(
       $this->getToAddresses(),
       $this->getCCAddresses());
@@ -115,7 +115,6 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
     $receiver_name       = null;
     $user_id             = null;
     $hash                = null;
-    $user_phids          = array();
     $user_names          = array();
     foreach ($this->getToAddresses() as $address) {
       if ($address == $create_task) {
@@ -150,20 +149,11 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
       }
     }
 
-    // since we haven't found a phabricator address, maybe this is
-    // someone trying to create a conpherence?
-    if (!$phabricator_address && $user_names) {
-      $users = id(new PhabricatorUser())
-        ->loadAllWhere('userName IN (%Ls)', $user_names);
-      $user_phids = mpull($users, 'getPHID');
-    }
-
     return array(
       $phabricator_address,
       $receiver_name,
       $user_id,
       $hash,
-      $user_phids
     );
   }
 
@@ -178,6 +168,22 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
       $sender = $receiver->loadSender($this);
       $receiver->validateSender($this, $sender);
 
+      $this->setAuthorPHID($sender->getPHID());
+
+      // TODO: Once everything can receive mail, nuke this.
+      $can_receive = false;
+      if ($receiver instanceof ManiphestCreateMailReceiver) {
+        $can_receive = true;
+      }
+      if ($receiver instanceof ConpherenceCreateThreadMailReceiver) {
+        $can_receive = true;
+      }
+
+      if ($can_receive) {
+        $receiver->receiveMail($this, $sender);
+        return $this->setMessage('OK')->save();
+      }
+
     } catch (PhabricatorMetaMTAReceivedMailProcessingException $ex) {
       $this
         ->setStatus($ex->getStatusCode())
@@ -189,58 +195,15 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
     list($to,
          $receiver_name,
          $user_id,
-         $hash,
-         $user_phids) = $this->getPhabricatorToInformation();
-    if (!$to && !$user_phids) {
+         $hash) = $this->getPhabricatorToInformation();
+    if (!$to) {
       $raw_to = idx($this->headers, 'to');
       return $this->setMessage("Unrecognized 'to' format: {$raw_to}")->save();
     }
 
     $from = idx($this->headers, 'from');
 
-    // TODO: Move this into `ManiphestCreateMailReceiver`.
-    if ($receiver instanceof ManiphestCreateMailReceiver) {
-      $receiver = new ManiphestTask();
-
-      $user = $sender;
-
-      $receiver->setAuthorPHID($user->getPHID());
-      $receiver->setOriginalEmailSource($from);
-      $receiver->setPriority(ManiphestTaskPriority::PRIORITY_TRIAGE);
-
-      $editor = new ManiphestTransactionEditor();
-      $editor->setActor($user);
-      $handler = $editor->buildReplyHandler($receiver);
-
-      $handler->setActor($user);
-      $handler->setExcludeMailRecipientPHIDs(
-        $this->loadExcludeMailRecipientPHIDs());
-      $handler->processEmail($this);
-
-      $this->setRelatedPHID($receiver->getPHID());
-      $this->setMessage('OK');
-
-      return $this->save();
-    }
-
-    // means we're creating a conpherence...!
-    if ($user_phids) {
-      $user = $sender;
-
-      $conpherence = id(new ConpherenceReplyHandler())
-        ->setMailReceiver(new ConpherenceThread())
-        ->setMailAddedParticipantPHIDs($user_phids)
-        ->setActor($user)
-        ->setExcludeMailRecipientPHIDs($this->loadExcludeMailRecipientPHIDs())
-        ->processEmail($this);
-
-      $this->setRelatedPHID($conpherence->getPHID());
-      $this->setMessage('OK');
-      return $this->save();
-    }
-
     $user = $sender;
-    $this->setAuthorPHID($user->getPHID());
 
     $receiver = self::loadReceiverObject($receiver_name);
     if (!$receiver) {