mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-24 06:20:56 +01:00
Separate "feed" and "notifications" better, allow stories to appear in notifications only
Summary: Depends on D19861. Ref T13222. See PHI996. Fixes T10743. Currently, notifications only work if a story also has a feed rendering. Separate "visible in feed" and "visible in notifications", and make notifications query only notifications and vice versa. Then, set the test notification stories to be visible in notifications only, not feed. This could be refined a bit (there's no way to have the two views render different values today, for example) but since the only actual use case we have right now is test notifications I don't want to go //too// crazy future-proofing it. I could imagine doing some more of this kind of stuff in Conpherence eventually, though, perhaps. Test Plan: Sent myself test notifications, saw them appear on my profile timeline and in the JS popup, and in my notifications menu, but not in feed. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13222, T10743 Differential Revision: https://secure.phabricator.com/D19864
This commit is contained in:
parent
ba83380565
commit
773b4eaa9e
10 changed files with 107 additions and 16 deletions
|
@ -34,7 +34,15 @@ final class PhabricatorFeedQuery
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function willFilterPage(array $data) {
|
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) {
|
protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
|
||||||
|
|
|
@ -418,6 +418,14 @@ abstract class PhabricatorFeedStory
|
||||||
return array();
|
return array();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function isVisibleInFeed() {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function isVisibleInNotifications() {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* -( PhabricatorPolicyInterface Implementation )-------------------------- */
|
/* -( PhabricatorPolicyInterface Implementation )-------------------------- */
|
||||||
|
|
||||||
|
|
|
@ -53,13 +53,13 @@ final class PhabricatorNotificationQuery
|
||||||
|
|
||||||
$data = queryfx_all(
|
$data = queryfx_all(
|
||||||
$conn,
|
$conn,
|
||||||
'SELECT story.*, notif.hasViewed FROM %T notif
|
'SELECT story.*, notif.hasViewed FROM %R notif
|
||||||
JOIN %T story ON notif.chronologicalKey = story.chronologicalKey
|
JOIN %R story ON notif.chronologicalKey = story.chronologicalKey
|
||||||
%Q
|
%Q
|
||||||
ORDER BY notif.chronologicalKey DESC
|
ORDER BY notif.chronologicalKey DESC
|
||||||
%Q',
|
%Q',
|
||||||
$notification_table->getTableName(),
|
$notification_table,
|
||||||
$story_table->getTableName(),
|
$story_table,
|
||||||
$this->buildWhereClause($conn),
|
$this->buildWhereClause($conn),
|
||||||
$this->buildLimitClause($conn));
|
$this->buildLimitClause($conn));
|
||||||
|
|
||||||
|
@ -93,7 +93,7 @@ final class PhabricatorNotificationQuery
|
||||||
(int)!$this->unread);
|
(int)!$this->unread);
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->keys) {
|
if ($this->keys !== null) {
|
||||||
$where[] = qsprintf(
|
$where[] = qsprintf(
|
||||||
$conn,
|
$conn,
|
||||||
'notif.chronologicalKey IN (%Ls)',
|
'notif.chronologicalKey IN (%Ls)',
|
||||||
|
@ -103,6 +103,16 @@ final class PhabricatorNotificationQuery
|
||||||
return $where;
|
return $where;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function willFilterPage(array $stories) {
|
||||||
|
foreach ($stories as $key => $story) {
|
||||||
|
if (!$story->isVisibleInNotifications()) {
|
||||||
|
unset($stories[$key]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return $stories;
|
||||||
|
}
|
||||||
|
|
||||||
protected function getResultCursor($item) {
|
protected function getResultCursor($item) {
|
||||||
return $item->getChronologicalKey();
|
return $item->getChronologicalKey();
|
||||||
}
|
}
|
||||||
|
|
|
@ -23,10 +23,14 @@ final class PhabricatorUserNotifyTransaction
|
||||||
return $this->getNewValue();
|
return $this->getNewValue();
|
||||||
}
|
}
|
||||||
|
|
||||||
public function shouldHideForFeed() {
|
public function shouldHideForNotifications() {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function shouldHideForFeed() {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
public function shouldHideForMail() {
|
public function shouldHideForMail() {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
|
@ -143,7 +143,7 @@ final class PhabricatorNotificationsSettingsPanel
|
||||||
->setCaption(
|
->setCaption(
|
||||||
pht(
|
pht(
|
||||||
'Phabricator can send real-time notifications to your web browser '.
|
'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.'))
|
'real-time updates.'))
|
||||||
->initBehavior(
|
->initBehavior(
|
||||||
'desktop-notifications-control',
|
'desktop-notifications-control',
|
||||||
|
|
|
@ -3428,7 +3428,19 @@ abstract class PhabricatorApplicationTransactionEditor
|
||||||
array $xactions,
|
array $xactions,
|
||||||
array $mailed_phids) {
|
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) {
|
if (!$xactions) {
|
||||||
return;
|
return;
|
||||||
|
|
|
@ -36,12 +36,7 @@ class PhabricatorApplicationTransactionFeedStory
|
||||||
// time because it's cheap and gets us better results when things change
|
// time because it's cheap and gets us better results when things change
|
||||||
// by letting the changes apply retroactively.
|
// by letting the changes apply retroactively.
|
||||||
|
|
||||||
$xaction_phids = $this->getValue('transactionPHIDs');
|
$xactions = $this->getTransactions();
|
||||||
|
|
||||||
$xactions = array();
|
|
||||||
foreach ($xaction_phids as $xaction_phid) {
|
|
||||||
$xactions[] = $this->getObject($xaction_phid);
|
|
||||||
}
|
|
||||||
|
|
||||||
foreach ($xactions as $key => $xaction) {
|
foreach ($xactions as $key => $xaction) {
|
||||||
if ($xaction->shouldHideForFeed()) {
|
if ($xaction->shouldHideForFeed()) {
|
||||||
|
@ -52,7 +47,7 @@ class PhabricatorApplicationTransactionFeedStory
|
||||||
if ($xactions) {
|
if ($xactions) {
|
||||||
$primary_phid = head($xactions)->getPHID();
|
$primary_phid = head($xactions)->getPHID();
|
||||||
} else {
|
} else {
|
||||||
$primary_phid = head($xaction_phids);
|
$primary_phid = head($this->getValue('transactionPHIDs'));
|
||||||
}
|
}
|
||||||
|
|
||||||
$this->primaryTransactionPHID = $primary_phid;
|
$this->primaryTransactionPHID = $primary_phid;
|
||||||
|
@ -61,6 +56,41 @@ class PhabricatorApplicationTransactionFeedStory
|
||||||
return $this->primaryTransactionPHID;
|
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() {
|
public function getPrimaryTransaction() {
|
||||||
return $this->getObject($this->getPrimaryTransactionPHID());
|
return $this->getObject($this->getPrimaryTransactionPHID());
|
||||||
}
|
}
|
||||||
|
|
|
@ -759,6 +759,10 @@ abstract class PhabricatorApplicationTransaction
|
||||||
return $this->shouldHide();
|
return $this->shouldHide();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function shouldHideForNotifications() {
|
||||||
|
return $this->shouldHideForFeed();
|
||||||
|
}
|
||||||
|
|
||||||
public function getTitleForMail() {
|
public function getTitleForMail() {
|
||||||
return id(clone $this)->setRenderingTarget('text')->getTitle();
|
return id(clone $this)->setRenderingTarget('text')->getTitle();
|
||||||
}
|
}
|
||||||
|
|
|
@ -108,6 +108,17 @@ abstract class PhabricatorModularTransaction
|
||||||
return parent::shouldHideForMail($xactions);
|
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() {
|
/* final */ public function getIcon() {
|
||||||
$icon = $this->getTransactionImplementation()->getIcon();
|
$icon = $this->getTransactionImplementation()->getIcon();
|
||||||
if ($icon !== null) {
|
if ($icon !== null) {
|
||||||
|
|
|
@ -59,6 +59,10 @@ abstract class PhabricatorModularTransactionType
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function shouldHideForNotifications() {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
public function getIcon() {
|
public function getIcon() {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue