1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

When Herald changes object subscribers, always hide the feed story

Summary:
Fixes T8952. These feed stories are not interesting and tend to be generated as collateral damage when a non-story update is made to an old task and someone has a "subscribe me" Herald rule.

Also clean up some of the Herald field/condition indexing behavior slightly.

Test Plan: Wrote a "Subscribe X" herald rule, made a trivial update to a task. Before: low-value feed story; after: no feed story.

Maniphest Tasks: T8952

Differential Revision: https://secure.phabricator.com/D20797
This commit is contained in:
epriestley 2019-09-09 13:09:46 -07:00
parent 4547714463
commit 7593a265d5
2 changed files with 19 additions and 17 deletions

View file

@ -177,27 +177,28 @@ abstract class HeraldField extends Phobject {
} }
public function getPHIDsAffectedByCondition(HeraldCondition $condition) { public function getPHIDsAffectedByCondition(HeraldCondition $condition) {
$phids = array(); try {
$standard_type = $this->getHeraldFieldStandardType(); $standard_type = $this->getHeraldFieldStandardType();
} catch (PhutilMethodNotImplementedException $ex) {
$standard_type = null;
}
switch ($standard_type) { switch ($standard_type) {
case self::STANDARD_PHID: case self::STANDARD_PHID:
case self::STANDARD_PHID_NULLABLE: case self::STANDARD_PHID_NULLABLE:
$phid = $condition->getValue();
if ($phid) {
$phids[] = $phid;
}
break;
case self::STANDARD_PHID_LIST: case self::STANDARD_PHID_LIST:
foreach ($condition->getValue() as $phid) { $phids = $condition->getValue();
$phids[] = $phid;
} if (!is_array($phids)) {
break; $phids = array();
} }
return $phids; return $phids;
} }
return array();
}
final public function setAdapter(HeraldAdapter $adapter) { final public function setAdapter(HeraldAdapter $adapter) {
$this->adapter = $adapter; $this->adapter = $adapter;
return $this; return $this;

View file

@ -775,6 +775,13 @@ abstract class PhabricatorApplicationTransaction
case PhabricatorTransactions::TYPE_TOKEN: case PhabricatorTransactions::TYPE_TOKEN:
case PhabricatorTransactions::TYPE_MFA: case PhabricatorTransactions::TYPE_MFA:
return true; return true;
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
// See T8952. When an application (usually Herald) modifies
// subscribers, this tends to be very uninteresting.
if ($this->isApplicationAuthor()) {
return true;
}
break;
case PhabricatorTransactions::TYPE_EDGE: case PhabricatorTransactions::TYPE_EDGE:
$edge_type = $this->getMetadataValue('edge:type'); $edge_type = $this->getMetadataValue('edge:type');
switch ($edge_type) { switch ($edge_type) {
@ -1387,12 +1394,6 @@ abstract class PhabricatorApplicationTransaction
return 25; return 25;
} }
if ($this->isApplicationAuthor()) {
// When applications (most often: Herald) change subscriptions it
// is very uninteresting.
return 1;
}
// In other cases, subscriptions are more interesting than comments // In other cases, subscriptions are more interesting than comments
// (which are shown anyway) but less interesting than any other type of // (which are shown anyway) but less interesting than any other type of
// transaction. // transaction.