Bug 1650104 - Enable reach event for Moments Page experiments r=andreio

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

browser/components/newtab/lib/ASRouter.jsm

@@ -111,7 +111,7 @@ const USE_REMOTE_L10N_PREF =
 // Experiment groups that need to report the reach event in Messaging-Experiments.
 // If you're adding new groups to it, make sure they're also added in the
 // `messaging_experiments.reach.objects` defined in "toolkit/components/telemetry/Events.yaml"
-const REACH_EVENT_GROUPS = ["cfr"];
+const REACH_EVENT_GROUPS = ["cfr", "moments-page"];
 const REACH_EVENT_CATEGORY = "messaging_experiments";
 const REACH_EVENT_METHOD = "reach";
 
@@ -383,7 +383,7 @@ const MessageLoaderUtils = {
           ) {
             experiments.push({
               group,
-              forReachEvent: true,
+              forReachEvent: { sent: false },
               experimentSlug: experimentData.slug,
               branchSlug: branch.slug,
               ...branch.value,
@@ -2045,7 +2045,10 @@ class _ASRouter {
     const nonReachMessages = [];
     for (const message of messages) {
       if (message.forReachEvent) {
+        if (!message.forReachEvent.sent) {
           this._recordReachEvent(message);
+          message.forReachEvent.sent = true;
+        }
       } else {
         nonReachMessages.push(message);
       }

browser/components/newtab/lib/MomentsPageHub.jsm

@@ -17,6 +17,13 @@ XPCOMUtils.defineLazyModuleGetters(this, {
 const SYSTEM_TICK_INTERVAL = 5 * 60 * 1000;
 const HOMEPAGE_OVERRIDE_PREF = "browser.startup.homepage_override.once";
 
+// For the "reach" event of Messaging Experiments
+const REACH_EVENT_CATEGORY = "messaging_experiments";
+const REACH_EVENT_METHOD = "reach";
+// Note it's not "moments-page" as Telemetry Events only accepts understores
+// for the event `object`
+const REACH_EVENT_OBJECT = "moments_page";
+
 class _MomentsPageHub {
   constructor() {
     this.id = "moments-page-hub";
@@ -97,16 +104,42 @@ class _MomentsPageHub {
     }
   }
 
+  _recordReachEvent(message) {
+    const extra = { branches: message.branchSlug };
+    Services.telemetry.recordEvent(
+      REACH_EVENT_CATEGORY,
+      REACH_EVENT_METHOD,
+      REACH_EVENT_OBJECT,
+      message.experimentSlug,
+      extra
+    );
+  }
+
   async messageRequest({ triggerId, template }) {
     const telemetryObject = { triggerId };
     TelemetryStopwatch.start("MS_MESSAGE_REQUEST_TIME_MS", telemetryObject);
-    const message = await this._handleMessageRequest({
+    const messages = await this._handleMessageRequest({
       triggerId,
       template,
+      returnAll: true,
     });
     TelemetryStopwatch.finish("MS_MESSAGE_REQUEST_TIME_MS", telemetryObject);
-    if (message) {
+
-      this.executeAction(message);
+    // Record the "reach" event for all the messages with `forReachEvent`,
+    // only execute action for the first message without forReachEvent.
+    const nonReachMessages = [];
+    for (const message of messages) {
+      if (message.forReachEvent) {
+        if (!message.forReachEvent.sent) {
+          this._recordReachEvent(message);
+          message.forReachEvent.sent = true;
+        }
+      } else {
+        nonReachMessages.push(message);
+      }
+    }
+    if (nonReachMessages.length) {
+      this.executeAction(nonReachMessages[0]);
     }
   }
 

browser/components/newtab/test/unit/asrouter/ASRouter.test.js

@@ -2333,7 +2333,7 @@ describe("ASRouter", () => {
         let messages = [
           {
             id: "foo1",
-            forReachEvent: true,
+            forReachEvent: { sent: false },
             experimentSlug: "exp01",
             branchSlug: "branch01",
             group: "cfr",
@@ -2351,7 +2351,7 @@ describe("ASRouter", () => {
           },
           {
             id: "foo3",
-            forReachEvent: true,
+            forReachEvent: { sent: false },
             experimentSlug: "exp02",
             branchSlug: "branch02",
             group: "cfr",
@@ -2371,6 +2371,30 @@ describe("ASRouter", () => {
         await Router.onMessage(msg);
         assert.calledTwice(Services.telemetry.recordEvent);
       });
+      it("should not record the Reach event if it's already sent", async () => {
+        let messages = [
+          {
+            id: "foo1",
+            forReachEvent: { sent: true },
+            experimentSlug: "exp01",
+            branchSlug: "branch01",
+            group: "cfr",
+            template: "simple_template",
+            trigger: { id: "foo" },
+            content: { title: "Foo1", body: "Foo123-1" },
+          },
+        ];
+        sandbox.stub(Router, "handleMessageRequest").resolves(messages);
+        sandbox.spy(Services.telemetry, "recordEvent");
+
+        const msg = fakeAsyncMessage({
+          type: "TRIGGER",
+          data: { trigger: { id: "foo" } },
+        });
+
+        await Router.onMessage(msg);
+        assert.notCalled(Services.telemetry.recordEvent);
+      });
     });
 
     describe(".includeBundle", () => {
@@ -4109,7 +4133,7 @@ describe("ASRouter", () => {
       assert.equal(result.messages[1].group, "cfr");
       assert.equal(result.messages[1].experimentSlug, "exp01");
       assert.equal(result.messages[1].branchSlug, "branch02");
-      assert.ok(result.messages[1].forReachEvent);
+      assert.deepEqual(result.messages[1].forReachEvent, { sent: false });
     });
     it("should fetch json from url", async () => {
       let result = await MessageLoaderUtils.loadMessagesForProvider({

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

@@ -20,10 +20,10 @@ describe("MomentsPageHub", () => {
     globals = new GlobalOverrider();
     sandbox = sinon.createSandbox();
     instance = new _MomentsPageHub();
-    const [msg] = (await PanelTestProvider.getMessages()).filter(
+    const messages = (await PanelTestProvider.getMessages()).filter(
       ({ template }) => template === "update_action"
     );
-    handleMessageRequestStub = sandbox.stub().resolves(msg);
+    handleMessageRequestStub = sandbox.stub().resolves(messages);
     addImpressionStub = sandbox.stub();
     blockMessageByIdStub = sandbox.stub();
     getStringPrefStub = sandbox.stub();
@@ -39,6 +39,9 @@ describe("MomentsPageHub", () => {
           getStringPref: getStringPrefStub,
           setStringPref: setStringPrefStub,
         },
+        telemetry: {
+          recordEvent: () => {},
+        },
       },
     });
   });
@@ -101,12 +104,13 @@ describe("MomentsPageHub", () => {
       assert.calledWithExactly(handleMessageRequestStub, {
         triggerId: "trigger",
         template: "template",
+        returnAll: true,
       });
     });
     it("shouldn't do anything if no message is provided", async () => {
       // Reset the call from `instance.init`
       setStringPrefStub.reset();
-      handleMessageRequestStub.resolves(null);
+      handleMessageRequestStub.resolves([]);
       await instance.messageRequest({ triggerId: "trigger" });
 
       assert.notCalled(setStringPrefStub);
@@ -136,6 +140,59 @@ describe("MomentsPageHub", () => {
         { triggerId: "trigger" }
       );
     });
+    it("should record Reach event for the Moments page experiment", async () => {
+      const momentsMessages = (await PanelTestProvider.getMessages()).filter(
+        ({ template }) => template === "update_action"
+      );
+      const messages = [
+        {
+          forReachEvent: { sent: false },
+          experimentSlug: "foo",
+          branchSlug: "bar",
+        },
+        ...momentsMessages,
+      ];
+      handleMessageRequestStub.resolves(messages);
+      sandbox.spy(global.Services.telemetry, "recordEvent");
+      sandbox.spy(instance, "executeAction");
+
+      await instance.messageRequest({ triggerId: "trigger" });
+
+      assert.calledOnce(global.Services.telemetry.recordEvent);
+      assert.calledOnce(instance.executeAction);
+    });
+    it("should not record the Reach event if it's already sent", async () => {
+      const messages = [
+        {
+          forReachEvent: { sent: true },
+          experimentSlug: "foo",
+          branchSlug: "bar",
+        },
+      ];
+      handleMessageRequestStub.resolves(messages);
+      sandbox.spy(global.Services.telemetry, "recordEvent");
+
+      await instance.messageRequest({ triggerId: "trigger" });
+
+      assert.notCalled(global.Services.telemetry.recordEvent);
+    });
+    it("should not trigger the action if it's only for the Reach event", async () => {
+      const messages = [
+        {
+          forReachEvent: { sent: false },
+          experimentSlug: "foo",
+          branchSlug: "bar",
+        },
+      ];
+      handleMessageRequestStub.resolves(messages);
+      sandbox.spy(global.Services.telemetry, "recordEvent");
+      sandbox.spy(instance, "executeAction");
+
+      await instance.messageRequest({ triggerId: "trigger" });
+
+      assert.calledOnce(global.Services.telemetry.recordEvent);
+      assert.notCalled(instance.executeAction);
+    });
   });
   describe("executeAction", () => {
     beforeEach(async () => {
@@ -147,7 +204,7 @@ describe("MomentsPageHub", () => {
       });
     });
     it("should set HOMEPAGE_OVERRIDE_PREF on `moments-wnp` action", async () => {
-      const msg = await handleMessageRequestStub();
+      const [msg] = await handleMessageRequestStub();
       sandbox.useFakeTimers();
       instance.executeAction(msg);
 
@@ -165,7 +222,7 @@ describe("MomentsPageHub", () => {
       );
     });
     it("should block after taking the action", async () => {
-      const msg = await handleMessageRequestStub();
+      const [msg] = await handleMessageRequestStub();
       instance.executeAction(msg);
 
       assert.calledOnce(blockMessageByIdStub);
@@ -174,7 +231,7 @@ describe("MomentsPageHub", () => {
     it("should compute expire based on expireDelta", async () => {
       sandbox.spy(instance, "getExpirationDate");
 
-      const msg = await handleMessageRequestStub();
+      const [msg] = await handleMessageRequestStub();
       instance.executeAction(msg);
 
       assert.calledOnce(instance.getExpirationDate);
@@ -186,7 +243,7 @@ describe("MomentsPageHub", () => {
     it("should compute expire based on expireDelta", async () => {
       sandbox.spy(instance, "getExpirationDate");
 
-      const msg = await handleMessageRequestStub();
+      const [msg] = await handleMessageRequestStub();
       const msgWithExpire = {
         ...msg,
         content: {
@@ -212,7 +269,7 @@ describe("MomentsPageHub", () => {
       );
     });
     it("should send user telemetry", async () => {
-      const msg = await handleMessageRequestStub();
+      const [msg] = await handleMessageRequestStub();
       const sendUserEventTelemetrySpy = sandbox.spy(
         instance,
         "sendUserEventTelemetry"