diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2016-12-26 23:15:41 +0000 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2016-12-26 23:15:41 +0000 |
commit | 40c9bec847cd95dbbecc42a1148fea0af4edc567 (patch) | |
tree | f4b0fb15ca3db84971f9573aea6ce86e8986bad6 | |
parent | 1119c1f6569755751dcb6789b944df73a1c7be12 (diff) | |
download | gitlab-ce-revert-79a816f3.tar.gz |
Revert "Merge branch '23638-remove-builds-tab' into 'master'"revert-79a816f3
This reverts merge request !7763
21 files changed, 141 insertions, 25 deletions
diff --git a/app/assets/javascripts/dispatcher.js.es6 b/app/assets/javascripts/dispatcher.js.es6 index 78f68a247a2..9b669b08b00 100644 --- a/app/assets/javascripts/dispatcher.js.es6 +++ b/app/assets/javascripts/dispatcher.js.es6 @@ -138,6 +138,7 @@ new MergedButtons(); break; case 'projects:merge_requests:commits': + case 'projects:merge_requests:builds': new MergedButtons(); break; case "projects:merge_requests:diffs": @@ -162,6 +163,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 860e7e066a0..f111e7280d0 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') { @@ -248,6 +255,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 3abebdfd032..f0cb5a9d4b4 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] @@ -201,6 +201,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 349181be784..4a08ed045f4 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 d1fa51ae7ee..7725558518f 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -65,6 +65,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 @@ -93,6 +98,8 @@ #commits.commits.tab-pane - # This tab is always loaded via AJAX + #builds.builds.tab-pane + - # This tab is always loaded via AJAX #pipelines.pipelines.tab-pane - # This tab is always loaded via AJAX #diffs.diffs.tab-pane 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 4d20acbef7a..baabd22b840 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 440b897ddc6..9e0b80205d8 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -649,6 +649,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 |