jenkins-bot has submitted this change and it was merged.
Change subject: Add filter to ApiEchoNotifications
......................................................................
Add filter to ApiEchoNotifications
It lets you query for read/unread/all notifications.
Bug: T119890
Change-Id: I5d09b5ff9474c77577734e9ceb0bcbcad6c99c0c
---
M includes/api/ApiEchoNotifications.php
M includes/mapper/NotificationMapper.php
2 files changed, 91 insertions(+), 55 deletions(-)
Approvals:
Legoktm: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/api/ApiEchoNotifications.php
b/includes/api/ApiEchoNotifications.php
index ba539fe..0c2ac92 100644
--- a/includes/api/ApiEchoNotifications.php
+++ b/includes/api/ApiEchoNotifications.php
@@ -24,7 +24,7 @@
if ( $params['groupbysection'] ) {
foreach ( $params['sections'] as $section ) {
$result[$section] =
$this->getSectionPropList(
- $user, $section,
$params['limit'],
+ $user, $section,
$params['filter'], $params['limit'],
$params[$section . 'continue'],
$params['format'], $params[$section . 'unreadfirst']
);
$this->getResult()->setIndexedTagName(
$result[$section]['list'], 'notification' );
@@ -39,7 +39,7 @@
$result = $this->getPropList(
$user,
$attributeManager->getUserEnabledEventsbySections( $user, 'web',
$params['sections'] ),
- $params['limit'], $params['continue'],
$params['format']
+ $params['filter'], $params['limit'],
$params['continue'], $params['format']
);
$this->getResult()->setIndexedTagName(
$result['list'], 'notification' );
// 'index' is built on top of 'list'
@@ -65,13 +65,14 @@
* Internal method for getting the property 'list' data for individual
section
* @param User $user
* @param string $section 'alert' or 'message'
+ * @param string $filter 'all', 'read' or 'unread'
* @param int $limit
* @param string $continue
* @param string $format
* @param boolean $unreadFirst
* @return array
*/
- protected function getSectionPropList( User $user, $section, $limit,
$continue, $format, $unreadFirst = false ) {
+ protected function getSectionPropList( User $user, $section, $filter,
$limit, $continue, $format, $unreadFirst = false ) {
$attributeManager = EchoAttributeManager::newFromGlobalVars();
$sectionEvents =
$attributeManager->getUserEnabledEventsbySections( $user, 'web', array(
$section ) );
@@ -82,7 +83,7 @@
);
} else {
$result = $this->getPropList(
- $user, $sectionEvents, $limit, $continue,
$format, $unreadFirst
+ $user, $sectionEvents, $filter, $limit,
$continue, $format, $unreadFirst
);
}
@@ -95,13 +96,14 @@
* of a set of sections or a single section
* @param User $user
* @param string[] $eventTypes
+ * @param string $filter 'all', 'read' or 'unread'
* @param int $limit
* @param string $continue
* @param string $format
* @param boolean $unreadFirst
* @return array
*/
- protected function getPropList( User $user, array $eventTypes, $limit,
$continue, $format, $unreadFirst = false ) {
+ protected function getPropList( User $user, array $eventTypes, $filter,
$limit, $continue, $format, $unreadFirst = false ) {
$result = array(
'list' => array(),
'continue' => null
@@ -109,57 +111,66 @@
$notifMapper = new EchoNotificationMapper();
- // Prefer unread notifications. We don't care about next offset
in this case
- if ( $unreadFirst ) {
- // query for unread notifications past 'continue'
(offset)
+ // check if we want both read & unread...
+ if ( in_array( 'read', $filter ) && in_array( '!read', $filter
) ) {
+ // Prefer unread notifications. We don't care about
next offset in this case
+ if ( $unreadFirst ) {
+ // query for unread notifications past
'continue' (offset)
+ $notifs = $notifMapper->fetchUnreadByUser(
$user, $limit + 1, $continue, $eventTypes );
+
+ /*
+ * 'continue' has a timestamp & id (to start
with, in case
+ * there would be multiple events with that
same timestamp)
+ * Unread notifications should always load
first, but may be
+ * older than read ones, but we can work with
current
+ * 'continue' format:
+ * * if there's no continue, first load unread
notifications
+ * * if there's a continue, fetch unread
notifications first
+ * * if there are no unread ones, continue
must've been
+ * about read notifications: fetch 'em
+ * * if there are unread ones but first one
doesn't match
+ * continue id, it must've been about read
notifications:
+ * discard unread & fetch read
+ */
+ if ( $notifs && $continue ) {
+ /** @var EchoNotification $first */
+ $first = reset( $notifs );
+ $continueId = intval( trim( strrchr(
$continue, '|' ), '|' ) );
+ if ( $first->getEvent()->getID() !==
$continueId ) {
+ // notification doesn't match
continue id, it must've been
+ // about read notifications:
discard all unread ones
+ $notifs = array();
+ }
+ }
+
+ // If there are less unread notifications than
we requested,
+ // then fill the result with some read
notifications
+ $count = count( $notifs );
+ // we need 1 more than $limit, so we can
respond 'continue'
+ if ( $count <= $limit ) {
+ // Query planner should be smart enough
that passing a short list of ids to exclude
+ // will only visit at most that number
of extra rows.
+ $mixedNotifs =
$notifMapper->fetchByUser(
+ $user,
+ $limit - $count + 1,
+ // if there were unread
notifications, 'continue' was for
+ // unread notifications and we
should start fetching read
+ // notifications from start
+ $count > 0 ? null : $continue,
+ $eventTypes,
+ array_keys( $notifs )
+ );
+ foreach ( $mixedNotifs as $notif ) {
+
$notifs[$notif->getEvent()->getId()] = $notif;
+ }
+ }
+ } else {
+ $notifs = $notifMapper->fetchByUser( $user,
$limit + 1, $continue, $eventTypes );
+ }
+ } elseif ( in_array( 'read', $filter ) ) {
+ $notifs = $notifMapper->fetchReadByUser( $user, $limit
+ 1, $continue, $eventTypes );
+ } else { // = if ( in_array( '!read', $filter ) ) {
$notifs = $notifMapper->fetchUnreadByUser( $user,
$limit + 1, $continue, $eventTypes );
-
- /*
- * 'continue' has a timestamp & id (to start with, in
case there
- * would be multiple events with that same timestamp)
- * Unread notifications should always load first, but
may be older
- * than read ones, but we can work with current
'continue' format:
- * * if there is no continue, first load unread
notifications
- * * if there is a continue, fetch unread notifications
first:
- * * if there are no unread ones, continue must've been
about read:
- * fetch 'em
- * * if there are unread ones but first one doesn't
match continue
- * id, it must've been about read: discard unread &
fetch read
- */
- if ( $notifs && $continue ) {
- /** @var EchoNotification $first */
- $first = reset( $notifs );
- $continueId = intval( trim( strrchr( $continue,
'|' ), '|' ) );
- if ( $first->getEvent()->getID() !==
$continueId ) {
- // notification doesn't match continue
id, it must've been
- // about read notifications: discard
all unread ones
- $notifs = array();
- }
- }
-
- // If there are less unread notifications than we
requested,
- // then fill the result with some read notifications
- $count = count( $notifs );
- // we need 1 more than $limit, so we can respond
'continue'
- if ( $count <= $limit ) {
- // Query planner should be smart enough that
passing a short list of ids to exclude
- // will only visit at most that number of extra
rows.
- $mixedNotifs = $notifMapper->fetchByUser(
- $user,
- $limit - $count + 1,
- // if there were unread notifications,
'continue' was for
- // unread notifications and we should
start fetching read
- // notifications from start
- $count > 0 ? null : $continue,
- $eventTypes,
- array_keys( $notifs )
- );
- foreach ( $mixedNotifs as $notif ) {
- $notifs[$notif->getEvent()->getId()] =
$notif;
- }
- }
- } else {
- $notifs = $notifMapper->fetchByUser( $user, $limit + 1,
$continue, $eventTypes );
}
foreach ( $notifs as $notif ) {
@@ -224,6 +235,14 @@
public function getAllowedParams() {
$sections = EchoAttributeManager::$sections;
$params = array(
+ 'filter' => array(
+ ApiBase::PARAM_ISMULTI => true,
+ ApiBase::PARAM_DFLT => 'read|!read',
+ ApiBase::PARAM_TYPE => array(
+ 'read',
+ '!read',
+ ),
+ ),
'prop' => array(
ApiBase::PARAM_ISMULTI => true,
ApiBase::PARAM_TYPE => array(
diff --git a/includes/mapper/NotificationMapper.php
b/includes/mapper/NotificationMapper.php
index 93a41f0..bdcdc8b 100644
--- a/includes/mapper/NotificationMapper.php
+++ b/includes/mapper/NotificationMapper.php
@@ -105,6 +105,23 @@
}
/**
+ * Get read notifications by user in the amount specified by limit
order by
+ * notification timestamp in descending order. We have an index to
retrieve
+ * unread notifications but it's not optimized for ordering by
timestamp. The
+ * descending order is only allowed if we keep the notification in low
volume,
+ * which is done via a deleteJob
+ * @param User $user
+ * @param int $limit
+ * @param string $continue Used for offset
+ * @param string[] $eventTypes
+ * @param int $dbSource Use master or slave database
+ * @return EchoNotification[]
+ */
+ public function fetchReadByUser( User $user, $limit, $continue, array
$eventTypes = array(), $dbSource = DB_SLAVE ) {
+ return $this->fetchByUserInternal( $user, $limit, $continue,
$eventTypes, array( 'notification_read_timestamp IS NOT NULL' ), $dbSource );
+ }
+
+ /**
* Get Notification by user in batch along with limit, offset etc
*
* @param User $user the user to get notifications for
--
To view, visit https://gerrit.wikimedia.org/r/258162
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5d09b5ff9474c77577734e9ceb0bcbcad6c99c0c
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
