Bug 1520889 - Pref for toggling spocs on and off. (#4718)

* Bug 1520889 - Pref for toggling spocs on and off. * review
mozilla · Jan 24, 2019 · 46828d5 · 46828d5
1 parent 5949260
commit 46828d5
Show file tree

Hide file tree

Showing 3 changed files with 94 additions and 25 deletions.
diff --git a/lib/ActivityStream.jsm b/lib/ActivityStream.jsm
@@ -217,6 +217,7 @@ const PREFS_CONFIG = new Map([
     title: "Configuration for the new pocket new tab",
     value: JSON.stringify({
       enabled: false,
+      show_spocs: true,
       // This is currently an exmple layout used for dev purposes.
       layout_endpoint: "https://getpocket.com/v3/newtab/layout?version=1&consumer_key=40249-e88c401e1b1f2242d9e441c4&layout_variant=basic",
     }),

diff --git a/lib/DiscoveryStreamFeed.jsm b/lib/DiscoveryStreamFeed.jsm
@@ -42,6 +42,12 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
     return this._prefCache.config;
   }
 
+  get showSpocs() {
+    // showSponsored is generally a use set spoc opt out,
+    // show_spocs is generally a mozilla set value.
+    return this.store.getState().Prefs.values.showSponsored && this.config.show_spocs;
+  }
+
   setupPrefs() {
     Services.prefs.addObserver(CONFIG_PREF_NAME, this);
     // Send the initial state of the pref on our reducer
@@ -157,31 +163,40 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
 
   async loadSpocs() {
     const cachedData = await this.cache.get() || {};
-    let {spocs} = cachedData;
-    if (!spocs || !(Date.now() - spocs.lastUpdated < SPOCS_FEEDS_UPDATE_TIME)) {
-      const spocsResponse = await this.fetchSpocs();
-      if (spocsResponse) {
-        spocs = {
-          lastUpdated: Date.now(),
-          data: spocsResponse,
-        };
-        await this.cache.set("spocs", spocs);
-      } else {
-        Cu.reportError("No response for spocs_endpoint prop");
-        // Use old data if we have it, otherwise nothing.
-        spocs = spocs || {};
+    let spocs;
+
+    if (this.showSpocs) {
+      spocs = cachedData.spocs;
+      if (!spocs || !(Date.now() - spocs.lastUpdated < SPOCS_FEEDS_UPDATE_TIME)) {
+        const spocsResponse = await this.fetchSpocs();
+        if (spocsResponse) {
+          spocs = {
+            lastUpdated: Date.now(),
+            data: spocsResponse,
+          };
+          await this.cache.set("spocs", spocs);
+        } else {
+          Cu.reportError("No response for spocs_endpoint prop");
+        }
       }
     }
 
-    if (spocs) {
-      this.store.dispatch(ac.BroadcastToContent({
-        type: at.DISCOVERY_STREAM_SPOCS_UPDATE,
-        data: {
-          lastUpdated: spocs.lastUpdated,
-          spocs: spocs.data,
-        },
-      }));
-    }
+    // Use good data if we have it, otherwise nothing.
+    // We can have no data if spocs set to off.
+    // We can have no data if request fails and there is no good cache.
+    // We want to send an update spocs or not, so client can render something.
+    spocs = spocs || {
+      lastUpdated: Date.now(),
+      data: {},
+    };
+
+    this.store.dispatch(ac.BroadcastToContent({
+      type: at.DISCOVERY_STREAM_SPOCS_UPDATE,
+      data: {
+        lastUpdated: spocs.lastUpdated,
+        spocs: spocs.data,
+      },
+    }));
   }
 
   async getComponentFeed(feedUrl) {
@@ -275,6 +290,12 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
         // When this feed is shutting down:
         this.uninitPrefs();
         break;
+      case at.PREF_CHANGED:
+        // Check if spocs was disabled. Remove them if they were.
+        if (action.data.name === "showSponsored") {
+          await this.loadSpocs();
+        }
+        break;
     }
   }
 };

diff --git a/test/unit/lib/DiscoveryStreamFeed.test.js b/test/unit/lib/DiscoveryStreamFeed.test.js
@@ -18,7 +18,7 @@ describe("DiscoveryStreamFeed", () => {
     sandbox = sinon.createSandbox();
     configPrefStub = sandbox.stub(global.Services.prefs, "getStringPref")
       .withArgs(CONFIG_PREF_NAME)
-      .returns(JSON.stringify({enabled: false, layout_endpoint: "foo.com"}));
+      .returns(JSON.stringify({enabled: false, show_spocs: false, layout_endpoint: "foo.com"}));
 
     // Fetch
     fetchStub = sandbox.stub(global, "fetch");
@@ -38,11 +38,11 @@ describe("DiscoveryStreamFeed", () => {
 
   describe("#observe", () => {
     it("should update state.DiscoveryStream.config when the pref changes", async () => {
-      configPrefStub.returns(JSON.stringify({enabled: true, layout_endpoint: "foo"}));
+      configPrefStub.returns(JSON.stringify({enabled: true, show_spocs: false, layout_endpoint: "foo"}));
 
       feed.observe(null, null, CONFIG_PREF_NAME);
 
-      assert.deepEqual(feed.store.getState().DiscoveryStream.config, {enabled: true, layout_endpoint: "foo"});
+      assert.deepEqual(feed.store.getState().DiscoveryStream.config, {enabled: true, show_spocs: false, layout_endpoint: "foo"});
     });
   });
 
@@ -173,6 +173,9 @@ describe("DiscoveryStreamFeed", () => {
   });
 
   describe("#loadSpocs", () => {
+    beforeEach(() => {
+      Object.defineProperty(feed, "showSpocs", {get: () => true});
+    });
     it("should fetch fresh data if cache is empty", async () => {
       sandbox.stub(feed.cache, "get").returns(Promise.resolve());
       sandbox.stub(feed, "fetchSpocs").returns(Promise.resolve("data"));
@@ -210,6 +213,19 @@ describe("DiscoveryStreamFeed", () => {
   });
 
   describe("#fetchSpocs", () => {
+    beforeEach(() => {
+      Object.defineProperty(feed, "showSpocs", {get: () => true});
+    });
+    it("should return null for fetchSpocs with no spocs_endpoint", async () => {
+      feed.store.dispatch(ac.BroadcastToContent({
+        type: at.DISCOVERY_STREAM_SPOCS_ENDPOINT,
+        data: "",
+      }));
+
+      const result = await feed.fetchSpocs();
+
+      assert.isNull(result);
+    });
     it("should return old spocs if fetch failed", async () => {
       sandbox.stub(feed.cache, "set").returns(Promise.resolve());
       feed.store.dispatch(ac.BroadcastToContent({
@@ -241,6 +257,37 @@ describe("DiscoveryStreamFeed", () => {
       assert.equal(feed.store.getState().DiscoveryStream.spocs.data, "new data");
     });
   });
+  describe("#showSpocs", () => {
+    it("should return false from showSpocs if user pref showSponsored is false", async () => {
+      feed.store.getState = () => ({
+        Prefs: {values: {showSponsored: false}},
+      });
+      Object.defineProperty(feed, "config", {get: () => ({show_spocs: true})});
+
+      assert.isFalse(feed.showSpocs);
+    });
+    it("should return false from showSpocs if DiscoveryStrea pref show_spocs is false", async () => {
+      feed.store.getState = () => ({
+        Prefs: {values: {showSponsored: true}},
+      });
+      Object.defineProperty(feed, "config", {get: () => ({show_spocs: false})});
+
+      assert.isFalse(feed.showSpocs);
+    });
+    it("should return true from showSpocs if both prefs are true", async () => {
+      feed.store.getState = () => ({
+        Prefs: {values: {showSponsored: true}},
+      });
+      Object.defineProperty(feed, "config", {get: () => ({show_spocs: true})});
+
+      assert.isTrue(feed.showSpocs);
+    });
+    it("should fire loadSpocs is showSponsored pref changes", async () => {
+      sandbox.stub(feed, "loadSpocs").returns(Promise.resolve());
+      await feed.onAction({type: at.PREF_CHANGED, data: {name: "showSponsored"}});
+      assert.calledOnce(feed.loadSpocs);
+    });
+  });
 
   describe("#clearCache", () => {
     it("should set .layout, .feeds and .spocs to {}", async () => {