Merge branch '29539-fix-pipelines-container-width-with-parallel-diff' into 'master'

app/assets/javascripts/commit/pipelines/pipelines_table.js

@@ -33,12 +33,11 @@ export default Vue.component('pipelines-table', {
    * @return {Object}
    */
   data() {
-    const pipelinesTableData = document.querySelector('#commit-pipeline-table-view').dataset;
     const store = new PipelineStore();
 
     return {
-      endpoint: pipelinesTableData.endpoint,
-      helpPagePath: pipelinesTableData.helpPagePath,
+      endpoint: null,
+      helpPagePath: null,
       store,
       state: store.state,
       isLoading: false,
@@ -65,6 +64,8 @@ export default Vue.component('pipelines-table', {
    *
    */
   beforeMount() {
+    this.endpoint = this.$el.dataset.endpoint;
+    this.helpPagePath = this.$el.dataset.helpPagePath;
     this.service = new PipelinesService(this.endpoint);
 
     this.fetchPipelines();

app/assets/javascripts/merge_request_tabs.js

@@ -4,8 +4,10 @@
 
 import Cookies from 'js-cookie';
 
-require('./breakpoints');
-require('./flash');
+import CommitPipelinesTable from './commit/pipelines/pipelines_table';
+
+import './breakpoints';
+import './flash';
 
 /* eslint-disable max-len */
 // MergeRequestTabs
@@ -97,6 +99,13 @@ require('./flash');
         .off('click', this.clickTab);
     }
 
+    destroy() {
+      this.unbindEvents();
+      if (this.commitPipelinesTable) {
+        this.commitPipelinesTable.$destroy();
+      }
+    }
+
     showTab(e) {
       e.preventDefault();
       this.activateTab($(e.target).data('action'));
@@ -128,12 +137,8 @@ require('./flash');
           this.expandViewContainer();
         }
       } else if (action === 'pipelines') {
-        if (this.pipelinesLoaded) {
-          return;
-        }
-        const pipelineTableViewEl = document.querySelector('#commit-pipeline-table-view');
-        gl.commits.pipelines.PipelinesTableBundle.$mount(pipelineTableViewEl);
-        this.pipelinesLoaded = true;
+        this.resetViewContainer();
+        this.loadPipelines();
       } else {
         this.expandView();
         this.resetViewContainer();
@@ -222,6 +227,18 @@ require('./flash');
       });
     }
 
+    loadPipelines() {
+      if (this.pipelinesLoaded) {
+        return;
+      }
+      const pipelineTableViewEl = document.querySelector('#commit-pipeline-table-view');
+      // Could already be mounted from the `pipelines_bundle`
+      if (pipelineTableViewEl) {
+        this.commitPipelinesTable = new CommitPipelinesTable().$mount(pipelineTableViewEl);
+      }
+      this.pipelinesLoaded = true;
+    }
+
     loadDiff(source) {
       if (this.diffsLoaded) {
         return;

lib/tasks/karma.rake

 unless Rails.env.production?
   namespace :karma do
     desc 'GitLab | Karma | Generate fixtures for JavaScript tests'
-    RSpec::Core::RakeTask.new(:fixtures) do |t|
+    RSpec::Core::RakeTask.new(:fixtures, [:pattern]) do |t, args|
+      args.with_defaults(pattern: 'spec/javascripts/fixtures/*.rb')
       ENV['NO_KNAPSACK'] = 'true'
-      t.pattern = 'spec/javascripts/fixtures/*.rb'
+      t.pattern = args[:pattern]
       t.rspec_opts = '--format documentation'
     end
 

spec/javascripts/commit/pipelines/pipelines_spec.js

@@ -9,7 +9,7 @@ describe('Pipelines table in Commits and Merge requests', () => {
     loadFixtures('static/pipelines_table.html.raw');
   });
 
-  describe('successfull request', () => {
+  describe('successful request', () => {
     describe('without pipelines', () => {
       const pipelinesEmptyResponse = (request, next) => {
         next(request.respondWith(JSON.stringify([]), {
@@ -17,24 +17,25 @@ describe('Pipelines table in Commits and Merge requests', () => {
         }));
       };
 
-      beforeEach(() => {
+      beforeEach(function () {
         Vue.http.interceptors.push(pipelinesEmptyResponse);
+
+        this.component = new PipelinesTable({
+          el: document.querySelector('#commit-pipeline-table-view'),
+        });
       });
 
-      afterEach(() => {
+      afterEach(function () {
         Vue.http.interceptors = _.without(
           Vue.http.interceptors, pipelinesEmptyResponse,
         );
+        this.component.$destroy();
       });
 
-      it('should render the empty state', (done) => {
-        const component = new PipelinesTable({
-          el: document.querySelector('#commit-pipeline-table-view'),
-        });
-
+      it('should render the empty state', function (done) {
         setTimeout(() => {
-          expect(component.$el.querySelector('.empty-state')).toBeDefined();
-          expect(component.$el.querySelector('.realtime-loading')).toBe(null);
+          expect(this.component.$el.querySelector('.empty-state')).toBeDefined();
+          expect(this.component.$el.querySelector('.realtime-loading')).toBe(null);
           done();
         }, 1);
       });
@@ -49,22 +50,23 @@ describe('Pipelines table in Commits and Merge requests', () => {
 
       beforeEach(() => {
         Vue.http.interceptors.push(pipelinesResponse);
+
+        this.component = new PipelinesTable({
+          el: document.querySelector('#commit-pipeline-table-view'),
+        });
       });
 
       afterEach(() => {
         Vue.http.interceptors = _.without(
           Vue.http.interceptors, pipelinesResponse,
         );
+        this.component.$destroy();
       });
 
       it('should render a table with the received pipelines', (done) => {
-        const component = new PipelinesTable({
-          el: document.querySelector('#commit-pipeline-table-view'),
-        });
-
         setTimeout(() => {
-          expect(component.$el.querySelectorAll('table > tbody > tr').length).toEqual(1);
-          expect(component.$el.querySelector('.realtime-loading')).toBe(null);
+          expect(this.component.$el.querySelectorAll('table > tbody > tr').length).toEqual(1);
+          expect(this.component.$el.querySelector('.realtime-loading')).toBe(null);
           done();
         }, 0);
       });
@@ -78,24 +80,25 @@ describe('Pipelines table in Commits and Merge requests', () => {
       }));
     };
 
-    beforeEach(() => {
+    beforeEach(function () {
       Vue.http.interceptors.push(pipelinesErrorResponse);
+
+      this.component = new PipelinesTable({
+        el: document.querySelector('#commit-pipeline-table-view'),
+      });
     });
 
-    afterEach(() => {
+    afterEach(function () {
       Vue.http.interceptors = _.without(
         Vue.http.interceptors, pipelinesErrorResponse,
       );
+      this.component.$destroy();
     });
 
-    it('should render empty state', (done) => {
-      const component = new PipelinesTable({
-        el: document.querySelector('#commit-pipeline-table-view'),
-      });
-
+    it('should render empty state', function (done) {
       setTimeout(() => {
-        expect(component.$el.querySelector('.js-pipelines-error-state')).toBeDefined();
-        expect(component.$el.querySelector('.realtime-loading')).toBe(null);
+        expect(this.component.$el.querySelector('.js-pipelines-error-state')).toBeDefined();
+        expect(this.component.$el.querySelector('.realtime-loading')).toBe(null);
         done();
       }, 0);
     });

spec/javascripts/fixtures/merge_requests.rb

@@ -6,6 +6,15 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
   let(:admin) { create(:admin) }
   let(:namespace) { create(:namespace, name: 'frontend-fixtures' )}
   let(:project) { create(:project, namespace: namespace, path: 'merge-requests-project') }
+  let(:merge_request) { create(:merge_request, :with_diffs, source_project: project, target_project: project, description: '- [ ] Task List Item') }
+  let(:pipeline) do
+    create(
+      :ci_pipeline,
+      project: merge_request.source_project,
+      ref: merge_request.source_branch,
+      sha: merge_request.diff_head_sha
+    )
+  end
 
   render_views
 
@@ -18,7 +27,8 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
   end
 
   it 'merge_requests/merge_request_with_task_list.html.raw' do |example|
-    merge_request = create(:merge_request, :with_diffs, source_project: project, target_project: project, description: '- [ ] Task List Item')
+    create(:ci_build, :pending, pipeline: pipeline)
+
     render_merge_request(example.description, merge_request)
   end
 

spec/javascripts/merge_request_tabs_spec.js

@@ -38,6 +38,10 @@ require('vendor/jquery.scrollTo');
       }
     });
 
+    afterEach(function () {
+      this.class.destroy();
+    });
+
     describe('#activateTab', function () {
       beforeEach(function () {
         spyOn($, 'ajax').and.callFake(function () {});
@@ -200,6 +204,42 @@ require('vendor/jquery.scrollTo');
         expect(this.subject('show')).toBe('/foo/bar/merge_requests/1');
       });
     });
+
+    describe('#tabShown', () => {
+      beforeEach(function () {
+        loadFixtures('merge_requests/merge_request_with_task_list.html.raw');
+      });
+
+      describe('with "Side-by-side"/parallel diff view', () => {
+        beforeEach(function () {
+          this.class.diffViewType = () => 'parallel';
+        });
+
+        it('maintains `container-limited` for pipelines tab', function (done) {
+          const asyncClick = function (selector) {
+            return new Promise((resolve) => {
+              setTimeout(() => {
+                document.querySelector(selector).click();
+                resolve();
+              });
+            });
+          };
+
+          asyncClick('.merge-request-tabs .pipelines-tab a')
+            .then(() => asyncClick('.merge-request-tabs .diffs-tab a'))
+            .then(() => asyncClick('.merge-request-tabs .pipelines-tab a'))
+            .then(() => {
+              const hasContainerLimitedClass = document.querySelector('.content-wrapper .container-fluid').classList.contains('container-limited');
+              expect(hasContainerLimitedClass).toBe(true);
+            })
+            .then(done)
+            .catch((err) => {
+              done.fail(`Something went wrong clicking MR tabs: ${err.message}\n${err.stack}`);
+            });
+        });
+      });
+    });
+
     describe('#loadDiff', function () {
       it('requires an absolute pathname', function () {
         spyOn($, 'ajax').and.callFake(function (options) {

spec/javascripts/vue_shared/components/pipelines_table_spec.js

@@ -21,6 +21,10 @@ describe('Pipelines Table', () => {
       }).$mount();
     });
 
+    afterEach(() => {
+      component.$destroy();
+    });
+
     it('should render a table', () => {
       expect(component.$el).toEqual('TABLE');
     });