Bug 1425496 - Add Recently Pocketed Items to Highlights draft
authorUrsula Sarracini <usarracini@mozilla.com>
Fri, 02 Feb 2018 17:06:06 -0500
changeset 750785 d6c42aae19de86f6085bf79c56bd4e6fc2a872fc
parent 749555 205707b678d24ba268b97668b78dc605f701a5e0
push id97752
push userusarracini@mozilla.com
push dateFri, 02 Feb 2018 22:06:29 +0000
bugs1425496
milestone60.0a1
Bug 1425496 - Add Recently Pocketed Items to Highlights MozReview-Commit-ID: LyXKkQkPXte
browser/components/customizableui/content/panelUI.js
browser/extensions/activity-stream/lib/HighlightsFeed.jsm
toolkit/modules/NewTabUtils.jsm
toolkit/modules/tests/xpcshell/test_NewTabUtils.js
--- a/browser/components/customizableui/content/panelUI.js
+++ b/browser/components/customizableui/content/panelUI.js
@@ -518,17 +518,18 @@ const PanelUI = {
    * Fetches the list of Recent Highlights and replaces the items in the Library
    * view with the results.
    */
   async fetchAndPopulateLibraryRecentHighlights() {
     let highlights = await NewTabUtils.activityStreamLinks.getHighlights({
       // As per bug 1402023, hard-coded limit, until Activity Stream develops a
       // richer list.
       numItems: 6,
-      withFavicons: true
+      withFavicons: true,
+      excludePocket: true
     }).catch(ex => {
       // Just hide the section if we can't retrieve the items from the database.
       Cu.reportError(ex);
       return [];
     });
 
     // Since the call above is asynchronous, the panel may be already hidden
     // at this point, but we still prepare the items for the next time the
--- a/browser/extensions/activity-stream/lib/HighlightsFeed.jsm
+++ b/browser/extensions/activity-stream/lib/HighlightsFeed.jsm
@@ -82,17 +82,18 @@ this.HighlightsFeed = class HighlightsFe
 
     // We broadcast when we want to force an update, so get fresh links
     if (options.broadcast) {
       this.linksCache.expire();
     }
 
     // Request more than the expected length to allow for items being removed by
     // deduping against Top Sites or multiple history from the same domain, etc.
-    const manyPages = await this.linksCache.request({numItems: MANY_EXTRA_LENGTH});
+    // Until bug 1425496 lands, do not include saved Pocket items in highlights
+    const manyPages = await this.linksCache.request({numItems: MANY_EXTRA_LENGTH, excludePocket: true});
 
     // Remove adult highlights if we need to
     const checkedAdult = this.store.getState().Prefs.values.filterAdult ?
       filterAdult(manyPages) : manyPages;
 
     // Remove any Highlights that are in Top Sites already
     const [, deduped] = this.dedupe.group(this.store.getState().TopSites.rows, checkedAdult);
 
--- a/toolkit/modules/NewTabUtils.jsm
+++ b/toolkit/modules/NewTabUtils.jsm
@@ -18,16 +18,19 @@ ChromeUtils.defineModuleGetter(this, "Pl
   "resource://gre/modules/PlacesUtils.jsm");
 
 ChromeUtils.defineModuleGetter(this, "PageThumbs",
   "resource://gre/modules/PageThumbs.jsm");
 
 ChromeUtils.defineModuleGetter(this, "BinarySearch",
   "resource://gre/modules/BinarySearch.jsm");
 
+ChromeUtils.defineModuleGetter(this, "pktApi",
+  "chrome://pocket/content/pktApi.jsm");
+
 XPCOMUtils.defineLazyGetter(this, "gCryptoHash", function() {
   return Cc["@mozilla.org/security/hash;1"].createInstance(Ci.nsICryptoHash);
 });
 
 XPCOMUtils.defineLazyGetter(this, "gUnicodeConverter", function() {
   let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"]
                     .createInstance(Ci.nsIScriptableUnicodeConverter);
   converter.charset = "utf8";
@@ -950,16 +953,21 @@ var ActivityStreamProvider = {
    *                    length, and favicon size (width)
    */
   _addFavicons(aLinks) {
     // Each link in the array needs a favicon for it's page - so we fire off a
     // promise for each link to compute the favicon data and attach it back to
     // the original link object. We must wait until all favicons for the array
     // of links are computed before returning
     return Promise.all(aLinks.map(link => new Promise(async resolve => {
+      // Never add favicon data for pocket items
+      if (link.type === "pocket") {
+        resolve(link);
+        return;
+      }
       let iconData;
       try {
         const linkUri = Services.io.newURI(link.url);
         iconData = await this._getIconData(linkUri);
 
         // Switch the scheme to try again with the other
         if (!iconData) {
           linkUri.scheme = linkUri.scheme === "https" ? "http" : "https";
@@ -970,16 +978,77 @@ var ActivityStreamProvider = {
       }
 
       // Add the icon data to the link if we have any
       resolve(Object.assign(link, iconData || {}));
     })));
   },
 
   /**
+   * Helper function which makes the call to the Pocket API to fetch the user's
+   * saved Pocket items.
+   */
+  fetchSavedPocketItems(requestData) {
+    if (!pktApi.isUserLoggedIn()) {
+      return Promise.resolve(null);
+    }
+    return new Promise((resolve, reject) => {
+      pktApi.retrieve(requestData, {
+        success(data) {
+          resolve(data);
+        },
+        error(error) {
+          reject(error);
+        }
+      });
+    });
+  },
+
+  /**
+   * Get the most recently Pocket-ed items from a user's Pocket list. See:
+   * https://getpocket.com/developer/docs/v3/retrieve for details
+   *
+   * @param {Object} aOptions
+   *   {int} numItems: The max number of pocket items to fetch
+   */
+  async getRecentlyPocketed(aOptions) {
+    const pocketSecondsAgo = Math.floor(Date.now() / 1000) - ACTIVITY_STREAM_DEFAULT_RECENT;
+    const requestData = {
+      detailType: "complete",
+      count: aOptions.numItems,
+      since: pocketSecondsAgo
+    };
+    let data;
+    try {
+      data = await this.fetchSavedPocketItems(requestData);
+      if (!data) {
+        return [];
+      }
+    } catch (e) {
+      Cu.reportError(e);
+      return [];
+    }
+    /* Extract relevant parts needed to show this card as a highlight:
+     * 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,
+                    title: item.resolved_title,
+                    url: item.resolved_url,
+                    item_id: item.item_id
+                  }));
+    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.
    *   {bool} ignoreBlocked: Do not filter out blocked links.
    *   {int}  numItems: Maximum number of items to return.
    */
   async getRecentBookmarks(aOptions) {
@@ -1291,30 +1360,36 @@ var ActivityStreamLinks = {
   },
 
   /**
    * 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.
    *
    * @return {Promise} Returns a promise with the array of links as the payload
    */
   async getHighlights(aOptions = {}) {
     aOptions.numItems = aOptions.numItems || ACTIVITY_STREAM_DEFAULT_LIMIT;
     const results = [];
 
     // First get bookmarks if we want them
     if (!aOptions.excludeBookmarks) {
       results.push(...await ActivityStreamProvider.getRecentBookmarks(aOptions));
     }
 
+    // Add the Pocket items if we need more and want them
+    if (aOptions.numItems - results.length > 0 && !aOptions.excludePocket) {
+      results.push(...await ActivityStreamProvider.getRecentlyPocketed(aOptions));
+    }
+
     // Add in history if we need more and want them
     if (aOptions.numItems - results.length > 0 && !aOptions.excludeHistory) {
       // Use the same numItems as bookmarks above in case we remove duplicates
       const history = await ActivityStreamProvider.getRecentHistory(aOptions);
 
       // Only include a url once in the result preferring the bookmark
       const bookmarkUrls = new Set(results.map(({url}) => url));
       for (const page of history) {
--- a/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
+++ b/toolkit/modules/tests/xpcshell/test_NewTabUtils.js
@@ -371,19 +371,27 @@ add_task(async function addFavicons() {
 
   // Check with http version of the link that doesn't have its own
   const nonHttps = [{url: links[0].url.replace("https", "http")}];
   await provider._addFavicons(nonHttps);
   Assert.equal(provider._faviconBytesToDataURI(nonHttps)[0].favicon, base64URL, "Got the same favicon");
   Assert.equal(nonHttps[0].faviconLength, links[0].faviconLength, "Got the same favicon length");
   Assert.equal(nonHttps[0].faviconSize, links[0].faviconSize, "Got the same favicon size");
   Assert.equal(nonHttps[0].mimeType, links[0].mimeType, "Got the same mime type");
+
+  // Check that we do not collect favicons for pocket items
+  const pocketItems = [{url: links[0].url}, {url: "https://mozilla1.com", type: "pocket"}];
+  await provider._addFavicons(pocketItems);
+  Assert.equal(provider._faviconBytesToDataURI(pocketItems)[0].favicon, base64URL, "Added favicon data only to the non-pocket item");
+  Assert.equal(pocketItems[1].favicon, null, "Did not add a favicon to the pocket item");
+  Assert.equal(pocketItems[1].mimeType, null, "Did not add mimeType to the pocket item");
+  Assert.equal(pocketItems[1].faviconSize, null, "Did not add a faviconSize to the pocket item");
 });
 
-add_task(async function getHighlights() {
+add_task(async function getHighlightsWithoutPocket() {
   const addMetadata = url => PlacesUtils.history.update({
     description: "desc",
     previewImageURL: "https://image/",
     url
   });
 
   await setUpActivityStreamTest();
 
@@ -494,16 +502,89 @@ add_task(async function getHighlights() 
   await PlacesTestUtils.addFavicons(new Map([[testURI, image1x1]]));
   links = await provider.getHighlights({ withFavicons: true });
   Assert.equal(links.length, 3, "We're not expecting a change in links");
   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/"
+  };
+  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 not have stored the second pocket item since it was deleted
+  Assert.equal(links.length, 2, "Should have 2 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");
+});
+
+add_task(async function getHighlightsWithPocketFailure() {
+  await setUpActivityStreamTest();
+
+  NewTabUtils.activityStreamProvider.fetchSavedPocketItems = function() {
+    throw new Error();
+  };
+  let provider = NewTabUtils.activityStreamLinks;
+  let links = await provider.getHighlights();
+  Assert.equal(links.length, 0, "Return empty links if we reject the promise");
+});
+
+add_task(async function getHighlightsWithPocketNoData() {
+  await setUpActivityStreamTest();
+
+  NewTabUtils.activityStreamProvider.fetchSavedPocketItems = () => {};
+
+  let provider = NewTabUtils.activityStreamLinks;
+  let links = await provider.getHighlights();
+  Assert.equal(links.length, 0, "Return empty links if we got no data back from the response");
+});
+
 add_task(async function getTopFrecentSites() {
   await setUpActivityStreamTest();
 
   let provider = NewTabUtils.activityStreamLinks;
   let links = await provider.getTopSites({topsiteFrecency: 100});
   Assert.equal(links.length, 0, "empty history yields empty links");
 
   // add a visit