jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/381370 )
Change subject: Fix: unmarshalled extracts that contain no paragraphs ...................................................................... Fix: unmarshalled extracts that contain no paragraphs - When an extract contains no paragraphs, unmarshal the entire extract as a property. This was the intended but broken behavior. - Omit undefined properties from PageSummary and add a test. - Rename parseExtract() and parameter to include the word "HTML" in their names for clarity. Change-Id: Ib451bd7c607249e0d6a840e88969e9971b7a75df --- M src/client/tsconfig.json M src/common/marshallers/page-unmarshaller.test.ts M src/common/marshallers/page-unmarshaller.ts 3 files changed, 59 insertions(+), 18 deletions(-) Approvals: Jhernandez: Looks good to me, approved jenkins-bot: Verified diff --git a/src/client/tsconfig.json b/src/client/tsconfig.json index 27816ce..6b83ffd 100644 --- a/src/client/tsconfig.json +++ b/src/client/tsconfig.json @@ -8,6 +8,6 @@ // https://github.com/Microsoft/TypeScript/issues/16675 // https://github.com/DefinitelyTyped/DefinitelyTyped/issues/15020 "module": "esnext", - "lib": [ "es5", "es2015.promise", "dom" ] + "lib": [ "es5", "es2015.iterable", "es2015.promise", "dom" ] } } diff --git a/src/common/marshallers/page-unmarshaller.test.ts b/src/common/marshallers/page-unmarshaller.test.ts index 22418a5..eada598 100644 --- a/src/common/marshallers/page-unmarshaller.test.ts +++ b/src/common/marshallers/page-unmarshaller.test.ts @@ -1,13 +1,49 @@ import * as assert from "assert"; import * as fetch from "node-fetch"; -import { pageSummaryReviver } from "../models/page"; +import { PageSummary, pageSummaryReviver } from "../models/page"; import { unmarshalPageSummary } from "./page-unmarshaller"; +import { RESTBase } from "./restbase"; +const ETAG = "802006980/4f754377-a235-11e7-a776-efb84f18649a"; const HEADERS = new fetch.Headers(); -HEADERS.append("etag", "802006980/4f754377-a235-11e7-a776-efb84f18649a"); +HEADERS.append("etag", ETAG); + +const NOW = new Date(Date.now()).toString(); describe("page-unmarshaller", () => { - it("unmarshalPageSummary() unmarshals", () => { + describe(".unmarshalPageSummary() unmarshals", () => { + // eslint-disable-next-line max-len + it("omits undefined properties and returns extract even when there are no paragraphs", () => { + const json: RESTBase.PageSummary.PageSummary = { + title: "title", + displaytitle: "displaytitle", + pageid: 1, + extract: "extract", + extract_html: "extract_html", + lang: "en", + dir: "ltr", + timestamp: NOW, + description: "description" + }; + const expected: PageSummary = { + pageID: 1, + titleText: "title", + titleHTML: "displaytitle", + descriptionText: "description", + lastModified: new Date(Date.parse(NOW)), + etag: ETAG, + wikiLanguageCode: "en", + localeDirection: "ltr", + extractText: "extract", + extractHTML: ["extract_html"] + }; + const input = { headers: HEADERS, json: json as {} }; + const actual = unmarshalPageSummary(input); + assert.deepStrictEqual(actual, expected); + }); + }); + + it("unmarshalPageSummary() unmarshals a server response", () => { const input = require("./page-restbase-mount-everest-input.test.json"); const result = unmarshalPageSummary({ headers: HEADERS, json: input }); const expected = JSON.parse( diff --git a/src/common/marshallers/page-unmarshaller.ts b/src/common/marshallers/page-unmarshaller.ts index 5033d78..83e5a41 100644 --- a/src/common/marshallers/page-unmarshaller.ts +++ b/src/common/marshallers/page-unmarshaller.ts @@ -58,18 +58,17 @@ // Domino was chosen for familiarity and because it's already used on the MCS // backend. This is a service-only dependency so the client filesize is not // affected at the expense of two different code paths. -const parseExtract = (extract: string) => { +const parseExtractHTML = (extractHTML: string) => { const element = typeof document === "undefined" ? require("domino").createDocument().body : document.implementation.createHTMLDocument("").body; - element.innerHTML = extract; - const paragraphs = Array.prototype.slice.call(element.querySelectorAll("p")); - return ( - paragraphs.map( - (paragraph: HTMLParagraphElement) => paragraph.outerHTML - ) || [extract] - ); + element.innerHTML = extractHTML; + + const paragraphs = Array.from(element.querySelectorAll("p")); + return paragraphs.length + ? paragraphs.map((paragraph: HTMLParagraphElement) => paragraph.outerHTML) + : [extractHTML]; }; export const unmarshalETag = (headers: IsomorphicHeaders): RESTBase.ETag => { @@ -88,7 +87,7 @@ json: JSONObject; }): PageSummary => { const type: RESTBase.PageSummary.PageSummary = json as any; - return { + const result: PageSummary = { wikiLanguageCode: type.lang, localeDirection: type.dir, pageID: type.pageid, @@ -97,11 +96,17 @@ titleHTML: type.displaytitle, descriptionText: type.description, extractText: type.extract, - extractHTML: parseExtract(type.extract_html), - thumbnail: type.thumbnail && unmarshalPageThumbnail(type.thumbnail as {}), - image: type.originalimage && unmarshalPageImage(type.originalimage as {}), - geolocation: - type.coordinates && unmarshalPageGeolocation(type.coordinates as {}), + extractHTML: parseExtractHTML(type.extract_html), etag: unmarshalETag(headers) }; + if (type.coordinates) { + result.geolocation = unmarshalPageGeolocation(type.coordinates as {}); + } + if (type.thumbnail) { + result.thumbnail = unmarshalPageThumbnail(type.thumbnail as {}); + } + if (type.originalimage) { + result.image = unmarshalPageImage(type.originalimage as {}); + } + return result; }; -- To view, visit https://gerrit.wikimedia.org/r/381370 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib451bd7c607249e0d6a840e88969e9971b7a75df Gerrit-PatchSet: 4 Gerrit-Project: marvin Gerrit-Branch: master Gerrit-Owner: Niedzielski <sniedziel...@wikimedia.org> Gerrit-Reviewer: Jhernandez <jhernan...@wikimedia.org> Gerrit-Reviewer: Niedzielski <sniedziel...@wikimedia.org> Gerrit-Reviewer: Sniedzielski <sniedziel...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits