From 77f8a1e392b64f51326df8aebdc77e97af07bfed Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <mail@zjvandeweg.nl>
Date: Mon, 2 Nov 2015 17:27:38 +0100
Subject: [PATCH] Merge when build succeeds
---
CHANGELOG | 1 +
.../stylesheets/pages/merge_requests.scss | 4 +-
.../projects/merge_requests_controller.rb | 35 ++++++++++++---
app/models/ci/commit.rb | 8 ++++
app/models/commit_status.rb | 33 ++++++++++++++
app/models/merge_request.rb | 13 ++++++
.../merge_when_build_succeeds_service.rb | 43 +++++++++++++++++++
.../merge_requests/refresh_service.rb | 8 +++-
app/services/system_note_service.rb | 14 ++++++
.../cancel_merge_when_build_succeeds.js.haml | 2 +
.../projects/merge_requests/merge.js.haml | 6 ++-
.../merge_requests/widget/_open.html.haml | 2 +
.../widget/open/_accept.html.haml | 18 +++++---
.../open/_merge_when_build_succeeds.html.haml | 27 ++++++++++++
config/routes.rb | 1 +
...ge_when_build_succeeds_to_merge_request.rb | 7 +++
db/schema.rb | 17 +++++---
17 files changed, 219 insertions(+), 20 deletions(-)
create mode 100644 app/services/merge_requests/merge_when_build_succeeds_service.rb
create mode 100644 app/views/projects/merge_requests/cancel_merge_when_build_succeeds.js.haml
create mode 100644 app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
create mode 100644 db/migrate/20151028152939_add_merge_when_build_succeeds_to_merge_request.rb
diff --git a/CHANGELOG b/CHANGELOG
index 0d89fca9fc1..195e53abd8b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,7 @@ v 8.2.0 (unreleased)
- Fix: 500 error returned if destroy request without HTTP referer (Kazuki Shimizu)
- Remove deprecated CI events from project settings page
- Use issue editor as cross reference comment author when issue is edited with a new mention.
+ - Merge when build succeeds (Zeger-Jan van de Weg)
v 8.1.1
- Fix cloning Wiki repositories via HTTP (Stan Hu)
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index f0b3667acca..b5a8c893678 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -19,18 +19,20 @@
.accept-merge-holder {
.accept-action {
display: inline-block;
+ float: left;
}
.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;
}
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 16c42386623..2f9b8a25edf 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -2,7 +2,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :module_enabled
before_action :merge_request, only: [
:edit, :update, :show, :diffs, :commits, :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]
before_action :validates_merge_request, only: [:show, :diffs, :commits]
@@ -149,15 +149,34 @@ 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_be_merged_by?(current_user)
+
+ if @merge_request.merge_when_build_succeeds?
+ @merge_request.reset_merge_when_build_succeeds
+ SystemNoteService.cancel_merge_when_build_succeeds(merge_request, @project, @current_user)
+ end
+ 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.active?
+ MergeRequests::MergeWhenBuildSucceedsService.new(@project,
+ current_user,
+ merge_params: 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
@@ -282,6 +301,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
diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb
index 13437b2483f..ebe4bace3b5 100644
--- a/app/models/ci/commit.rb
+++ b/app/models/ci/commit.rb
@@ -164,6 +164,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
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 0b73ab6d2eb..b1049fab788 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -1,3 +1,32 @@
+# == Schema Information
+#
+# 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
self.table_name = 'ci_builds'
@@ -46,6 +75,10 @@ class CommitStatus < ActiveRecord::Base
build.update_attributes finished_at: Time.now
end
+ after_transition 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'
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 85f37e49e62..c5f04dbcf4c 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -34,9 +34,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
@@ -385,6 +388,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
diff --git a/app/services/merge_requests/merge_when_build_succeeds_service.rb b/app/services/merge_requests/merge_when_build_succeeds_service.rb
new file mode 100644
index 00000000000..a4418360b8c
--- /dev/null
+++ b/app/services/merge_requests/merge_when_build_succeeds_service.rb
@@ -0,0 +1,43 @@
+module MergeRequests
+ class MergeWhenBuildSucceedsService < MergeRequests::BaseService
+ def execute(merge_request)
+ merge_request.merge_params.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
+ end
+
+ merge_request.save
+
+ unless already_approved
+ SystemNoteService.merge_when_build_succeeds(merge_request, @project, @current_user)
+ end
+ end
+
+ def trigger(build)
+ merge_requests = merge_request_from(build)
+
+ merge_requests.each do |merge_request|
+ next unless merge_request.merge_when_build_succeeds?
+
+ ci_commit = merge_request.ci_commit
+ if ci_commit && ci_commit.success? && merge_request.mergeable?
+ MergeWorker.perform_async(merge_request.id, merge_request.merge_user_id, merge_request.merge_params)
+ end
+ 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
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index d68bc79ecc0..335ef32abce 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -8,6 +8,7 @@ module MergeRequests
find_new_commits
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,12 @@ module MergeRequests
end
end
+ def reset_merge_when_build_succeeds
+ merge_requests_for_source_branch.each do |merge_request|
+ merge_request.reset_merge_when_build_succeeds
+ end
+ end
+
def find_new_commits
if branch_added?
@commits = []
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 708c2f00486..c9846e9f26f 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -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)
+ body = "Approved this request to be merged automatically when the build 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`
diff --git a/app/views/projects/merge_requests/cancel_merge_when_build_succeeds.js.haml b/app/views/projects/merge_requests/cancel_merge_when_build_succeeds.js.haml
new file mode 100644
index 00000000000..eab5be488b5
--- /dev/null
+++ b/app/views/projects/merge_requests/cancel_merge_when_build_succeeds.js.haml
@@ -0,0 +1,2 @@
+:plain
+ $('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/accept'))}");
diff --git a/app/views/projects/merge_requests/merge.js.haml b/app/views/projects/merge_requests/merge.js.haml
index 33321651e32..89aae17a606 100644
--- a/app/views/projects/merge_requests/merge.js.haml
+++ b/app/views/projects/merge_requests/merge.js.haml
@@ -1,6 +1,10 @@
-- if @status
+- case @status
+- when :success
:plain
merge_request_widget.mergeInProgress();
+- 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'))}");
diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml
index 0aad9bb3e88..e0013fb769a 100644
--- a/app/views/projects/merge_requests/widget/_open.html.haml
+++ b/app/views/projects/merge_requests/widget/_open.html.haml
@@ -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?
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 613525437ab..2afc5f81251 100644
--- a/app/views/projects/merge_requests/widget/open/_accept.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml
@@ -2,8 +2,15 @@
= 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" do
- Accept Merge Request
+ - ci_commit = @merge_request.ci_commit
+ - if ci_commit && ci_commit.active?
+ = f.button class: "btn btn-create btn-grouped merge_when_build_succeeds", name: "merge_when_build_succeeds" do
+ Merge when Build Succeeds
+ = f.button class: "btn btn-warning btn-grouped accept_merge_request" do
+ Accept Merge Request Now
+ - else
+ = f.button class: "btn btn-create btn-grouped accept_merge_request" 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
@@ -19,7 +26,8 @@
rows: 14, hint: true
:coffeescript
- $('.accept-mr-form').on 'ajax:before', ->
- btn = $('.accept_merge_request')
- btn.disable()
+ $('.accept_merge_request').on 'click', ->
+ btn = $(this)
btn.html("<i class='fa fa-spinner fa-spin'></i> Merge in progress")
+ $('.accept-mr-form').on 'ajax:sen', ->
+ $(".accept-mr-form :input").disable()
diff --git a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
new file mode 100644
index 00000000000..7e5385cf8b9
--- /dev/null
+++ b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
@@ -0,0 +1,27 @@
+%h4
+ Approved by #{link_to_member(@project, @merge_request.merge_user, avatar: true)}
+ to be merged automatically when
+ #{link_to "the build", ci_status_path(@merge_request.ci_commit)} succeeds
+%div
+ - if @merge_request.merge_params["should_remove_source_branch"]
+ = succeed '.' do
+ The changes will be merged into
+ %span.label-branch= @merge_request.target_branch
+ The source branch will be removed.
+ - elsif can_remove_branch?(@merge_request.source_project, @merge_request.source_branch)
+ - remove_source_branch_button = true
+ %p
+ = succeed '.' do
+ The changes will be merged into
+ %span.label-branch= @merge_request.target_branch
+ The source branch will not be removed.
+
+- if remove_source_branch_button || @merge_request.can_be_merged_by?(current_user)
+ .clearfix.prepend-top-10
+ - if remove_source_branch_button
+ = link_to merge_namespace_project_merge_request_path(@merge_request.source_project.namespace, @merge_request.source_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 @merge_request.can_be_merged_by?(current_user)
+ = link_to merge_namespace_project_merge_request_path(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request), remote: true, method: :delete, class: "btn btn-grouped btn-warning btn-sm" do
+ Cancel Automatic Merge
diff --git a/config/routes.rb b/config/routes.rb
index 0458f538eb6..917c3d3f1ed 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -556,6 +556,7 @@ Gitlab::Application.routes.draw do
get :diffs
get :commits
post :merge
+ delete :merge, action: :cancel_merge_when_build_succeeds
get :merge_check
get :ci_status
post :toggle_subscription
diff --git a/db/migrate/20151028152939_add_merge_when_build_succeeds_to_merge_request.rb b/db/migrate/20151028152939_add_merge_when_build_succeeds_to_merge_request.rb
new file mode 100644
index 00000000000..ceb52f0c222
--- /dev/null
+++ b/db/migrate/20151028152939_add_merge_when_build_succeeds_to_merge_request.rb
@@ -0,0 +1,7 @@
+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
diff --git a/db/schema.rb b/db/schema.rb
index 4bde9f0b748..825d95565be 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20151026182941) do
+ActiveRecord::Schema.define(version: 20151028152939) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -418,6 +418,7 @@ ActiveRecord::Schema.define(version: 20151026182941) do
end
add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree
+ add_index "labels", ["title"], name: "index_labels_on_title", using: :btree
create_table "members", force: true do |t|
t.integer "access_level", null: false
@@ -453,9 +454,9 @@ ActiveRecord::Schema.define(version: 20151026182941) 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: true 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"
@@ -464,13 +465,16 @@ ActiveRecord::Schema.define(version: 20151026182941) 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
@@ -499,6 +503,7 @@ ActiveRecord::Schema.define(version: 20151026182941) do
add_index "milestones", ["due_date"], name: "index_milestones_on_due_date", using: :btree
add_index "milestones", ["project_id", "iid"], name: "index_milestones_on_project_id_and_iid", unique: true, using: :btree
add_index "milestones", ["project_id"], name: "index_milestones_on_project_id", using: :btree
+ add_index "milestones", ["title"], name: "index_milestones_on_title", using: :btree
create_table "namespaces", force: true do |t|
t.string "name", null: false
--
GitLab