Fix Bug 1425457 - Include some extra info to user event ping.

docs/v2-system-addon/data_dictionary.md

@@ -147,6 +147,7 @@ Schema definitions/validations that can be used for tests can be found in `syste
 | `action` | [Required] Either `activity_stream_event`, `activity_stream_session`, or `activity_stream_performance`. | :one:
 | `addon_version` | [Required] The version of the Activity Stream addon. | :one:
 | `client_id` | [Required] An identifier for this client. | :one:
+| `card_type` | [Optional] ("bookmark", "pocket", "trending", "pinned") | :one:
 | `date` | [Auto populated by Onyx] The date in YYYY-MM-DD format. | :three:
 | `experiment_id` | [Optional] The unique identifier for a specific experiment. | :one:
 | `event_id` | [Required] An identifier shared by multiple performance pings that describe ane entire request flow. | :one:
@@ -170,7 +171,8 @@ and losing focus. | :one:
 | `ua` | [Auto populated by Onyx] The user agent string. | :two:
 | `unload_reason` | [Required] The reason the Activity Stream page lost focus. | :one:
 | `url` | [Optional] The URL of the recommendation shown in one of the highlights spots, if any. | :one:
-| `value` | [Required] An integer that represents the measured performance value. Can store counts, times in milliseconds, and should always be a positive integer.| :one:
+| `value` (performance) | [Required] An integer that represents the measured performance value. Can store counts, times in milliseconds, and should always be a positive integer.| :one:
+| `value` (event) | [Optional] An object with keys "icon_type" and "card_type" to record the extra information for event ping| :one:
 | `ver` | [Auto populated by Onyx] The version of the Onyx API the ping was sent to. | :one:
 | `highlights_size` | [Optional] The size of the Highlights set. | :one:
 | `highlights_data_late_by_ms` | [Optional] Time in ms it took for Highlights to become initialized | :one:
@@ -191,6 +193,7 @@ and losing focus. | :one:
 | `user_prefs` | [Required] The encoded integer of user's preferences. | :one: & :four:
 | `is_prerendered` | [Required] A boolean to signify whether the page is prerendered or not | :one:
 | `is_preloaded` | [Required] A boolean to signify whether the page is preloaded or not | :one:
+| `icon_type` | [Optional] ("tippytop", "rich_icon", "screenshot_with_icon", "screenshot", "no_image") | :one:
 | `region` | [Optional] An string maps to pref "browser.search.region", which is essentially the two letter ISO 3166-1 country code populated by the Firefox search service. Note that: 1). it reports "OTHER" for those regions with smaller Firefox user base (less than 10000) so that users cannot be uniquely identified; 2). it reports "UNSET" if this pref is missing; 3). it reports "EMPTY" if the value of this pref is an empty string. | :one:
 | `profile_creation_date` | [Optional] An integer to record the age of the Firefox profile as the total number of days since the UNIX epoch. | :one:
 

docs/v2-system-addon/data_events.md

@@ -68,6 +68,10 @@ A user event ping includes some basic metadata (tab id, addon version, etc.) as
   "event": "CLICK",
   "source": "TOP_SITES",
   "action_position": 2,
+  "value": {
+    "card_type": "pinned",
+    "icon_type": ["screenshot_with_icon" | "screenshot" | "tippytop" | "rich_icon" | "no_image"]
+  }
 
   // Basic metadata
   "action": "activity_stream_event",
@@ -87,6 +91,10 @@ A user event ping includes some basic metadata (tab id, addon version, etc.) as
   "event": "DELETE",
   "source": "TOP_SITES",
   "action_position": 2,
+  "value": {
+    "card_type": "pinned",
+    "icon_type": ["screenshot_with_icon" | "screenshot" | "tippytop" | "rich_icon" | "no_image"]
+  }
 
   // Basic metadata
   "action": "activity_stream_event",
@@ -106,6 +114,10 @@ A user event ping includes some basic metadata (tab id, addon version, etc.) as
   "event": "BLOCK",
   "source": "TOP_SITES",
   "action_position": 2,
+  "value": {
+    "card_type": "pinned",
+    "icon_type": ["screenshot_with_icon" | "screenshot" | "tippytop" | "rich_icon" | "no_image"]
+  }
 
   // Basic metadata
   "action": "activity_stream_event",
@@ -123,8 +135,11 @@ A user event ping includes some basic metadata (tab id, addon version, etc.) as
 ```js
 {
   "event": "BOOKMARK_ADD",
-  "source": "TOP_SITES",
+  "source": "HIGHLIGHTS",
   "action_position": 2,
+  "value": {
+    "card_type": "trending"
+  }
 
   // Basic metadata
   "action": "activity_stream_event",
@@ -142,8 +157,11 @@ A user event ping includes some basic metadata (tab id, addon version, etc.) as
 ```js
 {
   "event": "BOOKMARK_DELETE",
-  "source": "TOP_SITES",
+  "source": "HIGHLIGHTS",
   "action_position": 2,
+  "value": {
+    "card_type": "bookmark"
+  }
 
   // Basic metadata
   "action": "activity_stream_event",
@@ -163,6 +181,10 @@ A user event ping includes some basic metadata (tab id, addon version, etc.) as
   "event": "OPEN_NEW_WINDOW",
   "source": "TOP_SITES",
   "action_position": 2,
+  "value": {
+    "card_type": "pinned",
+    "icon_type": ["screenshot_with_icon" | "screenshot" | "tippytop" | "rich_icon" | "no_image"]
+  }
 
   // Basic metadata
   "action": "activity_stream_event",
@@ -182,6 +204,10 @@ A user event ping includes some basic metadata (tab id, addon version, etc.) as
   "event": "OPEN_PRIVATE_WINDOW",
   "source": "TOP_SITES",
   "action_position": 2,
+  "value": {
+    "card_type": "pinned",
+    "icon_type": ["screenshot_with_icon" | "screenshot" | "tippytop" | "rich_icon" | "no_image"]
+  }
 
   // Basic metadata
   "action": "activity_stream_event",

system-addon/content-src/components/Card/Card.jsx

@@ -68,6 +68,18 @@ export class Card extends React.PureComponent {
     });
   }
 
+  /**
+   * Report to telemetry additional information about the item.
+   */
+  _getTelemetryInfo() {
+    // Filter out "history" type for being the default
+    if (this.props.link.type !== "history") {
+      return {value: {card_type: this.props.link.type}};
+    }
+
+    return null;
+  }
+
   onLinkClick(event) {
     event.preventDefault();
     const {altKey, button, ctrlKey, metaKey, shiftKey} = event;
@@ -83,11 +95,11 @@ export class Card extends React.PureComponent {
         action_position: this.props.index
       }));
     } else {
-      this.props.dispatch(ac.UserEvent({
+      this.props.dispatch(ac.UserEvent(Object.assign({
         event: "CLICK",
         source: this.props.eventSource,
         action_position: this.props.index
-      }));
+      }, this._getTelemetryInfo())));
 
       if (this.props.shouldSendImpressionStats) {
         this.props.dispatch(ac.ImpressionStats({
@@ -166,6 +178,7 @@ export class Card extends React.PureComponent {
           onUpdate={this.onMenuUpdate}
           options={link.contextMenuOptions || contextMenuOptions}
           site={link}
+          siteInfo={this._getTelemetryInfo()}
           shouldSendImpressionStats={shouldSendImpressionStats} />
       }
    </li>);

system-addon/content-src/components/LinkMenu/LinkMenu.jsx

@@ -10,23 +10,24 @@ const DEFAULT_SITE_MENU_OPTIONS = ["CheckPinTopSite", "EditTopSite", "Separator"
 export class _LinkMenu extends React.PureComponent {
   getOptions() {
     const {props} = this;
-    const {site, index, source, isPrivateBrowsingEnabled} = props;
+    const {site, index, source, isPrivateBrowsingEnabled, siteInfo} = props;
 
     // Handle special case of default site
     const propOptions = !site.isDefault ? props.options : DEFAULT_SITE_MENU_OPTIONS;
 
-    const options = propOptions.map(o => LinkMenuOptions[o](site, index, source, isPrivateBrowsingEnabled)).map(option => {
+    const options = propOptions.map(o => LinkMenuOptions[o](site, index, source, isPrivateBrowsingEnabled, siteInfo)).map(option => {
       const {action, impression, id, string_id, type, userEvent} = option;
       if (!type && id) {
         option.label = props.intl.formatMessage({id: string_id || id});
         option.onClick = () => {
           props.dispatch(action);
           if (userEvent) {
-            props.dispatch(ac.UserEvent({
+            const userEventData = Object.assign({
               event: userEvent,
               source,
               action_position: index
-            }));
+            }, siteInfo);
+            props.dispatch(ac.UserEvent(userEventData));
           }
           if (impression && props.shouldSendImpressionStats) {
             props.dispatch(impression);

system-addon/content-src/components/TopSites/TopSite.jsx

@@ -134,12 +134,24 @@ export class TopSite extends React.PureComponent {
     this.onMenuUpdate = this.onMenuUpdate.bind(this);
   }
 
+  /**
+   * Report to telemetry additional information about the item.
+   */
+  _getTelemetryInfo() {
+    const value = {icon_type: this.props.link.iconType};
+    // Filter out "not_pinned" type for being the default
+    if (this.props.link.isPinned) {
+      value.card_type = "pinned";
+    }
+    return {value};
+  }
+
   userEvent(event) {
-    this.props.dispatch(ac.UserEvent({
+    this.props.dispatch(ac.UserEvent(Object.assign({
       event,
       source: TOP_SITES_SOURCE,
       action_position: this.props.index
-    }));
+    }, this._getTelemetryInfo())));
   }
 
   onLinkClick(ev) {
@@ -175,6 +187,7 @@ export class TopSite extends React.PureComponent {
               onUpdate={this.onMenuUpdate}
               options={TOP_SITES_CONTEXT_MENU_OPTIONS}
               site={link}
+              siteInfo={this._getTelemetryInfo()}
               source={TOP_SITES_SOURCE} />
           }
         </div>
@@ -356,7 +369,7 @@ export class _TopSiteList extends React.PureComponent {
     const maxNarrowVisibleIndex = props.TopSitesRows * 6;
 
     for (let i = 0, l = topSites.length; i < l; i++) {
-      const link = topSites[i];
+      const link = topSites[i] && Object.assign({}, topSites[i], {iconType: this.props.topSiteIconType(topSites[i])});
       const slotProps = {
         key: link ? link.url : holeIndex++,
         index: i

system-addon/content-src/components/TopSites/TopSites.jsx

@@ -9,25 +9,30 @@ import {TOP_SITES_MAX_SITES_PER_ROW} from "common/Reducers.jsm";
 import {TopSiteForm} from "./TopSiteForm";
 import {TopSiteList} from "./TopSite";
 
+function topSiteIconType(link) {
+  if (link.tippyTopIcon || link.faviconRef === "tippytop") {
+    return "tippytop";
+  }
+  if (link.faviconSize >= MIN_RICH_FAVICON_SIZE) {
+    return "rich_icon";
+  }
+  if (link.screenshot && link.faviconSize >= MIN_CORNER_FAVICON_SIZE) {
+    return "screenshot_with_icon";
+  }
+  if (link.screenshot) {
+    return "screenshot";
+  }
+  return "no_image";
+}
+
 /**
  * Iterates through TopSites and counts types of images.
  * @param acc Accumulator for reducer.
  * @param topsite Entry in TopSites.
  */
 function countTopSitesIconsTypes(topSites) {
   const countTopSitesTypes = (acc, link) => {
-    if (link.tippyTopIcon || link.faviconRef === "tippytop") {
-      acc.tippytop++;
-    } else if (link.faviconSize >= MIN_RICH_FAVICON_SIZE) {
-      acc.rich_icon++;
-    } else if (link.screenshot && link.faviconSize >= MIN_CORNER_FAVICON_SIZE) {
-      acc.screenshot_with_icon++;
-    } else if (link.screenshot) {
-      acc.screenshot++;
-    } else {
-      acc.no_image++;
-    }
-
+    acc[topSiteIconType(link)]++;
     return acc;
   };
 
@@ -104,7 +109,7 @@ export class _TopSites extends React.PureComponent {
         eventSource={TOP_SITES_SOURCE}
         Prefs={props.Prefs}
         dispatch={props.dispatch}>
-        <TopSiteList TopSites={props.TopSites} TopSitesRows={props.TopSitesRows} dispatch={props.dispatch} intl={props.intl} />
+        <TopSiteList TopSites={props.TopSites} TopSitesRows={props.TopSitesRows} dispatch={props.dispatch} intl={props.intl} topSiteIconType={topSiteIconType} />
         <div className="edit-topsites-wrapper">
           {editForm &&
             <div className="edit-topsites">

system-addon/content-src/lib/link-menu-options.js

@@ -72,15 +72,15 @@ export const LinkMenuOptions = {
       action_position: index
     })
   }),
-  DeleteUrl: (site, index, eventSource) => ({
+  DeleteUrl: (site, index, eventSource, isEnabled, siteInfo) => ({
     id: "menu_action_delete",
     icon: "delete",
     action: {
       type: at.DIALOG_OPEN,
       data: {
         onConfirm: [
           ac.AlsoToMain({type: at.DELETE_HISTORY_URL, data: {url: site.url, pocket_id: site.pocket_id, forceBlock: site.bookmarkGuid}}),
-          ac.UserEvent({event: "DELETE", source: eventSource, action_position: index})
+          ac.UserEvent(Object.assign({event: "DELETE", source: eventSource, action_position: index}, siteInfo))
         ],
         eventSource,
         body_string_id: ["confirm_history_delete_p1", "confirm_history_delete_notice_p2"],
@@ -153,6 +153,6 @@ export const LinkMenuOptions = {
   CheckPinTopSite: (site, index) => (site.isPinned ? LinkMenuOptions.UnpinTopSite(site) : LinkMenuOptions.PinTopSite(site, index)),
   CheckSavedToPocket: (site, index) => (site.pocket_id ? LinkMenuOptions.DeleteFromPocket(site) : LinkMenuOptions.SaveToPocket(site, index)),
   CheckBookmarkOrArchive: site => (site.pocket_id ? LinkMenuOptions.ArchiveFromPocket(site) : LinkMenuOptions.CheckBookmark(site)),
-  CheckDeleteHistoryOrEmpty: (site, index, eventSource) => (site.pocket_id ? LinkMenuOptions.EmptyItem() : LinkMenuOptions.DeleteUrl(site, index, eventSource)),
+  CheckDeleteHistoryOrEmpty: (site, index, eventSource, isEnabled, siteInfo) => (site.pocket_id ? LinkMenuOptions.EmptyItem() : LinkMenuOptions.DeleteUrl(site, index, eventSource, isEnabled, siteInfo)),
   OpenInPrivateWindow: (site, index, eventSource, isEnabled) => (isEnabled ? _OpenInPrivateWindow(site) : LinkMenuOptions.EmptyItem())
 };

system-addon/test/schemas/pings.js

@@ -88,7 +88,11 @@ export const UserEventAction = Joi.object().keys({
       "ARCHIVE_FROM_POCKET"
     ]).required(),
     source: Joi.valid(["TOP_SITES", "TOP_STORIES", "HIGHLIGHTS"]),
-    action_position: Joi.number().integer()
+    action_position: Joi.number().integer(),
+    value: Joi.object().keys({
+      icon_type: Joi.valid(["tippytop", "rich_icon", "screenshot_with_icon", "screenshot", "no_image"]),
+      card_type: Joi.valid(["bookmark", "trending", "pinned", "pocket"])
+    })
   }).required(),
   meta: Joi.object().keys({
     to: Joi.valid(MAIN_MESSAGE_TYPE).required(),

system-addon/test/unit/content-src/components/Card.test.jsx

@@ -36,6 +36,9 @@ describe("<Card>", () => {
   beforeEach(() => {
     wrapper = mountCardWithProps(DEFAULT_PROPS);
   });
+  afterEach(() => {
+    DEFAULT_PROPS.dispatch.reset();
+  });
   it("should render a Card component", () => assert.ok(wrapper.exists()));
   it("should add the right url", () => {
     assert.propertyVal(wrapper.find("a").props(), "href", DEFAULT_PROPS.link.url);
@@ -210,6 +213,23 @@ describe("<Card>", () => {
         tiles: [{id: DEFAULT_PROPS.link.guid, pos: DEFAULT_PROPS.index}]
       }));
     });
+    it("should provide card_type to telemetry info if type is not history", () => {
+      const link = Object.assign({}, DEFAULT_PROPS.link);
+      link.type = "bookmark";
+      wrapper = mountWithIntl(<Card {...Object.assign({}, DEFAULT_PROPS, {link})} />);
+      const card = wrapper.find(".card");
+      const event = {altKey: "1", button: "2", ctrlKey: "3", metaKey: "4", shiftKey: "5"};
+
+      card.simulate("click", Object.assign({}, event, {preventDefault: () => {}}));
+
+      assert.isUserEventAction(DEFAULT_PROPS.dispatch.secondCall.args[0]);
+      assert.calledWith(DEFAULT_PROPS.dispatch.secondCall, ac.UserEvent({
+        event: "CLICK",
+        source: DEFAULT_PROPS.eventSource,
+        action_position: DEFAULT_PROPS.index,
+        value: {card_type: link.type}
+      }));
+    });
     it("should notify Web Extensions with WEBEXT_CLICK if props.isWebExtension is true", () => {
       wrapper = mountCardWithProps(Object.assign({}, DEFAULT_PROPS, {isWebExtension: true, eventSource: "MyExtension", index: 3}));
       const card = wrapper.find(".card");

system-addon/test/unit/content-src/components/LinkMenu.test.jsx

@@ -139,7 +139,7 @@ describe("<LinkMenu>", () => {
   describe(".onClick", () => {
     const FAKE_INDEX = 3;
     const FAKE_SOURCE = "TOP_SITES";
-    const FAKE_SITE = {url: "https://foo.com", pocket_id: "1234", referrer: "https://foo.com/ref", title: "bar", bookmarkGuid: 1234, hostname: "foo", type: "history"};
+    const FAKE_SITE = {url: "https://foo.com", pocket_id: "1234", referrer: "https://foo.com/ref", title: "bar", bookmarkGuid: 1234, hostname: "foo", type: "bookmark"};
     const dispatch = sinon.stub();
     const propOptions = ["Separator", "RemoveBookmark", "AddBookmark", "OpenInNewWindow", "OpenInPrivateWindow", "BlockUrl", "DeleteUrl", "PinTopSite", "UnpinTopSite", "SaveToPocket", "DeleteFromPocket", "ArchiveFromPocket", "WebExtDismiss"];
     const expectedActionData = {
@@ -159,6 +159,7 @@ describe("<LinkMenu>", () => {
 
     const {options} = shallowWithIntl(<LinkMenu
       site={FAKE_SITE}
+      siteInfo={{value: {card_type: FAKE_SITE.type}}}
       dispatch={dispatch}
       index={FAKE_INDEX}
       isPrivateBrowsingEnabled={true}
@@ -202,6 +203,7 @@ describe("<LinkMenu>", () => {
           assert.isUserEventAction(action);
           assert.propertyVal(action.data, "source", FAKE_SOURCE);
           assert.propertyVal(action.data, "action_position", FAKE_INDEX);
+          assert.propertyVal(action.data.value, "card_type", FAKE_SITE.type);
         }
       });
       it(`should send impression stats for ${option.id}`, () => {

system-addon/test/unit/content-src/components/TopSites.test.jsx

@@ -20,6 +20,7 @@ const perfSvc = {
 const DEFAULT_PROPS = {
   TopSites: {initialized: true, rows: []},
   TopSitesRows: TOP_SITES_DEFAULT_ROWS,
+  topSiteIconType: () => "no_image",
   dispatch() {},
   intl: {formatMessage: x => x},
   perfSvc
@@ -523,7 +524,7 @@ describe("<TopSite>", () => {
     });
     it("should dispatch a UserEventAction with the right data", () => {
       const dispatch = sinon.stub();
-      const wrapper = shallow(<TopSite link={link} index={3} dispatch={dispatch} />);
+      const wrapper = shallow(<TopSite link={Object.assign({}, link, {iconType: "rich_icon", isPinned: true})} index={3} dispatch={dispatch} />);
 
       wrapper.find(TopSiteLink).simulate("click", {});
 
@@ -533,6 +534,8 @@ describe("<TopSite>", () => {
       assert.propertyVal(action.data, "event", "CLICK");
       assert.propertyVal(action.data, "source", "TOP_SITES");
       assert.propertyVal(action.data, "action_position", 3);
+      assert.propertyVal(action.data.value, "card_type", "pinned");
+      assert.propertyVal(action.data.value, "icon_type", "rich_icon");
     });
   });
 });