Fix Bug 1479413 - Remove preview provider and message after it has been displayed

content-src/asrouter/README.md

@@ -0,0 +1,29 @@
+# Activity Stream Router
+
+## Preferences `browser.newtab.activity-stream.asrouter.*`
+
+Name | Used for | Type | Example value
+---  | ---      | ---  | ---
+`whitelistHosts` | Whitelist a host in order to fetch messages from its endpoint | `[String]` |  `["gist.github.com", "gist.githubusercontent.com", "localhost:8000"]`
+`snippetsUrl` | The main remote endpoint that serves all snippet messages | `String` | `https://activity-stream-icons.services.mozilla.com/v1/messages.json.br`
+
+## Admin Interface
+
+* Navigate to `about:newtab#asrouter`
+  * See all available messages and message providers
+  * Block, unblock or force show a specific message
+
+## Snippet Preview
+
+* Whitelist the provider host that will serve the messages
+  * In `about:config`, `browser.newtab.activity-stream.asrouter.whitelistHosts` can contain a array of hosts
+  * Example value `["gist.github.com", "gist.githubusercontent.com", "localhost:8000"]`
+  * Errors are surfaced in the `Console` tab of the `Browser Toolbox` ([read how to enable](https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox))
+* Navigate to `about:newtab?endpoint=<URL>`
+  * Example `https://gist.githubusercontent.com/piatra/70234f08696c0a0509d7ba5568cd830f/raw/68370f34abc134142c64b6f0a9b9258a06de7aa3/messages.json`
+  * URL should be from an endpoint that was just whitelisted
+  * The snippet preview should imediately load
+  * The endpoint must be HTTPS, the host must be whitelisted
+  * Errors are surfaced in the `Console` tab of the `Browser Toolbox`
+
+### [Snippet message format documentation](https://github.com/mozilla/activity-stream/blob/master/content-src/asrouter/schemas/message-format.md)

lib/ASRouter.jsm

@@ -241,7 +241,8 @@ class _ASRouter {
         ASRouterTriggerListeners.get(triggerID).uninit();
       }
 
-      await this.setState(newState);
+      // We don't want to cache preview endpoints, remove them after messages are fetched
+      await this.setState(this._removePreviewEndpoint(newState));
       await this.cleanupImpressions();
     }
   }
@@ -466,7 +467,15 @@ class _ASRouter {
       message = await this._findMessage(msgs, target, trigger);
     }
 
-    await this.setState({lastMessageId: message ? message.id : null});
+    if (previewMsgs.length) {
+      // We don't want to cache preview messages, remove them after we selected the message to show
+      await this.setState(state => ({
+        lastMessageId: message.id,
+        messages: state.messages.filter(m => m.id !== message.id)
+      }));
+    } else {
+      await this.setState({lastMessageId: message ? message.id : null});
+    }
     await this._sendMessageToTarget(message, target, trigger);
   }
 
@@ -547,10 +556,14 @@ class _ASRouter {
     await this.onMessage({target, data: {type: "TRIGGER", trigger}});
   }
 
+  _removePreviewEndpoint(state) {
+    state.providers = state.providers.filter(p => p.id !== "preview");
+    return state;
+  }
+
   async _addPreviewEndpoint(url) {
     const providers = [...this.state.providers];
     if (this._validPreviewEndpoint(url) && !providers.find(p => p.url === url)) {
-      // Set update cycle to 0 to fetch new content on every page refresh
       providers.push({id: "preview", type: "remote", url, updateCycleInMs: 0});
       await this.setState({providers});
     }

test/unit/asrouter/ASRouter.test.js

@@ -329,27 +329,21 @@ describe("ASRouter", () => {
 
       assert.calledWith(msg.target.sendAsyncMessage, PARENT_TO_CHILD_MESSAGE_NAME, {type: "CLEAR_ALL"});
     });
-    it("should add the endpoint provided on CONNECT_UI_REQUEST", async () => {
+    it("should make a request to the provided endpoint on CONNECT_UI_REQUEST", async () => {
       const url = "https://snippets-admin.mozilla.org/foo";
       const msg = fakeAsyncMessage({type: "CONNECT_UI_REQUEST", data: {endpoint: {url}}});
       await Router.onMessage(msg);
 
-      assert.isDefined(Router.state.providers.find(p => p.url === url));
+      assert.calledWith(global.fetch, url);
+      assert.lengthOf(Router.state.providers.filter(p => p.url === url), 0);
     });
-    it("should add the endpoint provided on ADMIN_CONNECT_STATE", async () => {
+    it("should make a request to the provided endpoint on ADMIN_CONNECT_STATE and remove the endpoint", async () => {
       const url = "https://snippets-admin.mozilla.org/foo";
       const msg = fakeAsyncMessage({type: "ADMIN_CONNECT_STATE", data: {endpoint: {url}}});
       await Router.onMessage(msg);
 
-      assert.isDefined(Router.state.providers.find(p => p.url === url));
-    });
-    it("should not add the same endpoint twice", async () => {
-      const url = "https://snippets-admin.mozilla.org/foo";
-      const msg = fakeAsyncMessage({type: "CONNECT_UI_REQUEST", data: {endpoint: {url}}});
-      await Router.onMessage(msg);
-      await Router.onMessage(msg);
-
-      assert.lengthOf(Router.state.providers.filter(p => p.url === url), 1);
+      assert.calledWith(global.fetch, url);
+      assert.lengthOf(Router.state.providers.filter(p => p.url === url), 0);
     });
     it("should not add a url that is not from a whitelisted host", async () => {
       const url = "https://mozilla.org";
@@ -452,13 +446,14 @@ describe("ASRouter", () => {
       assert.calledOnce(Router.sendNextMessage);
       assert.calledWithExactly(Router.sendNextMessage, sinon.match.instanceOf(FakeRemotePageManager), {});
     });
-    it("should return the preview message if that's available", async () => {
+    it("should return the preview message if that's available and remove it from Router.state", async () => {
       const expectedObj = {provider: "preview"};
       Router.setState({messages: [expectedObj]});
 
       await Router.sendNextMessage(channel);
 
       assert.calledWith(channel.sendAsyncMessage, PARENT_TO_CHILD_MESSAGE_NAME, {type: "SET_MESSAGE", data: expectedObj});
+      assert.isUndefined(Router.state.messages.find(m => m.provider === "preview"));
     });
     it("should call _getBundledMessages if we request a message that needs to be bundled", async () => {
       sandbox.stub(Router, "_getBundledMessages").resolves();