Page MenuHomePhabricator
Files F13994297

D19021.id45616.diff
No OneTemporary
Actions

D19021.id45616.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
@@ -496,7 +496,6 @@
$phids[] = $object->getAuthorPHID();
}
- $status_resigned = PhabricatorAuditStatusConstants::RESIGNED;
foreach ($object->getAudits() as $audit) {
if (!$audit->isInteresting()) {
// Don't send mail to uninteresting auditors, like packages which
@@ -504,7 +503,7 @@
continue;
}
- if ($audit->getAuditStatus() != $status_resigned) {
+ if (!$audit->isResigned()) {
$phids[] = $audit->getAuditorPHID();
}
}
@@ -514,6 +513,18 @@
return $phids;
}
+ protected function newMailUnexpandablePHIDs(PhabricatorLiskDAO $object) {
+ $phids = array();
+
+ foreach ($object->getAudits() as $auditor) {
+ if ($auditor->isResigned()) {
+ $phids[] = $auditor->getAuditorPHID();
+ }
+ }
+
+ return $phids;
+ }
+
protected function buildMailBody(
PhabricatorLiskDAO $object,
array $xactions) {
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
@@ -644,6 +644,18 @@
return $phids;
}
+ protected function newMailUnexpandablePHIDs(PhabricatorLiskDAO $object) {
+ $phids = array();
+
+ foreach ($object->getReviewers() as $reviewer) {
+ if ($reviewer->isResigned()) {
+ $phids[] = $reviewer->getReviewerPHID();
+ }
+ }
+
+ return $phids;
+ }
+
protected function getMailAction(
PhabricatorLiskDAO $object,
array $xactions) {
diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php
--- a/src/applications/differential/storage/DifferentialRevision.php
+++ b/src/applications/differential/storage/DifferentialRevision.php
@@ -820,9 +820,15 @@
}
foreach ($reviewers as $reviewer) {
- if ($reviewer->getReviewerPHID() == $phid) {
- return true;
+ if ($reviewer->getReviewerPHID() !== $phid) {
+ continue;
}
+
+ if ($reviewer->isResigned()) {
+ continue;
+ }
+
+ return true;
}
return false;
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
@@ -6,6 +6,7 @@
private $applicationEmail;
private $actor;
private $excludePHIDs = array();
+ private $unexpandablePHIDs = array();
final public function setMailReceiver($mail_receiver) {
$this->validateMailReceiver($mail_receiver);
@@ -45,6 +46,15 @@
return $this->excludePHIDs;
}
+ public function setUnexpandablePHIDs(array $phids) {
+ $this->unexpandablePHIDs = $phids;
+ return $this;
+ }
+
+ public function getUnexpandablePHIDs() {
+ return $this->unexpandablePHIDs;
+ }
+
abstract public function validateMailReceiver($mail_receiver);
abstract public function getPrivateReplyHandlerEmailAddress(
PhabricatorUser $user);
@@ -297,6 +307,16 @@
$to_result = array();
$cc_result = array();
+ // "Unexpandable" users have disengaged from an object (for example,
+ // by resigning from a revision).
+
+ // If such a user is still a direct recipient (for example, they're still
+ // on the Subscribers list) they're fair game, but group targets (like
+ // projects) will no longer include them when expanded.
+
+ $unexpandable = $this->getUnexpandablePHIDs();
+ $unexpandable = array_fuse($unexpandable);
+
$all_phids = array_merge($to, $cc);
if ($all_phids) {
$map = id(new PhabricatorMetaMTAMemberQuery())
@@ -305,11 +325,21 @@
->execute();
foreach ($to as $phid) {
foreach ($map[$phid] as $expanded) {
+ if ($expanded !== $phid) {
+ if (isset($unexpandable[$expanded])) {
+ continue;
+ }
+ }
$to_result[$expanded] = $expanded;
}
}
foreach ($cc as $phid) {
foreach ($map[$phid] as $expanded) {
+ if ($expanded !== $phid) {
+ if (isset($unexpandable[$expanded])) {
+ continue;
+ }
+ }
$cc_result[$expanded] = $expanded;
}
}
diff --git a/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php b/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php
--- a/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryAuditRequest.php
@@ -72,6 +72,15 @@
return true;
}
+ public function isResigned() {
+ switch ($this->getAuditStatus()) {
+ case PhabricatorAuditStatusConstants::RESIGNED:
+ return true;
+ }
+
+ return false;
+ }
+
/* -( PhabricatorPolicyInterface )----------------------------------------- */
diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
--- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php
@@ -657,7 +657,8 @@
public function isAutomaticallySubscribed($phid) {
// TODO: This should also list auditors, but handling that is a bit messy
- // right now because we are not guaranteed to have the data.
+ // right now because we are not guaranteed to have the data. (It should not
+ // include resigned auditors.)
return ($phid == $this->getAuthorPHID());
}
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
@@ -77,6 +77,7 @@
private $oldTo = array();
private $oldCC = array();
private $mailRemovedPHIDs = array();
+ private $mailUnexpandablePHIDs = array();
private $transactionQueue = array();
@@ -1204,6 +1205,7 @@
$this->mailShouldSend = true;
$this->mailToPHIDs = $this->getMailTo($object);
$this->mailCCPHIDs = $this->getMailCC($object);
+ $this->mailUnexpandablePHIDs = $this->newMailUnexpandablePHIDs($object);
// Add any recipients who were previously on the notification list
// but were removed by this change.
@@ -2562,7 +2564,13 @@
$email_cc = $this->mailCCPHIDs;
$email_cc = array_merge($email_cc, $this->heraldEmailPHIDs);
+ $unexpandable = $this->mailUnexpandablePHIDs;
+ if (!is_array($unexpandable)) {
+ $unexpandable = array();
+ }
+
$targets = $this->buildReplyHandler($object)
+ ->setUnexpandablePHIDs($unexpandable)
->getMailTargets($email_to, $email_cc);
// Set this explicitly before we start swapping out the effective actor.
@@ -2817,6 +2825,11 @@
}
+ protected function newMailUnexpandablePHIDs(PhabricatorLiskDAO $object) {
+ return array();
+ }
+
+
/**
* @task mail
*/
@@ -3617,6 +3630,7 @@
'mailShouldSend',
'mustEncrypt',
'mailStamps',
+ 'mailUnexpandablePHIDs',
);
}

File Metadata

Mime Type
text/plain
Expires
Thu, Oct 24, 5:12 AM (3 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6745890
Default Alt Text
D19021.id45616.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility