Skip to content

G
gitlab-ce
Commit e6542f12 authored by Sarah Groff Hennigh-Palermo's avatar Sarah Groff Hennigh-Palermo
Browse files
Options

Merge branch '321100-centralize-invalid-ci-state-in-authoring-section' into 'master' 

Centralize shared state in Authoring section

See merge request gitlab-org/gitlab!58790
parents 43c7321e 5f410fcf
Show whitespace changes
Inline Side-by-side
Showing with 209 additions and 182 deletions
app/assets/javascripts/pipeline_editor/components/editor/ci_config_merged_preview.vue
View file @ e6542f12
<script>
import { GlAlert, GlIcon } from '@gitlab/ui';
import { GlIcon } from '@gitlab/ui';
import { uniqueId } from 'lodash';
import { __, s__ } from '~/locale';
import { DEFAULT, INVALID_CI_CONFIG } from '~/pipelines/constants';
import { s__ } from '~/locale';
import EditorLite from '~/vue_shared/components/editor_lite.vue';
export default {
i18n: {
viewOnlyMessage: s__('Pipelines|Merged YAML is view only'),
},
errorTexts: {
[INVALID_CI_CONFIG]: __('Your CI configuration file is invalid.'),
[DEFAULT]: __('An unknown error occurred.'),
},
components: {
EditorLite,
GlAlert,
GlIcon,
},
inject: ['ciConfigPath'],
props: {
isValid: {
type: Boolean,
required: true,
},
ciConfigData: {
type: Object,
required: true,
......@@ -35,52 +25,17 @@ export default {
};
},
computed: {
failure() {
switch (this.failureType) {
case INVALID_CI_CONFIG:
return this.$options.errorTexts[INVALID_CI_CONFIG];
default:
return this.$options.errorTexts[DEFAULT];
}
},
fileGlobalId() {
return `${this.ciConfigPath}-${uniqueId()}`;
},
hasError() {
return this.failureType;
},
mergedYaml() {
return this.ciConfigData.mergedYaml;
},
},
watch: {
ciConfigData: {
immediate: true,
handler() {
if (!this.isValid) {
this.reportFailure(INVALID_CI_CONFIG);
} else if (this.hasError) {
this.resetFailure();
}
},
},
},
methods: {
reportFailure(errorType) {
this.failureType = errorType;
},
resetFailure() {
this.failureType = null;
},
},
};
</script>
<template>
<div>
<gl-alert v-if="hasError" variant="danger" :dismissible="false">
{{ failure }}
</gl-alert>
<div v-else>
<div class="gl-display-flex gl-align-items-center">
<gl-icon :size="16" name="lock" class="gl-text-gray-500 gl-mr-3" />
{{ $options.i18n.viewOnlyMessage }}
......@@ -96,5 +51,4 @@ export default {
/>
</div>
</div>
</div>
</template>
app/assets/javascripts/pipeline_editor/components/pipeline_editor_tabs.vue
View file @ e6542f12
<script>
import { GlAlert, GlLoadingIcon, GlTabs, GlTab } from '@gitlab/ui';
import { GlAlert, GlLoadingIcon, GlTabs } from '@gitlab/ui';
import { s__ } from '~/locale';
import PipelineGraph from '~/pipelines/components/pipeline_graph/pipeline_graph.vue';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import {
CREATE_TAB,
EDITOR_APP_STATUS_EMPTY,
EDITOR_APP_STATUS_ERROR,
EDITOR_APP_STATUS_INVALID,
EDITOR_APP_STATUS_LOADING,
EDITOR_APP_STATUS_VALID,
LINT_TAB,
......@@ -24,6 +26,17 @@ export default {
tabGraph: s__('Pipelines|Visualize'),
tabLint: s__('Pipelines|Lint'),
tabMergedYaml: s__('Pipelines|View merged YAML'),
empty: {
visualization: s__(
'PipelineEditor|The pipeline visualization is displayed when the CI/CD configuration file has valid syntax.',
),
lint: s__(
'PipelineEditor|The CI/CD configuration is continuously validated. Errors and warnings are displayed when the CI/CD configuration file is not empty.',
),
merge: s__(
'PipelineEditor|The merged YAML view is displayed when the CI/CD configuration file has valid syntax.',
),
},
},
errorTexts: {
loadMergedYaml: s__('Pipelines|Could not load merged YAML content'),
......@@ -40,7 +53,6 @@ export default {
EditorTab,
GlAlert,
GlLoadingIcon,
GlTab,
GlTabs,
PipelineGraph,
TextEditor,
......@@ -66,6 +78,12 @@ export default {
// Not an invalid config and with `mergedYaml` data missing
return this.appStatus === EDITOR_APP_STATUS_ERROR;
},
isEmpty() {
return this.appStatus === EDITOR_APP_STATUS_EMPTY;
},
isInvalid() {
return this.appStatus === EDITOR_APP_STATUS_INVALID;
},
isValid() {
return this.appStatus === EDITOR_APP_STATUS_VALID;
},
......@@ -91,9 +109,12 @@ export default {
>
<text-editor :value="ciFileContent" v-on="$listeners" />
</editor-tab>
<gl-tab
<editor-tab
v-if="glFeatures.ciConfigVisualizationTab"
class="gl-mb-3"
:empty-message="$options.i18n.empty.visualization"
:is-empty="isEmpty"
:is-invalid="isInvalid"
:title="$options.i18n.tabGraph"
lazy
data-testid="visualization-tab"
......@@ -101,9 +122,11 @@ export default {
>
<gl-loading-icon v-if="isLoading" size="lg" class="gl-m-3" />
<pipeline-graph v-else :pipeline-data="ciConfigData" />
</gl-tab>
</editor-tab>
<editor-tab
class="gl-mb-3"
:empty-message="$options.i18n.empty.lint"
:is-empty="isEmpty"
:title="$options.i18n.tabLint"
data-testid="lint-tab"
@click="setCurrentTab($options.tabConstants.LINT_TAB)"
......@@ -111,9 +134,13 @@ export default {
<gl-loading-icon v-if="isLoading" size="lg" class="gl-m-3" />
<ci-lint v-else :is-valid="isValid" :ci-config="ciConfigData" />
</editor-tab>
<gl-tab
<editor-tab
v-if="glFeatures.ciConfigMergedTab"
class="gl-mb-3"
:empty-message="$options.i18n.empty.merge"
:keep-component-mounted="false"
:is-empty="isEmpty"
:is-invalid="isInvalid"
:title="$options.i18n.tabMergedYaml"
lazy
data-testid="merged-tab"
......@@ -123,12 +150,7 @@ export default {
<gl-alert v-else-if="hasAppError" variant="danger" :dismissible="false">
{{ $options.errorTexts.loadMergedYaml }}
</gl-alert>
<ci-config-merged-preview
v-else
:is-valid="isValid"
:ci-config-data="ciConfigData"
v-on="$listeners"
/>
</gl-tab>
<ci-config-merged-preview v-else :ci-config-data="ciConfigData" v-on="$listeners" />
</editor-tab>
</gl-tabs>
</template>
app/assets/javascripts/pipeline_editor/components/ui/editor_tab.vue
View file @ e6542f12
<script>
import { GlTab } from '@gitlab/ui';
import { GlAlert, GlTab } from '@gitlab/ui';
import { __, s__ } from '~/locale';
/**
* Wrapper of <gl-tab> to optionally lazily render this tab's content
* when its shown **without dismounting after its hidden**.
......@@ -10,10 +10,10 @@ import { GlTab } from '@gitlab/ui';
* API is the same as <gl-tab>, for example:
*
* <gl-tabs>
* <editor-tab title="Tab 1" :lazy="true">
* <editor-tab title="Tab 1" lazy>
* lazily mounted content (gets mounted if this is first tab)
* </editor-tab>
* <editor-tab title="Tab 2" :lazy="true">
* <editor-tab title="Tab 2" lazy>
* lazily mounted content
* </editor-tab>
* <editor-tab title="Tab 3">
......@@ -25,10 +25,26 @@ import { GlTab } from '@gitlab/ui';
* so it's contents are not dismounted.
*
* lazy is "false" by default, as in <gl-tab>.
*
* It is also possible to pass the `isEmpty` and or `isInvalid` to let
* the tab component handle that state on its own. For example:
*
* * <gl-tabs>
* <editor-tab-with-status title="Tab 1" :is-empty="isEmpty" :is-invalid="isInvalid">
* ...
* </editor-tab-with-status>
* Will be the same as normal, except it will only render the slot component
* if the status is not empty and not invalid. In any of these 2 cases, it will render
* a generic component and avoid mounting whatever it received in the slot.
* </gl-tabs>
*/
export default {
i18n: {
invalid: __('Your CI/CD configuration syntax is invalid. View Lint tab for more details.'),
},
components: {
GlAlert,
GlTab,
// Use a small renderless component to know when the tab content mounts because:
// - gl-tab always gets mounted, even if lazy is `true`. See:
......@@ -40,29 +56,63 @@ export default {
},
inheritAttrs: false,
props: {
emptyMessage: {
type: String,
required: false,
default: s__(
'PipelineEditor|This tab will be usable when the CI/CD configuration file is populated with valid syntax.',
),
},
isEmpty: {
type: Boolean,
required: false,
default: null,
},
isInvalid: {
type: Boolean,
required: false,
default: null,
},
lazy: {
type: Boolean,
required: false,
default: false,
},
keepComponentMounted: {
type: Boolean,
required: false,
default: true,
},
},
data() {
return {
isLazy: this.lazy,
};
},
computed: {
slots() {
return Object.keys(this.$slots);
},
},
methods: {
onContentMounted() {
// When a child is first mounted make the entire tab
// permanently mounted by setting 'lazy' to false.
// permanently mounted by setting 'lazy' to false unless
// explicitly opted out.
if (this.keepComponentMounted) {
this.isLazy = false;
}
},
},
};
</script>
<template>
<gl-tab :lazy="isLazy" v-bind="$attrs" v-on="$listeners">
<slot v-for="slot in Object.keys($slots)" :name="slot"></slot>
<gl-alert v-if="isEmpty" variant="tip">{{ emptyMessage }}</gl-alert>
<gl-alert v-else-if="isInvalid" variant="danger">{{ $options.i18n.invalid }}</gl-alert>
<template v-else>
<slot v-for="slot in slots" :name="slot"></slot>
<mount-spy @hook:mounted="onContentMounted" />
</template>
</gl-tab>
</template>
app/assets/javascripts/pipelines/components/pipeline_graph/pipeline_graph.vue
View file @ e6542f12
<script>
import { GlAlert } from '@gitlab/ui';
import { __ } from '~/locale';
import { CI_CONFIG_STATUS_INVALID } from '~/pipeline_editor/constants';
import { DRAW_FAILURE, DEFAULT, INVALID_CI_CONFIG, EMPTY_PIPELINE_DATA } from '../../constants';
import { DRAW_FAILURE, DEFAULT } from '../../constants';
import LinksLayer from '../graph_shared/links_layer.vue';
import JobPill from './job_pill.vue';
import StagePill from './stage_pill.vue';
......@@ -21,10 +20,6 @@ export default {
errorTexts: {
[DRAW_FAILURE]: __('Could not draw the lines for job relationships'),
[DEFAULT]: __('An unknown error occurred.'),
[EMPTY_PIPELINE_DATA]: __(
'The visualization will appear in this tab when the CI/CD configuration file is populated with valid syntax.',
),
[INVALID_CI_CONFIG]: __('Your CI configuration file is invalid.'),
},
props: {
pipelineData: {
......@@ -55,18 +50,6 @@ export default {
variant: 'danger',
dismissible: true,
};
case EMPTY_PIPELINE_DATA:
return {
text: this.$options.errorTexts[EMPTY_PIPELINE_DATA],
variant: 'tip',
dismissible: false,
};
case INVALID_CI_CONFIG:
return {
text: this.$options.errorTexts[INVALID_CI_CONFIG],
variant: 'danger',
dismissible: false,
};
default:
return {
text: this.$options.errorTexts[DEFAULT],
......@@ -81,18 +64,6 @@ export default {
hasHighlightedJob() {
return Boolean(this.highlightedJob);
},
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;
},
isInvalidCiConfig() {
return this.pipelineData?.status === CI_CONFIG_STATUS_INVALID;
},
isPipelineDataEmpty() {
return !this.isInvalidCiConfig && this.pipelineStages.length === 0;
},
pipelineStages() {
return this.pipelineData?.stages || [];
},
......@@ -101,15 +72,9 @@ export default {
pipelineData: {
immediate: true,
handler() {
if (this.isPipelineDataEmpty) {
this.reportFailure(EMPTY_PIPELINE_DATA);
} else if (this.isInvalidCiConfig) {
this.reportFailure(INVALID_CI_CONFIG);
} else {
this.$nextTick(() => {
this.computeGraphDimensions();
});
}
},
},
},
......@@ -172,12 +137,7 @@ export default {
>
{{ failure.text }}
</gl-alert>
<div
v-if="!hideGraph"
:id="containerId"
:ref="$options.CONTAINER_REF"
data-testid="graph-container"
>
<div :id="containerId" :ref="$options.CONTAINER_REF" data-testid="graph-container">
<links-layer
:pipeline-data="pipelineStages"
:pipeline-id="$options.PIPELINE_ID"
......
changelogs/unreleased/321100-centralize-invalid-ci-state-in-authoring-section.yml 0 → 100644
View file @ e6542f12
---
title: Centralize shared state in Authoring section
merge_request: 58790
author:
type: changed
locale/gitlab.pot
View file @ e6542f12
......@@ -23159,6 +23159,18 @@ msgstr ""
msgid "PipelineCharts|Total:"
msgstr ""
msgid "PipelineEditor|The CI/CD configuration is continuously validated. Errors and warnings are displayed when the CI/CD configuration file is not empty."
msgstr ""
msgid "PipelineEditor|The merged YAML view is displayed when the CI/CD configuration file has valid syntax."
msgstr ""
msgid "PipelineEditor|The pipeline visualization is displayed when the CI/CD configuration file has valid syntax."
msgstr ""
msgid "PipelineEditor|This tab will be usable when the CI/CD configuration file is populated with valid syntax."
msgstr ""
msgid "PipelineScheduleIntervalPattern|Custom (%{linkStart}Cron syntax%{linkEnd})"
msgstr ""
......@@ -31446,9 +31458,6 @@ msgstr ""
msgid "The value of the provided variable exceeds the %{count} character limit"
msgstr ""
msgid "The visualization will appear in this tab when the CI/CD configuration file is populated with valid syntax."
msgstr ""
msgid "The vulnerability is known, and has not been remediated or mitigated, but is considered to be an acceptable business risk."
msgstr ""
......@@ -36198,7 +36207,7 @@ msgstr ""
msgid "Your %{strong}%{plan_name}%{strong_close} subscription will expire on %{strong}%{expires_on}%{strong_close}. After that, you will not be able to create issues or merge requests as well as many other features."
msgstr ""
msgid "Your CI configuration file is invalid."
msgid "Your CI/CD configuration syntax is invalid. View Lint tab for more details."
msgstr ""
msgid "Your CSV export has started. It will be emailed to %{email} when complete."
......
spec/frontend/pipeline_editor/components/editor/ci_config_merged_preview_spec.js
View file @ e6542f12
import { GlAlert, GlIcon } from '@gitlab/ui';
import { GlIcon } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import { EDITOR_READY_EVENT } from '~/editor/constants';
import CiConfigMergedPreview from '~/pipeline_editor/components/editor/ci_config_merged_preview.vue';
import { INVALID_CI_CONFIG } from '~/pipelines/constants';
import { mockLintResponse, mockCiConfigPath } from '../../mock_data';
describe('Text editor component', () => {
......@@ -32,7 +31,6 @@ describe('Text editor component', () => {
});
};
const findAlert = () => wrapper.findComponent(GlAlert);
const findIcon = () => wrapper.findComponent(GlIcon);
const findEditor = () => wrapper.findComponent(MockEditorLite);
......@@ -40,24 +38,9 @@ describe('Text editor component', () => {
wrapper.destroy();
});
describe('when status is invalid', () => {
beforeEach(() => {
createComponent({ props: { isValid: false } });
});
it('show an error message', () => {
expect(findAlert().exists()).toBe(true);
expect(findAlert().text()).toBe(wrapper.vm.$options.errorTexts[INVALID_CI_CONFIG]);
});
it('hides the editor', () => {
expect(findEditor().exists()).toBe(false);
});
});
describe('when status is valid', () => {
beforeEach(() => {
createComponent({ props: { isValid: true } });
createComponent();
});
it('shows an information message that the section is not editable', () => {
......
spec/frontend/pipeline_editor/components/pipeline_editor_tabs_spec.js
View file @ e6542f12
......@@ -4,9 +4,12 @@ import { nextTick } from 'vue';
import CiConfigMergedPreview from '~/pipeline_editor/components/editor/ci_config_merged_preview.vue';
import CiLint from '~/pipeline_editor/components/lint/ci_lint.vue';
import PipelineEditorTabs from '~/pipeline_editor/components/pipeline_editor_tabs.vue';
import EditorTab from '~/pipeline_editor/components/ui/editor_tab.vue';
import {
EDITOR_APP_STATUS_EMPTY,
EDITOR_APP_STATUS_ERROR,
EDITOR_APP_STATUS_LOADING,
EDITOR_APP_STATUS_INVALID,
EDITOR_APP_STATUS_VALID,
} from '~/pipeline_editor/constants';
import PipelineGraph from '~/pipelines/components/pipeline_graph/pipeline_graph.vue';
......@@ -44,6 +47,7 @@ describe('Pipeline editor tabs component', () => {
provide: { ...mockProvide, ...provide },
stubs: {
TextEditor: MockTextEditor,
EditorTab,
},
});
};
......@@ -192,4 +196,24 @@ describe('Pipeline editor tabs component', () => {
});
});
});
describe('show tab content based on status', () => {
it.each`
appStatus | editor | viz | lint | merged
${undefined} | ${true} | ${true} | ${true} | ${true}
${EDITOR_APP_STATUS_EMPTY} | ${true} | ${false} | ${false} | ${false}
${EDITOR_APP_STATUS_INVALID} | ${true} | ${false} | ${true} | ${false}
${EDITOR_APP_STATUS_VALID} | ${true} | ${true} | ${true} | ${true}
`(
'when status is $appStatus, we show - editor:$editor | viz:$viz | lint:$lint | merged:$merged ',
({ appStatus, editor, viz, lint, merged }) => {
createComponent({ appStatus });
expect(findTextEditor().exists()).toBe(editor);
expect(findPipelineGraph().exists()).toBe(viz);
expect(findCiLint().exists()).toBe(lint);
expect(findMergedPreview().exists()).toBe(merged);
},
);
});
});
spec/frontend/pipeline_editor/components/ui/editor_tab_spec.js
View file @ e6542f12
import { GlTabs } from '@gitlab/ui';
import { GlAlert, GlTabs } from '@gitlab/ui';
import { mount } from '@vue/test-utils';
import { nextTick } from 'vue';
import EditorTab from '~/pipeline_editor/components/ui/editor_tab.vue';
const mockContent1 = 'MOCK CONTENT 1';
const mockContent2 = 'MOCK CONTENT 2';
const MockEditorLite = {
template: '<div>EDITOR</div>',
};
describe('~/pipeline_editor/components/ui/editor_tab.vue', () => {
let wrapper;
let mockChildMounted = jest.fn();
......@@ -37,22 +40,34 @@ describe('~/pipeline_editor/components/ui/editor_tab.vue', () => {
`,
};
const createWrapper = () => {
const createMockedWrapper = () => {
wrapper = mount(MockTabbedContent);
};
const createWrapper = ({ props } = {}) => {
wrapper = mount(EditorTab, {
propsData: props,
slots: {
default: MockEditorLite,
},
});
};
const findSlotComponent = () => wrapper.findComponent(MockEditorLite);
const findAlert = () => wrapper.findComponent(GlAlert);
beforeEach(() => {
mockChildMounted = jest.fn();
});
it('tabs are mounted lazily', async () => {
createWrapper();
createMockedWrapper();
expect(mockChildMounted).toHaveBeenCalledTimes(0);
});
it('first tab is only mounted after nextTick', async () => {
createWrapper();
createMockedWrapper();
await nextTick();
......@@ -60,6 +75,36 @@ describe('~/pipeline_editor/components/ui/editor_tab.vue', () => {
expect(mockChildMounted).toHaveBeenCalledWith(mockContent1);
});
describe('showing the tab content depending on `isEmpty` and `isInvalid`', () => {
it.each`
isEmpty | isInvalid | showSlotComponent | text
${undefined} | ${undefined} | ${true} | ${'renders'}
${false} | ${false} | ${true} | ${'renders'}
${undefined} | ${true} | ${false} | ${'hides'}
${true} | ${false} | ${false} | ${'hides'}
${false} | ${true} | ${false} | ${'hides'}
`(
'$text the slot component when isEmpty:$isEmpty and isInvalid:$isInvalid',
({ isEmpty, isInvalid, showSlotComponent }) => {
createWrapper({
props: { isEmpty, isInvalid },
});
expect(findSlotComponent().exists()).toBe(showSlotComponent);
expect(findAlert().exists()).toBe(!showSlotComponent);
},
);
it('can have a custom empty message', () => {
const text = 'my custom alert message';
createWrapper({ props: { isEmpty: true, emptyMessage: text } });
const alert = findAlert();
expect(alert.exists()).toBe(true);
expect(alert.text()).toBe(text);
});
});
describe('user interaction', () => {
const clickTab = async (testid) => {
wrapper.find(`[data-testid="${testid}"]`).trigger('click');
......@@ -67,7 +112,7 @@ describe('~/pipeline_editor/components/ui/editor_tab.vue', () => {
};
beforeEach(() => {
createWrapper();
createMockedWrapper();
});
it('mounts a tab once after selecting it', async () => {
......
spec/frontend/pipelines/pipeline_graph/pipeline_graph_spec.js
View file @ e6542f12
import { GlAlert } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import { CI_CONFIG_STATUS_INVALID, CI_CONFIG_STATUS_VALID } from '~/pipeline_editor/constants';
import { CI_CONFIG_STATUS_VALID } from '~/pipeline_editor/constants';
import LinksInner from '~/pipelines/components/graph_shared/links_inner.vue';
import LinksLayer from '~/pipelines/components/graph_shared/links_layer.vue';
import JobPill from '~/pipelines/components/pipeline_graph/job_pill.vue';
import PipelineGraph from '~/pipelines/components/pipeline_graph/pipeline_graph.vue';
import StagePill from '~/pipelines/components/pipeline_graph/stage_pill.vue';
import { DRAW_FAILURE, EMPTY_PIPELINE_DATA, INVALID_CI_CONFIG } from '~/pipelines/constants';
import { DRAW_FAILURE } from '~/pipelines/constants';
import { invalidNeedsData, pipelineData, singleStageData } from './mock_data';
describe('pipeline graph component', () => {
......@@ -42,31 +42,6 @@ describe('pipeline graph component', () => {
wrapper.destroy();
});
describe('with no data', () => {
beforeEach(() => {
wrapper = createComponent({ pipelineData: {} });
});
it('does not render the graph', () => {
expect(wrapper.text()).toBe(wrapper.vm.$options.errorTexts[EMPTY_PIPELINE_DATA]);
expect(findPipelineGraph().exists()).toBe(false);
expect(findAllStagePills()).toHaveLength(0);
expect(findAllJobPills()).toHaveLength(0);
});
});
describe('with `INVALID` status', () => {
beforeEach(() => {
wrapper = createComponent({ pipelineData: { status: CI_CONFIG_STATUS_INVALID } });
});
it('renders an error message and does not render the graph', () => {
expect(findAlert().exists()).toBe(true);
expect(findAlert().text()).toBe(wrapper.vm.$options.errorTexts[INVALID_CI_CONFIG]);
expect(findPipelineGraph().exists()).toBe(false);
});
});
describe('with `VALID` status', () => {
beforeEach(() => {
wrapper = createComponent({
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7