Commit bc5dae9b authored by Patrick Bajao's avatar Patrick Bajao

Add new preparing state

This `preparing` state will be used later on to determine whether
a merge request is mergeable or not
(via `MergeRequest#mergeable?`).

We also won't check mergeability when the `merge_status` is set
to `preparing`.

We set the `merge_status` to `unchecked` in
`MergeRequests::AfterCreateService` (which is called in
`NewMergeRequestWorker`) so we can check for mergeability once
everything's prepared.

Right now, no merge requests will have their `merge_status` to
`preparing` to keep backwards compatibility and to not result to
incidents/downtimes on multi-stage deploy (e.g. canary).

The `MergeRequest#mark_as_preparing` will be later called in
`MergeRequests::CreateService`. This will trigger the behavior
that we want to achieve with the `preparing` state.
parent 48e6b011
...@@ -191,8 +191,12 @@ class MergeRequest < ApplicationRecord ...@@ -191,8 +191,12 @@ class MergeRequest < ApplicationRecord
end end
state_machine :merge_status, initial: :unchecked do state_machine :merge_status, initial: :unchecked do
event :mark_as_preparing do
transition unchecked: :preparing
end
event :mark_as_unchecked do event :mark_as_unchecked do
transition [:can_be_merged, :checking] => :unchecked transition [:preparing, :can_be_merged, :checking] => :unchecked
transition [:cannot_be_merged, :cannot_be_merged_rechecking] => :cannot_be_merged_recheck transition [:cannot_be_merged, :cannot_be_merged_rechecking] => :cannot_be_merged_recheck
end end
...@@ -209,6 +213,7 @@ class MergeRequest < ApplicationRecord ...@@ -209,6 +213,7 @@ class MergeRequest < ApplicationRecord
transition [:unchecked, :cannot_be_merged_recheck, :checking, :cannot_be_merged_rechecking] => :cannot_be_merged transition [:unchecked, :cannot_be_merged_recheck, :checking, :cannot_be_merged_rechecking] => :cannot_be_merged
end end
state :preparing
state :unchecked state :unchecked
state :cannot_be_merged_recheck state :cannot_be_merged_recheck
state :checking state :checking
...@@ -237,7 +242,7 @@ class MergeRequest < ApplicationRecord ...@@ -237,7 +242,7 @@ class MergeRequest < ApplicationRecord
# Returns current merge_status except it returns `cannot_be_merged_rechecking` as `checking` # Returns current merge_status except it returns `cannot_be_merged_rechecking` as `checking`
# to avoid exposing unnecessary internal state # to avoid exposing unnecessary internal state
def public_merge_status def public_merge_status
cannot_be_merged_rechecking? ? 'checking' : merge_status cannot_be_merged_rechecking? || preparing? ? 'checking' : merge_status
end end
validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_or_merged_without_fork?] validates :source_project, presence: true, unless: [:allow_broken, :importing?, :closed_or_merged_without_fork?]
......
...@@ -3,6 +3,13 @@ ...@@ -3,6 +3,13 @@
module MergeRequests module MergeRequests
class AfterCreateService < MergeRequests::BaseService class AfterCreateService < MergeRequests::BaseService
def execute(merge_request) def execute(merge_request)
prepare_merge_request(merge_request)
merge_request.mark_as_unchecked if merge_request.preparing?
end
private
def prepare_merge_request(merge_request)
event_service.open_mr(merge_request, current_user) event_service.open_mr(merge_request, current_user)
merge_request_activity_counter.track_create_mr_action(user: current_user) merge_request_activity_counter.track_create_mr_action(user: current_user)
......
...@@ -5,8 +5,8 @@ module EE ...@@ -5,8 +5,8 @@ module EE
module AfterCreateService module AfterCreateService
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
override :execute override :prepare_merge_request
def execute(merge_request) def prepare_merge_request(merge_request)
super super
schedule_sync_for(merge_request.head_pipeline_id) schedule_sync_for(merge_request.head_pipeline_id)
......
...@@ -2899,6 +2899,14 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -2899,6 +2899,14 @@ RSpec.describe MergeRequest, factory_default: :keep do
expect(subject.mergeable?).to be_truthy expect(subject.mergeable?).to be_truthy
end end
it 'return true if #mergeable_state? is true and the MR #can_be_merged? is false' do
allow(subject).to receive(:mergeable_state?) { true }
expect(subject).to receive(:check_mergeability)
expect(subject).to receive(:can_be_merged?) { false }
expect(subject.mergeable?).to be_falsey
end
context 'with skip_ci_check option' do context 'with skip_ci_check option' do
before do before do
allow(subject).to receive_messages(check_mergeability: nil, allow(subject).to receive_messages(check_mergeability: nil,
...@@ -3076,6 +3084,7 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -3076,6 +3084,7 @@ RSpec.describe MergeRequest, factory_default: :keep do
where(:status, :public_status) do where(:status, :public_status) do
'cannot_be_merged_rechecking' | 'checking' 'cannot_be_merged_rechecking' | 'checking'
'preparing' | 'checking'
'checking' | 'checking' 'checking' | 'checking'
'cannot_be_merged' | 'cannot_be_merged' 'cannot_be_merged' | 'cannot_be_merged'
end end
......
...@@ -71,5 +71,27 @@ RSpec.describe MergeRequests::AfterCreateService do ...@@ -71,5 +71,27 @@ RSpec.describe MergeRequests::AfterCreateService do
it_behaves_like 'records an onboarding progress action', :merge_request_created do it_behaves_like 'records an onboarding progress action', :merge_request_created do
let(:namespace) { merge_request.target_project.namespace } let(:namespace) { merge_request.target_project.namespace }
end end
context 'when merge request is in unchecked state' do
before do
merge_request.mark_as_unchecked!
execute_service
end
it 'does not change its state' do
expect(merge_request.reload).to be_unchecked
end
end
context 'when merge request is in preparing state' do
before do
merge_request.mark_as_preparing!
execute_service
end
it 'marks the merge request as unchecked' do
expect(merge_request.reload).to be_unchecked
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment