Bug 1436449 - Rename item_id to pocket_id for clearer way to identify saved Pocket items in NewTabUtils draft
authorUrsula Sarracini <usarracini@mozilla.com>
Fri, 09 Feb 2018 15:24:31 -0500
changeset 753212 a060464c882534089a383546191cbb79bff874b3
parent 752511 0ac953fcddf10132eaecdb753d72b2ba5a43c32a
push id98519
push userusarracini@mozilla.com
push dateFri, 09 Feb 2018 20:29:16 +0000
bugs1436449
milestone60.0a1
Bug 1436449 - Rename item_id to pocket_id for clearer way to identify saved Pocket items in NewTabUtils MozReview-Commit-ID: 5DifxvQ6qF5
toolkit/modules/NewTabUtils.jsm
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -1026,21 +1026,29 @@ var ActivityStreamProvider = {
      * url, preview image, title, description, and the unique item_id
      * necessary for Pocket to identify the item
      */
     let items = Object.values(data.list)
                   // status "0" means not archived or deleted
                   .filter(item => item.status === "0")
                   .map(item => ({
                     description: item.excerpt,
-                    preview_image_url: item.has_image === "1" && item.image.src,
+                    preview_image_url: item.image && item.image.src,
                     title: item.resolved_title,
                     url: item.resolved_url,
-                    item_id: item.item_id
+                    pocket_id: item.item_id
                   }));
+
+    // Add bookmark guid for Pocket items that are also bookmarks
+    for (let item of items) {
+      const bookmarkData = await this.getBookmark({url: item.url});
+      if (bookmarkData) {
+        item.bookmarkGuid = bookmarkData.bookmarkGuid;
+      }
+    }
     return this._processHighlights(items, aOptions, "pocket");
   },
 
   /**
    * Get most-recently-created visited bookmarks for Activity Stream.
    *
    * @param {Object} aOptions
    *   {num}  bookmarkSecondsAgo: Maximum age of added bookmark.
@@ -1220,23 +1228,26 @@ var ActivityStreamProvider = {
     // Pick out the top links using the same comparer as before
     links = [...hosts.values()].sort(isOtherBetter).slice(0, origNumItems);
 
     // Get the favicons as data URI for now (until we use the favicon protocol)
     return this._faviconBytesToDataURI(await this._addFavicons(links));
   },
 
   /**
-   * Gets a specific bookmark given an id
+   * Gets a specific bookmark given some info about it
    *
-   * @param {String} aGuid
-   *          A bookmark guid to use as a refrence to fetch the bookmark
+   * @param {Obj} aInfo
+   *          An object with one and only one of the following properties:
+   *            - url
+   *            - guid
+   *            - parentGuid and index
    */
