Bug 1523408 - Ensure we don't fire impressions stat events in topsite… (#4747)

* Bug 1523408 - Ensure we don't fire impressions stat events in topsites via discovery stream

* Adding comment as to why we need to filter top stories impressions via source.
This commit is contained in:
ScottDowne 2019-02-04 12:38:23 -05:00 коммит произвёл GitHub
Родитель 83d427df3d
Коммит f4bf19ccec
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
2 изменённых файлов: 36 добавлений и 29 удалений

Просмотреть файл

@ -25,6 +25,7 @@ const STORIES_NOW_THRESHOLD = 24 * 60 * 60 * 1000; // 24 hours
const MIN_DOMAIN_AFFINITIES_UPDATE_TIME = 12 * 60 * 60 * 1000; // 12 hours
const DEFAULT_RECS_EXPIRE_TIME = 60 * 60 * 1000; // 1 hour
const SECTION_ID = "topstories";
const IMPRESSION_SOURCE = "TOP_STORIES";
const SPOC_IMPRESSION_TRACKING_PREF = "feeds.section.topstories.spoc.impressions";
const REC_IMPRESSION_TRACKING_PREF = "feeds.section.topstories.rec.impressions";
const OPTIONS_PREF = "feeds.section.topstories.options";
@ -656,21 +657,27 @@ this.TopStoriesFeed = class TopStoriesFeed {
}
break;
case at.TELEMETRY_IMPRESSION_STATS: {
const payload = action.data;
const viewImpression = !("click" in payload || "block" in payload || "pocket" in payload);
if (payload.tiles && viewImpression) {
if (this.shouldShowSpocs()) {
payload.tiles.forEach(t => {
if (this.spocCampaignMap.has(t.id)) {
this.recordCampaignImpression(this.spocCampaignMap.get(t.id));
}
});
}
if (this.personalized) {
const topRecs = payload.tiles
.filter(t => !this.spocCampaignMap.has(t.id))
.map(t => t.id);
this.recordTopRecImpressions(topRecs);
// We want to make sure we only track impressions from Top Stories,
// otherwise unexpected things that are not properly handled can happen.
// Example: Impressions from spocs on Discovery Stream can cause the
// Top Stories impressions pref to continuously grow, see bug #1523408
if (action.data.source === IMPRESSION_SOURCE) {
const payload = action.data;
const viewImpression = !("click" in payload || "block" in payload || "pocket" in payload);
if (payload.tiles && viewImpression) {
if (this.shouldShowSpocs()) {
payload.tiles.forEach(t => {
if (this.spocCampaignMap.has(t.id)) {
this.recordCampaignImpression(this.spocCampaignMap.get(t.id));
}
});
}
if (this.personalized) {
const topRecs = payload.tiles
.filter(t => !this.spocCampaignMap.has(t.id))
.map(t => t.id);
this.recordTopRecImpressions(topRecs);
}
}
}
break;

Просмотреть файл

@ -526,32 +526,32 @@ describe("Top Stories Feed", () => {
clock.tick(1);
let expectedPrefValue = JSON.stringify({1: 1, 2: 1, 3: 1});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {tiles: [{id: 1}, {id: 2}, {id: 3}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", tiles: [{id: 1}, {id: 2}, {id: 3}]}});
assert.calledWith(instance._prefs.set.firstCall, REC_IMPRESSION_TRACKING_PREF, expectedPrefValue);
// Only need to record first impression, so impression pref shouldn't change
instance._prefs.get = pref => expectedPrefValue;
clock.tick(1);
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {tiles: [{id: 1}, {id: 2}, {id: 3}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", tiles: [{id: 1}, {id: 2}, {id: 3}]}});
assert.calledOnce(instance._prefs.set);
// New first impressions should be added
clock.tick(1);
let expectedPrefValueTwo = JSON.stringify({1: 1, 2: 1, 3: 1, 4: 3, 5: 3, 6: 3});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {tiles: [{id: 4}, {id: 5}, {id: 6}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", tiles: [{id: 4}, {id: 5}, {id: 6}]}});
assert.calledWith(instance._prefs.set.secondCall, REC_IMPRESSION_TRACKING_PREF, expectedPrefValueTwo);
});
it("should not record top story impressions for non-view impressions", async () => {
instance._prefs = {get: pref => undefined, set: sinon.spy()};
instance.personalized = true;
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {click: 0, tiles: [{id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", click: 0, tiles: [{id: 1}]}});
assert.notCalled(instance._prefs.set);
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {block: 0, tiles: [{id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", block: 0, tiles: [{id: 1}]}});
assert.notCalled(instance._prefs.set);
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {pocket: 0, tiles: [{id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", pocket: 0, tiles: [{id: 1}]}});
assert.notCalled(instance._prefs.set);
});
it("should clean up top story impressions", async () => {
@ -973,18 +973,18 @@ describe("Top Stories Feed", () => {
await instance.fetchStories();
let expectedPrefValue = JSON.stringify({5: [0]});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {tiles: [{id: 3}, {id: 2}, {id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", tiles: [{id: 3}, {id: 2}, {id: 1}]}});
assert.calledWith(instance._prefs.set.firstCall, SPOC_IMPRESSION_TRACKING_PREF, expectedPrefValue);
clock.tick(1);
instance._prefs.get = pref => expectedPrefValue;
let expectedPrefValueCallTwo = JSON.stringify({5: [0, 1]});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {tiles: [{id: 3}, {id: 2}, {id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", tiles: [{id: 3}, {id: 2}, {id: 1}]}});
assert.calledWith(instance._prefs.set.secondCall, SPOC_IMPRESSION_TRACKING_PREF, expectedPrefValueCallTwo);
clock.tick(1);
instance._prefs.get = pref => expectedPrefValueCallTwo;
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {tiles: [{id: 3}, {id: 2}, {id: 4}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", tiles: [{id: 3}, {id: 2}, {id: 4}]}});
assert.calledWith(instance._prefs.set.thirdCall, SPOC_IMPRESSION_TRACKING_PREF, JSON.stringify({5: [0, 1], 6: [2]}));
});
it("should not record spoc/campaign impressions for non-view impressions", async () => {
@ -1007,13 +1007,13 @@ describe("Top Stories Feed", () => {
fetchStub.resolves({ok: true, status: 200, json: () => Promise.resolve(response)});
await instance.onInit();
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {click: 0, tiles: [{id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", click: 0, tiles: [{id: 1}]}});
assert.notCalled(instance._prefs.set);
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {block: 0, tiles: [{id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", block: 0, tiles: [{id: 1}]}});
assert.notCalled(instance._prefs.set);
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {pocket: 0, tiles: [{id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", pocket: 0, tiles: [{id: 1}]}});
assert.notCalled(instance._prefs.set);
});
it("should clean up spoc/campaign impressions", async () => {
@ -1033,9 +1033,9 @@ describe("Top Stories Feed", () => {
await instance.fetchStories();
// simulate impressions for campaign 5 and 6
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {tiles: [{id: 3}, {id: 2}, {id: 1}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", tiles: [{id: 3}, {id: 2}, {id: 1}]}});
instance._prefs.get = pref => (pref === SPOC_IMPRESSION_TRACKING_PREF) && JSON.stringify({5: [0]});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {tiles: [{id: 3}, {id: 2}, {id: 4}]}});
instance.onAction({type: at.TELEMETRY_IMPRESSION_STATS, data: {source: "TOP_STORIES", tiles: [{id: 3}, {id: 2}, {id: 4}]}});
let expectedPrefValue = JSON.stringify({5: [0], 6: [0]});
assert.calledWith(instance._prefs.set.secondCall, SPOC_IMPRESSION_TRACKING_PREF, expectedPrefValue);