revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 22:10:55 +01:00
Code Issues Releases Wiki Activity

Make mail delivery reasons code-based; include positive and negative reasons

Browse source 
Summary:
Ref T7731. Looking forward to T5791, I eventually anticipate writing an interface which looks like a webmail UI where users can review mail they've been sent and understand why they recieved (or did not receive) the mail. Roughly like `bin/mail list-outbound` / `bin/mail show-outbound` work today, but policy-aware (so you can only see messages where delivery was attempted to you).

We currently record a list of "reasons" why a mail is undeliverable, but this list is string-based (so it can not be translated once we start persisting it) and has only negative reasons (so it can not be used to fully understand reasons for delivery or nondelivery).

Make it code-based (so it can be translated) and allow both positive and negative reasons to be listed (so positive reasons can be understood).

Test Plan: Used `bin/mail show-outbound` to review mail delivery reasons, including the positive reason we currently have (forced delivery of authentication mail).

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T7731

Differential Revision: https://secure.phabricator.com/D12297
This commit is contained in:
epriestley 2015-04-06 10:01:11 -07:00
parent 63f4e66b11
commit c0e26c65e0
4 changed files with 88 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
src/applications/metamta/management/PhabricatorMailManagementShowOutboundWorkflow.php
View file

@ -127,9 +127,10 @@ final class PhabricatorMailManagementShowOutboundWorkflow
$info[] = ' '.coalesce($actor->getName(), $actor->getPHID());
} else {
$info[] = '! '.coalesce($actor->getName(), $actor->getPHID());
foreach ($actor->getUndeliverableReasons() as $reason) {
$info[] = ' - '.$reason;
}
}
foreach ($actor->getDeliverabilityReasons() as $reason) {
$desc = PhabricatorMetaMTAActor::getReasonDescription($reason);
$info[] = ' - '.$desc;
}
}

71
src/applications/metamta/query/PhabricatorMetaMTAActor.php
View file

