Implemented suggestions after the FE review

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 = '';

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;