diff options
39 files changed, 727 insertions, 164 deletions
diff --git a/.gitignore b/.gitignore index f5b6427ca03..881b3fb81ac 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ public/assets/ public/uploads.* public/uploads/ shared/artifacts/ +TODO rails_best_practices_output.html /tags tmp/ diff --git a/CHANGELOG b/CHANGELOG index bbd752fb1d0..31833117e4b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ 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) - Fix API setting of 'public' attribute to false will make a project private (Stan Hu) - Handle and report SSL errors in Web hook test (Stan Hu) - Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera) diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index f21ad694d06..7785c95563f 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -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; } diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3f47f2ddb2c..71df622eb0e 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] @@ -150,15 +150,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 @@ -283,6 +297,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 971e899de84..0f74a7f0688 100644 --- a/app/models/ci/commit.rb +++ b/app/models/ci/commit.rb @@ -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 diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index e70f4d37184..ff619965a57 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -1,34 +1,30 @@ # == 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' diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 080b7f7fb88..60fd2b9a757 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -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 diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index d619b72e3c2..cabc3d8fabb 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -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 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..5cf7404a493 --- /dev/null +++ b/app/services/merge_requests/merge_when_build_succeeds_service.rb @@ -0,0 +1,55 @@ +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 diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index e180edb4bf3..b26c7513f5b 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -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 = [] diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 09c159510cd..6975b2ee55b 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, 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` diff --git a/app/views/projects/merge_requests/_merge_request.html.haml b/app/views/projects/merge_requests/_merge_request.html.haml index 1d4c9b66c42..f7f932bdf36 100644 --- a/app/views/projects/merge_requests/_merge_request.html.haml +++ b/app/views/projects/merge_requests/_merge_request.html.haml @@ -1,11 +1,10 @@ -- 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') 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 518ecb9f00f..92ce479d463 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(#{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'))}"); diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml index ba5ad22bca7..9513a18f105 100644 --- a/app/views/projects/merge_requests/widget/_heading.html.haml +++ b/app/views/projects/merge_requests/widget/_heading.html.haml @@ -1,13 +1,12 @@ -- ci_commit = @merge_request.ci_commit -- if ci_commit - - status = ci_commit.status +- if @merge_request.ci_commit + - status = @merge_request.ci_commit.status .mr-widget-heading .ci_widget{class: "ci-#{status}"} - = ci_status_icon(ci_commit) + = ci_status_icon(@merge_request.ci_commit) %span CI build #{status} for #{@merge_request.last_commit_short_sha}. %span.ci-coverage - = link_to "View build details", ci_status_path(ci_commit) + = link_to "View build details", ci_status_path(@merge_request.ci_commit) - elsif @merge_request.has_ci? - # Compatibility with old CI integrations (ex jenkins) when you request status from CI server via AJAX diff --git a/app/views/projects/merge_requests/widget/_merged.html.haml b/app/views/projects/merge_requests/widget/_merged.html.haml index 5c6fece8c5c..8c2b5366a06 100644 --- a/app/views/projects/merge_requests/widget/_merged.html.haml +++ b/app/views/projects/merge_requests/widget/_merged.html.haml @@ -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(); }); - - 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 9b31014b581..6b4395fe4dc 100644 --- a/app/views/projects/merge_requests/widget/open/_accept.html.haml +++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml @@ -1,28 +1,62 @@ -- status_class = @merge_request.ci_commit ? " ci-#{@merge_request.ci_commit.status}" : nil +- status_class = @merge_request.ci_commit ? "ci-#{@merge_request.ci_commit.status}" : nil = 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 @merge_request.ci_commit && @merge_request.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(); }); 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..9788d68b54e --- /dev/null +++ b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml @@ -0,0 +1,27 @@ +%h4 + Set 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 + - 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 diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index 5d1a8555b7d..c87c0a252b1 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -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 diff --git a/config/routes.rb b/config/routes.rb index 3b151891a6b..b1f34700c00 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -571,8 +571,9 @@ Rails.application.routes.draw do member do get :diffs get :commits - post :merge get :merge_check + post :merge + post :cancel_merge_when_build_succeeds get :ci_status post :toggle_subscription end 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 fb59e187625..94b87040d88 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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 diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 82f2cef969f..366a1f8abec 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -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 { diff --git a/doc/workflow/merge_requests/disable_merge_when_build_succeeds.png b/doc/workflow/merge_requests/disable_merge_when_build_succeeds.png Binary files differnew file mode 100644 index 00000000000..a45a4890b62 --- /dev/null +++ b/doc/workflow/merge_requests/disable_merge_when_build_succeeds.png diff --git a/doc/workflow/merge_requests/enable_merge_when_build_succeeds.png b/doc/workflow/merge_requests/enable_merge_when_build_succeeds.png Binary files differnew file mode 100644 index 00000000000..62a46c9508b --- /dev/null +++ b/doc/workflow/merge_requests/enable_merge_when_build_succeeds.png diff --git a/doc/workflow/merge_when_build_succeeds.md b/doc/workflow/merge_when_build_succeeds.md new file mode 100644 index 00000000000..9bf6ddcc569 --- /dev/null +++ b/doc/workflow/merge_when_build_succeeds.md @@ -0,0 +1,20 @@ +# Merge When Build Succeeds + +Select a Merge Request to be merged if the build succeeds so the user does not have to wait for the build to finish and revisit the Merge Request to merge it after the build is done. + +## Enabling for a Merge Request + +Given an active build for a Merge Request, thus pending or running, a `Merge When Build Succeeds` button will appear to any user which can merge it. Once clicked, it ensures this merge request is merged when the build is successful. +When clicking the button, the merge parameters are also saved to allow the merge user to edit the commit message and remove the source branch if he can remove that branch. + +When this feature is enabled, a message will appear to notify other users. Also a note is posted on the thread. + +![Enable Merge When Build Succceeds](merge_requests/enable_merge_when_build_succeeds.png) + +## Canceling + +The automatic merge can be disabled by clicking the `Cancel Automatic Merge` button, or when a new commit is added to the Merge Request. In the former case a note is posted. In the latter case a user able to merge can enable the feature again. + +![Disable the automatic merge](merge_requests/disable_merge_when_build_succeeds.png) + +A failed build does not reset the automatic build so a build can be retried. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 96b73df6af9..81bf7a8222b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -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 diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index e7c5f808aea..3c1c6bda260 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -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 # diff --git a/spec/factories/ci/commits.rb b/spec/factories/ci/commits.rb index 79e000b7ccb..70e3fa319c6 100644 --- a/spec/factories/ci/commits.rb +++ b/spec/factories/ci/commits.rb @@ -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 diff --git a/spec/factories/commit_statuses.rb b/spec/factories/commit_statuses.rb index 52de437052d..8898b71e2a3 100644 --- a/spec/factories/commit_statuses.rb +++ b/spec/factories/commit_statuses.rb @@ -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' diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 729a49c9f72..5b4d7f41bc4 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -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] diff --git a/spec/features/merge_requests/merge_when_build_succeeds_spec.rb b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb new file mode 100644 index 00000000000..ee2fb25e9e5 --- /dev/null +++ b/spec/features/merge_requests/merge_when_build_succeeds_spec.rb @@ -0,0 +1,91 @@ +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, 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 + merge_request.source_project.team << [user, :master] + merge_request.source_branch = "feature" + merge_request.target_branch = "master" + merge_request.save! + + + 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 diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 567c911425c..0adf02db695 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -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 diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index c6d3aef0af9..a91fa735321 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -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 diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index c0961ceb11e..b0b28512560 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -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 diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb new file mode 100644 index 00000000000..188fda6211f --- /dev/null +++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb @@ -0,0 +1,84 @@ +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 diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 7ee4488521d..9a8174f95fd 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -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 diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index a45130bd473..15173cee0a2 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -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') } |