Merge branch '213252-hide-primary-checkbox' into 'master'

Better visualize type of node on Geo Form

Closes #213252

See merge request gitlab-org/gitlab!31784

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

 <script>
+import { GlBadge } from '@gitlab/ui';
 import { __ } from '~/locale';
 import GeoNodeForm from './geo_node_form.vue';
 
@@ -6,6 +7,7 @@ export default {
   name: 'GeoNodeFormApp',
   components: {
     GeoNodeForm,
+    GlBadge,
   },
   props: {
     selectiveSyncTypes: {
@@ -23,16 +25,32 @@ export default {
     },
   },
   computed: {
+    isNodePrimary() {
+      return this.node && this.node.primary;
+    },
     pageTitle() {
       return this.node ? __('Edit Geo Node') : __('New Geo Node');
     },
+    pillDetails() {
+      return {
+        variant: this.isNodePrimary ? 'primary' : 'light',
+        label: this.isNodePrimary ? __('Primary') : __('Secondary'),
+      };
+    },
   },
 };
 </script>
 
 <template>
   <article class="geo-node-form-container">
+    <div class="gl-display-flex gl-align-items-center">
       <h3 class="page-title">{{ pageTitle }}</h3>
+      <gl-badge
+        class="rounded-pill gl-font-sm gl-px-3 gl-py-2 gl-ml-3"
+        :variant="pillDetails.variant"
+        >{{ pillDetails.label }}</gl-badge
+      >
+    </div>
     <geo-node-form v-bind="$props" />
   </article>
 </template>

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

@@ -81,11 +81,6 @@ export default {
   <form>
     <geo-node-form-core :node-data="nodeData" />
     <section class="mt-3 pl-0 col-sm-6">
-      <gl-form-group>
-        <gl-form-checkbox id="node-primary-field" v-model="nodeData.primary">{{
-          __('This is a primary node')
-        }}</gl-form-checkbox>
-      </gl-form-group>
       <gl-form-group
         v-if="nodeData.primary"
         :label="__('Internal URL (optional)')"

ee/changelogs/unreleased/213252-hide-primary-checkbox.yml

+---
+title: Geo - Better visualize type of node on form
+merge_request: 31784
+author:
+type: changed

ee/spec/features/admin/geo/admin_geo_nodes_spec.rb

@@ -82,88 +82,44 @@ describe 'admin Geo Nodes', :js, :geo do
     end
   end
 
-  describe 'create a new Geo Nodes' do
-    let(:new_ssh_key) { attributes_for(:key)[:key] }
-
-    before do
-      visit new_admin_geo_node_path
-    end
-
-    it 'creates a new Geo Node' do
-      check 'node-primary-field'
-      fill_in 'node-name-field', with: 'a node name'
-      fill_in 'node-url-field', with: 'https://test.gitlab.com'
-      click_button 'Save'
-
-      wait_for_requests
-      expect(current_path).to eq admin_geo_nodes_path
-
-      page.within(find('.card', match: :first)) do
-        expect(page).to have_content(geo_node.url)
-      end
-    end
-
-    context 'toggles the visibility of secondary only params based on primary node checkbox' do
+  describe 'node form fields' do
     primary_only_fields = %w(node-internal-url-field node-reverification-interval-field)
     secondary_only_fields = %w(node-selective-synchronization-field node-repository-capacity-field node-file-capacity-field node-object-storage-field)
 
-      context 'by default' do
-        it 'node primary field is unchecked' do
-          expect(page).to have_unchecked_field('node-primary-field')
-        end
-
-        it 'renders no primary fields' do
-          expect_no_fields(primary_only_fields)
-        end
-
-        it 'renders all secondary fields' do
-          expect_fields(secondary_only_fields)
-        end
-      end
-
-      context 'when node primary field gets checked' do
-        before do
-          check 'node-primary-field'
-        end
+    it 'when primary renders only primary fields' do
+      geo_node.update(primary: true)
+      visit edit_admin_geo_node_path(geo_node)
 
-        it 'renders all primary fields' do
       expect_fields(primary_only_fields)
-        end
-
-        it 'renders no secondary fields' do
       expect_no_fields(secondary_only_fields)
     end
-      end
 
-      context 'when node primary field gets unchecked' do
-        before do
-          uncheck 'node-primary-field'
-        end
+    it 'when secondary renders only secondary fields' do
+      geo_node.update(primary: false)
+      visit edit_admin_geo_node_path(geo_node)
 
-        it 'renders no primary fields' do
       expect_no_fields(primary_only_fields)
-        end
-
-        it 'renders all secondary fields' do
       expect_fields(secondary_only_fields)
     end
   end
-    end
 
-    context 'with an existing primary node' do
+  describe 'create a new Geo Nodes' do
+    let(:new_ssh_key) { attributes_for(:key)[:key] }
+
     before do
-        create(:geo_node, :primary)
+      visit new_admin_geo_node_path
     end
 
-      it 'returns an error message when a another primary is attempted to be added' do
-        check 'node-primary-field'
-        fill_in 'node-url-field', with: 'https://another-primary.example.com'
+    it 'creates a new Geo Node' do
+      fill_in 'node-name-field', with: 'a node name'
+      fill_in 'node-url-field', with: 'https://test.gitlab.com'
       click_button 'Save'
 
       wait_for_requests
-        expect(current_path).to eq new_admin_geo_node_path
+      expect(current_path).to eq admin_geo_nodes_path
 
-        expect(page).to have_content(/There was an error saving this Geo Node.*primary node already exists/)
+      page.within(find('.card', match: :first)) do
+        expect(page).to have_content(geo_node.url)
       end
     end
   end
@@ -176,7 +132,6 @@ describe 'admin Geo Nodes', :js, :geo do
 
       fill_in 'node-url-field', with: 'http://newsite.com'
       fill_in 'node-internal-url-field', with: 'http://internal-url.com'
-      check 'node-primary-field'
       click_button 'Update'
 
       wait_for_requests

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

 import { shallowMount } from '@vue/test-utils';
+import { GlBadge } from '@gitlab/ui';
 import GeoNodeFormApp from 'ee/geo_node_form/components/app.vue';
 import GeoNodeForm from 'ee/geo_node_form/components/geo_node_form.vue';
-import { MOCK_SELECTIVE_SYNC_TYPES, MOCK_SYNC_SHARDS, MOCK_NODE } from '../mock_data';
+import { MOCK_SELECTIVE_SYNC_TYPES, MOCK_SYNC_SHARDS } from '../mock_data';
 
 describe('GeoNodeFormApp', () => {
   let wrapper;
@@ -23,6 +24,7 @@ describe('GeoNodeFormApp', () => {
   });
 
   const findGeoNodeFormTitle = () => wrapper.find('.page-title');
+  const findGeoNodeFormBadge = () => wrapper.find(GlBadge);
   const findGeoForm = () => wrapper.find(GeoNodeForm);
 
   describe('render', () => {
@@ -30,35 +32,34 @@ describe('GeoNodeFormApp', () => {
       createComponent();
     });
 
-    it('the Geo Node Form Title', () => {
-      expect(findGeoNodeFormTitle().exists()).toBe(true);
+    describe.each`
+      formType                     | node                  | title              | pillTitle      | variant
+      ${'create a secondary node'} | ${null}               | ${'New Geo Node'}  | ${'Secondary'} | ${'light'}
+      ${'update a secondary node'} | ${{ primary: false }} | ${'Edit Geo Node'} | ${'Secondary'} | ${'light'}
+      ${'update a primary node'}   | ${{ primary: true }}  | ${'Edit Geo Node'} | ${'Primary'}   | ${'primary'}
+    `(`form header`, ({ formType, node, title, pillTitle, variant }) => {
+      describe(`when node form is to ${formType}`, () => {
+        beforeEach(() => {
+          propsData.node = node;
+          createComponent();
         });
 
-    it('the Geo Node Form', () => {
-      expect(findGeoForm().exists()).toBe(true);
-    });
+        it(`sets the node form title to ${title}`, () => {
+          expect(findGeoNodeFormTitle().text()).toBe(title);
         });
 
-  describe('Geo Node Form Title', () => {
-    describe('when props.node is undefined', () => {
-      beforeEach(() => {
-        createComponent();
+        it(`sets the node form pill title to ${pillTitle}`, () => {
+          expect(findGeoNodeFormBadge().text()).toBe(pillTitle);
         });
 
-      it('sets title to `New Geo Node`', () => {
-        expect(findGeoNodeFormTitle().text()).toBe('New Geo Node');
+        it(`sets the node form pill variant to be ${variant}`, () => {
+          expect(findGeoNodeFormBadge().attributes('variant')).toBe(variant);
         });
       });
-
-    describe('when props.node is set', () => {
-      beforeEach(() => {
-        propsData.node = MOCK_NODE;
-        createComponent();
     });
 
-      it('sets title to `Edit Geo Node`', () => {
-        expect(findGeoNodeFormTitle().text()).toBe('Edit Geo Node');
-      });
+    it('the Geo Node Form', () => {
+      expect(findGeoForm().exists()).toBe(true);
     });
   });
 });

ee/spec/frontend/geo_node_form/components/geo_node_form_spec.js

@@ -29,7 +29,6 @@ describe('GeoNodeForm', () => {
   });
 
   const findGeoNodeFormCoreField = () => wrapper.find(GeoNodeFormCore);
-  const findGeoNodePrimaryField = () => wrapper.find('#node-primary-field');
   const findGeoNodeInternalUrlField = () => wrapper.find('#node-internal-url-field');
   const findGeoNodeFormCapacitiesField = () => wrapper.find(GeoNodeFormCapacities);
   const findGeoNodeObjectStorageField = () => wrapper.find('#node-object-storage-field');
@@ -42,19 +41,12 @@ describe('GeoNodeForm', () => {
     });
 
     describe.each`
-      primaryNode | showCore | showPrimary | showInternalUrl | showCapacities | showObjectStorage
-      ${true}     | ${true}  | ${true}     | ${true}         | ${true}        | ${false}
-      ${false}    | ${true}  | ${true}     | ${false}        | ${true}        | ${true}
+      primaryNode | showCore | showInternalUrl | showCapacities | showObjectStorage
+      ${true}     | ${true}  | ${true}         | ${true}        | ${false}
+      ${false}    | ${true}  | ${false}        | ${true}        | ${true}
     `(
       `conditional fields`,
-      ({
-        primaryNode,
-        showCore,
-        showPrimary,
-        showInternalUrl,
-        showCapacities,
-        showObjectStorage,
-      }) => {
+      ({ primaryNode, showCore, showInternalUrl, showCapacities, showObjectStorage }) => {
         beforeEach(() => {
           wrapper.setData({
             nodeData: { ...wrapper.vm.nodeData, primary: primaryNode },
@@ -65,10 +57,6 @@ describe('GeoNodeForm', () => {
           expect(findGeoNodeFormCoreField().exists()).toBe(showCore);
         });
 
-        it(`it ${showPrimary ? 'shows' : 'hides'} the Primary Field`, () => {
-          expect(findGeoNodePrimaryField().exists()).toBe(showPrimary);
-        });
-
         it(`it ${showInternalUrl ? 'shows' : 'hides'} the Internal URL Field`, () => {
           expect(findGeoNodeInternalUrlField().exists()).toBe(showInternalUrl);
         });

locale/gitlab.pot

@@ -18591,6 +18591,9 @@ msgstr ""
 msgid "Seats in license"
 msgstr ""
 
+msgid "Secondary"
+msgstr ""
+
 msgid "Secret"
 msgstr ""
 
@@ -21728,9 +21731,6 @@ msgstr ""
 msgid "This is a list of devices that have logged into your account. Revoke any sessions that you do not recognize."
 msgstr ""
 
-msgid "This is a primary node"
-msgstr ""
-
 msgid "This is a security log of important events involving your account."
 msgstr ""