Merge branch 'fix-ci-delete-variable-bug' into 'master'

Fix delete action bug on CI Variable Modal

See merge request gitlab-org/gitlab!27411

app/assets/javascripts/ci_variable_list/components/ci_variable_modal.vue

@@ -3,6 +3,7 @@ import { __ } from '~/locale';
 import { mapActions, mapState } from 'vuex';
 import { ADD_CI_VARIABLE_MODAL_ID } from '../constants';
 import {
+  GlButton,
   GlModal,
   GlFormSelect,
   GlFormGroup,
@@ -16,6 +17,7 @@ import {
 export default {
   modalId: ADD_CI_VARIABLE_MODAL_ID,
   components: {
+    GlButton,
     GlModal,
     GlFormSelect,
     GlFormGroup,
@@ -66,20 +68,6 @@ export default {
         attributes: { variant: 'success', disabled: !this.canSubmit },
       };
     },
-    deleteAction() {
-      if (this.variableBeingEdited) {
-        return {
-          text: __('Delete variable'),
-          attributes: { variant: 'danger', category: 'secondary' },
-        };
-      }
-      return null;
-    },
-    cancelAction() {
-      return {
-        text: __('Cancel'),
-      };
-    },
     maskedFeedback() {
       return __('This variable can not be masked');
     },
@@ -99,6 +87,7 @@ export default {
       } else {
         this.addVariable();
       }
+      this.hideModal();
     },
     resetModalHandler() {
       if (this.variableBeingEdited) {
@@ -107,20 +96,23 @@ export default {
         this.clearModal();
       }
     },
+    hideModal() {
+      this.$refs.modal.hide();
+    },
+    deleteVarAndClose() {
+      this.deleteVariable(this.variableBeingEdited);
+      this.hideModal();
+    },
   },
 };
 </script>
 
 <template>
   <gl-modal
+    ref="modal"
     :modal-id="$options.modalId"
     :title="modalActionText"
-    :action-primary="primaryAction"
-    :action-secondary="deleteAction"
-    :action-cancel="cancelAction"
-    @ok="updateOrAddVariable"
     @hidden="resetModalHandler"
-    @secondary="deleteVariable(variableBeingEdited)"
   >
     <form>
       <gl-form-group :label="__('Key')" label-for="ci-variable-key">
@@ -210,5 +202,23 @@ export default {
         </gl-form-checkbox>
       </gl-form-group>
     </form>
+    <template #modal-footer>
+      <gl-button @click="hideModal">{{ __('Cancel') }}</gl-button>
+      <gl-button
+        v-if="variableBeingEdited"
+        ref="deleteCiVariable"
+        category="secondary"
+        variant="danger"
+        @click="deleteVarAndClose"
+        >{{ __('Delete variable') }}</gl-button
+      >
+      <gl-button
+        ref="updateOrAddVariable"
+        :disabled="!canSubmit"
+        variant="success"
+        @click="updateOrAddVariable"
+        >{{ modalActionText }}
+      </gl-button>
+    </template>
   </gl-modal>
 </template>

app/controllers/groups/settings/ci_cd_controller.rb

@@ -7,7 +7,7 @@ module Groups
       before_action :authorize_admin_group!
       before_action :authorize_update_max_artifacts_size!, only: [:update]
       before_action do
-        push_frontend_feature_flag(:new_variables_ui, @group)
+        push_frontend_feature_flag(:new_variables_ui, @group, default_enabled: true)
       end
       before_action :define_variables, only: [:show, :create_deploy_token]
 

app/controllers/projects/settings/ci_cd_controller.rb

@@ -6,7 +6,7 @@ module Projects
       before_action :authorize_admin_pipeline!
       before_action :define_variables
       before_action do
-        push_frontend_feature_flag(:new_variables_ui, @project)
+        push_frontend_feature_flag(:new_variables_ui, @project, default_enabled: true)
       end
 
       def show

app/views/ci/variables/_index.html.haml

@@ -5,7 +5,7 @@
     - link_start = '<a href="%{url}">'.html_safe % { url: help_page_path('ci/variables/README', anchor: 'protected-variables') }
     = s_('Environment variables are configured by your administrator to be %{link_start}protected%{link_end} by default').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
 
-- if Feature.enabled?(:new_variables_ui, @project || @group)
+- if Feature.enabled?(:new_variables_ui, @project || @group, default_enabled: true)
   - is_group = !@group.nil?
 
   #js-ci-project-variables{ data: { endpoint: save_endpoint, project_id: @project&.id || '', group: is_group.to_s, maskable_regex: ci_variable_maskable_regex} }

changelogs/unreleased/fix-ci-delete-variable-bug.yml

+---
+title: Update UI for project and group settings CI variables
+merge_request: 27411
+author:
+type: added

spec/frontend/ci_variable_list/components/ci_variable_modal_spec.js

 import Vuex from 'vuex';
 import { createLocalVue, shallowMount } from '@vue/test-utils';
-import { GlModal } from '@gitlab/ui';
+import { GlButton } from '@gitlab/ui';
 import CiVariableModal from '~/ci_variable_list/components/ci_variable_modal.vue';
 import createStore from '~/ci_variable_list/store';
 import mockData from '../services/mock_data';
+import ModalStub from '../stubs';
 
 const localVue = createLocalVue();
 localVue.use(Vuex);
@@ -15,12 +16,23 @@ describe('Ci variable modal', () => {
   const createComponent = () => {
     store = createStore();
     wrapper = shallowMount(CiVariableModal, {
+      stubs: {
+        GlModal: ModalStub,
+      },
       localVue,
       store,
     });
   };
 
-  const findModal = () => wrapper.find(GlModal);
+  const findModal = () => wrapper.find(ModalStub);
+  const addOrUpdateButton = index =>
+    findModal()
+      .findAll(GlButton)
+      .at(index);
+  const deleteVariableButton = () =>
+    findModal()
+      .findAll(GlButton)
+      .at(1);
 
   beforeEach(() => {
     createComponent();
@@ -32,7 +44,7 @@ describe('Ci variable modal', () => {
   });
 
   it('button is disabled when no key/value pair are present', () => {
-    expect(findModal().props('actionPrimary').attributes.disabled).toBeTruthy();
+    expect(addOrUpdateButton(1).attributes('disabled')).toBeTruthy();
   });
 
   describe('Adding a new variable', () => {
@@ -42,11 +54,11 @@ describe('Ci variable modal', () => {
     });
 
     it('button is enabled when key/value pair are present', () => {
-      expect(findModal().props('actionPrimary').attributes.disabled).toBeFalsy();
+      expect(addOrUpdateButton(1).attributes('disabled')).toBeFalsy();
     });
 
     it('Add variable button dispatches addVariable action', () => {
-      findModal().vm.$emit('ok');
+      addOrUpdateButton(1).vm.$emit('click');
       expect(store.dispatch).toHaveBeenCalledWith('addVariable');
     });
 
@@ -63,11 +75,11 @@ describe('Ci variable modal', () => {
     });
 
     it('button text is Update variable when updating', () => {
-      expect(wrapper.vm.modalActionText).toBe('Update variable');
+      expect(addOrUpdateButton(2).text()).toBe('Update variable');
     });
 
     it('Update variable button dispatches updateVariable with correct variable', () => {
-      findModal().vm.$emit('ok');
+      addOrUpdateButton(2).vm.$emit('click');
       expect(store.dispatch).toHaveBeenCalledWith(
         'updateVariable',
         store.state.variableBeingEdited,
@@ -80,7 +92,7 @@ describe('Ci variable modal', () => {
     });
 
     it('dispatches deleteVariable with correct variable to delete', () => {
-      findModal().vm.$emit('secondary');
+      deleteVariableButton().vm.$emit('click');
       expect(store.dispatch).toHaveBeenCalledWith('deleteVariable', mockData.mockVariables[0]);
     });
   });

spec/frontend/ci_variable_list/stubs.js

+const ModalStub = {
+  name: 'glmodal-stub',
+  template: `
+    <div>
+      <slot></slot>
+      <slot name="modal-footer"></slot>
+    </div>
+  `,
+  methods: {
+    hide: jest.fn(),
+  },
+};
+
+export default ModalStub;