diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 147634f1cc47e6a3991f48b9c01072cef6deaa9f..ab521c6c1fcd920b501068215480171a6659c016 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -135,8 +135,18 @@ new TreeView(); } break; + case 'projects:pipelines:builds': case 'projects:pipelines:show': - new gl.Pipelines(); + const { controllerAction } = document.querySelector('.js-pipeline-container').dataset; + + new gl.Pipelines({ + initTabs: true, + tabsOptions: { + action: controllerAction, + defaultAction: 'pipelines', + parentEl: '.pipelines-tabs', + }, + }); break; case 'groups:activity': new gl.Activities(); diff --git a/app/assets/javascripts/extensions/element.js.es6 b/app/assets/javascripts/extensions/element.js.es6 index 6d9b0c4bc3ecab8f58e5adf4286a27c882818262..3f12ad9ff9f1c933995e17514b39ae5191b1afa5 100644 --- a/app/assets/javascripts/extensions/element.js.es6 +++ b/app/assets/javascripts/extensions/element.js.es6 @@ -1,9 +1,20 @@ /* global Element */ -/* eslint-disable consistent-return, max-len */ - -Element.prototype.matches = Element.prototype.matches || Element.prototype.msMatchesSelector; +/* eslint-disable consistent-return, max-len, no-empty, no-plusplus, func-names */ Element.prototype.closest = Element.prototype.closest || function closest(selector, selectedElement = this) { if (!selectedElement) return; return selectedElement.matches(selector) ? selectedElement : Element.prototype.closest(selector, selectedElement.parentElement); }; + +Element.prototype.matches = Element.prototype.matches || + Element.prototype.matchesSelector || + Element.prototype.mozMatchesSelector || + Element.prototype.msMatchesSelector || + Element.prototype.oMatchesSelector || + Element.prototype.webkitMatchesSelector || + function (s) { + const matches = (this.document || this.ownerDocument).querySelectorAll(s); + let i = matches.length; + while (--i >= 0 && matches.item(i) !== this) {} + return i > -1; + }; diff --git a/app/assets/javascripts/lib/utils/bootstrap_linked_tabs.js.es6 b/app/assets/javascripts/lib/utils/bootstrap_linked_tabs.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..e810ee85bd318e41c2a8ad881239d24dd9b804b4 --- /dev/null +++ b/app/assets/javascripts/lib/utils/bootstrap_linked_tabs.js.es6 @@ -0,0 +1,113 @@ +/** + * Linked Tabs + * + * Handles persisting and restores the current tab selection and content. + * Reusable component for static content. + * + * ### Example Markup + * + * <ul class="nav-links tab-links"> + * <li class="active"> + * <a data-action="tab1" data-target="#tab1" data-toggle="tab" href="/path/tab1"> + * Tab 1 + * </a> + * </li> + * <li class="groups-tab"> + * <a data-action="tab2" data-target="#tab2" data-toggle="tab" href="/path/tab2"> + * Tab 2 + * </a> + * </li> + * + * + * <div class="tab-content"> + * <div class="tab-pane" id="tab1"> + * Tab 1 Content + * </div> + * <div class="tab-pane" id="tab2"> + * Tab 2 Content + * </div> + * </div> + * + * + * ### How to use + * + * new window.gl.LinkedTabs({ + * action: "#{controller.action_name}", + * defaultAction: 'tab1', + * parentEl: '.tab-links' + * }); + */ + +(() => { + window.gl = window.gl || {}; + + window.gl.LinkedTabs = class LinkedTabs { + /** + * Binds the events and activates de default tab. + * + * @param {Object} options + */ + constructor(options) { + this.options = options || {}; + + this.defaultAction = this.options.defaultAction; + this.action = this.options.action || this.defaultAction; + + if (this.action === 'show') { + this.action = this.defaultAction; + } + + this.currentLocation = window.location; + + const tabSelector = `${this.options.parentEl} a[data-toggle="tab"]`; + + // since this is a custom event we need jQuery :( + $(document) + .off('shown.bs.tab', tabSelector) + .on('shown.bs.tab', tabSelector, e => this.tabShown(e)); + + this.activateTab(this.action); + } + + /** + * Handles the `shown.bs.tab` event to set the currect url action. + * + * @param {type} evt + * @return {Function} + */ + tabShown(evt) { + const source = evt.target.getAttribute('href'); + + return this.setCurrentAction(source); + } + + /** + * Updates the URL with the path that matched the given action. + * + * @param {String} source + * @return {String} + */ + setCurrentAction(source) { + const copySource = source; + + copySource.replace(/\/+$/, ''); + + const newState = `${copySource}${this.currentLocation.search}${this.currentLocation.hash}`; + + history.replaceState({ + turbolinks: true, + url: newState, + }, document.title, newState); + return newState; + } + + /** + * Given the current action activates the correct tab. + * http://getbootstrap.com/javascript/#tab-show + * Note: Will trigger `shown.bs.tab` + */ + activateTab() { + return $(`${this.options.parentEl} a[data-action='${this.action}']`).tab('show'); + } + }; +})(); diff --git a/app/assets/javascripts/pipelines.js.es6 b/app/assets/javascripts/pipelines.js.es6 index a84db9c02334fd5e62f325598f7b4e1305c443dc..72c6c4a1fcd393847262ee7322a8cec5a9a109ab 100644 --- a/app/assets/javascripts/pipelines.js.es6 +++ b/app/assets/javascripts/pipelines.js.es6 @@ -1,8 +1,15 @@ +//= require lib/utils/bootstrap_linked_tabs + /* eslint-disable */ ((global) => { class Pipelines { - constructor() { + constructor(options) { + + if (options.initTabs && options.tabsOptions) { + new global.LinkedTabs(options.tabsOptions); + } + this.addMarginToBuildColumns(); } diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 533af80aee0ab929b9005975d972ad7c6a343db5..85188cfdd4ca7e597da0bdc4d2de33ca564aee72 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -1,6 +1,6 @@ class Projects::PipelinesController < Projects::ApplicationController before_action :pipeline, except: [:index, :new, :create] - before_action :commit, only: [:show] + before_action :commit, only: [:show, :builds] before_action :authorize_read_pipeline! before_action :authorize_create_pipeline!, only: [:new, :create] before_action :authorize_update_pipeline!, only: [:retry, :cancel] @@ -32,6 +32,14 @@ class Projects::PipelinesController < Projects::ApplicationController def show end + def builds + respond_to do |format| + format.html do + render 'show' + end + end + end + def retry pipeline.retry_failed(current_user) diff --git a/app/views/projects/pipelines/_with_tabs.html.haml b/app/views/projects/pipelines/_with_tabs.html.haml index 718314701f92a39d8e34e8a082ef44d5fea20c9e..3464e155a1bc2b8c5217cf32242a670e3af5f730 100644 --- a/app/views/projects/pipelines/_with_tabs.html.haml +++ b/app/views/projects/pipelines/_with_tabs.html.haml @@ -1,14 +1,17 @@ .tabs-holder - %ul.nav-links.no-top.no-bottom - %li.active - = link_to "Pipeline", "#js-tab-pipeline", data: { target: '#js-tab-pipeline', action: 'pipeline', toggle: 'tab' }, class: 'pipeline-tab' - %li - = link_to "#js-tab-builds", data: { target: '#js-tab-builds', action: 'build', toggle: 'tab' }, class: 'builds-tab' do + %ul.pipelines-tabs.nav-links.no-top.no-bottom + %li.js-pipeline-tab-link + = link_to namespace_project_pipeline_path(@project.namespace, @project, @pipeline), data: { target: 'div#js-tab-pipeline', action: 'pipelines', toggle: 'tab' }, class: 'pipeline-tab' do + Pipeline + %li.js-builds-tab-link + = link_to builds_namespace_project_pipeline_path(@project.namespace, @project, @pipeline), data: {target: 'div#js-tab-builds', action: 'builds', toggle: 'tab' }, class: 'builds-tab' do Builds - %span.badge= pipeline.statuses.count + %span.badge.js-builds-counter= pipeline.statuses.count + + .tab-content - #js-tab-pipeline.tab-pane.active + #js-tab-pipeline.tab-pane .build-content.middle-block.pipeline-graph .pipeline-visualization %ul.stage-column-list diff --git a/app/views/projects/pipelines/show.html.haml b/app/views/projects/pipelines/show.html.haml index 8c6652a5f90f642f635d7db5c3f1ae0d1b5596af..29a41bc664bc7f792f426f997e4111058fa120cf 100644 --- a/app/views/projects/pipelines/show.html.haml +++ b/app/views/projects/pipelines/show.html.haml @@ -2,7 +2,7 @@ - page_title "Pipeline" = render "projects/pipelines/head" -%div{ class: container_class } +%div.js-pipeline-container{ class: container_class, data: { controller_action: "#{controller.action_name}" } } - if @commit = render "projects/pipelines/info" diff --git a/changelogs/unreleased/24814-pipeline-tabs.yml b/changelogs/unreleased/24814-pipeline-tabs.yml new file mode 100644 index 0000000000000000000000000000000000000000..f85e7576905f98f3fab932f806ae9d4ed9f94e82 --- /dev/null +++ b/changelogs/unreleased/24814-pipeline-tabs.yml @@ -0,0 +1,4 @@ +--- +title: Fix Cicking on tabs on pipeline page should set URL +merge_request: 7709 +author: diff --git a/config/routes/project.rb b/config/routes/project.rb index 1336484a39993fc5e71d2aa5b703b9d249aea6ca..0754f0ec3b0a1cca0a6b98a6cc2d820f834fd763 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -129,6 +129,7 @@ constraints(ProjectUrlConstrainer.new) do member do post :cancel post :retry + get :builds end end diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3350a3aeefcc404013729c254b559c2bd7f1f82e --- /dev/null +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -0,0 +1,154 @@ +require 'spec_helper' + +describe "Pipelines", feature: true, js: true do + include GitlabRoutingHelper + + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + + before do + login_as(user) + project.team << [user, :developer] + end + + describe 'GET /:project/pipelines/:id' do + let(:project) { create(:project) } + let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) } + + before do + @success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build') + @failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') + @running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy') + @manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual build') + @external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external') + end + + before { visit namespace_project_pipeline_path(project.namespace, project, pipeline) } + + it 'shows the pipeline graph' do + expect(page).to have_selector('.pipeline-visualization') + expect(page).to have_content('Build') + expect(page).to have_content('Test') + expect(page).to have_content('Deploy') + expect(page).to have_content('Retry failed') + expect(page).to have_content('Cancel running') + end + + it 'shows Pipeline tab pane as active' do + expect(page).to have_css('#js-tab-pipeline.active') + end + + context 'page tabs' do + it 'shows Pipeline and Builds tabs with link' do + expect(page).to have_link('Pipeline') + expect(page).to have_link('Builds') + end + + it 'shows counter in Builds tab' do + expect(page.find('.js-builds-counter').text).to eq(pipeline.statuses.count.to_s) + end + + it 'shows Pipeline tab as active' do + expect(page).to have_css('.js-pipeline-tab-link.active') + end + end + + context 'retrying builds' do + it { expect(page).not_to have_content('retried') } + + context 'when retrying' do + before { click_on 'Retry failed' } + + it { expect(page).not_to have_content('Retry failed') } + end + end + + context 'canceling builds' do + it { expect(page).not_to have_selector('.ci-canceled') } + + context 'when canceling' do + before { click_on 'Cancel running' } + + it { expect(page).not_to have_content('Cancel running') } + end + end + end + + describe 'GET /:project/pipelines/:id/builds' do + let(:project) { create(:project) } + let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) } + + before do + @success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build') + @failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') + @running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy') + @manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual build') + @external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external') + end + + before { visit builds_namespace_project_pipeline_path(project.namespace, project, pipeline)} + + it 'shows a list of builds' do + expect(page).to have_content('Test') + expect(page).to have_content(@success.id) + expect(page).to have_content('Deploy') + expect(page).to have_content(@failed.id) + expect(page).to have_content(@running.id) + expect(page).to have_content(@external.id) + expect(page).to have_content('Retry failed') + expect(page).to have_content('Cancel running') + expect(page).to have_link('Play') + end + + it 'shows Builds tab pane as active' do + expect(page).to have_css('#js-tab-builds.active') + end + + context 'page tabs' do + it 'shows Pipeline and Builds tabs with link' do + expect(page).to have_link('Pipeline') + expect(page).to have_link('Builds') + end + + it 'shows counter in Builds tab' do + expect(page.find('.js-builds-counter').text).to eq(pipeline.statuses.count.to_s) + end + + it 'shows Builds tab as active' do + expect(page).to have_css('li.js-builds-tab-link.active') + end + end + + context 'retrying builds' do + it { expect(page).not_to have_content('retried') } + + context 'when retrying' do + before { click_on 'Retry failed' } + + it { expect(page).not_to have_content('Retry failed') } + it { expect(page).to have_selector('.retried') } + end + end + + context 'canceling builds' do + it { expect(page).not_to have_selector('.ci-canceled') } + + context 'when canceling' do + before { click_on 'Cancel running' } + + it { expect(page).not_to have_content('Cancel running') } + it { expect(page).to have_selector('.ci-canceled') } + end + end + + context 'playing manual build' do + before do + within '.pipeline-holder' do + click_link('Play') + end + end + + it { expect(@manual.reload).to be_pending } + end + end +end diff --git a/spec/features/projects/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb similarity index 74% rename from spec/features/projects/pipelines_spec.rb rename to spec/features/projects/pipelines/pipelines_spec.rb index 10e5466fc85c7bf1c1555e97e59762ae4ad9d86a..f3731698a1832936ec7570c8d57e4e64820ae49b 100644 --- a/spec/features/projects/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -152,65 +152,6 @@ describe "Pipelines" do end end - describe 'GET /:project/pipelines/:id' do - let(:project) { create(:project) } - let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', sha: project.commit.id) } - - before do - @success = create(:ci_build, :success, pipeline: pipeline, stage: 'build', name: 'build') - @failed = create(:ci_build, :failed, pipeline: pipeline, stage: 'test', name: 'test', commands: 'test') - @running = create(:ci_build, :running, pipeline: pipeline, stage: 'deploy', name: 'deploy') - @manual = create(:ci_build, :manual, pipeline: pipeline, stage: 'deploy', name: 'manual build') - @external = create(:generic_commit_status, status: 'success', pipeline: pipeline, name: 'jenkins', stage: 'external') - end - - before { visit namespace_project_pipeline_path(project.namespace, project, pipeline) } - - it 'shows a list of builds' do - expect(page).to have_content('Test') - expect(page).to have_content(@success.id) - expect(page).to have_content('Deploy') - expect(page).to have_content(@failed.id) - expect(page).to have_content(@running.id) - expect(page).to have_content(@external.id) - expect(page).to have_content('Retry failed') - expect(page).to have_content('Cancel running') - expect(page).to have_link('Play') - end - - context 'retrying builds' do - it { expect(page).not_to have_content('retried') } - - context 'when retrying' do - before { click_on 'Retry failed' } - - it { expect(page).not_to have_content('Retry failed') } - it { expect(page).to have_selector('.retried') } - end - end - - context 'canceling builds' do - it { expect(page).not_to have_selector('.ci-canceled') } - - context 'when canceling' do - before { click_on 'Cancel running' } - - it { expect(page).not_to have_content('Cancel running') } - it { expect(page).to have_selector('.ci-canceled') } - end - end - - context 'playing manual build' do - before do - within '.pipeline-holder' do - click_link('Play') - end - end - - it { expect(@manual.reload).to be_pending } - end - end - describe 'POST /:project/pipelines' do let(:project) { create(:project) } diff --git a/spec/javascripts/bootstrap_linked_tabs_spec.js.es6 b/spec/javascripts/bootstrap_linked_tabs_spec.js.es6 new file mode 100644 index 0000000000000000000000000000000000000000..9aa3c50611de88d0ec983495d2c5eb15350b7927 --- /dev/null +++ b/spec/javascripts/bootstrap_linked_tabs_spec.js.es6 @@ -0,0 +1,55 @@ +//= require lib/utils/bootstrap_linked_tabs + +(() => { + describe('Linked Tabs', () => { + fixture.preload('linked_tabs'); + + beforeEach(() => { + fixture.load('linked_tabs'); + }); + + describe('when is initialized', () => { + it('should activate the tab correspondent to the given action', () => { + const linkedTabs = new window.gl.LinkedTabs({ // eslint-disable-line + action: 'tab1', + defaultAction: 'tab1', + parentEl: '.linked-tabs', + }); + + expect(document.querySelector('#tab1').classList).toContain('active'); + }); + + it('should active the default tab action when the action is show', () => { + const linkedTabs = new window.gl.LinkedTabs({ // eslint-disable-line + action: 'show', + defaultAction: 'tab1', + parentEl: '.linked-tabs', + }); + + expect(document.querySelector('#tab1').classList).toContain('active'); + }); + }); + + describe('on click', () => { + it('should change the url according to the clicked tab', () => { + const historySpy = spyOn(history, 'replaceState').and.callFake(() => {}); + + const linkedTabs = new window.gl.LinkedTabs({ // eslint-disable-line + action: 'show', + defaultAction: 'tab1', + parentEl: '.linked-tabs', + }); + + const secondTab = document.querySelector('.linked-tabs li:nth-child(2) a'); + const newState = secondTab.getAttribute('href') + linkedTabs.currentLocation.search + linkedTabs.currentLocation.hash; + + secondTab.click(); + + expect(historySpy).toHaveBeenCalledWith({ + turbolinks: true, + url: newState, + }, document.title, newState); + }); + }); + }); +})(); diff --git a/spec/javascripts/fixtures/linked_tabs.html.haml b/spec/javascripts/fixtures/linked_tabs.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..93c0cf97ff0979b055b9bdee4e8d716984fda08b --- /dev/null +++ b/spec/javascripts/fixtures/linked_tabs.html.haml @@ -0,0 +1,13 @@ +%ul.nav.nav-tabs.linked-tabs + %li + %a{ href: 'foo/bar/1', data: { target: 'div#tab1', action: 'tab1', toggle: 'tab' } } + Tab 1 + %li + %a{ href: 'foo/bar/1/context', data: { target: 'div#tab2', action: 'tab2', toggle: 'tab' } } + Tab 2 + +.tab-content + #tab1.tab-pane + Tab 1 Content + #tab2.tab-pane + Tab 2 Content