jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/404534 )
Change subject: Refine description selection
......................................................................
Refine description selection
Simplifies the description picking logic away from its current two-
step approach to a single step.
Additionally, drops the secondary fallback behavior of returning the
first available description in the event a description is not
available in either the preferred language or the configured fallback
(English), since users are not likely to have any use for a
description in a totally random language.
Change-Id: Idc86f90674ef1bee4f6bbbb64b8058ed105d97f4
---
M lib/feed/featured-image.js
M test/lib/feed/featured-image-unit.js
2 files changed, 23 insertions(+), 30 deletions(-)
Approvals:
BearND: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/feed/featured-image.js b/lib/feed/featured-image.js
index a67fc72..5406640 100644
--- a/lib/feed/featured-image.js
+++ b/lib/feed/featured-image.js
@@ -15,6 +15,7 @@
const HTTPError = sUtil.HTTPError;
const COMMONS_URL = 'commons.wikimedia.org';
+const DESCRIPTION_FALLBACK_LANG = 'en';
/**
* @prop {!string} lang Localization language
@@ -74,25 +75,21 @@
* @param {?Object} descriptions POTD descriptions via the Picture of the Day
template,
* parsed from the page HTML. Requires that the input be an object of the
form
* { "en": "foo", "de": "bar" }.
- * @param {!String} lang the desired description language
- * @return {?String} the description language to use
+ * @param {!String} preferredLang the desired description language
+ * @return {?Description} the resolved caption, if available
*/
-function pickDescriptionLang(descriptions, lang) {
+function getDescription(descriptions, preferredLang) {
if (!(descriptions && typeof (descriptions) === 'object')) {
return;
}
- const fallbackLang = 'en';
-
- if (descriptions[lang]) {
- return lang;
+ if (descriptions[preferredLang]) {
+ return new Description(preferredLang, descriptions[preferredLang]);
}
- if (descriptions[fallbackLang]) {
- return fallbackLang;
+ if (descriptions[DESCRIPTION_FALLBACK_LANG]) {
+ return new Description(DESCRIPTION_FALLBACK_LANG,
descriptions[DESCRIPTION_FALLBACK_LANG]);
}
-
- return Object.keys(descriptions)[0];
}
function buildPayload(page, lang) {
@@ -183,13 +180,7 @@
}).then((response) => {
const doc = domino.createDocument(response.body);
removeLangLabels(doc);
- const descriptions = queryDescriptions(doc);
-
- // todo: should we just send all langs like parsoid does?
- const resolvedLang = pickDescriptionLang(descriptions, lang);
- if (resolvedLang) {
- ret.payload.description = new Description(resolvedLang,
descriptions[resolvedLang]);
- }
+ ret.payload.description = getDescription(queryDescriptions(doc), lang);
return ret;
}).catch((err) => {
if (err.status === 504) {
@@ -211,6 +202,6 @@
module.exports = {
promise,
testing: {
- pickDescriptionLang
+ getDescription
}
};
diff --git a/test/lib/feed/featured-image-unit.js
b/test/lib/feed/featured-image-unit.js
index df30b5a..576037f 100644
--- a/test/lib/feed/featured-image-unit.js
+++ b/test/lib/feed/featured-image-unit.js
@@ -2,27 +2,29 @@
const assert = require('../../utils/assert');
const featuredImage = require('../../../lib/feed/featured-image'); // module
under test
-const pickDescriptionLang = featuredImage.testing.pickDescriptionLang;
+const getDescription = featuredImage.testing.getDescription;
describe('featured-image-unit', () => {
- it('pickDescriptionLang resolves to lang if present', () => {
- const result = pickDescriptionLang({ 'en': 'foo', 'de': 'bar' }, 'de');
- assert.deepEqual(result, 'de');
+ it('getDescription returns description for preferred lang if present', ()
=> {
+ const result = getDescription({ 'en':'foo','de':'bar' }, 'de');
+ assert.deepEqual(result.lang, 'de');
+ assert.deepEqual(result.text, 'bar');
});
- it('pickDescriptionLang falls back to en if lang not present', () => {
- const result = pickDescriptionLang({ 'en': 'foo', 'de': 'bar' }, 'ja');
- assert.deepEqual(result, 'en');
+ it('getDescription falls back to en description if preferred lang not
present', () => {
+ const result = getDescription({ 'en':'foo','de':'bar' }, 'ja');
+ assert.deepEqual(result.lang, 'en');
+ assert.deepEqual(result.text, 'foo');
});
- it('pickDescriptionLang returns undefined for non-object input', () => {
- const result = pickDescriptionLang("<span>lol</span>", 'zh');
+ it('getDescription returns undefined for non-object input', () => {
+ const result = getDescription("<span>lol</span>", 'zh');
assert.deepEqual(result, undefined);
});
- it('pickDescriptionLang returns undefined for undefined input', () => {
- const result = pickDescriptionLang(undefined, 'es');
+ it('getDescription returns undefined for undefined input', () => {
+ const result = getDescription(undefined, 'es');
assert.deepEqual(result, undefined);
});
--
To view, visit https://gerrit.wikimedia.org/r/404534
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idc86f90674ef1bee4f6bbbb64b8058ed105d97f4
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/services/mobileapps
Gerrit-Branch: master
Gerrit-Owner: Mholloway <mhollo...@wikimedia.org>
Gerrit-Reviewer: BearND <bsitzm...@wikimedia.org>
Gerrit-Reviewer: Fjalapeno <cfl...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Mholloway <mhollo...@wikimedia.org>
Gerrit-Reviewer: Mhurd <mh...@wikimedia.org>
Gerrit-Reviewer: Ppchelko <ppche...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
