Conpherence - reply handler integration

Summary: Added a reply handler. A few problems -- first, I can't seem to get this to actually send me email so I haven't been able to reply (which I would have done by generating a reply, then copying the raw email into scripts/mail_handler.php). Second, the subject is often terrible on these emails -- unless the conpherence is named its something gross like "E4:" Third, on create I am noticing an error on array_combine() which I think is related to the need to write array_combine_not_broken or what have you I saw go by... (PhabricatorTransactionEditor does array_combine(xaction->getOldValue(), xaction->getOldValue()) and complains that the arrays are empty)

Test Plan: noted that /mail/ said mails were being sent

Reviewers: epriestley

Reviewed By: epriestley

CC: chad, aran, Korvin

Maniphest Tasks: T2399

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

src/__phutil_library_map__.php

@@ -195,6 +195,7 @@ phutil_register_library_map(array(
     'ConduitCallTestCase' => 'applications/conduit/call/__tests__/ConduitCallTestCase.php',
     'ConduitException' => 'applications/conduit/protocol/ConduitException.php',
     'ConduitSSHWorkflow' => 'applications/conduit/ssh/ConduitSSHWorkflow.php',
+    'ConpherenceConfigOptions' => 'applications/conpherence/config/ConpherenceConfigOptions.php',
     'ConpherenceConstants' => 'applications/conpherence/constants/ConpherenceConstants.php',
     'ConpherenceController' => 'applications/conpherence/controller/ConpherenceController.php',
     'ConpherenceDAO' => 'applications/conpherence/storage/ConpherenceDAO.php',
@@ -205,6 +206,7 @@ phutil_register_library_map(array(
     'ConpherenceParticipant' => 'applications/conpherence/storage/ConpherenceParticipant.php',
     'ConpherenceParticipantQuery' => 'applications/conpherence/query/ConpherenceParticipantQuery.php',
     'ConpherenceParticipationStatus' => 'applications/conpherence/constants/ConpherenceParticipationStatus.php',
+    'ConpherenceReplyHandler' => 'applications/conpherence/mail/ConpherenceReplyHandler.php',
     'ConpherenceThread' => 'applications/conpherence/storage/ConpherenceThread.php',
     'ConpherenceThreadQuery' => 'applications/conpherence/query/ConpherenceThreadQuery.php',
     'ConpherenceTransaction' => 'applications/conpherence/storage/ConpherenceTransaction.php',
@@ -1666,6 +1668,7 @@ phutil_register_library_map(array(
     'ConduitCallTestCase' => 'PhabricatorTestCase',
     'ConduitException' => 'Exception',
     'ConduitSSHWorkflow' => 'PhabricatorSSHWorkflow',
+    'ConpherenceConfigOptions' => 'PhabricatorApplicationConfigOptions',
     'ConpherenceController' => 'PhabricatorController',
     'ConpherenceDAO' => 'PhabricatorLiskDAO',
     'ConpherenceEditor' => 'PhabricatorApplicationTransactionEditor',
@@ -1675,6 +1678,7 @@ phutil_register_library_map(array(
     'ConpherenceParticipant' => 'ConpherenceDAO',
     'ConpherenceParticipantQuery' => 'PhabricatorOffsetPagedQuery',
     'ConpherenceParticipationStatus' => 'ConpherenceConstants',
+    'ConpherenceReplyHandler' => 'PhabricatorMailReplyHandler',
     'ConpherenceThread' =>
     array(
       0 => 'ConpherenceDAO',

src/applications/config/option/PhabricatorMetaMTAConfigOptions.php

@@ -235,6 +235,13 @@ EODOC
           pht(
             'Controls whether Phabricator sends email "From" users.'))
         ->setDescription($send_as_user_desc),
+      $this->newOption(
+        'metamta.reply-handler-domain',
+        'string',
+        'phabricator.example.com')
+        ->setDescription(pht(
+          'Domain used for reply email addresses. Some applications can '.
+          'configure this domain.')),
       $this->newOption('metamta.reply.show-hints', 'bool', true)
         ->setBoolOptions(
           array(

src/applications/conpherence/config/ConpherenceConfigOptions.php

@@ -0,0 +1,24 @@
+<?php
+
+final class ConpherenceConfigOptions
+  extends PhabricatorApplicationConfigOptions {
+
+  public function getName() {
+    return pht('Copherence');
+  }
+
+  public function getDescription() {
+    return pht('Configure Conpherence messaging.');
+  }
+
+  public function getOptions() {
+    return array(
+      $this->newOption(
+        'metamta.conpherence.subject-prefix',
+        'string',
+        '[Conpherence]')
+        ->setDescription(pht('Subject prefix for Conpherence mail.')),
+    );
+  }
+
+}

src/applications/conpherence/controller/ConpherenceNewController.php

@@ -60,6 +60,11 @@ final class ConpherenceNewController extends ConpherenceController {
         $xactions[] = id(new ConpherenceTransaction())
           ->setTransactionType(ConpherenceTransactionType::TYPE_PARTICIPANTS)
           ->setNewValue(array('+' => $participants));
+        if ($files) {
+          $xactions[] = id(new ConpherenceTransaction())
+            ->setTransactionType(ConpherenceTransactionType::TYPE_FILES)
+            ->setNewValue(array('+' => mpull($files, 'getPHID')));
+        }
         $xactions[] = id(new ConpherenceTransaction())
           ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
           ->attachComment(
@@ -67,11 +72,6 @@ final class ConpherenceNewController extends ConpherenceController {
             ->setContent($message)
             ->setConpherencePHID($conpherence->getPHID())
           );
-        if ($files) {
-          $xactions[] = id(new ConpherenceTransaction())
-            ->setTransactionType(ConpherenceTransactionType::TYPE_FILES)
-            ->setNewValue(array('+' => mpull($files, 'getPHID')));
-        }
         $content_source = PhabricatorContentSource::newForSource(
           PhabricatorContentSource::SOURCE_WEB,
           array(

src/applications/conpherence/editor/ConpherenceEditor.php

@@ -120,33 +120,40 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
   }
 
   protected function supportsMail() {
-    return false;
+    return true;
   }
 
-  /* TODO
-
   protected function buildReplyHandler(PhabricatorLiskDAO $object) {
     return id(new ConpherenceReplyHandler())
+      ->setActor($this->getActor())
       ->setMailReceiver($object);
   }
 
   protected function buildMailTemplate(PhabricatorLiskDAO $object) {
     $id = $object->getID();
     $title = $object->getTitle();
+    if (!$title) {
+      $title = pht(
+        '%s sent you a message.',
+        $this->getActor()->getUserName()
+      );
+    }
     $phid = $object->getPHID();
-    $original_name = $object->getOriginalName();
 
     return id(new PhabricatorMetaMTAMail())
-      ->setSubject("C{$id}: {$title}")
+      ->setSubject("E{$id}: {$title}")
-      ->addHeader('Thread-Topic', "C{$id}: {$phid}");
+      ->addHeader('Thread-Topic', "E{$id}: {$phid}");
   }
 
   protected function getMailTo(PhabricatorLiskDAO $object) {
     $participants = $object->getParticipants();
-    $participants[$this->requireActor()->getPHID()] = true;
     return array_keys($participants);
   }
 
+  protected function getMailCC(PhabricatorLiskDAO $object) {
+    return array();
+  }
+
   protected function buildMailBody(
     PhabricatorLiskDAO $object,
     array $xactions) {
@@ -162,7 +169,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor {
   protected function getMailSubjectPrefix() {
     return PhabricatorEnv::getEnvConfig('metamta.conpherence.subject-prefix');
   }
-   */
 
   protected function supportsFeed() {
     return false;

src/applications/conpherence/mail/ConpherenceReplyHandler.php

@@ -0,0 +1,69 @@
+<?php
+
+/**
+ * @group conpherence
+ */
+final class ConpherenceReplyHandler extends PhabricatorMailReplyHandler {
+
+  public function validateMailReceiver($mail_receiver) {
+    if (!($mail_receiver instanceof ConpherenceThread)) {
+      throw new Exception("Mail receiver is not a ConpherenceThread!");
+    }
+  }
+
+  public function getPrivateReplyHandlerEmailAddress(
+    PhabricatorObjectHandle $handle) {
+    return $this->getDefaultPrivateReplyHandlerEmailAddress($handle, 'E');
+  }
+
+  public function getPublicReplyHandlerEmailAddress() {
+    return $this->getDefaultPublicReplyHandlerEmailAddress('E');
+  }
+
+  public function getReplyHandlerInstructions() {
+    if ($this->supportsReplies()) {
+      return pht('Reply to comment and attach files.');
+    } else {
+      return null;
+    }
+  }
+
+  protected function receiveEmail(PhabricatorMetaMTAReceivedMail $mail) {
+    $conpherence = $this->getMailReceiver();
+    $user = $this->getActor();
+
+    $body = $mail->getCleanTextBody();
+    $body = trim($body);
+    $file_phids = $mail->getAttachments();
+    $body = $this->enhanceBodyWithAttachments($body, $file_phids);
+
+    $xactions = array();
+    if ($file_phids) {
+      $xactions[] = id(new ConpherenceTransaction())
+        ->setTransactionType(ConpherenceTransactionType::TYPE_FILES)
+        ->setNewValue(array('+' => $file_phids));
+    }
+    $xactions[] = id(new ConpherenceTransaction())
+      ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
+      ->attachComment(
+        id(new ConpherenceTransactionComment())
+        ->setContent($body)
+        ->setConpherencePHID($conpherence->getPHID())
+      );
+
+    $content_source = PhabricatorContentSource::newForSource(
+      PhabricatorContentSource::SOURCE_EMAIL,
+      array(
+        'id' => $mail->getID(),
+      ));
+
+    $editor = id(new ConpherenceEditor())
+      ->setActor($user)
+      ->setContentSource($content_source)
+      ->setParentMessageID($mail->getMessageID())
+      ->applyTransactions($conpherence, $xactions);
+
+    return null;
+  }
+
+}

src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php

@@ -37,7 +37,11 @@ abstract class PhabricatorMailReplyHandler {
   abstract public function validateMailReceiver($mail_receiver);
   abstract public function getPrivateReplyHandlerEmailAddress(
     PhabricatorObjectHandle $handle);
-  abstract public function getReplyHandlerDomain();
+  public function getReplyHandlerDomain() {
+    return PhabricatorEnv::getEnvConfig(
+      'metamta.reply-handler-domain'
+    );
+  }
   abstract public function getReplyHandlerInstructions();
   abstract protected function receiveEmail(
     PhabricatorMetaMTAReceivedMail $mail);

src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php

@@ -289,6 +289,10 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
     } else if ($receiver instanceof PhabricatorRepositoryCommit) {
       $handler = PhabricatorAuditCommentEditor::newReplyHandlerForCommit(
         $receiver);
+    } else if ($receiver instanceof ConpherenceThread) {
+      $handler = id(new ConpherenceEditor())
+        ->setActor($user)
+        ->buildReplyHandler($receiver);
     }
 
     $handler->setActor($user);
@@ -331,6 +335,9 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
       case 'C':
         $class_obj = new PhabricatorRepositoryCommit();
         break;
+      case 'E':
+        $class_obj = new ConpherenceThread();
+        break;
       default:
         return null;
     }

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -15,6 +15,7 @@ abstract class PhabricatorApplicationTransactionEditor
   private $isNewObject;
   private $mentionedPHIDs;
   private $continueOnNoEffect;
+  private $parentMessageID;
 
   private $isPreview;
 
@@ -40,6 +41,18 @@ abstract class PhabricatorApplicationTransactionEditor
     return $this->continueOnNoEffect;
   }
 
+  /**
+   * Not strictly necessary, but reply handlers ideally set this value to
+   * make email threading work better.
+   */
+  public function setParentMessageID($parent_message_id) {
+    $this->parentMessageID = $parent_message_id;
+    return $this;
+  }
+  public function getParentMessageID() {
+    return $this->parentMessageID;
+  }
+
   protected function getIsNewObject() {
     return $this->isNewObject;
   }
@@ -669,8 +682,9 @@ abstract class PhabricatorApplicationTransactionEditor
       ->setIsBulk(true)
       ->setBody($body->render());
 
-    // TODO
+    if ($this->getParentMessageID()) {
-    //  ->setParentMessageID(...)
+      $template->setParentMessageID($this->getParentMessageID());
+    }
 
     $mails = $this
       ->buildReplyHandler($object)