summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinya Maeda <shinya@gitlab.com>2019-05-22 18:45:27 +0700
committerShinya Maeda <shinya@gitlab.com>2019-06-03 13:15:29 +0700
commitd4b46936633a3b2a0248b4572b4a1dc7b2ba8531 (patch)
treebf5c1cf5a8ebf1568ca62576d63ff793ce29764f
parent96744d0befd7e298c08e6d20a4f504086a717c35 (diff)
downloadgitlab-ce-d4b46936633a3b2a0248b4572b4a1dc7b2ba8531.tar.gz
Abstract auto merge processes
We have one auto merge strategy today - Merge When Pipeline Succeeds. In order to add more strategies for Merge Train feature, we abstract the architecture to be more extensible. Removed arguments Fix spec
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.vue4
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue16
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js2
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js9
-rw-r--r--app/controllers/projects/merge_requests_controller.rb42
-rw-r--r--app/helpers/merge_requests_helper.rb2
-rw-r--r--app/models/merge_request.rb24
-rw-r--r--app/presenters/merge_request_presenter.rb6
-rw-r--r--app/serializers/merge_request_widget_entity.rb10
-rw-r--r--app/services/auto_merge/merge_when_pipeline_succeeds_service.rb54
-rw-r--r--app/services/auto_merge_service.rb50
-rw-r--r--app/services/merge_requests/merge_when_pipeline_succeeds_service.rb47
-rw-r--r--app/services/merge_requests/refresh_service.rb8
-rw-r--r--app/services/merge_requests/update_service.rb2
-rw-r--r--app/services/notification_service.rb2
-rw-r--r--app/workers/pipeline_success_worker.rb7
-rw-r--r--changelogs/unreleased/abstract-auto-merge.yml5
-rw-r--r--config/routes/project.rb2
-rw-r--r--lib/api/merge_requests.rb11
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb18
-rw-r--r--spec/factories/merge_requests.rb3
-rw-r--r--spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb15
-rw-r--r--spec/features/merge_request/user_sees_merge_widget_spec.rb3
-rw-r--r--spec/frontend/vue_mr_widget/stores/get_state_key_spec.js2
-rw-r--r--spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js14
-rw-r--r--spec/models/merge_request_spec.rb18
-rw-r--r--spec/presenters/merge_request_presenter_spec.rb10
-rw-r--r--spec/requests/api/merge_requests_spec.rb6
-rw-r--r--spec/serializers/merge_request_widget_entity_spec.rb46
-rw-r--r--spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb (renamed from spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb)48
-rw-r--r--spec/services/auto_merge_service_spec.rb140
-rw-r--r--spec/services/merge_requests/push_options_handler_service_spec.rb5
-rw-r--r--spec/services/merge_requests/refresh_service_spec.rb10
-rw-r--r--spec/services/merge_requests/update_service_spec.rb3
-rw-r--r--spec/workers/pipeline_success_worker_spec.rb9
35 files changed, 478 insertions, 175 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.vue
index 1b3af2fccf2..88e1ccbaf35 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.vue
@@ -57,7 +57,7 @@ export default {
removeSourceBranch() {
const options = {
sha: this.mr.sha,
- merge_when_pipeline_succeeds: true,
+ auto_merge_strategy: 'merge_when_pipeline_succeeds',
should_remove_source_branch: true,
};
@@ -85,7 +85,7 @@ export default {
<h4 class="d-flex align-items-start">
<span class="append-right-10">
{{ s__('mrWidget|Set by') }}
- <mr-widget-author :author="mr.setToMWPSBy" />
+ <mr-widget-author :author="mr.setToAutoMergeBy" />
{{ s__('mrWidget|to be merged automatically when the pipeline succeeds') }}
</span>
<a
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
index 851939d5d4e..c7915f3918b 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue
@@ -31,7 +31,7 @@ export default {
return {
removeSourceBranch: this.mr.shouldRemoveSourceBranch,
mergeWhenBuildSucceeds: false,
- setToMergeWhenPipelineSucceeds: false,
+ autoMergeStrategy: null,
isMakingRequest: false,
isMergingImmediately: false,
commitMessage: this.mr.commitMessage,
@@ -42,7 +42,7 @@ export default {
};
},
computed: {
- shouldShowMergeWhenPipelineSucceedsText() {
+ shouldShowAutoMergeText() {
return this.mr.isPipelineActive;
},
status() {
@@ -87,7 +87,7 @@ export default {
mergeButtonText() {
if (this.isMergingImmediately) {
return __('Merge in progress');
- } else if (this.shouldShowMergeWhenPipelineSucceedsText) {
+ } else if (this.shouldShowAutoMergeText) {
return __('Merge when pipeline succeeds');
}
@@ -104,7 +104,7 @@ export default {
return enableSquashBeforeMerge && commitsCount > 1;
},
shouldShowMergeControls() {
- return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText;
+ return this.mr.isMergeAllowed || this.shouldShowAutoMergeText;
},
shouldShowSquashEdit() {
return this.squashBeforeMerge && this.shouldShowSquashBeforeMerge;
@@ -126,12 +126,16 @@ export default {
this.isMergingImmediately = true;
}
- this.setToMergeWhenPipelineSucceeds = mergeWhenBuildSucceeds === true;
+ if (mergeWhenBuildSucceeds === true) {
+ this.autoMergeStrategy = 'merge_when_pipeline_succeeds'
+ } else {
+ this.autoMergeStrategy = null
+ }
const options = {
sha: this.mr.sha,
commit_message: this.commitMessage,
- merge_when_pipeline_succeeds: this.setToMergeWhenPipelineSucceeds,
+ auto_merge_strategy: this.autoMergeStrategy,
should_remove_source_branch: this.removeSourceBranch === true,
squash: this.squashBeforeMerge,
squash_commit_message: this.squashCommitMessage,
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js
index 0cc4fd59f5e..f56875acfe9 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js
@@ -23,7 +23,7 @@ export default function deviseState(data) {
return stateKey.pipelineBlocked;
} else if (this.isSHAMismatch) {
return stateKey.shaMismatch;
- } else if (this.mergeWhenPipelineSucceeds) {
+ } else if (this.autoMergeEnabled) {
return this.mergeError ? stateKey.autoMergeFailed : stateKey.mergeWhenPipelineSucceeds;
} else if (!this.canMerge) {
return stateKey.notAllowedToMerge;
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index 45708d78886..1e5727d6132 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -61,7 +61,7 @@ export default class MergeRequestStore {
this.updatedAt = data.updated_at;
this.metrics = MergeRequestStore.buildMetrics(data.metrics);
- this.setToMWPSBy = MergeRequestStore.formatUserObject(data.merge_user || {});
+ this.setToAutoMergeBy = MergeRequestStore.formatUserObject(data.merge_user || {});
this.mergeUserId = data.merge_user_id;
this.currentUserId = gon.current_user_id;
this.sourceBranchPath = data.source_branch_path;
@@ -70,12 +70,13 @@ export default class MergeRequestStore {
this.targetBranchPath = data.target_branch_commits_path;
this.targetBranchTreePath = data.target_branch_tree_path;
this.conflictResolutionPath = data.conflict_resolution_path;
- this.cancelAutoMergePath = data.cancel_merge_when_pipeline_succeeds_path;
+ this.cancelAutoMergePath = data.cancel_auto_merge_path;
this.removeWIPPath = data.remove_wip_path;
this.sourceBranchRemoved = !data.source_branch_exists;
this.shouldRemoveSourceBranch = data.remove_source_branch || false;
this.onlyAllowMergeIfPipelineSucceeds = data.only_allow_merge_if_pipeline_succeeds || false;
- this.mergeWhenPipelineSucceeds = data.merge_when_pipeline_succeeds || false;
+ this.autoMergeEnabled = data.auto_merge_enabled || false;
+ this.autoMergeStrategy = data.auto_merge_strategy;
this.mergePath = data.merge_path;
this.ffOnlyEnabled = data.ff_only_enabled;
this.shouldBeRebased = !!data.should_be_rebased;
@@ -93,7 +94,7 @@ export default class MergeRequestStore {
this.canRemoveSourceBranch = currentUser.can_remove_source_branch || false;
this.canMerge = !!data.merge_path;
this.canCreateIssue = currentUser.can_create_issue || false;
- this.canCancelAutomaticMerge = !!data.cancel_merge_when_pipeline_succeeds_path;
+ this.canCancelAutomaticMerge = !!data.cancel_auto_merge_path;
this.isSHAMismatch = this.sha !== data.diff_head_sha;
this.canBeMerged = data.can_be_merged || false;
this.isMergeAllowed = data.mergeable || false;
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 8f177895b08..a3ceb76845e 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -145,14 +145,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
render partial: 'projects/merge_requests/widget/commit_change_content', layout: false
end
- def cancel_merge_when_pipeline_succeeds
- unless @merge_request.can_cancel_merge_when_pipeline_succeeds?(current_user)
+ def cancel_auto_merge
+ unless @merge_request.can_cancel_auto_merge?(current_user)
return access_denied!
end
- ::MergeRequests::MergeWhenPipelineSucceedsService
- .new(@project, current_user)
- .cancel(@merge_request)
+ AutoMergeService.new(project, current_user).cancel(@merge_request)
render json: serialize_widget(@merge_request)
end
@@ -229,12 +227,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end
def merge_params_attributes
- [:should_remove_source_branch, :commit_message, :squash_commit_message, :squash]
+ [:should_remove_source_branch, :commit_message, :squash_commit_message, :squash, :auto_merge_strategy]
end
- def merge_when_pipeline_succeeds_active?
- params[:merge_when_pipeline_succeeds].present? &&
- @merge_request.head_pipeline && @merge_request.head_pipeline.active?
+ def auto_merge_requested?
+ # Support params[:merge_when_pipeline_succeeds] during the transition period
+ params[:auto_merge_strategy].present? || params[:merge_when_pipeline_succeeds].present?
end
def close_merge_request_if_no_source_project
@@ -258,9 +256,9 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end
def merge!
- # Disable the CI check if merge_when_pipeline_succeeds is enabled since we have
+ # Disable the CI check if auto_merge_strategy is specified since we have
# to wait until CI completes to know
- unless @merge_request.mergeable?(skip_ci_check: merge_when_pipeline_succeeds_active?)
+ unless @merge_request.mergeable?(skip_ci_check: auto_merge_requested?)
return :failed
end
@@ -274,24 +272,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@merge_request.update(merge_error: nil, squash: merge_params.fetch(:squash, false))
- if params[:merge_when_pipeline_succeeds].present?
- return :failed unless @merge_request.actual_head_pipeline
-
- if @merge_request.actual_head_pipeline.active?
- ::MergeRequests::MergeWhenPipelineSucceedsService
- .new(@project, current_user, merge_params)
- .execute(@merge_request)
-
- :merge_when_pipeline_succeeds
- elsif @merge_request.actual_head_pipeline.success?
- # This can be triggered when a user clicks the auto merge button while
- # the tests finish at about the same time
- @merge_request.merge_async(current_user.id, merge_params)
-
- :success
- else
- :failed
- end
+ if auto_merge_requested?
+ AutoMergeService.new(project, current_user, merge_params)
+ .execute(merge_request,
+ params[:auto_merge_strategy] || AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
else
@merge_request.merge_async(current_user.id, merge_params)
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 991ca42c445..2de4e92e33e 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -103,7 +103,7 @@ module MergeRequestsHelper
def merge_params(merge_request)
{
- merge_when_pipeline_succeeds: true,
+ auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS,
should_remove_source_branch: true,
sha: merge_request.diff_head_sha,
squash: merge_request.squash
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 311ba1ce6bd..2602738901b 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -165,7 +165,7 @@ class MergeRequest < ApplicationRecord
validates :source_branch, presence: true
validates :target_project, presence: true
validates :target_branch, presence: true
- validates :merge_user, presence: true, if: :merge_when_pipeline_succeeds?, unless: :importing?
+ validates :merge_user, presence: true, if: :auto_merge_enabled?, unless: :importing?
validate :validate_branches, unless: [:allow_broken, :importing?, :closed_without_fork?]
validate :validate_fork, unless: :closed_without_fork?
validate :validate_target_project, on: :create
@@ -196,6 +196,7 @@ class MergeRequest < ApplicationRecord
alias_attribute :project, :target_project
alias_attribute :project_id, :target_project_id
+ alias_attribute :auto_merge_enabled, :merge_when_pipeline_succeeds
def self.reference_prefix
'!'
@@ -391,7 +392,7 @@ class MergeRequest < ApplicationRecord
def merge_participants
participants = [author]
- if merge_when_pipeline_succeeds? && !participants.include?(merge_user)
+ if auto_merge_enabled? && !participants.include?(merge_user)
participants << merge_user
end
@@ -782,7 +783,7 @@ class MergeRequest < ApplicationRecord
project.ff_merge_must_be_possible? && !ff_merge_possible?
end
- def can_cancel_merge_when_pipeline_succeeds?(current_user)
+ def can_cancel_auto_merge?(current_user)
can_be_merged_by?(current_user) || self.author == current_user
end
@@ -801,6 +802,16 @@ class MergeRequest < ApplicationRecord
Gitlab::Utils.to_boolean(merge_params['force_remove_source_branch'])
end
+ def auto_merge_strategy
+ return unless auto_merge_enabled?
+
+ merge_params['auto_merge_strategy'] || AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS
+ end
+
+ def auto_merge_strategy=(strategy)
+ merge_params['auto_merge_strategy'] = strategy
+ end
+
def remove_source_branch?
should_remove_source_branch? || force_remove_source_branch?
end
@@ -973,15 +984,16 @@ class MergeRequest < ApplicationRecord
end
end
- def reset_merge_when_pipeline_succeeds
- return unless merge_when_pipeline_succeeds?
+ def reset_auto_merge
+ return unless auto_merge_enabled?
- self.merge_when_pipeline_succeeds = false
+ self.auto_merge_enabled = false
self.merge_user = nil
if merge_params
merge_params.delete('should_remove_source_branch')
merge_params.delete('commit_message')
merge_params.delete('squash_commit_message')
+ merge_params.delete('auto_merge_strategy')
end
self.save
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index ba0711ca867..9c44ed711a6 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -22,9 +22,9 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end
end
- def cancel_merge_when_pipeline_succeeds_path
- if can_cancel_merge_when_pipeline_succeeds?(current_user)
- cancel_merge_when_pipeline_succeeds_project_merge_request_path(project, merge_request)
+ def cancel_auto_merge_path
+ if can_cancel_auto_merge?(current_user)
+ cancel_auto_merge_project_merge_request_path(project, merge_request)
end
end
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index b130f447cce..a428930dbbf 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -9,7 +9,11 @@ class MergeRequestWidgetEntity < IssuableEntity
expose :merge_params
expose :merge_status
expose :merge_user_id
- expose :merge_when_pipeline_succeeds
+ expose :auto_merge_enabled
+ expose :auto_merge_strategy
+ expose :available_auto_merge_strategies do |merge_request|
+ AutoMergeService.new(merge_request.project, current_user).available_strategies(merge_request) # rubocop: disable CodeReuse/ServiceClass
+ end
expose :source_branch
expose :source_branch_protected do |merge_request|
merge_request.source_project.present? && ProtectedBranch.protected?(merge_request.source_project, merge_request.source_branch)
@@ -182,8 +186,8 @@ class MergeRequestWidgetEntity < IssuableEntity
presenter(merge_request).remove_wip_path
end
- expose :cancel_merge_when_pipeline_succeeds_path do |merge_request|
- presenter(merge_request).cancel_merge_when_pipeline_succeeds_path
+ expose :cancel_auto_merge_path do |merge_request|
+ presenter(merge_request).cancel_auto_merge_path
end
expose :create_issue_to_resolve_discussions_path do |merge_request|
diff --git a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb
new file mode 100644
index 00000000000..d0586468859
--- /dev/null
+++ b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+module AutoMerge
+ class MergeWhenPipelineSucceedsService < BaseService
+ def execute(merge_request)
+ return :failed unless merge_request.actual_head_pipeline
+
+ if merge_request.actual_head_pipeline.active?
+ merge_request.merge_params.merge!(params)
+
+ unless merge_request.auto_merge_enabled?
+ merge_request.auto_merge_enabled = true
+ merge_request.merge_user = @current_user
+ merge_request.auto_merge_strategy = AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS
+
+ SystemNoteService.merge_when_pipeline_succeeds(merge_request, @project, @current_user, merge_request.diff_head_commit)
+ end
+
+ return :failed unless merge_request.save
+
+ :merge_when_pipeline_succeeds
+ elsif merge_request.actual_head_pipeline.success?
+ # This can be triggered when a user clicks the auto merge button while
+ # the tests finish at about the same time
+ merge_request.merge_async(current_user.id, merge_params)
+
+ :success
+ else
+ :failed
+ end
+ end
+
+ def process(merge_request)
+ return unless merge_request.actual_head_pipeline&.success?
+ return unless merge_request.mergeable?
+
+ merge_request.merge_async(merge_request.merge_user_id, merge_request.merge_params)
+ end
+
+ def cancel(merge_request)
+ if merge_request.reset_auto_merge
+ SystemNoteService.cancel_merge_when_pipeline_succeeds(merge_request, @project, @current_user)
+
+ success
+ else
+ error("Can't cancel the automatic merge", 406)
+ end
+ end
+
+ def available_for?(merge_request)
+ merge_request.actual_head_pipeline&.active?
+ end
+ end
+end
diff --git a/app/services/auto_merge_service.rb b/app/services/auto_merge_service.rb
new file mode 100644
index 00000000000..a3a780ff388
--- /dev/null
+++ b/app/services/auto_merge_service.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+class AutoMergeService < BaseService
+ STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS = 'merge_when_pipeline_succeeds'.freeze
+ STRATEGIES = [STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS].freeze
+
+ class << self
+ def all_strategies
+ STRATEGIES
+ end
+
+ def get_service_class(strategy)
+ return unless all_strategies.include?(strategy)
+
+ "::AutoMerge::#{strategy.camelize}Service".constantize
+ end
+ end
+
+ def execute(merge_request, strategy)
+ service = get_service_instance(strategy)
+
+ return :failed unless service&.available_for?(merge_request)
+
+ service.execute(merge_request)
+ end
+
+ def process(merge_request)
+ return unless merge_request.auto_merge_enabled?
+
+ get_service_instance(merge_request.auto_merge_strategy).process(merge_request)
+ end
+
+ def cancel(merge_request)
+ return error("Can't cancel the automatic merge", 406) unless merge_request.auto_merge_enabled?
+
+ get_service_instance(merge_request.auto_merge_strategy).cancel(merge_request)
+ end
+
+ def available_strategies(merge_request)
+ self.class.all_strategies.select do |strategy|
+ get_service_instance(strategy).available_for?(merge_request)
+ end
+ end
+
+ private
+
+ def get_service_instance(strategy)
+ self.class.get_service_class(strategy)&.new(project, current_user, params)
+ end
+end
diff --git a/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb b/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
deleted file mode 100644
index 973e5b64e88..00000000000
--- a/app/services/merge_requests/merge_when_pipeline_succeeds_service.rb
+++ /dev/null
@@ -1,47 +0,0 @@
-# frozen_string_literal: true
-
-module MergeRequests
- class MergeWhenPipelineSucceedsService < MergeRequests::BaseService
- # Marks the passed `merge_request` to be merged when the pipeline 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_pipeline_succeeds?
-
- unless already_approved
- merge_request.merge_when_pipeline_succeeds = true
- merge_request.merge_user = @current_user
-
- SystemNoteService.merge_when_pipeline_succeeds(merge_request, @project, @current_user, merge_request.diff_head_commit)
- end
-
- merge_request.save
- end
-
- # Triggers the automatic merge of merge_request once the pipeline succeeds
- def trigger(pipeline)
- return unless pipeline.success?
-
- pipeline_merge_requests(pipeline) do |merge_request|
- next unless merge_request.merge_when_pipeline_succeeds?
- next unless merge_request.mergeable?
-
- merge_request.merge_async(merge_request.merge_user_id, merge_request.merge_params)
- end
- end
-
- # Cancels the automatic merge
- def cancel(merge_request)
- if merge_request.merge_when_pipeline_succeeds? && merge_request.open?
- merge_request.reset_merge_when_pipeline_succeeds
- SystemNoteService.cancel_merge_when_pipeline_succeeds(merge_request, @project, @current_user)
-
- success
- else
- error("Can't cancel the automatic merge", 406)
- end
- end
- end
-end
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 3abea1ad1ae..08130a531ee 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -24,7 +24,7 @@ module MergeRequests
reload_merge_requests
outdate_suggestions
refresh_pipelines_on_merge_requests
- reset_merge_when_pipeline_succeeds
+ cancel_auto_merge
mark_pending_todos_done
cache_merge_requests_closing_issues
@@ -142,8 +142,10 @@ module MergeRequests
end
end
- def reset_merge_when_pipeline_succeeds
- merge_requests_for_source_branch.each(&:reset_merge_when_pipeline_succeeds)
+ def cancel_auto_merge
+ merge_requests_for_source_branch.each do |merge_request|
+ AutoMergeService.new(project, current_user).cancel(merge_request)
+ end
end
def mark_pending_todos_done
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index 55546432ce4..6a0f3000ffb 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -89,7 +89,7 @@ module MergeRequests
merge_request.update(merge_error: nil)
if merge_request.head_pipeline && merge_request.head_pipeline.active?
- MergeRequests::MergeWhenPipelineSucceedsService.new(project, current_user).execute(merge_request)
+ AutoMergeService.new(project, current_user).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
else
merge_request.merge_async(current_user.id, {})
end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index f797c0f11c6..5aa804666f0 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -238,7 +238,7 @@ class NotificationService
merge_request,
current_user,
:merged_merge_request_email,
- skip_current_user: !merge_request.merge_when_pipeline_succeeds?
+ skip_current_user: !merge_request.auto_merge_enabled?
)
end
diff --git a/app/workers/pipeline_success_worker.rb b/app/workers/pipeline_success_worker.rb
index 4f349ed922c..ce6c88c85c1 100644
--- a/app/workers/pipeline_success_worker.rb
+++ b/app/workers/pipeline_success_worker.rb
@@ -9,9 +9,10 @@ class PipelineSuccessWorker
# rubocop: disable CodeReuse/ActiveRecord
def perform(pipeline_id)
Ci::Pipeline.find_by(id: pipeline_id).try do |pipeline|
- MergeRequests::MergeWhenPipelineSucceedsService
- .new(pipeline.project, nil)
- .trigger(pipeline)
+ pipeline.all_merge_requests.preload(:merge_user).each do |merge_request|
+ AutoMergeService.new(pipeline.project, merge_request.merge_user)
+ .process(merge_request)
+ end
end
end
# rubocop: enable CodeReuse/ActiveRecord
diff --git a/changelogs/unreleased/abstract-auto-merge.yml b/changelogs/unreleased/abstract-auto-merge.yml
new file mode 100644
index 00000000000..d3069a3e500
--- /dev/null
+++ b/changelogs/unreleased/abstract-auto-merge.yml
@@ -0,0 +1,5 @@
+---
+title: Refactor and abstract Auto Merge Processes
+merge_request: 28595
+author:
+type: other
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 840dc4c7844..d44ff62bc2a 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -208,7 +208,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
member do
get :commit_change_content
post :merge
- post :cancel_merge_when_pipeline_succeeds
+ post :cancel_auto_merge
get :pipeline_status
get :ci_environments_status
post :toggle_subscription
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index ce85772e4ed..955624404f1 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -386,9 +386,8 @@ module API
)
if merge_when_pipeline_succeeds && merge_request.head_pipeline && merge_request.head_pipeline.active?
- ::MergeRequests::MergeWhenPipelineSucceedsService
- .new(merge_request.target_project, current_user, merge_params)
- .execute(merge_request)
+ AutoMergeService.new(merge_request.target_project, current_user, merge_params)
+ .execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
else
::MergeRequests::MergeService
.new(merge_request.target_project, current_user, merge_params)
@@ -429,11 +428,9 @@ module API
post ':id/merge_requests/:merge_request_iid/cancel_merge_when_pipeline_succeeds' do
merge_request = find_project_merge_request(params[:merge_request_iid])
- unauthorized! unless merge_request.can_cancel_merge_when_pipeline_succeeds?(current_user)
+ unauthorized! unless merge_request.can_cancel_auto_merge?(current_user)
- ::MergeRequests::MergeWhenPipelineSucceedsService
- .new(merge_request.target_project, current_user)
- .cancel(merge_request)
+ AutoMergeService.new(merge_request.target_project, current_user).cancel(merge_request)
end
desc 'Rebase the merge request against its target branch' do
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index f4a18dcba51..f8c0ab55eb4 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -429,8 +429,9 @@ describe Projects::MergeRequestsController do
it 'sets the MR to merge when the pipeline succeeds' do
service = double(:merge_when_pipeline_succeeds_service)
+ allow(service).to receive(:available_for?) { true }
- expect(MergeRequests::MergeWhenPipelineSucceedsService)
+ expect(AutoMerge::MergeWhenPipelineSucceedsService)
.to receive(:new).with(project, anything, anything)
.and_return(service)
expect(service).to receive(:execute).with(merge_request)
@@ -713,9 +714,9 @@ describe Projects::MergeRequestsController do
end
end
- describe 'POST cancel_merge_when_pipeline_succeeds' do
+ describe 'POST cancel_auto_merge' do
subject do
- post :cancel_merge_when_pipeline_succeeds,
+ post :cancel_auto_merge,
params: {
format: :json,
namespace_id: merge_request.project.namespace.to_param,
@@ -725,14 +726,15 @@ describe Projects::MergeRequestsController do
xhr: true
end
- it 'calls MergeRequests::MergeWhenPipelineSucceedsService' do
- mwps_service = double
+ it 'calls AutoMergeService' do
+ auto_merge_service = double
- allow(MergeRequests::MergeWhenPipelineSucceedsService)
+ allow(AutoMergeService)
.to receive(:new)
- .and_return(mwps_service)
+ .and_return(auto_merge_service)
- expect(mwps_service).to receive(:cancel).with(merge_request)
+ allow(auto_merge_service).to receive(:available_strategies).with(merge_request)
+ expect(auto_merge_service).to receive(:cancel).with(merge_request)
subject
end
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index e8df5094b83..0b6a43b13a9 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -95,7 +95,8 @@ FactoryBot.define do
end
trait :merge_when_pipeline_succeeds do
- merge_when_pipeline_succeeds true
+ auto_merge_enabled true
+ auto_merge_strategy AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS
merge_user { author }
end
diff --git a/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb
index d4ad11b3585..e7b92dc5535 100644
--- a/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb
+++ b/spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb
@@ -74,11 +74,12 @@ describe 'Merge request > User merges when pipeline succeeds', :js do
source_project: project,
title: 'Bug NS-04',
author: user,
- merge_user: user,
- merge_params: { force_remove_source_branch: '1' })
+ merge_user: user)
end
before do
+ merge_request.merge_params['force_remove_source_branch'] = '0'
+ merge_request.save!
click_link "Cancel automatic merge"
end
@@ -102,11 +103,11 @@ describe 'Merge request > User merges when pipeline succeeds', :js do
context 'when merge when pipeline succeeds is enabled' do
let(:merge_request) do
- create(:merge_request_with_diffs, :simple, source_project: project,
- author: user,
- merge_user: user,
- title: 'MepMep',
- merge_when_pipeline_succeeds: true)
+ create(:merge_request_with_diffs, :simple, :merge_when_pipeline_succeeds,
+ source_project: project,
+ author: user,
+ merge_user: user,
+ title: 'MepMep')
end
let!(:build) do
create(:ci_build, pipeline: pipeline)
diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb
index 93ddde623fe..1477307ed7b 100644
--- a/spec/features/merge_request/user_sees_merge_widget_spec.rb
+++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb
@@ -314,7 +314,8 @@ describe 'Merge request > User sees merge widget', :js do
context 'view merge request with MWPS enabled but automatically merge fails' do
before do
merge_request.update(
- merge_when_pipeline_succeeds: true,
+ auto_merge_enabled: true,
+ auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS,
merge_user: merge_request.author,
merge_error: 'Something went wrong'
)
diff --git a/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js b/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js
index b356ea85cad..9797549498b 100644
--- a/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js
+++ b/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js
@@ -31,7 +31,7 @@ describe('getStateKey', () => {
expect(bound()).toEqual('notAllowedToMerge');
- context.mergeWhenPipelineSucceeds = true;
+ context.autoMergeEnabled = true;
expect(bound()).toEqual('mergeWhenPipelineSucceeds');
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
index 368c997d318..daca51c6156 100644
--- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js
@@ -80,7 +80,7 @@ describe('ReadyToMerge', () => {
it('should have default data', () => {
expect(vm.mergeWhenBuildSucceeds).toBeFalsy();
expect(vm.useCommitMessageWithDescription).toBeFalsy();
- expect(vm.setToMergeWhenPipelineSucceeds).toBeFalsy();
+ expect(vm.setToAutoMerge).toBeFalsy();
expect(vm.showCommitMessageEditor).toBeFalsy();
expect(vm.isMakingRequest).toBeFalsy();
expect(vm.isMergingImmediately).toBeFalsy();
@@ -91,17 +91,17 @@ describe('ReadyToMerge', () => {
});
describe('computed', () => {
- describe('shouldShowMergeWhenPipelineSucceedsText', () => {
+ describe('shouldShowAutoMergeText', () => {
it('should return true with active pipeline', () => {
vm.mr.isPipelineActive = true;
- expect(vm.shouldShowMergeWhenPipelineSucceedsText).toBeTruthy();
+ expect(vm.shouldShowAutoMergeText).toBeTruthy();
});
it('should return false with inactive pipeline', () => {
vm.mr.isPipelineActive = false;
- expect(vm.shouldShowMergeWhenPipelineSucceedsText).toBeFalsy();
+ expect(vm.shouldShowAutoMergeText).toBeFalsy();
});
});
@@ -325,7 +325,7 @@ describe('ReadyToMerge', () => {
vm.handleMergeButtonClick(true);
setTimeout(() => {
- expect(vm.setToMergeWhenPipelineSucceeds).toBeTruthy();
+ expect(vm.setToAutoMerge).toBeTruthy();
expect(vm.isMakingRequest).toBeTruthy();
expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested');
@@ -345,7 +345,7 @@ describe('ReadyToMerge', () => {
vm.handleMergeButtonClick(false, true);
setTimeout(() => {
- expect(vm.setToMergeWhenPipelineSucceeds).toBeFalsy();
+ expect(vm.setToAutoMerge).toBeFalsy();
expect(vm.isMakingRequest).toBeTruthy();
expect(eventHub.$emit).toHaveBeenCalledWith('FailedToMerge', undefined);
@@ -363,7 +363,7 @@ describe('ReadyToMerge', () => {
vm.handleMergeButtonClick();
setTimeout(() => {
- expect(vm.setToMergeWhenPipelineSucceeds).toBeFalsy();
+ expect(vm.setToAutoMerge).toBeFalsy();
expect(vm.isMakingRequest).toBeTruthy();
expect(vm.initiateMergePolling).toHaveBeenCalled();
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index c72b6e9033d..43c61c48fe5 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1038,14 +1038,28 @@ describe MergeRequest do
end
end
- describe "#reset_merge_when_pipeline_succeeds" do
+ describe "#auto_merge_strategy" do
+ subject { merge_request.auto_merge_strategy }
+
+ let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) }
+
+ it { is_expected.to eq('merge_when_pipeline_succeeds') }
+
+ context 'when auto merge is disabled' do
+ let(:merge_request) { create(:merge_request) }
+
+ it { is_expected.to be_nil }
+ end
+ end
+
+ describe "#reset_auto_merge" do
let(:merge_if_green) do
create :merge_request, merge_when_pipeline_succeeds: true, merge_user: create(:user),
merge_params: { "should_remove_source_branch" => "1", "commit_message" => "msg" }
end
it "sets the item to false" do
- merge_if_green.reset_merge_when_pipeline_succeeds
+ merge_if_green.reset_auto_merge
merge_if_green.reload
expect(merge_if_green.merge_when_pipeline_succeeds).to be_falsey
diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb
index 0e1aed42cc5..6408b0bd748 100644
--- a/spec/presenters/merge_request_presenter_spec.rb
+++ b/spec/presenters/merge_request_presenter_spec.rb
@@ -207,25 +207,25 @@ describe MergeRequestPresenter do
end
end
- describe '#cancel_merge_when_pipeline_succeeds_path' do
+ describe '#cancel_auto_merge_path' do
subject do
described_class.new(resource, current_user: user)
- .cancel_merge_when_pipeline_succeeds_path
+ .cancel_auto_merge_path
end
context 'when can cancel mwps' do
it 'returns path' do
- allow(resource).to receive(:can_cancel_merge_when_pipeline_succeeds?)
+ allow(resource).to receive(:can_cancel_auto_merge?)
.with(user)
.and_return(true)
- is_expected.to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}/cancel_merge_when_pipeline_succeeds")
+ is_expected.to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}/cancel_auto_merge")
end
end
context 'when cannot cancel mwps' do
it 'returns nil' do
- allow(resource).to receive(:can_cancel_merge_when_pipeline_succeeds?)
+ allow(resource).to receive(:can_cancel_auto_merge?)
.with(user)
.and_return(false)
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 5c94a87529b..9f9180bc8c9 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -1473,7 +1473,7 @@ describe API::MergeRequests do
end
it "enables merge when pipeline succeeds if the pipeline is active" do
- allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(pipeline)
+ allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, actual_head_pipeline: pipeline)
allow(pipeline).to receive(:active?).and_return(true)
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge", user), params: { merge_when_pipeline_succeeds: true }
@@ -1484,7 +1484,7 @@ describe API::MergeRequests do
end
it "enables merge when pipeline succeeds if the pipeline is active and only_allow_merge_if_pipeline_succeeds is true" do
- allow_any_instance_of(MergeRequest).to receive(:head_pipeline).and_return(pipeline)
+ allow_any_instance_of(MergeRequest).to receive_messages(head_pipeline: pipeline, actual_head_pipeline: pipeline)
allow(pipeline).to receive(:active?).and_return(true)
project.update_attribute(:only_allow_merge_if_pipeline_succeeds, true)
@@ -1950,7 +1950,7 @@ describe API::MergeRequests do
describe 'POST :id/merge_requests/:merge_request_iid/cancel_merge_when_pipeline_succeeds' do
before do
- ::MergeRequests::MergeWhenPipelineSucceedsService.new(merge_request.target_project, user).execute(merge_request)
+ ::AutoMergeService.new(merge_request.target_project, user).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
end
it 'removes the merge_when_pipeline_succeeds status' do
diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index b89898f26f7..a27c22191f4 100644
--- a/spec/serializers/merge_request_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -297,4 +297,50 @@ describe MergeRequestWidgetEntity do
end
end
end
+
+ describe 'auto merge' do
+ context 'when auto merge is enabled' do
+ let(:resource) { create(:merge_request, :merge_when_pipeline_succeeds) }
+
+ it 'returns auto merge related information' do
+ expect(subject[:auto_merge_enabled]).to be_truthy
+ expect(subject[:auto_merge_strategy]).to eq('merge_when_pipeline_succeeds')
+ end
+ end
+
+ context 'when auto merge is not enabled' do
+ let(:resource) { create(:merge_request) }
+
+ it 'returns auto merge related information' do
+ expect(subject[:auto_merge_enabled]).to be_falsy
+ expect(subject[:auto_merge_strategy]).to be_nil
+ end
+ end
+
+ context 'when head pipeline is running' do
+ before do
+ create(:ci_pipeline, :running, project: project,
+ ref: resource.source_branch,
+ sha: resource.diff_head_sha)
+ resource.update_head_pipeline
+ end
+
+ it 'returns available auto merge strategies' do
+ expect(subject[:available_auto_merge_strategies]).to eq(%w[merge_when_pipeline_succeeds])
+ end
+ end
+
+ context 'when head pipeline is finished' do
+ before do
+ create(:ci_pipeline, :success, project: project,
+ ref: resource.source_branch,
+ sha: resource.diff_head_sha)
+ resource.update_head_pipeline
+ end
+
+ it 'returns available auto merge strategies' do
+ expect(subject[:available_auto_merge_strategies]).to be_empty
+ end
+ end
+ end
end
diff --git a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb
index 96f61f3f103..a20bf8e17e4 100644
--- a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
+++ b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-describe MergeRequests::MergeWhenPipelineSucceedsService do
+describe AutoMerge::MergeWhenPipelineSucceedsService do
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
@@ -21,6 +21,27 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
described_class.new(project, user, commit_message: 'Awesome message')
end
+ describe "#available_for?" do
+ subject { service.available_for?(mr_merge_if_green_enabled) }
+
+ let(:pipeline_status) { :running }
+
+ before do
+ create(:ci_pipeline, pipeline_status, ref: mr_merge_if_green_enabled.source_branch,
+ sha: mr_merge_if_green_enabled.diff_head_sha,
+ project: mr_merge_if_green_enabled.source_project)
+ mr_merge_if_green_enabled.update_head_pipeline
+ end
+
+ it { is_expected.to be_truthy }
+
+ context 'when the head piipeline succeeded' do
+ let(:pipeline_status) { :success }
+
+ it { is_expected.to be_falsy }
+ end
+ end
+
describe "#execute" do
let(:merge_request) do
create(:merge_request, target_project: project, source_project: project,
@@ -30,8 +51,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
context 'first time enabling' do
before do
allow(merge_request)
- .to receive(:head_pipeline)
- .and_return(pipeline)
+ .to receive_messages(head_pipeline: pipeline, actual_head_pipeline: pipeline)
service.execute(merge_request)
end
@@ -39,7 +59,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
it 'sets the params, merge_user, and flag' do
expect(merge_request).to be_valid
expect(merge_request.merge_when_pipeline_succeeds).to be_truthy
- expect(merge_request.merge_params).to eq commit_message: 'Awesome message'
+ expect(merge_request.merge_params).to include commit_message: 'Awesome message'
expect(merge_request.merge_user).to be user
end
@@ -54,8 +74,8 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
let(:build) { create(:ci_build, ref: mr_merge_if_green_enabled.source_branch) }
before do
- allow(mr_merge_if_green_enabled).to receive(:head_pipeline)
- .and_return(pipeline)
+ allow(mr_merge_if_green_enabled)
+ .to receive_messages(head_pipeline: pipeline, actual_head_pipeline: pipeline)
allow(mr_merge_if_green_enabled).to receive(:mergeable?)
.and_return(true)
@@ -72,7 +92,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
end
end
- describe "#trigger" do
+ describe "#process" do
let(:merge_request_ref) { mr_merge_if_green_enabled.source_branch }
let(:merge_request_head) do
project.commit(mr_merge_if_green_enabled.source_branch).id
@@ -86,8 +106,11 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
end
it "merges all merge requests with merge when the pipeline succeeds enabled" do
+ allow(mr_merge_if_green_enabled)
+ .to receive_messages(head_pipeline: triggering_pipeline, actual_head_pipeline: triggering_pipeline)
+
expect(MergeWorker).to receive(:perform_async)
- service.trigger(triggering_pipeline)
+ service.process(mr_merge_if_green_enabled)
end
end
@@ -99,7 +122,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
it 'does not merge request' do
expect(MergeWorker).not_to receive(:perform_async)
- service.trigger(old_pipeline)
+ service.process(mr_merge_if_green_enabled)
end
end
@@ -111,7 +134,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
it 'does not merge request' do
expect(MergeWorker).not_to receive(:perform_async)
- service.trigger(unrelated_pipeline)
+ service.process(mr_merge_if_green_enabled)
end
end
@@ -125,8 +148,11 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
end
it 'merges the associated merge request' do
+ allow(mr_merge_if_green_enabled)
+ .to receive_messages(head_pipeline: pipeline, actual_head_pipeline: pipeline)
+
expect(MergeWorker).to receive(:perform_async)
- service.trigger(pipeline)
+ service.process(mr_merge_if_green_enabled)
end
end
end
diff --git a/spec/services/auto_merge_service_spec.rb b/spec/services/auto_merge_service_spec.rb
new file mode 100644
index 00000000000..d0eefed3150
--- /dev/null
+++ b/spec/services/auto_merge_service_spec.rb
@@ -0,0 +1,140 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AutoMergeService do
+ set(:project) { create(:project) }
+ set(:user) { create(:user) }
+ let(:service) { described_class.new(project, user) }
+
+ describe '.all_strategies' do
+ subject { described_class.all_strategies }
+
+ it 'returns all strategies' do
+ is_expected.to eq(AutoMergeService::STRATEGIES)
+ end
+ end
+
+ describe '#available_strategies' do
+ subject { service.available_strategies(merge_request) }
+
+ let(:merge_request) { create(:merge_request) }
+ let(:pipeline_status) { :running }
+
+ before do
+ create(:ci_pipeline, pipeline_status, ref: merge_request.source_branch,
+ sha: merge_request.diff_head_sha,
+ project: merge_request.source_project)
+
+ merge_request.update_head_pipeline
+ end
+
+ it 'returns available strategies' do
+ is_expected.to include('merge_when_pipeline_succeeds')
+ end
+
+ context 'when the head piipeline succeeded' do
+ let(:pipeline_status) { :success }
+
+ it 'returns available strategies' do
+ is_expected.to be_empty
+ end
+ end
+ end
+
+ describe '.get_service_class' do
+ subject { described_class.get_service_class(strategy) }
+
+ let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS }
+
+ it 'returns service instance' do
+ is_expected.to eq(AutoMerge::MergeWhenPipelineSucceedsService)
+ end
+
+ context 'when strategy is not present' do
+ let(:strategy) { }
+
+ it 'returns nil' do
+ is_expected.to be_nil
+ end
+ end
+ end
+
+ describe '#execute' do
+ subject { service.execute(merge_request, strategy) }
+
+ let(:merge_request) { create(:merge_request) }
+ let(:pipeline_status) { :running }
+ let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS }
+
+ before do
+ create(:ci_pipeline, pipeline_status, ref: merge_request.source_branch,
+ sha: merge_request.diff_head_sha,
+ project: merge_request.source_project)
+
+ merge_request.update_head_pipeline
+ end
+
+ it 'delegates to a relevant service instance' do
+ expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service|
+ expect(service).to receive(:execute).with(merge_request)
+ end
+
+ subject
+ end
+
+ context 'when the head piipeline succeeded' do
+ let(:pipeline_status) { :success }
+
+ it 'returns failed' do
+ is_expected.to eq(:failed)
+ end
+ end
+ end
+
+ describe '#process' do
+ subject { service.process(merge_request) }
+
+ let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) }
+
+ it 'delegates to a relevant service instance' do
+ expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service|
+ expect(service).to receive(:process).with(merge_request)
+ end
+
+ subject
+ end
+
+ context 'when auto merge is not enabled' do
+ let(:merge_request) { create(:merge_request) }
+
+ it 'returns nil' do
+ is_expected.to be_nil
+ end
+ end
+ end
+
+ describe '#cancel' do
+ subject { service.cancel(merge_request) }
+
+ let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) }
+
+ it 'delegates to a relevant service instance' do
+ expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service|
+ expect(service).to receive(:cancel).with(merge_request)
+ end
+
+ subject
+ end
+
+ context 'when auto merge is not enabled' do
+ let(:merge_request) { create(:merge_request) }
+
+ it 'returns error' do
+ expect(subject[:message]).to eq("Can't cancel the automatic merge")
+ expect(subject[:status]).to eq(:error)
+ expect(subject[:http_status]).to eq(406)
+ end
+ end
+ end
+end
diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb
index f7a39bb42d5..54b9c6dae38 100644
--- a/spec/services/merge_requests/push_options_handler_service_spec.rb
+++ b/spec/services/merge_requests/push_options_handler_service_spec.rb
@@ -76,10 +76,11 @@ describe MergeRequests::PushOptionsHandlerService do
shared_examples_for 'a service that can set the merge request to merge when pipeline succeeds' do
subject(:last_mr) { MergeRequest.last }
- it 'sets merge_when_pipeline_succeeds' do
+ it 'sets auto_merge_enabled' do
service.execute
- expect(last_mr.merge_when_pipeline_succeeds).to eq(true)
+ expect(last_mr.auto_merge_enabled).to eq(true)
+ expect(last_mr.auto_merge_strategy).to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
end
it 'sets merge_user to the user' do
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 7258428589f..6ba67c7165c 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -23,7 +23,8 @@ describe MergeRequests::RefreshService do
source_branch: 'master',
target_branch: 'feature',
target_project: @project,
- merge_when_pipeline_succeeds: true,
+ auto_merge_enabled: true,
+ auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS,
merge_user: @user)
@another_merge_request = create(:merge_request,
@@ -31,7 +32,8 @@ describe MergeRequests::RefreshService do
source_branch: 'master',
target_branch: 'test',
target_project: @project,
- merge_when_pipeline_succeeds: true,
+ auto_merge_enabled: true,
+ auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS,
merge_user: @user)
@fork_merge_request = create(:merge_request,
@@ -83,7 +85,7 @@ describe MergeRequests::RefreshService do
expect(@merge_request.notes).not_to be_empty
expect(@merge_request).to be_open
- expect(@merge_request.merge_when_pipeline_succeeds).to be_falsey
+ expect(@merge_request.auto_merge_enabled).to be_falsey
expect(@merge_request.diff_head_sha).to eq(@newrev)
expect(@fork_merge_request).to be_open
expect(@fork_merge_request.notes).to be_empty
@@ -292,7 +294,7 @@ describe MergeRequests::RefreshService do
expect(@merge_request.notes).not_to be_empty
expect(@merge_request).to be_open
- expect(@merge_request.merge_when_pipeline_succeeds).to be_falsey
+ expect(@merge_request.auto_merge_enabled).to be_falsey
expect(@merge_request.diff_head_sha).to eq(@newrev)
expect(@fork_merge_request).to be_open
expect(@fork_merge_request.notes).to be_empty
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index ba4c9ce60f3..fbfcd95e204 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -217,8 +217,9 @@ describe MergeRequests::UpdateService, :mailer do
head_pipeline_of: merge_request
)
- expect(MergeRequests::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user)
+ expect(AutoMerge::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user, {})
.and_return(service_mock)
+ allow(service_mock).to receive(:available_for?) { true }
expect(service_mock).to receive(:execute).with(merge_request)
end
diff --git a/spec/workers/pipeline_success_worker_spec.rb b/spec/workers/pipeline_success_worker_spec.rb
index 4cbe384b47a..b511edfa620 100644
--- a/spec/workers/pipeline_success_worker_spec.rb
+++ b/spec/workers/pipeline_success_worker_spec.rb
@@ -5,12 +5,13 @@ require 'spec_helper'
describe PipelineSuccessWorker do
describe '#perform' do
context 'when pipeline exists' do
- let(:pipeline) { create(:ci_pipeline, status: 'success') }
+ let(:pipeline) { create(:ci_pipeline, status: 'success', ref: merge_request.source_branch, project: merge_request.source_project) }
+ let(:merge_request) { create(:merge_request) }
it 'performs "merge when pipeline succeeds"' do
- expect_any_instance_of(
- MergeRequests::MergeWhenPipelineSucceedsService
- ).to receive(:trigger)
+ expect_next_instance_of(AutoMergeService) do |auto_merge|
+ expect(auto_merge).to receive(:process)
+ end
described_class.new.perform(pipeline.id)
end