Skip to content

Commit

Permalink
Merge pull request mozilla#4085 from sarracini/bug_1451091
Browse files Browse the repository at this point in the history
Fix Bug 1451091 - Chronologically order highlights
  • Loading branch information
sarracini committed Apr 9, 2018
2 parents 55062ce + f3883b3 commit daa20aa
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
27 changes: 26 additions & 1 deletion system-addon/lib/HighlightsFeed.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,29 @@ this.HighlightsFeed = class HighlightsFeed {
}, []) : []);
}

/**
* Chronologically sort highlights of all types except 'visited'. Then just append
* the rest at the end of highlights.
* @param {Array} pages The full list of links to order.
* @return {Array} A sorted array of highlights
*/
_orderHighlights(pages) {
const splitHighlights = {chronologicalCandidates: [], visited: []};
for (let page of pages) {
// If we have a page that is both a history item and a bookmark, treat it
// as a bookmark
if (page.type === "history" && !page.bookmarkGuid) {
splitHighlights.visited.push(page);
} else {
splitHighlights.chronologicalCandidates.push(page);
}
}

return splitHighlights.chronologicalCandidates
.sort((a, b) => a.date_added < b.date_added)
.concat(splitHighlights.visited);
}

/**
* Refresh the highlights data for content.
* @param {bool} options.broadcast Should the update be broadcasted.
Expand All @@ -111,9 +134,11 @@ this.HighlightsFeed = class HighlightsFeed {
excludePocket: !this.store.getState().Prefs.values["section.highlights.includePocket"]
});

const orderedPages = this._orderHighlights(manyPages);

// Remove adult highlights if we need to
const checkedAdult = this.store.getState().Prefs.values.filterAdult ?
filterAdult(manyPages) : manyPages;
filterAdult(orderedPages) : orderedPages;

// Remove any Highlights that are in Top Sites already
const [, deduped] = this.dedupe.group(this.store.getState().TopSites.rows, checkedAdult);
Expand Down
20 changes: 20 additions & 0 deletions system-addon/test/unit/lib/HighlightsFeed.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,26 @@ describe("Highlights Feed", () => {
assert.calledOnce(feed.linksCache.request);
assert.calledOnce(fakeNewTabUtils.activityStreamLinks.getHighlights);
});
it("should chronologically order highlight data types", async () => {
links = [
{url: "https://site0.com", type: "bookmark", bookmarkGuid: "1234", date_added: Date.now() - 80}, // 4th newest
{url: "https://site1.com", type: "history", bookmarkGuid: "1234", date_added: Date.now() - 60}, // 3rd newest
{url: "https://site2.com", type: "history", date_added: Date.now() - 160}, // append at the end
{url: "https://site3.com", type: "history", date_added: Date.now() - 60}, // append at the end
{url: "https://site4.com", type: "pocket", date_added: Date.now()}, // newest highlight
{url: "https://site5.com", type: "pocket", date_added: Date.now() - 100}, // 5th newest
{url: "https://site6.com", type: "bookmark", bookmarkGuid: "1234", date_added: Date.now() - 40} // 2nd newest
];

let highlights = await fetchHighlights();
assert.equal(highlights[0].url, links[4].url);
assert.equal(highlights[1].url, links[6].url);
assert.equal(highlights[2].url, links[1].url);
assert.equal(highlights[3].url, links[0].url);
assert.equal(highlights[4].url, links[5].url);
assert.equal(highlights[5].url, links[2].url);
assert.equal(highlights[6].url, links[3].url);
});
it("should add hostname and hasImage to each link", async () => {
links = [{url: "https://mozilla.org"}];

Expand Down

0 comments on commit daa20aa

Please sign in to comment.