Merge branch '338231-fix-geo-removal-logic' into 'master'

Geo 2.0 Regression - Add ability to remove primary

See merge request gitlab-org/gitlab!68005

doc/administration/geo/replication/disable_geo.md

@@ -35,6 +35,7 @@ to do that.
 
 To remove the **primary** site:
 
+1. [Remove all secondary Geo sites](remove_geo_site.md)
 1. On the top bar, select **Menu >** **{admin}** **Admin**.
 1. On the left sidebar, select **Geo > Nodes**.
 1. Select **Remove** for the **primary** node.

doc/administration/geo/replication/version_specific_updates.md

@@ -11,6 +11,16 @@ Review this page for update instructions for your version. These steps
 accompany the [general steps](updating_the_geo_nodes.md#general-update-steps)
 for updating Geo nodes.
 
+## Updating to GitLab 14.0/14.1
+
+We found an issue where [Primary sites can not be removed from the UI](https://gitlab.com/gitlab-org/gitlab/-/issues/338231).
+
+This bug only exists in the UI and does not block the removal of Primary sites using any other method.
+
+### If you have already updated to an affected version and need to remove your Primary site
+
+You can manually remove the Primary site by using the [Geo Nodes API](../../../api/geo_nodes.md#delete-a-geo-node).
+
 ## Updating to GitLab 13.12
 
 We found an issue where [secondary nodes re-download all LFS files](https://gitlab.com/gitlab-org/gitlab/-/issues/334550) upon update. This bug:

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

@@ -14,9 +14,9 @@ export default {
       'Geo|With GitLab Geo, you can install a special read-only and replicated instance anywhere. %{linkStart}Learn more%{linkEnd}',
     ),
     addSite: s__('Geo|Add site'),
-    modalTitle: s__('Geo|Remove secondary node'),
+    modalTitle: s__('Geo|Remove node'),
     modalBody: s__(
-      'Geo|Removing a Geo secondary node stops the synchronization to that node. Are you sure?',
+      'Geo|Removing a Geo node stops the synchronization to and from that node. Are you sure?',
     ),
     primarySite: s__('Geo|Primary site'),
     secondarySite: s__('Geo|Secondary site'),

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

 <script>
 import { GlButton } from '@gitlab/ui';
+import { mapGetters } from 'vuex';
 import { __ } from '~/locale';
 
 export default {
@@ -17,6 +18,9 @@ export default {
       required: true,
     },
   },
+  computed: {
+    ...mapGetters(['canRemoveNode']),
+  },
 };
 </script>
 
@@ -28,7 +32,7 @@ export default {
     <gl-button
       variant="danger"
       category="secondary"
-      :disabled="node.primary"
+      :disabled="!canRemoveNode(node.id)"
       data-testid="geo-desktop-remove-action"
       @click="$emit('remove')"
       >{{ $options.i18n.removeButtonLabel }}</gl-button

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

 <script>
 import { GlDropdown, GlDropdownItem, GlIcon } from '@gitlab/ui';
+import { mapGetters } from 'vuex';
 import { __ } from '~/locale';
 
 export default {
@@ -20,8 +21,9 @@ export default {
     },
   },
   computed: {
+    ...mapGetters(['canRemoveNode']),
     dropdownRemoveClass() {
-      return this.node.primary ? 'gl-text-gray-400' : 'gl-text-red-500';
+      return this.canRemoveNode(this.node.id) ? 'gl-text-red-500' : 'gl-text-gray-400';
     },
   },
 };
@@ -34,7 +36,7 @@ export default {
     </template>
     <gl-dropdown-item :href="node.webEditUrl">{{ $options.i18n.editButtonLabel }}</gl-dropdown-item>
     <gl-dropdown-item
-      :disabled="node.primary"
+      :disabled="!canRemoveNode(node.id)"
       data-testid="geo-mobile-remove-action"
       @click="$emit('remove')"
     >

ee/app/assets/javascripts/geo_nodes/store/getters.js

@@ -53,3 +53,9 @@ export const syncInfo = (state) => (id) => {
     };
   });
 };
+
+export const canRemoveNode = (state) => (id) => {
+  const node = state.nodes.find((n) => n.id === id);
+
+  return !node.primary || state.nodes.length === 1;
+};

ee/spec/frontend/geo_nodes/components/header/geo_node_actions_desktop_spec.js

 import { GlButton } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
+import Vue from 'vue';
+import Vuex from 'vuex';
 import GeoNodeActionsDesktop from 'ee/geo_nodes/components/header/geo_node_actions_desktop.vue';
 import { MOCK_NODES } from 'ee_jest/geo_nodes/mock_data';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
+Vue.use(Vuex);
+
 describe('GeoNodeActionsDesktop', () => {
   let wrapper;
 
@@ -11,9 +15,17 @@ describe('GeoNodeActionsDesktop', () => {
     node: MOCK_NODES[0],
   };
 
-  const createComponent = (props) => {
+  const createComponent = (props, getters) => {
+    const store = new Vuex.Store({
+      getters: {
+        canRemoveNode: () => () => true,
+        ...getters,
+      },
+    });
+
     wrapper = extendedWrapper(
       shallowMount(GeoNodeActionsDesktop, {
+        store,
         propsData: {
           ...defaultProps,
           ...props,
@@ -56,16 +68,16 @@ describe('GeoNodeActionsDesktop', () => {
     });
 
     describe.each`
-      primary  | disabled
-      ${true}  | ${'true'}
-      ${false} | ${undefined}
-    `(`conditionally`, ({ primary, disabled }) => {
+      canRemoveNode | disabled
+      ${false}      | ${'true'}
+      ${true}       | ${undefined}
+    `(`conditionally`, ({ canRemoveNode, disabled }) => {
       beforeEach(() => {
-        createComponent({ node: { primary } });
+        createComponent({}, { canRemoveNode: () => () => canRemoveNode });
       });
 
-      describe(`when primary is ${primary}`, () => {
-        it(`does ${primary ? '' : 'not '}disable the Desktop Remove button`, () => {
+      describe(`when canRemoveNode is ${canRemoveNode}`, () => {
+        it(`does ${canRemoveNode ? 'not ' : ''}disable the Desktop Remove button`, () => {
           expect(findGeoDesktopActionsRemoveButton().attributes('disabled')).toBe(disabled);
         });
       });

ee/spec/frontend/geo_nodes/components/header/geo_node_actions_mobile_spec.js

 import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
+import Vue from 'vue';
+import Vuex from 'vuex';
 import GeoNodeActionsMobile from 'ee/geo_nodes/components/header/geo_node_actions_mobile.vue';
 import { MOCK_NODES } from 'ee_jest/geo_nodes/mock_data';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 
+Vue.use(Vuex);
+
 describe('GeoNodeActionsMobile', () => {
   let wrapper;
 
@@ -11,9 +15,17 @@ describe('GeoNodeActionsMobile', () => {
     node: MOCK_NODES[0],
   };
 
-  const createComponent = (props) => {
+  const createComponent = (props, getters) => {
+    const store = new Vuex.Store({
+      getters: {
+        canRemoveNode: () => () => true,
+        ...getters,
+      },
+    });
+
     wrapper = extendedWrapper(
       shallowMount(GeoNodeActionsMobile, {
+        store,
         propsData: {
           ...defaultProps,
           ...props,
@@ -62,17 +74,17 @@ describe('GeoNodeActionsMobile', () => {
     });
 
     describe.each`
-      primary  | disabled     | dropdownClass
-      ${true}  | ${'true'}    | ${'gl-text-gray-400'}
-      ${false} | ${undefined} | ${'gl-text-red-500'}
-    `(`conditionally`, ({ primary, disabled, dropdownClass }) => {
+      canRemoveNode | disabled     | dropdownClass
+      ${false}      | ${'true'}    | ${'gl-text-gray-400'}
+      ${true}       | ${undefined} | ${'gl-text-red-500'}
+    `(`conditionally`, ({ canRemoveNode, disabled, dropdownClass }) => {
       beforeEach(() => {
-        createComponent({ node: { primary } });
+        createComponent({}, { canRemoveNode: () => () => canRemoveNode });
       });
 
-      describe(`when primary is ${primary}`, () => {
+      describe(`when canRemoveNode is ${canRemoveNode}`, () => {
         it(`does ${
-          primary ? '' : 'not '
+          canRemoveNode ? 'not ' : ''
         }disable the Mobile Remove dropdown item and adds proper class`, () => {
           expect(findGeoMobileActionsRemoveDropdownItem().attributes('disabled')).toBe(disabled);
           expect(findGeoMobileActionsRemoveDropdownItem().find('span').classes(dropdownClass)).toBe(

ee/spec/frontend/geo_nodes/store/getters_spec.js

@@ -51,4 +51,22 @@ describe('GeoNodes Store Getters', () => {
       expect(getters.syncInfo(state)(MOCK_NODES[1].id)).toStrictEqual(MOCK_SECONDARY_SYNC_INFO);
     });
   });
+
+  describe.each`
+    nodeToRemove     | nodes              | canRemove
+    ${MOCK_NODES[0]} | ${[MOCK_NODES[0]]} | ${true}
+    ${MOCK_NODES[0]} | ${MOCK_NODES}      | ${false}
+    ${MOCK_NODES[1]} | ${[MOCK_NODES[1]]} | ${true}
+    ${MOCK_NODES[1]} | ${MOCK_NODES}      | ${true}
+  `(`canRemoveNode`, ({ nodeToRemove, nodes, canRemove }) => {
+    describe(`when node.primary ${nodeToRemove.primary} and total nodes is ${nodes.length}`, () => {
+      beforeEach(() => {
+        state.nodes = nodes;
+      });
+
+      it(`should return ${canRemove}`, () => {
+        expect(getters.canRemoveNode(state)(nodeToRemove.id)).toBe(canRemove);
+      });
+    });
+  });
 });

locale/gitlab.pot

@@ -14966,13 +14966,10 @@ msgstr ""
 msgid "Geo|Remove node"
 msgstr ""
 
-msgid "Geo|Remove secondary node"
-msgstr ""
-
 msgid "Geo|Remove tracking database entry"
 msgstr ""
 
-msgid "Geo|Removing a Geo secondary node stops the synchronization to that node. Are you sure?"
+msgid "Geo|Removing a Geo node stops the synchronization to and from that node. Are you sure?"
 msgstr ""
 
 msgid "Geo|Replicated data is verified with the secondary node(s) using checksums"