Bug 1436449 - Rename item_id to pocket_id for clearer way to identify saved Pocket items in NewTabUtils
MozReview-Commit-ID: 5DifxvQ6qF5
--- 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();
};