Page MenuHomePhabricator
Files F15410226

D19179.diff
No OneTemporary
Actions

D19179.diff
View Options

diff --git a/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php b/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php
--- a/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php
+++ b/src/applications/differential/xaction/DifferentialRevisionWrongStateTransaction.php
@@ -35,7 +35,8 @@
$this->renderValue($status->getDisplayName()));
}
- public function getTitleForFeed() {
- return null;
+ public function shouldHideForFeed() {
+ return true;
}
+
}
diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php
--- a/src/applications/maniphest/storage/ManiphestTransaction.php
+++ b/src/applications/maniphest/storage/ManiphestTransaction.php
@@ -40,22 +40,6 @@
return parent::shouldGenerateOldValue();
}
- public function shouldHideForFeed() {
- // NOTE: Modular transactions don't currently support this, and it has
- // very few callsites, and it's publish-time rather than display-time.
- // This should probably become a supported, display-time behavior. For
- // discussion, see T12787.
-
- // Hide "alice created X, a task blocking Y." from feed because it
- // will almost always appear adjacent to "alice created Y".
- $is_new = $this->getMetadataValue('blocker.new');
- if ($is_new) {
- return true;
- }
-
- return parent::shouldHideForFeed();
- }
-
public function getRequiredHandlePHIDs() {
$phids = parent::getRequiredHandlePHIDs();
diff --git a/src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php
--- a/src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php
+++ b/src/applications/maniphest/xaction/ManiphestTaskUnblockTransaction.php
@@ -112,5 +112,15 @@
return 'fa-shield';
}
+ public function shouldHideForFeed() {
+ // Hide "alice created X, a task blocking Y." from feed because it
+ // will almost always appear adjacent to "alice created Y".
+ $is_new = $this->getMetadataValue('blocker.new');
+ if ($is_new) {
+ return true;
+ }
+
+ return parent::shouldHideForFeed();
+ }
}
diff --git a/src/applications/phriction/storage/PhrictionTransaction.php b/src/applications/phriction/storage/PhrictionTransaction.php
--- a/src/applications/phriction/storage/PhrictionTransaction.php
+++ b/src/applications/phriction/storage/PhrictionTransaction.php
@@ -54,18 +54,6 @@
return parent::shouldHideForMail($xactions);
}
- public function shouldHideForFeed() {
- switch ($this->getTransactionType()) {
- case PhrictionDocumentMoveToTransaction::TRANSACTIONTYPE:
- case PhrictionDocumentMoveAwayTransaction::TRANSACTIONTYPE:
- return true;
- case PhrictionDocumentTitleTransaction::TRANSACTIONTYPE:
- return $this->getMetadataValue('stub:create:phid', false);
- }
- return parent::shouldHideForFeed();
- }
-
-
public function getMailTags() {
$tags = array();
switch ($this->getTransactionType()) {
diff --git a/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php
--- a/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php
+++ b/src/applications/phriction/xaction/PhrictionDocumentMoveAwayTransaction.php
@@ -59,4 +59,8 @@
return 'fa-arrows';
}
+ public function shouldHideForFeed() {
+ return true;
+ }
+
}
diff --git a/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php
--- a/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php
+++ b/src/applications/phriction/xaction/PhrictionDocumentMoveToTransaction.php
@@ -102,4 +102,8 @@
return 'fa-arrows';
}
+ public function shouldHideForFeed() {
+ return true;
+ }
+
}
diff --git a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php
--- a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php
+++ b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php
@@ -6,6 +6,8 @@
class PhabricatorApplicationTransactionFeedStory
extends PhabricatorFeedStory {
+ private $primaryTransactionPHID;
+
public function getPrimaryObjectPHID() {
return $this->getValue('objectPHID');
}
@@ -27,7 +29,36 @@
}
protected function getPrimaryTransactionPHID() {
- return head($this->getValue('transactionPHIDs'));
+ if ($this->primaryTransactionPHID === null) {
+ // Transactions are filtered and sorted before they're stored, but the
+ // rendering logic can change between the time an edit occurs and when
+ // we actually render the story. Recalculate the filtering at display
+ // time because it's cheap and gets us better results when things change
+ // by letting the changes apply retroactively.
+
+ $xaction_phids = $this->getValue('transactionPHIDs');
+
+ $xactions = array();
+ foreach ($xaction_phids as $xaction_phid) {
+ $xactions[] = $this->getObject($xaction_phid);
+ }
+
+ foreach ($xactions as $key => $xaction) {
+ if ($xaction->shouldHideForFeed()) {
+ unset($xactions[$key]);
+ }
+ }
+
+ if ($xactions) {
+ $primary_phid = head($xactions)->getPHID();
+ } else {
+ $primary_phid = head($xaction_phids);
+ }
+
+ $this->primaryTransactionPHID = $primary_phid;
+ }
+
+ return $this->primaryTransactionPHID;
}
public function getPrimaryTransaction() {
diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php
--- a/src/applications/transactions/storage/PhabricatorModularTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php
@@ -92,6 +92,14 @@
return parent::shouldHide();
}
+ final public function shouldHideForFeed() {
+ if ($this->getTransactionImplementation()->shouldHideForFeed()) {
+ return true;
+ }
+
+ return parent::shouldHideForFeed();
+ }
+
/* final */ public function getIcon() {
$icon = $this->getTransactionImplementation()->getIcon();
if ($icon !== null) {
diff --git a/src/applications/transactions/storage/PhabricatorModularTransactionType.php b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
--- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php
@@ -47,6 +47,10 @@
return false;
}
+ public function shouldHideForFeed() {
+ return false;
+ }
+
public function getIcon() {
return null;
}

File Metadata

Mime Type
text/plain
Expires
Thu, Mar 20, 6:55 AM (2 d, 19 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7712433
Default Alt Text
D19179.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility