Merge branch '213712-move-track_event-out-of-haml-and-into-javascript-client' into 'master'

Resolve "Move track_event out of HAML and into JavaScript client"

Closes #213712

See merge request gitlab-org/gitlab!29527

app/assets/javascripts/tracking.js

@@ -14,11 +14,8 @@ const DEFAULT_SNOWPLOW_OPTIONS = {
   linkClickTracking: false,
 };
 
-const eventHandler = (e, func, opts = {}) => {
-  const el = e.target.closest('[data-track-event]');
-  const action = el && el.dataset.trackEvent;
-  if (!action) return;
-
+const createEventPayload = (el, { suffix = '' } = {}) => {
+  const action = el.dataset.trackEvent + (suffix || '');
   let value = el.dataset.trackValue || el.value || undefined;
   if (el.type === 'checkbox' && !el.checked) value = false;
 
@@ -29,7 +26,19 @@ const eventHandler = (e, func, opts = {}) => {
     context: el.dataset.trackContext,
   };
 
-  func(opts.category, action + (opts.suffix || ''), omitBy(data, isUndefined));
+  return {
+    action,
+    data: omitBy(data, isUndefined),
+  };
+};
+
+const eventHandler = (e, func, opts = {}) => {
+  const el = e.target.closest('[data-track-event]');
+
+  if (!el) return;
+
+  const { action, data } = createEventPayload(el, opts);
+  func(opts.category, action, data);
 };
 
 const eventHandlers = (category, func) => {
@@ -62,17 +71,30 @@ export default class Tracking {
     return window.snowplow('trackStructEvent', category, action, label, property, value, contexts);
   }
 
-  static bindDocument(category = document.body.dataset.page, documentOverride = null) {
-    const el = documentOverride || document;
-    if (!this.enabled() || el.trackingBound) return [];
+  static bindDocument(category = document.body.dataset.page, parent = document) {
+    if (!this.enabled() || parent.trackingBound) return [];
 
-    el.trackingBound = true;
+    // eslint-disable-next-line no-param-reassign
+    parent.trackingBound = true;
 
     const handlers = eventHandlers(category, (...args) => this.event(...args));
-    handlers.forEach(event => el.addEventListener(event.name, event.func));
+    handlers.forEach(event => parent.addEventListener(event.name, event.func));
     return handlers;
   }
 
+  static trackLoadEvents(category = document.body.dataset.page, parent = document) {
+    if (!this.enabled()) return [];
+
+    const loadEvents = parent.querySelectorAll('[data-track-event="render"]');
+
+    loadEvents.forEach(element => {
+      const { action, data } = createEventPayload(element);
+      this.event(category, action, data);
+    });
+
+    return loadEvents;
+  }
+
   static mixin(opts = {}) {
     return {
       computed: {
@@ -111,6 +133,7 @@ export function initUserTracking() {
   if (opts.linkClickTracking) window.snowplow('enableLinkClickTracking');
 
   Tracking.bindDocument();
+  Tracking.trackLoadEvents();
 
   document.dispatchEvent(new Event('SnowplowInitialized'));
 }

ee/app/views/layouts/header/_user_notification_dot.html.haml

 - return unless show_ci_minutes_notification_dot?(project, namespace)
 
--# Tracking events from the template is not ideal and we are moving this to the client in https://gitlab.com/gitlab-org/gitlab/-/issues/213712
-- track_event('show_buy_ci_minutes_notification', label: current_user.namespace.actual_plan_name, property: 'user_dropdown')
-
-%span.header-user-notification-dot.rounded-circle.position-relative
+%span.header-user-notification-dot.rounded-circle.position-relative{ data: { track_label: "show_buy_ci_minutes_notification", track_property: current_user.namespace.actual_plan_name, track_event: 'render' } }

ee/spec/features/buy_ci_minutes_notification_dot_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe 'User notification dot', :aggregate_failures do
-  let_it_be(:user) { create(:user) }
-  let(:project) { create(:project, namespace: group, creator: user, shared_runners_enabled: true) }
-  let(:group) { create(:group) }
-  let!(:user_pipelines) { create(:ci_pipeline, user: user, project: project) }
-
-  before do
-    stub_experiment_for_user(ci_notification_dot: true)
-    group.add_developer(user)
-
-    sign_in(user)
-  end
-
-  context 'when ci minutes are below threshold' do
-    before do
-      allow(Gitlab).to receive(:com?) { true }
-
-      group.update(last_ci_minutes_usage_notification_level: 30, shared_runners_minutes_limit: 10)
-      allow_any_instance_of(EE::Namespace).to receive(:shared_runners_remaining_minutes).and_return(2)
-    end
-
-    it 'shows notification dot' do
-      expect_next_instance_of(ProjectsController) do |ctrl|
-        expect(ctrl).to receive(:track_event)
-                          .with('show_buy_ci_minutes_notification', label: 'free', property: 'user_dropdown')
-      end
-
-      visit project_path(project)
-
-      expect(page).to have_css('span', class: 'header-user-notification-dot')
-    end
-  end
-
-  context 'when ci minutes are not below threshold' do
-    let(:group) { create(:group, :with_not_used_build_minutes_limit) }
-
-    it 'shows notification dot' do
-      expect_next_instance_of(ProjectsController) do |ctrl|
-        expect(ctrl).not_to receive(:track_event)
-      end
-
-      visit project_path(project)
-
-      expect(page).not_to have_css('span', class: 'header-user-notification-dot')
-    end
-  end
-end

ee/spec/views/layouts/application.html.haml_spec.rb

@@ -24,11 +24,10 @@ describe 'layouts/application' do
       end
 
       it 'has the notification dot' do
-        expect(view).to receive(:track_event).with('show_buy_ci_minutes_notification', label: 'free', property: 'user_dropdown')
-
         render
 
         expect(rendered).to have_css('span', class: 'header-user-notification-dot')
+        expect(rendered).to have_selector('[data-track-event="render"]')
       end
     end
 
@@ -37,6 +36,7 @@ describe 'layouts/application' do
         render
 
         expect(rendered).not_to have_css('span', class: 'header-user-notification-dot')
+        expect(rendered).not_to have_selector('[data-track-event="render"]')
       end
     end
   end

lib/gitlab/tracking.rb

@@ -9,13 +9,6 @@ module Gitlab
     module ControllerConcern
       extend ActiveSupport::Concern
 
-      included do
-        # Tracking events from the template is not ideal and we are moving this to the client in https://gitlab.com/gitlab-org/gitlab/-/issues/213712
-        # In the meantime, using this method from the view is frowned upon and this line will likely be removed
-        # in the near future
-        helper_method :track_event
-      end
-
       protected
 
       def track_event(action = action_name, **args)

spec/frontend/tracking_spec.js

@@ -4,6 +4,7 @@ import Tracking, { initUserTracking } from '~/tracking';
 describe('Tracking', () => {
   let snowplowSpy;
   let bindDocumentSpy;
+  let trackLoadEventsSpy;
 
   beforeEach(() => {
     window.snowplow = window.snowplow || (() => {});
@@ -18,6 +19,7 @@ describe('Tracking', () => {
   describe('initUserTracking', () => {
     beforeEach(() => {
       bindDocumentSpy = jest.spyOn(Tracking, 'bindDocument').mockImplementation(() => null);
+      trackLoadEventsSpy = jest.spyOn(Tracking, 'trackLoadEvents').mockImplementation(() => null);
     });
 
     it('calls through to get a new tracker with the expected options', () => {
@@ -58,6 +60,11 @@ describe('Tracking', () => {
       initUserTracking();
       expect(bindDocumentSpy).toHaveBeenCalled();
     });
+
+    it('tracks page loaded events', () => {
+      initUserTracking();
+      expect(trackLoadEventsSpy).toHaveBeenCalled();
+    });
   });
 
   describe('.event', () => {
@@ -127,6 +134,7 @@ describe('Tracking', () => {
         <input type="checkbox" data-track-event="toggle_checkbox" value="_value_" checked/>
         <input class="dropdown" data-track-event="toggle_dropdown"/>
         <div data-track-event="nested_event"><span class="nested"></span></div>
+        <input data-track-eventbogus="click_bogusinput" data-track-label="_label_" value="_value_"/>
       `);
     });
 
@@ -139,6 +147,12 @@ describe('Tracking', () => {
       });
     });
 
+    it('does not bind to clicks on elements without [data-track-event]', () => {
+      trigger('[data-track-eventbogus="click_bogusinput"]');
+
+      expect(eventSpy).not.toHaveBeenCalled();
+    });
+
     it('allows value override with the data-track-value attribute', () => {
       trigger('[data-track-event="click_input2"]');
 
@@ -178,6 +192,44 @@ describe('Tracking', () => {
     });
   });
 
+  describe('tracking page loaded events', () => {
+    let eventSpy;
+
+    beforeEach(() => {
+      eventSpy = jest.spyOn(Tracking, 'event');
+      setHTMLFixture(`
+        <input data-track-event="render" data-track-label="label1" value="_value_" data-track-property="_property_"/>
+        <span data-track-event="render" data-track-label="label2" data-track-value="_value_">
+          Something
+        </span>
+        <input data-track-event="_render_bogus_" data-track-label="label3" value="_value_" data-track-property="_property_"/>
+      `);
+      Tracking.trackLoadEvents('_category_'); // only happens once
+    });
+
+    it('sends tracking events when [data-track-event="render"] is on an element', () => {
+      expect(eventSpy.mock.calls).toEqual([
+        [
+          '_category_',
+          'render',
+          {
+            label: 'label1',
+            value: '_value_',
+            property: '_property_',
+          },
+        ],
+        [
+          '_category_',
+          'render',
+          {
+            label: 'label2',
+            value: '_value_',
+          },
+        ],
+      ]);
+    });
+  });
+
   describe('tracking mixin', () => {
     describe('trackingOptions', () => {
       it('return the options defined on initialisation', () => {