Bug 1864775 - listen for updates to spoc endpoint allowlist. r=thecount,barret

* Add nimbus variables for updating these preferences via nimbus
  rollouts
* Also listen for updates to the spoc endpoint allowlist (and user clear
  endpoint)
* Add basic unit test exercising the change

Also tested e2e with synthetic experiment ala
https://experimenter.info/desktop-enroll-locally

Differential Revision: https://phabricator.services.mozilla.com/D193615
This commit is contained in:
Marion Lang 2023-11-23 00:53:04 +00:00
parent 916658f69e
commit d0e5546438
5 changed files with 65 additions and 34 deletions

View file

@ -1697,6 +1697,8 @@ class DiscoveryStreamFeed {
case PREF_HARDCODED_BASIC_LAYOUT:
case PREF_SPOCS_ENDPOINT:
case PREF_SPOCS_ENDPOINT_QUERY:
case PREF_SPOCS_CLEAR_ENDPOINT:
case PREF_ENDPOINTS:
// This is a config reset directly related to Discovery Stream pref.
this.configReset();
break;

View file

@ -7,19 +7,21 @@ test_newtab({
.returns(
"https://example.com/browser/browser/components/newtab/test/browser/topstories.json"
);
await pushPrefs([
"browser.newtabpage.activity-stream.discoverystream.config",
JSON.stringify({
api_key_pref: "extensions.pocket.oAuthConsumerKey",
collapsible: true,
enabled: true,
personalized: true,
}),
]);
await pushPrefs([
"browser.newtabpage.activity-stream.discoverystream.endpoints",
"https://example.com",
]);
await pushPrefs(
[
"browser.newtabpage.activity-stream.discoverystream.config",
JSON.stringify({
api_key_pref: "extensions.pocket.oAuthConsumerKey",
collapsible: true,
enabled: true,
personalized: true,
}),
],
[
"browser.newtabpage.activity-stream.discoverystream.endpoints",
"https://example.com",
]
);
},
test: async function test_card_render() {
await ContentTaskUtils.waitForCondition(

View file

@ -371,8 +371,8 @@ function test_newtab(testInfo, browserURL = "about:newtab") {
await after(contentResult);
} finally {
// Clean up for next tests
await scopedPopPrefs();
BrowserTestUtils.removeTab(tab);
await scopedPopPrefs();
}
};

View file

@ -12,8 +12,8 @@ import { reducers } from "common/Reducers.sys.mjs";
import { PersistentCache } from "lib/PersistentCache.sys.mjs";
const CONFIG_PREF_NAME = "discoverystream.config";
const DUMMY_ENDPOINT = "https://getpocket.cdn.mozilla.net/dummy";
const ENDPOINTS_PREF_NAME = "discoverystream.endpoints";
const DUMMY_ENDPOINT = "https://getpocket.cdn.mozilla.net/dummy";
const SPOC_IMPRESSION_TRACKING_PREF = "discoverystream.spoc.impressions";
const REC_IMPRESSION_TRACKING_PREF = "discoverystream.rec.impressions";
const THIRTY_MINUTES = 30 * 60 * 1000;
@ -153,7 +153,11 @@ describe("DiscoveryStreamFeed", () => {
});
it("should disallow unexpected endpoints", async () => {
feed.store.getState = () => ({
Prefs: { values: { [ENDPOINTS_PREF_NAME]: "https://other.site" } },
Prefs: {
values: {
[ENDPOINTS_PREF_NAME]: "https://other.site",
},
},
});
const response = await feed.fetchFromEndpoint(DUMMY_ENDPOINT);
@ -841,9 +845,9 @@ describe("DiscoveryStreamFeed", () => {
});
it("should properly transform spocs using placements", async () => {
sandbox.stub(feed.cache, "get").returns(Promise.resolve());
sandbox
.stub(feed, "fetchFromEndpoint")
.resolves({ spocs: { items: [{ id: "data" }] } });
sandbox.stub(feed, "fetchFromEndpoint").resolves({
spocs: { items: [{ id: "data" }] },
});
sandbox.stub(feed.cache, "set").returns(Promise.resolve());
await feed.loadSpocs(feed.store.dispatch);
@ -902,7 +906,8 @@ describe("DiscoveryStreamFeed", () => {
);
});
it("should return expected data if normalizeSpocsItems returns no spoc data", async () => {
// We don't need this for just this test, we are setting placements manually.
// We don't need this for just this test, we are setting placements
// manually.
feed.getPlacements.restore();
Object.defineProperty(feed, "showSponsoredStories", {
get: () => true,
@ -941,7 +946,8 @@ describe("DiscoveryStreamFeed", () => {
});
});
it("should use title and context on spoc data", async () => {
// We don't need this for just this test, we are setting placements manually.
// We don't need this for just this test, we are setting placements
// manually.
feed.getPlacements.restore();
Object.defineProperty(feed, "showSponsoredStories", {
get: () => true,
@ -986,7 +992,8 @@ describe("DiscoveryStreamFeed", () => {
global.NimbusFeatures.pocketNewtab.getVariable
.withArgs("topSitesContileSovEnabled")
.returns(true);
// We don't need this for just this test, we are setting placements manually.
// We don't need this for just this test, we are setting placements
// manually.
feed.getPlacements.restore();
Object.defineProperty(feed, "showSponsoredStories", {
get: () => true,
@ -1381,7 +1388,7 @@ describe("DiscoveryStreamFeed", () => {
it("should not fail with no endpoint", async () => {
sandbox.stub(feed.store, "getState").returns({
Prefs: {
values: { "discoverystream.endpointSpocsClear": null },
values: { PREF_SPOCS_CLEAR_ENDPOINT: null },
},
});
sandbox.stub(feed, "fetchFromEndpoint").resolves(null);
@ -1415,7 +1422,7 @@ describe("DiscoveryStreamFeed", () => {
});
it("should properly call clearSpocs when sponsored content is changed", async () => {
sandbox.stub(feed, "clearSpocs").returns(Promise.resolve());
//sandbox.stub(feed, "updatePlacements").returns();
// sandbox.stub(feed, "updatePlacements").returns();
sandbox.stub(feed, "loadSpocs").returns();
await feed.onAction({
@ -2554,6 +2561,17 @@ describe("DiscoveryStreamFeed", () => {
assert.calledOnce(feed.refreshAll);
});
it("shoud update allowlist", async () => {
assert.equal(
feed.store.getState().Prefs.values[ENDPOINTS_PREF_NAME],
DUMMY_ENDPOINT
);
setPref(ENDPOINTS_PREF_NAME, "sick-kickflip.mozilla.net");
assert.equal(
feed.store.getState().Prefs.values[ENDPOINTS_PREF_NAME],
"sick-kickflip.mozilla.net"
);
});
});
describe("#onAction: SYSTEM_TICK", () => {
@ -2909,9 +2927,9 @@ describe("DiscoveryStreamFeed", () => {
});
it("should not refresh layout on startup if it is under THIRTY_MINUTES", async () => {
feed.loadLayout.restore();
sandbox
.stub(feed.cache, "get")
.resolves({ layout: { lastUpdated: Date.now(), layout: {} } });
sandbox.stub(feed.cache, "get").resolves({
layout: { lastUpdated: Date.now(), layout: {} },
});
sandbox.stub(feed, "fetchFromEndpoint").resolves({ layout: {} });
await feed.refreshAll({ isStartup: true });
@ -2921,9 +2939,9 @@ describe("DiscoveryStreamFeed", () => {
it("should refresh spocs on startup if it was served from cache", async () => {
feed.loadSpocs.restore();
sandbox.stub(feed, "getPlacements").returns([{ name: "spocs" }]);
sandbox
.stub(feed.cache, "get")
.resolves({ spocs: { lastUpdated: Date.now() } });
sandbox.stub(feed.cache, "get").resolves({
spocs: { lastUpdated: Date.now() },
});
clock.tick(THIRTY_MINUTES + 1);
await feed.refreshAll({ isStartup: true });
@ -2937,9 +2955,9 @@ describe("DiscoveryStreamFeed", () => {
});
it("should not refresh spocs on startup if it is under THIRTY_MINUTES", async () => {
feed.loadSpocs.restore();
sandbox
.stub(feed.cache, "get")
.resolves({ spocs: { lastUpdated: Date.now() } });
sandbox.stub(feed.cache, "get").resolves({
spocs: { lastUpdated: Date.now() },
});
sandbox.stub(feed, "fetchFromEndpoint").resolves("data");
await feed.refreshAll({ isStartup: true });
@ -2984,7 +3002,8 @@ describe("DiscoveryStreamFeed", () => {
await feed.refreshAll({ isStartup: true });
assert.calledOnce(feed.fetchFromEndpoint);
// Once from cache, once to update the feed, once to update that all feeds are done.
// Once from cache, once to update the feed, once to update that all
// feeds are done.
assert.calledThrice(feed.store.dispatch);
assert.equal(
feed.store.dispatch.secondCall.args[0].type,

View file

@ -752,6 +752,14 @@ pocketNewtab:
description: The URL for the spocs endpoint.
type: string
setPref: "browser.newtabpage.activity-stream.discoverystream.spocs-endpoint"
spocsEndpointAllowlist:
description: Comma separated list of allowed endpoints for fetching spocs
type: string
setPref: "browser.newtabpage.activity-stream.discoverystream.endpoints"
spocsClearEndpoint:
description: URL for deleting any server data when a user opts out of sponsored content
type: string
setPref: "browser.newtabpage.activity-stream.discoverystream.endpointSpocsClear"
regionStoriesConfig:
description: A comma-separated list of region to get stories for.
type: string