Bug 1671670 - Update story sort to consider priority prop. r=gvn
authorScott <scott.downe@gmail.com>
Tue, 27 Oct 2020 17:07:51 +0000
changeset 554749 8b5452963b2cb9c754af6b8149439c3ca844199c
parent 554748 70e2b6b0b177ceb3c70569bcb0c795ad20723a83
child 554750 073a0a988ae1aefebc4e7d6d0a74c74625f1b23f
push id37898
push userabutkovits@mozilla.com
push dateWed, 28 Oct 2020 09:24:21 +0000
treeherdermozilla-central@83bf4fd3b1fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgvn
bugs1671670
milestone84.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1671670 - Update story sort to consider priority prop. r=gvn Differential Revision: https://phabricator.services.mozilla.com/D94647
browser/components/newtab/lib/DiscoveryStreamFeed.jsm
browser/components/newtab/test/unit/lib/DiscoveryStreamFeed.test.js
--- a/browser/components/newtab/lib/DiscoveryStreamFeed.jsm
+++ b/browser/components/newtab/lib/DiscoveryStreamFeed.jsm
@@ -957,16 +957,46 @@ this.DiscoveryStreamFeed = class Discove
   observe(subject, topic, data) {
     switch (topic) {
       case "idle-daily":
         this.updateDomainAffinityScores();
         break;
     }
   }
 
+  /*
+   * This function is used to sort any type of story, both spocs and recs.
+   * This uses hierarchical sorting, first sorting by priority, then by score within a priority.
+   * This function could be sorting an array of spocs or an array of recs.
+   * A rec would have priority undefined, and a spoc would probably have a priority set.
+   * Priority is sorted ascending, so low numbers are the highest priority.
+   * Score is sorted descending, so high numbers are the highest score.
+   * Undefined priority values are considered the lowest priority.
+   * A negative priority is considered the same as undefined, lowest priority.
+   * A negative priority is unlikely and not currently supported or expected.
+   * A negative score is a possible use case.
+   */
+  sortItem(a, b) {
+    // If the priorities are the same, sort based on score.
+    // If both item priorities are undefined,
+    // we can safely sort via score.
+    if (a.priority === b.priority) {
+      return b.score - a.score;
+    } else if (!a.priority || a.priority <= 0) {
+      // If priority is undefined or an unexpected value,
+      // consider it lowest priority.
+      return 1;
+    } else if (!b.priority || b.priority <= 0) {
+      // Also consider this case lowest priority.
+      return -1;
+    }
+    // Our primary sort for items with priority.
+    return a.priority - b.priority;
+  }
+
   async scoreItems(items, type) {
     const filtered = [];
     const scoreStart = Cu.now();
     const spocsPersonalized = this.store.getState().Prefs.values[
       PREF_SPOCS_PERSONALIZED
     ];
     const recsPersonalized = this.store.getState().Prefs.values[
       PREF_RECS_PERSONALIZED
@@ -983,17 +1013,17 @@ this.DiscoveryStreamFeed = class Discove
       .filter(s => {
         if (s.score >= s.min_score) {
           return true;
         }
         filtered.push(s);
         return false;
       })
       // Sort by highest scores.
-      .sort((a, b) => b.score - a.score);
+      .sort(this.sortItem);
 
     if (this.personalized && personalizedByType) {
       this.providerSwitcher.dispatchRelevanceScoreDuration(scoreStart);
     }
     return { data, filtered };
   }
 
   async scoreItem(item, personalizedByType) {
--- a/browser/components/newtab/test/unit/lib/DiscoveryStreamFeed.test.js
+++ b/browser/components/newtab/test/unit/lib/DiscoveryStreamFeed.test.js
@@ -1195,16 +1195,64 @@ describe("DiscoveryStreamFeed", () => {
 
       assert.deepEqual(result, [
         { id: 1, flight_id: 1, item_score: 0.9, score: 0.9, min_score: 0.1 },
         { id: 2, flight_id: 2, item_score: 0.8, score: 0.8, min_score: 0.1 },
         { id: 3, flight_id: 3, item_score: 0.7, score: 0.7, min_score: 0.1 },
       ]);
     });
 
+    it("should sort based on priority", async () => {
+      const { data: result } = await feed.scoreItems([
+        { id: 6, flight_id: 6, priority: 2, item_score: 0.7, min_score: 0.1 },
+        { id: 2, flight_id: 3, priority: 1, item_score: 0.2, min_score: 0.1 },
+        { id: 4, flight_id: 4, item_score: 0.6, min_score: 0.1 },
+        { id: 5, flight_id: 5, priority: 2, item_score: 0.8, min_score: 0.1 },
+        { id: 3, flight_id: 3, item_score: 0.8, min_score: 0.1 },
+        { id: 1, flight_id: 1, priority: 1, item_score: 0.3, min_score: 0.1 },
+      ]);
+
+      assert.deepEqual(result, [
+        {
+          id: 1,
+          flight_id: 1,
+          priority: 1,
+          score: 0.3,
+          item_score: 0.3,
+          min_score: 0.1,
+        },
+        {
+          id: 2,
+          flight_id: 3,
+          priority: 1,
+          score: 0.2,
+          item_score: 0.2,
+          min_score: 0.1,
+        },
+        {
+          id: 5,
+          flight_id: 5,
+          priority: 2,
+          score: 0.8,
+          item_score: 0.8,
+          min_score: 0.1,
+        },
+        {
+          id: 6,
+          flight_id: 6,
+          priority: 2,
+          score: 0.7,
+          item_score: 0.7,
+          min_score: 0.1,
+        },
+        { id: 3, flight_id: 3, item_score: 0.8, score: 0.8, min_score: 0.1 },
+        { id: 4, flight_id: 4, item_score: 0.6, score: 0.6, min_score: 0.1 },
+      ]);
+    });
+
     it("should remove items with scores lower than min_score", async () => {
       const { data: result, filtered } = await feed.scoreItems([
         { id: 2, flight_id: 2, item_score: 0.8, min_score: 0.9 },
         { id: 3, flight_id: 3, item_score: 0.7, min_score: 0.7 },
         { id: 1, flight_id: 1, item_score: 0.9, min_score: 0.8 },
       ]);
 
       assert.deepEqual(result, [