diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2017-01-21 10:41:13 +0000 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2017-01-21 10:41:13 +0000 |
commit | 382761472b15fd01c9312f885ebe14e62c8426aa (patch) | |
tree | 9a05950e1a0b9a5dfee5e9e8810444b43ddfcb71 | |
parent | 21c225cdaab77d531e4eff10c778406de3f549f3 (diff) | |
parent | 5f84368cba3e5c517b20fc6956a742bcd10569f8 (diff) | |
download | gitlab-ce-382761472b15fd01c9312f885ebe14e62c8426aa.tar.gz |
Merge branch 'fix/revert-removing-build-tab-again' into '8-16-stable'
Revert removing builds tab on '8-16-stable'
See merge request !8661
21 files changed, 141 insertions, 25 deletions
diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 99a34651639..e0255d64a65 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -152,6 +152,7 @@ new MergedButtons(); break; case 'projects:merge_requests:commits': + case 'projects:merge_requests:builds': new MergedButtons(); break; case "projects:merge_requests:diffs": @@ -176,6 +177,9 @@ container: '.js-pipeline-table', }); break; + case 'projects:commit:builds': + new gl.Pipelines(); + break; case 'projects:commits:show': case 'projects:activity': shortcut_handler = new ShortcutsNavigation(); diff --git a/app/assets/javascripts/merge_request_tabs.js.es6 b/app/assets/javascripts/merge_request_tabs.js.es6 index 4c8c28af755..8a925591600 100644 --- a/app/assets/javascripts/merge_request_tabs.js.es6 +++ b/app/assets/javascripts/merge_request_tabs.js.es6 @@ -59,13 +59,16 @@ class MergeRequestTabs { - constructor({ action, setUrl, stubLocation } = {}) { + constructor({ action, setUrl, buildsLoaded, stubLocation } = {}) { this.diffsLoaded = false; + this.buildsLoaded = false; this.pipelinesLoaded = false; this.commitsLoaded = false; this.fixedLayoutPref = null; this.setUrl = setUrl !== undefined ? setUrl : true; + this.buildsLoaded = buildsLoaded || false; + this.setCurrentAction = this.setCurrentAction.bind(this); this.tabShown = this.tabShown.bind(this); this.showTab = this.showTab.bind(this); @@ -116,6 +119,10 @@ $.scrollTo('.merge-request-details .merge-request-tabs', { offset: -navBarHeight, }); + } else if (action === 'builds') { + this.loadBuilds($target.attr('href')); + this.expandView(); + this.resetViewContainer(); } else if (action === 'pipelines') { this.loadPipelines($target.attr('href')); this.expandView(); @@ -173,8 +180,8 @@ setCurrentAction(action) { this.currentAction = action === 'show' ? 'notes' : action; - // Remove a trailing '/commits' '/diffs' '/pipelines' '/new' '/new/diffs' - let newState = location.pathname.replace(/\/(commits|diffs|pipelines|new|new\/diffs)(\.html)?\/?$/, ''); + // Remove a trailing '/commits' '/diffs' '/builds' '/pipelines' '/new' '/new/diffs' + let newState = location.pathname.replace(/\/(commits|diffs|builds|pipelines|new|new\/diffs)(\.html)?\/?$/, ''); // Append the new action if we're on a tab other than 'notes' if (this.currentAction !== 'notes') { @@ -243,6 +250,22 @@ }); } + loadBuilds(source) { + if (this.buildsLoaded) { + return; + } + this.ajaxGet({ + url: `${source}.json`, + success: (data) => { + document.querySelector('div#builds').innerHTML = data.html; + gl.utils.localTimeAgo($('.js-timeago', 'div#builds')); + this.buildsLoaded = true; + new gl.Pipelines(); + this.scrollToElement('#builds'); + }, + }); + } + loadPipelines(source) { if (this.pipelinesLoaded) { return; diff --git a/app/assets/javascripts/merge_request_widget.js.es6 b/app/assets/javascripts/merge_request_widget.js.es6 index 0305aeb07d9..e47047c4cca 100644 --- a/app/assets/javascripts/merge_request_widget.js.es6 +++ b/app/assets/javascripts/merge_request_widget.js.es6 @@ -74,7 +74,7 @@ MergeRequestWidget.prototype.addEventListeners = function() { var allowedPages; - allowedPages = ['show', 'commits', 'pipelines', 'changes']; + allowedPages = ['show', 'commits', 'builds', 'pipelines', 'changes']; $(document).on('page:change.merge_request', (function(_this) { return function() { var page; @@ -173,6 +173,7 @@ message = message.replace('{{title}}', data.title); notify(title, message, _this.opts.gitlab_icon, function() { this.close(); + return Turbolinks.visit(_this.opts.builds_path); }); } } diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 791ed88db30..8197d9e4c99 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -8,11 +8,13 @@ class Projects::CommitController < Projects::ApplicationController # Authorize before_action :require_non_empty_project - before_action :authorize_download_code! + before_action :authorize_download_code!, except: [:cancel_builds, :retry_builds] + before_action :authorize_update_build!, only: [:cancel_builds, :retry_builds] before_action :authorize_read_pipeline!, only: [:pipelines] + before_action :authorize_read_commit_status!, only: [:builds] before_action :commit - before_action :define_commit_vars, only: [:show, :diff_for_path, :pipelines] - before_action :define_status_vars, only: [:show, :pipelines] + before_action :define_commit_vars, only: [:show, :diff_for_path, :builds, :pipelines] + before_action :define_status_vars, only: [:show, :builds, :pipelines] before_action :define_note_vars, only: [:show, :diff_for_path] before_action :authorize_edit_tree!, only: [:revert, :cherry_pick] @@ -33,6 +35,25 @@ class Projects::CommitController < Projects::ApplicationController def pipelines end + def builds + end + + def cancel_builds + ci_builds.running_or_pending.each(&:cancel) + + redirect_back_or_default default: builds_namespace_project_commit_path(project.namespace, project, commit.sha) + end + + def retry_builds + ci_builds.latest.failed.each do |build| + if build.retryable? + Ci::Build.retry(build, current_user) + end + end + + redirect_back_or_default default: builds_namespace_project_commit_path(project.namespace, project, commit.sha) + end + def branches @branches = @project.repository.branch_names_contains(commit.id) @tags = @project.repository.tag_names_contains(commit.id) @@ -77,6 +98,10 @@ class Projects::CommitController < Projects::ApplicationController @noteable = @commit ||= @project.commit(params[:id]) end + def ci_builds + @ci_builds ||= Ci::Build.where(pipeline: pipelines) + end + def define_commit_vars return git_not_found! unless commit @@ -108,6 +133,8 @@ class Projects::CommitController < Projects::ApplicationController def define_status_vars @ci_pipelines = project.pipelines.where(sha: commit.sha) + @statuses = CommitStatus.where(pipeline: @ci_pipelines).relevant + @builds = Ci::Build.where(pipeline: @ci_pipelines).relevant end def assign_change_commit_vars(mr_source_branch) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 9ac5bf4b9f8..2ea33a1b90a 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -9,10 +9,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController before_action :module_enabled before_action :merge_request, only: [ - :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :pipelines, :merge, :merge_check, + :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines, :merge, :merge_check, :ci_status, :ci_environments_status, :toggle_subscription, :cancel_merge_when_build_succeeds, :remove_wip, :resolve_conflicts, :assign_related_issues ] - before_action :validates_merge_request, only: [:show, :diffs, :commits, :pipelines] + before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds, :pipelines] before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines] before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds, :merge_check] before_action :define_commit_vars, only: [:diffs] @@ -205,6 +205,17 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + def builds + respond_to do |format| + format.html do + define_discussion_vars + + render 'show' + end + format.json { render json: { html: view_to_html_string('projects/merge_requests/show/_builds') } } + end + end + def pipelines @pipelines = @merge_request.all_pipelines diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 94f3b480178..d9f5e01f0dc 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -1,7 +1,7 @@ module CiStatusHelper def ci_status_path(pipeline) project = pipeline.project - namespace_project_pipeline_path(project.namespace, project, pipeline) + builds_namespace_project_commit_path(project.namespace, project, pipeline.sha) end # Is used by Commit and Merge Request Widget diff --git a/app/views/projects/_last_commit.html.haml b/app/views/projects/_last_commit.html.haml index e1fea8ccf3d..7f530708947 100644 --- a/app/views/projects/_last_commit.html.haml +++ b/app/views/projects/_last_commit.html.haml @@ -1,8 +1,7 @@ - - ref = local_assigns.fetch(:ref) - status = commit.status(ref) - if status - = link_to pipelines_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{status}" do + = link_to builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{status}" do = ci_icon_for_status(status) = ci_label_for_status(status) diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index b15be0d861d..057a720a54a 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -7,7 +7,7 @@ = link_to pipeline_path(@build.pipeline) do %strong ##{@build.pipeline.id} for commit - = link_to namespace_project_commit_path(@project.namespace, @project, @build.pipeline.sha) do + = link_to ci_status_path(@build.pipeline) do %strong= @build.pipeline.short_sha from = link_to namespace_project_commits_path(@project.namespace, @project, @build.ref) do diff --git a/app/views/projects/commit/_builds.html.haml b/app/views/projects/commit/_builds.html.haml new file mode 100644 index 00000000000..b7087749428 --- /dev/null +++ b/app/views/projects/commit/_builds.html.haml @@ -0,0 +1,2 @@ +- @ci_pipelines.each do |pipeline| + = render "pipeline", pipeline: pipeline, pipeline_details: true diff --git a/app/views/projects/commit/_ci_menu.html.haml b/app/views/projects/commit/_ci_menu.html.haml index 13ab2253733..cbfd99ca448 100644 --- a/app/views/projects/commit/_ci_menu.html.haml +++ b/app/views/projects/commit/_ci_menu.html.haml @@ -8,3 +8,7 @@ = link_to pipelines_namespace_project_commit_path(@project.namespace, @project, @commit.id) do Pipelines %span.badge= @ci_pipelines.count + = nav_link(path: 'commit#builds') do + = link_to builds_namespace_project_commit_path(@project.namespace, @project, @commit.id) do + Builds + %span.badge= @statuses.count diff --git a/app/views/projects/commit/builds.html.haml b/app/views/projects/commit/builds.html.haml new file mode 100644 index 00000000000..077b2d2725b --- /dev/null +++ b/app/views/projects/commit/builds.html.haml @@ -0,0 +1,9 @@ +- @no_container = true +- page_title "Builds", "#{@commit.title} (#{@commit.short_id})", "Commits" += render "projects/commits/head" + +%div{ class: container_class } + = render "commit_box" + + = render "ci_menu" + = render "builds" diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index 34ead6427e0..4c5404f5c57 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -34,6 +34,11 @@ = link_to url_for(params), data: {target: 'div#pipelines', action: 'pipelines', toggle: 'tab'} do Pipelines %span.badge= @pipelines.size + - if @pipeline.present? + %li.builds-tab + = link_to url_for(params), data: {target: 'div#builds', action: 'builds', toggle: 'tab'} do + Builds + %span.badge= @statuses_count %li.diffs-tab = link_to url_for(params.merge(action: 'new_diffs')), data: {target: 'div#diffs', action: 'new/diffs', toggle: 'tab'} do Changes @@ -44,6 +49,9 @@ = render "projects/merge_requests/show/commits" #diffs.diffs.tab-pane - # This tab is always loaded via AJAX + - if @pipeline.present? + #builds.builds.tab-pane + = render "projects/merge_requests/show/builds" - if @pipelines.any? #pipelines.pipelines.tab-pane = render "projects/merge_requests/show/pipelines" @@ -58,5 +66,6 @@ }); :javascript var merge_request = new MergeRequest({ - action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}" + action: "#{(@show_changes_tab ? 'new/diffs' : 'new')}", + buildsLoaded: "#{@pipeline.present? ? 'true' : 'false'}" }); diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 2a7cd3a19d0..4555b869999 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -68,6 +68,11 @@ = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do Pipelines %span.badge= @pipelines.size + - if @pipeline.present? + %li.builds-tab + = link_to builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#builds', action: 'builds', toggle: 'tab' } do + Builds + %span.badge= @statuses_count %li.diffs-tab = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do Changes @@ -95,6 +100,8 @@ - # This tab is always loaded via AJAX #pipelines.pipelines.tab-pane - # This tab is always loaded via AJAX + #builds.builds.tab-pane + - # This tab is always loaded via AJAX #diffs.diffs.tab-pane - # This tab is always loaded via AJAX diff --git a/app/views/projects/merge_requests/show/_builds.html.haml b/app/views/projects/merge_requests/show/_builds.html.haml new file mode 100644 index 00000000000..808ef7fed27 --- /dev/null +++ b/app/views/projects/merge_requests/show/_builds.html.haml @@ -0,0 +1 @@ += render "projects/commit/pipeline", pipeline: @pipeline, link_to_commit: true diff --git a/app/views/projects/merge_requests/widget/_show.html.haml b/app/views/projects/merge_requests/widget/_show.html.haml index 38328501ffd..a8918c85dde 100644 --- a/app/views/projects/merge_requests/widget/_show.html.haml +++ b/app/views/projects/merge_requests/widget/_show.html.haml @@ -24,10 +24,12 @@ preparing: "{{status}} build", normal: "Build {{status}}" }, + builds_path: "#{builds_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", pipelines_path: "#{pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}" }; if (typeof merge_request_widget !== 'undefined') { + clearInterval(merge_request_widget.fetchBuildStatusInterval); merge_request_widget.cancelPolling(); merge_request_widget.clearEventListeners(); } diff --git a/config/routes/project.rb b/config/routes/project.rb index 6620b765e02..82645ac6d77 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -32,7 +32,10 @@ constraints(ProjectUrlConstrainer.new) do resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do member do get :branches + get :builds get :pipelines + post :cancel_builds + post :retry_builds post :revert post :cherry_pick get :diff_for_path @@ -91,6 +94,7 @@ constraints(ProjectUrlConstrainer.new) do get :diffs get :conflicts get :conflict_for_path + get :builds get :pipelines get :merge_check post :merge diff --git a/features/project/commits/commits.feature b/features/project/commits/commits.feature index 3459cce03f9..1776c07e60e 100644 --- a/features/project/commits/commits.feature +++ b/features/project/commits/commits.feature @@ -47,6 +47,8 @@ Feature: Project Commits And repository contains ".gitlab-ci.yml" file When I click on commit link Then I see commit ci info + And I click status link + Then I see builds list Scenario: I browse commit with side-by-side diff view Given I click on commit link diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index 18e267294e4..007dfb67a77 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -166,6 +166,15 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps expect(page).to have_content "Pipeline #1 for 570e7b2a pending" end + step 'I click status link' do + find('.commit-ci-menu').click_link "Builds" + end + + step 'I see builds list' do + expect(page).to have_content "Pipeline #1 for 570e7b2a pending" + expect(page).to have_content "1 build" + end + step 'I search "submodules" commits' do fill_in 'commits-search', with: 'submodules' end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 7ea3ea4f376..c4d599def03 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -667,6 +667,10 @@ describe Projects::MergeRequestsController do end end + describe 'GET builds' do + it_behaves_like "loads labels", :builds + end + describe 'GET pipelines' do it_behaves_like "loads labels", :pipelines end diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb index 73c5ef31edc..142649297cc 100644 --- a/spec/features/merge_requests/created_from_fork_spec.rb +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -54,14 +54,14 @@ feature 'Merge request created from fork' do scenario 'user visits a pipelines page', js: true do visit_merge_request(merge_request) - page.within('.merge-request-tabs') { click_link 'Pipelines' } + page.within('.merge-request-tabs') { click_link 'Builds' } page.within('table.ci-table') do - expect(page).to have_content pipeline.status - expect(page).to have_content pipeline.id + expect(page).to have_content 'rspec' + expect(page).to have_content 'spinach' end - expect(page.find('a.btn-remove')[:href]) + expect(find_link('Cancel running')[:href]) .to include fork_project.path_with_namespace end end diff --git a/spec/features/projects/commit/builds_spec.rb b/spec/features/projects/commit/builds_spec.rb index 33f1c323af1..fcdf7870f34 100644 --- a/spec/features/projects/commit/builds_spec.rb +++ b/spec/features/projects/commit/builds_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'project commit pipelines' do +feature 'project commit builds' do given(:project) { create(:project) } background do @@ -16,13 +16,11 @@ feature 'project commit pipelines' do ref: 'master') end - scenario 'user views commit pipelines page' do - visit pipelines_namespace_project_commit_path(project.namespace, project, project.commit.sha) + scenario 'user views commit builds page' do + visit builds_namespace_project_commit_path(project.namespace, + project, project.commit.sha) - page.within('.table-holder') do - expect(page).to have_content project.pipelines[0].status # pipeline status - expect(page).to have_content project.pipelines[0].id # pipeline ids - end + expect(page).to have_content('Builds') end end end |