diff --git a/CHANGELOG b/CHANGELOG index 0d89fca9fc1283104d07915e0726f1f7e1047da4..195e53abd8b9ce529124064346238002b16aeb45 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 f0b3667accacf29df3f96e4bcdd98073348c7688..b5a8c89367871ab20fff0096b7a3d41d71bf021e 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 16c42386623347e43ec941e93b1ca138b6c7fa0d..2f9b8a25edf637889477adaff0be8ee7aaac37cd 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 13437b2483f1f6a6f9fef45227419c196bd28c75..ebe4bace3b59ecccf959e652e3a6478d2153a435 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 0b73ab6d2eb591fe4d030c98e5fbefb4a2f95f76..b1049fab788513af0d3eb37a6254cc7dd56447a9 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 85f37e49e625faed80368f481672782d36dd0b7a..c5f04dbcf4c53722a54c484c78d39c8e23a88475 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 0000000000000000000000000000000000000000..a4418360b8c336be7966840c4559c49d7eef9f13 --- /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 d68bc79ecc064fe5134aaf5def2ad1e07190df4e..335ef32abced5aa67354b0dd46e2773cfca35464 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 708c2f00486aef8ca0d54aacee44fcc0d166beab..c9846e9f26f7e134becb0fd8049c48913af2bda9 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 0000000000000000000000000000000000000000..eab5be488b5d32faed0037aa1b9004e5901dc88f --- /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 33321651e32fcf9ff2bb4b9cb43db4eacfa139b9..89aae17a606f7682a7d85fe4658ef22a934614d0 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 0aad9bb3e88e0507eb56af53618271f2e525c499..e0013fb769ae562365288872a6061cc2ff82a3f8 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 613525437abf42110df95c044c258b3480794819..2afc5f81251c0ca4edf7496dae9c978c1d612a42 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 0000000000000000000000000000000000000000..7e5385cf8b9a686a72ac8648039ea0475ee34784 --- /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 0458f538eb6f8b9ce50e08c2774ead42ec61f950..917c3d3f1edae0f29dc16e4d6331698e2d7d1adb 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 0000000000000000000000000000000000000000..ceb52f0c2224752e070b03387e7b962e39047766 --- /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 4bde9f0b7482d13c251bbabd24f46f8b9286e2ce..825d95565bec3b6ac12911ed111acae7d3ad6845 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