Default to subtle MR mege button until CI status is available
See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9245
This commit is contained in:
parent
b6a945b393
commit
605fff9123
9 changed files with 123 additions and 36 deletions
|
@ -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';
|
||||
}
|
||||
|
||||
this.setMergeButtonClass(stateClass, $html);
|
||||
};
|
||||
|
||||
MergeRequestWidget.prototype.showCICoverage = function(coverage) {
|
||||
var text;
|
||||
text = 'Coverage ' + coverage + '%';
|
||||
return $('.ci_widget:visible .ci-coverage').text(text);
|
||||
};
|
||||
|
||||
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 {
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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?
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
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
|
||||
|
|
Loading…
Reference in a new issue