@ -2,9 +2,25 @@
final class PhabricatorMetaMTAActor {
const STATUS_DELIVERABLE = 'deliverable';
const STATUS_UNDELIVERABLE = 'undeliverable';
const REASON_UNLOADABLE = 'unloadable';
const REASON_UNMAILABLE = 'unmailable';
const REASON_NO_ADDRESS = 'noaddress';
const REASON_DISABLED = 'disabled';
const REASON_MAIL_DISABLED = 'maildisabled';
const REASON_EXTERNAL_TYPE = 'exernaltype';
const REASON_RESPONSE = 'response';
const REASON_SELF = 'self';
const REASON_MAILTAGS = 'mailtags';
const REASON_BOT = 'bot';
const REASON_FORCE = 'force';
private $phid;
private $emailAddress;
private $name;
private $status = self::STATUS_DELIVERABLE;
private $reasons = array();
public function setName($name) {
@ -36,15 +52,66 @@ final class PhabricatorMetaMTAActor {
public function setUndeliverable($reason) {
$this->reasons[] = $reason;
$this->status = self::STATUS_UNDELIVERABLE;
return $this;
}
public function setDeliverable($reason) {
$this->reasons[] = $reason;
$this->status = self::STATUS_DELIVERABLE;
return $this;
}
public function isDeliverable() {
return empty($this->reasons);
return ($this->status === self::STATUS_DELIVERABLE);
}
public function getUndeliverableReasons() {
public function getDeliverabilityReasons() {
return $this->reasons;
}
public static function getReasonDescription($reason) {
$descriptions = array(
self::REASON_DISABLED => pht(
'This user is disabled; disabled users do not receive mail.'),
self::REASON_BOT => pht(
'This user is a bot; bot accounts do not receive mail.'),
self::REASON_NO_ADDRESS => pht(
'Unable to load an email address for this PHID.'),
self::REASON_EXTERNAL_TYPE => pht(
'Only external accounts of type "email" are deliverable; this '.
'account has a different type.'),
self::REASON_UNMAILABLE => pht(
'This PHID type does not correspond to a mailable object.'),
self::REASON_RESPONSE => pht(
'This message is a response to another email message, and this '.
'recipient received the original email message, so we are not '.
'sending them this substantially similar message (for example, '.
'the sender used "Reply All" instead of "Reply" in response to '.
'mail from Phabricator).'),
self::REASON_SELF => pht(
'This recipient is the user whose actions caused delivery of '.
'this message, but they have set preferences so they do not '.
'receive mail about their own actions (Settings > Email '.
'Preferences > Self Actions).'),
self::REASON_MAIL_DISABLED => pht(
'This recipient has disabled all email notifications '.
'(Settings > Email Preferences > Email Notifications).'),
self::REASON_MAILTAGS => pht(
'This mail has tags which control which users receive it, and '.
'this recipient has not elected to receive mail with any of '.
'the tags on this message (Settings > Email Preferences).'),
self::REASON_UNLOADABLE => pht(
'Unable to load user record for this PHID.'),
self::REASON_FORCE => pht(
'Delivery of this mail is forced and ignores deliver preferences. '.
'Mail which uses forced delivery is usually related to account '.
'management or authentication. For example, password reset email '.
'ignores mail preferences.'),
);
return idx($descriptions, $reason, pht('Unknown Reason ("%s")', $reason));
}
}

26
src/applications/metamta/query/PhabricatorMetaMTAActorQuery.php
View file

@ -70,17 +70,14 @@ final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
$user = idx($users, $phid);
if (!$user) {
$actor->setUndeliverable(
pht('Unable to load user record for this PHID.'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_UNLOADABLE);
} else {
$actor->setName($this->getUserName($user));
if ($user->getIsDisabled()) {
$actor->setUndeliverable(
pht('This user is disabled; disabled users do not receive mail.'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_DISABLED);
}
if ($user->getIsSystemAgent()) {
$actor->setUndeliverable(
pht('This user is a bot; bot accounts do not receive mail.'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_BOT);
}
// NOTE: We do send email to unapproved users, and to unverified users,
@ -91,8 +88,7 @@ final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
$email = idx($emails, $phid);
if (!$email) {
$actor->setUndeliverable(
pht('Unable to load email record for this PHID.'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_NO_ADDRESS);
} else {
$actor->setEmailAddress($email->getAddress());
}
@ -113,18 +109,14 @@ final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
$xuser = idx($xusers, $phid);
if (!$xuser) {
$actor->setUndeliverable(
pht('Unable to load external user record for this PHID.'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_UNLOADABLE);
continue;
}
$actor->setName($xuser->getDisplayName());
if ($xuser->getAccountType() != 'email') {
$actor->setUndeliverable(
pht(
'Only external accounts of type "email" are deliverable; this '.
'account has a different type.'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_EXTERNAL_TYPE);
continue;
}
@ -146,9 +138,7 @@ final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
$list = idx($lists, $phid);
if (!$list) {
$actor->setUndeliverable(
pht(
'Unable to load mailing list record for this PHID.'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_UNLOADABLE);
continue;
}
@ -160,7 +150,7 @@ final class PhabricatorMetaMTAActorQuery extends PhabricatorQuery {
private function loadUnknownActors(array $actors, array $phids) {
foreach ($phids as $phid) {
$actor = $actors[$phid];
$actor->setUndeliverable(pht('This PHID type is not mailable.'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_UNMAILABLE);
}
}

27
src/applications/metamta/storage/PhabricatorMetaMTAMail.php
View file

@ -852,6 +852,9 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
if ($this->getForceDelivery()) {
// If we're forcing delivery, skip all the opt-out checks.
foreach ($actors as $actor) {
$actor->setDeliverable(PhabricatorMetaMTAActor::REASON_FORCE);
}
return $actors;
}
@ -861,13 +864,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
if (!$actor) {
continue;
}
$actor->setUndeliverable(
pht(
'This message is a response to another email message, and this '.
'recipient received the original email message, so we are not '.
'sending them this substantially similar message (for example, '.
'the sender used "Reply All" instead of "Reply" in response to '.
'mail from Phabricator).'));
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_RESPONSE);
}
// Exclude the actor if their preferences are set.
@ -885,12 +882,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
->loadPreferences()
->getPreference($pref_key);
if ($exclude_self) {
$from_actor->setUndeliverable(
pht(
'This recipient is the user whose actions caused delivery of '.
'this message, but they have set preferences so they do not '.
'receive mail about their own actions (Settings > Email '.
'Preferences > Self Actions).'));
$from_actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_SELF);
}
}
}
@ -907,9 +899,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
false);
if ($exclude) {
$actors[$phid]->setUndeliverable(
pht(
'This recipient has disabled all email notifications '.
'(Settings > Email Preferences > Email Notifications).'));
PhabricatorMetaMTAActor::REASON_MAIL_DISABLED);
}
}
@ -937,10 +927,7 @@ final class PhabricatorMetaMTAMail extends PhabricatorMetaMTADAO {
if (!$send) {
$actors[$phid]->setUndeliverable(
pht(
'This mail has tags which control which users receive it, and '.
'this recipient has not elected to receive mail with any of '.
'the tags on this message (Settings > Email Preferences).'));
PhabricatorMetaMTAActor::REASON_MAILTAGS);
}
}
}