jenkins-bot has submitted this change and it was merged.
Change subject: Hygiene: Generalise search api requests across MobileFrontend
......................................................................
Hygiene: Generalise search api requests across MobileFrontend
New:
* Nearby and Watchlist now show Wikidata descriptions in beta. When pushing to
stable we will want to consider whether to feature flag these separately.
Changes:
* Update Gateways to return arrays of Page objects
* Support displaytitle logic so that display titles are shown in all page
lists when available
* Allow configuration of search parameters across the site (e.g. for
Wikidata)
* Use small thumbnails for all PageList renderings. Having a different
thumbnail size just for search/watchlist doesn't make sense.
* Remove the confusing code for Wikidata descriptions, add the logic to the
stable module but protected behind the flag
By adding two config variables, this allows a MediaWiki instance
to tweak how searches are performed, where they might be different
for example Wikidata.
It also allows us to avoid getting warnings in API requests where
an optional extension such as Geodata or PageImages may not be installed.
Bug: T115646
Bug: T115014
Change-Id: I49cf14db7857b077b8536ac654c04a4af2f4d2ee
---
M includes/Config.php
M includes/MobileFrontend.hooks.php
M includes/Resources.php
A includes/config/Search.php
M resources/mobile.nearby/NearbyGateway.js
M resources/mobile.search.api/SearchGateway.js
D resources/mobile.search.beta.api/SearchGateway.js
M resources/mobile.startup/Page.js
M resources/mobile.watchlist/WatchListGateway.js
M resources/skins.minerva.scripts/search.js
M tests/qunit/mobile.search.api/test_SearchGateway.js
D tests/qunit/mobile.search.beta.api/test_SearchApiGateway.js
12 files changed, 208 insertions(+), 204 deletions(-)
Approvals:
Florianschmidtwelzow: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Config.php b/includes/Config.php
index 64442c9..bed0508 100644
--- a/includes/Config.php
+++ b/includes/Config.php
@@ -5,6 +5,7 @@
require_once __DIR__ . "/config/Experimental.php";
require_once __DIR__ . "/config/Legacy.php";
require_once __DIR__ . "/config/Nearby.php";
+ require_once __DIR__ . "/config/Search.php";
require_once __DIR__ . "/config/Site.php";
require_once __DIR__ . "/config/Skin.php";
require_once __DIR__ . "/config/Uploads.php";
diff --git a/includes/MobileFrontend.hooks.php
b/includes/MobileFrontend.hooks.php
index 3dc9884..b778f55 100644
--- a/includes/MobileFrontend.hooks.php
+++ b/includes/MobileFrontend.hooks.php
@@ -334,9 +334,34 @@
$context = MobileContext::singleton();
$config = $context->getMFConfig();
+ $pageProps = $config->get( 'MFQueryPropModules' );
+ $searchParams = $config->get( 'MFSearchAPIParams' );
+ // Avoid API warnings and allow integration with optional
extensions.
+ if ( defined( 'PAGE_IMAGES_INSTALLED' ) ) {
+ $pageProps[] = 'pageimages';
+ $searchParams = array_merge_recursive( $searchParams,
array(
+ 'piprop' => 'thumbnail',
+ 'pithumbsize' => MobilePage::SMALL_IMAGE_WIDTH,
+ 'pilimit' => 50,
+ ) );
+ }
+
+ // When set turn on Wikidata descriptions
+ // https://phabricator.wikimedia.org/T101719
+ if ( $config->get( 'MFUseWikibaseDescription' ) ) {
+ if ( !in_array( 'pageterms', $pageProps ) ) {
+ $pageProps[] = 'pageterms';
+ }
+ $searchParams = array_merge_recursive( $searchParams,
array(
+ 'wbptterms' => 'description',
+ ) );
+ }
+
// Get the licensing agreement that is displayed in the
uploading interface.
$wgMFUploadLicense = MobileFrontendSkinHooks::getLicense(
'upload' );
$vars += array(
+ 'wgMFSearchAPIParams' => $searchParams,
+ 'wgMFQueryPropModules' => $pageProps,
'wgMFNearbyEndpoint' => $config->get(
'MFNearbyEndpoint' ),
'wgMFThumbnailSizes' => array(
'tiny' => MobilePage::TINY_IMAGE_WIDTH,
diff --git a/includes/Resources.php b/includes/Resources.php
index dad4e7b..4dd5b64 100644
--- a/includes/Resources.php
+++ b/includes/Resources.php
@@ -416,6 +416,7 @@
'mobile.startup' => $wgMFResourceFileModuleBoilerplate + array(
'dependencies' => array(
'mobile.context',
+ 'mobile.modifiedBar',
'mobile.browser',
'mobile.oo',
'mobile.user',
@@ -691,15 +692,6 @@
),
'styles' => array(
'resources/mobile.search.beta/SearchOverlay.less',
- ),
- ),
-
- 'mobile.search.beta.api' => $wgMFResourceParsedMessageModuleBoilerplate
+ array(
- 'dependencies' => array(
- 'mobile.search.api',
- ),
- 'scripts' => array(
- 'resources/mobile.search.beta.api/SearchGateway.js',
),
),
diff --git a/includes/config/Search.php b/includes/config/Search.php
new file mode 100644
index 0000000..502262d
--- /dev/null
+++ b/includes/config/Search.php
@@ -0,0 +1,20 @@
+<?php
+// Needs to be called within MediaWiki; not standalone
+if ( !defined( 'MEDIAWIKI' ) ) {
+ die( 'Not an entry point.' );
+}
+
+/**
+ * Define a set of params that should be passed in every gateway query.
+ */
+$wgMFSearchAPIParams = array(
+ // https://phabricator.wikimedia.org/T115646
+ 'ppprop' => 'displaytitle',
+);
+
+/**
+ * Define a set of page props that should be associated with requests for
pages via the API.
+ */
+$wgMFQueryPropModules = array(
+ 'pageprops',
+);
diff --git a/resources/mobile.nearby/NearbyGateway.js
b/resources/mobile.nearby/NearbyGateway.js
index 926c28f..25e9fbe 100644
--- a/resources/mobile.nearby/NearbyGateway.js
+++ b/resources/mobile.nearby/NearbyGateway.js
@@ -123,19 +123,15 @@
self = this;
requestParams = {
- action: 'query',
colimit: 'max',
- prop: 'pageprops|pageimages|coordinates',
- ppprop: 'displaytitle',
- pithumbsize: mw.config.get(
'wgMFThumbnailSizes' ).small,
- pilimit: limit,
+ prop: [ 'coordinates' ].concat( mw.config.get(
'wgMFQueryPropModules' ) ),
generator: 'geosearch',
ggsradius: range,
ggsnamespace: ns,
ggslimit: limit,
formatversion: 2
};
- $.extend( requestParams, params );
+ $.extend( requestParams, params, mw.config.get(
'wgMFSearchAPIParams' ) );
this.api.ajax( requestParams ).then( function ( resp ) {
var pages;
@@ -168,10 +164,7 @@
pages = $.map( pages, function ( page, i ) {
var coords, lngLat, p;
- // FIXME: API returns pageid rather
than id, should we rename Page option ?
- page.id = page.pageid;
- page.displayTitle = page.pageprops &&
page.pageprops.displaytitle || '';
- p = new Page( page );
+ p = Page.newFromJSON( page );
p.anchor = 'item_' + i;
if ( page.coordinates && loc ) { //
FIXME: protect against bug 47133 (remove when resolved)
coords = page.coordinates[0];
diff --git a/resources/mobile.search.api/SearchGateway.js
b/resources/mobile.search.api/SearchGateway.js
index 751e762..24ee6f3 100644
--- a/resources/mobile.search.api/SearchGateway.js
+++ b/resources/mobile.search.api/SearchGateway.js
@@ -25,21 +25,24 @@
* @return {Object}
*/
getApiData: function ( query ) {
- return {
- action: 'query',
+ var data = $.extend( {
generator: 'prefixsearch',
gpssearch: query,
gpsnamespace: this.searchNamespace,
gpslimit: 15,
- prop: 'pageimages',
- piprop: 'thumbnail',
- pithumbsize: mw.config.get(
'wgMFThumbnailSizes' ).tiny,
- pilimit: 15,
+ prop: mw.config.get( 'wgMFQueryPropModules' ),
redirects: '',
list: 'prefixsearch',
pssearch: query,
pslimit: 15
- };
+ }, mw.config.get( 'wgMFSearchAPIParams' ) );
+
+ // If PageImages is being used configure further.
+ if ( data.pilimit ) {
+ data.pilimit = 15;
+ data.pithumbsize = mw.config.get(
'wgMFThumbnailSizes' ).tiny;
+ }
+ return data;
},
/**
@@ -75,14 +78,12 @@
* @return {Object} data needed to create a {Page}
* @private
*/
- _getPageData: function ( query, info ) {
- return {
- id: info.pageid,
- displayTitle: this._highlightSearchTerm(
info.displayTitle || info.title, query ),
- title: info.title,
- url: mw.util.getUrl( info.title ),
- thumbnail: info.thumbnail
- };
+ _getPage: function ( query, info ) {
+ var page = Page.newFromJSON( info );
+ // Highlight the search term
+ // FIXME: Given that displayTitle could have html in it
be safe and just highlight text.
+ page.displayTitle = this._highlightSearchTerm(
page.title, query );
+ return page;
},
/**
@@ -137,17 +138,17 @@
if ( info ) {
// return all
possible page data
pageIds.push(
id );
- results.push(
self._getPageData( query, info ) );
+ results.push(
self._getPage( query, info ) );
} else {
mwTitle =
mw.Title.newFromText( page.title, self._searchNamespace );
- results.push( {
+ results.push(
new Page( {
id:
page.pageid,
heading: self._highlightSearchTerm( page.title, query ),
title:
page.title,
displayTitle: mwTitle.getNameText(),
url:
mwTitle.getUrl()
- } );
+ } ) );
}
}
} );
@@ -174,9 +175,7 @@
// resolve the Deferred object
result.resolve( {
query: query,
- results: $.map(
self._processData( query, data ), function ( item ) {
- return new
Page( item );
- } )
+ results:
self._processData( query, data )
} );
} )
.fail( function () {
diff --git a/resources/mobile.search.beta.api/SearchGateway.js
b/resources/mobile.search.beta.api/SearchGateway.js
deleted file mode 100644
index fd78ae8..0000000
--- a/resources/mobile.search.beta.api/SearchGateway.js
+++ /dev/null
@@ -1,43 +0,0 @@
-( function ( M ) {
- var SearchGateway = M.require( 'mobile.search.api/SearchGateway' );
-
- /**
- * The Api renders pages with Wikidata descriptions
- * @class SearchGatewayBeta
- * @extends SearchGateway
- * @inheritdoc
- */
- function SearchGatewayBeta() {
- SearchGatewayBeta.parent.apply( this, arguments );
- }
- OO.inheritClass( SearchGatewayBeta, SearchGateway );
- /**
- * In addition to the base data, we need to get Wikidata description
for the page too
- * @inheritdoc
- */
- SearchGatewayBeta.prototype.getApiData = function ( query ) {
- var data = SearchGateway.prototype.getApiData.call( this, query
);
- data.prop = data.prop + '|pageterms';
- data.wbptterms = 'description';
- return data;
- };
-
- /**
- * Add wikidataDescription (if it exists) to the page data
- * @inheritdoc
- * @returns {Object} data needed to create a {Page}
- * @private
- */
- SearchGatewayBeta.prototype._getPageData = function ( query, info ) {
- var data = SearchGateway.prototype._getPageData.call( this,
query, info ),
- terms = info.terms;
-
- if ( terms && terms.description && terms.description.length ) {
- data.wikidataDescription = terms.description[0];
- }
- return data;
- };
-
- M.define( 'mobile.search.beta.api/SearchGateway', SearchGatewayBeta );
-
-}( mw.mobileFrontend ) );
diff --git a/resources/mobile.startup/Page.js b/resources/mobile.startup/Page.js
index 16c6367..2dd8ec1 100644
--- a/resources/mobile.startup/Page.js
+++ b/resources/mobile.startup/Page.js
@@ -1,6 +1,7 @@
( function ( M, $ ) {
var Page,
+ time = M.require( 'mobile.modifiedBar/time' ),
View = M.require( 'mobile.view/View' ),
Section = M.require( 'mobile.startup/Section' ),
Thumbnail = M.require( 'mobile.startup/Thumbnail' );
@@ -64,14 +65,15 @@
initialize: function ( options ) {
var thumb;
+ this.options = options;
// Fallback if no displayTitle provided
- options.displayTitle = options.displayTitle ||
options.title;
+ options.displayTitle = this.getDisplayTitle();
options.languageUrl = mw.util.getUrl(
'Special:MobileLanguages/' + options.title );
View.prototype.initialize.apply( this, arguments );
// allow usage in templates.
// FIXME: Should View map all options to properties?
this.title = options.title;
- this.displayTitle = options.displayTitle ||
options.title;
+ this.displayTitle = options.displayTitle;
this.thumbnail = options.thumbnail;
this.url = options.url || mw.util.getUrl( options.title
);
this.id = options.id;
@@ -82,7 +84,14 @@
}
this.wikidataDescription = options.wikidataDescription;
},
-
+ /**
+ * Retrieve the title that should be displayed to the user
+ * @method
+ * @return {String} HTML
+ */
+ getDisplayTitle: function () {
+ return this.options.displayTitle || this.options.title;
+ },
/**
* Determine if current page is in a specified namespace
* @method
@@ -293,6 +302,51 @@
}
} );
+ /**
+ * Create a Page object from an API response.
+ *
+ * @static
+ * @param {Object} resp as representing a page in the API
+ * @returns {Page}
+ */
+ Page.newFromJSON = function ( resp ) {
+ var revision, displayTitle,
+ thumb = resp.thumbnail,
+ pageprops = resp.pageprops || {
+ displaytitle: resp.title
+ },
+ terms = resp.terms;
+
+ if ( pageprops || terms ) {
+ // The label is either the display title or the label
pageprop (the latter used by Wikidata)
+ // Long term we want to consolidate these.
+ displayTitle = terms && terms.label ? terms.label[0] :
pageprops.displaytitle;
+ }
+ // Add Wikidata descriptions if available (T101719)
+ if ( terms && terms.description && terms.description.length ) {
+ resp.wikidataDescription = terms.description[0];
+ }
+
+ if ( thumb ) {
+ resp.thumbnail.isLandscape = thumb.width > thumb.height;
+ }
+
+ // page may or may not exist.
+ if ( resp.revisions && resp.revisions[0] ) {
+ revision = resp.revisions[0];
+ resp.lastModified = time.getLastModifiedMessage( new
Date( revision.timestamp ).getTime() / 1000,
+ revision.user );
+ }
+
+ return new Page(
+ $.extend( resp, {
+ id: resp.pageid,
+ isMissing: resp.missing ? true : false,
+ url: mw.util.getUrl( resp.title ),
+ displayTitle: displayTitle
+ } )
+ );
+ };
M.define( 'mobile.startup/Page', Page );
}( mw.mobileFrontend, jQuery ) );
diff --git a/resources/mobile.watchlist/WatchListGateway.js
b/resources/mobile.watchlist/WatchListGateway.js
index 1b34abd..852806f 100644
--- a/resources/mobile.watchlist/WatchListGateway.js
+++ b/resources/mobile.watchlist/WatchListGateway.js
@@ -1,5 +1,6 @@
( function ( M, $ ) {
- var time = M.require( 'mobile.modifiedBar/time' );
+ var Page = M.require( 'mobile.startup/Page' );
+
/**
* @class WatchListGateway
* @param {mw.Api} api
@@ -34,18 +35,14 @@
load: function () {
var self = this,
params = $.extend( {
- action: 'query',
- prop: 'pageimages|info|revisions',
- piprop: 'thumbnail',
- pithumbsize: mw.config.get(
'wgMFThumbnailSizes' ).tiny,
- pilimit: this.limit,
+ prop: [ 'info', 'revisions' ].concat(
mw.config.get( 'wgMFQueryPropModules' ) ),
format: 'json',
formatversion: 2,
rvprop: 'timestamp|user',
generator: 'watchlistraw',
gwrnamespace: '0',
gwrlimit: this.limit
- }, this.continueParams );
+ }, mw.config.get( 'wgMFSearchAPIParams' ),
this.continueParams );
if ( this.canContinue === false ) {
return $.Deferred();
@@ -74,6 +71,7 @@
/**
* Parse api response data into pagelist item format
* @param {Object[]} data
+ * @return {Page[]}
*/
parseData: function ( data ) {
var pages;
@@ -102,30 +100,7 @@
// Transform the items to a sensible format
return $.map( pages, function ( item ) {
- var revision, thumb, data;
-
- thumb = item.thumbnail;
-
- if ( thumb ) {
- thumb.isLandscape = thumb.width >
thumb.height;
- }
-
- data = {
- isMissing: item.missing ? true : false,
- displayTitle: item.title,
- id: item.pageid,
- url: mw.util.getUrl( item.title ),
- thumbnail: thumb
- };
-
- // page may or may not exist.
- if ( item.revisions && item.revisions[0] ) {
- revision = item.revisions[0];
- data.lastModified =
time.getLastModifiedMessage( new Date( revision.timestamp ).getTime() / 1000,
- revision.user );
- }
-
- return data;
+ return Page.newFromJSON( item );
} );
}
diff --git a/resources/skins.minerva.scripts/search.js
b/resources/skins.minerva.scripts/search.js
index 4eb100a..c92328e 100644
--- a/resources/skins.minerva.scripts/search.js
+++ b/resources/skins.minerva.scripts/search.js
@@ -12,13 +12,6 @@
overlay: 'mobile.search/SearchOverlay'
};
- if ( context.isBetaGroupMember() ) {
- moduleConfig = $.extend( moduleConfig, {
- modules: [ 'mobile.search.beta.api',
'mobile.search.beta' ],
- api: 'mobile.search.beta.api/SearchGateway'
- } );
- }
-
/**
* Reveal the search overlay
* @param {jQuery.Event} ev
diff --git a/tests/qunit/mobile.search.api/test_SearchGateway.js
b/tests/qunit/mobile.search.api/test_SearchGateway.js
index 9a2b99f..ec63167 100644
--- a/tests/qunit/mobile.search.api/test_SearchGateway.js
+++ b/tests/qunit/mobile.search.api/test_SearchGateway.js
@@ -105,4 +105,79 @@
} );
} );
+ QUnit.module( 'MobileFrontend SearchGateway (Wikidata Descriptions)', {
+ setup: function () {
+ var data = {
+ query: {
+ pages: {
+ 2: {
+ pageid: 2,
+ ns: 0,
+ title: 'Brad Pitt',
+ index: 2,
+ terms: {
+ description: [
'American actor' ]
+ }
+ },
+ 4: {
+ pageid: 4,
+ ns: 0,
+ title: 'Bradley Cooper',
+ index: 3,
+ terms: {
+ description: [
'American actor and film producer' ]
+ }
+ },
+ 5: {
+ pageid: 5,
+ ns: 0,
+ title: 'Braddy',
+ index: 1
+ }
+ },
+ prefixsearch: [
+ {
+ ns: 0,
+ title: 'Braddy',
+ pageid: 5
+ },
+ {
+ ns: 0,
+ title: 'Brad Pitt',
+ pageid: 2
+ },
+ {
+ ns: 0,
+ title: 'Bradley Cooper',
+ pageid: 4
+ }
+ ]
+ }
+ };
+ this.sandbox.stub( mw.Api.prototype, 'get' ).returns(
$.Deferred().resolve( data ) );
+ }
+ } );
+
+ QUnit.asyncTest( 'Wikidata Description in search results', 3, function
( assert ) {
+ var searchApi = new SearchGateway( new mw.Api() );
+ searchApi.search( 'brad' ).done( function ( resp ) {
+ var results = resp.results;
+ QUnit.start();
+ assert.ok(
+ results[0].wikidataDescription === undefined,
+ 'Braddy does not have a Wikidata description.'
+ );
+ assert.equal(
+ results[1].wikidataDescription,
+ 'American actor',
+ 'Yes, Brad Pitt is an actor.'
+ );
+ assert.equal(
+ results[2].wikidataDescription,
+ 'American actor and film producer',
+ 'Yes, Cooper is an actor.'
+ );
+ } );
+
+ } );
}( jQuery, mw.mobileFrontend ) );
diff --git a/tests/qunit/mobile.search.beta.api/test_SearchApiGateway.js
b/tests/qunit/mobile.search.beta.api/test_SearchApiGateway.js
deleted file mode 100644
index b6312ac..0000000
--- a/tests/qunit/mobile.search.beta.api/test_SearchApiGateway.js
+++ /dev/null
@@ -1,80 +0,0 @@
-( function ( $, M ) {
- var SearchGateway = M.require( 'mobile.search.beta.api/SearchGateway' );
-
- QUnit.module( 'MobileFrontend SearchGateway', {
- setup: function () {
- var data = {
- query: {
- pages: {
- 2: {
- pageid: 2,
- ns: 0,
- title: 'Brad Pitt',
- index: 2,
- terms: {
- description: [
'American actor' ]
- }
- },
- 4: {
- pageid: 4,
- ns: 0,
- title: 'Bradley Cooper',
- index: 3,
- terms: {
- description: [
'American actor and film producer' ]
- }
- },
- 5: {
- pageid: 5,
- ns: 0,
- title: 'Braddy',
- index: 1
- }
- },
- prefixsearch: [
- {
- ns: 0,
- title: 'Braddy',
- pageid: 5
- },
- {
- ns: 0,
- title: 'Brad Pitt',
- pageid: 2
- },
- {
- ns: 0,
- title: 'Bradley Cooper',
- pageid: 4
- }
- ]
- }
- };
- this.sandbox.stub( mw.Api.prototype, 'get' ).returns(
$.Deferred().resolve( data ) );
- }
- } );
-
- QUnit.asyncTest( 'Wikidata Description in search results', 3, function
( assert ) {
- var searchApi = new SearchGateway( new mw.Api() );
- searchApi.search( 'brad' ).done( function ( resp ) {
- var results = resp.results;
- QUnit.start();
- assert.ok(
- results[0].wikidataDescription === undefined,
- 'Braddy does not have a Wikidata description.'
- );
- assert.equal(
- results[1].wikidataDescription,
- 'American actor',
- 'Yes, Brad Pitt is an actor.'
- );
- assert.equal(
- results[2].wikidataDescription,
- 'American actor and film producer',
- 'Yes, Cooper is an actor.'
- );
- } );
-
- } );
-
-}( jQuery, mw.mobileFrontend ) );
--
To view, visit https://gerrit.wikimedia.org/r/246282
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I49cf14db7857b077b8536ac654c04a4af2f4d2ee
Gerrit-PatchSet: 12
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Florianschmidtwelzow <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
