diff --git a/src/applications/feed/PhabricatorFeedStoryPublisher.php b/src/applications/feed/PhabricatorFeedStoryPublisher.php --- a/src/applications/feed/PhabricatorFeedStoryPublisher.php +++ b/src/applications/feed/PhabricatorFeedStoryPublisher.php @@ -192,6 +192,8 @@ * @return list 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 @@ 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 diff --git a/src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php b/src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php --- a/src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php +++ b/src/applications/metamta/query/PhabricatorMetaMTAMemberQuery.php @@ -59,6 +59,11 @@ } 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 @@ 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())); + } + } 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 @@ -360,21 +360,10 @@ } $phids = mpull($handles, 'getPHID'); - $map = id(new PhabricatorMetaMTAMemberQuery()) + $results = id(new PhabricatorMetaMTAMemberQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withPHIDs($phids) - ->execute(); - - $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; - } - } + ->executeExpansion(); return id(new PhabricatorHandleQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -773,40 +773,30 @@ * 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 A list of all recipients to whom delivery will be * attempted. * @task recipients */ public function buildRecipientList() { - $actors = $this->loadActors( - array_merge( - $this->getToPHIDs(), - $this->getCcPHIDs())); + $actors = $this->loadAllActors(); $actors = $this->filterDeliverableActors($actors); return mpull($actors, 'getPHID'); } public function loadAllActors() { - $actor_phids = array_merge( - array($this->getParam('from')), - $this->getToPHIDs(), - $this->getCcPHIDs()); - - $this->loadRecipientExpansions($actor_phids); + $actor_phids = $this->getAllActorPHIDs(); $actor_phids = $this->expandRecipients($actor_phids); - return $this->loadActors($actor_phids); } - private function loadRecipientExpansions(array $phids) { - $expansions = id(new PhabricatorMetaMTAMemberQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withPHIDs($phids) - ->execute(); - - $this->recipientExpansionMap = $expansions; - - return $this; + private function getAllActorPHIDs() { + return array_merge( + array($this->getParam('from')), + $this->getToPHIDs(), + $this->getCcPHIDs()); } /** @@ -821,19 +811,17 @@ */ private function expandRecipients(array $phids) { if ($this->recipientExpansionMap === null) { - throw new Exception( - pht( - 'Call loadRecipientExpansions() before expandRecipients()!')); + $all_phids = $this->getAllActorPHIDs(); + $this->recipientExpansionMap = id(new PhabricatorMetaMTAMemberQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs($all_phids) + ->execute(); } $results = array(); foreach ($phids as $phid) { - if (!isset($this->recipientExpansionMap[$phid])) { - $results[$phid] = $phid; - } else { - foreach ($this->recipientExpansionMap[$phid] as $recipient_phid) { - $results[$recipient_phid] = $recipient_phid; - } + foreach ($this->recipientExpansionMap[$phid] as $recipient_phid) { + $results[$recipient_phid] = $recipient_phid; } }