Resolve "Merge request widget - CI information has different margins"

6 files changed, 204 insertions, 174 deletions

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js
deleted file mode 100644
index 029832bdd27..00000000000
--- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.js
+++ /dev/null
@@ -1,90 +0,0 @@
-import PipelineStage from '../../pipelines/components/stage.vue';
-import ciIcon from '../../vue_shared/components/ci_icon.vue';
-import icon from '../../vue_shared/components/icon.vue';
-
-export default {
-  name: 'MRWidgetPipeline',
-  props: {
-    mr: { type: Object, required: true },
-  },
-  components: {
-    'pipeline-stage': PipelineStage,
-    ciIcon,
-    icon,
-  },
-  computed: {
-    hasPipeline() {
-      return this.mr.pipeline && Object.keys(this.mr.pipeline).length > 0;
-    },
-    hasCIError() {
-      const { hasCI, ciStatus } = this.mr;
-
-      return hasCI && !ciStatus;
-    },
-    stageText() {
-      return this.mr.pipeline.details.stages.length > 1 ? 'stages' : 'stage';
-    },
-    status() {
-      return this.mr.pipeline.details.status || {};
-    },
-  },
-  template: `
-    <div
-      v-if="hasPipeline || hasCIError"
-      class="mr-widget-heading">
-      <div class="ci-widget media">
-        <template v-if="hasCIError">
-          <div class="ci-status-icon ci-status-icon-failed ci-error js-ci-error append-right-10">
-            <span
-              aria-hidden="true">
-              <icon
-                name="status_failed"/>
-            </span>
-          </div>
-          <div class="media-body">
-            Could not connect to the CI server. Please check your settings and try again
-          </div>
-        </template>
-        <template v-else-if="hasPipeline">
-          <div class="ci-status-icon append-right-10">
-            <a
-              class="icon-link"
-              :href="this.status.details_path">
-              <ci-icon :status="status" />
-            </a>
-          </div>
-          <div class="media-body">
-            <span>
-              Pipeline
-              <a
-                :href="mr.pipeline.path"
-                class="pipeline-id">#{{mr.pipeline.id}}</a>
-            </span>
-            <span class="mr-widget-pipeline-graph">
-              <span class="stage-cell">
-                <div
-                  v-if="mr.pipeline.details.stages.length > 0"
-                  v-for="stage in mr.pipeline.details.stages"
-                  class="stage-container dropdown js-mini-pipeline-graph">
-                  <pipeline-stage :stage="stage" />
-                </div>
-              </span>
-            </span>
-            <span>
-              {{mr.pipeline.details.status.label}} for
-              <a
-                :href="mr.pipeline.commit.commit_path"
-                class="commit-sha js-commit-link">
-                {{mr.pipeline.commit.short_id}}</a>.
-            </span>
-            <span
-              v-if="mr.pipeline.coverage"
-              class="js-mr-coverage">
-              Coverage {{mr.pipeline.coverage}}%
-            </span>
-          </div>
-        </template>
-      </div>
-    </div>
-  `,
-};
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue
new file mode 100644
index 00000000000..dbc65462377
--- /dev/null
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue
@@ -0,0 +1,104 @@
+<script>
+  import pipelineStage from '../../pipelines/components/stage.vue';
+  import ciIcon from '../../vue_shared/components/ci_icon.vue';
+  import icon from '../../vue_shared/components/icon.vue';
+
+  export default {
+    name: 'MRWidgetPipeline',
+    props: {
+      pipeline: {
+        type: Object,
+        required: true,
+      },
+      // This prop needs to be camelCase, html attributes are case insensive
+      // https://vuejs.org/v2/guide/components.html#camelCase-vs-kebab-case
+      hasCi: {
+        type: Boolean,
+        required: false,
+      },
+      ciStatus: {
+        type: String,
+        required: false,
+      },
+    },
+    components: {
+      pipelineStage,
+      ciIcon,
+      icon,
+    },
+    computed: {
+      hasPipeline() {
+        return this.pipeline && Object.keys(this.pipeline).length > 0;
+      },
+      hasCIError() {
+        return this.hasCi && !this.ciStatus;
+      },
+      status() {
+        return this.pipeline.details &&
+          this.pipeline.details.status ? this.pipeline.details.status : {};
+      },
+      hasStages() {
+        return this.pipeline.details &&
+          this.pipeline.details.stages &&
+          this.pipeline.details.stages.length;
+      },
+    },
+  };
+</script>
+
+<template>
+  <div
+    v-if="hasPipeline || hasCIError"
+    class="mr-widget-heading">
+    <div class="ci-widget media">
+      <template v-if="hasCIError">
+        <div class="ci-status-icon ci-status-icon-failed ci-error js-ci-error append-right-10">
+          <icon name="status_failed" />
+        </div>
+        <div class="media-body">
+          Could not connect to the CI server. Please check your settings and try again
+        </div>
+      </template>
+      <template v-else-if="hasPipeline">
+        <a
+          class="append-right-10"
+          :href="this.status.details_path">
+          <ci-icon :status="status" />
+        </a>
+
+        <div class="media-body">
+          Pipeline
+          <a
+            :href="pipeline.path"
+            class="pipeline-id">
+            #{{pipeline.id}}
+          </a>
+
+          {{pipeline.details.status.label}} for
+
+          <a
+            :href="pipeline.commit.commit_path"
+            class="commit-sha js-commit-link">
+            {{pipeline.commit.short_id}}</a>.
+
+          <span class="mr-widget-pipeline-graph">
+            <span class="stage-cell">
+              <div
+                v-if="hasStages"
+                v-for="(stage, i) in pipeline.details.stages"
+                :key="i"
+                class="stage-container dropdown js-mini-pipeline-graph">
+                <pipeline-stage :stage="stage" />
+              </div>
+            </span>
+          </span>
+
+          <template v-if="pipeline.coverage">
+            Coverage {{pipeline.coverage}}%
+          </template>
+
+        </div>
+      </template>
+    </div>
+  </div>
+</template>
diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js
index 49340c232c8..5bd8b99420a 100644
--- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js
+++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js
@@ -13,7 +13,7 @@ export { default as Vue } from 'vue';
 export { default as SmartInterval } from '~/smart_interval';
 export { default as WidgetHeader } from './components/mr_widget_header';
 export { default as WidgetMergeHelp } from './components/mr_widget_merge_help';
