Merge branch '435181-add-additional-properties-templates' into 'master'

Add additional internal event properties to template methods

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148297



Merged-by: Robert Hunt <rhunt@gitlab.com>
Approved-by: Sharmad Nachnolkar <snachnolkar@gitlab.com>
Approved-by: Robert Hunt <rhunt@gitlab.com>
Approved-by: Sebastian Rehm <srehm@gitlab.com>
Reviewed-by: Ankit Panchal <apanchal@gitlab.com>
Co-authored-by: Ankit <apanchal@gitlab.com>

app/assets/javascripts/tracking/internal_events.js

@@ -83,9 +83,9 @@ const InternalEvents = {
     const loadEvents = parent.querySelectorAll(LOAD_INTERNAL_EVENTS_SELECTOR);
 
     loadEvents.forEach((element) => {
-      const action = createInternalEventPayload(element);
-      if (action) {
-        this.trackEvent(action);
+      const { event, additionalProperties = {} } = createInternalEventPayload(element);
+      if (event) {
+        this.trackEvent(event, additionalProperties);
       }
     });
 

app/assets/javascripts/tracking/utils.js

@@ -72,9 +72,15 @@ export const createEventPayload = (el, { suffix = '' } = {}) => {
 };
 
 export const createInternalEventPayload = (el) => {
-  const { eventTracking } = el?.dataset || {};
+  const { eventTracking, eventLabel, eventProperty, eventValue } = el?.dataset || {};
 
-  return eventTracking;
+  return {
+    event: eventTracking,
+    additionalProperties: omitBy(
+      { label: eventLabel, property: eventProperty, value: parseInt(eventValue, 10) || undefined },
+      isUndefined,
+    ),
+  };
 };
 
 export const InternalEventHandler = (e, func) => {
@@ -83,9 +89,9 @@ export const InternalEventHandler = (e, func) => {
   if (!el) {
     return;
   }
-  const event = createInternalEventPayload(el);
+  const { event, additionalProperties = {} } = createInternalEventPayload(el);
 
-  func(event);
+  func(event, additionalProperties);
 };
 
 export const eventHandler = (e, func, opts = {}) => {

doc/development/internal_analytics/internal_event_instrumentation/migration.md

@@ -105,7 +105,7 @@ For raw JavaScript:
    });
 ```
 
-If you are using `data-track-action` in the component, you have to change it to `data-event-tracking` to migrate to Internal Events Tracking.
+If you are using `data-track-action` in the component, you have to change it to `data-event-tracking` to migrate to Internal Events Tracking. If there are additional tracking attributes like `data-track-label`, `data-track-property` and `data-track-value` then you can replace them with `data-event-label`, `data-event-property` and `data-event-value` respectively.
 
 For example, if a button is defined like this:
 
@@ -131,6 +131,8 @@ This can be converted to Internal Events Tracking like this:
   :aria-label="externalUrlLabel"
   target="_blank"
   data-event-tracking="click_toggle_external_button"
+  data-event-label="diff_toggle_external_button"
+  data-event-property="diff_toggle_external"
   icon="external-link"
 />
 ```

doc/development/internal_analytics/internal_event_instrumentation/quick_start.md

@@ -199,6 +199,33 @@ import { InternalEvents } from '~/tracking';
 InternalEvents.trackEvent('i_code_review_user_apply_suggestion');
 ```
 
+#### Data-event attribute
+
+This attribute ensures that if we want to track GitLab internal events for a button, we do not need to write JavaScript code on Click handler. Instead, we can just add a data-event-tracking attribute with event value and it should work. This can also be used with HAML views.
+
+```html
+  <gl-button
+    data-event-tracking="i_analytics_dev_ops_adoption"
+  >
+   Click Me
+  </gl-button>
+```
+
+#### Haml
+
+```haml
+= render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle',  data: { event_tracking: 'action' }}) do
+```
+
+#### Internal events on render
+
+Sometimes we want to send internal events when the component is rendered or loaded. In these cases, we can add the `data-event-tracking-load="true"` attribute:
+
+```haml
+= render Pajamas::ButtonComponent.new(button_options: { data: { event_tracking_load: 'true', event_tracking: 'i_devops' } }) do
+        = _("New project")
+```
+
 #### Additional properties
 
 Additional properties can be passed when tracking events. They can be used to save additional data related to given event. It is possible to send a maximum of three additional properties with keys `label` (string), `property` (string) and `value`(numeric).
@@ -223,29 +250,20 @@ For raw JavaScript:
    });
 ```
 
-#### Data-track attribute
+For data-event attributes:
 
-This attribute ensures that if we want to track GitLab internal events for a button, we do not need to write JavaScript code on Click handler. Instead, we can just add a data-event-tracking attribute with event value and it should work. This can also be used with HAML views.
-
-```html
-  <gl-button
+```javascript
+ <gl-button
     data-event-tracking="i_analytics_dev_ops_adoption"
+    data-event-label="gitlab_devops_button_label"
+    data-event-property="nav_core_menu"
   >
    Click Me
   </gl-button>
 ```
 
-#### Haml
+For Haml:
 
 ```haml
-= render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle',  data: { event_tracking: 'action' }}) do
-```
-
-#### Internal events on render
-
-Sometimes we want to send internal events when the component is rendered or loaded. In these cases, we can add the `data-event-tracking-load="true"` attribute:
-
-```haml
-= render Pajamas::ButtonComponent.new(button_options: { data: { event_tracking_load: 'true', event_tracking: 'i_devops' } }) do
-        = _("New project")
+= render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle',  data: { event_tracking: 'action', event_label: 'gitlab_settings_button_label', event_property: 'settings_menu', event_value: 2 }}) do
 ```

spec/frontend/tracking/internal_events_spec.js

@@ -213,7 +213,32 @@ describe('InternalEvents', () => {
         querySelectorAllMock.mockReturnValue(mockElements);
 
         const result = InternalEvents.trackInternalLoadEvents();
-        expect(trackEventSpy).toHaveBeenCalledWith(action);
+        expect(trackEventSpy).toHaveBeenCalledWith(action, {});
+        expect(trackEventSpy).toHaveBeenCalledTimes(1);
+        expect(querySelectorAllMock).toHaveBeenCalledWith(LOAD_INTERNAL_EVENTS_SELECTOR);
+        expect(result).toEqual(mockElements);
+      });
+
+      it('should track event along with additional Properties if action exists', () => {
+        mockElements = [
+          {
+            dataset: {
+              eventTracking: action,
+              eventTrackingLoad: true,
+              eventProperty: 'test-property',
+              eventLabel: 'test-label',
+              eventValue: 2,
+            },
+          },
+        ];
+        querySelectorAllMock.mockReturnValue(mockElements);
+
+        const result = InternalEvents.trackInternalLoadEvents();
+        expect(trackEventSpy).toHaveBeenCalledWith(action, {
+          label: 'test-label',
+          property: 'test-property',
+          value: 2,
+        });
         expect(trackEventSpy).toHaveBeenCalledTimes(1);
         expect(querySelectorAllMock).toHaveBeenCalledWith(LOAD_INTERNAL_EVENTS_SELECTOR);
         expect(result).toEqual(mockElements);

spec/frontend/tracking/utils_spec.js

@@ -103,7 +103,22 @@ describe('~/tracking/utils', () => {
       it('should return event name from element', () => {
         const mockEl = { dataset: { eventTracking: 'click' } };
         const result = createInternalEventPayload(mockEl);
-        expect(result).toEqual('click');
+        expect(result).toEqual({ additionalProperties: {}, event: 'click' });
+      });
+      it('should return event and additional Properties from element', () => {
+        const mockEl = {
+          dataset: {
+            eventTracking: 'click',
+            eventProperty: 'test-property',
+            eventLabel: 'test-label',
+            eventValue: 2,
+          },
+        };
+        const result = createInternalEventPayload(mockEl);
+        expect(result).toEqual({
+          additionalProperties: { property: 'test-property', label: 'test-label', value: 2 },
+          event: 'click',
+        });
       });
     });
 
@@ -127,7 +142,7 @@ describe('~/tracking/utils', () => {
         InternalEventHandler(mockEvent, mockFunc);
 
         if (shouldCallFunc) {
-          expect(mockFunc).toHaveBeenCalledWith(payload);
+          expect(mockFunc).toHaveBeenCalledWith(payload, {});
         } else {
           expect(mockFunc).not.toHaveBeenCalled();
         }