jenkins-bot has submitted this change and it was merged.
Change subject: Also support continuation requests for $unreadFirst
......................................................................
Also support continuation requests for $unreadFirst
Right now, it'll only respond a certain, fixed, amount,
not allowing you to paginate the list.
Note: haven't properly tested all possible cases yet!
Change-Id: I84761b13a1b9203cb8e3fcc80941d739cd28659f
---
M includes/NotifUser.php
M includes/api/ApiEchoNotifications.php
M includes/formatters/EchoFlyoutFormatter.php
M includes/mapper/NotificationMapper.php
M tests/phpunit/mapper/NotificationMapperTest.php
5 files changed, 57 insertions(+), 29 deletions(-)
Approvals:
Legoktm: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/NotifUser.php b/includes/NotifUser.php
index 4f5dcac..a6927e7 100644
--- a/includes/NotifUser.php
+++ b/includes/NotifUser.php
@@ -309,7 +309,7 @@
$eventTypesToLoad =
$attributeManager->getUserEnabledEventsbySections( $this->mUser, 'web', array(
$section ) );
}
- $notifications = $this->notifMapper->fetchUnreadByUser(
$this->mUser, 1, $eventTypesToLoad, $dbSource );
+ $notifications = $this->notifMapper->fetchUnreadByUser(
$this->mUser, 1, null, $eventTypesToLoad, $dbSource );
if ( $notifications ) {
$notification = reset( $notifications );
$timestamp = $notification->getTimestamp();
@@ -344,7 +344,7 @@
// After this 'mark read', is there any unread
edit-user-talk
// remaining? If not, we should clear the newtalk flag.
if ( $this->mUser->getNewtalk() ) {
- $unreadEditUserTalk =
$this->notifMapper->fetchUnreadByUser( $this->mUser, 1, array( 'edit-user-talk'
), DB_MASTER );
+ $unreadEditUserTalk =
$this->notifMapper->fetchUnreadByUser( $this->mUser, 1, null, array(
'edit-user-talk' ), DB_MASTER );
if ( count( $unreadEditUserTalk ) === 0 ) {
$this->mUser->setNewtalk( false );
}
@@ -379,7 +379,7 @@
$attributeManager = EchoAttributeManager::newFromGlobalVars();
$eventTypes =
$attributeManager->getUserEnabledEventsbySections( $this->mUser, 'web',
$sections );
- $notifs = $this->notifMapper->fetchUnreadByUser( $this->mUser,
$wgEchoMaxUpdateCount, $eventTypes );
+ $notifs = $this->notifMapper->fetchUnreadByUser( $this->mUser,
$wgEchoMaxUpdateCount, null, $eventTypes );
$eventIds = array_filter(
array_map( function ( EchoNotification $notif ) {
diff --git a/includes/api/ApiEchoNotifications.php
b/includes/api/ApiEchoNotifications.php
index 7e41579..ba539fe 100644
--- a/includes/api/ApiEchoNotifications.php
+++ b/includes/api/ApiEchoNotifications.php
@@ -111,17 +111,46 @@
// Prefer unread notifications. We don't care about next offset
in this case
if ( $unreadFirst ) {
- $notifs = $notifMapper->fetchUnreadByUser( $user,
$limit, $eventTypes );
+ // 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 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 );
- if ( $count < $limit ) {
+ // 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,
- null,
+ $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 )
);
@@ -141,11 +170,9 @@
}
// Generate offset if necessary
- if ( !$unreadFirst ) {
- if ( count( $result['list'] ) > $limit ) {
- $lastItem = array_pop( $result['list'] );
- $result['continue'] =
$lastItem['timestamp']['utcunix'] . '|' . $lastItem['id'];
- }
+ if ( count( $result['list'] ) > $limit ) {
+ $lastItem = array_pop( $result['list'] );
+ $result['continue'] = $lastItem['timestamp']['utcunix']
. '|' . $lastItem['id'];
}
return $result;
diff --git a/includes/formatters/EchoFlyoutFormatter.php
b/includes/formatters/EchoFlyoutFormatter.php
index 4f7e378..3fedd64 100644
--- a/includes/formatters/EchoFlyoutFormatter.php
+++ b/includes/formatters/EchoFlyoutFormatter.php
@@ -1,5 +1,4 @@
<?php
-use EchoLinkNormalizer;
/**
* A formatter for the notification flyout popup
diff --git a/includes/mapper/NotificationMapper.php
b/includes/mapper/NotificationMapper.php
index e3915af..93a41f0 100644
--- a/includes/mapper/NotificationMapper.php
+++ b/includes/mapper/NotificationMapper.php
@@ -95,12 +95,13 @@
* 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 fetchUnreadByUser( User $user, $limit, array
$eventTypes = array(), $dbSource = DB_SLAVE ) {
- return $this->fetchByUserInternal( $user, $limit, $eventTypes,
array( 'notification_read_timestamp' => null ), $dbSource );
+ public function fetchUnreadByUser( User $user, $limit, $continue, array
$eventTypes = array(), $dbSource = DB_SLAVE ) {
+ return $this->fetchByUserInternal( $user, $limit, $continue,
$eventTypes, array( 'notification_read_timestamp' => null ), $dbSource );
}
/**
@@ -121,27 +122,19 @@
$conds[] = 'event_id NOT IN ( ' . $dbr->makeList(
$excludeEventIds ) . ' ) ';
}
- $offset = $this->extractQueryOffset( $continue );
-
- // Start points are specified
- if ( $offset['timestamp'] && $offset['offset'] ) {
- $ts = $dbr->addQuotes( $dbr->timestamp(
$offset['timestamp'] ) );
- // The offset and timestamp are those of the first
notification we want to return
- $conds[] = "notification_timestamp < $ts OR (
notification_timestamp = $ts AND notification_event <= " . $offset['offset'] .
" )";
- }
-
- return $this->fetchByUserInternal( $user, $limit, $eventTypes,
$conds );
+ return $this->fetchByUserInternal( $user, $limit, $continue,
$eventTypes, $conds );
}
/**
* @param User $user the user to get notifications for
* @param int $limit The maximum number of notifications to return
+ * @param string $continue Used for offset
* @param array $eventTypes Event types to load
* @param array $conds Additional query conditions.
* @param int $dbSource Use master or slave database
* @return EchoNotification[]
*/
- protected function fetchByUserInternal( User $user, $limit, array
$eventTypes = array(), array $conds = array(), $dbSource = DB_SLAVE ) {
+ protected function fetchByUserInternal( User $user, $limit, $continue,
array $eventTypes = array(), array $conds = array(), $dbSource = DB_SLAVE ) {
$dbr = $this->dbFactory->getEchoDb( $dbSource );
if ( !$eventTypes ) {
@@ -161,6 +154,15 @@
'notification_bundle_base' => 1
) + $conds;
+ $offset = $this->extractQueryOffset( $continue );
+
+ // Start points are specified
+ if ( $offset['timestamp'] && $offset['offset'] ) {
+ $ts = $dbr->addQuotes( $dbr->timestamp(
$offset['timestamp'] ) );
+ // The offset and timestamp are those of the first
notification we want to return
+ $conds[] = "notification_timestamp < $ts OR (
notification_timestamp = $ts AND notification_event <= " . $offset['offset'] .
" )";
+ }
+
$res = $dbr->select(
array( 'echo_notification', 'echo_event',
'echo_target_page' ),
'*',
diff --git a/tests/phpunit/mapper/NotificationMapperTest.php
b/tests/phpunit/mapper/NotificationMapperTest.php
index 03fc008..40277d8 100644
--- a/tests/phpunit/mapper/NotificationMapperTest.php
+++ b/tests/phpunit/mapper/NotificationMapperTest.php
@@ -12,7 +12,7 @@
public function fetchUnreadByUser( User $user, $limit, array
$eventTypes = array() ) {
// Unsuccessful select
$notifMapper = new EchoNotificationMapper(
$this->mockMWEchoDbFactory( array( 'select' => false ) ) );
- $res = $notifMapper->fetchUnreadByUser( $this->mockUser(), 10,
'' );
+ $res = $notifMapper->fetchUnreadByUser( $this->mockUser(), 10,
null, '' );
$this->assertEmpty( $res );
// Successful select
@@ -34,11 +34,11 @@
)
);
$notifMapper = new EchoNotificationMapper(
$this->mockMWEchoDbFactory( array( 'select' => $dbResult ) ) );
- $res = $notifMapper->fetchUnreadByUser( $this->mockUser(), 10,
'', array() );
+ $res = $notifMapper->fetchUnreadByUser( $this->mockUser(), 10,
null, '', array() );
$this->assertEmpty( $res );
$notifMapper = new EchoNotificationMapper(
$this->mockMWEchoDbFactory( array( 'select' => $dbResult ) ) );
- $res = $notifMapper->fetchUnreadByUser( $this->mockUser(), 10,
'', array( 'test_event' ) );
+ $res = $notifMapper->fetchUnreadByUser( $this->mockUser(), 10,
null, '', array( 'test_event' ) );
$this->assertTrue( is_array( $res ) );
$this->assertGreaterThan( 0, count( $res ) );
foreach ( $res as $row ) {
--
To view, visit https://gerrit.wikimedia.org/r/257962
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I84761b13a1b9203cb8e3fcc80941d739cd28659f
Gerrit-PatchSet: 4
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
