1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-30 01:10:58 +01:00

Expand aggregate email recipients prior to multiplexing

Summary:
Ref T4361. Before we figure out which To/CC are addressable, try to expand To/CC. Specifically, the supported expansion right now is project PHIDs expanding to all their members.

Because of the way multiplexing works, we have to do this in two places: explicitly in `multiplexMail()`, and when sending mail that wasn't multiplexed. This is messy; eventually we can get rid of it (after ApplicationTransactions are everywhere).

This has some rough edges, but should basically give us what we need to make stuff like projects mailable. Particularly, it deals with most issues in D7436:

  - I got around the resolution/multiplexing issue by resolving aggregate mailables separately from mailable actors.
  - We get to keep the Project PHID as a To/CC/Reviewer/Whatever until the last second.
  - Users won't get two emails for being a CC and also a member of a CC'd project.
  - We can degrade to the list stuff this way if we want, by having the project aggregate yield a single list PHID.

Test Plan: Added a comment to a revision with a project reviewer, got mail to all the project's members.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4361

Differential Revision: https://secure.phabricator.com/D8117
This commit is contained in:
epriestley 2014-02-01 14:35:55 -08:00
parent 4b0ef353e4
commit eca7d3feda
4 changed files with 160 additions and 2 deletions

View file

@ -1648,6 +1648,7 @@ phutil_register_library_map(array(
'PhabricatorMetaMTAMailTestCase' => 'applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php',
'PhabricatorMetaMTAMailgunReceiveController' => 'applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php',
'PhabricatorMetaMTAMailingList' => 'applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php',
'PhabricatorMetaMTAMemberQuery' => 'applications/metamta/query/PhabricatorMetaMTAMemberQuery.php',
'PhabricatorMetaMTAPermanentFailureException' => 'applications/metamta/exception/PhabricatorMetaMTAPermanentFailureException.php',
'PhabricatorMetaMTAReceivedMail' => 'applications/metamta/storage/PhabricatorMetaMTAReceivedMail.php',
'PhabricatorMetaMTAReceivedMailProcessingException' => 'applications/metamta/exception/PhabricatorMetaMTAReceivedMailProcessingException.php',
@ -4325,6 +4326,7 @@ phutil_register_library_map(array(
0 => 'PhabricatorMetaMTADAO',
1 => 'PhabricatorPolicyInterface',
),
'PhabricatorMetaMTAMemberQuery' => 'PhabricatorQuery',
'PhabricatorMetaMTAPermanentFailureException' => 'Exception',
'PhabricatorMetaMTAReceivedMail' => 'PhabricatorMetaMTADAO',
'PhabricatorMetaMTAReceivedMailProcessingException' => 'Exception',

View file

@ -0,0 +1,69 @@
<?php
/**
* Expands aggregate mail recipients into their component mailables. For
* example, a project currently expands into all of its members.
*/
final class PhabricatorMetaMTAMemberQuery extends PhabricatorQuery {
private $phids = array();
private $viewer;
public function setViewer(PhabricatorUser $viewer) {
$this->viewer = $viewer;
return $this;
}
public function getViewer() {
return $this->viewer;
}
public function withPHIDs(array $phids) {
$this->phids = $phids;
return $this;
}
public function execute() {
$phids = array_fuse($this->phids);
$actors = array();
$type_map = array();
foreach ($phids as $phid) {
$type_map[phid_get_type($phid)][] = $phid;
}
// TODO: Generalize this somewhere else.
$results = array();
foreach ($type_map as $type => $phids) {
switch ($type) {
case PhabricatorProjectPHIDTypeProject::TYPECONST:
// TODO: For now, project members are always on the "mailing list"
// implied by the project, but we should differentiate members and
// subscribers (i.e., allow you to unsubscribe from mail about
// a project).
$projects = id(new PhabricatorProjectQuery())
->setViewer($this->getViewer())
->needMembers(true)
->withPHIDs($phids)
->execute();
$projects = mpull($projects, null, 'getPHID');
foreach ($phids as $phid) {
$project = idx($projects, $phid);
if (!$project) {
$results[$phid] = array();
} else {
$results[$phid] = $project->getMemberPHIDs();
}
}
break;
default:
break;
}
}
return $results;
}
}

View file

@ -198,6 +198,15 @@ EOBODY;
}
}
// TODO: This is pretty messy. We should really be doing all of this
// multiplexing in the task queue, but that requires significant rewriting
// in the general case. ApplicationTransactions can do it fairly easily,
// but other mail sites currently can not, so we need to support this
// junky version until they catch up and we can swap things over.
$to_handles = $this->expandRecipientHandles($to_handles);
$cc_handles = $this->expandRecipientHandles($cc_handles);
$tos = mpull($to_handles, null, 'getPHID');
$ccs = mpull($cc_handles, null, 'getPHID');
@ -219,6 +228,8 @@ EOBODY;
$body .= $this->getRecipientsSummary($to_handles, $cc_handles);
foreach ($recipients as $phid => $recipient) {
$mail = clone $mail_template;
if (isset($to_handles[$phid])) {
$mail->addTos(array($phid));
@ -332,4 +343,32 @@ EOBODY;
return rtrim($body);
}
private function expandRecipientHandles(array $handles) {
if (!$handles) {
return array();
}
$phids = mpull($handles, 'getPHID');
$map = 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;
}
}
return id(new PhabricatorHandleQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withPHIDs($results)
->execute();
}
}

View file

@ -19,6 +19,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
private $excludePHIDs = array();
private $overrideNoSelfMail = false;
private $recipientExpansionMap;
public function __construct() {
@ -379,7 +380,8 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
$mailer->addReplyTo($value, $reply_to_name);
break;
case 'to':
$to_actors = array_select_keys($deliverable_actors, $value);
$to_phids = $this->expandRecipients($value);
$to_actors = array_select_keys($deliverable_actors, $to_phids);
$add_to = array_merge(
$add_to,
mpull($to_actors, 'getEmailAddress'));
@ -388,7 +390,8 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
$add_to = array_merge($add_to, $value);
break;
case 'cc':
$cc_actors = array_select_keys($deliverable_actors, $value);
$cc_phids = $this->expandRecipients($value);
$cc_actors = array_select_keys($deliverable_actors, $cc_phids);
$add_cc = array_merge(
$add_cc,
mpull($cc_actors, 'getEmailAddress'));
@ -685,9 +688,54 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
$this->getToPHIDs(),
$this->getCcPHIDs());
$this->loadRecipientExpansions($actor_phids);
$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;
}
/**
* Expand a list of recipient PHIDs (possibly including aggregate recipients
* like projects) into a deaggregated list of individual recipient PHIDs.
* For example, this will expand project PHIDs into a list of the project's
* members.
*
* @param list<phid> List of recipient PHIDs, possibly including aggregate
* recipients.
* @return list<phid> Deaggregated list of mailable recipients.
*/
private function expandRecipients(array $phids) {
if ($this->recipientExpansionMap === null) {
throw new Exception(
pht(
'Call loadRecipientExpansions() before expandRecipients()!'));
}
$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;
}
}
}
return array_keys($results);
}
private function filterDeliverableActors(array $actors) {
assert_instances_of($actors, 'PhabricatorMetaMTAActor');
$deliverable_actors = array();