From 4149fd944e814a2e860e2eb6eeff77f47c6d014c Mon Sep 17 00:00:00 2001 From: Stephane Bisson Date: Wed, 13 Sep 2017 12:58:16 -0400 Subject: [PATCH] RCFilters: Live update: no data returns 204 Return status 204 when peek=1 and there is no data Also refactor _fetchChangesList to allow checking for new changes and pulling them different handling. Bug: T173613 Change-Id: I8fe2556156bac3d0cfa4f557ae82a163b6eb4d37 --- .../specialpage/ChangesListSpecialPage.php | 2 +- .../mw.rcfilters.Controller.js | 105 +++++++++--------- 2 files changed, 53 insertions(+), 54 deletions(-) diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php index 0762bf7f8d..43012afd86 100644 --- a/includes/specialpage/ChangesListSpecialPage.php +++ b/includes/specialpage/ChangesListSpecialPage.php @@ -533,7 +533,7 @@ abstract class ChangesListSpecialPage extends SpecialPage { // Used by "live update" and "view newest" to check // if there's new changes with minimal data transfer if ( $this->getRequest()->getBool( 'peek' ) ) { - $code = $rows->numRows() > 0 ? 200 : 304; + $code = $rows->numRows() > 0 ? 200 : 204; $this->getOutput()->setStatusCode( $code ); return; } diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js index ee74ac5f97..b07df579e3 100644 --- a/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js +++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.Controller.js @@ -518,14 +518,14 @@ } this._checkForNewChanges() - .then( function ( data ) { + .then( function ( newChanges ) { if ( !this._shouldCheckForNewChanges() ) { // by the time the response is received, // it may not be appropriate anymore return; } - if ( data.changes !== 'NO_RESULTS' ) { + if ( newChanges ) { if ( this.changesListModel.getLiveUpdate() ) { return this.updateChangesList( null, this.LIVE_UPDATE ); } else { @@ -551,19 +551,21 @@ /** * Check if new changes, newer than those currently shown, are available * - * @return {jQuery.Promise} Promise object that resolves after trying - * to fetch 1 change newer than the last known 'from' parameter value + * @return {jQuery.Promise} Promise object that resolves with a bool + * specifying if there are new changes or not * * @private */ mw.rcfilters.Controller.prototype._checkForNewChanges = function () { - return this._fetchChangesList( - 'liveUpdate', - { - limit: 1, - // temporarily disabled ( T173613#3591657 ) - // peek: 1, // bypasses all UI - from: this.changesListModel.getNextFrom() + var params = { + limit: 1, + peek: 1, // bypasses ChangesList specific UI + from: this.changesListModel.getNextFrom() + }; + return this._queryChangesList( 'liveUpdate', params ).then( + function ( data ) { + // no result is 204 with the 'peek' param + return data.status === 200; } ); }; @@ -1115,22 +1117,20 @@ }; /** - * Fetch the list of changes from the server for the current filters + * Query the list of changes from the server for the current filters * - * @param {string} [counterId='updateChangesList'] Id for this request. To allow concurrent requests + * @param {string} counterId Id for this request. To allow concurrent requests * not to invalidate each other. * @param {Object} [params={}] Parameters to add to the query * - * @return {jQuery.Promise} Promise object that will resolve with the changes list - * or with a string denoting no results. + * @return {jQuery.Promise} Promise object resolved with { content, status } */ - mw.rcfilters.Controller.prototype._fetchChangesList = function ( counterId, params ) { + mw.rcfilters.Controller.prototype._queryChangesList = function ( counterId, params ) { var uri = this._getUpdatedUri(), stickyParams = this.filtersModel.getStickyParams(), requestId, latestRequest; - counterId = counterId || 'updateChangesList'; params = params || {}; params.action = 'render'; // bypasses MW chrome @@ -1152,53 +1152,52 @@ return $.ajax( uri.toString(), { contentType: 'html' } ) .then( - function ( html, reason ) { - var $parsed, - pieces; - + function ( content, message, jqXHR ) { if ( !latestRequest() ) { return $.Deferred().reject(); } - - if ( params.peek && reason === 'notmodified' ) { - return { - changes: 'NO_RESULTS' - }; + return { + content: content, + status: jqXHR.status + }; + }, + // RC returns 404 when there is no results + function ( jqXHR ) { + if ( latestRequest() ) { + return $.Deferred().resolve( + { + content: jqXHR.responseText, + status: jqXHR.status + } + ).promise(); } + } + ); + }; - // Because of action=render, the response is a list of nodes. - // It has to be put under a root node so it can be queried. - $parsed = $( '
' ).append( $( $.parseHTML( html ) ) ); - - pieces = { - // Changes list - changes: $parsed.find( '.mw-changeslist' ).first().contents(), - // Fieldset - fieldset: $parsed.find( 'fieldset.cloptions' ).first() - }; + /** + * Fetch the list of changes from the server for the current filters + * + * @return {jQuery.Promise} Promise object that will resolve with the changes list + * and the fieldset. + */ + mw.rcfilters.Controller.prototype._fetchChangesList = function () { + return this._queryChangesList( 'updateChangesList' ) + .then( + function ( data ) { + var $parsed = $( '
' ).append( $( $.parseHTML( data.content ) ) ), + pieces = { + // Changes list + changes: $parsed.find( '.mw-changeslist' ).first().contents(), + // Fieldset + fieldset: $parsed.find( 'fieldset.cloptions' ).first() + }; - // Watchlist returns 200 when there is no results if ( pieces.changes.length === 0 ) { pieces.changes = 'NO_RESULTS'; } return pieces; - }, - // RC returns 404 when there is no results - function ( responseObj ) { - var $parsed; - - if ( !latestRequest() ) { - return $.Deferred().reject(); - } - - $parsed = $( $.parseHTML( responseObj.responseText ) ); - - // Force a resolve state to this promise - return $.Deferred().resolve( { - changes: 'NO_RESULTS', - fieldset: $parsed.find( 'fieldset.cloptions' ).first() - } ).promise(); } ); }; -- 2.20.1