summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Slaughter <pslaughter@gitlab.com>2019-04-04 02:44:16 -0500
committerPaul Slaughter <pslaughter@gitlab.com>2019-04-04 02:54:31 -0500
commit1e58686a6affeb3673acdb98acbe2f29ce583fe7 (patch)
treeb9aad6c1fbb40eaef967bc77c29f9d1d5343456d
parent6202820c169ce1f1c5887a1e792455808a11401a (diff)
downloadgitlab-ce-59995-ide-status-bar-vue-render-error.tar.gz
Fix vue render error for IDE status bar59995-ide-status-bar-vue-render-error
**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.
-rw-r--r--app/assets/javascripts/ide/components/pipelines/list.vue14
-rw-r--r--app/assets/javascripts/ide/stores/modules/pipelines/mutations.js3
-rw-r--r--app/assets/javascripts/ide/stores/modules/pipelines/state.js1
-rw-r--r--spec/javascripts/ide/components/pipelines/list_spec.js31
-rw-r--r--spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js106
5 files changed, 94 insertions, 61 deletions
diff --git a/app/assets/javascripts/ide/components/pipelines/list.vue b/app/assets/javascripts/ide/components/pipelines/list.vue
index 451c8030e16..5ae73b2fc9c 100644
--- a/app/assets/javascripts/ide/components/pipelines/list.vue
+++ b/app/assets/javascripts/ide/components/pipelines/list.vue
@@ -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"
diff --git a/app/assets/javascripts/ide/stores/modules/pipelines/mutations.js b/app/assets/javascripts/ide/stores/modules/pipelines/mutations.js
index b4be100cb07..eaaa82cb339 100644
--- a/app/assets/javascripts/ide/stores/modules/pipelines/mutations.js
+++ b/app/assets/javascripts/ide/stores/modules/pipelines/mutations.js
@@ -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) {
diff --git a/app/assets/javascripts/ide/stores/modules/pipelines/state.js b/app/assets/javascripts/ide/stores/modules/pipelines/state.js
index 8651e267b53..8dfa0ec491f 100644
--- a/app/assets/javascripts/ide/stores/modules/pipelines/state.js
+++ b/app/assets/javascripts/ide/stores/modules/pipelines/state.js
@@ -1,5 +1,6 @@
export default () => ({
isLoadingPipeline: true,
+ hasLoadedPipeline: false,
isLoadingJobs: false,
latestPipeline: null,
stages: [],
diff --git a/spec/javascripts/ide/components/pipelines/list_spec.js b/spec/javascripts/ide/components/pipelines/list_spec.js
index 68487733cb9..80829f2358a 100644
--- a/spec/javascripts/ide/components/pipelines/list_spec.js
+++ b/spec/javascripts/ide/components/pipelines/list_spec.js
@@ -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);
});
});
});
diff --git a/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js b/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js
index eb7346bd5fc..b558c45f574 100644
--- a/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js
+++ b/spec/javascripts/ide/stores/modules/pipelines/mutations_spec.js
@@ -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);
+ });
});
});