Commit dd885b67 authored by Eric Eastwood's avatar Eric Eastwood

Default to subtle MR mege button until CI status is available

See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9245

Conflicts:
	app/assets/javascripts/merge_request_widget.js.es6
	app/views/projects/merge_requests/widget/open/_accept.html.haml
parent 30ed0e09
......@@ -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) {
......
......@@ -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');
});
......
......@@ -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 {
......
......@@ -333,6 +333,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
......@@ -470,6 +471,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def ci_status
pipeline = @merge_request.head_pipeline
@pipelines = @merge_request.all_pipelines
if pipeline
status = pipeline.status
......@@ -488,7 +490,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
......
......@@ -711,7 +711,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?
......
- 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
......
---
title: Default to subtle MR mege button until CI status is available
merge_request:
author:
......@@ -34,7 +34,7 @@ feature 'Merge immediately', :feature, :js do
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
......
......@@ -3,8 +3,8 @@ require 'rails_helper'
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 @@ describe 'Merge request', :feature, :js do
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 @@ describe 'Merge request', :feature, :js do
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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment