Commit f3511e36 authored by Frédéric Caplette's avatar Frédéric Caplette Committed by Sarah Groff Hennigh-Palermo

Refactor error handling for CI config visualization

- Refactor the error handling to stop using warnings
- Consolidate the system into one error object
- Replace lodash `isEmpty` with a simple length check
parent d6d1304e
<script> <script>
import { isEmpty } from 'lodash';
import { GlAlert } from '@gitlab/ui'; import { GlAlert } from '@gitlab/ui';
import { __ } from '~/locale'; import { __ } from '~/locale';
import JobPill from './job_pill.vue'; import JobPill from './job_pill.vue';
...@@ -22,8 +21,6 @@ export default { ...@@ -22,8 +21,6 @@ export default {
errorTexts: { errorTexts: {
[DRAW_FAILURE]: __('Could not draw the lines for job relationships'), [DRAW_FAILURE]: __('Could not draw the lines for job relationships'),
[DEFAULT]: __('An unknown error occurred.'), [DEFAULT]: __('An unknown error occurred.'),
},
warningTexts: {
[EMPTY_PIPELINE_DATA]: __( [EMPTY_PIPELINE_DATA]: __(
'The visualization will appear in this tab when the CI/CD configuration file is populated with valid syntax.', 'The visualization will appear in this tab when the CI/CD configuration file is populated with valid syntax.',
), ),
...@@ -46,24 +43,24 @@ export default { ...@@ -46,24 +43,24 @@ export default {
}; };
}, },
computed: { computed: {
hideGraph() {
// We won't even try to render the graph with these condition
// because it would cause additional errors down the line for the user
// which is confusing.
return this.isPipelineDataEmpty || this.isInvalidCiConfig;
},
pipelineStages() { pipelineStages() {
return this.pipelineData?.stages || []; return this.pipelineData?.stages || [];
}, },
isPipelineDataEmpty() { isPipelineDataEmpty() {
return !this.isInvalidCiConfig && isEmpty(this.pipelineStages); return !this.isInvalidCiConfig && this.pipelineStages.length === 0;
}, },
isInvalidCiConfig() { isInvalidCiConfig() {
return this.pipelineData?.status === CI_CONFIG_STATUS_INVALID; return this.pipelineData?.status === CI_CONFIG_STATUS_INVALID;
}, },
showAlert() {
return this.hasError || this.hasWarning;
},
hasError() { hasError() {
return this.failureType; return this.failureType;
}, },
hasWarning() {
return this.warning;
},
hasHighlightedJob() { hasHighlightedJob() {
return Boolean(this.highlightedJob); return Boolean(this.highlightedJob);
}, },
...@@ -75,26 +72,32 @@ export default { ...@@ -75,26 +72,32 @@ export default {
return this.warning; return this.warning;
}, },
failure() { failure() {
const text = this.$options.errorTexts[this.failureType] || this.$options.errorTexts[DEFAULT]; switch (this.failureType) {
case DRAW_FAILURE:
return { text, variant: 'danger', dismissible: true }; return {
}, text: this.$options.errorTexts[DRAW_FAILURE],
warning() { variant: 'danger',
if (this.isPipelineDataEmpty) { dismissible: true,
return { };
text: this.$options.warningTexts[EMPTY_PIPELINE_DATA], case EMPTY_PIPELINE_DATA:
variant: 'tip', return {
dismissible: false, text: this.$options.errorTexts[EMPTY_PIPELINE_DATA],
}; variant: 'tip',
} else if (this.isInvalidCiConfig) { dismissible: false,
return { };
text: this.$options.warningTexts[INVALID_CI_CONFIG], case INVALID_CI_CONFIG:
variant: 'danger', return {
dismissible: false, text: this.$options.errorTexts[INVALID_CI_CONFIG],
}; variant: 'danger',
dismissible: false,
};
default:
return {
text: this.$options.errorTexts[DEFAULT],
variant: 'danger',
dismissible: true,
};
} }
return null;
}, },
viewBox() { viewBox() {
return [0, 0, this.width, this.height]; return [0, 0, this.width, this.height];
...@@ -122,6 +125,24 @@ export default { ...@@ -122,6 +125,24 @@ export default {
return []; return [];
}, },
}, },
watch: {
isPipelineDataEmpty: {
immediate: true,
handler(isDataEmpty) {
if (isDataEmpty) {
this.reportFailure(EMPTY_PIPELINE_DATA);
}
},
},
isInvalidCiConfig: {
immediate: true,
handler(isInvalid) {
if (isInvalid) {
this.reportFailure(INVALID_CI_CONFIG);
}
},
},
},
mounted() { mounted() {
if (!this.isPipelineDataEmpty && !this.isInvalidCiConfig) { if (!this.isPipelineDataEmpty && !this.isInvalidCiConfig) {
// This guarantee that all sub-elements are rendered // This guarantee that all sub-elements are rendered
...@@ -201,7 +222,7 @@ export default { ...@@ -201,7 +222,7 @@ export default {
<template> <template>
<div> <div>
<gl-alert <gl-alert
v-if="showAlert" v-if="hasError"
:variant="alert.variant" :variant="alert.variant"
:dismissible="alert.dismissible" :dismissible="alert.dismissible"
@dismiss="alert.dismissible ? resetFailure : null" @dismiss="alert.dismissible ? resetFailure : null"
...@@ -209,7 +230,7 @@ export default { ...@@ -209,7 +230,7 @@ export default {
{{ alert.text }} {{ alert.text }}
</gl-alert> </gl-alert>
<div <div
v-if="!hasWarning" v-if="!hideGraph"
:id="$options.CONTAINER_ID" :id="$options.CONTAINER_ID"
:ref="$options.CONTAINER_REF" :ref="$options.CONTAINER_REF"
class="gl-display-flex gl-bg-gray-50 gl-px-4 gl-overflow-auto gl-relative gl-py-7" class="gl-display-flex gl-bg-gray-50 gl-px-4 gl-overflow-auto gl-relative gl-py-7"
......
...@@ -37,7 +37,7 @@ describe('pipeline graph component', () => { ...@@ -37,7 +37,7 @@ describe('pipeline graph component', () => {
}); });
it('renders an empty section', () => { it('renders an empty section', () => {
expect(wrapper.text()).toBe(wrapper.vm.$options.warningTexts[EMPTY_PIPELINE_DATA]); expect(wrapper.text()).toBe(wrapper.vm.$options.errorTexts[EMPTY_PIPELINE_DATA]);
expect(findPipelineGraph().exists()).toBe(false); expect(findPipelineGraph().exists()).toBe(false);
expect(findAllStagePills()).toHaveLength(0); expect(findAllStagePills()).toHaveLength(0);
expect(findAllJobPills()).toHaveLength(0); expect(findAllJobPills()).toHaveLength(0);
...@@ -51,7 +51,7 @@ describe('pipeline graph component', () => { ...@@ -51,7 +51,7 @@ describe('pipeline graph component', () => {
it('renders an error message and does not render the graph', () => { it('renders an error message and does not render the graph', () => {
expect(findAlert().exists()).toBe(true); expect(findAlert().exists()).toBe(true);
expect(findAlert().text()).toBe(wrapper.vm.$options.warningTexts[INVALID_CI_CONFIG]); expect(findAlert().text()).toBe(wrapper.vm.$options.errorTexts[INVALID_CI_CONFIG]);
expect(findPipelineGraph().exists()).toBe(false); expect(findPipelineGraph().exists()).toBe(false);
}); });
}); });
......
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