From 188e155b06294d2adc770804ffd078fa6e31f459 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Caplette?= <fcaplette@gitlab.com>
Date: Tue, 23 Aug 2022 09:18:16 -0400
Subject: [PATCH] Fix dropdown in CI/CD variables not showing the current type

We were using the wrong constants in the new CI/CD variable dropdown
which resulted in the currently selected type not to be shown. This
commit now make use of the right constant. We are also adding tests.
---
 .../components/ci_variable_modal.vue            | 14 +++++++++-----
 .../javascripts/ci_variable_list/constants.js   | 10 +++++-----
 .../components/ci_variable_modal_spec.js        | 17 +++++++++++++++++
 spec/frontend/ci_variable_list/mocks.js         |  6 +++---
 4 files changed, 34 insertions(+), 13 deletions(-)

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 5ba63de8c96306..1ae06773ea86b5 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
@@ -108,7 +108,6 @@ export default {
     return {
       newEnvironments: [],
       isTipDismissed: getCookie(AWS_TIP_DISMISSED_COOKIE_NAME) === 'true',
-      typeOptions: variableOptions,
       validationErrorEventProperty: '',
       variable: { ...defaultVariableState, ...this.selectedVariable },
     };
@@ -259,6 +258,7 @@ export default {
     },
   },
   defaultScope: allEnvironments.text,
+  variableOptions,
 };
 </script>
 
@@ -298,16 +298,20 @@ export default {
         />
       </gl-form-group>
 
-      <div class="d-flex">
-        <gl-form-group :label="__('Type')" label-for="ci-variable-type" class="w-50 gl-mr-5">
+      <div class="gl-display-flex">
+        <gl-form-group :label="__('Type')" label-for="ci-variable-type" class="gl-w-half gl-mr-5">
           <gl-form-select
             id="ci-variable-type"
             v-model="variable.variableType"
-            :options="typeOptions"
+            :options="$options.variableOptions"
           />
         </gl-form-group>
 
-        <gl-form-group label-for="ci-variable-env" class="w-50" data-testid="environment-scope">
+        <gl-form-group
+          label-for="ci-variable-env"
+          class="gl-w-half"
+          data-testid="environment-scope"
+        >
           <template #label>
             {{ __('Environment scope') }}
             <gl-link
diff --git a/app/assets/javascripts/ci_variable_list/constants.js b/app/assets/javascripts/ci_variable_list/constants.js
index 5d22974ffbbf9d..e2dd28cdaa17ff 100644
--- a/app/assets/javascripts/ci_variable_list/constants.js
+++ b/app/assets/javascripts/ci_variable_list/constants.js
@@ -10,7 +10,7 @@ export const displayText = {
 };
 
 export const variableTypes = {
-  variableType: 'ENV_VAR',
+  envType: 'ENV_VAR',
   fileType: 'FILE',
 };
 
@@ -29,13 +29,13 @@ export const allEnvironments = {
 export const variableText = {
   [types.variableType]: __('Variable'),
   [types.fileType]: __('File'),
-  [variableTypes.variableType]: __('Variable'),
+  [variableTypes.envType]: __('Variable'),
   [variableTypes.fileType]: __('File'),
 };
 
 export const variableOptions = [
-  { value: types.variableType, text: variableText[types.variableType] },
-  { value: types.fileType, text: variableText[types.fileType] },
+  { value: variableTypes.envType, text: variableText[variableTypes.envType] },
+  { value: variableTypes.fileType, text: variableText[variableTypes.fileType] },
 ];
 
 export const defaultVariableState = {
@@ -44,7 +44,7 @@ export const defaultVariableState = {
   masked: false,
   protected: false,
   value: '',
-  variableType: types.variableType,
+  variableType: variableTypes.envType,
 };
 
 // eslint-disable-next-line @gitlab/require-i18n-strings
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 e5019e3261eeb9..94a1c5e16e72be 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
@@ -11,6 +11,7 @@ import {
   EVENT_ACTION,
   ENVIRONMENT_SCOPE_LINK_TITLE,
   instanceString,
+  variableOptions,
 } from '~/ci_variable_list/constants';
 import { mockVariablesWithScopes } from '../mocks';
 import ModalStub from '../stubs';
@@ -72,6 +73,7 @@ describe('Ci variable modal', () => {
   const findValueField = () => wrapper.find('#ci-variable-value');
   const findEnvScopeLink = () => wrapper.findByTestId('environment-scope-link');
   const findEnvScopeInput = () => wrapper.findByTestId('environment-scope').find(GlFormInput);
+  const findVariableTypeDropdown = () => wrapper.find('#ci-variable-type');
 
   afterEach(() => {
     wrapper.destroy();
@@ -284,6 +286,21 @@ describe('Ci variable modal', () => {
     });
   });
 
+  describe('variable type dropdown', () => {
+    describe('default behaviour', () => {
+      beforeEach(() => {
+        createComponent({ mountFn: mountExtended });
+      });
+
+      it('adds each option as a dropdown item', () => {
+        expect(findVariableTypeDropdown().findAll('option')).toHaveLength(variableOptions.length);
+        variableOptions.forEach((v) => {
+          expect(findVariableTypeDropdown().text()).toContain(v.text);
+        });
+      });
+    });
+  });
+
   describe('Validations', () => {
     const maskError = 'This variable can not be masked.';
 
diff --git a/spec/frontend/ci_variable_list/mocks.js b/spec/frontend/ci_variable_list/mocks.js
index b030005cb8d830..6d633c8b7400a4 100644
--- a/spec/frontend/ci_variable_list/mocks.js
+++ b/spec/frontend/ci_variable_list/mocks.js
@@ -16,8 +16,8 @@ export const mockVariables = (kind) => {
       key: 'my-var',
       masked: false,
       protected: true,
-      value: 'env_val',
-      variableType: variableTypes.variableType,
+      value: 'variable_value',
+      variableType: variableTypes.envType,
     },
     {
       __typename: `Ci${kind}Variable`,
@@ -25,7 +25,7 @@ export const mockVariables = (kind) => {
       key: 'secret',
       masked: true,
       protected: false,
-      value: 'the_secret_value',
+      value: 'another_value',
       variableType: variableTypes.fileType,
     },
   ];
-- 
GitLab