Fix handling of notifications with project members

Summary: Fixes T7377. We don't expand projects into members when sending notifications right now. Instead, expand them.

Test Plan:
  - Added a project as a reviewer to a revision, made a comment, saw project members receive a read notification + email (with appropriate preferences).
  - There's meaningful test coverage on the core mail stuff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7377

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

src/applications/feed/PhabricatorFeedStoryPublisher.php

@@ -192,6 +192,8 @@ final class PhabricatorFeedStoryPublisher {
    * @return list<phid> List of actual subscribers.
    */
   private function filterSubscribedPHIDs(array $phids) {
+    $phids = $this->expandRecipients($phids);
+
     $tags = $this->getMailTags();
     if ($tags) {
       $all_prefs = id(new PhabricatorUserPreferences())->loadAllWhere(
@@ -234,6 +236,13 @@ final class PhabricatorFeedStoryPublisher {
     return array_values(array_unique($keep));
   }
 
+  private function expandRecipients(array $phids) {
+    return id(new PhabricatorMetaMTAMemberQuery())
+      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      ->withPHIDs($phids)
+      ->executeExpansion();
+  }
+
   /**
    * We generate a unique chronological key for each story type because we want
    * to be able to page through the stream with a cursor (i.e., select stories

src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php

@@ -59,6 +59,11 @@ final class PhabricatorMetaMTAMemberQuery extends PhabricatorQuery {
           }
           break;
         default:
+          // For other types, just map the PHID to itself without modification.
+          // This allows callers to do less work.
+          foreach ($phids as $phid) {
+            $results[$phid] = array($phid);
+          }
           break;
       }
     }
@@ -66,4 +71,13 @@ final class PhabricatorMetaMTAMemberQuery extends PhabricatorQuery {
     return $results;
   }
 
+
+  /**
+   * Execute the query, merging results into a single list of unique member
+   * PHIDs.
+   */
+  public function executeExpansion() {
+    return array_unique(array_mergev($this->execute()));
+  }
+
 }

src/applications/metamta/replyhandler/PhabricatorMailReplyHandler.php

@@ -360,21 +360,10 @@ abstract class PhabricatorMailReplyHandler {
     }
 
     $phids = mpull($handles, 'getPHID');
-    $map = id(new PhabricatorMetaMTAMemberQuery())
+    $results = id(new PhabricatorMetaMTAMemberQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())
       ->withPHIDs($phids)
-      ->execute();
+      ->executeExpansion();
-
-    $results = array();
-    foreach ($phids as $phid) {
-      if (isset($map[$phid])) {
-        foreach ($map[$phid] as $expanded_phid) {
-          $results[$expanded_phid] = $expanded_phid;
-        }
-      } else {
-        $results[$phid] = $phid;
-      }
-    }
 
     return id(new PhabricatorHandleQuery())
       ->setViewer(PhabricatorUser::getOmnipotentUser())

src/applications/metamta/storage/PhabricatorMetaMTAMail.php

@@ -773,40 +773,30 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
    * Get all of the recipients for this mail, after preference filters are
    * applied. This list has all objects to whom delivery will be attempted.
    *
+   * Note that this expands recipients into their members, because delivery
+   * is never directly attempted to aggregate actors like projects.
+   *
    * @return  list<phid>  A list of all recipients to whom delivery will be
    *                      attempted.
    * @task recipients
    */
   public function buildRecipientList() {
-    $actors = $this->loadActors(
+    $actors = $this->loadAllActors();
-      array_merge(
-        $this->getToPHIDs(),
-        $this->getCcPHIDs()));
     $actors = $this->filterDeliverableActors($actors);
     return mpull($actors, 'getPHID');
   }
 
   public function loadAllActors() {
-    $actor_phids = array_merge(
+    $actor_phids = $this->getAllActorPHIDs();
-      array($this->getParam('from')),
-      $this->getToPHIDs(),
-      $this->getCcPHIDs());
-
-    $this->loadRecipientExpansions($actor_phids);
     $actor_phids = $this->expandRecipients($actor_phids);
-
     return $this->loadActors($actor_phids);
   }
 
-  private function loadRecipientExpansions(array $phids) {
+  private function getAllActorPHIDs() {
-    $expansions = id(new PhabricatorMetaMTAMemberQuery())
+    return array_merge(
-      ->setViewer(PhabricatorUser::getOmnipotentUser())
+      array($this->getParam('from')),
-      ->withPHIDs($phids)
+      $this->getToPHIDs(),
-      ->execute();
+      $this->getCcPHIDs());
-
-    $this->recipientExpansionMap = $expansions;
-
-    return $this;
   }
 
   /**
@@ -821,19 +811,17 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
    */
   private function expandRecipients(array $phids) {
     if ($this->recipientExpansionMap === null) {
-      throw new Exception(
+      $all_phids = $this->getAllActorPHIDs();
-        pht(
+      $this->recipientExpansionMap = id(new PhabricatorMetaMTAMemberQuery())
-          'Call loadRecipientExpansions() before expandRecipients()!'));
+        ->setViewer(PhabricatorUser::getOmnipotentUser())
+        ->withPHIDs($all_phids)
+        ->execute();
     }
 
     $results = array();
     foreach ($phids as $phid) {
-      if (!isset($this->recipientExpansionMap[$phid])) {
+      foreach ($this->recipientExpansionMap[$phid] as $recipient_phid) {
-        $results[$phid] = $phid;
+        $results[$recipient_phid] = $recipient_phid;
-      } else {
-        foreach ($this->recipientExpansionMap[$phid] as $recipient_phid) {
-          $results[$recipient_phid] = $recipient_phid;
-        }
       }
     }