Merge branch '9682-disable-remove-primary' into 'master'

Geo Nodes - Conditional Removal Button

Closes #9682

See merge request gitlab-org/gitlab!27836

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

@@ -200,6 +200,9 @@ export default {
     hideNodeActionModal() {
       this.$root.$emit('bv::hide::modal', this.modalId);
     },
+    nodeRemovalAllowed(node) {
+      return !node.primary || this.nodes.length <= 1;
+    },
   },
 };
 </script>
@@ -219,6 +222,7 @@ export default {
       :primary-node="node.primary"
       :node-actions-allowed="nodeActionsAllowed"
       :node-edit-allowed="nodeEditAllowed"
+      :node-removal-allowed="nodeRemovalAllowed(node)"
       :geo-troubleshooting-help-path="geoTroubleshootingHelpPath"
     />
     <gl-modal

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

 <script>
+import { GlButton, GlTooltipDirective } from '@gitlab/ui';
 import { __, s__ } from '~/locale';
 import eventHub from '../event_hub';
 import { NODE_ACTIONS } from '../constants';
@@ -7,6 +8,10 @@ import Icon from '~/vue_shared/components/icon.vue';
 export default {
   components: {
     Icon,
+    GlButton,
+  },
+  directives: {
+    GlTooltip: GlTooltipDirective,
   },
   props: {
     node: {
@@ -21,6 +26,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    nodeRemovalAllowed: {
+      type: Boolean,
+      required: true,
+    },
     nodeMissingOauth: {
       type: Boolean,
       required: true,
@@ -39,6 +48,11 @@ export default {
     isSecondaryNode() {
       return !this.node.primary;
     },
+    disabledRemovalTooltip() {
+      return this.nodeRemovalAllowed
+        ? ''
+        : s__('Geo Nodes|Cannot remove a primary node if there is a secondary node');
+    },
   },
   methods: {
     onToggleNode() {
@@ -83,48 +97,60 @@ export default {
 
 <template>
   <div class="d-flex align-items-center justify-content-end geo-node-actions">
-    <a v-if="isSecondaryNode" :href="node.geoProjectsUrl" class="btn btn-sm mx-1 " target="_blank">
+    <a
+      v-if="isSecondaryNode"
+      :href="node.geoProjectsUrl"
+      class="btn btn-sm mx-1 sm-column-spacing"
+      target="_blank"
+    >
       <icon v-if="!node.current" name="external-link" /> {{ __('Open projects') }}
     </a>
     <template v-if="nodeActionsAllowed">
-      <button
+      <gl-button
         v-if="nodeMissingOauth"
-        type="button"
-        class="btn btn-sm btn-default mx-1"
+        class="btn btn-sm btn-default mx-1 sm-column-spacing"
         @click="onRepairNode"
       >
         {{ s__('Repair authentication') }}
-      </button>
-      <button
+      </gl-button>
+      <gl-button
         v-if="isToggleAllowed"
         :class="{
           'btn-warning': node.enabled,
           'btn-success': !node.enabled,
         }"
-        type="button"
-        class="btn btn-sm mx-1"
+        class="btn btn-sm mx-1 sm-column-spacing"
         @click="onToggleNode"
       >
         <icon :name="nodeToggleIcon" />
         {{ nodeToggleLabel }}
-      </button>
-      <a v-if="nodeEditAllowed" :href="node.editPath" class="btn btn-sm mx-1"> {{ __('Edit') }} </a>
-      <button
+      </gl-button>
+      <a v-if="nodeEditAllowed" :href="node.editPath" class="btn btn-sm mx-1 sm-column-spacing">
+        {{ __('Edit') }}
+      </a>
+      <gl-button
         v-if="isSecondaryNode"
-        type="button"
-        class="btn btn-sm btn-danger mx-1"
+        class="btn btn-sm btn-danger mx-1 sm-column-spacing"
+        :disabled="!nodeRemovalAllowed"
         @click="onRemoveSecondaryNode"
       >
         {{ __('Remove') }}
-      </button>
-      <button
+      </gl-button>
+      <div
+        v-gl-tooltip.hover
+        name="disabledRemovalTooltip"
+        class="mx-1 sm-column-spacing"
+        :title="disabledRemovalTooltip"
+      >
+        <gl-button
           v-if="!isSecondaryNode"
-        type="button"
-        class="btn btn-sm btn-danger mx-1"
+          class="btn btn-sm btn-danger w-100"
+          :disabled="!nodeRemovalAllowed"
           @click="onRemovePrimaryNode"
         >
           {{ __('Remove') }}
-      </button>
+        </gl-button>
+      </div>
     </template>
   </div>
 </template>

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

@@ -33,6 +33,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    nodeRemovalAllowed: {
+      type: Boolean,
+      required: true,
+    },
     geoTroubleshootingHelpPath: {
       type: String,
       required: true,
@@ -72,6 +76,7 @@ export default {
       :node-details="nodeDetails"
       :node-actions-allowed="nodeActionsAllowed"
       :node-edit-allowed="nodeEditAllowed"
+      :node-removal-allowed="nodeRemovalAllowed"
       :version-mismatch="hasVersionMismatch"
     />
     <node-details-section-sync v-if="!node.primary" :node-details="nodeDetails" />

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

@@ -29,6 +29,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    nodeRemovalAllowed: {
+      type: Boolean,
+      required: true,
+    },
     geoTroubleshootingHelpPath: {
       type: String,
       required: true,
@@ -91,6 +95,7 @@ export default {
       :node-details="nodeDetails"
       :node-edit-allowed="nodeEditAllowed"
       :node-actions-allowed="nodeActionsAllowed"
+      :node-removal-allowed="nodeRemovalAllowed"
       :geo-troubleshooting-help-path="geoTroubleshootingHelpPath"
     />
     <div v-if="isNodeDetailsFailed">

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

@@ -18,6 +18,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    nodeRemovalAllowed: {
+      type: Boolean,
+      required: true,
+    },
     geoTroubleshootingHelpPath: {
       type: String,
       required: true,
@@ -35,6 +39,7 @@ export default {
       :primary-node="node.primary"
       :node-actions-allowed="nodeActionsAllowed"
       :node-edit-allowed="nodeEditAllowed"
+      :node-removal-allowed="nodeRemovalAllowed"
       :geo-troubleshooting-help-path="geoTroubleshootingHelpPath"
     />
   </div>

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

@@ -26,6 +26,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    nodeRemovalAllowed: {
+      type: Boolean,
+      required: true,
+    },
     versionMismatch: {
       type: Boolean,
       required: true,
@@ -47,11 +51,21 @@ export default {
 
 <template>
   <div class="row-fluid clearfix py-3 primary-section">
-    <div class="col-md-8">
+    <div class="col-md-12">
+      <div class="d-flex geo-node-actions-container">
         <div class="d-flex flex-column">
           <span class="text-secondary-700 js-node-url-title">{{ s__('GeoNodes|Node URL') }}</span>
           <span class="mt-1 font-weight-bold js-node-url-value">{{ node.url }}</span>
         </div>
+        <geo-node-actions
+          class="flex-grow-1"
+          :node="node"
+          :node-actions-allowed="nodeActionsAllowed"
+          :node-edit-allowed="nodeEditAllowed"
+          :node-removal-allowed="nodeRemovalAllowed"
+          :node-missing-oauth="nodeDetails.missingOAuthApplication"
+        />
+      </div>
       <div class="d-flex flex-column mt-2">
         <span class="text-secondary-700 js-node-version-title">{{
           s__('GeoNodes|GitLab version')
@@ -65,11 +79,5 @@ export default {
       </div>
       <geo-node-health-status :status="nodeHealthStatus" />
     </div>
-    <geo-node-actions
-      :node="node"
-      :node-actions-allowed="nodeActionsAllowed"
-      :node-edit-allowed="nodeEditAllowed"
-      :node-missing-oauth="nodeDetails.missingOAuthApplication"
-    />
   </div>
 </template>

ee/app/assets/stylesheets/pages/geo_nodes.scss

@@ -3,11 +3,15 @@
     flex-direction: column;
     margin: 0 1rem;
 
-    .btn-sm {
+    .sm-column-spacing {
       width: 100%;
       margin-top: 1rem;
     }
   }
+
+  .geo-node-actions-container {
+    flex-direction: column;
+  }
 }
 
 .project-card-errors {

ee/changelogs/unreleased/9682-disable-remove-primary.yml

+---
+title: Disabled Primary Node Removal button when removal is not allowed
+merge_request: 27836
+author:
+type: changed

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

@@ -445,6 +445,31 @@ describe('AppComponent', () => {
         expect(rootEmit).toHaveBeenCalledWith('bv::hide::modal', vm.modalId);
       });
     });
+
+    describe('nodeRemovalAllowed', () => {
+      describe.each`
+        primaryNode | nodesLength | nodeRemovalAllowed
+        ${false}    | ${2}        | ${true}
+        ${false}    | ${1}        | ${true}
+        ${true}     | ${2}        | ${false}
+        ${true}     | ${1}        | ${true}
+      `(
+        'with (primaryNode = $primaryNode, nodesLength = $nodesLength)',
+        ({ primaryNode, nodesLength, nodeRemovalAllowed }) => {
+          const testPhrasing = nodeRemovalAllowed ? 'allow' : 'disallow';
+          let node;
+
+          beforeEach(() => {
+            node = { ...mockNode, primary: primaryNode };
+            vm.store.state.nodes = [mockNode, node].slice(0, nodesLength);
+          });
+
+          it(`should ${testPhrasing} node removal`, () => {
+            expect(vm.nodeRemovalAllowed(node)).toBe(nodeRemovalAllowed);
+          });
+        },
+      );
+    });
   });
 
   describe('created', () => {

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

@@ -12,6 +12,7 @@ const createComponent = (
   node = mockNodes[0],
   nodeEditAllowed = true,
   nodeActionsAllowed = true,
+  nodeRemovalAllowed = true,
   nodeMissingOauth = false,
 ) => {
   const Component = Vue.extend(geoNodeActionsComponent);
@@ -20,6 +21,7 @@ const createComponent = (
     node,
     nodeEditAllowed,
     nodeActionsAllowed,
+    nodeRemovalAllowed,
     nodeMissingOauth,
   });
 };
@@ -65,6 +67,23 @@ describe('GeoNodeActionsComponent', () => {
         vmX.$destroy();
       });
     });
+
+    describe('disabledRemovalTooltip', () => {
+      describe.each`
+        nodeRemovalAllowed | tooltip
+        ${true}            | ${''}
+        ${false}           | ${'Cannot remove a primary node if there is a secondary node'}
+      `('when nodeRemovalAllowed is $nodeRemovalAllowed', ({ nodeRemovalAllowed, tooltip }) => {
+        beforeEach(() => {
+          vm = createComponent(mockNodes[0], true, true, nodeRemovalAllowed, false);
+        });
+
+        it('renders the correct tooltip', () => {
+          const tip = vm.$el.querySelector('div[name=disabledRemovalTooltip]');
+          expect(tip.title).toBe(tooltip);
+        });
+      });
+    });
   });
 
   describe('methods', () => {
@@ -128,5 +147,29 @@ describe('GeoNodeActionsComponent', () => {
       expect(vm.$el.classList.contains('geo-node-actions')).toBe(true);
       expect(vm.$el.querySelectorAll('.btn-sm').length).not.toBe(0);
     });
+
+    describe.each`
+      nodeRemovalAllowed | buttonDisabled
+      ${false}           | ${true}
+      ${true}            | ${false}
+    `(
+      `when nodeRemovalAllowed is $nodeRemovalAllowed`,
+      ({ nodeRemovalAllowed, buttonDisabled }) => {
+        let removeButton;
+
+        beforeEach(() => {
+          vm = createComponent(mockNodes[0], true, true, nodeRemovalAllowed, false);
+          removeButton = vm.$el.querySelector('.btn-danger');
+        });
+
+        it('has the correct button text', () => {
+          expect(removeButton.innerText.trim()).toBe('Remove');
+        });
+
+        it(`the button's disabled attribute should be ${buttonDisabled}`, () => {
+          expect(removeButton.disabled).toBe(buttonDisabled);
+        });
+      },
+    );
   });
 });

ee/spec/frontend/geo_nodes/components/geo_node_details_spec.js

@@ -12,6 +12,7 @@ describe('GeoNodeDetailsComponent', () => {
     nodeDetails: mockNodeDetails,
     nodeActionsAllowed: true,
     nodeEditAllowed: true,
+    nodeRemovalAllowed: true,
     geoTroubleshootingHelpPath: '/foo/bar',
   };
 

ee/spec/frontend/geo_nodes/components/geo_node_item_spec.js

@@ -15,6 +15,7 @@ describe('GeoNodeItemComponent', () => {
     primaryNode: true,
     nodeActionsAllowed: true,
     nodeEditAllowed: true,
+    nodeRemovalAllowed: true,
     geoTroubleshootingHelpPath: '/foo/bar',
   };
 

ee/spec/frontend/geo_nodes/components/geo_nodes_list_spec.js

@@ -11,6 +11,7 @@ const createComponent = () => {
     nodes: mockNodes,
     nodeActionsAllowed: true,
     nodeEditAllowed: true,
+    nodeRemovalAllowed: true,
     geoTroubleshootingHelpPath: '/foo/bar',
   });
 };

ee/spec/frontend/geo_nodes/components/node_detail_sections/node_details_section_main_spec.js

@@ -11,6 +11,7 @@ const createComponent = ({
   nodeDetails = Object.assign({}, mockNodeDetails),
   nodeActionsAllowed = true,
   nodeEditAllowed = true,
+  nodeRemovalAllowed = true,
   versionMismatch = false,
 }) => {
   const Component = Vue.extend(NodeDetailsSectionMainComponent);
@@ -20,6 +21,7 @@ const createComponent = ({
     nodeDetails,
     nodeActionsAllowed,
     nodeEditAllowed,
+    nodeRemovalAllowed,
     versionMismatch,
   });
 };

locale/gitlab.pot

@@ -9144,6 +9144,9 @@ msgstr ""
 msgid "Geo Nodes"
 msgstr ""
 
+msgid "Geo Nodes|Cannot remove a primary node if there is a secondary node"
+msgstr ""
+
 msgid "Geo Settings"
 msgstr ""