Commit bd1122ee authored by Paul Slaughter's avatar Paul Slaughter 🔴

Fix vue render error for IDE status bar

**What?**

A Vue warning that `ide_status_bar` sent a `Boolean` to a `String`
property (`img-src).

**What was the fix?**

Previously, `latestPipeline` could be one of the following values:

|          |        |
|----------|--------|
| `null`   | The pipeline hasn't loaded yet |
| `false`  | The pipeline has loaded, but nothing was returned. |
| `Object` | The piepline has loaded. |

Giving a semantic meaning to different falsey values hurts
maintainability. This commit fixes the above problem by removing the
`false` value and introducing a `hasLoadedPipeline` state property.
parent 7926384f
......@@ -24,7 +24,13 @@ export default {
...mapState(['pipelinesEmptyStateSvgPath', 'links']),
...mapGetters(['currentProject']),
...mapGetters('pipelines', ['jobsCount', 'failedJobsCount', 'failedStages', 'pipelineFailed']),
...mapState('pipelines', ['isLoadingPipeline', 'latestPipeline', 'stages', 'isLoadingJobs']),
...mapState('pipelines', [
'isLoadingPipeline',
'hasLoadedPipeline',
'latestPipeline',
'stages',
'isLoadingJobs',
]),
ciLintText() {
return sprintf(
__('You can test your .gitlab-ci.yml in %{linkStart}CI Lint%{linkEnd}.'),
......@@ -36,7 +42,7 @@ export default {
);
},
showLoadingIcon() {
return this.isLoadingPipeline && this.latestPipeline === null;
return this.isLoadingPipeline && !this.hasLoadedPipeline;
},
},
created() {
......@@ -51,7 +57,7 @@ export default {
<template>
<div class="ide-pipeline">
<gl-loading-icon v-if="showLoadingIcon" :size="2" class="prepend-top-default" />
<template v-else-if="latestPipeline !== null">
<template v-else-if="hasLoadedPipeline">
<header v-if="latestPipeline" class="ide-tree-header ide-pipeline-header">
<ci-icon :status="latestPipeline.details.status" :size="24" />
<span class="prepend-left-8">
......@@ -62,7 +68,7 @@ export default {
</span>
</header>
<empty-state
v-if="latestPipeline === false"
v-if="!latestPipeline"
:help-page-path="links.ciHelpPagePath"
:empty-state-svg-path="pipelinesEmptyStateSvgPath"
:can-set-ci="true"
......
......@@ -10,6 +10,7 @@ export default {
},
[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](state, pipeline) {
state.isLoadingPipeline = false;
state.hasLoadedPipeline = true;
if (pipeline) {
state.latestPipeline = {
......@@ -34,7 +35,7 @@ export default {
};
});
} else {
state.latestPipeline = false;
state.latestPipeline = null;
}
},
[types.REQUEST_JOBS](state, id) {
......
export default () => ({
isLoadingPipeline: true,
hasLoadedPipeline: false,
isLoadingJobs: false,
latestPipeline: null,
stages: [],
......
......@@ -11,6 +11,8 @@ describe('IDE pipelines list', () => {
let vm;
let mock;
const findLoadingState = () => vm.$el.querySelector('.loading-container');
beforeEach(done => {
const store = createStore();
......@@ -95,7 +97,7 @@ describe('IDE pipelines list', () => {
describe('empty state', () => {
it('renders pipelines empty state', done => {
vm.$store.state.pipelines.latestPipeline = false;
vm.$store.state.pipelines.latestPipeline = null;
vm.$nextTick(() => {
expect(vm.$el.querySelector('.empty-state')).not.toBe(null);
......@@ -106,15 +108,30 @@ describe('IDE pipelines list', () => {
});
describe('loading state', () => {
it('renders loading state when there is no latest pipeline', done => {
vm.$store.state.pipelines.latestPipeline = null;
beforeEach(() => {
vm.$store.state.pipelines.isLoadingPipeline = true;
});
vm.$nextTick(() => {
expect(vm.$el.querySelector('.loading-container')).not.toBe(null);
it('does not render when pipeline has loaded before', done => {
vm.$store.state.pipelines.hasLoadedPipeline = true;
done();
});
vm.$nextTick()
.then(() => {
expect(findLoadingState()).toBe(null);
})
.then(done)
.catch(done.fail);
});
it('renders loading state when there is no latest pipeline', done => {
vm.$store.state.pipelines.hasLoadedPipeline = false;
vm.$nextTick()
.then(() => {
expect(findLoadingState()).not.toBe(null);
})
.then(done)
.catch(done.fail);
});
});
});
......@@ -27,63 +27,71 @@ describe('IDE pipelines mutations', () => {
});
describe(types.RECEIVE_LASTEST_PIPELINE_SUCCESS, () => {
it('sets loading to false on success', () => {
mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](
mockedState,
fullPipelinesResponse.data.pipelines[0],
);
const itSetsPipelineLoadingStates = () => {
it('sets has loaded to true', () => {
expect(mockedState.hasLoadedPipeline).toBe(true);
});
expect(mockedState.isLoadingPipeline).toBe(false);
});
it('sets loading to false on success', () => {
expect(mockedState.isLoadingPipeline).toBe(false);
});
};
describe('with pipeline', () => {
beforeEach(() => {
mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](
mockedState,
fullPipelinesResponse.data.pipelines[0],
);
});
it('sets latestPipeline', () => {
mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](
mockedState,
fullPipelinesResponse.data.pipelines[0],
);
itSetsPipelineLoadingStates();
expect(mockedState.latestPipeline).toEqual({
id: '51',
path: 'test',
commit: { id: '123' },
details: { status: jasmine.any(Object) },
yamlError: undefined,
it('sets latestPipeline', () => {
expect(mockedState.latestPipeline).toEqual({
id: '51',
path: 'test',
commit: { id: '123' },
details: { status: jasmine.any(Object) },
yamlError: undefined,
});
});
});
it('does not set latest pipeline if pipeline is null', () => {
mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](mockedState, null);
expect(mockedState.latestPipeline).toEqual(false);
it('sets stages', () => {
expect(mockedState.stages.length).toBe(2);
expect(mockedState.stages).toEqual([
{
id: 0,
dropdownPath: stages[0].dropdown_path,
name: stages[0].name,
status: stages[0].status,
isCollapsed: false,
isLoading: false,
jobs: [],
},
{
id: 1,
dropdownPath: stages[1].dropdown_path,
name: stages[1].name,
status: stages[1].status,
isCollapsed: false,
isLoading: false,
jobs: [],
},
]);
});
});
it('sets stages', () => {
mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](
mockedState,
fullPipelinesResponse.data.pipelines[0],
);
describe('with null', () => {
beforeEach(() => {
mutations[types.RECEIVE_LASTEST_PIPELINE_SUCCESS](mockedState, null);
});
expect(mockedState.stages.length).toBe(2);
expect(mockedState.stages).toEqual([
{
id: 0,
dropdownPath: stages[0].dropdown_path,
name: stages[0].name,
status: stages[0].status,
isCollapsed: false,
isLoading: false,
jobs: [],
},
{
id: 1,
dropdownPath: stages[1].dropdown_path,
name: stages[1].name,
status: stages[1].status,
isCollapsed: false,
isLoading: false,
jobs: [],
},
]);
itSetsPipelineLoadingStates();
it('does not set latest pipeline if pipeline is null', () => {
expect(mockedState.latestPipeline).toEqual(null);
});
});
});
......
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