jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/385406 )
Change subject: Sort media endpoint results in order of appearance
......................................................................
Sort media endpoint results in order of appearance
We can leverage the fact that Document.querySelectorAll returns results in
document order to sort the image results in order of appearance.
Bug: T177430
Change-Id: I184d44c4c2babdd225cb83b73d9e87aaeaf0f496
---
M lib/gallery.js
M routes/media.js
M test/features/media/pagecontent.js
A test/lib/media/media-test.js
4 files changed, 68 insertions(+), 16 deletions(-)
Approvals:
BearND: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/gallery.js b/lib/gallery.js
index 95232b3..7e9a66e 100644
--- a/lib/gallery.js
+++ b/lib/gallery.js
@@ -1,13 +1,40 @@
'use strict';
const BBPromise = require('bluebird');
+const domino = require('domino');
const mwapi = require('./mwapi');
const api = require('./api-util');
+const Title = require('mediawiki-title').Title;
const MAX_ITEM_COUNT = 256;
const MIN_IMAGE_SIZE = 64;
const MAX_IMAGE_WIDTH = 1280;
+
+function dbKey(str, si) {
+ return Title.newFromText(str, si).getPrefixedDBKey();
+}
+
+/**
+ * Sort an array of media items in place by their order of appearance in an
HTML document.
+ * @param {!string} html a raw HTML document
+ * @param {!Array} media an array of media items as returned by
gallery.collectionPromise
+ * @param {!Object} si a site info object as returned by mwapi.getSiteInfo
+ */
+function sort(html, media, si) {
+ const doc = domino.createDocument(html);
+ const images = doc.querySelectorAll('img,video');
+ const titles = [];
+ // TODO: handle Mathoid-rendered math images
+ images.forEach((img) => {
+ if (img.hasAttribute('resource')) {
+ titles.push(img.getAttribute('resource').replace(/^\.\//, ''));
+ }
+ });
+ media.items.sort((a, b) => {
+ return titles.indexOf(dbKey(a.title, si)) -
titles.indexOf(dbKey(b.title, si));
+ });
+}
function getExtMetadataValues(extmetadata) {
const ext = {};
@@ -94,9 +121,6 @@
const mime = item.imageinfo[0].mime;
isVideo = mime.indexOf('ogg') > -1 || mime.indexOf('video') > -1;
- // request details individually, to keep the order
- // detailsPromises.push(galleryItemsPromise(domain, item.title,
isVideo));
-
if (isVideo) {
videos.push(item.title);
} else {
@@ -155,5 +179,6 @@
}
module.exports = {
+ sort,
collectionPromise
};
diff --git a/routes/media.js b/routes/media.js
index 0f84af6..392d028 100644
--- a/routes/media.js
+++ b/routes/media.js
@@ -4,16 +4,10 @@
const mUtil = require('../lib/mobile-util');
const parsoid = require('../lib/parsoid-access');
const sUtil = require('../lib/util');
+const mwapi = require('../lib/mwapi');
const gallery = require('../lib/gallery');
-/**
- * The main router object
- */
const router = sUtil.router();
-
-/**
- * The main application object reported when this module is require()d
- */
let app;
/**
@@ -22,11 +16,15 @@
*/
router.get('/media/:title', (req, res) => {
return BBPromise.props({
- page: parsoid.pageJsonPromise(app, req),
- media: gallery.collectionPromise(app, req)
+ page: parsoid.pageHtmlPromise(app, req),
+ media: gallery.collectionPromise(app, req),
+ siteinfo: mwapi.getSiteInfo(app, req)
}).then((response) => {
+ if (response.media.items && response.media.items.length > 1) {
+ gallery.sort(response.page.html, response.media,
response.siteinfo);
+ }
res.status(200);
- mUtil.setETag(res, response.page.revision);
+ mUtil.setETag(res, response.page.meta.revision);
mUtil.setContentType(res, mUtil.CONTENT_TYPES.unpublished);
res.json(response.media).end();
});
diff --git a/test/features/media/pagecontent.js
b/test/features/media/pagecontent.js
index 2915565..d2dee97 100644
--- a/test/features/media/pagecontent.js
+++ b/test/features/media/pagecontent.js
@@ -1,9 +1,9 @@
'use strict';
-const assert = require('../../utils/assert.js');
const preq = require('preq');
-const server = require('../../utils/server.js');
-const headers = require('../../utils/headers.js');
+const assert = require('../../utils/assert');
+const server = require('../../utils/server');
+const headers = require('../../utils/headers');
describe('media', function() {
diff --git a/test/lib/media/media-test.js b/test/lib/media/media-test.js
new file mode 100644
index 0000000..26e9b9c
--- /dev/null
+++ b/test/lib/media/media-test.js
@@ -0,0 +1,29 @@
+'use strict';
+
+const assert = require('../../utils/assert');
+const sort = require('../../../lib/gallery').sort;
+
+const siteInfo = require('../../fixtures/siteinfo_enwiki.json');
+const page = '<html><img resource="./File:Foo.jpg"/><video
resource="./File:Bar.ogv"/></html>';
+const unsorted = {
+ items: [
+ { title: "File:Bar.ogv" },
+ { title: "File:Foo.jpg" }
+ ]
+};
+const sorted = {
+ items: [
+ { title: "File:Foo.jpg" },
+ { title: "File:Bar.ogv" }
+ ]
+};
+
+describe('lib:media', () => {
+
+ it('Results should be sorted in order of appearance on the page', () => {
+ const result = unsorted;
+ sort(page, result, siteInfo);
+ assert.deepEqual(result, sorted);
+ });
+
+});
--
To view, visit https://gerrit.wikimedia.org/r/385406
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I184d44c4c2babdd225cb83b73d9e87aaeaf0f496
Gerrit-PatchSet: 11
Gerrit-Project: mediawiki/services/mobileapps
Gerrit-Branch: master
Gerrit-Owner: Mholloway <[email protected]>
Gerrit-Reviewer: BearND <[email protected]>
Gerrit-Reviewer: Dbrant <[email protected]>
Gerrit-Reviewer: Fjalapeno <[email protected]>
Gerrit-Reviewer: GWicke <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Mholloway <[email protected]>
Gerrit-Reviewer: Mhurd <[email protected]>
Gerrit-Reviewer: Mobrovac <[email protected]>
Gerrit-Reviewer: Ppchelko <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
