Bug 1657306 - Fixing old newtab AS ignoring user pref. r=gvn

Differential Revision: https://phabricator.services.mozilla.com/D88642

browser/components/newtab/lib/TopStoriesFeed.jsm

@@ -54,6 +54,7 @@ const DISCOVERY_STREAM_PREF_ENABLED_PATH =
   "browser.newtabpage.activity-stream.discoverystream.enabled";
 const REC_IMPRESSION_TRACKING_PREF = "feeds.section.topstories.rec.impressions";
 const OPTIONS_PREF = "feeds.section.topstories.options";
+const PREF_USER_TOPSTORIES = "feeds.section.topstories";
 const MAX_LIFETIME_CAP = 500; // Guard against misconfiguration on the server
 const DISCOVERY_STREAM_PREF = "discoverystream.config";
 
@@ -778,23 +779,26 @@ this.TopStoriesFeed = class TopStoriesFeed {
     return false;
   }
 
-  lazyLoadTopStories(dsPref) {
-    let _dsPref = dsPref;
-    if (!_dsPref) {
-      _dsPref = this.store.getState().Prefs.values[DISCOVERY_STREAM_PREF];
+  lazyLoadTopStories(options = {}) {
+    let { dsPref, userPref } = options;
+    if (!dsPref) {
+      dsPref = this.store.getState().Prefs.values[DISCOVERY_STREAM_PREF];
+    }
+    if (!userPref) {
+      userPref = this.store.getState().Prefs.values[PREF_USER_TOPSTORIES];
     }
 
     try {
       this.discoveryStreamEnabled =
-        JSON.parse(_dsPref).enabled &&
+        JSON.parse(dsPref).enabled &&
         this.store.getState().Prefs.values[DISCOVERY_STREAM_PREF_ENABLED];
     } catch (e) {
       // Load activity stream top stories if fail to determine discovery stream state
       this.discoveryStreamEnabled = false;
     }
 
-    // Return without invoking initialization if top stories are loaded
-    if (this.storiesLoaded) {
+    // Return without invoking initialization if top stories are loaded, or preffed off.
+    if (this.storiesLoaded || !userPref) {
       return;
     }
 
@@ -811,11 +815,19 @@ this.TopStoriesFeed = class TopStoriesFeed {
         break;
       case at.PREF_CHANGED:
         if (action.data.name === DISCOVERY_STREAM_PREF) {
-          this.lazyLoadTopStories(action.data.value);
+          this.lazyLoadTopStories({ dsPref: action.data.value });
         }
         if (action.data.name === DISCOVERY_STREAM_PREF_ENABLED) {
           this.lazyLoadTopStories();
         }
+        if (action.data.name === PREF_USER_TOPSTORIES) {
+          if (action.data.value) {
+            // init topstories if value if true.
+            this.lazyLoadTopStories({ userPref: action.data.value });
+          } else {
+            this.uninit();
+          }
+        }
         break;
       case at.UNINIT:
         this.uninit();
@@ -900,7 +912,15 @@ this.TopStoriesFeed = class TopStoriesFeed {
       }
       case at.PREF_CHANGED:
         if (action.data.name === DISCOVERY_STREAM_PREF) {
-          this.lazyLoadTopStories(action.data.value);
+          this.lazyLoadTopStories({ dsPref: action.data.value });
+        }
+        if (action.data.name === PREF_USER_TOPSTORIES) {
+          if (action.data.value) {
+            // init topstories if value if true.
+            this.lazyLoadTopStories({ userPref: action.data.value });
+          } else {
+            this.uninit();
+          }
         }
         // Check if spocs was disabled. Remove them if they were.
         if (action.data.name === "showSponsored" && !action.data.value) {

browser/components/newtab/test/unit/lib/TopStoriesFeed.test.js

@@ -95,7 +95,14 @@ describe("Top Stories Feed", () => {
     instance = new TopStoriesFeed();
     instance.store = {
       getState() {
-        return { Prefs: { values: { showSponsored: true } } };
+        return {
+          Prefs: {
+            values: {
+              showSponsored: true,
+              "feeds.section.topstories": true,
+            },
+          },
+        };
       },
       dispatch: sinon.spy(),
     };
@@ -117,6 +124,7 @@ describe("Top Stories Feed", () => {
         Prefs: {
           values: {
             "discoverystream.config": JSON.stringify({ enabled: true }),
+            "feeds.section.topstories": true,
           },
         },
       });
@@ -129,6 +137,7 @@ describe("Top Stories Feed", () => {
         Prefs: {
           values: {
             "discoverystream.config": JSON.stringify({ enabled: false }),
+            "feeds.section.topstories": true,
           },
         },
       });
@@ -172,6 +181,7 @@ describe("Top Stories Feed", () => {
           values: {
             "discoverystream.config": JSON.stringify({ enabled: true }),
             "discoverystream.enabled": true,
+            "feeds.section.topstories": true,
           },
         },
       });
@@ -193,6 +203,7 @@ describe("Top Stories Feed", () => {
         Prefs: {
           values: {
             "discoverystream.config": JSON.stringify({ enabled: false }),
+            "feeds.section.topstories": true,
           },
         },
       });
@@ -258,6 +269,79 @@ describe("Top Stories Feed", () => {
       assert.calledOnce(instance.lazyLoadTopStories);
       assert.notCalled(instance.onInit);
     });
+    it("should not init props if ds pref is true", () => {
+      sinon.stub(instance, "initializeProperties");
+      instance.propertiesInitialized = false;
+      instance.store.getState = () => ({
+        Prefs: {
+          values: {
+            "discoverystream.config": JSON.stringify({ enabled: false }),
+            "discoverystream.enabled": true,
+            "feeds.section.topstories": true,
+          },
+        },
+      });
+      instance.lazyLoadTopStories({
+        dsPref: JSON.stringify({ enabled: true }),
+      });
+      assert.notCalled(instance.initializeProperties);
+    });
+    it("should fire init if user pref is true", () => {
+      sinon.stub(instance, "onInit");
+      instance.store.getState = () => ({
+        Prefs: {
+          values: {
+            "discoverystream.config": JSON.stringify({ enabled: false }),
+            "discoverystream.enabled": false,
+            "feeds.section.topstories": false,
+          },
+        },
+      });
+      instance.lazyLoadTopStories({ userPref: true });
+      assert.calledOnce(instance.onInit);
+    });
+    it("should fire uninit if topstories update to false", () => {
+      sinon.stub(instance, "uninit");
+      instance.discoveryStreamEnabled = false;
+      instance.onAction({
+        type: at.PREF_CHANGED,
+        data: {
+          value: false,
+          name: "feeds.section.topstories",
+        },
+      });
+      assert.calledOnce(instance.uninit);
+      instance.discoveryStreamEnabled = true;
+      instance.onAction({
+        type: at.PREF_CHANGED,
+        data: {
+          value: false,
+          name: "feeds.section.topstories",
+        },
+      });
+      assert.calledTwice(instance.uninit);
+    });
+    it("should fire lazyLoadTopstories if topstories update to true", () => {
+      sinon.stub(instance, "lazyLoadTopStories");
+      instance.discoveryStreamEnabled = false;
+      instance.onAction({
+        type: at.PREF_CHANGED,
+        data: {
+          value: true,
+          name: "feeds.section.topstories",
+        },
+      });
+      assert.calledOnce(instance.lazyLoadTopStories);
+      instance.discoveryStreamEnabled = true;
+      instance.onAction({
+        type: at.PREF_CHANGED,
+        data: {
+          value: true,
+          name: "feeds.section.topstories",
+        },
+      });
+      assert.calledTwice(instance.lazyLoadTopStories);
+    });
   });
 
   describe("#init", () => {