From a70208e8fe12a3cfe16cd5ad72bc96141d1f1213 Mon Sep 17 00:00:00 2001 From: bsitu Date: Fri, 3 May 2013 16:58:56 -0700 Subject: [PATCH] (bug 48054) Echo API should use a standard continue parameter Change-Id: I1d5333db4a4d58a1ecfa8dfe562cda5a35093f8b --- api/ApiEchoNotifications.php | 17 ++++++++-------- includes/DbEchoBackend.php | 13 +++++++------ includes/EchoBackend.php | 28 ++++++++++++++++++++++++--- modules/special/ext.echo.special.js | 19 +++++------------- special/SpecialNotifications.php | 30 +++++++++-------------------- 5 files changed, 55 insertions(+), 52 deletions(-) diff --git a/api/ApiEchoNotifications.php b/api/ApiEchoNotifications.php index 8f4b71c1c..4fa3b45be 100644 --- a/api/ApiEchoNotifications.php +++ b/api/ApiEchoNotifications.php @@ -22,14 +22,14 @@ class ApiEchoNotifications extends ApiQueryBase { $result = array(); if ( in_array( 'list', $prop ) ) { - $result['list'] = self::getNotifications( $user, $params['format'], $params['limit'] + 1, $params['timestamp'], $params['offset'] ); + $result['list'] = self::getNotifications( $user, $params['format'], $params['limit'] + 1, $params['continue'] ); // check if there is more elements than we request if ( count( $result['list'] ) > $params['limit'] ) { - array_pop( $result['list'] ); - $result['more'] = '1'; + $lastItem = array_pop( $result['list'] ); + $result['continue'] = $lastItem['timestamp']['unix'] . '|' . $lastItem['id']; } else { - $result['more'] = '0'; + $result['continue'] = null; } $this->getResult()->setIndexedTagName( $result['list'], 'notification' ); } @@ -59,17 +59,16 @@ class ApiEchoNotifications extends ApiQueryBase { * @param $user User the user to get notifications for * @param $format string/bool false to not format any notifications, string to a specific output format * @param $limit int The maximum number of notifications to return - * @param $timestamp int The timestamp to start from - * @param $offset int The notification event id to start from + * @param $continue string Used for offset * @return array */ - public static function getNotifications( $user, $format = false, $limit = 20, $timestamp = 0, $offset = 0 ) { + public static function getNotifications( $user, $format = false, $limit = 20, $continue = null ) { global $wgEchoBackend; $output = array(); // TODO: Make 'web' based on a new API param? - $res = $wgEchoBackend->loadNotifications( $user, $limit, $timestamp, $offset, 'web' ); + $res = $wgEchoBackend->loadNotifications( $user, $limit, $continue, 'web' ); foreach ( $res as $row ) { $event = EchoEvent::newFromRow( $row ); @@ -196,6 +195,7 @@ class ApiEchoNotifications extends ApiQueryBase { 'timestamp' => array( ApiBase::PARAM_TYPE => 'integer', ), + 'continue' => null, ); } @@ -209,6 +209,7 @@ class ApiEchoNotifications extends ApiQueryBase { 'limit' => 'The maximum number of notifications to return.', 'offset' => 'Notification event id to start from (requires timestamp param to be passed as well)', 'timestamp' => 'Timestamp to start from', + 'continue' => 'When more results are available, use this to continue', ); } diff --git a/includes/DbEchoBackend.php b/includes/DbEchoBackend.php index 3e570d2b0..132ea5fe9 100644 --- a/includes/DbEchoBackend.php +++ b/includes/DbEchoBackend.php @@ -33,13 +33,12 @@ class MWDbEchoBackend extends MWEchoBackend { /** * @param $user User the user to get notifications for * @param $limit int The maximum number of notifications to return - * @param $timestamp int The timestamp to start from - * @param $offset int The notification event id to start from + * @param $continue string Used for offset * @param $outputFormat string The output format of the notifications (web, * email, etc.) * @return array */ - public function loadNotifications( $user, $limit, $timestamp, $offset, $outputFormat = 'web' ) { + public function loadNotifications( $user, $limit, $continue, $outputFormat = 'web' ) { $dbr = MWEchoDbFactory::getDB( DB_SLAVE ); $eventTypesToLoad = EchoNotificationController::getUserEnabledEvents( $user, $outputFormat ); @@ -54,10 +53,12 @@ class MWDbEchoBackend extends MWEchoBackend { 'notification_bundle_base' => 1 ); + $offset = $this->extractQueryOffset( $continue ); + // Start points are specified - if ( $timestamp && $offset ) { - $conds[] = 'notification_timestamp <= ' . $dbr->addQuotes( $dbr->timestamp( $timestamp ) ); - $conds[] = 'notification_event < ' . intval( $offset ); + if ( $offset['timestamp'] && $offset['offset'] ) { + $conds[] = 'notification_timestamp <= ' . $dbr->addQuotes( $dbr->timestamp( $offset['timestamp'] ) ); + $conds[] = 'notification_event <= ' . $offset['offset']; } $res = $dbr->select( diff --git a/includes/EchoBackend.php b/includes/EchoBackend.php index 8f1cde7b0..e63e7fabd 100644 --- a/includes/EchoBackend.php +++ b/includes/EchoBackend.php @@ -25,6 +25,28 @@ abstract class MWEchoBackend { return new $className(); } + /** + * Extract the offset used for notification list + * @param $continue String Used for offset + * @param @return array + */ + protected function extractQueryOffset( $continue ) { + $offset = array ( + 'timestamp' => 0, + 'offset' => 0, + ); + if ( $continue ) { + $values = explode( '|', $continue, 3 ); + if ( count( $values ) !== 2 ) { + throw new MWException( 'Invalid continue param: ' . $continue ); + } + $offset['timestamp'] = (int)$values[0]; + $offset['offset'] = (int)$values[1]; + } + + return $offset; + } + /** * Create a new notification * @param $row array @@ -35,11 +57,11 @@ abstract class MWEchoBackend { * Load notifications based on the parameters * @param $user User the user to get notifications for * @param $limit int The maximum number of notifications to return - * @param $timestamp int The timestamp to start from - * @param $offset int The notification event id to start from + * @param $continue string Used for offset + * @param $outputFormat string The output format of the notifications (web, email, etc.) * @return array */ - abstract public function loadNotifications( $user, $limit, $timestamp, $offset ); + abstract public function loadNotifications( $user, $limit, $continue, $outputFormat = 'web' ); /** * Get the bundle data for user/hash diff --git a/modules/special/ext.echo.special.js b/modules/special/ext.echo.special.js index 034d711ce..7388aa70c 100644 --- a/modules/special/ext.echo.special.js +++ b/modules/special/ext.echo.special.js @@ -3,11 +3,9 @@ mw.echo.special = { - 'timestamp': 0, - 'offset': 0, + 'notcontinue': null, 'header': '', 'processing': false, - 'moreData': '0', /** * Initialize the property in special notification page. @@ -28,8 +26,7 @@ } } ); - _this.timestamp = mw.config.get( 'wgEchoStartTimestamp' ); - _this.offset = mw.config.get( 'wgEchoStartOffset' ); + _this.notcontinue = mw.config.get( 'wgEchoNextContinue' ); _this.header = mw.config.get( 'wgEchoDateHeader' ); // Set up each individual notification with a close box and dismiss @@ -78,8 +75,7 @@ 'meta' : 'notifications', 'notformat' : 'html', 'notprop' : 'index|list', - 'nottimestamp': this.timestamp, - 'notoffset': this.offset, + 'notcontinue': this.notcontinue, 'notlimit': mw.config.get( 'wgEchoDisplayNum' ) }, { @@ -112,14 +108,9 @@ if ( $li.find( '.mw-echo-dismiss' ).length ) { mw.echo.setUpDismissability( $li ); } - - // update the timestamp and offset to get data from - // this is used for next data retrieval - _this.timestamp = data.timestamp.unix; - _this.offset = data.id; } ); - _this.moreData = notifications.more; + _this.notcontinue = notifications['continue']; if ( unread.length > 0 ) { _this.markAsRead( unread ); } else { @@ -161,7 +152,7 @@ }, 'onSuccess': function() { - if ( this.moreData === '0' ) { + if ( !this.notcontinue ) { $( '#mw-echo-more' ).hide(); } this.processing = false; diff --git a/special/SpecialNotifications.php b/special/SpecialNotifications.php index 3a5a0487d..b7d6f6609 100644 --- a/special/SpecialNotifications.php +++ b/special/SpecialNotifications.php @@ -25,15 +25,9 @@ class SpecialNotifications extends SpecialPage { return; } - // The timestamp and offset to pull current set of data from, this + // The continue parameter to pull current set of data from, this // would be used for browsers with javascript disabled - $timestamp = $offset = 0; - $paging = $this->getRequest()->getVal( 'paging', false ); - if ( $paging ) { - $paging = explode( '|', $paging, 2 ); - $timestamp = intval( $paging[0] ); - $offset = intval( $paging[1] ); - } + $continue = $this->getRequest()->getVal( 'continue', null ); // Preferences link $html = Html::rawElement( 'a', array( @@ -44,7 +38,7 @@ class SpecialNotifications extends SpecialPage { ) ); // Pull the notifications - $notif = ApiEchoNotifications::getNotifications( $user, 'html', self::$displayNum + 1, $timestamp, $offset ); + $notif = ApiEchoNotifications::getNotifications( $user, 'html', self::$displayNum + 1, $continue ); // If there are no notifications, display a message saying so if ( !$notif ) { @@ -53,15 +47,12 @@ class SpecialNotifications extends SpecialPage { return; } - // The timestamp and offset to pull next set of data from - $nextTimestamp = $nextOffset = 0; - // Check if there is more data to load for next request if ( count( $notif ) > self::$displayNum ) { - array_pop( $notif ); - $more = true; + $lastItem = array_pop( $notif ); + $nextContinue = $lastItem['timestamp']['unix'] . '|' . $lastItem['id']; } else { - $more = false; + $nextContinue = null; } // Add the notifications to the page (interspersed with date headers) @@ -80,19 +71,17 @@ class SpecialNotifications extends SpecialPage { $class .= ' mw-echo-unread'; $unread[] = $row['id']; } - $nextTimestamp = $row['timestamp']['unix']; - $nextOffset = $row['id']; $notices .= Html::rawElement( 'li', array( 'class' => $class, 'data-notification-category' => $row['category'] ), $row['*'] ); } $html .= Html::rawElement( 'ul', array( 'id' => 'mw-echo-special-container' ), $notices ); // Build the more link - if ( $more ) { + if ( $nextContinue ) { $html .= Html::element( 'a', array( 'href' => SpecialPage::getTitleFor( 'Notifications' )->getLinkURL( - array( 'paging' => intval( $nextTimestamp ) . '|' . intval( $nextOffset ) ) + array( 'continue' => $nextContinue ) ), 'id' => 'mw-echo-more' ), @@ -105,8 +94,7 @@ class SpecialNotifications extends SpecialPage { $out->addJsConfigVars( array( 'wgEchoDisplayNum' => self::$displayNum, - 'wgEchoStartTimestamp' => $nextTimestamp, - 'wgEchoStartOffset' => $nextOffset, + 'wgEchoNextContinue' => $nextContinue, 'wgEchoFeedbackPage' => $wgEchoFeedbackPage, 'wgEchoDateHeader' => $dateHeader )