Merge branch '37503-improve-removal-modal' into 'master'

Replace depricatedModal (Geo) with GlModal

Closes #37503

See merge request gitlab-org/gitlab!22125

ee/app/assets/javascripts/geo_nodes/components/app.vue

 <script>
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlLoadingIcon, GlModal } from '@gitlab/ui';
 import { __, s__ } from '~/locale';
 import Flash from '~/flash';
-import DeprecatedModal from '~/vue_shared/components/deprecated_modal.vue';
 import SmartInterval from '~/smart_interval';
 
 import eventHub from '../event_hub';
@@ -13,7 +12,7 @@ import GeoNodeItem from './geo_node_item.vue';
 
 export default {
   components: {
-    DeprecatedModal,
+    GlModal,
     GeoNodeItem,
     GlLoadingIcon,
   },
@@ -43,12 +42,13 @@ export default {
     return {
       isLoading: true,
       hasError: false,
-      showModal: false,
       targetNode: null,
       targetNodeActionType: '',
+      modalTitle: '',
       modalKind: 'warning',
       modalMessage: '',
       modalActionLabel: '',
+      modalId: 'node-action',
     };
   },
   computed: {
@@ -168,7 +168,7 @@ export default {
         });
     },
     handleNodeAction() {
-      this.showModal = false;
+      this.hideNodeActionModal();
 
       if (this.targetNodeActionType === NODE_ACTIONS.TOGGLE) {
         this.toggleNode(this.targetNode);
@@ -182,21 +182,23 @@ export default {
       modalKind = 'warning',
       modalMessage,
       modalActionLabel,
+      modalTitle,
     }) {
       this.targetNode = node;
       this.targetNodeActionType = actionType;
       this.modalKind = modalKind;
       this.modalMessage = modalMessage;
       this.modalActionLabel = modalActionLabel;
+      this.modalTitle = modalTitle;
 
       if (actionType === NODE_ACTIONS.TOGGLE && !node.enabled) {
         this.toggleNode(this.targetNode);
       } else {
-        this.showModal = true;
+        this.$root.$emit('bv::show::modal', this.modalId);
       }
     },
     hideNodeActionModal() {
-      this.showModal = false;
+      this.$root.$emit('bv::hide::modal', this.modalId);
     },
   },
 };
