Bug 1456145 - Follow-up on impression pings for AS Router

system-addon/content-src/asrouter/asrouter-content.jsx

@@ -1,17 +1,13 @@
 import {actionCreators as ac} from "common/Actions.jsm";
 import {OUTGOING_MESSAGE_NAME as AS_GENERAL_OUTGOING_MESSAGE_NAME} from "content-src/lib/init-store";
-import {ImpressionsWrapper} from "content-src/components/Sections/ImpressionsWrapper";
+import {ImpressionsWrapper} from "./components/ImpressionsWrapper/ImpressionsWrapper";
 import React from "react";
 import ReactDOM from "react-dom";
 import {SimpleSnippet} from "./templates/SimpleSnippet/SimpleSnippet";
 
 const INCOMING_MESSAGE_NAME = "ASRouter:parent-to-child";
 const OUTGOING_MESSAGE_NAME = "ASRouter:child-to-parent";
 
-// Note: Provider is hardcoded right now since we only have one message provider.
-// When we have more than one, it will need to come from the message data.
-const PROVIDER = "snippets";
-
 export const ASRouterUtils = {
   addListener(listener) {
     global.addMessageListener(INCOMING_MESSAGE_NAME, listener);
@@ -28,44 +24,45 @@ export const ASRouterUtils = {
   unblockById(id) {
     ASRouterUtils.sendMessage({type: "UNBLOCK_MESSAGE_BY_ID", data: {id}});
   },
-  sendUserActionTelemetry(data) {
-    const eventType =  `${PROVIDER}_user_event`;
-    const payload = ac.ASRouterUserEvent(Object.assign({}, data, {action: eventType}));
-    global.sendAsyncMessage(AS_GENERAL_OUTGOING_MESSAGE_NAME, payload);
-  },
-  sendImpressionStats({id, campaign, template}) {
-    const eventType =  `${PROVIDER}_impression`;
-    const payload = ac.ASRouterUserEvent(Object.assign({}, {id, campaign, template}, {action: eventType}));
-    global.sendAsyncMessage(AS_GENERAL_OUTGOING_MESSAGE_NAME, payload);
-  },
   getNextMessage() {
     ASRouterUtils.sendMessage({type: "GET_NEXT_MESSAGE"});
   },
   overrideMessage(id) {
     ASRouterUtils.sendMessage({type: "OVERRIDE_MESSAGE", data: {id}});
+  },
+  sendTelemetry(ping) {
+    const payload = ac.ASRouterUserEvent(ping);
+    global.sendAsyncMessage(AS_GENERAL_OUTGOING_MESSAGE_NAME, payload);
   }
 };
 
+// Note: nextProps/prevProps refer to props passed to <ImpressionsWrapper />, not <ASRouterUISurface />
+function shouldSendImpressionOnUpdate(nextProps, prevProps) {
+  return (nextProps.message.id && (!prevProps.message || prevProps.message.id !== nextProps.message.id));
+}
+
 export class ASRouterUISurface extends React.PureComponent {
   constructor(props) {
     super(props);
     this.onMessageFromParent = this.onMessageFromParent.bind(this);
+    this.sendImpression = this.sendImpression.bind(this);
+    this.sendUserActionTelemetry = this.sendUserActionTelemetry.bind(this);
     this.state = {message: {}};
-    this.dispatchImpressionStats = this.dispatchImpressionStats.bind(this);
-    this.shouldSendImpressionsOnUpdate = this.shouldSendImpressionsOnUpdate.bind(this);
   }
 
-  dispatchImpressionStats() {
-    ASRouterUtils.sendImpressionStats(this.state.message);
-  }
+  sendUserActionTelemetry(extraProps = {}) {
+    const {message} = this.state;
+    const eventType =  `${message.provider}_user_event`;
 
-  shouldSendImpressionsOnUpdate(prevProps) {
-    const {state} = this;
-    if (state.message.id && (!prevProps.message || prevProps.message.id !== state.message.id)) {
-      return true;
-    }
+    ASRouterUtils.sendTelemetry(Object.assign({
+      message_id: message.id,
+      source: this.props.id,
+      action: eventType
+    }, extraProps));
+  }
 
-    return false;
+  sendImpression() {
+    this.sendUserActionTelemetry({event: "IMPRESSION"});
   }
 
   onBlockById(id) {
@@ -89,29 +86,31 @@ export class ASRouterUISurface extends React.PureComponent {
   }
 
   componentWillUnmount() {
-    ASRouterUtils.removeMessageListener(this.onMessageFromParent);
+    ASRouterUtils.removeListener(this.onMessageFromParent);
   }
 
   render() {
     const {message} = this.state;
     if (!message.id) { return null; }
-    return (<ImpressionsWrapper document={global.document}
-        dispatchImpressionStats={this.dispatchImpressionStats}
+    return (<ImpressionsWrapper
         message={message}
-        sendOnMount={true}
-        shouldSendImpressionsOnUpdate={this.shouldSendImpressionsOnUpdate}
-        shouldSendImpressionStats={true}>
+        sendImpression={this.sendImpression}
+        shouldSendImpressionOnUpdate={shouldSendImpressionOnUpdate}
+        // This helps with testing
+        document={this.props.document}>
         <SimpleSnippet
           {...message}
           UISurface={this.props.id}
           getNextMessage={ASRouterUtils.getNextMessage}
           onBlock={this.onBlockById(message.id)}
-          sendUserActionTelemetry={ASRouterUtils.sendUserActionTelemetry} />
+          sendUserActionTelemetry={this.sendUserActionTelemetry} />
       </ImpressionsWrapper>
     );
   }
 }
 
+ASRouterUISurface.defaultProps = {document: global.document};
+
 export function initASRouter() {
   ReactDOM.render(<ASRouterUISurface id="NEWTAB_FOOTER_BAR" />, document.getElementById("snippets-container"));
 }

system-addon/content-src/asrouter/components/ImpressionsWrapper/ImpressionsWrapper.jsx

@@ -10,9 +10,9 @@ export class ImpressionsWrapper extends React.PureComponent {
   // This sends an event when a user sees a set of new content. If content
   // changes while the page is hidden (i.e. preloaded or on a hidden tab),
   // only send the event if the page becomes visible again.
-  sendImpressionStatsOrAddListener() {
+  sendImpressionOrAddListener() {
     if (this.props.document.visibilityState === VISIBLE) {
-      this.props.dispatchImpressionStats();
+      this.props.sendImpression();
     } else {
       // We should only ever send the latest impression stats ping, so remove any
       // older listeners.
@@ -23,7 +23,7 @@ export class ImpressionsWrapper extends React.PureComponent {
       // When the page becomes visible, send the impression stats ping if the section isn't collapsed.
       this._onVisibilityChange = () => {
         if (this.props.document.visibilityState === VISIBLE) {
-          this.props.dispatchImpressionStats();
+          this.props.sendImpression();
           this.props.document.removeEventListener(VISIBILITY_CHANGE_EVENT, this._onVisibilityChange);
         }
       };
@@ -39,17 +39,22 @@ export class ImpressionsWrapper extends React.PureComponent {
 
   componentDidMount() {
     if (this.props.sendOnMount) {
-      this.sendImpressionStatsOrAddListener();
+      this.sendImpressionOrAddListener();
     }
   }
 
   componentDidUpdate(prevProps) {
-    if (prevProps && this.props.shouldSendImpressionsOnUpdate(prevProps)) {
-      this.sendImpressionStatsOrAddListener();
+    if (this.props.shouldSendImpressionOnUpdate(this.props, prevProps)) {
+      this.sendImpressionOrAddListener();
     }
   }
 
   render() {
     return this.props.children;
   }
 }
+
+ImpressionsWrapper.defaultProps = {
+  document: global.document,
+  sendOnMount: true
+};

system-addon/content-src/asrouter/components/SnippetBase/SnippetBase.jsx

@@ -7,7 +7,7 @@ export class SnippetBase extends React.PureComponent {
   }
 
   onBlockClicked() {
-    this.props.sendUserActionTelemetry({event: "BLOCK", source: this.props.UISurface, message_id: this.props.id});
+    this.props.sendUserActionTelemetry({event: "BLOCK"});
     this.props.onBlock();
   }
 

system-addon/content-src/asrouter/templates/SimpleSnippet/SimpleSnippet.jsx

@@ -12,7 +12,7 @@ export class SimpleSnippet extends React.PureComponent {
   }
 
   onButtonClick() {
-    this.props.sendUserActionTelemetry({event: "CLICK_BUTTON", source: this.props.UISurface, message_id: this.props.id});
+    this.props.sendUserActionTelemetry({event: "CLICK_BUTTON"});
   }
 
   render() {

system-addon/lib/ASRouter.jsm

@@ -7,31 +7,14 @@ const ONE_HOUR_IN_MS = 60 * 60 * 1000;
 // This is a temporary endpoint until we have something for snippets
 const SNIPPETS_ENDPOINT = "https://activity-stream-icons.services.mozilla.com/v1/messages.json.br";
 
-const ONBOARDING_MESSAGES = [
+const LOCAL_TEST_MESSAGES = [
   {
-    id: "ONBOARDING_1",
+    id: "LOCAL_TEST_THEMES",
     template: "simple_snippet",
     content: {
-      title: "Find it faster",
-      text: "Access all of your favorite search engines with a click. Search the whole Web or just one website from the search box.",
-      button_label: "Learn More",
-      button_url: "https://mozilla.org"
-    }
-  },
-  {
-    id: "ONBOARDING_2",
-    template: "simple_snippet",
-    content: {
-      title: "Make Firefox your go-to-browser",
-      text: "It doesn't take much to get the most from Firefox. Just set Firefox as your default browser and put control, customization, and protection on autopilot."
-    }
-  },
-  {
-    id: "ONBOARDING_3",
-    template: "simple_snippet",
-    content: {
-      title: "Did you know?",
-      text: "All human beings are born free and equal in dignity and rights. They are endowed with reason and conscience and should act towards one another in a spirit of brotherhood."
+      text: "Your browser is ready for a makeover. Don't worry, you've got tons of options.",
+      button_label: "Check them out here",
+      button_url: "https://addons.mozilla.org/en-US/firefox/themes"
     }
   }
 ];
@@ -322,7 +305,7 @@ this._ASRouter = _ASRouter;
  */
 this.ASRouter = new _ASRouter({
   providers: [
-    {id: "onboarding", type: "local", messages: ONBOARDING_MESSAGES},
+    {id: "onboarding", type: "local", messages: LOCAL_TEST_MESSAGES},
     {id: "snippets", type: "remote", url: SNIPPETS_ENDPOINT, updateCycleInMs: ONE_HOUR_IN_MS * 4}
   ]
 });