From b7689303c8a0a77ba75917c38cf0229270a7d816 Mon Sep 17 00:00:00 2001 From: Scott Date: Wed, 2 Sep 2020 18:46:54 +0000 Subject: [PATCH] Bug 1657306 - Fixing old newtab AS ignoring user pref. r=gvn Differential Revision: https://phabricator.services.mozilla.com/D88642 --- .../components/newtab/lib/TopStoriesFeed.jsm | 38 ++++++-- .../test/unit/lib/TopStoriesFeed.test.js | 86 ++++++++++++++++++- 2 files changed, 114 insertions(+), 10 deletions(-) diff --git a/browser/components/newtab/lib/TopStoriesFeed.jsm b/browser/components/newtab/lib/TopStoriesFeed.jsm index 3a06f601c437..7d9dc667b1d6 100644 --- a/browser/components/newtab/lib/TopStoriesFeed.jsm +++ b/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) { diff --git a/browser/components/newtab/test/unit/lib/TopStoriesFeed.test.js b/browser/components/newtab/test/unit/lib/TopStoriesFeed.test.js index a8fdb4ed7b2a..37fb606e38b0 100644 --- a/browser/components/newtab/test/unit/lib/TopStoriesFeed.test.js +++ b/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", () => {