Bug 1561540 - CFR messages should have a priority field that allows sorting (#5156)

content-src/asrouter/docs/targeting-attributes.md

@@ -35,6 +35,7 @@ Please note that some targeting attributes require stricter controls on the tele
 * [xpinstallEnabled](#xpinstallEnabled)
 * [hasPinnedTabs](#haspinnedtabs)
 * [hasAccessedFxAPanel](#hasaccessedfxapanel)
+* [isWhatsNewPanelEnabled](#iswhatsnewpanelenabled)
 
 ## Detailed usage
 
@@ -485,3 +486,13 @@ Boolean pref that gets set the first time the user opens the FxA toolbar panel
 ```ts
 declare const hasAccessedFxAPanel: boolean;
 ```
+
+### `isWhatsNewPanelEnabled`
+
+Boolean pref that controls if the What's New panel feature is enabled
+
+#### Definition
+
+```ts
+declare const isWhatsNewPanelEnabled: boolean;
+```

content-src/asrouter/templates/OnboardingMessage/ToolbarBadgeMessage.schema.json

@@ -6,6 +6,16 @@
   "properties": {
     "target": {
       "type": "string"
+    },
+    "action": {
+      "type": "object",
+      "properties": {
+        "id": {
+          "type": "string"
+        }
+      },
+      "additionalProperties": false,
+      "required": ["id"]
     }
   },
   "additionalProperties": false,

lib/ASRouter.jsm

@@ -730,6 +730,11 @@ class _ASRouter {
       blockMessageById: this.blockMessageById,
     });
     ToolbarPanelHub.init();
+    ToolbarBadgeHub.init(this.waitForInitialized, {
+      handleMessageRequest: this.handleMessageRequest,
+      addImpression: this.addImpression,
+      blockMessageById: this.blockMessageById,
+    });
 
     this._loadLocalProviders();
 
@@ -746,12 +751,16 @@ class _ASRouter {
       (await this._storage.get("providerImpressions")) || {};
     const previousSessionEnd =
       (await this._storage.get("previousSessionEnd")) || 0;
+    // Infinity so that we default to false (firefoxVersion > previousSessionFirefoxVersion)
+    const previousSessionFirefoxVersion =
+      (await this._storage.get("previousSessionFirefoxVersion")) || Infinity;
     await this.setState({
       messageBlockList,
       providerBlockList,
       messageImpressions,
       providerImpressions,
       previousSessionEnd,
+      previousSessionFirefoxVersion,
     });
     this._updateMessageProviders();
     await this.loadMessagesFromAllProviders();
@@ -771,6 +780,10 @@ class _ASRouter {
 
   uninit() {
     this._storage.set("previousSessionEnd", Date.now());
+    this._storage.set(
+      "previousSessionFirefoxVersion",
+      ASRouterTargeting.Environment.firefoxVersion
+    );
 
     this.messageChannel.sendAsyncMessage(OUTGOING_MESSAGE_NAME, {
       type: "CLEAR_ALL",
@@ -1031,15 +1044,26 @@ class _ASRouter {
   // Return an object containing targeting parameters used to select messages
   _getMessagesContext() {
     const {
+      messageImpressions,
       previousSessionEnd,
+      previousSessionFirefoxVersion,
       trailheadInterrupt,
       trailheadTriplet,
     } = this.state;
 
     return {
+      get messageImpressions() {
+        return messageImpressions;
+      },
       get previousSessionEnd() {
         return previousSessionEnd;
       },
+      get previousSessionFirefoxVersion() {
+        // Any comparison with `undefined` will return false
+        return isNaN(previousSessionFirefoxVersion)
+          ? undefined
+          : previousSessionFirefoxVersion;
+      },
       get trailheadInterrupt() {
         return trailheadInterrupt;
       },
@@ -1264,7 +1288,7 @@ class _ASRouter {
         }
         break;
       case "toolbar_badge":
-        ToolbarBadgeHub.registerBadgeNotificationListener(message);
+        ToolbarBadgeHub.registerBadgeNotificationListener(message, { force });
         break;
       default:
         target.sendAsyncMessage(OUTGOING_MESSAGE_NAME, {

lib/ASRouterTargeting.jsm

@@ -206,6 +206,35 @@ function sortMessagesByTargeting(messages) {
   });
 }
 
+/**
+ * Sort messages in descending order based on the value of `priority`
+ * Messages with no `priority` are ranked lowest (even after a message with
+ * priority 0).
+ */
+function sortMessagesByPriority(messages) {
+  return messages.sort((a, b) => {
+    if (isNaN(a.priority) && isNaN(b.priority)) {
+      return 0;
+    }
+    if (!isNaN(a.priority) && isNaN(b.priority)) {
+      return -1;
+    }
+    if (isNaN(a.priority) && !isNaN(b.priority)) {
+      return 1;
+    }
+
+    // Descending order; higher priority comes first
+    if (a.priority > b.priority) {
+      return -1;
+    }
+    if (a.priority < b.priority) {
+      return 1;
+    }
+
+    return 0;
+  });
+}
+
 const TargetingGetters = {
   get locale() {
     return Services.locale.appLocaleAsLangTag;
@@ -365,6 +394,12 @@ const TargetingGetters = {
       true
     );
   },
+  get isWhatsNewPanelEnabled() {
+    return Services.prefs.getBoolPref(
+      "browser.messaging-system.whatsNewPanel.enabled",
+      false
+    );
+  },
 };
 
 this.ASRouterTargeting = {
@@ -471,7 +506,8 @@ this.ASRouterTargeting = {
    */
   async findMatchingMessage({ messages, trigger, context, onError }) {
     const weightSortedMessages = sortMessagesByWeightedRank([...messages]);
-    const sortedMessages = sortMessagesByTargeting(weightSortedMessages);
+    let sortedMessages = sortMessagesByTargeting(weightSortedMessages);
+    sortedMessages = sortMessagesByPriority(sortedMessages);
     const triggerContext = trigger ? trigger.context : {};
     const combinedContext = this.combineContexts(context, triggerContext);
 

lib/OnboardingMessageProvider.jsm

@@ -14,6 +14,7 @@ const { AddonRepository } = ChromeUtils.import(
 );
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
+const FIREFOX_VERSION = parseInt(Services.appinfo.version.match(/\d+/), 10);
 const L10N = new Localization([
   "branding/brand.ftl",
   "browser/branding/brandings.ftl",
@@ -475,6 +476,27 @@ const ONBOARDING_MESSAGES = async () => [
     targeting: `!hasAccessedFxAPanel && !usesFirefoxSync && isFxAEnabled == true`,
     trigger: { id: "toolbarBadgeUpdate" },
   },
+  {
+    id: `WHATS_NEW_BADGE_${FIREFOX_VERSION}`,
+    template: "toolbar_badge",
+    content: {
+      target: "whats-new-menu-button",
+      action: { id: "show-whatsnew-button" },
+    },
+    priority: 1,
+    trigger: { id: "toolbarBadgeUpdate" },
+    frequency: {
+      // Makes it so that we track impressions for this message while at the
+      // same time it can have unlimited impressions
+      lifetime: Infinity,
+    },
+    // Never saw this message or saw it in the past 4 days or more recent
+    targeting: `isWhatsNewPanelEnabled &&
+      (firefoxVersion > previousSessionFirefoxVersion &&
+        messageImpressions[.id == 'WHATS_NEW_BADGE_${FIREFOX_VERSION}']|length == 0) ||
+      (messageImpressions[.id == 'WHATS_NEW_BADGE_${FIREFOX_VERSION}']|length >= 1 &&
+        currentDate|date - messageImpressions[.id == 'WHATS_NEW_BADGE_${FIREFOX_VERSION}'][0] <= 4 * 24 * 3600 * 1000)`,
+  },
 ];
 
 const OnboardingMessageProvider = {

lib/ToolbarBadgeHub.jsm

@@ -8,6 +8,11 @@ ChromeUtils.defineModuleGetter(
   "EveryWindow",
   "resource:///modules/EveryWindow.jsm"
 );
+ChromeUtils.defineModuleGetter(
+  this,
+  "ToolbarPanelHub",
+  "resource://activity-stream/lib/ToolbarPanelHub.jsm"
+);
 
 const notificationsByWindow = new WeakMap();
 
@@ -37,6 +42,14 @@ class _ToolbarBadgeHub {
     this.messageRequest("toolbarBadgeUpdate");
   }
 
+  executeAction({ id }) {
+    switch (id) {
+      case "show-whatsnew-button":
+        ToolbarPanelHub.enableToolbarButton();
+        break;
+    }
+  }
+
   removeAllNotifications() {
     // Will call uninit on every window
     EveryWindow.unregisterCallback(this.id);
@@ -53,6 +66,9 @@ class _ToolbarBadgeHub {
 
   addToolbarNotification(win, message) {
     const document = win.browser.ownerDocument;
+    if (message.content.action) {
+      this.executeAction(message.content.action);
+    }
     let toolbarbutton = document.getElementById(message.content.target);
     if (toolbarbutton) {
       toolbarbutton.setAttribute("badged", true);
@@ -71,9 +87,15 @@ class _ToolbarBadgeHub {
     return null;
   }
 
-  registerBadgeNotificationListener(message) {
+  registerBadgeNotificationListener(message, options = {}) {
     this._addImpression(message);
 
+    // We need to clear any existing notifications and only show
+    // the one set by devtools
+    if (options.force) {
+      EveryWindow.unregisterCallback(this.id);
+    }
+
     EveryWindow.registerCallback(
       this.id,
       win => {

lib/ToolbarPanelHub.jsm

@@ -33,9 +33,6 @@ class _ToolbarPanelHub {
   init() {
     if (this.whatsNewPanelEnabled) {
       this.enableAppmenuButton();
-      // TODO: this will eventually be called when the badge message gets triggered
-      // instead of here in init.
-      this.enableToolbarButton();
     }
   }
 

test/browser/browser_asrouter_targeting.js

@@ -823,6 +823,22 @@ add_task(async function check_hasAccessedFxAPanel() {
   );
 });
 
+add_task(async function check_isWhatsNewPanelEnabled() {
+  is(
+    await ASRouterTargeting.Environment.isWhatsNewPanelEnabled,
+    false,
+    "Not enabled yet"
+  );
+
+  await pushPrefs(["browser.messaging-system.whatsNewPanel.enabled", true]);
+
+  is(
+    await ASRouterTargeting.Environment.isWhatsNewPanelEnabled,
+    true,
+    "Should update based on pref"
+  );
+});
+
 add_task(async function checkCFRPinnedTabsTargetting() {
   const now = Date.now();
   const timeMinutesAgo = numMinutes => now - numMinutes * 60 * 1000;

test/unit/asrouter/ASRouter.test.js

@@ -60,6 +60,7 @@ describe("ASRouter", () => {
   let messageImpressions;
   let providerImpressions;
   let previousSessionEnd;
+  let previousSessionFirefoxVersion;
   let fetchStub;
   let clock;
   let getStringPrefStub;
@@ -87,6 +88,9 @@ describe("ASRouter", () => {
     getStub
       .withArgs("previousSessionEnd")
       .returns(Promise.resolve(previousSessionEnd));
+    getStub
+      .withArgs("previousSessionFirefoxVersion")
+      .returns(Promise.resolve(previousSessionFirefoxVersion));
     return {
       get: getStub,
       set: sandbox.stub().returns(Promise.resolve()),
@@ -113,6 +117,7 @@ describe("ASRouter", () => {
     messageImpressions = {};
     providerImpressions = {};
     previousSessionEnd = 100;
+    previousSessionFirefoxVersion = 69;
     sandbox = sinon.createSandbox();
 
     sandbox.spy(ASRouterPreferences, "init");
@@ -141,14 +146,14 @@ describe("ASRouter", () => {
       uninit: sandbox.stub(),
       _forceShowMessage: sandbox.stub(),
     };
-    FakeToolbarBadgeHub = {
-      init: sandbox.stub(),
-      registerBadgeNotificationListener: sandbox.stub(),
-    };
     FakeToolbarPanelHub = {
       init: sandbox.stub(),
       uninit: sandbox.stub(),
     };
+    FakeToolbarBadgeHub = {
+      init: sandbox.stub(),
+      registerBadgeNotificationListener: sandbox.stub(),
+    };
     globals.set({
       AttributionCode: fakeAttributionCode,
       // Testing framework doesn't know how to `defineLazyModuleGetter` so we're
@@ -814,6 +819,44 @@ describe("ASRouter", () => {
 
       assert.deepEqual(result, message1);
     });
+    it("should get unblocked messages that match trigger and template", async () => {
+      const message1 = {
+        id: "1",
+        campaign: "foocampaign",
+        template: "badge",
+        trigger: { id: "foo" },
+      };
+      const message2 = {
+        id: "2",
+        campaign: "foocampaign",
+        template: "snippet",
+        trigger: { id: "foo" },
+      };
+      await Router.setState({ messages: [message2, message1] });
+      // Just return the first message provided as arg
+      sandbox.stub(Router, "_findMessage").callsFake(messages => messages[0]);
+
+      const result = Router.handleMessageRequest({
+        triggerId: "foo",
+        template: "badge",
+      });
+
+      assert.deepEqual(result, message1);
+    });
+    it("should have previousSessionFirefoxVersion in the message context", () => {
+      assert.propertyVal(
+        Router._getMessagesContext(),
+        "previousSessionFirefoxVersion",
+        parseInt(AppConstants.MOZ_APP_VERSION, 10)
+      );
+    });
+    it("should have messageImpressions in the message context", () => {
+      assert.propertyVal(
+        Router._getMessagesContext(),
+        "messageImpressions",
+        Router.state.messageImpressions
+      );
+    });
   });
 
   describe("blocking", () => {
@@ -897,12 +940,17 @@ describe("ASRouter", () => {
     it("should save previousSessionEnd", () => {
       Router.uninit();
 
-      assert.calledOnce(Router._storage.set);
+      assert.calledTwice(Router._storage.set);
       assert.calledWithExactly(
         Router._storage.set,
         "previousSessionEnd",
         sinon.match.number
       );
+      assert.calledWithExactly(
+        Router._storage.set,
+        "previousSessionFirefoxVersion",
+        sinon.match.number
+      );
     });
   });
 

test/unit/asrouter/ASRouterFeed.test.js

@@ -31,7 +31,13 @@ describe("ASRouterFeed", () => {
     globals.set("BookmarkPanelHub", FakeBookmarkPanelHub);
     globals.set("ToolbarBadgeHub", FakeToolbarBadgeHub);
     globals.set("ToolbarPanelHub", FakeToolbarPanelHub);
+
     Router = new _ASRouter({ providers: [FAKE_LOCAL_PROVIDER] });
+    FakeToolbarPanelHub = {
+      init: sandbox.stub(),
+      uninit: sandbox.stub(),
+    };
+
     storage = {
       get: sandbox.stub().returns(Promise.resolve([])),
       set: sandbox.stub().returns(Promise.resolve()),

test/unit/asrouter/ASRouterTargeting.test.js

@@ -102,6 +102,101 @@ describe("#CachedTargetingGetter", () => {
     assert.isDefined(result);
     assert.equal(result.id, "FXA_1");
   });
+  describe("sortMessagesByPriority", () => {
+    it("should sort messages in descending priority order", async () => {
+      const [
+        m1,
+        m2,
+        m3,
+      ] = await OnboardingMessageProvider.getUntranslatedMessages();
+      const checkMessageTargetingStub = sandbox
+        .stub(ASRouterTargeting, "checkMessageTargeting")
+        .resolves(false);
+      sandbox.stub(ASRouterTargeting, "isTriggerMatch").resolves(true);
+
+      await ASRouterTargeting.findMatchingMessage({
+        messages: [
+          { ...m1, priority: 0 },
+          { ...m2, priority: 1 },
+          { ...m3, priority: 2 },
+        ],
+        trigger: "testing",
+      });
+
+      assert.equal(checkMessageTargetingStub.callCount, 3);
+
+      const [arg_m1] = checkMessageTargetingStub.firstCall.args;
+      assert.equal(arg_m1.id, m3.id);
+
+      const [arg_m2] = checkMessageTargetingStub.secondCall.args;
+      assert.equal(arg_m2.id, m2.id);
+
+      const [arg_m3] = checkMessageTargetingStub.thirdCall.args;
+      assert.equal(arg_m3.id, m1.id);
+    });
+    it("should sort messages with no priority last", async () => {
+      const [
+        m1,
+        m2,
+        m3,
+      ] = await OnboardingMessageProvider.getUntranslatedMessages();
+      const checkMessageTargetingStub = sandbox
+        .stub(ASRouterTargeting, "checkMessageTargeting")
+        .resolves(false);
+      sandbox.stub(ASRouterTargeting, "isTriggerMatch").resolves(true);
+
+      await ASRouterTargeting.findMatchingMessage({
+        messages: [
+          { ...m1, priority: 0 },
+          { ...m2, priority: undefined },
+          { ...m3, priority: 2 },
+        ],
+        trigger: "testing",
+      });
+
+      assert.equal(checkMessageTargetingStub.callCount, 3);
+
+      const [arg_m1] = checkMessageTargetingStub.firstCall.args;
+      assert.equal(arg_m1.id, m3.id);
+
+      const [arg_m2] = checkMessageTargetingStub.secondCall.args;
+      assert.equal(arg_m2.id, m1.id);
+
+      const [arg_m3] = checkMessageTargetingStub.thirdCall.args;
+      assert.equal(arg_m3.id, m2.id);
+    });
+    it("should keep the order of messages with same priority unchanged", async () => {
+      const [
+        m1,
+        m2,
+        m3,
+      ] = await OnboardingMessageProvider.getUntranslatedMessages();
+      const checkMessageTargetingStub = sandbox
+        .stub(ASRouterTargeting, "checkMessageTargeting")
+        .resolves(false);
+      sandbox.stub(ASRouterTargeting, "isTriggerMatch").resolves(true);
+
+      await ASRouterTargeting.findMatchingMessage({
+        messages: [
+          { ...m1, priority: 2, targeting: undefined, rank: 1 },
+          { ...m2, priority: undefined, targeting: undefined, rank: 1 },
+          { ...m3, priority: 2, targeting: undefined, rank: 1 },
+        ],
+        trigger: "testing",
+      });
+
+      assert.equal(checkMessageTargetingStub.callCount, 3);
+
+      const [arg_m1] = checkMessageTargetingStub.firstCall.args;
+      assert.equal(arg_m1.id, m1.id);
+
+      const [arg_m2] = checkMessageTargetingStub.secondCall.args;
+      assert.equal(arg_m2.id, m3.id);
+
+      const [arg_m3] = checkMessageTargetingStub.thirdCall.args;
+      assert.equal(arg_m3.id, m2.id);
+    });
+  });
   describe("combineContexts", () => {
     it("should combine the properties of the two objects", () => {
       const joined = ASRouterTargeting.combineContexts(

test/unit/lib/ToolbarBadgeHub.test.js

@@ -1,12 +1,14 @@
 import { _ToolbarBadgeHub } from "lib/ToolbarBadgeHub.jsm";
 import { GlobalOverrider } from "test/unit/utils";
 import { OnboardingMessageProvider } from "lib/OnboardingMessageProvider.jsm";
+import { _ToolbarPanelHub } from "lib/ToolbarPanelHub.jsm";
 
 describe("BookmarkPanelHub", () => {
   let sandbox;
   let instance;
   let fakeAddImpression;
   let fxaMessage;
+  let whatsnewMessage;
   let fakeElement;
   let globals;
   let everyWindowStub;
@@ -15,15 +17,9 @@ describe("BookmarkPanelHub", () => {
     sandbox = sinon.createSandbox();
     instance = new _ToolbarBadgeHub();
     fakeAddImpression = sandbox.stub();
-    [
-      ,
-      ,
-      ,
-      ,
-      ,
-      ,
-      fxaMessage,
-    ] = await OnboardingMessageProvider.getUntranslatedMessages();
+    const msgs = await OnboardingMessageProvider.getUntranslatedMessages();
+    fxaMessage = msgs.find(({ id }) => id === "FXA_ACCOUNTS_BADGE");
+    whatsnewMessage = msgs.find(({ id }) => id.includes("WHATS_NEW_BADGE_"));
     fakeElement = {
       setAttribute: sandbox.stub(),
       removeAttribute: sandbox.stub(),
@@ -128,6 +124,16 @@ describe("BookmarkPanelHub", () => {
         { once: true }
       );
     });
+    it("should execute actions if they exist", () => {
+      sandbox.stub(instance, "executeAction");
+      instance.addToolbarNotification(target, whatsnewMessage);
+
+      assert.calledOnce(instance.executeAction);
+      assert.calledWithExactly(
+        instance.executeAction,
+        whatsnewMessage.content.action
+      );
+    });
   });
   describe("registerBadgeNotificationListener", () => {
     beforeEach(() => {
@@ -174,6 +180,24 @@ describe("BookmarkPanelHub", () => {
       assert.calledOnce(instance.removeToolbarNotification);
       assert.calledWithExactly(instance.removeToolbarNotification, fakeElement);
     });
+    it("should unregister notifications when forcing a badge via devtools", () => {
+      instance.registerBadgeNotificationListener(fxaMessage, { force: true });
+
+      assert.calledOnce(everyWindowStub.unregisterCallback);
+      assert.calledWithExactly(everyWindowStub.unregisterCallback, instance.id);
+    });
+  });
+  describe("executeAction", () => {
+    it("should call ToolbarPanelHub.enableToolbarButton", () => {
+      const stub = sandbox.stub(
+        _ToolbarPanelHub.prototype,
+        "enableToolbarButton"
+      );
+
+      instance.executeAction({ id: "show-whatsnew-button" });
+
+      assert.calledOnce(stub);
+    });
   });
   describe("removeToolbarNotification", () => {
     it("should remove the notification", () => {

test/unit/ping-centre/PingCentre.test.js

@@ -340,6 +340,7 @@ describe("PingCentre", () => {
           locale: FAKE_LOCALE,
           topic: "activity-stream",
           client_id: FAKE_TELEMETRY_ID,
+          version: "69.0a1",
           release_channel: FAKE_UPDATE_CHANNEL,
         },
         fakePingJSON
@@ -363,6 +364,7 @@ describe("PingCentre", () => {
         {
           locale: FAKE_LOCALE,
           client_id: FAKE_TELEMETRY_ID,
+          version: "69.0a1",
           release_channel: FAKE_UPDATE_CHANNEL,
         },
         fakePingJSON
@@ -424,6 +426,7 @@ describe("PingCentre", () => {
           locale: FAKE_LOCALE,
           topic: "activity-stream",
           client_id: FAKE_TELEMETRY_ID,
+          version: "69.0a1",
           release_channel: FAKE_UPDATE_CHANNEL,
         },
         fakePingJSON
@@ -524,6 +527,7 @@ describe("PingCentre", () => {
         {
           locale: FAKE_LOCALE,
           client_id: FAKE_TELEMETRY_ID,
+          version: "69.0a1",
           release_channel: FAKE_UPDATE_CHANNEL,
         },
         fakePingJSON

test/unit/unit-entry.js

@@ -42,7 +42,7 @@ const TEST_GLOBAL = {
       return Promise.resolve({ addons: [], fullData: false });
     },
   },
-  AppConstants: { MOZILLA_OFFICIAL: true },
+  AppConstants: { MOZILLA_OFFICIAL: true, MOZ_APP_VERSION: "69.0a1" },
   UpdateUtils: { getUpdateChannel() {} },
   BrowserWindowTracker: { getTopWindow() {} },
   ChromeUtils: {
@@ -288,7 +288,7 @@ const TEST_GLOBAL = {
       getEnumerator: () => [],
     },
     ww: { registerNotification() {}, unregisterNotification() {} },
-    appinfo: { appBuildID: "20180710100040" },
+    appinfo: { appBuildID: "20180710100040", version: "69.0a1" },
   },
   XPCOMUtils: {
     defineLazyGetter(object, name, f) {