diff --git a/app/assets/javascripts/settings_panels.js b/app/assets/javascripts/settings_panels.js index 0bb28ba4ebdc4c76d931cb36d7e6bc93ea663fbd..0167864d1d550a2dd28b1241b62325fcb922ccf8 100644 --- a/app/assets/javascripts/settings_panels.js +++ b/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(), }); } diff --git a/app/assets/javascripts/tracking/internal_events.js b/app/assets/javascripts/tracking/internal_events.js index dc3885fe276c12410fb569b753e60a3450db69d3..ae442455c0046914a77d013e7a5cb14fe5db5c31 100644 --- a/app/assets/javascripts/tracking/internal_events.js +++ b/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); }, }, }; diff --git a/doc/development/internal_analytics/internal_event_instrumentation/migration.md b/doc/development/internal_analytics/internal_event_instrumentation/migration.md index 0d06509e5f655de4c8b82169c412e450098fdd1f..74f2d82527b8460169f7dc4084958bb982205b95 100644 --- a/doc/development/internal_analytics/internal_event_instrumentation/migration.md +++ b/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 diff --git a/doc/development/internal_analytics/internal_event_instrumentation/quick_start.md b/doc/development/internal_analytics/internal_event_instrumentation/quick_start.md index e092ffe2d0de517ce553d53a95e96c5aa925e3fe..02a902d941d2f8e2b32eb250ca1562607c4118c8 100644 --- a/doc/development/internal_analytics/internal_event_instrumentation/quick_start.md +++ b/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 diff --git a/spec/frontend/tracking/internal_events_spec.js b/spec/frontend/tracking/internal_events_spec.js index d72ef4086f51c4d8a44eac73813c87aa040fe5fe..7d773733e3c353629e299d8048a038d375b58c55 100644 --- a/spec/frontend/tracking/internal_events_spec.js +++ b/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, + ); }); });