Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15410665
D12300.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
11 KB
Referenced Files
None
Subscribers
None
D12300.diff
View Options
diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -6,7 +6,6 @@
const MAX_FILES_SHOWN_IN_EMAIL = 1000;
private $auditReasonMap = array();
- private $heraldEmailPHIDs = array();
private $affectedFiles;
private $rawPatch;
@@ -627,9 +626,6 @@
protected function getMailTo(PhabricatorLiskDAO $object) {
$phids = array();
- if ($this->heraldEmailPHIDs) {
- $phids = $this->heraldEmailPHIDs;
- }
if ($object->getAuthorPHID()) {
$phids[] = $object->getAuthorPHID();
@@ -924,8 +920,6 @@
->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
->setNewValue($add_ccs);
- $this->heraldEmailPHIDs = $adapter->getEmailPHIDs();
-
HarbormasterBuildable::applyBuildPlans(
$object->getPHID(),
$object->getRepository()->getPHID(),
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -3,7 +3,6 @@
final class DifferentialTransactionEditor
extends PhabricatorApplicationTransactionEditor {
- private $heraldEmailPHIDs;
private $changedPriorToCommitURI;
private $isCloseByCommit;
private $repositoryPHIDOverride = false;
@@ -1154,18 +1153,6 @@
return $phids;
}
- protected function getMailCC(PhabricatorLiskDAO $object) {
- $phids = parent::getMailCC($object);
-
- if ($this->heraldEmailPHIDs) {
- foreach ($this->heraldEmailPHIDs as $phid) {
- $phids[] = $phid;
- }
- }
-
- return $phids;
- }
-
protected function getMailAction(
PhabricatorLiskDAO $object,
array $xactions) {
@@ -1730,9 +1717,6 @@
}
}
- // Save extra email PHIDs for later.
- $this->heraldEmailPHIDs = $adapter->getEmailPHIDs();
-
// Apply build plans.
HarbormasterBuildable::applyBuildPlans(
$adapter->getDiff()->getPHID(),
diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
--- a/src/applications/herald/adapter/HeraldAdapter.php
+++ b/src/applications/herald/adapter/HeraldAdapter.php
@@ -110,11 +110,16 @@
private $customActions = null;
private $queuedTransactions = array();
private $emailPHIDs = array();
+ private $forcedEmailPHIDs = array();
public function getEmailPHIDs() {
return array_values($this->emailPHIDs);
}
+ public function getForcedEmailPHIDs() {
+ return array_values($this->forcedEmailPHIDs);
+ }
+
public function getCustomActions() {
if ($this->customActions === null) {
$custom_actions = id(new PhutilSymbolLoader())
@@ -1038,6 +1043,14 @@
foreach ($effect->getTarget() as $phid) {
$this->emailPHIDs[$phid] = $phid;
+
+ // If this is a personal rule, we'll force delivery of a real email. This
+ // effect is stronger than notification preferences, so you get an actual
+ // email even if your preferences are set to "Notify" or "Ignore".
+ $rule = $effect->getRule();
+ if ($rule->isPersonalRule()) {
+ $this->forcedEmailPHIDs[$phid] = $phid;
+ }
}
return new HeraldApplyTranscript(
diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
--- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php
+++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php
@@ -3,8 +3,6 @@
final class ManiphestTransactionEditor
extends PhabricatorApplicationTransactionEditor {
- private $heraldEmailPHIDs = array();
-
public function getEditorApplicationClass() {
return 'PhabricatorManiphestApplication';
}
@@ -394,20 +392,6 @@
);
}
- protected function getMailCC(PhabricatorLiskDAO $object) {
- $phids = array();
-
- foreach (parent::getMailCC($object) as $phid) {
- $phids[] = $phid;
- }
-
- foreach ($this->heraldEmailPHIDs as $phid) {
- $phids[] = $phid;
- }
-
- return $phids;
- }
-
public function getMailTagsMap() {
return array(
ManiphestTransaction::MAILTAG_STATUS =>
@@ -521,7 +505,6 @@
HeraldAdapter $adapter,
HeraldTranscript $transcript) {
- $this->heraldEmailPHIDs = $adapter->getEmailPHIDs();
$xactions = array();
$cc_phids = $adapter->getCcPHIDs();
diff --git a/src/applications/metamta/query/PhabricatorMetaMTAActor.php b/src/applications/metamta/query/PhabricatorMetaMTAActor.php
--- a/src/applications/metamta/query/PhabricatorMetaMTAActor.php
+++ b/src/applications/metamta/query/PhabricatorMetaMTAActor.php
@@ -16,6 +16,7 @@
const REASON_MAILTAGS = 'mailtags';
const REASON_BOT = 'bot';
const REASON_FORCE = 'force';
+ const REASON_FORCE_HERALD = 'force-herald';
private $phid;
private $emailAddress;
@@ -108,6 +109,9 @@
'Mail which uses forced delivery is usually related to account '.
'management or authentication. For example, password reset email '.
'ignores mail preferences.'),
+ self::REASON_FORCE_HERALD => pht(
+ 'This recipient was added by a "Send me an Email" rule in Herald, '.
+ 'which overrides some delivery settings.'),
);
return idx($descriptions, $reason, pht('Unknown Reason ("%s")', $reason));
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
@@ -141,6 +141,15 @@
return $this->getParam('exclude', array());
}
+ public function setForceHeraldMailRecipientPHIDs(array $force) {
+ $this->setParam('herald-force-recipients', $force);
+ return $this;
+ }
+
+ private function getForceHeraldMailRecipientPHIDs() {
+ return $this->getParam('herald-force-recipients', array());
+ }
+
public function getTranslation(array $objects) {
$default_translation = PhabricatorEnv::getEnvConfig('translation.provider');
$return = null;
@@ -851,7 +860,10 @@
}
if ($this->getForceDelivery()) {
- // If we're forcing delivery, skip all the opt-out checks.
+ // If we're forcing delivery, skip all the opt-out checks. We don't
+ // bother annotating reasoning on the mail in this case because it should
+ // always be obvious why the mail hit this rule (e.g., it is a password
+ // reset mail).
foreach ($actors as $actor) {
$actor->setDeliverable(PhabricatorMetaMTAActor::REASON_FORCE);
}
@@ -867,6 +879,22 @@
$actor->setUndeliverable(PhabricatorMetaMTAActor::REASON_RESPONSE);
}
+ // Before running more rules, save a list of the actors who were
+ // deliverable before we started running preference-based rules. This stops
+ // us from trying to send mail to disabled users just because a Herald rule
+ // added them, for example.
+ $deliverable = array();
+ foreach ($actors as $phid => $actor) {
+ if ($actor->isDeliverable()) {
+ $deliverable[] = $phid;
+ }
+ }
+
+ // For the rest of the rules, order matters. We're going to run all the
+ // possible rules in order from weakest to strongest, and let the strongest
+ // matching rule win. The weaker rules leave annotations behind which help
+ // users understand why the mail was routed the way it was.
+
// Exclude the actor if their preferences are set.
$from_phid = $this->getParam('from');
$from_actor = idx($actors, $from_phid);
@@ -892,17 +920,6 @@
$actor_phids);
$all_prefs = mpull($all_prefs, null, 'getUserPHID');
- // Exclude recipients who don't want any mail.
- foreach ($all_prefs as $phid => $prefs) {
- $exclude = $prefs->getPreference(
- PhabricatorUserPreferences::PREFERENCE_NO_MAIL,
- false);
- if ($exclude) {
- $actors[$phid]->setUndeliverable(
- PhabricatorMetaMTAActor::REASON_MAIL_DISABLED);
- }
- }
-
$value_email = PhabricatorUserPreferences::MAILTAG_PREFERENCE_EMAIL;
// Exclude all recipients who have set preferences to not receive this type
@@ -932,6 +949,33 @@
}
}
+ // If recipients were initially deliverable and were added by "Send me an
+ // email" Herald rules, annotate them as such and make them deliverable
+ // again, overriding any changes made by the "self mail" and "mail tags"
+ // settings.
+ $force_recipients = $this->getForceHeraldMailRecipientPHIDs();
+ $force_recipients = array_fuse($force_recipients);
+ if ($force_recipients) {
+ foreach ($deliverable as $phid) {
+ if (isset($force_recipients[$phid])) {
+ $actors[$phid]->setDeliverable(
+ PhabricatorMetaMTAActor::REASON_FORCE_HERALD);
+ }
+ }
+ }
+
+ // Exclude recipients who don't want any mail. This rule is very strong
+ // and runs last.
+ foreach ($all_prefs as $phid => $prefs) {
+ $exclude = $prefs->getPreference(
+ PhabricatorUserPreferences::PREFERENCE_NO_MAIL,
+ false);
+ if ($exclude) {
+ $actors[$phid]->setUndeliverable(
+ PhabricatorMetaMTAActor::REASON_MAIL_DISABLED);
+ }
+ }
+
return $actors;
}
diff --git a/src/applications/phriction/editor/PhrictionTransactionEditor.php b/src/applications/phriction/editor/PhrictionTransactionEditor.php
--- a/src/applications/phriction/editor/PhrictionTransactionEditor.php
+++ b/src/applications/phriction/editor/PhrictionTransactionEditor.php
@@ -13,7 +13,6 @@
private $skipAncestorCheck;
private $contentVersion;
private $processContentVersionError = true;
- private $heraldEmailPHIDs = array();
public function setDescription($description) {
$this->description = $description;
@@ -369,16 +368,6 @@
);
}
- protected function getMailCC(PhabricatorLiskDAO $object) {
- $phids = array();
-
- foreach ($this->heraldEmailPHIDs as $phid) {
- $phids[] = $phid;
- }
-
- return $phids;
- }
-
public function getMailTagsMap() {
return array(
PhrictionTransaction::MAILTAG_TITLE =>
@@ -801,8 +790,6 @@
->setNewValue(array('+' => $value));
}
- $this->heraldEmailPHIDs = $adapter->getEmailPHIDs();
-
return $xactions;
}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1967,8 +1967,15 @@
return;
}
- $email_to = array_filter(array_unique($this->getMailTo($object)));
- $email_cc = array_filter(array_unique($this->getMailCC($object)));
+ $email_force = array();
+ $email_to = $this->getMailTo($object);
+ $email_cc = $this->getMailCC($object);
+
+ $adapter = $this->getHeraldAdapter();
+ if ($adapter) {
+ $email_cc = array_merge($email_cc, $adapter->getEmailPHIDs());
+ $email_force = $adapter->getForcedEmailPHIDs();
+ }
$phids = array_merge($email_to, $email_cc);
$handles = id(new PhabricatorHandleQuery())
@@ -1993,6 +2000,7 @@
->setThreadID($this->getMailThreadID($object), $this->getIsNewObject())
->setRelatedPHID($object->getPHID())
->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs())
+ ->setForceHeraldMailRecipientPHIDs($email_force)
->setMailTags($mail_tags)
->setIsBulk(true)
->setBody($body->render())
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Thu, Mar 20, 8:09 AM (1 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7712215
Default Alt Text
D12300.diff (11 KB)
Attached To
Mode
D12300: Allow "send me an email" in personal rules to punch through settings
Attached
Detach File
Event Timeline
Log In to Comment