Geo Nodes - Conditional Removal

A primary node is unable to be removed if
a secondary node exists.  However,
the UI doesn't reflect this.

This MR adds logic to disable the
remove button on a Primary node
in the UI if there are additional
nodes.

A tooltip will pop up on the
disabled button with an explanation
why it is disabled.

This MR also moves up the node actions
to be inline with the Node URL.

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
-        v-if="!isSecondaryNode"
-        type="button"
-        class="btn btn-sm btn-danger mx-1"
-        @click="onRemovePrimaryNode"
+      </gl-button>
+      <div
+        v-gl-tooltip.hover
+        name="disabledRemovalTooltip"
+        class="mx-1 sm-column-spacing"
+        :title="disabledRemovalTooltip"
       >
-        {{ __('Remove') }}
-      </button>
+        <gl-button
+          v-if="!isSecondaryNode"
+          class="btn btn-sm btn-danger w-100"
+          :disabled="!nodeRemovalAllowed"
+          @click="onRemovePrimaryNode"
+        >
+          {{ __('Remove') }}
+        </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,10 +51,20 @@ export default {
 
 <template>
   <div class="row-fluid clearfix py-3 primary-section">
-    <div class="col-md-8">
-      <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 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">{{
@@ -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

@@ -9100,6 +9100,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 ""