diff options
author | Eric Eastwood <contact@ericeastwood.com> | 2017-02-14 16:08:30 -0600 |
---|---|---|
committer | Eric Eastwood <contact@ericeastwood.com> | 2017-03-02 02:49:03 -0600 |
commit | 605fff91236bb225bb1bddad116ccae3502930e5 (patch) | |
tree | fb209e6f9eb98b6bbafac295bd81581422394b73 /app | |
parent | b6a945b39354ec2b2c09fc5f6904dfbf8990df26 (diff) | |
download | gitlab-ce-28010-mr-merge-button-default-to-danger.tar.gz |
Default to subtle MR mege button until CI status is available28010-mr-merge-button-default-to-danger
See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9245
Diffstat (limited to 'app')
6 files changed, 52 insertions, 32 deletions
diff --git a/app/assets/javascripts/merge_request_widget.js.es6 b/app/assets/javascripts/merge_request_widget.js.es6 index 00c6c050612..5f1bd474a0c 100644 --- a/app/assets/javascripts/merge_request_widget.js.es6 +++ b/app/assets/javascripts/merge_request_widget.js.es6 @@ -129,8 +129,9 @@ require('./smart_interval'); }; MergeRequestWidget.prototype.getMergeStatus = function() { - return $.get(this.opts.merge_check_url, function(data) { + return $.get(this.opts.merge_check_url, (data) => { var $html = $(data); + this.updateMergeButton(this.status, this.hasCi, $html); $('.mr-widget-body').replaceWith($html.find('.mr-widget-body')); $('.mr-widget-footer').replaceWith($html.find('.mr-widget-footer')); }); @@ -154,9 +155,9 @@ require('./smart_interval'); return $.getJSON(this.opts.ci_status_url, (function(_this) { return function(data) { var message, status, title; - if (!data.status) { - return; - } + _this.status = data.status; + _this.hasCi = data.has_ci; + _this.updateMergeButton(_this.status, _this.hasCi); if (data.environments && data.environments.length) _this.renderEnvironments(data.environments); if (data.status !== _this.opts.ci_status || data.sha !== _this.opts.ci_sha || @@ -232,36 +233,45 @@ require('./smart_interval'); return; } $('.ci_widget').hide(); - allowed_states = ["failed", "canceled", "running", "pending", "success", "success_with_warnings", "skipped", "not_found"]; - if (indexOf.call(allowed_states, state) !== -1) { - $('.ci_widget.ci-' + state).show(); + $('.ci_widget.ci-' + state).show(); + + this.initMiniPipelineGraph(); + }; + + MergeRequestWidget.prototype.showCICoverage = function(coverage) { + var text = `Coverage ${coverage}%`; + return $('.ci_widget:visible .ci-coverage').text(text); + }; + + MergeRequestWidget.prototype.updateMergeButton = function(state, hasCi, $html) { + const allowed_states = ["failed", "canceled", "running", "pending", "success", "success_with_warnings", "skipped", "not_found"]; + let stateClass = 'btn-danger'; + if (!hasCi) { + stateClass = 'btn-create'; + } else if (indexOf.call(allowed_states, state) !== -1) { switch (state) { case "failed": case "canceled": case "not_found": - this.setMergeButtonClass('btn-danger'); + stateClass = 'btn-danger'; break; case "running": - this.setMergeButtonClass('btn-info'); + stateClass = 'btn-info'; break; case "success": case "success_with_warnings": - this.setMergeButtonClass('btn-create'); + stateClass = 'btn-create'; } } else { $('.ci_widget.ci-error').show(); - this.setMergeButtonClass('btn-danger'); + stateClass = 'btn-danger'; } - }; - MergeRequestWidget.prototype.showCICoverage = function(coverage) { - var text; - text = 'Coverage ' + coverage + '%'; - return $('.ci_widget:visible .ci-coverage').text(text); + this.setMergeButtonClass(stateClass, $html); }; - MergeRequestWidget.prototype.setMergeButtonClass = function(css_class) { - return $('.js-merge-button,.accept-action .dropdown-toggle').removeClass('btn-danger btn-info btn-create').addClass(css_class); + MergeRequestWidget.prototype.setMergeButtonClass = function(css_class, $html = $('.mr-state-widget')) { + return $html.find('.js-merge-button').removeClass('btn-danger btn-info btn-create').addClass(css_class); }; MergeRequestWidget.prototype.updatePipelineUrls = function(id) { diff --git a/app/assets/javascripts/merge_request_widget/ci_bundle.js.es6 b/app/assets/javascripts/merge_request_widget/ci_bundle.js.es6 index 547dfa9e677..21d7c3e168e 100644 --- a/app/assets/javascripts/merge_request_widget/ci_bundle.js.es6 +++ b/app/assets/javascripts/merge_request_widget/ci_bundle.js.es6 @@ -15,14 +15,14 @@ }); $(document) - .off('click', '.accept_merge_request') - .on('click', '.accept_merge_request', () => { - $('.js-merge-button').html('<i class="fa fa-spinner fa-spin"></i> Merge in progress'); + .off('click', '.accept-merge-request') + .on('click', '.accept-merge-request', () => { + $('.js-merge-button, .js-merge-when-pipeline-succeeds-button').html('<i class="fa fa-spinner fa-spin"></i> Merge in progress'); }); $(document) - .off('click', '.merge_when_pipeline_succeeds') - .on('click', '.merge_when_pipeline_succeeds', () => { + .off('click', '.merge-when-pipeline-succeeds') + .on('click', '.merge-when-pipeline-succeeds', () => { $('#merge_when_pipeline_succeeds').val('1'); }); diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 0b0c4bc130d..a629a5333d7 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -29,7 +29,7 @@ background-color: $gl-success; } - .accept_merge_request { + .accept-merge-request { &.ci-pending, &.ci-running { @include btn-blue; @@ -42,6 +42,12 @@ @include btn-red; } } + + .dropdown-toggle { + .fa { + color: inherit; + } + } } .accept-control { diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 76519022381..82f9b6e06db 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -324,6 +324,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def merge_check @merge_request.check_if_can_be_merged + @pipelines = @merge_request.all_pipelines render partial: "projects/merge_requests/widget/show.html.haml", layout: false end @@ -446,6 +447,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def ci_status pipeline = @merge_request.head_pipeline + @pipelines = @merge_request.all_pipelines if pipeline status = pipeline.status @@ -464,7 +466,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController sha: (merge_request.diff_head_commit.short_id if merge_request.diff_head_sha), status: status, coverage: coverage, - pipeline: pipeline.try(:id) + pipeline: pipeline.try(:id), + has_ci: @merge_request.has_ci? } render json: response diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 81bde54d5dc..0f7b8311588 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -684,7 +684,10 @@ class MergeRequest < ActiveRecord::Base end def has_ci? - source_project.try(:ci_service) && commits.any? + has_ci_integration = source_project.try(:ci_service) + uses_gitlab_ci = all_pipelines.any? + + (has_ci_integration || uses_gitlab_ci) && commits.any? end def branch_missing? diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml index 1fa987bf537..c94c7944c0b 100644 --- a/app/views/projects/merge_requests/widget/open/_accept.html.haml +++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml @@ -1,8 +1,6 @@ - content_for :page_specific_javascripts do = page_specific_javascript_bundle_tag('merge_request_widget') -- status_class = @pipeline ? " ci-#{@pipeline.status}" : nil - = form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-quick-submit js-requires-input' } do |f| = hidden_field_tag :authenticity_token, form_authenticity_token = hidden_field_tag :sha, @merge_request.diff_head_sha @@ -11,10 +9,10 @@ .accept-action - if @pipeline && @pipeline.active? %span.btn-group - = button_tag class: "btn btn-create js-merge-button merge_when_pipeline_succeeds" do + = button_tag class: "btn btn-info js-merge-when-pipeline-succeeds-button merge-when-pipeline-succeeds" do Merge When Pipeline Succeeds - unless @project.only_allow_merge_if_pipeline_succeeds? - = button_tag class: "btn btn-success dropdown-toggle", 'data-toggle' => 'dropdown' do + = button_tag class: "btn btn-info dropdown-toggle", 'data-toggle' => 'dropdown' do = icon('caret-down') %span.sr-only Select Merge Moment @@ -24,11 +22,11 @@ = icon('check fw') Merge When Pipeline Succeeds %li - = link_to "#", class: "accept_merge_request" do + = link_to "#", class: "accept-merge-request" do = icon('warning fw') Merge Immediately - else - = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}" do + = f.button class: "btn btn-grouped js-merge-button accept-merge-request" do Accept Merge Request - if @merge_request.force_remove_source_branch? .accept-control |