Commit 7e8dc22d authored by Douwe Maan's avatar Douwe Maan

Merge branch 'merge-if-green' into 'master'

Merge when build succeeds

### What does this MR do?
Adds a button to a MR when the build/ci is running so it can be merged when/if the build is successfull.

### Are there points in the code the reviewer needs to double check?
English spelling and whether or not the grammer is correct.

### Why was this MR needed?
When you expect its all good, and don't want to revisit the current MR it can be accepted allready.

### What are the relevant issue numbers / Feature requests?
Fixes #2640 -- although `Merge if green` is replaced with `Merge when the build succeeds` the general idea is the same.

### Screenshots (if relevant)
![Screenshot_from_2015-12-07_10-34-39](/uploads/b90b558b6ad6c8266bc9c96120d86f96/Screenshot_from_2015-12-07_10-34-39.png)

![Screenshot_from_2015-11-02_17-26-56](/uploads/9f52f56e3c5e9ec63cb8f42978c92a52/Screenshot_from_2015-11-02_17-26-56.png)
![Screenshot_from_2015-11-02_17-27-03](/uploads/119fc18c9b15ff73dc38010ac5b6244a/Screenshot_from_2015-11-02_17-27-03.png)

![Screenshot_from_2015-12-03_10-08-14](/uploads/bad7b35c7d129981d43631877c958be0/Screenshot_from_2015-12-03_10-08-14.png)

![Screenshot_from_2015-11-02_17-27-16](/uploads/b616d750a16cc11ba72f2ca84213515e/Screenshot_from_2015-11-02_17-27-16.png)

### Further considerations
What if there are minor things needed solving, e.g. Rubocop, the current implementation will reset the approved status of the MR. It might be a consideration keep the approval for team member, or even guests. This would require an extra option in the Admin screen, though might add extra value.

#### TODO
- [x] Docs
- [x] Specs

/cc @DouweM @rspeicher

