Page MenuHomePhabricator
Files F14103629

D20284.id48420.diff
No OneTemporary
Actions

D20284.id48420.diff
View Options

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
@@ -1816,31 +1816,52 @@
$users = mpull($users, null, 'getPHID');
foreach ($phids as $key => $phid) {
- // Do not subscribe mentioned users
- // who do not have VIEW Permissions
- if ($object instanceof PhabricatorPolicyInterface
- && !PhabricatorPolicyFilter::hasCapability(
- $users[$phid],
- $object,
- PhabricatorPolicyCapability::CAN_VIEW)
- ) {
+ $user = idx($users, $phid);
+
+ // Don't subscribe invalid users.
+ if (!$user) {
unset($phids[$key]);
- } else {
- if ($object->isAutomaticallySubscribed($phid)) {
+ continue;
+ }
+
+ // Don't subscribe bots that get mentioned. If users truly intend
+ // to subscribe them, they can add them explicitly, but it's generally
+ // not useful to subscribe bots to objects.
+ if ($user->getIsSystemAgent()) {
+ unset($phids[$key]);
+ continue;
+ }
+
+ // Do not subscribe mentioned users who do not have permission to see
+ // the object.
+ if ($object instanceof PhabricatorPolicyInterface) {
+ $can_view = PhabricatorPolicyFilter::hasCapability(
+ $user,
+ $object,
+ PhabricatorPolicyCapability::CAN_VIEW);
+ if (!$can_view) {
unset($phids[$key]);
+ continue;
}
}
+
+ // Don't subscribe users who are already automatically subscribed.
+ if ($object->isAutomaticallySubscribed($phid)) {
+ unset($phids[$key]);
+ continue;
+ }
}
+
$phids = array_values($phids);
}
- // No else here to properly return null should we unset all subscriber
+
if (!$phids) {
return null;
}
- $xaction = newv(get_class(head($xactions)), array());
- $xaction->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS);
- $xaction->setNewValue(array('+' => $phids));
+ $xaction = $object->getApplicationTransactionTemplate()
+ ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
+ ->setNewValue(array('+' => $phids));
return $xaction;
}
@@ -2876,6 +2897,24 @@
}
}
+ $actor = $this->getActor();
+
+ $user = id(new PhabricatorPeopleQuery())
+ ->setViewer($actor)
+ ->withPHIDs(array($actor_phid))
+ ->executeOne();
+ if (!$user) {
+ return $xactions;
+ }
+
+ // When a bot acts (usually via the API), don't automatically subscribe
+ // them as a side effect. They can always subscribe explicitly if they
+ // want, and bot subscriptions normally just clutter things up since bots
+ // usually do not read email.
+ if ($user->getIsSystemAgent()) {
+ return $xactions;
+ }
+
$xaction = newv(get_class(head($xactions)), array());
$xaction->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS);
$xaction->setNewValue(array('+' => array($actor_phid)));

File Metadata

Mime Type
text/plain
Expires
Wed, Nov 27, 10:20 PM (18 h, 36 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6790820
Default Alt Text
D20284.id48420.diff (3 KB)

Event Timeline

Phabricator · Built by Phacility