Merge branch '9826-fix-broken-downstreams' into 'master'

Renders inline downstream & upstream pipelines

Closes #9826

See merge request gitlab-org/gitlab-ee!9627

ee/app/assets/javascripts/pipelines/components/graph/linked_pipeline.vue

@@ -48,9 +48,11 @@ export default {
       v-gl-tooltip
       :title="tooltipText"
       class="js-linked-pipeline-content linked-pipeline-content"
+      :class="`js-pipeline-expand-${pipeline.id}`"
       @click="onClickLinkedPipeline"
     >
-      <ci-status :status="pipelineStatus" class="js-linked-pipeline-status" />
+      <gl-loading-icon v-if="pipeline.isLoading" class="js-linked-pipeline-loading d-inline" />
+      <ci-status v-else :status="pipelineStatus" class="js-linked-pipeline-status" />
 
       <span class="str-truncated align-bottom"> {{ projectName }} &#8226; #{{ pipeline.id }} </span>
     </gl-button>

ee/app/assets/javascripts/pipelines/mixins/graph_pipeline_bundle_mixin.js

@@ -3,6 +3,19 @@ import { __ } from '~/locale';
 
 export default {
   methods: {
+    getExpandedPipelines(pipeline) {
+      this.mediator.service
+        .getPipeline(this.mediator.getExpandedParameters())
+        .then(response => {
+          this.mediator.store.toggleLoading(pipeline);
+          this.mediator.store.storePipeline(response.data);
+          this.mediator.poll.enable({ data: this.mediator.getExpandedParameters() });
+        })
+        .catch(() => {
+          this.mediator.store.toggleLoading(pipeline);
+          flash(__('An error occurred while fetching the pipeline.'));
+        });
+    },
     /**
      * Called when a linked pipeline is clicked.
      *
@@ -17,8 +30,15 @@ export default {
     clickPipeline(parentPipeline, pipeline, openMethod, closeMethod) {
       if (!pipeline.isExpanded) {
         this.mediator.store[openMethod](parentPipeline, pipeline);
+        this.mediator.store.toggleLoading(pipeline);
+        this.mediator.poll.stop();
+
+        this.getExpandedPipelines(pipeline);
       } else {
         this.mediator.store[closeMethod](pipeline);
+        this.mediator.poll.stop();
+
+        this.mediator.poll.enable({ data: this.mediator.getExpandedParameters() });
       }
     },
     clickTriggeredByPipeline(parentPipeline, pipeline) {

ee/app/assets/javascripts/pipelines/stores/pipeline_store.js

@@ -6,6 +6,11 @@ import CePipelineStore from '~/pipelines/stores/pipeline_store';
  * Extends CE store with the logic to handle the upstream/downstream pipelines
  */
 export default class PipelineStore extends CePipelineStore {
+  constructor() {
+    super();
+
+    this.state.expandedPipelines = [];
+  }
   /**
    * For the triggered pipelines adds the `isExpanded` key
    *
@@ -15,17 +20,31 @@ export default class PipelineStore extends CePipelineStore {
    * @param {Object} pipeline
    */
   storePipeline(pipeline = {}) {
-    super.storePipeline(pipeline);
+    const pipelineCopy = Object.assign({}, pipeline);
 
-    if (pipeline.triggered_by) {
-      this.state.pipeline.triggered_by = [pipeline.triggered_by];
+    if (pipelineCopy.triggered_by) {
+      pipelineCopy.triggered_by = [pipelineCopy.triggered_by];
 
-      this.parseTriggeredByPipelines(this.state.pipeline.triggered_by[0]);
+      const oldTriggeredBy =
+        this.state.pipeline &&
+        this.state.pipeline.triggered_by &&
+        this.state.pipeline.triggered_by[0];
+
+      this.parseTriggeredByPipelines(oldTriggeredBy, pipelineCopy.triggered_by[0]);
     }
 
-    if (pipeline.triggered && pipeline.triggered.length) {
-      this.state.pipeline.triggered.forEach(el => this.parseTriggeredPipelines(el));
+    if (pipelineCopy.triggered && pipelineCopy.triggered.length) {
+      pipelineCopy.triggered.forEach(el => {
+        const oldPipeline =
+          this.state.pipeline &&
+          this.state.pipeline.triggered &&
+          this.state.pipeline.triggered.find(element => element.id === el.id);
+
+        this.parseTriggeredPipelines(oldPipeline, el);
+      });
     }
+
+    this.state.pipeline = pipelineCopy;
   }
 
   /**
@@ -38,15 +57,18 @@ export default class PipelineStore extends CePipelineStore {
    * @param {Array} parentPipeline
    * @param {Object} pipeline
    */
-  parseTriggeredByPipelines(pipeline) {
+  parseTriggeredByPipelines(oldPipeline = {}, newPipeline) {
     // keep old value in case it's opened because we're polling
-    Vue.set(pipeline, 'isExpanded', pipeline.isExpanded || false);
 
-    if (pipeline.triggered_by) {
-      if (!_.isArray(pipeline.triggered_by)) {
-        Object.assign(pipeline, { triggered_by: [pipeline.triggered_by] });
+    Vue.set(newPipeline, 'isExpanded', oldPipeline.isExpanded || false);
+    // add isLoading property
+    Vue.set(newPipeline, 'isLoading', false);
+
+    if (newPipeline.triggered_by) {
+      if (!_.isArray(newPipeline.triggered_by)) {
+        Object.assign(newPipeline, { triggered_by: [newPipeline.triggered_by] });
       }
-      this.parseTriggeredByPipelines(pipeline.triggered_by[0]);
+      this.parseTriggeredByPipelines(oldPipeline, newPipeline.triggered_by[0]);
     }
   }
 
@@ -55,12 +77,19 @@ export default class PipelineStore extends CePipelineStore {
    * @param {Array} parentPipeline
    * @param {Object} pipeline
    */
-  parseTriggeredPipelines(pipeline) {
+  parseTriggeredPipelines(oldPipeline = {}, newPipeline) {
     // keep old value in case it's opened because we're polling
-    Vue.set(pipeline, 'isExpanded', pipeline.isExpanded || false);
+    Vue.set(newPipeline, 'isExpanded', oldPipeline.isExpanded || false);
 
-    if (pipeline.triggered && pipeline.triggered.length > 0) {
-      pipeline.triggered.forEach(el => this.parseTriggeredPipelines(el));
+    // add isLoading property
+    Vue.set(newPipeline, 'isLoading', false);
+
+    if (newPipeline.triggered && newPipeline.triggered.length > 0) {
+      newPipeline.triggered.forEach(el => {
+        const oldTriggered =
+          oldPipeline.triggered && oldPipeline.triggered.find(element => element.id === el.id);
+        this.parseTriggeredPipelines(oldTriggered, el);
+      });
     }
   }
 
@@ -70,7 +99,7 @@ export default class PipelineStore extends CePipelineStore {
    * @param {Object} pipeline
    */
   resetTriggeredByPipeline(parentPipeline, pipeline) {
-    parentPipeline.triggered_by.forEach(el => PipelineStore.closePipeline(el));
+    parentPipeline.triggered_by.forEach(el => this.closePipeline(el));
 
     if (pipeline.triggered_by && pipeline.triggered_by) {
       this.resetTriggeredByPipeline(pipeline, pipeline.triggered_by);
@@ -85,7 +114,7 @@ export default class PipelineStore extends CePipelineStore {
     // first we need to reset all triggeredBy pipelines
     this.resetTriggeredByPipeline(parentPipeline, pipeline);
 
-    PipelineStore.openPipeline(pipeline);
+    this.openPipeline(pipeline);
   }
 
   /**
@@ -94,7 +123,7 @@ export default class PipelineStore extends CePipelineStore {
    * @param {Object} pipeline
    */
   closeTriggeredByPipeline(pipeline) {
-    PipelineStore.closePipeline(pipeline);
+    this.closePipeline(pipeline);
 
     if (pipeline.triggered_by && pipeline.triggered_by.length) {
       pipeline.triggered_by.forEach(triggeredBy => this.closeTriggeredByPipeline(triggeredBy));
@@ -121,7 +150,8 @@ export default class PipelineStore extends CePipelineStore {
    */
   openTriggeredPipeline(parentPipeline, pipeline) {
     this.resetTriggeredPipelines(parentPipeline, pipeline);
-    PipelineStore.openPipeline(pipeline);
+
+    this.openPipeline(pipeline);
   }
 
   /**
@@ -129,7 +159,7 @@ export default class PipelineStore extends CePipelineStore {
    * @param {Object} pipeline
    */
   closeTriggeredPipeline(pipeline) {
-    PipelineStore.closePipeline(pipeline);
+    this.closePipeline(pipeline);
 
     if (pipeline.triggered && pipeline.triggered.length) {
       pipeline.triggered.forEach(triggered => this.closeTriggeredPipeline(triggered));
@@ -140,15 +170,31 @@ export default class PipelineStore extends CePipelineStore {
    * Utility function, Closes the given pipeline
    * @param {Object} pipeline
    */
-  static closePipeline(pipeline) {
+  closePipeline(pipeline) {
     Vue.set(pipeline, 'isExpanded', false);
+    // remove the pipeline from the parameters
+    this.removeExpandedPipelineToRequestData(pipeline.id);
   }
 
   /**
    * Utility function, Opens the given pipeline
    * @param {Object} pipeline
    */
-  static openPipeline(pipeline) {
+  openPipeline(pipeline) {
     Vue.set(pipeline, 'isExpanded', true);
+    // add the pipeline to the parameters
+    this.addExpandedPipelineToRequestData(pipeline.id);
+  }
+  // eslint-disable-next-line class-methods-use-this
+  toggleLoading(pipeline) {
+    Vue.set(pipeline, 'isLoading', !pipeline.isLoading);
+  }
+
+  addExpandedPipelineToRequestData(id) {
+    this.state.expandedPipelines.push(id);
+  }
+
+  removeExpandedPipelineToRequestData(id) {
+    this.state.expandedPipelines.splice(this.state.expandedPipelines.findIndex(el => el === id), 1);
   }
 }

ee/changelogs/unreleased/9826-fix-broken-downstreams.yml

+---
+title: Renders inline downstream & upstream pipelines
+merge_request: 9627
+author:
+type: fixed

ee/db/fixtures/development/25_downstream_pipelines.rb

+# frozen_string_literal: true
+# EE fixture
+Gitlab::Seeder.quiet do
+  Project.all.sample(5).each do |project|
+    project.ci_pipelines.all.sample(2).each do |pipeline|
+      next if pipeline.source_pipeline
+
+      target_pipeline = Ci::Pipeline
+        .where.not(project: project)
+        .order('random()').first
+
+      # link to source pipeline
+      pipeline.sourced_pipelines.create!(
+        source_job: pipeline.builds.all.sample,
+        source_project: pipeline.project,
+        project: target_pipeline.project,
+        pipeline: target_pipeline
+      )
+    end
+  end
+end

ee/spec/features/projects/pipelines/pipeline_spec.rb

@@ -6,9 +6,89 @@ describe 'Pipeline', :js do
 
   before do
     sign_in(user)
+
     project.add_developer(user)
   end
 
+  describe 'GET /:project/pipelines/:id' do
+    let(:pipeline) { create(:ci_pipeline, :with_job, project: project, ref: 'master', sha: project.commit.id, user: user) }
+
+    subject { visit project_pipeline_path(project, pipeline) }
+
+    context 'triggered and triggered by pipelines' do
+      let(:upstream_pipeline) { create(:ci_pipeline, :with_job) }
+      let(:downstream_pipeline) { create(:ci_pipeline, :with_job) }
+
+      before do
+        upstream_pipeline.project.add_developer(user)
+        downstream_pipeline.project.add_developer(user)
+
+        create_link(upstream_pipeline, pipeline)
+        create_link(pipeline, downstream_pipeline)
+      end
+
+      it 'renders upstream pipeline' do
+        subject
+
+        expect(page).to have_content(upstream_pipeline.id)
+        expect(page).to have_content(upstream_pipeline.project.name)
+      end
+
+      context 'expands the upstream pipeline on click' do
+        it 'should expand the upstream on click' do
+          subject
+
+          page.find(".js-pipeline-expand-#{upstream_pipeline.id}").click
+          wait_for_requests
+          expect(page).to have_selector(".js-upstream-pipeline-#{upstream_pipeline.id}")
+        end
+
+        it 'should close the expanded upstream on click' do
+          subject
+
+          # open
+          page.find(".js-pipeline-expand-#{upstream_pipeline.id}").click
+          wait_for_requests
+
+          # close
+          page.find(".js-pipeline-expand-#{upstream_pipeline.id}").click
+
+          expect(page).not_to have_selector(".js-upstream-pipeline-#{upstream_pipeline.id}")
+        end
+      end
+
+      it 'renders downstream pipeline' do
+        subject
+
+        expect(page).to have_content(downstream_pipeline.id)
+        expect(page).to have_content(downstream_pipeline.project.name)
+      end
+
+      context 'expands the downstream pipeline on click' do
+        it 'should expand the downstream on click' do
+          subject
+
+          page.find(".js-pipeline-expand-#{downstream_pipeline.id}").click
+          wait_for_requests
+          expect(page).to have_selector(".js-downstream-pipeline-#{downstream_pipeline.id}")
+        end
+
+        it 'should close the expanded downstream on click' do
+          subject
+
+          # open
+          page.find(".js-pipeline-expand-#{downstream_pipeline.id}").click
+          wait_for_requests
+
+          # close
+          page.find(".js-pipeline-expand-#{downstream_pipeline.id}").click
+
+          expect(page).not_to have_selector(".js-downstream-pipeline-#{downstream_pipeline.id}")
+        end
+      end
+    end
+  end
+
   describe 'GET /:project/pipelines/:id/security' do
     let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) }
 
@@ -83,4 +163,15 @@ describe 'Pipeline', :js do
       end
     end
   end
+
+  private
+
+  def create_link(source_pipeline, pipeline)
+    source_pipeline.sourced_pipelines.create!(
+      source_job: source_pipeline.builds.all.sample,
+      source_project: source_pipeline.project,
+      project: pipeline.project,
+      pipeline: pipeline
+    )
+  end
 end

ee/spec/javascripts/pipelines/graph/linked_pipeline_spec.js

@@ -67,6 +67,24 @@ describe('Linked pipeline', () => {
       expect(titleAttr).toContain(mockPipeline.project.name);
       expect(titleAttr).toContain(mockPipeline.details.status.label);
     });
+
+    it('does not render the loading icon when isLoading is false', () => {
+      expect(vm.$el.querySelector('.js-linked-pipeline-loading')).toBeNull();
+    });
+  });
+
+  describe('when isLoading is true', () => {
+    const props = {
+      pipeline: { ...mockPipeline, isLoading: true },
+    };
+
+    beforeEach(() => {
+      vm = mountComponent(Component, props);
+    });
+
+    it('renders a loading icon', () => {
+      expect(vm.$el.querySelector('.js-linked-pipeline-loading')).not.toBeNull();
+    });
   });
 
   describe('on click', () => {

ee/spec/javascripts/pipelines/stores/pipeline_store.js

@@ -20,26 +20,30 @@ describe('EE Pipeline store', () => {
         expect(store.state.pipeline.triggered_by.length).toEqual(1);
       });
 
-      it('adds isExpanding key set to false', () => {
+      it('adds isExpanding & isLoading keys set to false', () => {
         expect(store.state.pipeline.triggered_by[0].isExpanded).toEqual(false);
+        expect(store.state.pipeline.triggered_by[0].isLoading).toEqual(false);
       });
 
       it('parses nested triggered_by', () => {
         expect(store.state.pipeline.triggered_by[0].triggered_by.length).toEqual(1);
         expect(store.state.pipeline.triggered_by[0].triggered_by[0].isExpanded).toEqual(false);
+        expect(store.state.pipeline.triggered_by[0].triggered_by[0].isLoading).toEqual(false);
       });
     });
 
     describe('triggered', () => {
-      it('adds isExpanding key set to false for each triggered pipeline', () => {
+      it('adds isExpanding & isLoading keys set to false for each triggered pipeline', () => {
         store.state.pipeline.triggered.forEach(pipeline => {
           expect(pipeline.isExpanded).toEqual(false);
+          expect(pipeline.isLoading).toEqual(false);
         });
       });
 
       it('parses nested triggered pipelines', () => {
         store.state.pipeline.triggered[1].triggered.forEach(pipeline => {
           expect(pipeline.isExpanded).toEqual(false);
+          expect(pipeline.isLoading).toEqual(false);
         });
       });
     });
@@ -130,4 +134,32 @@ describe('EE Pipeline store', () => {
       expect(store.state.pipeline.triggered[0].isExpanded).toEqual(false);
     });
   });
+
+  describe('toggleLoading', () => {
+    beforeEach(() => {
+      store.storePipeline(data);
+    });
+
+    it('toggles the isLoading property for the given pipeline', () => {
+      store.togglePipeline(store.state.pipeline.triggered[0]);
+
+      expect(store.state.pipeline.triggered[0].isLoading).toEqual(true);
+    });
+  });
+
+  describe('addExpandedPipelineToRequestData', () => {
+    it('pushes the given id to expandedPipelines array', () => {
+      store.addExpandedPipelineToRequestData('213231');
+
+      expect(store.state.expandedPipelines).toEqual(['213231']);
+    });
+  });
+
+  describe('removeExpandedPipelineToRequestData', () => {
+    it('pushes the given id to expandedPipelines array', () => {
+      store.removeExpandedPipelineToRequestData('213231');
+
+      expect(store.state.expandedPipelines).toEqual([]);
+    });
+  });
 });