Bug 1613739 - Pref to switch story rows based on region r=gvn

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

--HG--
extra : moz-landing-system : lando

browser/app/profile/firefox.js

@@ -1296,12 +1296,12 @@ pref("browser.newtabpage.activity-stream.asrouter.useRemoteL10n", true);
 pref("browser.newtabpage.activity-stream.discoverystream.enabled", true);
 pref("browser.newtabpage.activity-stream.discoverystream.hardcoded-basic-layout", false);
 pref("browser.newtabpage.activity-stream.discoverystream.spocs-endpoint", "");
-// List of langs that get the 7 row layout.
-pref("browser.newtabpage.activity-stream.discoverystream.lang-layout-config", "en");
 // List of regions that get stories by default.
 pref("browser.newtabpage.activity-stream.discoverystream.region-stories-config", "US,DE,CA");
 // List of regions that get spocs by default.
 pref("browser.newtabpage.activity-stream.discoverystream.region-spocs-config", "US");
+// List of regions that get the 7 row layout.
+pref("browser.newtabpage.activity-stream.discoverystream.region-layout-config", "US,CA");
 // Switch between different versions of the recommendation provider.
 pref("browser.newtabpage.activity-stream.discoverystream.personalization.version", 1);
 // Configurable keys used by personalization version 2.

browser/components/newtab/docs/v2-system-addon/preferences.md

@@ -193,6 +193,22 @@ A comma separated list of geos that by default have stories enabled in newtab. I
 
 A comma separated list of geos that by default have spocs enabled in newtab. It matches the client's geo with that list.
 
+#### `browser.newtabpage.activity-stream.discoverystream.region-layout-config`
+
+- Type: `string`
+- Default: `US,CA`
+- Pref Type: Firefox
+
+A comma separated list of geos that have 7 rows of stories enabled in newtab. It matches the client's geo with that list.
+
+#### `browser.newtabpage.activity-stream.discoverystream.region-basic-layout`
+
+- Type: `boolean`
+- Default: false
+- Pref Type: AS
+
+If this is `true` newtabs with stories enabled see 1 row. It is set programmatically based on the result from region-layout-config.
+
 #### `browser.newtabpage.activity-stream.discoverystream.spocs-endpoint`
 
 - Type: `string`

browser/components/newtab/lib/ActivityStream.jsm

@@ -141,6 +141,8 @@ const REGION_STORIES_CONFIG =
   "browser.newtabpage.activity-stream.discoverystream.region-stories-config";
 const REGION_SPOCS_CONFIG =
   "browser.newtabpage.activity-stream.discoverystream.region-spocs-config";
