Merge branch '41613-fix-redundant-modal' into 'master'

Make modal dialog common for Groups tree app

Closes #41613

See merge request gitlab-org/gitlab-ce!16311

app/assets/javascripts/groups/components/app.vue

 <script>
 /* global Flash */
 
+import { s__ } from '~/locale';
+import loadingIcon from '~/vue_shared/components/loading_icon.vue';
+import modal from '~/vue_shared/components/modal.vue';
+import { getParameterByName } from '~/lib/utils/common_utils';
+import { mergeUrlParams } from '~/lib/utils/url_utility';
+
 import eventHub from '../event_hub';
-import { getParameterByName } from '../../lib/utils/common_utils';
-import loadingIcon from '../../vue_shared/components/loading_icon.vue';
 import { COMMON_STR } from '../constants';
-import { mergeUrlParams } from '../../lib/utils/url_utility';
 import groupsComponent from './groups.vue';
 
 export default {
   components: {
     loadingIcon,
+    modal,
     groupsComponent,
   },
   props: {
@@ -32,6 +36,10 @@ export default {
       isLoading: true,
       isSearchEmpty: false,
       searchEmptyMessage: '',
+      showModal: false,
+      groupLeaveConfirmationMessage: '',
+      targetGroup: null,
+      targetParentGroup: null,
     };
   },
   computed: {
@@ -48,7 +56,7 @@ export default {
 
     eventHub.$on('fetchPage', this.fetchPage);
     eventHub.$on('toggleChildren', this.toggleChildren);
-    eventHub.$on('leaveGroup', this.leaveGroup);
+    eventHub.$on('showLeaveGroupModal', this.showLeaveGroupModal);
     eventHub.$on('updatePagination', this.updatePagination);
     eventHub.$on('updateGroups', this.updateGroups);
   },
@@ -58,7 +66,7 @@ export default {
   beforeDestroy() {
     eventHub.$off('fetchPage', this.fetchPage);
     eventHub.$off('toggleChildren', this.toggleChildren);
-    eventHub.$off('leaveGroup', this.leaveGroup);
+    eventHub.$off('showLeaveGroupModal', this.showLeaveGroupModal);
     eventHub.$off('updatePagination', this.updatePagination);
     eventHub.$off('updateGroups', this.updateGroups);
   },
@@ -141,14 +149,23 @@ export default {
         parentGroup.isOpen = false;
       }
     },
-    leaveGroup(group, parentGroup) {
-      const targetGroup = group;
-      targetGroup.isBeingRemoved = true;
-      this.service.leaveGroup(targetGroup.leavePath)
+    showLeaveGroupModal(group, parentGroup) {
+      this.targetGroup = group;
+      this.targetParentGroup = parentGroup;
+      this.showModal = true;
+      this.groupLeaveConfirmationMessage = s__(`GroupsTree|Are you sure you want to leave the "${group.fullName}" group?`);
+    },
+    hideLeaveGroupModal() {
+      this.showModal = false;
+    },
+    leaveGroup() {
+      this.showModal = false;
+      this.targetGroup.isBeingRemoved = true;
+      this.service.leaveGroup(this.targetGroup.leavePath)
         .then(res => res.json())
         .then((res) => {
           $.scrollTo(0);
-          this.store.removeGroup(targetGroup, parentGroup);
+          this.store.removeGroup(this.targetGroup, this.targetParentGroup);
           Flash(res.notice, 'notice');
         })
         .catch((err) => {
@@ -157,7 +174,7 @@ export default {
             message = COMMON_STR.LEAVE_FORBIDDEN;
           }
           Flash(message);
-          targetGroup.isBeingRemoved = false;
+          this.targetGroup.isBeingRemoved = false;
         });
     },
     updatePagination(headers) {
@@ -190,5 +207,14 @@ export default {
       :search-empty-message="searchEmptyMessage"
       :page-info="pageInfo"
     />
+    <modal
+      v-show="showModal"
+      :primary-button-label="__('Leave')"
+      kind="warning"
+      :title="__('Are you sure?')"
+      :text="groupLeaveConfirmationMessage"
+      @cancel="hideLeaveGroupModal"
+      @submit="leaveGroup"
+    />
   </div>
 </template>

app/assets/javascripts/groups/components/item_actions.vue

 <script>
-  import { s__ } from '~/locale';
-  import tooltip from '~/vue_shared/directives/tooltip';
-  import icon from '~/vue_shared/components/icon.vue';
-  import modal from '~/vue_shared/components/modal.vue';
-  import eventHub from '../event_hub';
-  import { COMMON_STR } from '../constants';
+import tooltip from '~/vue_shared/directives/tooltip';
+import icon from '~/vue_shared/components/icon.vue';
+import eventHub from '../event_hub';
+import { COMMON_STR } from '../constants';
 
-  export default {
-    components: {
-      icon,
-      modal,
+export default {
+  components: {
+    icon,
+  },
+  directives: {
+    tooltip,
+  },
+  props: {
+    parentGroup: {
+      type: Object,
+      required: false,
+      default: () => ({}),
     },
-    directives: {
-      tooltip,
+    group: {
+      type: Object,
+      required: true,
     },
-    props: {
-      parentGroup: {
-        type: Object,
-        required: false,
-        default: () => ({}),
-      },
-      group: {
-        type: Object,
-        required: true,
-      },
+  },
+  computed: {
+    leaveBtnTitle() {
+      return COMMON_STR.LEAVE_BTN_TITLE;
     },
-    data() {
-      return {
-        modalStatus: false,
-      };
+    editBtnTitle() {
+      return COMMON_STR.EDIT_BTN_TITLE;
     },
-    computed: {
-      leaveBtnTitle() {
-        return COMMON_STR.LEAVE_BTN_TITLE;
-      },
-      editBtnTitle() {
-        return COMMON_STR.EDIT_BTN_TITLE;
-      },
-      leaveConfirmationMessage() {
-        return s__(`GroupsTree|Are you sure you want to leave the "${this.group.fullName}" group?`);
-      },
+  },
+  methods: {
+    onLeaveGroup() {
+      eventHub.$emit('showLeaveGroupModal', this.group, this.parentGroup);
     },
-    methods: {
-      onLeaveGroup() {
-        this.modalStatus = true;
-      },
-      leaveGroup() {
-        this.modalStatus = false;
-        eventHub.$emit('leaveGroup', this.group, this.parentGroup);
-      },
-    },
-  };
+  },
+};
 </script>
 
 <template>
@@ -78,14 +63,5 @@
       class="leave-group btn no-expand">
       <icon name="leave"/>
     </a>
-    <modal
-      v-show="modalStatus"
-      :primary-button-label="__('Leave')"
-      kind="warning"
-      :title="__('Are you sure?')"
-      :text="__('Are you sure you want to leave this group?')"
-      :body="leaveConfirmationMessage"
-      @submit="leaveGroup"
-    />
   </div>
 </template>

changelogs/unreleased/41613-fix-redundant-modal.yml

+---
+title: Make modal dialog common for Groups tree app
+merge_request: 16311
+author:
+type: fixed

spec/javascripts/groups/components/app_spec.js

@@ -256,6 +256,36 @@ describe('AppComponent', () => {
       });
     });
 
+    describe('showLeaveGroupModal', () => {
+      it('caches candidate group (as props) which is to be left', () => {
+        const group = Object.assign({}, mockParentGroupItem);
+        expect(vm.targetGroup).toBe(null);
+        expect(vm.targetParentGroup).toBe(null);
+        vm.showLeaveGroupModal(group, mockParentGroupItem);
+        expect(vm.targetGroup).not.toBe(null);
+        expect(vm.targetParentGroup).not.toBe(null);
+      });
+
+      it('updates props which show modal confirmation dialog', () => {
+        const group = Object.assign({}, mockParentGroupItem);
+        expect(vm.showModal).toBeFalsy();
+        expect(vm.groupLeaveConfirmationMessage).toBe('');
+        vm.showLeaveGroupModal(group, mockParentGroupItem);
+        expect(vm.showModal).toBeTruthy();
+        expect(vm.groupLeaveConfirmationMessage).toBe(`Are you sure you want to leave the "${group.fullName}" group?`);
+      });
+    });
+
+    describe('hideLeaveGroupModal', () => {
+      it('hides modal confirmation which is shown before leaving the group', () => {
+        const group = Object.assign({}, mockParentGroupItem);
+        vm.showLeaveGroupModal(group, mockParentGroupItem);
+        expect(vm.showModal).toBeTruthy();
+        vm.hideLeaveGroupModal();
+        expect(vm.showModal).toBeFalsy();
+      });
+    });
+
     describe('leaveGroup', () => {
       let groupItem;
       let childGroupItem;
@@ -265,21 +295,24 @@ describe('AppComponent', () => {
         groupItem.children = mockChildren;
         childGroupItem = groupItem.children[0];
         groupItem.isChildrenLoading = false;
+        vm.targetGroup = childGroupItem;
+        vm.targetParentGroup = groupItem;
       });
 
-      it('should leave group and remove group item from tree', (done) => {
+      it('hides modal confirmation leave group and remove group item from tree', (done) => {
         const notice = `You left the "${childGroupItem.fullName}" group.`;
         spyOn(vm.service, 'leaveGroup').and.returnValue(returnServicePromise({ notice }));
         spyOn(vm.store, 'removeGroup').and.callThrough();
         spyOn(window, 'Flash');
         spyOn($, 'scrollTo');
 
-        vm.leaveGroup(childGroupItem, groupItem);
-        expect(childGroupItem.isBeingRemoved).toBeTruthy();
-        expect(vm.service.leaveGroup).toHaveBeenCalledWith(childGroupItem.leavePath);
+        vm.leaveGroup();
+        expect(vm.showModal).toBeFalsy();
+        expect(vm.targetGroup.isBeingRemoved).toBeTruthy();
+        expect(vm.service.leaveGroup).toHaveBeenCalledWith(vm.targetGroup.leavePath);
         setTimeout(() => {
           expect($.scrollTo).toHaveBeenCalledWith(0);
-          expect(vm.store.removeGroup).toHaveBeenCalledWith(childGroupItem, groupItem);
+          expect(vm.store.removeGroup).toHaveBeenCalledWith(vm.targetGroup, vm.targetParentGroup);
           expect(window.Flash).toHaveBeenCalledWith(notice, 'notice');
           done();
         }, 0);
@@ -291,13 +324,13 @@ describe('AppComponent', () => {
         spyOn(vm.store, 'removeGroup').and.callThrough();
         spyOn(window, 'Flash');
 
-        vm.leaveGroup(childGroupItem, groupItem);
-        expect(childGroupItem.isBeingRemoved).toBeTruthy();
+        vm.leaveGroup();
+        expect(vm.targetGroup.isBeingRemoved).toBeTruthy();
         expect(vm.service.leaveGroup).toHaveBeenCalledWith(childGroupItem.leavePath);
         setTimeout(() => {
           expect(vm.store.removeGroup).not.toHaveBeenCalled();
           expect(window.Flash).toHaveBeenCalledWith(message);
-          expect(childGroupItem.isBeingRemoved).toBeFalsy();
+          expect(vm.targetGroup.isBeingRemoved).toBeFalsy();
           done();
         }, 0);
       });
@@ -309,12 +342,12 @@ describe('AppComponent', () => {
         spyOn(window, 'Flash');
 
         vm.leaveGroup(childGroupItem, groupItem);
-        expect(childGroupItem.isBeingRemoved).toBeTruthy();
+        expect(vm.targetGroup.isBeingRemoved).toBeTruthy();
         expect(vm.service.leaveGroup).toHaveBeenCalledWith(childGroupItem.leavePath);
         setTimeout(() => {
           expect(vm.store.removeGroup).not.toHaveBeenCalled();
           expect(window.Flash).toHaveBeenCalledWith(message);
-          expect(childGroupItem.isBeingRemoved).toBeFalsy();
+          expect(vm.targetGroup.isBeingRemoved).toBeFalsy();
           done();
         }, 0);
       });
@@ -364,7 +397,7 @@ describe('AppComponent', () => {
       Vue.nextTick(() => {
         expect(eventHub.$on).toHaveBeenCalledWith('fetchPage', jasmine.any(Function));
         expect(eventHub.$on).toHaveBeenCalledWith('toggleChildren', jasmine.any(Function));
-        expect(eventHub.$on).toHaveBeenCalledWith('leaveGroup', jasmine.any(Function));
+        expect(eventHub.$on).toHaveBeenCalledWith('showLeaveGroupModal', jasmine.any(Function));
         expect(eventHub.$on).toHaveBeenCalledWith('updatePagination', jasmine.any(Function));
         expect(eventHub.$on).toHaveBeenCalledWith('updateGroups', jasmine.any(Function));
         newVm.$destroy();
@@ -404,7 +437,7 @@ describe('AppComponent', () => {
       Vue.nextTick(() => {
         expect(eventHub.$off).toHaveBeenCalledWith('fetchPage', jasmine.any(Function));
         expect(eventHub.$off).toHaveBeenCalledWith('toggleChildren', jasmine.any(Function));
-        expect(eventHub.$off).toHaveBeenCalledWith('leaveGroup', jasmine.any(Function));
+        expect(eventHub.$off).toHaveBeenCalledWith('showLeaveGroupModal', jasmine.any(Function));
         expect(eventHub.$off).toHaveBeenCalledWith('updatePagination', jasmine.any(Function));
         expect(eventHub.$off).toHaveBeenCalledWith('updateGroups', jasmine.any(Function));
         done();
@@ -439,5 +472,14 @@ describe('AppComponent', () => {
         done();
       });
     });
+
+    it('renders modal confirmation dialog', () => {
+      vm.groupLeaveConfirmationMessage = 'Are you sure you want to leave the "foo" group?';
+      vm.showModal = true;
+      const modalDialogEl = vm.$el.querySelector('.modal');
+      expect(modalDialogEl).not.toBe(null);
+      expect(modalDialogEl.querySelector('.modal-title').innerText.trim()).toBe('Are you sure?');
+      expect(modalDialogEl.querySelector('.btn.btn-warning').innerText.trim()).toBe('Leave');
+    });
   });
 });

spec/javascripts/groups/components/item_actions_spec.js

@@ -26,32 +26,12 @@ describe('ItemActionsComponent', () => {
     vm.$destroy();
   });
 
-  describe('computed', () => {
-    describe('leaveConfirmationMessage', () => {
-      it('should return appropriate string for leave group confirmation', () => {
-        expect(vm.leaveConfirmationMessage).toBe('Are you sure you want to leave the "platform / hardware" group?');
-      });
-    });
-  });
-
   describe('methods', () => {
     describe('onLeaveGroup', () => {
-      it('should change `modalStatus` prop to `true` which shows confirmation dialog', () => {
-        expect(vm.modalStatus).toBeFalsy();
-        vm.onLeaveGroup();
-        expect(vm.modalStatus).toBeTruthy();
-      });
-    });
-
-    describe('leaveGroup', () => {
-      it('should change `modalStatus` prop to `false` and emit `leaveGroup` event with required params when called with `leaveConfirmed` as `true`', () => {
+      it('emits `showLeaveGroupModal` event with `group` and `parentGroup` props', () => {
         spyOn(eventHub, '$emit');
-        vm.modalStatus = true;
-
-        vm.leaveGroup();
-
-        expect(vm.modalStatus).toBeFalsy();
-        expect(eventHub.$emit).toHaveBeenCalledWith('leaveGroup', vm.group, vm.parentGroup);
+        vm.onLeaveGroup();
+        expect(eventHub.$emit).toHaveBeenCalledWith('showLeaveGroupModal', vm.group, vm.parentGroup);
       });
     });
   });
@@ -72,7 +52,8 @@ describe('ItemActionsComponent', () => {
       expect(editBtn.getAttribute('href')).toBe(group.editPath);
       expect(editBtn.getAttribute('aria-label')).toBe('Edit group');
       expect(editBtn.dataset.originalTitle).toBe('Edit group');
-      expect(editBtn.querySelector('i.fa.fa-cogs')).toBeDefined();
+      expect(editBtn.querySelectorAll('svg use').length).not.toBe(0);
+      expect(editBtn.querySelector('svg use').getAttribute('xlink:href')).toContain('#settings');
 
       newVm.$destroy();
     });
@@ -88,17 +69,10 @@ describe('ItemActionsComponent', () => {
       expect(leaveBtn.getAttribute('href')).toBe(group.leavePath);
       expect(leaveBtn.getAttribute('aria-label')).toBe('Leave this group');
       expect(leaveBtn.dataset.originalTitle).toBe('Leave this group');
-      expect(leaveBtn.querySelector('i.fa.fa-sign-out')).toBeDefined();
+      expect(leaveBtn.querySelectorAll('svg use').length).not.toBe(0);
+      expect(leaveBtn.querySelector('svg use').getAttribute('xlink:href')).toContain('#leave');
 
       newVm.$destroy();
     });
-
-    it('should show modal dialog when `modalStatus` is set to `true`', () => {
-      vm.modalStatus = true;
-      const modalDialogEl = vm.$el.querySelector('.modal');
-      expect(modalDialogEl).toBeDefined();
-      expect(modalDialogEl.querySelector('.modal-title').innerText.trim()).toBe('Are you sure?');
-      expect(modalDialogEl.querySelector('.btn.btn-warning').innerText.trim()).toBe('Leave');
-    });
   });
 });