Merge branch '434505-fix-order-of-arguments' into 'master'

Update Order of arguments in trackEvent method

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



Merged-by: Phil Hughes <me@iamphill.com>
Approved-by: Sebastian Rehm <srehm@gitlab.com>
Approved-by: Angelo Gulina <agulina@gitlab.com>
Approved-by: Phil Hughes <me@iamphill.com>
Reviewed-by: Angelo Gulina <agulina@gitlab.com>
Reviewed-by: Ankit Panchal <apanchal@gitlab.com>
Co-authored-by: Ankit <apanchal@gitlab.com>

app/assets/javascripts/settings_panels.js

@@ -26,7 +26,7 @@ export function expandSection(sectionArg) {
       .one('animationend.animateSection', () => $section.removeClass('animating'));
   }
 
-  InternalEvents.trackEvent('click_expand_panel_on_settings', undefined, {
+  InternalEvents.trackEvent('click_expand_panel_on_settings', {
     label: $section.find('.settings-title').text(),
   });
 }

app/assets/javascripts/tracking/internal_events.js

@@ -13,14 +13,14 @@ const InternalEvents = {
   /**
    *
    * @param {string} event
+   * @param {Object} additionalProperties - Object containing additional data for the event tracking.
+   * Supports `value`(number), `property`(string), and `label`(string) as keys.
    * @param {string} category - The category of the event. This is optional and
    * defaults to the page name where the event was triggered. It's advised not to use
    * this parameter for new events unless absolutely necessary.
-   * @param {Object} additionalProperties - Object containing additional data for the event tracking.
-   * Supports `value`(number), `property`(string), and `label`(string) as keys.
    *
    */
-  trackEvent(event, category = undefined, additionalProperties = {}) {
+  trackEvent(event, additionalProperties = {}, category = undefined) {
     validateAdditionalProperties(additionalProperties);
 
     API.trackInternalEvent(event, additionalProperties);
@@ -43,8 +43,8 @@ const InternalEvents = {
   mixin() {
     return {
       methods: {
-        trackEvent(event, category = undefined, additionalProperties = {}) {
-          InternalEvents.trackEvent(event, category, additionalProperties);
+        trackEvent(event, additionalProperties = {}, category = undefined) {
+          InternalEvents.trackEvent(event, additionalProperties, category);
         },
       },
     };

doc/development/internal_analytics/internal_event_instrumentation/migration.md

@@ -76,10 +76,10 @@ import { InternalEvents } from '~/tracking';
 mixins: [InternalEvents.mixin()]
 ...
 ...
-this.trackEvent('action', 'category')
+this.trackEvent('action', {}, 'category')
 ```
 
-If you are currently passing `category` and need to keep it, it can be passed as the second argument in the `trackEvent` method, as illustrated in the previous example. Nonetheless, it is strongly advised against using the `category` parameter for new events. This is because, by default, the category field is populated with information about where the event was triggered.
+If you are currently passing `category` and need to keep it, it can be passed as the third argument in the `trackEvent` method, as illustrated in the previous example. Nonetheless, it is strongly advised against using the `category` parameter for new events. This is because, by default, the category field is populated with information about where the event was triggered.
 
 You can use [this MR](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/123901/diffs) as an example. It migrates the `devops_adoption_app` component to use Internal Events Tracking.
 
@@ -88,7 +88,7 @@ If you are using `label`, `value`, and `property` in Snowplow tracking, you can
 For Vue Mixin:
 
 ```javascript
-   this.trackEvent('i_code_review_user_apply_suggestion', undefined, {
+   this.trackEvent('i_code_review_user_apply_suggestion', {
     label: 'push_event',
     property: 'golang',
     value: 20
@@ -98,7 +98,7 @@ For Vue Mixin:
 For raw JavaScript:
 
 ```javascript
-   InternalEvents.trackEvent('i_code_review_user_apply_suggestion', undefined, {
+   InternalEvents.trackEvent('i_code_review_user_apply_suggestion', {
     label: 'admin',
     property: 'system',
     value: 20

doc/development/internal_analytics/internal_event_instrumentation/quick_start.md

@@ -196,7 +196,7 @@ Additional properties can be passed when tracking events. They can be used to sa
 For Vue Mixin:
 
 ```javascript
-   this.trackEvent('i_code_review_user_apply_suggestion', undefined, {
+   this.trackEvent('i_code_review_user_apply_suggestion', {
     label: 'push_event',
     property: 'golang',
     value: 20
@@ -206,7 +206,7 @@ For Vue Mixin:
 For raw JavaScript:
 
 ```javascript
-   InternalEvents.trackEvent('i_code_review_user_apply_suggestion', undefined, {
+   InternalEvents.trackEvent('i_code_review_user_apply_suggestion', {
     label: 'admin',
     property: 'system',
     value: 20

spec/frontend/tracking/internal_events_spec.js

@@ -30,7 +30,7 @@ describe('InternalEvents', () => {
     const category = 'TestCategory';
 
     it('trackEvent calls API.trackInternalEvent with correct arguments', () => {
-      InternalEvents.trackEvent(event, category);
+      InternalEvents.trackEvent(event, {}, category);
 
       expect(API.trackInternalEvent).toHaveBeenCalledTimes(1);
       expect(API.trackInternalEvent).toHaveBeenCalledWith(event, {});
@@ -39,7 +39,7 @@ describe('InternalEvents', () => {
     it('trackEvent calls Tracking.event with correct arguments including category', () => {
       jest.spyOn(Tracking, 'event').mockImplementation(() => {});
 
-      InternalEvents.trackEvent(event, category);
+      InternalEvents.trackEvent(event, {}, category);
 
       expect(Tracking.event).toHaveBeenCalledWith(category, event, expect.any(Object));
     });
@@ -47,7 +47,7 @@ describe('InternalEvents', () => {
     it('trackEvent calls Tracking.event with event name, category and additional properties', () => {
       jest.spyOn(Tracking, 'event').mockImplementation(() => {});
 
-      InternalEvents.trackEvent(event, category, allowedAdditionalProps);
+      InternalEvents.trackEvent(event, allowedAdditionalProps, category);
 
       expect(Tracking.event).toHaveBeenCalledWith(
         category,
@@ -73,7 +73,7 @@ describe('InternalEvents', () => {
     it('trackEvent calls trackBrowserSDK with event name and additional Properties', () => {
       jest.spyOn(InternalEvents, 'trackBrowserSDK').mockImplementation(() => {});
 
-      InternalEvents.trackEvent(event, undefined, allowedAdditionalProps);
+      InternalEvents.trackEvent(event, allowedAdditionalProps);
 
       expect(InternalEvents.trackBrowserSDK).toHaveBeenCalledTimes(1);
       expect(InternalEvents.trackBrowserSDK).toHaveBeenCalledWith(event, {
@@ -93,7 +93,7 @@ describe('InternalEvents', () => {
       };
 
       expect(() => {
-        InternalEvents.trackEvent(event, category, additionalProps);
+        InternalEvents.trackEvent(event, additionalProps, category);
       }).toThrow(/Disallowed additional properties were provided:/);
 
       expect(InternalEvents.trackBrowserSDK).not.toHaveBeenCalled();
@@ -116,7 +116,7 @@ describe('InternalEvents', () => {
           this.trackEvent(event);
         },
         handleButton2Click() {
-          this.trackEvent(event, undefined, {
+          this.trackEvent(event, {
             property: 'value',
             label: 'value',
             value: 2,
@@ -136,7 +136,7 @@ describe('InternalEvents', () => {
       await wrapper.findByTestId('button').trigger('click');
 
       expect(trackEventSpy).toHaveBeenCalledTimes(1);
-      expect(trackEventSpy).toHaveBeenCalledWith(event, undefined, {});
+      expect(trackEventSpy).toHaveBeenCalledWith(event, {}, undefined);
     });
 
     it('this.trackEvent function calls InternalEvent`s track function with an event and additional Properties', async () => {
@@ -145,11 +145,15 @@ describe('InternalEvents', () => {
       await wrapper.findByTestId('button2').trigger('click');
 
       expect(trackEventSpy).toHaveBeenCalledTimes(1);
-      expect(trackEventSpy).toHaveBeenCalledWith(event, undefined, {
-        property: 'value',
-        label: 'value',
-        value: 2,
-      });
+      expect(trackEventSpy).toHaveBeenCalledWith(
+        event,
+        {
+          property: 'value',
+          label: 'value',
+          value: 2,
+        },
+        undefined,
+      );
     });
   });