diff options
author | Fatih Acet <acetfatih@gmail.com> | 2016-12-01 21:20:59 +0000 |
---|---|---|
committer | Fatih Acet <acetfatih@gmail.com> | 2016-12-01 21:20:59 +0000 |
commit | 629624f30f6ad2f31681772adf88ace6f28727c9 (patch) | |
tree | ed902b1dbc583ad63ce38d457054ea0e576a2970 | |
parent | 1646b81e2a4aa94c82a669b135f875713450eacc (diff) | |
parent | aa2d6eec9e36acdff679d3a5ef17db0780f51447 (diff) | |
download | gitlab-ce-629624f30f6ad2f31681772adf88ace6f28727c9.tar.gz |
Merge branch '24814-pipeline-tabs' into 'master'
Pipelines tabs
## What does this MR do?
Changes the URL when the builds tab is clicked making it possible to be shared.
1. Adds a standard way to handle linked tabs:
* This behaviour is already present in the merge requests, commit and user `show` page.
* This MR introduces a reusable way to accomplish this behaviour for pages with static content.
2. Adds test:
* For the linked tabs reusable class
* For the pipelines tabs
## Why was this MR needed?
To allow having a sharable URL that represented the opened tab
![tabs](/uploads/91e663c12c6e9ac46a17aa3a9489dc72/tabs.gif)
## Does this MR meet the acceptance criteria?
- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #24814
See merge request !7709
-rw-r--r-- | app/assets/javascripts/dispatcher.js.es6 | 12 | ||||
-rw-r--r-- | app/assets/javascripts/extensions/element.js.es6 | 17 | ||||
-rw-r--r-- | app/assets/javascripts/lib/utils/bootstrap_linked_tabs.js.es6 | 113 | ||||
-rw-r--r-- | app/assets/javascripts/pipelines.js.es6 | 9 | ||||
-rw-r--r-- | app/controllers/projects/pipelines_controller.rb | 10 | ||||
-rw-r--r-- | app/views/projects/pipelines/_with_tabs.html.haml | 17 | ||||
-rw-r--r-- | app/views/projects/pipelines/show.html.haml | 2 | ||||
-rw-r--r-- | changelogs/unreleased/24814-pipeline-tabs.yml | 4 | ||||
-rw-r--r-- | config/routes/project.rb | 1 | ||||
-rw-r--r-- | spec/features/projects/pipelines/pipeline_spec.rb | 154 | ||||
-rw-r--r-- | spec/features/projects/pipelines/pipelines_spec.rb (renamed from spec/features/projects/pipelines_spec.rb) | 59 | ||||
-rw-r--r-- | spec/javascripts/bootstrap_linked_tabs_spec.js.es6 | 55 | ||||
-rw-r--r-- | spec/javascripts/fixtures/linked_tabs.html.haml | 13 |
13 files changed, 393 insertions, 73 deletions
diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 147634f1cc4..ab521c6c1fc 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 6d9b0c4bc3e..3f12ad9ff9f 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 00000000000..e810ee85bd3 --- /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 a84db9c0233..72c6c4a1fcd 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 533af80aee0..85188cfdd4c 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 718314701f9..3464e155a1b 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 8c6652a5f90..29a41bc664b 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 00000000000..f85e7576905 --- /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 1336484a399..0754f0ec3b0 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 00000000000..3350a3aeefc --- /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 index 10e5466fc85..f3731698a18 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 00000000000..9aa3c50611d --- /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 00000000000..93c0cf97ff0 --- /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 |