Commit 4c11b1fd authored by Zack Cuddy's avatar Zack Cuddy

Cleanup Legacy Geo Error

In the GeoNodeItem and GeoNodeHeader
there is some old legacy error code.

None of it is ever set to true and thus is never used.

This MR cleans all that code up.
parent ed84d447
<script> <script>
import { GlLoadingIcon } from '@gitlab/ui'; import { GlLoadingIcon } from '@gitlab/ui';
import { s__ } from '~/locale';
import icon from '~/vue_shared/components/icon.vue'; import icon from '~/vue_shared/components/icon.vue';
import tooltip from '~/vue_shared/directives/tooltip'; import tooltip from '~/vue_shared/directives/tooltip';
...@@ -17,49 +16,17 @@ export default { ...@@ -17,49 +16,17 @@ export default {
type: Object, type: Object,
required: true, required: true,
}, },
nodeDetails: {
type: Object,
required: true,
},
nodeDetailsLoading: { nodeDetailsLoading: {
type: Boolean, type: Boolean,
required: true, required: true,
}, },
nodeDetailsFailed: {
type: Boolean,
required: true,
},
}, },
computed: { computed: {
isNodeHTTP() { isNodeHTTP() {
return this.node.url.startsWith('http://'); return this.node.url.startsWith('http://');
}, },
showNodeStatusIcon() { showNodeWarningIcon() {
if (this.nodeDetailsLoading) { return !this.nodeDetailsLoading && this.isNodeHTTP;
return false;
}
return this.isNodeHTTP || this.nodeDetailsFailed;
},
nodeStatusIconClass() {
return [
'ml-2',
{ 'text-danger-500': this.nodeDetailsFailed, 'text-warning-500': !this.nodeDetailsFailed },
];
},
nodeStatusIconName() {
if (this.nodeDetailsFailed) {
return 'status_failed_borderless';
}
return 'warning';
},
nodeStatusIconTooltip() {
if (this.nodeDetailsFailed) {
return '';
}
return s__(
'GeoNodes|You have configured Geo nodes using an insecure HTTP connection. We recommend the use of HTTPS.',
);
}, },
}, },
}; };
...@@ -76,12 +43,16 @@ export default { ...@@ -76,12 +43,16 @@ export default {
class="node-details-loading prepend-left-10 inline" class="node-details-loading prepend-left-10 inline"
/> />
<icon <icon
v-if="showNodeStatusIcon" v-if="showNodeWarningIcon"
v-tooltip v-tooltip
:name="nodeStatusIconName" class="ml-2 text-warning-500"
name="warning"
:size="18" :size="18"
:class="nodeStatusIconClass" :title="
:title="nodeStatusIconTooltip" s__(
'GeoNodes|You have configured Geo nodes using an insecure HTTP connection. We recommend the use of HTTPS.',
)
"
data-container="body" data-container="body"
data-placement="bottom" data-placement="bottom"
/> />
......
<script> <script>
import { GlLink } from '@gitlab/ui';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
import GeoNodeHeader from './geo_node_header.vue'; import GeoNodeHeader from './geo_node_header.vue';
...@@ -8,7 +6,6 @@ import GeoNodeDetails from './geo_node_details.vue'; ...@@ -8,7 +6,6 @@ import GeoNodeDetails from './geo_node_details.vue';
export default { export default {
components: { components: {
GlLink,
GeoNodeHeader, GeoNodeHeader,
GeoNodeDetails, GeoNodeDetails,
}, },
...@@ -41,20 +38,10 @@ export default { ...@@ -41,20 +38,10 @@ export default {
data() { data() {
return { return {
isNodeDetailsLoading: true, isNodeDetailsLoading: true,
isNodeDetailsFailed: false,
nodeHealthStatus: '', nodeHealthStatus: '',
errorMessage: '',
nodeDetails: {}, nodeDetails: {},
}; };
}, },
computed: {
showNodeDetails() {
if (!this.isNodeDetailsLoading) {
return !this.isNodeDetailsFailed;
}
return false;
},
},
created() { created() {
eventHub.$on('nodeDetailsLoaded', this.handleNodeDetails); eventHub.$on('nodeDetailsLoaded', this.handleNodeDetails);
}, },
...@@ -68,8 +55,6 @@ export default { ...@@ -68,8 +55,6 @@ export default {
handleNodeDetails(nodeDetails) { handleNodeDetails(nodeDetails) {
if (this.node.id === nodeDetails.id) { if (this.node.id === nodeDetails.id) {
this.isNodeDetailsLoading = false; this.isNodeDetailsLoading = false;
this.isNodeDetailsFailed = false;
this.errorMessage = '';
this.nodeDetails = nodeDetails; this.nodeDetails = nodeDetails;
this.nodeHealthStatus = nodeDetails.health; this.nodeHealthStatus = nodeDetails.health;
} }
...@@ -83,14 +68,9 @@ export default { ...@@ -83,14 +68,9 @@ export default {
<template> <template>
<div :class="{ 'node-action-active': node.nodeActionActive }" class="card"> <div :class="{ 'node-action-active': node.nodeActionActive }" class="card">
<geo-node-header <geo-node-header :node="node" :node-details-loading="isNodeDetailsLoading" />
:node="node"
:node-details="nodeDetails"
:node-details-loading="isNodeDetailsLoading"
:node-details-failed="isNodeDetailsFailed"
/>
<geo-node-details <geo-node-details
v-if="showNodeDetails" v-if="!isNodeDetailsLoading"
:node="node" :node="node"
:node-details="nodeDetails" :node-details="nodeDetails"
:node-edit-allowed="nodeEditAllowed" :node-edit-allowed="nodeEditAllowed"
...@@ -98,13 +78,5 @@ export default { ...@@ -98,13 +78,5 @@ export default {
:node-removal-allowed="nodeRemovalAllowed" :node-removal-allowed="nodeRemovalAllowed"
:geo-troubleshooting-help-path="geoTroubleshootingHelpPath" :geo-troubleshooting-help-path="geoTroubleshootingHelpPath"
/> />
<div v-if="isNodeDetailsFailed">
<p class="p-3 mb-0 bg-danger-100 text-danger-500">
{{ errorMessage
}}<gl-link :href="geoTroubleshootingHelpPath">{{
s__('Geo|Please refer to Geo Troubleshooting.')
}}</gl-link>
</p>
</div>
</div> </div>
</template> </template>
...@@ -48,47 +48,24 @@ describe('GeoNodeHeader', () => { ...@@ -48,47 +48,24 @@ describe('GeoNodeHeader', () => {
}); });
}); });
describe('showNodeStatusIcon', () => { describe.each`
it('returns `false` when Node details are still loading', done => { nodeDetailsLoading | url | showWarning
vm.nodeDetailsLoading = true; ${false} | ${'https://127.0.0.1:3001'} | ${false}
Vue.nextTick() ${false} | ${'http://127.0.0.1:3001'} | ${true}
.then(() => { ${true} | ${'https://127.0.0.1:3001'} | ${false}
expect(vm.showNodeStatusIcon).toBe(false); ${true} | ${'http://127.0.0.1:3001'} | ${false}
}) `(`showNodeWarningIcon`, ({ nodeDetailsLoading, url, showWarning }) => {
.then(done) beforeEach(() => {
.catch(done.fail); vm.nodeDetailsLoading = nodeDetailsLoading;
vm.node.url = url;
}); });
it('returns `true` when Node details failed to load', done => { it(`should return ${showWarning}`, () => {
vm.nodeDetailsFailed = true; expect(vm.showNodeWarningIcon).toBe(showWarning);
Vue.nextTick()
.then(() => {
expect(vm.showNodeStatusIcon).toBe(true);
})
.then(done)
.catch(done.fail);
}); });
it('returns `true` when Node details loaded and Node URL is non-HTTPS', done => { it(`should ${showWarning ? 'render' : 'not render'} the status icon`, () => {
vm.nodeDetailsLoading = false; expect(Boolean(vm.$el.querySelector('.ic-warning'))).toBe(showWarning);
vm.nodeDetailsFailed = false;
vm.node.url = mockNode.url;
Vue.nextTick()
.then(() => {
expect(vm.showNodeStatusIcon).toBe(true);
})
.then(done)
.catch(done.fail);
});
it('returns `false` when Node details loaded and Node URL is HTTPS', done => {
vm.node.url = 'https://127.0.0.1:3001/';
Vue.nextTick()
.then(() => {
expect(vm.showNodeStatusIcon).toBe(false);
})
.then(done)
.catch(done.fail);
}); });
}); });
}); });
......
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import { GlLink } from '@gitlab/ui';
import geoNodeItemComponent from 'ee/geo_nodes/components/geo_node_item.vue'; import geoNodeItemComponent from 'ee/geo_nodes/components/geo_node_item.vue';
import GeoNodeDetails from 'ee/geo_nodes/components/geo_node_details.vue';
import eventHub from 'ee/geo_nodes/event_hub'; import eventHub from 'ee/geo_nodes/event_hub';
import { mockNode, mockNodeDetails } from '../mock_data'; import { mockNode, mockNodeDetails } from '../mock_data';
...@@ -32,6 +32,8 @@ describe('GeoNodeItemComponent', () => { ...@@ -32,6 +32,8 @@ describe('GeoNodeItemComponent', () => {
createComponent(); createComponent();
}); });
const findGeoNodeDetails = () => wrapper.find(GeoNodeDetails);
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
}); });
...@@ -39,50 +41,11 @@ describe('GeoNodeItemComponent', () => { ...@@ -39,50 +41,11 @@ describe('GeoNodeItemComponent', () => {
describe('data', () => { describe('data', () => {
it('returns default data props', () => { it('returns default data props', () => {
expect(wrapper.vm.isNodeDetailsLoading).toBe(true); expect(wrapper.vm.isNodeDetailsLoading).toBe(true);
expect(wrapper.vm.isNodeDetailsFailed).toBe(false);
expect(wrapper.vm.nodeHealthStatus).toBe(''); expect(wrapper.vm.nodeHealthStatus).toBe('');
expect(wrapper.vm.errorMessage).toBe('');
expect(typeof wrapper.vm.nodeDetails).toBe('object'); expect(typeof wrapper.vm.nodeDetails).toBe('object');
}); });
}); });
describe('computed', () => {
let httpsNode;
beforeEach(() => {
// Altered mock data for secure URL
httpsNode = Object.assign({}, mockNode, {
id: mockNodeDetails.id,
url: 'https://127.0.0.1:3001/',
});
createComponent({ node: httpsNode });
});
describe('showNodeDetails', () => {
it('returns `false` if Node details are still loading', () => {
wrapper.vm.isNodeDetailsLoading = true;
expect(wrapper.vm.showNodeDetails).toBeFalsy();
});
it('returns `false` if Node details failed to load', () => {
wrapper.vm.isNodeDetailsLoading = false;
wrapper.vm.isNodeDetailsFailed = true;
expect(wrapper.vm.showNodeDetails).toBeFalsy();
});
it('returns `true` if Node details loaded', () => {
wrapper.vm.handleNodeDetails(mockNodeDetails);
wrapper.vm.isNodeDetailsLoading = false;
wrapper.vm.isNodeDetailsFailed = false;
expect(wrapper.vm.showNodeDetails).toBeTruthy();
});
});
});
describe('methods', () => { describe('methods', () => {
describe('handleNodeDetails', () => { describe('handleNodeDetails', () => {
describe('with matching ID', () => { describe('with matching ID', () => {
...@@ -100,8 +63,6 @@ describe('GeoNodeItemComponent', () => { ...@@ -100,8 +63,6 @@ describe('GeoNodeItemComponent', () => {
wrapper.vm.handleNodeDetails(mockNodeDetails); wrapper.vm.handleNodeDetails(mockNodeDetails);
expect(wrapper.vm.isNodeDetailsLoading).toBeFalsy(); expect(wrapper.vm.isNodeDetailsLoading).toBeFalsy();
expect(wrapper.vm.isNodeDetailsFailed).toBeFalsy();
expect(wrapper.vm.errorMessage).toBe('');
expect(wrapper.vm.nodeDetails).toBe(mockNodeDetails); expect(wrapper.vm.nodeDetails).toBe(mockNodeDetails);
expect(wrapper.vm.nodeHealthStatus).toBe(mockNodeDetails.health); expect(wrapper.vm.nodeHealthStatus).toBe(mockNodeDetails.health);
}); });
...@@ -144,27 +105,26 @@ describe('GeoNodeItemComponent', () => { ...@@ -144,27 +105,26 @@ describe('GeoNodeItemComponent', () => {
describe('template', () => { describe('template', () => {
it('renders container element', () => { it('renders container element', () => {
expect(wrapper.vm.$el.classList.contains('card', 'geo-node-item')).toBe(true); expect(wrapper.classes('card')).toBeTruthy();
}); });
describe('with error', () => { describe('when isNodeDetailsLoading is true', () => {
let err;
beforeEach(() => { beforeEach(() => {
err = 'Something error message'; wrapper.setData({ isNodeDetailsLoading: true });
wrapper.setData({ errorMessage: err, isNodeDetailsFailed: true });
}); });
it('renders node error message', () => { it('does not render details section', () => {
const findErrorMessage = () => wrapper.find('.bg-danger-100'); expect(findGeoNodeDetails().exists()).toBeFalsy();
});
});
describe('when isNodeDetailsLoading is false', () => {
beforeEach(() => {
wrapper.setData({ isNodeDetailsLoading: false });
});
expect(findErrorMessage().exists()).toBeTruthy(); it('renders details section', () => {
expect(findErrorMessage().text()).toContain(err); expect(findGeoNodeDetails().exists()).toBeTruthy();
expect(
findErrorMessage()
.find(GlLink)
.attributes('href'),
).toBe('/foo/bar');
}); });
}); });
}); });
......
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