diff --git a/src/applications/feed/query/PhabricatorFeedQuery.php b/src/applications/feed/query/PhabricatorFeedQuery.php index ce472a6026..a35f14da57 100644 --- a/src/applications/feed/query/PhabricatorFeedQuery.php +++ b/src/applications/feed/query/PhabricatorFeedQuery.php @@ -34,7 +34,15 @@ final class PhabricatorFeedQuery } protected function willFilterPage(array $data) { - return PhabricatorFeedStory::loadAllFromRows($data, $this->getViewer()); + $stories = PhabricatorFeedStory::loadAllFromRows($data, $this->getViewer()); + + foreach ($stories as $key => $story) { + if (!$story->isVisibleInFeed()) { + unset($stories[$key]); + } + } + + return $stories; } protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) { diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index c3984573c7..e0c65c7dc2 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -418,6 +418,14 @@ abstract class PhabricatorFeedStory return array(); } + public function isVisibleInFeed() { + return true; + } + + public function isVisibleInNotifications() { + return true; + } + /* -( PhabricatorPolicyInterface Implementation )-------------------------- */ diff --git a/src/applications/notification/query/PhabricatorNotificationQuery.php b/src/applications/notification/query/PhabricatorNotificationQuery.php index b40bdee066..0063c06e45 100644 --- a/src/applications/notification/query/PhabricatorNotificationQuery.php +++ b/src/applications/notification/query/PhabricatorNotificationQuery.php @@ -53,13 +53,13 @@ final class PhabricatorNotificationQuery $data = queryfx_all( $conn, - 'SELECT story.*, notif.hasViewed FROM %T notif - JOIN %T story ON notif.chronologicalKey = story.chronologicalKey + 'SELECT story.*, notif.hasViewed FROM %R notif + JOIN %R story ON notif.chronologicalKey = story.chronologicalKey %Q ORDER BY notif.chronologicalKey DESC %Q', - $notification_table->getTableName(), - $story_table->getTableName(), + $notification_table, + $story_table, $this->buildWhereClause($conn), $this->buildLimitClause($conn)); @@ -93,7 +93,7 @@ final class PhabricatorNotificationQuery (int)!$this->unread); } - if ($this->keys) { + if ($this->keys !== null) { $where[] = qsprintf( $conn, 'notif.chronologicalKey IN (%Ls)', @@ -103,6 +103,16 @@ final class PhabricatorNotificationQuery return $where; } + protected function willFilterPage(array $stories) { + foreach ($stories as $key => $story) { + if (!$story->isVisibleInNotifications()) { + unset($stories[$key]); + } + } + + return $stories; + } + protected function getResultCursor($item) { return $item->getChronologicalKey(); } diff --git a/src/applications/people/xaction/PhabricatorUserNotifyTransaction.php b/src/applications/people/xaction/PhabricatorUserNotifyTransaction.php index bc5657d22e..4a527f8265 100644 --- a/src/applications/people/xaction/PhabricatorUserNotifyTransaction.php +++ b/src/applications/people/xaction/PhabricatorUserNotifyTransaction.php @@ -23,10 +23,14 @@ final class PhabricatorUserNotifyTransaction return $this->getNewValue(); } - public function shouldHideForFeed() { + public function shouldHideForNotifications() { return false; } + public function shouldHideForFeed() { + return true; + } + public function shouldHideForMail() { return true; } diff --git a/src/applications/settings/panel/PhabricatorNotificationsSettingsPanel.php b/src/applications/settings/panel/PhabricatorNotificationsSettingsPanel.php index e75c2b99ee..797bcafcb3 100644 --- a/src/applications/settings/panel/PhabricatorNotificationsSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorNotificationsSettingsPanel.php @@ -143,7 +143,7 @@ final class PhabricatorNotificationsSettingsPanel ->setCaption( pht( 'Phabricator can send real-time notifications to your web browser '. - 'or to your desktop. Select where you\'d want to receive these '. + 'or to your desktop. Select where you want to receive these '. 'real-time updates.')) ->initBehavior( 'desktop-notifications-control', diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 9e946a249e..738fcf098b 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -3428,7 +3428,19 @@ abstract class PhabricatorApplicationTransactionEditor array $xactions, array $mailed_phids) { - $xactions = mfilter($xactions, 'shouldHideForFeed', true); + // Remove transactions which don't publish feed stories or notifications. + // These never show up anywhere, so we don't need to do anything with them. + foreach ($xactions as $key => $xaction) { + if (!$xaction->shouldHideForFeed()) { + continue; + } + + if (!$xaction->shouldHideForNotifications()) { + continue; + } + + unset($xactions[$key]); + } if (!$xactions) { return; diff --git a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php index 305e38ab09..0e8531a22d 100644 --- a/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php +++ b/src/applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php @@ -36,12 +36,7 @@ class PhabricatorApplicationTransactionFeedStory // 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); - } + $xactions = $this->getTransactions(); foreach ($xactions as $key => $xaction) { if ($xaction->shouldHideForFeed()) { @@ -52,7 +47,7 @@ class PhabricatorApplicationTransactionFeedStory if ($xactions) { $primary_phid = head($xactions)->getPHID(); } else { - $primary_phid = head($xaction_phids); + $primary_phid = head($this->getValue('transactionPHIDs')); } $this->primaryTransactionPHID = $primary_phid; @@ -61,6 +56,41 @@ class PhabricatorApplicationTransactionFeedStory return $this->primaryTransactionPHID; } + public function isVisibleInNotifications() { + $xactions = $this->getTransactions(); + + foreach ($xactions as $key => $xaction) { + if (!$xaction->shouldHideForNotifications()) { + return true; + } + } + + return false; + } + + public function isVisibleInFeed() { + $xactions = $this->getTransactions(); + + foreach ($xactions as $key => $xaction) { + if (!$xaction->shouldHideForFeed()) { + return true; + } + } + + return false; + } + + private function getTransactions() { + $xaction_phids = $this->getValue('transactionPHIDs'); + + $xactions = array(); + foreach ($xaction_phids as $xaction_phid) { + $xactions[] = $this->getObject($xaction_phid); + } + + return $xactions; + } + public function getPrimaryTransaction() { return $this->getObject($this->getPrimaryTransactionPHID()); } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 9e024b43aa..f38c56acb3 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -759,6 +759,10 @@ abstract class PhabricatorApplicationTransaction return $this->shouldHide(); } + public function shouldHideForNotifications() { + return $this->shouldHideForFeed(); + } + public function getTitleForMail() { return id(clone $this)->setRenderingTarget('text')->getTitle(); } diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php index eef4b8e5d9..f6aece2a7f 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransaction.php +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -108,6 +108,17 @@ abstract class PhabricatorModularTransaction return parent::shouldHideForMail($xactions); } + final public function shouldHideForNotifications() { + $hide = $this->getTransactionImplementation()->shouldHideForNotifications(); + + // Returning "null" means "use the default behavior". + if ($hide === null) { + return parent::shouldHideForNotifications(); + } + + return $hide; + } + /* 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 index ca82af1c66..35dd3ac197 100644 --- a/src/applications/transactions/storage/PhabricatorModularTransactionType.php +++ b/src/applications/transactions/storage/PhabricatorModularTransactionType.php @@ -59,6 +59,10 @@ abstract class PhabricatorModularTransactionType return false; } + public function shouldHideForNotifications() { + return null; + } + public function getIcon() { return null; }