-  async getBookmark(aGuid) {
-    let bookmark = await PlacesUtils.bookmarks.fetch(aGuid);
+  async getBookmark(aInfo) {
+    let bookmark = await PlacesUtils.bookmarks.fetch(aInfo);
     if (!bookmark) {
       return null;
     }
     let result = {};
     result.bookmarkGuid = bookmark.guid;
     result.bookmarkTitle = bookmark.title;
     result.lastModified = bookmark.lastModified.getTime();
     result.url = bookmark.url.href;
@@ -1351,16 +1362,29 @@ var ActivityStreamLinks = {
    */
   deleteHistoryEntry(aUrl) {
     const url = aUrl;
     PinnedLinks.unpin({url});
     return PlacesUtils.history.remove(url);
   },
 
   /**
+   * Helper function which makes the call to the Pocket API to delete an item from
+   * a user's saved to Pocket feed.
+   *
+   * @param {Integer} aItemID
+   *           The unique pocket ID used to find the item to be deleted
+   *
+   *@returns {Promise} Returns a promise at completion
+   */
+  deletePocketEntry(aItemID) {
+    return new Promise((success, error) => pktApi.deleteItem(aItemID, {success, error}));
+  },
+
+  /**
    * Get the Highlights links to show on Activity Stream
    *
    * @param {Object} aOptions
    *   {bool} excludeBookmarks: Don't add bookmark items.
    *   {bool} excludeHistory: Don't add history items.
    *   {bool} excludePocket: Don't add Pocket items.
    *   {bool} withFavicons: Add favicon data: URIs, when possible.
    *   {int}  numItems: Maximum number of (bookmark or history) items to return.
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -505,63 +505,80 @@ add_task(async function getHighlightsWit
   Assert.equal(links[0].favicon, image1x1, "Link 1 should contain a favicon");
   Assert.equal(links[1].favicon, null, "Link 2 has no favicon data");
   Assert.equal(links[2].favicon, null, "Link 3 has no favicon data");
 });
 
 add_task(async function getHighlightsWithPocketSuccess() {
   await setUpActivityStreamTest();
 
-  const fakeResponse = {
-    list: {
-      "12345": {
-        image: {src: "foo.com/img.png"},
-        excerpt: "A description for foo",
-        resolved_title: "A title for foo",
-        resolved_url: "http://www.foo.com",
-        item_id: "12345",
-        status: "0",
-        has_image: "1"
-      },
-      "56789": {
-        item_id: "56789",
-        status: "2",
-      }
-    }
-  };
-
   // Add a bookmark
   let bookmark = {
       parentGuid: PlacesUtils.bookmarks.unfiledGuid,
       title: "foo",
       description: "desc",
       preview_image_url: "foo.com/img.png",
       url: "https://mozilla1.com/"
   };
+
+  const fakeResponse = {
+    list: {
+      "123": {
+        image: {src: "foo.com/img.png"},
+        excerpt: "A description for foo",
+        resolved_title: "A title for foo",
+        resolved_url: "http://www.foo.com",
+        item_id: "123",
+        status: "0"
+      },
+      "456": {
+        item_id: "456",
+        status: "2",
+      },
+      "789": {
+        resolved_url: bookmark.url,
+        excerpt: bookmark.description,
+        resolved_title: bookmark.title,
+        image: {src: bookmark.preview_image_url},
+        status: "0",
+        item_id: "789"
+      }
+    }
+  };
+
   await PlacesUtils.bookmarks.insert(bookmark);
   await PlacesTestUtils.addVisits(bookmark.url);
 
   NewTabUtils.activityStreamProvider.fetchSavedPocketItems = () => fakeResponse;
   let provider = NewTabUtils.activityStreamLinks;
   let links = await provider.getHighlights();
 
-  // We should have 1 bookmark followed by 1 pocket story in highlights
+  // We should have 1 bookmark followed by 2 pocket stories in highlights
   // We should not have stored the second pocket item since it was deleted
-  Assert.equal(links.length, 2, "Should have 2 links in highlights");
+  Assert.equal(links.length, 3, "Should have 3 links in highlights");
 
   // First highlight should be a bookmark
   Assert.equal(links[0].url, bookmark.url, "The first link is the bookmark");
 
   // Second highlight should be a Pocket item with the correct fields to display
-  Assert.equal(links[1].url, fakeResponse.list["12345"].resolved_url, "Correct Pocket item");
-  Assert.equal(links[1].type, "pocket", "Attached the correct type");
-  Assert.equal(links[1].preview_image_url, fakeResponse.list["12345"].image.src, "Correct preview image was added");
-  Assert.equal(links[1].title, fakeResponse.list["12345"].resolved_title, "Correct title was added");
-  Assert.equal(links[1].description, fakeResponse.list["12345"].excerpt, "Correct description was added");
-  Assert.equal(links[1].item_id, fakeResponse.list["12345"].item_id, "item_id was preserved");
+  let pocketItem = fakeResponse.list["123"];
+  let currentLink = links[1];
+  Assert.equal(currentLink.url, pocketItem.resolved_url, "Correct Pocket item");
+  Assert.equal(currentLink.type, "pocket", "Attached the correct type");
+  Assert.equal(currentLink.preview_image_url, pocketItem.image.src, "Correct preview image was added");
+  Assert.equal(currentLink.title, pocketItem.resolved_title, "Correct title was added");
+  Assert.equal(currentLink.description, pocketItem.excerpt, "Correct description was added");
+  Assert.equal(currentLink.pocket_id, pocketItem.item_id, "item_id was preserved");
+  Assert.equal(currentLink.bookmarkGuid, undefined, "Should not have a bookmarkGuid");
+
+  // Third highlight should still be a Pocket item but since it was bookmarked, it has a bookmarkGuid
+  pocketItem = fakeResponse.list["789"];
+  currentLink = links[2];
+  Assert.equal(currentLink.url, pocketItem.resolved_url, "Correct Pocket item");
+  Assert.ok(currentLink.bookmarkGuid, "Attached a bookmarkGuid for this Pocket item");
 });
 
 add_task(async function getHighlightsWithPocketFailure() {
   await setUpActivityStreamTest();
 
   NewTabUtils.activityStreamProvider.fetchSavedPocketItems = function() {
     throw new Error();
   };