From e86fc3b1594d55652cd0258049747a1d5e9e905d Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Tue, 3 Nov 2015 15:39:48 -0800 Subject: [PATCH] Fall back to CirrusSearch's morelike: feature When no related articles have been specified by an editor we instead hit request pages similar to the current page using the CirrusSearch extension's "morelike:" feature [0]. Changes: * Config variable introduced RelatedArticlesUseCirrusSearch which allows you to turn on use of the CirrusSearch API. * Introduce a RelatedPagesGateway for dealing with making the API call and returning consistent results * Move the "simple" API call for hydrating related pages fetched from the wgRelatedArticles configuration variable into RelatedPagesGateway * Reduce the bootstrap module to just a bootstrap module! Bug: T116707 Change-Id: Ia0ced1d7ae57c0939d1f5af275aa9d393f1420b1 --- .jshintrc | 3 +- extension.json | 24 +++- includes/ReadMoreHooks.php | 29 +++++ .../index.js | 82 +++---------- .../index.js | 11 +- .../RelatedPagesGateway.js | 108 ++++++++++++++++++ .../test_RelatedPagesGateway.js | 71 ++++++++++++ 7 files changed, 251 insertions(+), 77 deletions(-) create mode 100644 resources/ext.relatedArticles.readMore/RelatedPagesGateway.js create mode 100644 tests/qunit/ext.relatedArticles.readMore/test_RelatedPagesGateway.js diff --git a/.jshintrc b/.jshintrc index a8fc35ce..c305b65e 100644 --- a/.jshintrc +++ b/.jshintrc @@ -16,6 +16,7 @@ "globals": { "mw": false, "$": false, - "jQuery": true + "jQuery": true, + "OO": true } } diff --git a/extension.json b/extension.json index 7de70a75..1aa8fac4 100644 --- a/extension.json +++ b/extension.json @@ -19,6 +19,9 @@ "RelatedArticlesMagic": "RelatedArticles.i18n.magic.php" }, "Hooks": { + "ResourceLoaderTestModules": [ + "RelatedArticles\\ReadMoreHooks::onResourceLoaderTestModules" + ], "ParserFirstCallInit": [ "RelatedArticles\\Hooks::onParserFirstCallInit" ], @@ -51,12 +54,25 @@ }, "manifest_version": 1, "ResourceModules": { + "ext.relatedArticles.readMore": { + "scripts": [ + "resources/ext.relatedArticles.readMore/RelatedPagesGateway.js" + ], + "dependencies": [ + "oojs" + ], + "targets": [ + "mobile", + "desktop" + ] + }, "ext.relatedArticles.readMore.bootstrap": { "scripts": [ "resources/ext.relatedArticles.readMore.bootstrap/index.js" ], "dependencies": [ - "mediawiki.api" + "mediawiki.api", + "ext.relatedArticles.readMore" ], "targets": [ "mobile", @@ -78,6 +94,12 @@ ] } }, + "config": { + "RelatedArticlesUseCirrusSearch": false + }, + "ConfigRegistry": { + "relatedarticles": "GlobalVarConfig::newInstance" + }, "ResourceFileModulePaths": { "localBasePath": "", "remoteExtPath": "RelatedArticles" diff --git a/includes/ReadMoreHooks.php b/includes/ReadMoreHooks.php index ad106b35..3fad227c 100644 --- a/includes/ReadMoreHooks.php +++ b/includes/ReadMoreHooks.php @@ -4,8 +4,34 @@ namespace RelatedArticles; use OutputPage; use Skin; +use ConfigFactory; class ReadMoreHooks { + /** + * Register QUnit tests. + * @see https://www.mediawiki.org/wiki/Manual:Hooks/ResourceLoaderTestModules + * + * @param array $modules + * @param ResourceLoader $rl + * @return bool + */ + public static function onResourceLoaderTestModules( &$modules, &$rl ) { + $boilerplate = array( + 'localBasePath' => __DIR__ . '/../tests/qunit/', + 'remoteExtPath' => 'RelatedArticles/tests/qunit', + 'targets' => array( 'desktop', 'mobile' ), + ); + + $modules['qunit']['ext.relatedArticles.readMore.tests'] = $boilerplate + array( + 'scripts' => array( + 'ext.relatedArticles.readMore/test_RelatedPagesGateway.js', + ), + 'dependencies' => array( + 'ext.relatedArticles.readMore', + ), + ); + return true; + } /** * Handler for the MakeGlobalVariablesScript hook. @@ -18,7 +44,10 @@ class ReadMoreHooks { * @return boolean Always true */ public static function onMakeGlobalVariablesScript( &$vars, OutputPage $out ) { + $config = ConfigFactory::getDefaultInstance()->makeConfig( 'relatedarticles' ); + $vars['wgRelatedArticles'] = $out->getProperty( 'RelatedArticles' ); + $vars['wgRelatedArticlesUseCirrusSearch'] = $config->get( 'RelatedArticlesUseCirrusSearch' ); return true; } diff --git a/resources/ext.relatedArticles.readMore.bootstrap/index.js b/resources/ext.relatedArticles.readMore.bootstrap/index.js index dd988f69..8b1d08f9 100644 --- a/resources/ext.relatedArticles.readMore.bootstrap/index.js +++ b/resources/ext.relatedArticles.readMore.bootstrap/index.js @@ -1,80 +1,28 @@ ( function ( $ ) { - var relatedArticles = mw.config.get( 'wgRelatedArticles' ) || [], - config = mw.config.get( [ 'skin', 'wgNamespaceNumber', 'wgMFMode', 'wgIsMainPage' ] ), - module; + var config = mw.config.get( [ 'skin', 'wgNamespaceNumber', 'wgMFMode', 'wgIsMainPage' ] ), + relatedPages = new mw.relatedArticles.RelatedPagesGateway( new mw.Api(), + mw.config.get( 'wgPageName' ), mw.config.get( 'wgRelatedArticles' ), + mw.config.get( 'wgRelatedArticlesUseCirrusSearch' ) ), - // Limit number of related articles to 4 (more of them increases likelihood of reader ignoring). - relatedArticles = relatedArticles.slice( 0, 4 ); - - /** - * Retrieves the data required to render a card. - * - * Given a title, the following additional information is retrieved - * from the API: - * - * * The ID of the page corresponding to the title - * * The thumbnail, if any - * * The Wikidata description, if any - * - * @private - * - * @param {string[]} titles - * @return {jQuery.Promise} - */ - function getData( titles ) { - var api = new mw.Api(); - - return api.get( { - action: 'query', - prop: 'pageimages|pageterms', - wbptterms: 'description', - pilimit: titles.length, - 'continue': '', - - titles: titles - } ).then( function ( data ) { - if ( !data.query || !data.query.pages ) { - return []; - } - - return $.map( data.query.pages, function ( page ) { - var result = { - id: page.pageid, - title: page.title, - thumbnail: page.thumbnail, - description: undefined - }; - - if ( - page.terms && - page.terms.description && - page.terms.description.length > 0 - ) { - result.description = page.terms.description[ 0 ]; - } - - return result; - } ); - } ); - } + LIMIT = 4; if ( - relatedArticles.length > 0 && config.wgNamespaceNumber === 0 && !config.wgIsMainPage && config.skin === 'minerva' && config.wgMFMode === 'beta' ) { - module = 'ext.relatedArticles.readMore.minerva'; - - $( function () { - $.when( - mw.loader.using( module ), - getData( relatedArticles ) - ).done( function ( _, data ) { - mw.track( 'ext.relatedArticles.init', { pages: data } ); - } ); + $.when( + // Note we load dependencies here rather than ResourceLoader + // to avoid PHP exceptions when MobileFrontend not installed + // which should never happen given the if statement. + mw.loader.using( [ 'mobile.pagelist.scripts', 'ext.relatedArticles.readMore.minerva' ] ), + relatedPages.getForCurrentPage( LIMIT ) + ).done( function ( _, pages ) { + if ( pages.length ) { + mw.track( 'ext.relatedArticles.init', pages ); + } } ); } diff --git a/resources/ext.relatedArticles.readMore.minerva/index.js b/resources/ext.relatedArticles.readMore.minerva/index.js index 7a6b3b31..807a182e 100644 --- a/resources/ext.relatedArticles.readMore.minerva/index.js +++ b/resources/ext.relatedArticles.readMore.minerva/index.js @@ -13,23 +13,18 @@ var Page = mw.mobileFrontend.require( 'mobile.startup/Page' ); return $.map( pages, function ( rawPage ) { - return new Page( { - id: rawPage.id, - title: rawPage.title, - thumbnail: rawPage.thumbnail, - wikidataDescription: rawPage.description - } ); + return new Page( rawPage ); } ); } - mw.trackSubscribe( 'ext.relatedArticles.init', function ( _, data ) { + mw.trackSubscribe( 'ext.relatedArticles.init', function ( _, pages ) { var WatchstarPageList = mw.mobileFrontend.require( 'mobile.pagelist.scripts/WatchstarPageList' ), pageList, $container = $( '#content' ), $readMore; pageList = new WatchstarPageList( { - pages: convertPages( data.pages ), + pages: convertPages( pages ), // FIXME: When the user clicks any watchstar, a // MobileWebWatching event is logged. Watchstar, which diff --git a/resources/ext.relatedArticles.readMore/RelatedPagesGateway.js b/resources/ext.relatedArticles.readMore/RelatedPagesGateway.js new file mode 100644 index 00000000..53a8cd4a --- /dev/null +++ b/resources/ext.relatedArticles.readMore/RelatedPagesGateway.js @@ -0,0 +1,108 @@ +( function ( $ ) { + + // FIXME: Move into separate file as this module becomes larger. + mw.relatedArticles = {}; + + /** + * @class RelatedPagesGateway + * @param {mw.Api} api + * @param {string} currentPage the page that the editorCuratedArticles relate to + * @param {Array} editorCuratedArticles a list of articles curated by editors for the current page + * @param {boolean} useCirrusSearch whether to hit the API when no editor curated articles are available + */ + function RelatedPagesGateway( api, currentPage, editorCuratedArticles, useCirrusSearch ) { + this.api = api; + this.currentPage = currentPage; + this.editorCuratedArticles = editorCuratedArticles || []; + this.useCirrusSearch = useCirrusSearch; + } + OO.initClass( RelatedPagesGateway ); + + /** + * Gets the related pages for the current page. + * + * If there are related pages assigned to this page using the `related` + * parser function, then they are returned. + * + * If there aren't any related pages assigned to the page, then the + * CirrusSearch extension's {@link https://www.mediawiki.org/wiki/Help:CirrusSearch#morelike: "morelike:" feature} + * is used. If the CirrusSearch extension isn't installed, then the API + * call will fail gracefully and no related pages will be returned. + * Thus the dependency on the CirrusSearch extension is soft. + * + * Related pages will have the following information: + * + * * The ID of the page corresponding to the title + * * The thumbnail, if any + * * The Wikidata description, if any + * + * @method + * @param {number} limit of articles to get + * @return {jQuery.Promise} + */ + RelatedPagesGateway.prototype.getForCurrentPage = function ( limit ) { + var parameters = { + action: 'query', + formatversion: 2, + prop: 'pageimages|pageterms', + piprop: 'thumbnail', + pithumbsize: 80, + wbptterms: 'description' + }, + relatedArticles = ( this.editorCuratedArticles ).slice( 0, limit ); + + if ( relatedArticles.length ) { + parameters.pilimit = relatedArticles.length; + parameters[ 'continue' ] = ''; // jscs:ignore requireDotNotation + + parameters.titles = relatedArticles; + } else if ( this.useCirrusSearch ) { + parameters.pilimit = limit; + + parameters.generator = 'search'; + parameters.gsrsearch = 'morelike:' + this.currentPage; + parameters.gsrnamespace = '0'; + parameters.gsrlimit = limit; + } else { + return $.Deferred().resolve( [] ); + } + + return this.api.get( parameters ) + .then( getPages ) + .then( processPages ); + }; + + /** + * @ignore + */ + function getPages( result ) { + return result && result.query && result.query.pages ? result.query.pages : []; + } + + /** + * @ignore + */ + function processPages( pages ) { + return $.map( pages, function ( page ) { + var result = { + id: page.pageid, + isMissing: !page.pageid, + title: page.title, + thumbnail: page.thumbnail, + wikidataDescription: undefined + }; + + if ( + page.terms && + page.terms.description && + page.terms.description.length > 0 + ) { + result.wikidataDescription = page.terms.description[ 0 ]; + } + + return result; + } ); + } + + mw.relatedArticles.RelatedPagesGateway = RelatedPagesGateway; +}( jQuery ) ); diff --git a/tests/qunit/ext.relatedArticles.readMore/test_RelatedPagesGateway.js b/tests/qunit/ext.relatedArticles.readMore/test_RelatedPagesGateway.js new file mode 100644 index 00000000..cb3641c4 --- /dev/null +++ b/tests/qunit/ext.relatedArticles.readMore/test_RelatedPagesGateway.js @@ -0,0 +1,71 @@ +( function ( M, $ ) { + var RelatedPagesGateway = mw.relatedArticles.RelatedPagesGateway, + relatedPages = { + query: { + pages: { + 123: { + id: 123, + title: 'Oh noes', + ns: 0, + thumbnail: { + source: 'http://placehold.it/200x100' + } + } + } + } + }, + emptyRelatedPages = { + query: { + pages: { + } + } + }; + + QUnit.module( 'RelatedArticles readMore - Related pages api', { + setup: function () { + this.api = new mw.Api(); + } + } ); + + QUnit.test( 'Returns an array with the results when api responds', 2, function ( assert ) { + var gateway = new RelatedPagesGateway( this.api, 'Foo', null, true ); + this.sandbox.stub( this.api, 'get' ).returns( $.Deferred().resolve( relatedPages ) ); + + gateway.getForCurrentPage( 1 ).then( function ( results ) { + assert.ok( $.isArray( results ), 'Results must be an array' ); + assert.strictEqual( results[ 0 ].title, 'Oh noes' ); + } ); + } ); + + QUnit.test( 'Empty related pages is handled fine.', 2, function ( assert ) { + var gateway = new RelatedPagesGateway( this.api, 'Foo', null, true ); + this.sandbox.stub( this.api, 'get' ).returns( $.Deferred().resolve( emptyRelatedPages ) ); + + gateway.getForCurrentPage( 1 ).then( function ( results ) { + assert.ok( $.isArray( results ), 'Results must be an array' ); + assert.strictEqual( results.length, 0 ); + } ); + } ); + + QUnit.test( 'Empty related pages with no cirrus search is handled fine. No API request.', 3, function ( assert ) { + var gateway = new RelatedPagesGateway( this.api, 'Foo', [], false ), + spy = this.sandbox.stub( this.api, 'get' ).returns( $.Deferred().resolve( relatedPages ) ); + + gateway.getForCurrentPage( 1 ).then( function ( results ) { + assert.ok( $.isArray( results ), 'Results must be an array' ); + assert.ok( !spy.called, 'API is not invoked' ); + assert.strictEqual( results.length, 0 ); + } ); + } ); + + QUnit.test( 'Related pages from editor curated content', 1, function ( assert ) { + var gateway = new RelatedPagesGateway( this.api, 'Foo', [ { title: 1 } ], false ); + this.sandbox.stub( this.api, 'get' ).returns( $.Deferred().resolve( relatedPages ) ); + + gateway.getForCurrentPage( 1 ).then( function ( results ) { + assert.strictEqual( results.length, 1, + 'API still hit despite cirrus being disabled.' ); + } ); + } ); + +}( mw.mobileFrontend, jQuery ) );