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

Reply via email to