Commit 225522e3 authored by Kerri Miller's avatar Kerri Miller

Merge branch '291012-preparing-mr-state' into 'master'

Implement new preparing internal merge_status

See merge request gitlab-org/gitlab!54900
parents c4226b11 275b5988
...@@ -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] => :unchecked transition [:preparing, :can_be_merged, :checking, :unchecked] => :unchecked
transition [:cannot_be_merged, :cannot_be_merged_rechecking, :cannot_be_merged_recheck] => :cannot_be_merged_recheck transition [:cannot_be_merged, :cannot_be_merged_rechecking, :cannot_be_merged_recheck] => :cannot_be_merged_recheck
end end
...@@ -237,7 +241,7 @@ class MergeRequest < ApplicationRecord ...@@ -237,7 +241,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?]
...@@ -1054,6 +1058,8 @@ class MergeRequest < ApplicationRecord ...@@ -1054,6 +1058,8 @@ class MergeRequest < ApplicationRecord
end end
def mergeable?(skip_ci_check: false, skip_discussions_check: false) def mergeable?(skip_ci_check: false, skip_discussions_check: false)
return false if preparing?
return false unless mergeable_state?(skip_ci_check: skip_ci_check, return false unless mergeable_state?(skip_ci_check: skip_ci_check,
skip_discussions_check: skip_discussions_check) skip_discussions_check: skip_discussions_check)
......
...@@ -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)
notification_service.new_merge_request(merge_request, current_user) notification_service.new_merge_request(merge_request, current_user)
......
...@@ -14,6 +14,8 @@ module MergeRequests ...@@ -14,6 +14,8 @@ module MergeRequests
end end
def after_create(issuable) def after_create(issuable)
issuable.mark_as_preparing
# Add new items to MergeRequests::AfterCreateService if they can # Add new items to MergeRequests::AfterCreateService if they can
# be performed in Sidekiq # be performed in Sidekiq
NewMergeRequestWorker.perform_async(issuable.id, current_user.id) NewMergeRequestWorker.perform_async(issuable.id, current_user.id)
......
---
title: Implement new preparing internal merge_status
merge_request: 54900
author:
type: other
...@@ -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)
......
...@@ -2885,6 +2885,11 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -2885,6 +2885,11 @@ RSpec.describe MergeRequest, factory_default: :keep do
describe '#mergeable?' do describe '#mergeable?' do
subject { build_stubbed(:merge_request) } subject { build_stubbed(:merge_request) }
it 'returns false if still preparing' do
expect(subject).to receive(:preparing?) { true }
expect(subject.mergeable?).to be_falsey
end
it 'returns false if #mergeable_state? is false' do it 'returns false if #mergeable_state? is false' do
expect(subject).to receive(:mergeable_state?) { false } expect(subject).to receive(:mergeable_state?) { false }
...@@ -3075,6 +3080,7 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -3075,6 +3080,7 @@ RSpec.describe MergeRequest, factory_default: :keep do
subject { build(:merge_request, merge_status: status) } subject { build(:merge_request, merge_status: status) }
where(:status, :public_status) do where(:status, :public_status) do
'preparing' | 'checking'
'cannot_be_merged_rechecking' | 'checking' 'cannot_be_merged_rechecking' | 'checking'
'checking' | 'checking' 'checking' | 'checking'
'cannot_be_merged' | 'cannot_be_merged' 'cannot_be_merged' | 'cannot_be_merged'
......
...@@ -67,5 +67,27 @@ RSpec.describe MergeRequests::AfterCreateService do ...@@ -67,5 +67,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
...@@ -67,6 +67,10 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do ...@@ -67,6 +67,10 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
expect(Event.where(attributes).count).to eq(1) expect(Event.where(attributes).count).to eq(1)
end end
it 'sets the merge_status to preparing' do
expect(merge_request.reload).to be_preparing
end
describe 'when marked with /wip' do describe 'when marked with /wip' do
context 'in title and in description' do context 'in title and in description' do
let(:opts) do let(:opts) do
......
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