(nobug) - Prevent duplicate ids in the messageBlockList (#4641)

lib/ASRouter.jsm

@@ -205,6 +205,10 @@ const MessageLoaderUtils = {
       messages = [];
       Cu.reportError(new Error(`Tried to load messages for ${provider.id} but the result was not an Array.`));
     }
+    // Filter out messages we temporarily want to exclude
+    if (provider.exclude && provider.exclude.length) {
+      messages = messages.filter(message => !provider.exclude.includes(message.id));
+    }
     const lastUpdated = Date.now();
     return {
       messages: messages.map(msg => ({weight: 100, ...msg, provider: provider.id}))
@@ -340,15 +344,10 @@ class _ASRouter {
       }
     }
 
-    // Group existing blocked messages with messages blocked through preferences
-    const excludeList = ASRouterPreferences.providers.filter(p => p.exclude)
-      .reduce((blocked, p) => blocked.concat(p.exclude), this.state.messageBlockList);
-
     this.setState(prevState => ({
       providers,
       // Clear any messages from removed providers
       messages: [...prevState.messages.filter(message => providerIDs.includes(message.provider))],
-      messageBlockList: excludeList,
     }));
   }
 

test/unit/asrouter/ASRouter.test.js

@@ -130,14 +130,6 @@ describe("ASRouter", () => {
 
       assert.deepEqual(Router.state.messageBlockList, ["foo"]);
     });
-    it("should set state.messageBlockList to the block list in ASRouterPreferences", async () => {
-      setMessageProviderPref([{id: "onboarding", type: "local", exclude: ["RTAMO"]}]);
-      messageBlockList = ["foo"];
-      Router = new _ASRouter();
-      await Router.init(channel, createFakeStorage(), dispatchStub);
-
-      assert.deepEqual(Router.state.messageBlockList, ["foo", "RTAMO"]);
-    });
     it("should set state.messageImpressions to the messageImpressions object in persistent storage", async () => {
       // Note that messageImpressions are only kept if a message exists in router and has a .frequency property,
       // otherwise they will be cleaned up by .cleanupImpressions()

test/unit/asrouter/MessageLoaderUtils.test.js

@@ -36,6 +36,14 @@ describe("MessageLoaderUtils", () => {
       assert.propertyVal(message, "id", "foo");
       assert.propertyVal(message, "provider", "provider123");
     });
+    it("should filter out local messages listed in the `exclude` field", async () => {
+      const sourceMessage = {id: "foo"};
+      const provider = {id: "provider123", type: "local", messages: [sourceMessage], exclude: ["foo"]};
+
+      const result = await MessageLoaderUtils.loadMessagesForProvider(provider, FAKE_STORAGE);
+
+      assert.lengthOf(result.messages, 0);
+    });
     it("should return messages for remote provider", async () => {
       const sourceMessage = {id: "foo"};
       fetchStub.resolves({ok: true, status: 200, json: () => Promise.resolve({messages: [sourceMessage]}), headers: FAKE_RESPONSE_HEADERS});