diff --git a/app/assets/javascripts/merge_request_widget.js.es6 b/app/assets/javascripts/merge_request_widget.js.es6
index 94fc4d8e5fbe5f1b4394c2834e966e980f86e937..30d0ef104381ee39589d5b4dd0de5c4195c8b961 100644
--- a/app/assets/javascripts/merge_request_widget.js.es6
+++ b/app/assets/javascripts/merge_request_widget.js.es6
@@ -145,9 +145,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'));
$('.approvals-components').replaceWith($html.find('.approvals-components'));
@@ -176,9 +176,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 ||
@@ -254,36 +254,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 4db22622f30223469f19507088ce64780dffeba4..2be4125153a2fc1605adc7fb91bae706200d2a79 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 7bbc59e27a94627a823831d994050c45b06c8725..f802801aae4047f266ba9a895c944d092549aca1 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 e4c31b64db79681a6e571194d23242c1346e4252..41e91fa731e977cb41a0a8e3121070d862daba99 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -335,6 +335,7 @@ def remove_wip
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
@@ -472,6 +473,7 @@ def assign_related_issues
def ci_status
pipeline = @merge_request.head_pipeline
+ @pipelines = @merge_request.all_pipelines
if pipeline
status = pipeline.status
@@ -490,7 +492,8 @@ def ci_status
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 eeb07f54cd29466b5bbbd4b75ebdbee9757788b4..3279cc8b8c45b7e5e07c7140f97274e88ddd98f7 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -707,7 +707,10 @@ def locked_long_ago?
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 0714b5f6c9c508e8fcb056a85353a89b549a2a2a..150968e1d18b9942382aed7c131cd7ed8664e19b 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", disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
+ = button_tag class: "btn btn-info js-merge-when-pipeline-succeeds-button merge-when-pipeline-succeeds", disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
Merge When Pipeline Succeeds
- unless @project.only_allow_merge_if_pipeline_succeeds?
- = button_tag class: "btn btn-success dropdown-toggle", 'data-toggle' => 'dropdown', disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
+ = button_tag class: "btn btn-info dropdown-toggle", 'data-toggle' => 'dropdown', disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" 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}", disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
+ = f.button class: "btn btn-grouped js-merge-button accept-merge-request", disabled: !@merge_request.approved?, ":disabled" => "disableAcceptance" do
Accept Merge Request
- if @merge_request.force_remove_source_branch?
.accept-control
diff --git a/changelogs/unreleased/28010-mr-merge-button-default-to-danger.yml b/changelogs/unreleased/28010-mr-merge-button-default-to-danger.yml
new file mode 100644
index 0000000000000000000000000000000000000000..06bb669ceace8794c3e3b9af582290bd91f63ed1
--- /dev/null
+++ b/changelogs/unreleased/28010-mr-merge-button-default-to-danger.yml
@@ -0,0 +1,4 @@
+---
+title: Default to subtle MR mege button until CI status is available
+merge_request:
+author:
diff --git a/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb b/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb
index f2f8f11ab283272d8c0d8abc18aaa5013a4da168..0ceaf7bc830bb1faf907a5fa5e6ca2917dea040a 100644
--- a/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb
+++ b/spec/features/merge_requests/merge_immediately_with_pipeline_spec.rb
@@ -34,7 +34,7 @@
click_link 'Merge Immediately'
- expect(find('.js-merge-button')).to have_content('Merge in progress')
+ expect(find('.js-merge-when-pipeline-succeeds-button')).to have_content('Merge in progress')
end
end
end
diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb
index 4ad944366c8285196df8831bf9b9e7745efcc71a..b575aeff0d8814ccf862c96b49a05486b8e47ef5 100644
--- a/spec/features/merge_requests/widget_spec.rb
+++ b/spec/features/merge_requests/widget_spec.rb
@@ -3,8 +3,8 @@
describe 'Merge request', :feature, :js do
include WaitForAjax
- let(:project) { create(:project) }
let(:user) { create(:user) }
+ let(:project) { create(:project) }
let(:merge_request) { create(:merge_request, source_project: project) }
before do
@@ -31,7 +31,7 @@
wait_for_ajax
- expect(page).to have_selector('.accept_merge_request')
+ expect(page).to have_selector('.accept-merge-request')
end
end
@@ -51,6 +51,69 @@
expect(find('.js-environment-link')[:href]).to include(environment.formatted_external_url)
end
end
+
+ it 'shows green accept merge request button' do
+ # Wait for the `ci_status` and `merge_check` requests
+ wait_for_ajax
+ expect(page).to have_selector('.accept-merge-request.btn-create')
+ end
+ end
+
+ context 'view merge request with external CI service' do
+ before do
+ create(:service, project: project,
+ active: true,
+ type: 'CiService',
+ category: 'ci')
+
+ visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+ end
+
+ it 'has danger button while waiting for external CI status' do
+ # Wait for the `ci_status` and `merge_check` requests
+ wait_for_ajax
+ expect(page).to have_selector('.accept-merge-request.btn-danger')
+ end
+ end
+
+ context 'view merge request with failed GitLab CI pipelines' do
+ before do
+ commit_status = create(:commit_status, project: project, status: 'failed')
+ pipeline = create(:ci_pipeline, project: project,
+ sha: merge_request.diff_head_sha,
+ ref: merge_request.source_branch,
+ status: 'failed',
+ statuses: [commit_status])
+ create(:ci_build, :pending, pipeline: pipeline)
+
+ visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+ end
+
+ it 'has danger button when not succeeded' do
+ # Wait for the `ci_status` and `merge_check` requests
+ wait_for_ajax
+ expect(page).to have_selector('.accept-merge-request.btn-danger')
+ end
+ end
+
+ context 'view merge request with MWBS button' do
+ before do
+ commit_status = create(:commit_status, project: project, status: 'pending')
+ pipeline = create(:ci_pipeline, project: project,
+ sha: merge_request.diff_head_sha,
+ ref: merge_request.source_branch,
+ status: 'pending',
+ statuses: [commit_status])
+ create(:ci_build, :pending, pipeline: pipeline)
+
+ visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+ end
+
+ it 'has info button when MWBS button' do
+ # Wait for the `ci_status` and `merge_check` requests
+ wait_for_ajax
+ expect(page).to have_selector('.merge-when-pipeline-succeeds.btn-info')
+ end
end
context 'merge error' do