From 77f8a1e392b64f51326df8aebdc77e97af07bfed Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Mon, 2 Nov 2015 17:27:38 +0100 Subject: Merge when build succeeds --- CHANGELOG | 1 + app/assets/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 ++++++++++++++++++++++ app/services/merge_requests/refresh_service.rb | 8 +++- app/services/system_note_service.rb | 14 +++++++ .../cancel_merge_when_build_succeeds.js.haml | 2 + app/views/projects/merge_requests/merge.js.haml | 6 ++- .../projects/merge_requests/widget/_open.html.haml | 2 + .../merge_requests/widget/open/_accept.html.haml | 18 ++++++--- .../open/_merge_when_build_succeeds.html.haml | 27 ++++++++++++++ config/routes.rb | 1 + ...d_merge_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(" 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 -- cgit v1.2.1