From 9e6da4220611f4c4f8669f80636ea6f51a11287e Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 13 May 2013 08:10:02 -0700
Subject: [PATCH] Fix some issues with Maniphest inbound email

Summary:
Fixes T3181.

  - Inbound `bugs@` mail is broken right now if it doesn't use the new external user stuff, because it calls `$user->getPhabricatorUser()` on an object which is already a `PhabricatorUser`. Instead, build the right `$user` object from the external user earlier on.
  - Maniphest mail is nuking or otherwise awkwardly altering CCs. Make this work properly.
  - Make sure "!unsubscribe" works correctly.

Test Plan: Sent `bugs@` mail. Sent CC mail. Sent "!unsubscribe" mail.

Reviewers: btrahan, chad

Reviewed By: chad

CC: aran, tido

Maniphest Tasks: T3181

Differential Revision: https://secure.phabricator.com/D5911
---
 .../maniphest/ManiphestReplyHandler.php       | 15 +++++++----
 .../PhabricatorMetaMTAReceivedMail.php        | 25 ++++++++++---------
 2 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/src/applications/maniphest/ManiphestReplyHandler.php b/src/applications/maniphest/ManiphestReplyHandler.php
index 0956a5a899..50c3114d66 100644
--- a/src/applications/maniphest/ManiphestReplyHandler.php
+++ b/src/applications/maniphest/ManiphestReplyHandler.php
@@ -60,7 +60,7 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
     $template->setContentSource($content_source);
     $template->setAuthorPHID($user->getPHID());
 
-
+    $is_unsub = false;
     if ($is_new_task) {
       // If this is a new task, create a "User created this task." transaction
       // and then set the title and description.
@@ -100,6 +100,7 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
           $new_value = $user->getPHID();
           break;
         case 'unsubscribe':
+          $is_unsub = true;
           $ttype = ManiphestTransactionType::TYPE_CCS;
           $ccs = $task->getCCPHIDs();
           foreach ($ccs as $k => $phid) {
@@ -119,11 +120,15 @@ final class ManiphestReplyHandler extends PhabricatorMailReplyHandler {
       $xactions[] = $xaction;
     }
 
-    $task->setCCPHIDs(array($user->getPHID()));
     $ccs = $mail->loadCCPHIDs();
-    if ($ccs) {
-      $old_ccs = $task->getCCPHIDs();
-      $new_ccs = array_unique(array_merge($old_ccs, $ccs));
+    $old_ccs = $task->getCCPHIDs();
+    $new_ccs = array_merge($old_ccs, $ccs);
+    if (!$is_unsub) {
+      $new_ccs[] = $user->getPHID();
+    }
+    $new_ccs = array_unique($new_ccs);
+
+    if (array_diff($new_ccs, $old_ccs)) {
       $cc_xaction = clone $template;
       $cc_xaction->setTransactionType(ManiphestTransactionType::TYPE_CCS);
       $cc_xaction->setNewValue($new_ccs);
diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
index 67b17c9069..afc47c2ba3 100644
--- a/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
+++ b/src/applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php
@@ -217,19 +217,20 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
         if ($allow_email_users) {
           $email = new PhutilEmailAddress($from);
 
-          $user = id(new PhabricatorExternalAccount())->loadOneWhere(
+          $xuser = id(new PhabricatorExternalAccount())->loadOneWhere(
             'accountType = %s AND accountDomain IS NULL and accountID = %s',
-            'email', $email->getAddress());
-
-          if (!$user) {
-            $user = new PhabricatorExternalAccount();
-            $user->setAccountID($email->getAddress());
-            $user->setAccountType('email');
-            $user->setDisplayName($email->getDisplayName());
-            $user->save();
+            'email',
+            $email->getAddress());
 
+          if (!$xuser) {
+            $xuser = new PhabricatorExternalAccount();
+            $xuser->setAccountID($email->getAddress());
+            $xuser->setAccountType('email');
+            $xuser->setDisplayName($email->getDisplayName());
+            $xuser->save();
           }
 
+          $user = $xuser->getPhabricatorUser();
         } else {
           $default_author = PhabricatorEnv::getEnvConfig(
             'metamta.maniphest.default-public-author');
@@ -260,12 +261,12 @@ final class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO {
       $receiver->setPriority(ManiphestTaskPriority::PRIORITY_TRIAGE);
 
       $editor = new ManiphestTransactionEditor();
-      $editor->setActor($user->getPhabricatorUser());
+      $editor->setActor($user);
       $handler = $editor->buildReplyHandler($receiver);
 
-      $handler->setActor($user->getPhabricatorUser());
+      $handler->setActor($user);
       $handler->setExcludeMailRecipientPHIDs(
-      $this->loadExcludeMailRecipientPHIDs());
+        $this->loadExcludeMailRecipientPHIDs());
       $handler->processEmail($this);
 
       $this->setRelatedPHID($receiver->getPHID());