-export { default as WidgetPipeline } from './components/mr_widget_pipeline';
+export { default as WidgetPipeline } from './components/mr_widget_pipeline.vue';
 export { default as WidgetDeployment } from './components/mr_widget_deployment';
 export { default as WidgetRelatedLinks } from './components/mr_widget_related_links';
 export { default as MergedState } from './components/states/mr_widget_merged';
diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
index aaff9ee6518..f82938aa8a9 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
@@ -236,7 +236,10 @@ export default {
       <mr-widget-header :mr="mr" />
       <mr-widget-pipeline
         v-if="shouldRenderPipelines"
-        :mr="mr" />
+        :pipeline="mr.pipeline"
+        :ci-status="mr.ciStatus"
+        :has-ci="mr.hasCI"
+        />
       <mr-widget-deployment
         v-if="shouldRenderDeployments"
         :mr="mr"
diff --git a/changelogs/unreleased/38395-mr-widget-ci.yml b/changelogs/unreleased/38395-mr-widget-ci.yml
new file mode 100644
index 00000000000..5109f1bec44
--- /dev/null
+++ b/changelogs/unreleased/38395-mr-widget-ci.yml
@@ -0,0 +1,6 @@
+---
+title: Moves mini graph of pipeline to the end of sentence in MR widget. Cleans HTML
+  and tests
+merge_request:
+author:
+type: fixed
diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js
index 33ed0cb4342..d7af956c9c1 100644
--- a/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_pipeline_spec.js
@@ -1,140 +1,147 @@
 import Vue from 'vue';
-import pipelineComponent from '~/vue_merge_request_widget/components/mr_widget_pipeline';
+import pipelineComponent from '~/vue_merge_request_widget/components/mr_widget_pipeline.vue';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 import mockData from '../mock_data';
 
-const createComponent = (mr) => {
-  const Component = Vue.extend(pipelineComponent);
-  return new Component({
-    el: document.createElement('div'),
-    propsData: { mr },
-  });
-};
-
 describe('MRWidgetPipeline', () => {
-  describe('props', () => {
-    it('should have props', () => {
-      const { mr } = pipelineComponent.props;
+  let vm;
+  let Component;
 
-      expect(mr.type instanceof Object).toBeTruthy();
-      expect(mr.required).toBeTruthy();
-    });
+  beforeEach(() => {
+    Component = Vue.extend(pipelineComponent);
   });
 
-  describe('components', () => {
-    it('should have components added', () => {
-      expect(pipelineComponent.components['pipeline-stage']).toBeDefined();
-      expect(pipelineComponent.components.ciIcon).toBeDefined();
-    });
+  afterEach(() => {
+    vm.$destroy();
   });
 
   describe('computed', () => {
     describe('hasPipeline', () => {
       it('should return true when there is a pipeline', () => {
-        expect(Object.keys(mockData.pipeline).length).toBeGreaterThan(0);
-
-        const vm = createComponent({
+        vm = mountComponent(Component, {
           pipeline: mockData.pipeline,
+          ciStatus: 'success',
+          hasCi: true,
         });
 
-        expect(vm.hasPipeline).toBeTruthy();
+        expect(vm.hasPipeline).toEqual(true);
       });
 
       it('should return false when there is no pipeline', () => {
-        const vm = createComponent({
-          pipeline: null,
+        vm = mountComponent(Component, {
+          pipeline: {},
         });
 
-        expect(vm.hasPipeline).toBeFalsy();
+        expect(vm.hasPipeline).toEqual(false);
       });
     });
 
     describe('hasCIError', () => {
       it('should return false when there is no CI error', () => {
-        const vm = createComponent({
+        vm = mountComponent(Component, {
           pipeline: mockData.pipeline,
-          hasCI: true,
+          hasCi: true,
           ciStatus: 'success',
         });
 
-        expect(vm.hasCIError).toBeFalsy();
+        expect(vm.hasCIError).toEqual(false);
       });
 
       it('should return true when there is a CI error', () => {
-        const vm = createComponent({
+        vm = mountComponent(Component, {
           pipeline: mockData.pipeline,
-          hasCI: true,
+          hasCi: true,
           ciStatus: null,
         });
 
-        expect(vm.hasCIError).toBeTruthy();
+        expect(vm.hasCIError).toEqual(true);
       });
     });
   });
 
-  describe('template', () => {
-    let vm;
-    let el;
-    const { pipeline } = mockData;
-    const mr = {
-      hasCI: true,
-      ciStatus: 'success',
-      pipelineDetailedStatus: pipeline.details.status,
-      pipeline,
-    };
-
-    beforeEach(() => {
-      vm = createComponent(mr);
-      el = vm.$el;
-    });
+  describe('rendered output', () => {
+    it('should render CI error', () => {
+      vm = mountComponent(Component, {
+        pipeline: mockData.pipeline,
+        hasCi: true,
+        ciStatus: null,
+      });
 
-    it('should render template elements correctly', () => {
-      expect(el.classList.contains('mr-widget-heading')).toBeTruthy();
-      expect(el.querySelectorAll('.ci-status-icon.ci-status-icon-success').length).toEqual(1);
-      expect(el.querySelector('.pipeline-id').textContent).toContain(`#${pipeline.id}`);
-      expect(el.innerText).toContain('passed');
-      expect(el.querySelector('.pipeline-id').getAttribute('href')).toEqual(pipeline.path);
-      expect(el.querySelectorAll('.stage-container').length).toEqual(2);
-      expect(el.querySelector('.js-ci-error')).toEqual(null);
-      expect(el.querySelector('.js-commit-link').getAttribute('href')).toEqual(pipeline.commit.commit_path);
-      expect(el.querySelector('.js-commit-link').textContent).toContain(pipeline.commit.short_id);
-      expect(el.querySelector('.js-mr-coverage').textContent).toContain(`Coverage ${pipeline.coverage}%`);
+      expect(
+        vm.$el.querySelector('.media-body').textContent.trim(),
+      ).toEqual('Could not connect to the CI server. Please check your settings and try again');
     });
 
-    it('should list single stage', (done) => {
-      pipeline.details.stages.splice(0, 1);
+    describe('with a pipeline', () => {
+      beforeEach(() => {
+        vm = mountComponent(Component, {
+          pipeline: mockData.pipeline,
+          hasCi: true,
+          ciStatus: 'success',
+        });
+      });
+
+      it('should render pipeline ID', () => {
+        expect(
+          vm.$el.querySelector('.pipeline-id').textContent.trim(),
+        ).toEqual(`#${mockData.pipeline.id}`);
+      });
+
+      it('should render pipeline status and commit id', () => {
+        expect(
+          vm.$el.querySelector('.media-body').textContent.trim(),
+        ).toContain(mockData.pipeline.details.status.label);
 
-      Vue.nextTick(() => {
-        expect(el.querySelectorAll('.stage-container button').length).toEqual(1);
-        done();
+        expect(
+          vm.$el.querySelector('.js-commit-link').textContent.trim(),
+        ).toEqual(mockData.pipeline.commit.short_id);
+
+        expect(
+          vm.$el.querySelector('.js-commit-link').getAttribute('href'),
+        ).toEqual(mockData.pipeline.commit.commit_path);
       });
-    });
 
-    it('should not have stages when there is no stage', (done) => {
-      vm.mr.pipeline.details.stages = [];
+      it('should render pipeline graph', () => {
+        expect(vm.$el.querySelector('.mr-widget-pipeline-graph')).toBeDefined();
+        expect(vm.$el.querySelectorAll('.stage-container').length).toEqual(mockData.pipeline.details.stages.length);
+      });
 
-      Vue.nextTick(() => {
-        expect(el.querySelectorAll('.stage-container button').length).toEqual(0);
-        done();
+      it('should render coverage information', () => {
+        expect(
+          vm.$el.querySelector('.media-body').textContent,
+        ).toContain(`Coverage ${mockData.pipeline.coverage}`);
       });
     });
 
-    it('should not have coverage text when pipeline has no coverage info', (done) => {
-      vm.mr.pipeline.coverage = null;
+    describe('without coverage', () => {
+      it('should not render a coverage', () => {
+        const mockCopy = Object.assign({}, mockData);
+        delete mockCopy.pipeline.coverage;
 
-      Vue.nextTick(() => {
-        expect(el.querySelector('.js-mr-coverage')).toEqual(null);
-        done();
+        vm = mountComponent(Component, {
+          pipeline: mockCopy.pipeline,
+          hasCi: true,
+          ciStatus: 'success',
+        });
+
+        expect(
+          vm.$el.querySelector('.media-body').textContent,
+        ).not.toContain('Coverage');
       });
     });
 
-    it('should show CI error when there is a CI error', (done) => {
-      vm.mr.ciStatus = null;
+    describe('without a pipeline graph', () => {
+      it('should not render a pipeline graph', () => {
+        const mockCopy = Object.assign({}, mockData);
+        delete mockCopy.pipeline.details.stages;
+
+        vm = mountComponent(Component, {
+          pipeline: mockCopy.pipeline,
+          hasCi: true,
+          ciStatus: 'success',
+        });
 
-      Vue.nextTick(() => {
-        expect(el.querySelectorAll('.js-ci-error').length).toEqual(1);
-        expect(el.innerText).toContain('Could not connect to the CI server');
-        expect(el.querySelector('.ci-status-icon svg use').getAttribute('xlink:href')).toContain('status_failed');
-        done();
+        expect(vm.$el.querySelector('.js-mini-pipeline-graph')).toEqual(null);
       });
     });
   });