From bd1122ee2fd5c8083c945390a1593b2b5774d8a1 Mon Sep 17 00:00:00 2001
From: Paul Slaughter <pslaughter@gitlab.com>
Date: Thu, 4 Apr 2019 02:44:16 -0500
Subject: 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.
---
 .../javascripts/ide/components/pipelines/list.vue  |  14 ++-
 .../ide/stores/modules/pipelines/mutations.js      |   3 +-
 .../ide/stores/modules/pipelines/state.js          |   1 +
 .../ide/components/pipelines/list_spec.js          |  31 ++++--
 .../ide/stores/modules/pipelines/mutations_spec.js | 106 +++++++++++----------
 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);
+      });
     });
   });
 
-- 
cgit v1.2.1