+const REGION_LAYOUT_CONFIG =
+  "browser.newtabpage.activity-stream.discoverystream.region-layout-config";
 
 // Determine if spocs should be shown for a geo/locale
 function showSpocs({ geo }) {
@@ -514,6 +516,21 @@ const PREFS_CONFIG = new Map([
       value: false,
     },
   ],
+  [
+    "discoverystream.region-basic-layout",
+    {
+      title: "Decision to use basic layout based on region.",
+      getValue: ({ geo }) => {
+        const preffedRegionsString =
+          Services.prefs.getStringPref(REGION_LAYOUT_CONFIG) || "";
+        const preffedRegions = preffedRegionsString
+          .split(",")
+          .map(s => s.trim());
+
+        return !preffedRegions.includes(geo);
+      },
+    },
+  ],
   [
     "discoverystream.spoc.impressions",
     {

browser/components/newtab/lib/DiscoveryStreamFeed.jsm

@@ -58,7 +58,7 @@ const PREF_IMPRESSION_ID = "browser.newtabpage.activity-stream.impressionId";
 const PREF_ENABLED = "discoverystream.enabled";
 const PREF_HARDCODED_BASIC_LAYOUT = "discoverystream.hardcoded-basic-layout";
 const PREF_SPOCS_ENDPOINT = "discoverystream.spocs-endpoint";
-const PREF_LANG_LAYOUT_CONFIG = "discoverystream.lang-layout-config";
+const PREF_REGION_BASIC_LAYOUT = "discoverystream.region-basic-layout";
 const PREF_TOPSTORIES = "feeds.section.topstories";
 const PREF_SPOCS_CLEAR_ENDPOINT = "discoverystream.endpointSpocsClear";
 const PREF_SHOW_SPONSORED = "showSponsored";
@@ -376,15 +376,10 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
     }
 
     if (!layoutResp || !layoutResp.layout) {
-      const langLayoutConfig =
-        this.store.getState().Prefs.values[PREF_LANG_LAYOUT_CONFIG] || "";
-
       const isBasic =
         this.config.hardcoded_basic_layout ||
         this.store.getState().Prefs.values[PREF_HARDCODED_BASIC_LAYOUT] ||
-        !langLayoutConfig
-          .split(",")
-          .find(lang => this.locale.startsWith(lang.trim()));
+        this.store.getState().Prefs.values[PREF_REGION_BASIC_LAYOUT];
 
       // Set a hardcoded layout if one is needed.
       // Changing values in this layout in memory object is unnecessary.
@@ -1551,7 +1546,6 @@ this.DiscoveryStreamFeed = class DiscoveryStreamFeed {
       case PREF_ENABLED:
       case PREF_HARDCODED_BASIC_LAYOUT:
       case PREF_SPOCS_ENDPOINT:
-      case PREF_LANG_LAYOUT_CONFIG:
         // This is a config reset directly related to Discovery Stream pref.
         this.configReset();
         break;

browser/components/newtab/lib/PrefsFeed.jsm

@@ -113,7 +113,6 @@ this.PrefsFeed = class PrefsFeed {
 
     this._setBoolPref(values, "discoverystream.enabled", false);
     this._setBoolPref(values, "discoverystream.hardcoded-basic-layout", false);
-    this._setStringPref(values, "discoverystream.lang-layout-config", "");
     this._setStringPref(
       values,
       "discoverystream.personalization.modelKeys",

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

@@ -270,6 +270,44 @@ describe("ActivityStream", () => {
       );
     });
   });
+  describe("discoverystream.region-basic-layout config", () => {
+    let getStringPrefStub;
+    beforeEach(() => {
+      sandbox.stub(global.Services.prefs, "prefHasUserValue").returns(true);
+      getStringPrefStub = sandbox.stub(global.Services.prefs, "getStringPref");
+      getStringPrefStub.withArgs("browser.search.region").returns("CA");
+
+      sandbox
+        .stub(global.Services.locale, "appLocaleAsBCP47")
+        .get(() => "en-CA");
+    });
+    it("should enable 1 row layout pref based on region layout pref", () => {
+      getStringPrefStub
+        .withArgs(
+          "browser.newtabpage.activity-stream.discoverystream.region-layout-config"
+        )
+        .returns("US");
+
+      as._updateDynamicPrefs();
+
+      assert.isTrue(
+        PREFS_CONFIG.get("discoverystream.region-basic-layout").value
+      );
+    });
+    it("should enable 7 row layout pref based on region layout pref", () => {
+      getStringPrefStub
+        .withArgs(
+          "browser.newtabpage.activity-stream.discoverystream.region-layout-config"
+        )
+        .returns("US,CA");
+
+      as._updateDynamicPrefs();
+
+      assert.isFalse(
+        PREFS_CONFIG.get("discoverystream.region-basic-layout").value
+      );
+    });
+  });
   describe("_updateDynamicPrefs topstories default value", () => {
     let getStringPrefStub;
     let appLocaleAsBCP47Stub;

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

@@ -315,7 +315,7 @@ describe("DiscoveryStreamFeed", () => {
       const { layout } = feed.store.getState().DiscoveryStream;
       assert.equal(layout[0].components[2].properties.items, 3);
     });
-    it("should use 1 row layout if locale lang doesn't support 7 row layout", async () => {
+    it("should use 1 row layout if specified", async () => {
       feed.config.hardcoded_layout = true;
       feed.store = createStore(combineReducers(reducers), {
         Prefs: {
@@ -327,11 +327,10 @@ describe("DiscoveryStreamFeed", () => {
             }),
             [ENDPOINTS_PREF_NAME]: DUMMY_ENDPOINT,
             "discoverystream.enabled": true,
-            "discoverystream.lang-layout-config": "en",
+            "discoverystream.region-basic-layout": true,
           },
         },
       });
-      feed.locale = "de-DE";
       sandbox.stub(feed, "fetchLayout").returns(Promise.resolve(""));
 
       await feed.loadLayout(feed.store.dispatch);
@@ -339,7 +338,7 @@ describe("DiscoveryStreamFeed", () => {
       const { layout } = feed.store.getState().DiscoveryStream;
       assert.equal(layout[0].components[2].properties.items, 3);
     });
-    it("should use 7 row layout if locale lang supports it", async () => {
+    it("should use 7 row layout if specified", async () => {
       feed.config.hardcoded_layout = true;
       feed.store = createStore(combineReducers(reducers), {
         Prefs: {
@@ -351,11 +350,10 @@ describe("DiscoveryStreamFeed", () => {
             }),
             [ENDPOINTS_PREF_NAME]: DUMMY_ENDPOINT,
             "discoverystream.enabled": true,
-            "discoverystream.lang-layout-config": "en,de",
+            "discoverystream.region-basic-layout": false,
           },
         },
       });
-      feed.locale = "de-DE";
       sandbox.stub(feed, "fetchLayout").returns(Promise.resolve(""));
 
       await feed.loadLayout(feed.store.dispatch);