@@ -219,14 +221,17 @@ export default {
       :node-edit-allowed="nodeEditAllowed"
       :geo-troubleshooting-help-path="geoTroubleshootingHelpPath"
     />
-    <deprecated-modal
-      v-show="showModal"
-      :title="__('Are you sure?')"
-      :kind="modalKind"
-      :text="modalMessage"
-      :primary-button-label="modalActionLabel"
+    <gl-modal
+      :modal-id="modalId"
+      :title="modalTitle"
+      :ok-variant="modalKind"
+      :ok-title="modalActionLabel"
       @cancel="hideNodeActionModal"
-      @submit="handleNodeAction"
-    />
+      @ok="handleNodeAction"
+    >
+      <template>
+        {{ modalMessage }}
+      </template>
+    </gl-modal>
   </div>
 </template>

ee/app/assets/javascripts/geo_nodes/components/geo_node_actions.vue

@@ -45,8 +45,9 @@ export default {
       eventHub.$emit('showNodeActionModal', {
         actionType: NODE_ACTIONS.TOGGLE,
         node: this.node,
-        modalMessage: s__('GeoNodes|Pausing replication stops the sync process.'),
+        modalMessage: s__('GeoNodes|Pausing replication stops the sync process. Are you sure?'),
         modalActionLabel: this.nodeToggleLabel,
+        modalTitle: __('Pause replication'),
       });
     },
     onRemoveSecondaryNode() {
@@ -55,9 +56,10 @@ export default {
         node: this.node,
         modalKind: 'danger',
         modalMessage: s__(
-          'GeoNodes|Removing a secondary node stops the sync process. It is not currently possible to add back the same node without losing some data. We only recommend setting up a new secondary node in this case. Are you sure?',
+          'GeoNodes|Removing a Geo secondary node stops the synchronization to that node. Are you sure?',
         ),
-        modalActionLabel: __('Remove'),
+        modalActionLabel: __('Remove node'),
+        modalTitle: __('Remove secondary node'),
       });
     },
     onRemovePrimaryNode() {
@@ -66,9 +68,10 @@ export default {
         node: this.node,
         modalKind: 'danger',
         modalMessage: s__(
-          'GeoNodes|Removing a primary node stops the sync process for all nodes. Syncing cannot be resumed without losing some data on all secondaries. In this case we would recommend setting up all nodes from scratch. Are you sure?',
+          'GeoNodes|Removing a Geo primary node stops the synchronization to that node. Are you sure?',
         ),
-        modalActionLabel: __('Remove'),
+        modalActionLabel: __('Remove node'),
+        modalTitle: __('Remove primary node'),
       });
     },
     onRepairNode() {

ee/changelogs/unreleased/37503-improve-removal-modal.yml

+---
+title: Replace depricatedModal (Geo) with GlModal
+merge_request: 22125
+author:
+type: changed

ee/spec/javascripts/geo_nodes/components/app_spec.js

@@ -57,12 +57,13 @@ describe('AppComponent', () => {
     it('returns default data props', () => {
       expect(vm.isLoading).toBe(true);
       expect(vm.hasError).toBe(false);
-      expect(vm.showModal).toBe(false);
       expect(vm.targetNode).toBeNull();
       expect(vm.targetNodeActionType).toBe('');
       expect(vm.modalKind).toBe('warning');
       expect(vm.modalMessage).toBe('');
       expect(vm.modalActionLabel).toBe('');
+      expect(vm.modalTitle).toBe('');
+      expect(vm.modalId).toBe('node-action');
     });
   });
 
@@ -330,27 +331,27 @@ describe('AppComponent', () => {
     });
 
     describe('handleNodeAction', () => {
-      it('sets `showModal` to false and calls `toggleNode` when `targetNodeActionType` is `toggle`', () => {
+      it('calls `toggleNode` and `hideNodeActionModal` when `targetNodeActionType` is `toggle`', () => {
         vm.targetNode = { ...mockNode };
         vm.targetNodeActionType = NODE_ACTIONS.TOGGLE;
-        vm.showModal = true;
+        spyOn(vm, 'hideNodeActionModal');
         spyOn(vm, 'toggleNode').and.stub();
 
         vm.handleNodeAction();
 
-        expect(vm.showModal).toBe(false);
+        expect(vm.hideNodeActionModal).toHaveBeenCalled();
         expect(vm.toggleNode).toHaveBeenCalledWith(vm.targetNode);
       });
 
-      it('sets `showModal` to false and calls `removeNode` when `targetNodeActionType` is `remove`', () => {
+      it('calls `removeNode` and `hideNodeActionModal` when `targetNodeActionType` is `remove`', () => {
         vm.targetNode = { ...mockNode };
         vm.targetNodeActionType = NODE_ACTIONS.REMOVE;
-        vm.showModal = true;
+        spyOn(vm, 'hideNodeActionModal');
         spyOn(vm, 'removeNode').and.stub();
 
         vm.handleNodeAction();
 
-        expect(vm.showModal).toBe(false);
+        expect(vm.hideNodeActionModal).toHaveBeenCalled();
         expect(vm.removeNode).toHaveBeenCalledWith(vm.targetNode);
       });
     });
@@ -360,12 +361,16 @@ describe('AppComponent', () => {
       let modalKind;
       let modalMessage;
       let modalActionLabel;
+      let modalTitle;
+      let rootEmit;
 
       beforeEach(() => {
         node = { ...mockNode };
         modalKind = 'warning';
         modalMessage = 'Foobar message';
         modalActionLabel = 'Disable';
+        modalTitle = 'Test title';
+        rootEmit = spyOn(vm.$root, '$emit');
       });
 
       it('sets target node and modal config props on component', () => {
@@ -375,6 +380,7 @@ describe('AppComponent', () => {
           modalKind,
           modalMessage,
           modalActionLabel,
+          modalTitle,
         });
 
         expect(vm.targetNode).toBe(node);
@@ -382,9 +388,10 @@ describe('AppComponent', () => {
         expect(vm.modalKind).toBe(modalKind);
         expect(vm.modalMessage).toBe(modalMessage);
         expect(vm.modalActionLabel).toBe(modalActionLabel);
+        expect(vm.modalTitle).toBe(modalTitle);
       });
 
-      it('sets showModal to `true` when actionType is `toggle` and node is enabled', () => {
+      it('emits `bv::show::modal` when actionType is `toggle` and node is enabled', () => {
         node.enabled = true;
         vm.showNodeActionModal({
           actionType: NODE_ACTIONS.TOGGLE,
@@ -392,9 +399,10 @@ describe('AppComponent', () => {
           modalKind,
           modalMessage,
           modalActionLabel,
+          modalTitle,
         });
 
-        expect(vm.showModal).toBe(true);
+        expect(rootEmit).toHaveBeenCalledWith('bv::show::modal', vm.modalId);
       });
 
       it('calls toggleNode when actionType is `toggle` and node.enabled is `false`', () => {
@@ -407,12 +415,13 @@ describe('AppComponent', () => {
           modalKind,
           modalMessage,
           modalActionLabel,
+          modalTitle,
         });
 
         expect(vm.toggleNode).toHaveBeenCalledWith(vm.targetNode);
       });
 
-      it('sets showModal to `true` when actionType is not `toggle`', () => {
+      it('emits `bv::show::modal` when actionType is not `toggle`', () => {
         node.enabled = true;
         vm.showNodeActionModal({
           actionType: NODE_ACTIONS.REMOVE,
@@ -422,16 +431,16 @@ describe('AppComponent', () => {
           modalActionLabel,
         });
 
-        expect(vm.showModal).toBe(true);
+        expect(rootEmit).toHaveBeenCalledWith('bv::show::modal', vm.modalId);
       });
     });
 
     describe('hideNodeActionModal', () => {
-      it('sets `showModal` to `false`', () => {
-        vm.showModal = true;
+      it('emits `bv::hide::modal`', () => {
+        const rootEmit = spyOn(vm.$root, '$emit');
         vm.hideNodeActionModal();
 
-        expect(vm.showModal).toBe(false);
+        expect(rootEmit).toHaveBeenCalledWith('bv::hide::modal', vm.modalId);
       });
     });
   });

ee/spec/javascripts/geo_nodes/components/geo_node_actions_spec.js

@@ -67,21 +67,22 @@ describe('GeoNodeActionsComponent', () => {
 
   describe('methods', () => {
     describe('onToggleNode', () => {
-      it('emits showNodeActionModal with actionType `toggle`, node reference, modalMessage and modalActionLabel', () => {
+      it('emits showNodeActionModal with actionType `toggle`, node reference, modalMessage, modalActionLabel, and modalTitle', () => {
         spyOn(eventHub, '$emit');
         vm.onToggleNode();
 
         expect(eventHub.$emit).toHaveBeenCalledWith('showNodeActionModal', {
           actionType: NODE_ACTIONS.TOGGLE,
           node: vm.node,
-          modalMessage: 'Pausing replication stops the sync process.',
+          modalMessage: 'Pausing replication stops the sync process. Are you sure?',
           modalActionLabel: vm.nodeToggleLabel,
+          modalTitle: 'Pause replication',
         });
       });
     });
 
     describe('onRemovePrimaryNode', () => {
-      it('emits showNodeActionModal with actionType `remove`, node reference, modalKind, modalMessage and modalActionLabel', () => {
+      it('emits showNodeActionModal with actionType `remove`, node reference, modalKind, modalMessage, modalActionLabel, and modalTitle', () => {
         spyOn(eventHub, '$emit');
         vm.onRemovePrimaryNode();
 
@@ -90,14 +91,15 @@ describe('GeoNodeActionsComponent', () => {
           node: vm.node,
           modalKind: 'danger',
           modalMessage:
-            'Removing a primary node stops the sync process for all nodes. Syncing cannot be resumed without losing some data on all secondaries. In this case we would recommend setting up all nodes from scratch. Are you sure?',
-          modalActionLabel: 'Remove',
+            'Removing a Geo primary node stops the synchronization to that node. Are you sure?',
+          modalActionLabel: 'Remove node',
+          modalTitle: 'Remove primary node',
         });
       });
     });
 
     describe('onRemoveSecondaryNode', () => {
-      it('emits showNodeActionModal with actionType `remove`, node reference, modalKind, modalMessage and modalActionLabel', () => {
+      it('emits showNodeActionModal with actionType `remove`, node reference, modalKind, modalMessage, modalActionLabel, and modalTitle', () => {
         spyOn(eventHub, '$emit');
         vm.onRemoveSecondaryNode();
 
@@ -106,8 +108,9 @@ describe('GeoNodeActionsComponent', () => {
           node: vm.node,
           modalKind: 'danger',
           modalMessage:
-            'Removing a secondary node stops the sync process. It is not currently possible to add back the same node without losing some data. We only recommend setting up a new secondary node in this case. Are you sure?',
-          modalActionLabel: 'Remove',
+            'Removing a Geo secondary node stops the synchronization to that node. Are you sure?',
+          modalActionLabel: 'Remove node',
+          modalTitle: 'Remove secondary node',
         });
       });
     });

locale/gitlab.pot

@@ -8209,13 +8209,13 @@ msgstr ""
 msgid "GeoNodes|Out of sync"
 msgstr ""
 
-msgid "GeoNodes|Pausing replication stops the sync process."
+msgid "GeoNodes|Pausing replication stops the sync process. Are you sure?"
 msgstr ""
 
-msgid "GeoNodes|Removing a primary node stops the sync process for all nodes. Syncing cannot be resumed without losing some data on all secondaries. In this case we would recommend setting up all nodes from scratch. Are you sure?"
+msgid "GeoNodes|Removing a Geo primary node stops the synchronization to that node. Are you sure?"
 msgstr ""
 
-msgid "GeoNodes|Removing a secondary node stops the sync process. It is not currently possible to add back the same node without losing some data. We only recommend setting up a new secondary node in this case. Are you sure?"
+msgid "GeoNodes|Removing a Geo secondary node stops the synchronization to that node. Are you sure?"
 msgstr ""
 
 msgid "GeoNodes|Replication slot WAL"
@@ -14961,15 +14961,24 @@ msgstr ""
 msgid "Remove milestone"
 msgstr ""
 
+msgid "Remove node"
+msgstr ""
+
 msgid "Remove parent epic from an epic"
 msgstr ""
 
+msgid "Remove primary node"
+msgstr ""
+
 msgid "Remove priority"
 msgstr ""
 
 msgid "Remove project"
 msgstr ""
 
+msgid "Remove secondary node"
+msgstr ""
+
 msgid "Remove spent time"
 msgstr ""