diff --git a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue index d0f3915ba117de46cb292e60fd4eeedb4dd3c6fe..e630ce71bd383191425e24ae255cda647891fc24 100644 --- a/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue +++ b/app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue @@ -209,21 +209,26 @@ export default { } }, trackVariableValidationErrors() { - if (this.variable.secret_value?.length && !this.validationErrorEventProperty) { + const property = this.getTrackingErrorProperty(); + if (!this.validationErrorEventProperty && property) { + this.track(EVENT_ACTION, { property }); + this.validationErrorEventProperty = property; + } + }, + getTrackingErrorProperty() { + let property; + if (this.variable.secret_value?.length && !property) { if (this.displayMaskedError && this.maskableRegex?.length) { const supportedChars = this.maskableRegex.replace('^', '').replace(/{(\d,)}\$/, ''); const regex = new RegExp(supportedChars, 'g'); - - const error = this.variable.secret_value.replace(regex, ''); - - this.track(EVENT_ACTION, { property: error }); - this.validationErrorEventProperty = error; + property = this.variable.secret_value.replace(regex, ''); } if (this.containsVariableReference) { - this.track(EVENT_ACTION, { property: '$' }); - this.validationErrorEventProperty = '$'; + property = '$'; } } + + return property; }, resetValidationErrorEvents() { this.validationErrorEventProperty = ''; diff --git a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js index 52e29c792eda098b232e418fef778d7dfeb52e16..7c4ff67feb3eb65d02bc4592a7b3d9c809d20a2b 100644 --- a/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js @@ -128,10 +128,10 @@ describe('Ci variable modal', () => { }); describe.each` - value | secret | rendered | event_sent - ${'value'} | ${'secret_value'} | ${false} | ${0} - ${'dollar$ign'} | ${'dollar$ign'} | ${true} | ${1} - `('Adding a new variable', ({ value, secret, rendered, event_sent }) => { + value | secret | rendered + ${'value'} | ${'secret_value'} | ${false} + ${'dollar$ign'} | ${'dollar$ign'} | ${true} + `('Adding a new variable', ({ value, secret, rendered }) => { beforeEach(() => { const [variable] = mockData.mockVariables; const invalidKeyVariable = { @@ -149,10 +149,6 @@ describe('Ci variable modal', () => { const warning = wrapper.find(`[data-testid='contains-variable-reference']`); expect(warning.exists()).toBe(rendered); }); - - it(`${rendered ? 'sends' : 'does not send'} the variable reference tracking event`, () => { - expect(trackingSpy).toHaveBeenCalledTimes(event_sent); - }); }); describe('Editing a variable', () => { @@ -254,6 +250,43 @@ describe('Ci variable modal', () => { }); }); + describe.each` + value | secret | masked | eventSent | trackingErrorProperty + ${'value'} | ${'secretValue'} | ${false} | ${0} | ${null} + ${'shortMasked'} | ${'short'} | ${true} | ${0} | ${null} + ${'withDollar$Sign'} | ${'dollar$ign'} | ${false} | ${1} | ${'$'} + ${'withDollar$Sign'} | ${'dollar$ign'} | ${true} | ${1} | ${'$'} + ${'unsupported'} | ${'unsupported|char'} | ${true} | ${1} | ${'|'} + ${'unsupportedMasked'} | ${'unsupported|char'} | ${false} | ${0} | ${null} + `('Adding a new variable', ({ value, secret, masked, eventSent, trackingErrorProperty }) => { + beforeEach(() => { + const [variable] = mockData.mockVariables; + const invalidKeyVariable = { + ...variable, + key: 'key', + value, + secret_value: secret, + masked, + }; + createComponent(mount); + store.state.variable = invalidKeyVariable; + trackingSpy = mockTracking(undefined, wrapper.element, jest.spyOn); + }); + + it(`${ + eventSent > 0 ? 'sends the correct' : 'does not send the' + } variable validation tracking event`, () => { + expect(trackingSpy).toHaveBeenCalledTimes(eventSent); + + if (eventSent > 0) { + expect(trackingSpy).toHaveBeenCalledWith(undefined, EVENT_ACTION, { + label: EVENT_LABEL, + property: trackingErrorProperty, + }); + } + }); + }); + describe('when both states are valid', () => { beforeEach(() => { const [variable] = mockData.mockVariables;