See merge request !1729
parents 291678d6 90315114
Please view this file on the master branch, on stable branches it's out of date.
v 8.3.0 (unreleased)
- Merge when build succeeds (Zeger-Jan van de Weg)
- Bump gollum-lib to 4.1.0 (Stan Hu)
- Fix broken group avatar upload under "New group" (Stan Hu)
- Update project repositorize size and commit count during import:repos task (Stan Hu)
......
......@@ -18,6 +18,7 @@
.accept-merge-holder {
.accept-action {
display: inline-block;
float: left;
.accept_merge_request {
&.ci-pending,
......@@ -36,14 +37,15 @@
.accept-control {
display: inline-block;
float: left;
margin: 0;
margin-left: 20px;
padding: 5px;
padding-top: 12px;
line-height: 20px;
&.right {
float: right;
padding-top: 12px;
a {
color: $gl-gray;
}
......
......@@ -2,11 +2,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :module_enabled
before_action :merge_request, only: [
:edit, :update, :show, :diffs, :commits, :builds, :merge, :merge_check,
:ci_status, :toggle_subscription
:ci_status, :toggle_subscription, :cancel_merge_when_build_succeeds
]
before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits, :builds]
before_action :validates_merge_request, only: [:show, :diffs, :commits, :builds]
before_action :define_show_vars, only: [:show, :diffs, :commits, :builds]
before_action :define_widget_vars, only: [:merge, :cancel_merge_when_build_succeeds]
before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds]
# Allow read any merge_request
......@@ -164,15 +165,29 @@ class Projects::MergeRequestsController < Projects::ApplicationController
render partial: "projects/merge_requests/widget/show.html.haml", layout: false
end
def cancel_merge_when_build_succeeds
return access_denied! unless @merge_request.can_cancel_merge_when_build_succeeds?(current_user)
MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user).cancel(@merge_request)
end
def merge
return access_denied! unless @merge_request.can_be_merged_by?(current_user)
if @merge_request.mergeable?
@merge_request.update(merge_error: nil)
MergeWorker.perform_async(@merge_request.id, current_user.id, params)
@status = true
unless @merge_request.mergeable?
@status = :failed
return
end
@merge_request.update(merge_error: nil)
if params[:merge_when_build_succeeds] && @merge_request.ci_commit && @merge_request.ci_commit.active?
MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user, merge_params)
.execute(@merge_request)
@status = :merge_when_build_succeeds
else
@status = false
MergeWorker.perform_async(@merge_request.id, current_user.id, params)
@status = :success
end
end
......@@ -287,6 +302,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
end
def define_widget_vars
@ci_commit = @merge_request.ci_commit
end
def invalid_mr
# Render special view for MR with removed source or target branch
render 'invalid'
......@@ -300,6 +319,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
)
end
def merge_params
params.permit(:should_remove_source_branch, :commit_message)
end
# Make sure merge requests created before 8.0
# have head file in refs/merge-requests/
def ensure_ref_fetched
......
......@@ -165,6 +165,14 @@ module Ci
status == 'canceled'
end
def active?
running? || pending?
end
def complete?
canceled? || success? || failed?
end
def duration
duration_array = latest_statuses.map(&:duration).compact
duration_array.reduce(:+).to_i
......
# == Schema Information
#
# Table name: ci_builds
#
# id :integer not null, primary key
# project_id :integer
# status :string(255)
# finished_at :datetime
# trace :text
# created_at :datetime
# updated_at :datetime
# started_at :datetime
# runner_id :integer
# coverage :float
# commit_id :integer
# commands :text
# job_id :integer
# name :string(255)
# deploy :boolean default(FALSE)
# options :text
# allow_failure :boolean default(FALSE), not null
# stage :string(255)
# trigger_request_id :integer
# stage_idx :integer
# tag :boolean
# ref :string(255)
# user_id :integer
# type :string(255)
# target_url :string(255)
# description :string(255)
# artifacts_file :text
# project_id integer
# status string
# finished_at datetime
# trace text
# created_at datetime
# updated_at datetime
# started_at datetime
# runner_id integer
# coverage float
# commit_id integer
# commands text
# job_id integer
# name string
# deploy boolean default: false
# options text
# allow_failure boolean default: false, null: false
# stage string
# trigger_request_id integer
# stage_idx integer
# tag boolean
# ref string
# user_id integer
# type string
# target_url string
# description string
#
class CommitStatus < ActiveRecord::Base
......@@ -79,6 +75,10 @@ class CommitStatus < ActiveRecord::Base
build.update_attributes finished_at: Time.now
end
after_transition [:pending, :running] => :success do |build, transition|
MergeRequests::MergeWhenBuildSucceedsService.new(build.commit.gl_project, nil).trigger(build)
end
state :pending, value: 'pending'
state :running, value: 'running'
state :failed, value: 'failed'
......
......@@ -2,25 +2,28 @@
#
# Table name: merge_requests
#
# id :integer not null, primary key
# target_branch :string(255) not null
# source_branch :string(255) not null
# source_project_id :integer not null
# author_id :integer
# assignee_id :integer
# title :string(255)
# created_at :datetime
# updated_at :datetime
# milestone_id :integer
# state :string(255)
# merge_status :string(255)
# target_project_id :integer not null
# iid :integer
# description :text
# position :integer default(0)
# locked_at :datetime
# updated_by_id :integer
# merge_error :string(255)
# id :integer not null, primary key
# target_branch :string(255) not null
# source_branch :string(255) not null
# source_project_id :integer not null
# author_id :integer
# assignee_id :integer
# title :string(255)
# created_at :datetime
# updated_at :datetime
# milestone_id :integer
# state :string(255)
# merge_status :string(255)
# target_project_id :integer not null
# iid :integer
# description :text
# position :integer default(0)
# locked_at :datetime
# updated_by_id :integer
# merge_error :string(255)
# merge_params :text (serialized to hash)
# merge_when_build_succeeds :boolean default(false), not null
# merge_user_id :integer
#
require Rails.root.join("app/models/commit")
......@@ -35,9 +38,12 @@ class MergeRequest < ActiveRecord::Base
belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project"
belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project"
belongs_to :merge_user, class_name: "User"
has_one :merge_request_diff, dependent: :destroy
serialize :merge_params, Hash
after_create :create_merge_request_diff
after_update :update_merge_request_diff
......@@ -121,6 +127,7 @@ class MergeRequest < ActiveRecord::Base
validates :source_branch, presence: true
validates :target_project, presence: true
validates :target_branch, presence: true
validates :merge_user, presence: true, if: :merge_when_build_succeeds?
validate :validate_branches
validate :validate_fork
......@@ -258,6 +265,16 @@ class MergeRequest < ActiveRecord::Base
end
end
def can_cancel_merge_when_build_succeeds?(current_user)
can_be_merged_by?(current_user) || self.author == current_user
end
def can_remove_source_branch?(current_user)
!source_project.protected_branch?(source_branch) &&
!source_project.root_ref?(source_branch) &&
Ability.abilities.allowed?(current_user, :push_code, source_project)
end
def mr_and_commit_notes
# Fetch comments only from last 100 commits
commits_for_notes_limit = 100
......@@ -393,6 +410,16 @@ class MergeRequest < ActiveRecord::Base
message
end
def reset_merge_when_build_succeeds
return unless merge_when_build_succeeds?
self.merge_when_build_succeeds = false
self.merge_user = nil
self.merge_params = nil
self.save
end
# Return array of possible target branches
# depends on target project of MR
def target_branches
......@@ -480,8 +507,6 @@ class MergeRequest < ActiveRecord::Base
end
def ci_commit
if last_commit and source_project
source_project.ci_commit(last_commit.id)
end
@ci_commit ||= source_project.ci_commit(last_commit.id) if last_commit && source_project
end
end
......@@ -6,15 +6,12 @@ module MergeRequests
# Executed when you do merge via GitLab UI
#
class MergeService < MergeRequests::BaseService
attr_reader :merge_request, :commit_message
attr_reader :merge_request
def execute(merge_request, commit_message)
@commit_message = commit_message
def execute(merge_request)
@merge_request = merge_request
unless @merge_request.mergeable?
return error('Merge request is not mergeable')
end
return error('Merge request is not mergeable') unless @merge_request.mergeable?
merge_request.in_locked_state do
if commit
......@@ -32,7 +29,7 @@ module MergeRequests
committer = repository.user_to_committer(current_user)
options = {
message: commit_message,
message: params[:commit_message] || merge_request.merge_commit_message,
author: committer,
committer: committer
}
......@@ -46,6 +43,11 @@ module MergeRequests
def after_merge
MergeRequests::PostMergeService.new(project, current_user).execute(merge_request)
if params[:should_remove_source_branch]
DeleteBranchService.new(@merge_request.source_project, current_user).
execute(merge_request.source_branch)
end
end
end
end
module MergeRequests
class MergeWhenBuildSucceedsService < MergeRequests::BaseService
# Marks the passed `merge_request` to be merged when the build succeeds or
# updates the params for the automatic merge
def execute(merge_request)
merge_request.merge_params.merge!(params)
# The service is also called when the merge params are updated.
already_approved = merge_request.merge_when_build_succeeds?
unless already_approved
merge_request.merge_when_build_succeeds = true
merge_request.merge_user = @current_user
SystemNoteService.merge_when_build_succeeds(merge_request, @project, @current_user, merge_request.last_commit)
end
merge_request.save
end
# Triggers the automatic merge of merge_request once the build succeeds
def trigger(build)
merge_requests = merge_request_from(build)
merge_requests.each do |merge_request|
next unless merge_request.merge_when_build_succeeds?
if merge_request.ci_commit && merge_request.ci_commit.success? && merge_request.mergeable?
MergeWorker.perform_async(merge_request.id, merge_request.merge_user_id, merge_request.merge_params)
end
end
end
# Cancels the automatic merge
def cancel(merge_request)
if merge_request.merge_when_build_succeeds? && merge_request.open?
merge_request.reset_merge_when_build_succeeds
SystemNoteService.cancel_merge_when_build_succeeds(merge_request, @project, @current_user)
success
else
error("Can't cancel the automatic merge", 406)
end
end
private
def merge_request_from(build)
merge_requests = @project.origin_merge_requests.opened.where(source_branch: build.ref).to_a
merge_requests += @project.fork_merge_requests.opened.where(source_branch: build.ref).to_a
merge_requests.uniq.select(&:source_project)
end
end
end
......@@ -11,6 +11,7 @@ module MergeRequests
# empty diff during a manual merge
close_merge_requests
reload_merge_requests
reset_merge_when_build_succeeds
# Leave a system note if a branch was deleted/added
if branch_added? || branch_removed?
......@@ -57,7 +58,6 @@ module MergeRequests
merge_requests = filter_merge_requests(merge_requests)
merge_requests.each do |merge_request|
if merge_request.source_branch == @branch_name || force_push?
merge_request.reload_code
merge_request.mark_as_unchecked
......@@ -76,6 +76,10 @@ module MergeRequests
end
end
def reset_merge_when_build_succeeds
merge_requests_for_source_branch.each(&:reset_merge_when_build_succeeds)
end
def find_new_commits
if branch_added?
@commits = []
......
......@@ -130,6 +130,20 @@ class SystemNoteService
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when 'merge when build succeeds' is executed
def self.merge_when_build_succeeds(noteable, project, author, last_commit)
body = "Enabled an automatic merge when the build for #{last_commit.to_reference(project)} succeeds"
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when 'merge when build succeeds' is canceled
def self.cancel_merge_when_build_succeeds(noteable, project, author)
body = "Canceled the automatic merge"
create_note(noteable: noteable, project: project, author: author, note: body)
end
# Called when the title of a Noteable is changed
#
# noteable - Noteable object that responds to `title`
......
- ci_commit = merge_request.ci_commit
%li{ class: mr_css_classes(merge_request) }
.merge-request-title
%span.merge-request-title-text
= link_to_gfm merge_request.title, merge_request_path(merge_request), class: "row_title"
.pull-right.light
- if ci_commit
= render_ci_status(ci_commit)
- if merge_request.ci_commit
= render_ci_status(merge_request.ci_commit)
- if merge_request.merged?
%span
= icon('check')
......
:plain
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/accept'))}");
- if @status
- case @status
- when :success
:plain
merge_request_widget.mergeInProgress(#{params[:should_remove_source_branch] == '1'});
- when :merge_when_build_succeeds
:plain
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/merge_when_build_succeeds'))}");
- else
:plain
$('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/reload'))}");
......@@ -14,7 +14,7 @@
= @merge_request.target_branch
The source branch has been removed.
- elsif can_remove_branch?(@merge_request.source_project, @merge_request.source_branch)
- elsif @merge_request.can_remove_source_branch?(current_user)
.remove_source_branch_widget
%p
= succeed '.' do
......@@ -50,5 +50,3 @@
$('.remove_source_branch_in_progress').hide();
$('.remove_source_branch_widget.failed').show();
});
......@@ -13,6 +13,8 @@
= render 'projects/merge_requests/widget/open/conflicts'
- elsif @merge_request.work_in_progress?
= render 'projects/merge_requests/widget/open/wip'
- elsif @merge_request.merge_when_build_succeeds?
= render 'projects/merge_requests/widget/open/merge_when_build_succeeds'
- elsif !@merge_request.can_be_merged_by?(current_user)
= render 'projects/merge_requests/widget/open/not_allowed'
- elsif @merge_request.can_be_merged?
......
......@@ -3,26 +3,60 @@
= form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-requires-input' } do |f|
= hidden_field_tag :authenticity_token, form_authenticity_token
.accept-merge-holder.clearfix.js-toggle-container
.accept-action
= f.button class: "btn btn-create accept_merge_request#{status_class}" do
Accept Merge Request
- if can_remove_branch?(@merge_request.source_project, @merge_request.source_branch) && !@merge_request.for_fork?
.accept-control.checkbox
= label_tag :should_remove_source_branch, class: "remove_source_checkbox" do
= check_box_tag :should_remove_source_branch
Remove source branch
.accept-control.right
= link_to "#", class: "modify-merge-commit-link js-toggle-button" do
= icon('edit')
Modify commit message
.js-toggle-content.hide.prepend-top-20
.clearfix
.accept-action
- if @ci_commit && @ci_commit.active?
%span.btn-group
= link_to "#", class: "btn btn-create merge_when_build_succeeds" do
Merge When Build Succeeds
%a.btn.btn-success.dropdown-toggle{ 'data-toggle' => 'dropdown' }
%span.caret
%span.sr-only
Select Merge Moment
%ul.dropdown-menu.dropdown-menu-right{ role: 'menu' }
%li
= link_to "#", class: "merge_when_build_succeeds" do
= icon('check fw')
Merge When Build Succeeds
%li
= link_to "#", class: "accept_merge_request" do
= icon('warning fw')
Merge Immediately
- else
= f.button class: "btn btn-create btn-grouped accept_merge_request #{status_class}" do
Accept Merge Request
- if @merge_request.can_remove_source_branch?(current_user)
.accept-control.checkbox
= label_tag :should_remove_source_branch, class: "remove_source_checkbox" do
= check_box_tag :should_remove_source_branch
Remove source branch
.accept-control.right
= link_to "#", class: "modify-merge-commit-link js-toggle-button" do
= icon('edit')
Modify commit message
.js-toggle-content.hide.prepend-top-default
= render 'shared/commit_message_container', params: params,
text: @merge_request.merge_commit_message,
rows: 14, hint: true
= hidden_field_tag :merge_when_build_succeeds, "", autocomplete: "off"
:javascript
$('.accept-mr-form').on('ajax:before', function() {
var btn = $('.accept_merge_request');
btn.disable();
btn.html("<i class='fa fa-spinner fa-spin'></i> Merge in progress");
$('.accept_merge_request').on('click', function() {
$(this).html("<i class='fa fa-spinner fa-spin'></i> Merge in progress");
});
$('.accept-mr-form').on('ajax:send', function() {
$(".accept-mr-form :input").disable();
});
$('a.accept_merge_request').on('click', function(e) {
e.preventDefault();
$(this).closest("form").submit();
});
$('a.merge_when_build_succeeds').on('click', function(e) {
e.preventDefault();
$("#merge_when_build_succeeds").val("1");
$(this).closest("form").submit();
});
%h4
Set by #{link_to_member(@project, @merge_request.merge_user, avatar: true)}
to be merged automatically when the build succeeds.
%div
- should_remove_source_branch = @merge_request.merge_params["should_remove_source_branch"].present?
%p
= succeed '.' do
The changes will be merged into
%span.label-branch= @merge_request.target_branch
- if should_remove_source_branch
The source branch will be removed.
- else
The source branch will not be removed.
- remove_source_branch_button = @merge_request.can_remove_source_branch?(current_user) && !should_remove_source_branch
- user_can_cancel_automatic_merge = @merge_request.can_cancel_merge_when_build_succeeds?(current_user)
- if remove_source_branch_button || user_can_cancel_automatic_merge
.clearfix.prepend-top-10
- if remove_source_branch_button
= link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
= icon('times')
Remove Source Branch When Merged
- if user_can_cancel_automatic_merge
= link_to cancel_merge_when_build_succeeds_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request), remote: true, method: :post, class: "btn btn-grouped btn-warning btn-sm" do
Cancel Automatic Merge
......@@ -8,16 +8,7 @@ class MergeWorker
current_user = User.find(current_user_id)
merge_request = MergeRequest.find(merge_request_id)
result = MergeRequests::MergeService.new(merge_request.target_project, current_user).
execute(merge_request, params[:commit_message])
if result[:status] == :success && params[:should_remove_source_branch].present?
DeleteBranchService.new(merge_request.source_project, current_user).
execute(merge_request.source_branch)
merge_request.source_project.repository.expire_branch_names
end
result
MergeRequests::MergeService.new(merge_request.target_project, current_user, params).
execute(merge_request)
end
end
......@@ -573,8 +573,9 @@ Rails.application.routes.draw do
get :commits
get :diffs
get :builds
post :merge
get :merge_check
post :merge
post :cancel_merge_when_build_succeeds
get :ci_status
post :toggle_subscription
end
......
class AddMergeWhenBuildSucceedsToMergeRequest < ActiveRecord::Migration
def change
add_column :merge_requests, :merge_params, :text
add_column :merge_requests, :merge_when_build_succeeds, :boolean, default: false, null: false
add_column :merge_requests, :merge_user_id, :integer
end
end
......@@ -476,9 +476,9 @@ ActiveRecord::Schema.define(version: 20151203162133) do
add_index "merge_request_diffs", ["merge_request_id"], name: "index_merge_request_diffs_on_merge_request_id", unique: true, using: :btree
create_table "merge_requests", force: :cascade do |t|
t.string "target_branch", null: false
t.string "source_branch", null: false
t.integer "source_project_id", null: false
t.string "target_branch", null: false
t.string "source_branch", null: false
t.integer "source_project_id", null: false
t.integer "author_id"
t.integer "assignee_id"
t.string "title"
......@@ -487,13 +487,16 @@ ActiveRecord::Schema.define(version: 20151203162133) do
t.integer "milestone_id"
t.string "state"
t.string "merge_status"
t.integer "target_project_id", null: false
t.integer "target_project_id", null: false
t.integer "iid"
t.text "description"
t.integer "position", default: 0
t.integer "position", default: 0
t.datetime "locked_at"
t.integer "updated_by_id"
t.string "merge_error"
t.text "merge_params"
t.boolean "merge_when_build_succeeds", default: false, null: false
t.integer "merge_user_id"
end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
......
......@@ -335,9 +335,57 @@ PUT /projects/:id/merge_request/:merge_request_id/merge
Parameters:
- `id` (required) - The ID of a project
- `merge_request_id` (required) - ID of MR
- `merge_commit_message` (optional) - Custom merge commit message
- `id` (required) - The ID of a project
- `merge_request_id` (required) - ID of MR
- `merge_commit_message` (optional) - Custom merge commit message
- `should_remove_source_branch` (optional) - if `true` removes the source branch
- `merged_when_build_succeeds` (optional) - if `true` the MR is merge when the build succeeds
```json
{
"id": 1,
"target_branch": "master",
"source_branch": "test1",
"project_id": 3,
"title": "test1",
"state": "merged",
"upvotes": 0,
"downvotes": 0,
"author": {
"id": 1,
"username": "admin",
"email": "admin@example.com",
"name": "Administrator",
"state": "active",
"created_at": "2012-04-29T08:46:00Z"
},
"assignee": {
"id": 1,
"username": "admin",
"email": "admin@example.com",
"name": "Administrator",
"state": "active",
"created_at": "2012-04-29T08:46:00Z"
}
}
```
## Cancel Merge When Build Succeeds
If successful you'll get `200 OK`.
If you don't have permissions to accept this merge request - you'll get a 401
If the merge request is already merged or closed - you get 405 and error message 'Method Not Allowed'
In case the merge request is not set to be merged when the build succeeds, you'll also get a 406 error.
```
PUT /projects/:id/merge_request/:merge_request_id/cancel_merge_when_build_succeeds
```
Parameters:
- `id` (required) - The ID of a project
- `merge_request_id` (required) - ID of MR
```json
{
......
......@@ -17,4 +17,5 @@
- [Milestones](milestones.md)
- [Merge Requests](merge_requests.md)
- ["Work In Progress" Merge Requests](wip_merge_requests.md)
- [Merge When Build Succeeds](merge_when_build_succeeds.md)
- [Manage large binaries with Git LFS](lfs/manage_large_binaries_with_git_lfs.md)
# Merge When Build Succeeds
When reviewing a merge request that looks ready to merge but still has one or more CI builds running, you can set it to be merged automatically when all builds succeed. This way, you don't have to wait for the builds to finish and remember to merge the request manually.
![Enable](merge_when_build_succeeds/enable.png)
When you hit the "Merge When Build Succeeds" button, the status of the merge request will be updated to represent the impending merge. If you cannot wait for the build to succeed and want to merge immediately, this option is available in the dropdown menu on the right of the main button.
Both team developers and the author of the merge request have the option to cancel the automatic merge if they find a reason why it shouldn't be merged after all.
![Status](merge_when_build_succeeds/status.png)
When the build succeeds, the merge request will automatically be merged. When the build fails, the author gets a chance to retry any failed builds, or to push new commits to fix the failure.
When the builds are retried and succeed on the second try, the merge request will automatically be merged after all. When the merge request is updated with new commits, the automatic merge is automatically canceled to allow the new changes to be reviewed.
......@@ -171,6 +171,7 @@ module API
expose :description
expose :work_in_progress?, as: :work_in_progress
expose :milestone, using: Entities::Milestone
expose :merge_when_build_succeeds
end
class MergeRequestChanges < MergeRequest
......
......@@ -195,46 +195,54 @@ module API
# Merge MR
#
# Parameters:
# id (required) - The ID of a project
# merge_request_id (required) - ID of MR
# merge_commit_message (optional) - Custom merge commit message
# id (required) - The ID of a project
# merge_request_id (required) - ID of MR
# merge_commit_message (optional) - Custom merge commit message
# should_remove_source_branch (optional) - When true, the source branch will be deleted if possible
# merge_when_build_succeeds (optional) - When true, this MR will be merged when the build succeeds
# Example:
# PUT /projects/:id/merge_request/:merge_request_id/merge
#
put ":id/merge_request/:merge_request_id/merge" do
merge_request = user_project.merge_requests.find(params[:merge_request_id])
allowed = ::Gitlab::GitAccess.new(current_user, user_project).
can_push_to_branch?(merge_request.target_branch)
# Merge request can not be merged
# because user dont have permissions to push into target branch
unauthorized! unless merge_request.can_be_merged_by?(current_user)
not_allowed! if !merge_request.open? || merge_request.work_in_progress?
if allowed
if merge_request.unchecked?
merge_request.check_if_can_be_merged
end
merge_request.check_if_can_be_merged if merge_request.unchecked?
if merge_request.open? && !merge_request.work_in_progress?
if merge_request.can_be_merged?
commit_message = params[:merge_commit_message] || merge_request.merge_commit_message
::MergeRequests::MergeService.new(merge_request.target_project, current_user).
execute(merge_request, commit_message)
present merge_request, with: Entities::MergeRequest
else
render_api_error!('Branch cannot be merged', 405)
end
else
# Merge request can not be merged
# because it is already closed/merged or marked as WIP
not_allowed!
end
render_api_error!('Branch cannot be merged', 406) unless merge_request.can_be_merged?
merge_params = {
commit_message: params[:merge_commit_message],
should_remove_source_branch: params[:should_remove_source_branch]
}
if parse_boolean(params[:merge_when_build_succeeds]) && merge_request.ci_commit && merge_request.ci_commit.active?
::MergeRequests::MergeWhenBuildSucceedsService.new(merge_request.target_project, current_user, merge_params).
execute(merge_request)
else
# Merge request can not be merged
# because user dont have permissions to push into target branch
unauthorized!
::MergeRequests::MergeService.new(merge_request.target_project, current_user, merge_params).
execute(merge_request)
end
present merge_request, with: Entities::MergeRequest
end
# Cancel Merge if Merge When build succeeds is enabled
# Parameters:
# id (required) - The ID of a project
# merge_request_id (required) - ID of MR
#
post ":id/merge_request/:merge_request_id/cancel_merge_when_build_succeeds" do
merge_request = user_project.merge_requests.find(params[:merge_request_id])
unauthorized! unless merge_request.can_cancel_merge_when_build_succeeds?(current_user)
::MergeRequest::MergeWhenBuildSucceedsService.new(merge_request.target_project, current_user).cancel(merge_request)
end
# Get a merge request's comments
#
......
......@@ -2,17 +2,18 @@
#
# Table name: commits
#
# id :integer not null, primary key
# project_id :integer
# ref :string(255)
# sha :string(255)
# before_sha :string(255)
# push_data :text
# created_at :datetime
# updated_at :datetime
# tag :boolean default(FALSE)
# yaml_errors :text
# committed_at :datetime
# id :integer not null, primary key
# project_id :integer
# ref :string(255)
# sha :string(255)
# before_sha :string(255)
# push_data :text
# created_at :datetime
# updated_at :datetime
# tag :boolean default(FALSE)
# yaml_errors :text
# committed_at :datetime
# gl_project_id :integer
#
# Read about factories at https://github.com/thoughtbot/factory_girl
......
......@@ -5,7 +5,7 @@ FactoryGirl.define do
name 'default'
status 'success'
description 'commit status'
commit factory: :ci_commit
commit factory: :ci_commit_with_one_job
factory :generic_commit_status, class: GenericCommitStatus do
name 'generic'
......
......@@ -65,6 +65,11 @@ FactoryGirl.define do
target_branch "master"
end
trait :merge_when_build_succeeds do
merge_when_build_succeeds true
merge_user author
end
factory :closed_merge_request, traits: [:closed]
factory :reopened_merge_request, traits: [:reopened]
factory :merge_request_with_diffs, traits: [:with_diffs]
......
require 'spec_helper'
feature 'Merge When Build Succeeds', feature: true, js: true do
let(:user) { create(:user) }
let(:project) { create(:project, :public) }
let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: "Bug NS-04") }
before do
project.team << [user, :master]
project.enable_ci
end
context "Active build for Merge Request" do
let!(:ci_commit) { create(:ci_commit, gl_project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) }
let!(:ci_build) { create(:ci_build, commit: ci_commit) }
before do
login_as user
visit_merge_request(merge_request)
end
it 'displays the Merge When Build Succeeds button' do
expect(page).to have_link "Merge When Build Succeeds"
end
context "Merge When Build succeeds enabled" do
before do
click_link "Merge When Build Succeeds"
end
it 'activates Merge When Build Succeeds feature' do
expect(page).to have_link "Cancel Automatic Merge"
expect(page).to have_content "Set by #{user.name} to be merged automatically when the build succeeds."
expect(page).to have_content "The source branch will not be removed."
visit_merge_request(merge_request) # Needed to refresh the page
expect(page).to have_content /Enabled an automatic merge when the build for [0-9a-f]{8} succeeds/i
end
end
end
context 'When it is enabled' do
let(:merge_request) do
create(:merge_request_with_diffs, :simple, source_project: project, author: user,
merge_user: user, title: "MepMep", merge_when_build_succeeds: true)
end
let!(:ci_commit) { create(:ci_commit, gl_project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) }
let!(:ci_build) { create(:ci_build, commit: ci_commit) }
before do
login_as user
visit_merge_request(merge_request)
end
it 'cancels the automatic merge' do
click_link "Cancel Automatic Merge"
expect(page).to have_link "Merge When Build Succeeds"
visit_merge_request(merge_request) # Needed to refresh the page
expect(page).to have_content "Canceled the automatic merge"
end
it "allows the user to remove the source branch" do
expect(page).to have_link "Remove Source Branch When Merged"
click_link "Remove Source Branch When Merged"
expect(page).to have_content "The source branch will be removed"
end
end
context 'Build is not active' do
it "should not allow for enabling" do
visit_merge_request(merge_request)
expect(page).not_to have_link "Merge When Build Succeeds"
end
end
def visit_merge_request(merge_request)
visit namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request)
end
end
......@@ -31,7 +31,7 @@ describe MergeRequest do
describe 'associations' do
it { is_expected.to belong_to(:target_project).with_foreign_key(:target_project_id).class_name('Project') }
it { is_expected.to belong_to(:source_project).with_foreign_key(:source_project_id).class_name('Project') }
it { is_expected.to belong_to(:merge_user).class_name("User") }
it { is_expected.to have_one(:merge_request_diff).dependent(:destroy) }
end
......@@ -48,12 +48,32 @@ describe MergeRequest do
describe 'validation' do
it { is_expected.to validate_presence_of(:target_branch) }
it { is_expected.to validate_presence_of(:source_branch) }
context "Validation of merge user with Merge When Build succeeds" do
it "allows user to be nil when the feature is disabled" do
expect(subject).to be_valid
end
it "is invalid without merge user" do
subject.merge_when_build_succeeds = true
expect(subject).not_to be_valid
end
it "is valid with merge user" do
subject.merge_when_build_succeeds = true
subject.merge_user = build(:user)
expect(subject).to be_valid
end
end
end
describe 'respond to' do
it { is_expected.to respond_to(:unchecked?) }
it { is_expected.to respond_to(:can_be_merged?) }
it { is_expected.to respond_to(:cannot_be_merged?) }
it { is_expected.to respond_to(:merge_params) }
it { is_expected.to respond_to(:merge_when_build_succeeds) }
end
describe '#to_reference' do
......@@ -172,6 +192,50 @@ describe MergeRequest do
end
end
describe '#can_remove_source_branch?' do
let(:user) { create(:user) }
let(:user2) { create(:user) }
before do
subject.source_project.team << [user, :master]
subject.source_branch = "feature"
subject.target_branch = "master"
subject.save!
end
it "can't be removed when its a protected branch" do
allow(subject.source_project).to receive(:protected_branch?).and_return(true)
expect(subject.can_remove_source_branch?(user)).to be_falsey
end
it "cant remove a root ref" do
subject.source_branch = "master"
subject.target_branch = "feature"
expect(subject.can_remove_source_branch?(user)).to be_falsey
end
it "is unable to remove the source branch for a project the user cannot push to" do
expect(subject.can_remove_source_branch?(user2)).to be_falsey
end
it "is can be removed in all other cases" do
expect(subject.can_remove_source_branch?(user)).to be_truthy
end
end
describe "#reset_merge_when_build_succeeds" do
let(:merge_if_green) { create :merge_request, merge_when_build_succeeds: true, merge_user: create(:user) }
it "sets the item to false" do
merge_if_green.reset_merge_when_build_succeeds
merge_if_green.reload
expect(merge_if_green.merge_when_build_succeeds).to be_falsey
end
end
describe "#hook_attrs" do
it "has all the required keys" do
attrs = subject.hook_attrs
......
......@@ -320,19 +320,21 @@ describe API::API, api: true do
end
describe "PUT /projects/:id/merge_request/:merge_request_id/merge" do
let(:ci_commit) { create(:ci_commit_without_jobs) }
it "should return merge_request in case of success" do
put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user)
expect(response.status).to eq(200)
end
it "should return 405 if branch can't be merged" do
it "should return 406 if branch can't be merged" do
allow_any_instance_of(MergeRequest).
to receive(:can_be_merged?).and_return(false)
put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user)
expect(response.status).to eq(405)
expect(response.status).to eq(406)
expect(json_response['message']).to eq('Branch cannot be merged')
end
......@@ -357,6 +359,17 @@ describe API::API, api: true do
expect(response.status).to eq(401)
expect(json_response['message']).to eq('401 Unauthorized')
end
it "enables merge when build succeeds if the ci is active" do
allow_any_instance_of(MergeRequest).to receive(:ci_commit).and_return(ci_commit)
allow(ci_commit).to receive(:active?).and_return(true)
put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user), merge_when_build_succeeds: true
expect(response.status).to eq(200)
expect(json_response['title']).to eq('Test')
expect(json_response['merge_when_build_succeeds']).to eq(true)
end
end
describe "PUT /projects/:id/merge_request/:merge_request_id" do
......
......@@ -13,12 +13,13 @@ describe MergeRequests::MergeService do
describe :execute do
context 'valid params' do
let(:service) { MergeRequests::MergeService.new(project, user, {}) }
let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
before do
allow(service).to receive(:execute_hooks)
perform_enqueued_jobs do
service.execute(merge_request, 'Awesome message')
service.execute(merge_request)
end
end
......@@ -38,14 +39,14 @@ describe MergeRequests::MergeService do
end
context "error handling" do
let(:service) { MergeRequests::MergeService.new(project, user, {}) }
let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') }
it 'saves error if there is an exception' do
allow(service).to receive(:repository).and_raise("error")
allow(service).to receive(:execute_hooks)
service.execute(merge_request, 'Awesome message')
service.execute(merge_request)
expect(merge_request.merge_error).to eq("Something went wrong during merge")
end
......
require 'spec_helper'
describe MergeRequests::MergeWhenBuildSucceedsService do
let(:user) { create(:user) }
let(:merge_request) { create(:merge_request) }
let(:mr_merge_if_green_enabled) do
create(:merge_request, merge_when_build_succeeds: true, merge_user: user,
source_branch: "source_branch", target_branch: project.default_branch,
source_project: project, target_project: project, state: "opened")
end
let(:project) { create(:project) }
let(:ci_commit) { create(:ci_commit_with_one_job, ref: mr_merge_if_green_enabled.source_branch, gl_project: project) }
let(:service) { MergeRequests::MergeWhenBuildSucceedsService.new(project, user, commit_message: 'Awesome message') }
describe "#execute" do
context 'first time enabling' do
before do
allow(merge_request).to receive(:ci_commit).and_return(ci_commit)
service.execute(merge_request)
end
it 'sets the params, merge_user, and flag' do
expect(merge_request).to be_valid
expect(merge_request.merge_when_build_succeeds).to be_truthy
expect(merge_request.merge_params).to eq commit_message: 'Awesome message'
expect(merge_request.merge_user).to be user
end
it 'creates a system note' do
note = merge_request.notes.last
expect(note.note).to match /Enabled an automatic merge when the build for (\w+\/\w+@)?[0-9a-z]{8}/
end
end
context 'already approved' do
let(:service) { MergeRequests::MergeWhenBuildSucceedsService.new(project, user, new_key: true) }
let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch) }
before do
allow(mr_merge_if_green_enabled).to receive(:ci_commit).and_return(ci_commit)
allow(mr_merge_if_green_enabled).to receive(:mergeable?).and_return(true)
allow(ci_commit).to receive(:success?).and_return(true)
end
it 'updates the merge params' do
expect(SystemNoteService).not_to receive(:merge_when_build_succeeds)
service.execute(mr_merge_if_green_enabled)
expect(mr_merge_if_green_enabled.merge_params).to have_key(:new_key)
end
end
end
describe "#trigger" do
let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch, status: "success") }
it "merges all merge requests with merge when build succeeds enabled" do
allow_any_instance_of(MergeRequest).to receive(:ci_commit).and_return(ci_commit)
allow(ci_commit).to receive(:success?).and_return(true)
expect(MergeWorker).to receive(:perform_async)
service.trigger(build)
end
end
describe "#cancel" do
before do
service.cancel(mr_merge_if_green_enabled)
end
it "resets all the merge_when_build_succeeds params" do
expect(mr_merge_if_green_enabled.merge_when_build_succeeds).to be_falsey
expect(mr_merge_if_green_enabled.merge_params).to eq({})
expect(mr_merge_if_green_enabled.merge_user).to be nil
end
it 'Posts a system note' do
note = mr_merge_if_green_enabled.notes.last
expect(note.note).to include 'Canceled the automatic merge'
end
end
end
......@@ -17,7 +17,9 @@ describe MergeRequests::RefreshService do
source_project: @project,
source_branch: 'master',
target_branch: 'feature',
target_project: @project)
target_project: @project,
merge_when_build_succeeds: true,
merge_user: @user)
@fork_merge_request = create(:merge_request,
source_project: @fork_project,
......@@ -46,6 +48,7 @@ describe MergeRequests::RefreshService do
it { expect(@merge_request.notes).not_to be_empty }
it { expect(@merge_request).to be_open }
it { expect(@merge_request.merge_when_build_succeeds).to be_falsey}
it { expect(@fork_merge_request).to be_open }
it { expect(@fork_merge_request.notes).to be_empty }
end
......@@ -146,6 +149,7 @@ describe MergeRequests::RefreshService do
end
end
def reload_mrs
@merge_request.reload
@fork_merge_request.reload
......
......@@ -207,6 +207,32 @@ describe SystemNoteService do
end
end
describe '.merge_when_build_succeeds' do
let(:ci_commit) { build :ci_commit_without_jobs }
let(:noteable) { create :merge_request }
subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.last_commit) }
it_behaves_like 'a system note'
it "posts the Merge When Build Succeeds system note" do
expect(subject.note).to match /Enabled an automatic merge when the build for (\w+\/\w+@)?[0-9a-f]{40} succeeds/
end
end
describe '.cancel_merge_when_build_succeeds' do
let(:ci_commit) { build :ci_commit_without_jobs }
let(:noteable) { create :merge_request }
subject { described_class.cancel_merge_when_build_succeeds(noteable, project, author) }
it_behaves_like 'a system note'
it "posts the Merge When Build Succeeds system note" do
expect(subject.note).to eq "Canceled the automatic merge"
end
end
describe '.change_title' do
subject { described_class.change_title(noteable, project, author, 'Old title') }
......
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