Commit 34cafd8a authored by Zack Cuddy's avatar Zack Cuddy

Better visualize type of node on Geo Form

This MR adds a pill at the top of the Geo
Form that identifies what type of node
you are creating/updating.

Also, creating Primary nodes from the UI
is no longer supported.  This change
also removes the redundant Primary
node checkbox.
parent c911a72d
<script> <script>
import { GlBadge } from '@gitlab/ui';
import { __ } from '~/locale'; import { __ } from '~/locale';
import GeoNodeForm from './geo_node_form.vue'; import GeoNodeForm from './geo_node_form.vue';
...@@ -6,6 +7,7 @@ export default { ...@@ -6,6 +7,7 @@ export default {
name: 'GeoNodeFormApp', name: 'GeoNodeFormApp',
components: { components: {
GeoNodeForm, GeoNodeForm,
GlBadge,
}, },
props: { props: {
selectiveSyncTypes: { selectiveSyncTypes: {
...@@ -23,16 +25,32 @@ export default { ...@@ -23,16 +25,32 @@ export default {
}, },
}, },
computed: { computed: {
isNodePrimary() {
return this.node && this.node.primary;
},
pageTitle() { pageTitle() {
return this.node ? __('Edit Geo Node') : __('New Geo Node'); return this.node ? __('Edit Geo Node') : __('New Geo Node');
}, },
pillDetails() {
return {
variant: this.isNodePrimary ? 'primary' : 'light',
label: this.isNodePrimary ? __('Primary') : __('Secondary'),
};
},
}, },
}; };
</script> </script>
<template> <template>
<article class="geo-node-form-container"> <article class="geo-node-form-container">
<h3 class="page-title">{{ pageTitle }}</h3> <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" /> <geo-node-form v-bind="$props" />
</article> </article>
</template> </template>
...@@ -81,11 +81,6 @@ export default { ...@@ -81,11 +81,6 @@ export default {
<form> <form>
<geo-node-form-core :node-data="nodeData" /> <geo-node-form-core :node-data="nodeData" />
<section class="mt-3 pl-0 col-sm-6"> <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 <gl-form-group
v-if="nodeData.primary" v-if="nodeData.primary"
:label="__('Internal URL (optional)')" :label="__('Internal URL (optional)')"
......
---
title: Geo - Better visualize type of node on form
merge_request: 31784
author:
type: changed
...@@ -82,6 +82,27 @@ describe 'admin Geo Nodes', :js, :geo do ...@@ -82,6 +82,27 @@ describe 'admin Geo Nodes', :js, :geo do
end end
end end
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)
it 'when primary renders only primary fields' do
geo_node.update(primary: true)
visit edit_admin_geo_node_path(geo_node)
expect_fields(primary_only_fields)
expect_no_fields(secondary_only_fields)
end
it 'when secondary renders only secondary fields' do
geo_node.update(primary: false)
visit edit_admin_geo_node_path(geo_node)
expect_no_fields(primary_only_fields)
expect_fields(secondary_only_fields)
end
end
describe 'create a new Geo Nodes' do describe 'create a new Geo Nodes' do
let(:new_ssh_key) { attributes_for(:key)[:key] } let(:new_ssh_key) { attributes_for(:key)[:key] }
...@@ -90,7 +111,6 @@ describe 'admin Geo Nodes', :js, :geo do ...@@ -90,7 +111,6 @@ describe 'admin Geo Nodes', :js, :geo do
end end
it 'creates a new Geo Node' do it 'creates a new Geo Node' do
check 'node-primary-field'
fill_in 'node-name-field', with: 'a node name' fill_in 'node-name-field', with: 'a node name'
fill_in 'node-url-field', with: 'https://test.gitlab.com' fill_in 'node-url-field', with: 'https://test.gitlab.com'
click_button 'Save' click_button 'Save'
...@@ -102,70 +122,6 @@ describe 'admin Geo Nodes', :js, :geo do ...@@ -102,70 +122,6 @@ describe 'admin Geo Nodes', :js, :geo do
expect(page).to have_content(geo_node.url) expect(page).to have_content(geo_node.url)
end end
end end
context 'toggles the visibility of secondary only params based on primary node checkbox' 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 '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 '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
before do
create(:geo_node, :primary)
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'
click_button 'Save'
wait_for_requests
expect(current_path).to eq new_admin_geo_node_path
expect(page).to have_content(/There was an error saving this Geo Node.*primary node already exists/)
end
end
end end
describe 'update an existing Geo Node' do describe 'update an existing Geo Node' do
...@@ -176,7 +132,6 @@ describe 'admin Geo Nodes', :js, :geo do ...@@ -176,7 +132,6 @@ describe 'admin Geo Nodes', :js, :geo do
fill_in 'node-url-field', with: 'http://newsite.com' fill_in 'node-url-field', with: 'http://newsite.com'
fill_in 'node-internal-url-field', with: 'http://internal-url.com' fill_in 'node-internal-url-field', with: 'http://internal-url.com'
check 'node-primary-field'
click_button 'Update' click_button 'Update'
wait_for_requests wait_for_requests
......
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import { GlBadge } from '@gitlab/ui';
import GeoNodeFormApp from 'ee/geo_node_form/components/app.vue'; import GeoNodeFormApp from 'ee/geo_node_form/components/app.vue';
import GeoNodeForm from 'ee/geo_node_form/components/geo_node_form.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', () => { describe('GeoNodeFormApp', () => {
let wrapper; let wrapper;
...@@ -23,6 +24,7 @@ describe('GeoNodeFormApp', () => { ...@@ -23,6 +24,7 @@ describe('GeoNodeFormApp', () => {
}); });
const findGeoNodeFormTitle = () => wrapper.find('.page-title'); const findGeoNodeFormTitle = () => wrapper.find('.page-title');
const findGeoNodeFormBadge = () => wrapper.find(GlBadge);
const findGeoForm = () => wrapper.find(GeoNodeForm); const findGeoForm = () => wrapper.find(GeoNodeForm);
describe('render', () => { describe('render', () => {
...@@ -30,35 +32,34 @@ describe('GeoNodeFormApp', () => { ...@@ -30,35 +32,34 @@ describe('GeoNodeFormApp', () => {
createComponent(); createComponent();
}); });
it('the Geo Node Form Title', () => { describe.each`
expect(findGeoNodeFormTitle().exists()).toBe(true); 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', () => { it(`sets the node form title to ${title}`, () => {
expect(findGeoForm().exists()).toBe(true); expect(findGeoNodeFormTitle().text()).toBe(title);
}); });
});
describe('Geo Node Form Title', () => { it(`sets the node form pill title to ${pillTitle}`, () => {
describe('when props.node is undefined', () => { expect(findGeoNodeFormBadge().text()).toBe(pillTitle);
beforeEach(() => { });
createComponent();
});
it('sets title to `New Geo Node`', () => { it(`sets the node form pill variant to be ${variant}`, () => {
expect(findGeoNodeFormTitle().text()).toBe('New Geo Node'); expect(findGeoNodeFormBadge().attributes('variant')).toBe(variant);
});
}); });
}); });
describe('when props.node is set', () => { it('the Geo Node Form', () => {
beforeEach(() => { expect(findGeoForm().exists()).toBe(true);
propsData.node = MOCK_NODE;
createComponent();
});
it('sets title to `Edit Geo Node`', () => {
expect(findGeoNodeFormTitle().text()).toBe('Edit Geo Node');
});
}); });
}); });
}); });
...@@ -29,7 +29,6 @@ describe('GeoNodeForm', () => { ...@@ -29,7 +29,6 @@ describe('GeoNodeForm', () => {
}); });
const findGeoNodeFormCoreField = () => wrapper.find(GeoNodeFormCore); const findGeoNodeFormCoreField = () => wrapper.find(GeoNodeFormCore);
const findGeoNodePrimaryField = () => wrapper.find('#node-primary-field');
const findGeoNodeInternalUrlField = () => wrapper.find('#node-internal-url-field'); const findGeoNodeInternalUrlField = () => wrapper.find('#node-internal-url-field');
const findGeoNodeFormCapacitiesField = () => wrapper.find(GeoNodeFormCapacities); const findGeoNodeFormCapacitiesField = () => wrapper.find(GeoNodeFormCapacities);
const findGeoNodeObjectStorageField = () => wrapper.find('#node-object-storage-field'); const findGeoNodeObjectStorageField = () => wrapper.find('#node-object-storage-field');
...@@ -42,19 +41,12 @@ describe('GeoNodeForm', () => { ...@@ -42,19 +41,12 @@ describe('GeoNodeForm', () => {
}); });
describe.each` describe.each`
primaryNode | showCore | showPrimary | showInternalUrl | showCapacities | showObjectStorage primaryNode | showCore | showInternalUrl | showCapacities | showObjectStorage
${true} | ${true} | ${true} | ${true} | ${true} | ${false} ${true} | ${true} | ${true} | ${true} | ${false}
${false} | ${true} | ${true} | ${false} | ${true} | ${true} ${false} | ${true} | ${false} | ${true} | ${true}
`( `(
`conditional fields`, `conditional fields`,
({ ({ primaryNode, showCore, showInternalUrl, showCapacities, showObjectStorage }) => {
primaryNode,
showCore,
showPrimary,
showInternalUrl,
showCapacities,
showObjectStorage,
}) => {
beforeEach(() => { beforeEach(() => {
wrapper.setData({ wrapper.setData({
nodeData: { ...wrapper.vm.nodeData, primary: primaryNode }, nodeData: { ...wrapper.vm.nodeData, primary: primaryNode },
...@@ -65,10 +57,6 @@ describe('GeoNodeForm', () => { ...@@ -65,10 +57,6 @@ describe('GeoNodeForm', () => {
expect(findGeoNodeFormCoreField().exists()).toBe(showCore); 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`, () => { it(`it ${showInternalUrl ? 'shows' : 'hides'} the Internal URL Field`, () => {
expect(findGeoNodeInternalUrlField().exists()).toBe(showInternalUrl); expect(findGeoNodeInternalUrlField().exists()).toBe(showInternalUrl);
}); });
......
...@@ -18591,6 +18591,9 @@ msgstr "" ...@@ -18591,6 +18591,9 @@ msgstr ""
msgid "Seats in license" msgid "Seats in license"
msgstr "" msgstr ""
msgid "Secondary"
msgstr ""
msgid "Secret" msgid "Secret"
msgstr "" msgstr ""
...@@ -21728,9 +21731,6 @@ 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." msgid "This is a list of devices that have logged into your account. Revoke any sessions that you do not recognize."
msgstr "" msgstr ""
msgid "This is a primary node"
msgstr ""
msgid "This is a security log of important events involving your account." msgid "This is a security log of important events involving your account."
msgstr "" msgstr ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment