Commit 9c4caff3 authored by Kerri Miller's avatar Kerri Miller

Merge branch '351190-fix-stuck-mergeability-check' into 'master'

Ensure mergeability check runs on specific cases

See merge request gitlab-org/gitlab!79311
parents d06b6428 76e5f067
...@@ -100,10 +100,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -100,10 +100,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
# rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/AbcSize
def show def show
close_merge_request_if_no_source_project close_merge_request_if_no_source_project
@merge_request.check_mergeability(async: true)
if Feature.disabled?(:check_mergeability_async_in_widget, @project, default_enabled: :yaml)
@merge_request.check_mergeability(async: true)
end
respond_to do |format| respond_to do |format|
format.html do format.html do
......
...@@ -8,7 +8,6 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity ...@@ -8,7 +8,6 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity
expose :merged_commit_sha expose :merged_commit_sha
expose :short_merged_commit_sha expose :short_merged_commit_sha
expose :merge_error expose :merge_error
expose :public_merge_status, as: :merge_status
expose :merge_user_id expose :merge_user_id
expose :source_branch expose :source_branch
expose :source_project_id expose :source_project_id
...@@ -26,6 +25,11 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity ...@@ -26,6 +25,11 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity
expose :source_branch_exists?, as: :source_branch_exists expose :source_branch_exists?, as: :source_branch_exists
expose :branch_missing?, as: :branch_missing expose :branch_missing?, as: :branch_missing
expose :merge_status do |merge_request|
merge_request.check_mergeability(async: true)
merge_request.public_merge_status
end
expose :default_squash_commit_message do |merge_request| expose :default_squash_commit_message do |merge_request|
merge_request.default_squash_commit_message(user: request.current_user) merge_request.default_squash_commit_message(user: request.current_user)
end end
......
...@@ -24,8 +24,6 @@ class MergeRequestPollWidgetEntity < Grape::Entity ...@@ -24,8 +24,6 @@ class MergeRequestPollWidgetEntity < Grape::Entity
end end
expose :mergeable do |merge_request, options| expose :mergeable do |merge_request, options|
next merge_request.mergeable? if Feature.disabled?(:check_mergeability_async_in_widget, merge_request.project, default_enabled: :yaml)
merge_request.mergeable? merge_request.mergeable?
end end
......
...@@ -7,7 +7,7 @@ module MergeRequests ...@@ -7,7 +7,7 @@ module MergeRequests
def execute(merge_request) def execute(merge_request)
prepare_for_mergeability(merge_request) if early_prepare_for_mergeability?(merge_request) prepare_for_mergeability(merge_request) if early_prepare_for_mergeability?(merge_request)
prepare_merge_request(merge_request) prepare_merge_request(merge_request)
mark_as_unchecked(merge_request) unless early_prepare_for_mergeability?(merge_request) check_mergeability(merge_request) unless early_prepare_for_mergeability?(merge_request)
end end
private private
...@@ -15,7 +15,7 @@ module MergeRequests ...@@ -15,7 +15,7 @@ module MergeRequests
def prepare_for_mergeability(merge_request) def prepare_for_mergeability(merge_request)
create_pipeline_for(merge_request, current_user) create_pipeline_for(merge_request, current_user)
merge_request.update_head_pipeline merge_request.update_head_pipeline
mark_as_unchecked(merge_request) check_mergeability(merge_request)
end end
def prepare_merge_request(merge_request) def prepare_merge_request(merge_request)
...@@ -55,8 +55,13 @@ module MergeRequests ...@@ -55,8 +55,13 @@ module MergeRequests
end end
end end
def mark_as_unchecked(merge_request) def check_mergeability(merge_request)
merge_request.mark_as_unchecked if merge_request.preparing? return unless merge_request.preparing?
# Need to set to unchecked to be able to check for mergeability or else
# it'll be a no-op.
merge_request.mark_as_unchecked
merge_request.check_mergeability(async: true)
end end
end end
end end
......
---
name: check_mergeability_async_in_widget
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58178
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326567
milestone: '13.11'
type: development
group: group::source code
default_enabled: false
...@@ -57,19 +57,13 @@ RSpec.describe Projects::MergeRequestsController do ...@@ -57,19 +57,13 @@ RSpec.describe Projects::MergeRequestsController do
merge_request.mark_as_unchecked! merge_request.mark_as_unchecked!
end end
context 'check_mergeability_async_in_widget feature flag is disabled' do it 'checks mergeability asynchronously' do
before do expect_next_instance_of(MergeRequests::MergeabilityCheckService) do |service|
stub_feature_flags(check_mergeability_async_in_widget: false) expect(service).not_to receive(:execute)
expect(service).to receive(:async_execute)
end end
it 'checks mergeability asynchronously' do go
expect_next_instance_of(MergeRequests::MergeabilityCheckService) do |service|
expect(service).not_to receive(:execute)
expect(service).to receive(:async_execute)
end
go
end
end end
end end
......
...@@ -21,12 +21,6 @@ RSpec.describe MergeRequestPollCachedWidgetEntity do ...@@ -21,12 +21,6 @@ RSpec.describe MergeRequestPollCachedWidgetEntity do
is_expected.to include(:target_branch_sha) is_expected.to include(:target_branch_sha)
end end
it 'has public_merge_status as merge_status' do
expect(resource).to receive(:public_merge_status).and_return('checking')
expect(subject[:merge_status]).to eq 'checking'
end
it 'has blob path data' do it 'has blob path data' do
allow(resource).to receive_messages( allow(resource).to receive_messages(
base_pipeline: pipeline, base_pipeline: pipeline,
...@@ -38,6 +32,20 @@ RSpec.describe MergeRequestPollCachedWidgetEntity do ...@@ -38,6 +32,20 @@ RSpec.describe MergeRequestPollCachedWidgetEntity do
expect(subject[:blob_path]).to include(:head_path) expect(subject[:blob_path]).to include(:head_path)
end end
describe 'merge_status' do
it 'calls for MergeRequest#check_mergeability' do
expect(resource).to receive(:check_mergeability).with(async: true)
subject[:merge_status]
end
it 'has public_merge_status as merge_status' do
expect(resource).to receive(:public_merge_status).and_return('checking')
expect(subject[:merge_status]).to eq 'checking'
end
end
describe 'diverged_commits_count' do describe 'diverged_commits_count' do
context 'when MR open and its diverging' do context 'when MR open and its diverging' do
it 'returns diverged commits count' do it 'returns diverged commits count' do
......
...@@ -180,16 +180,6 @@ RSpec.describe MergeRequestPollWidgetEntity do ...@@ -180,16 +180,6 @@ RSpec.describe MergeRequestPollWidgetEntity do
it 'calculates mergeability and returns true' do it 'calculates mergeability and returns true' do
expect(subject[:mergeable]).to eq(true) expect(subject[:mergeable]).to eq(true)
end end
context 'when check_mergeability_async_in_widget is disabled' do
before do
stub_feature_flags(check_mergeability_async_in_widget: false)
end
it 'calculates mergeability and returns true' do
expect(subject[:mergeable]).to eq(true)
end
end
end end
end end
end end
...@@ -89,10 +89,10 @@ RSpec.describe MergeRequests::AfterCreateService do ...@@ -89,10 +89,10 @@ RSpec.describe MergeRequests::AfterCreateService do
merge_request.mark_as_preparing! merge_request.mark_as_preparing!
end end
it 'marks the merge request as unchecked' do it 'checks for mergeability' do
execute_service expect(merge_request).to receive(:check_mergeability).with(async: true)
expect(merge_request.reload).to be_unchecked execute_service
end end
context 'when preparing for mergeability fails' do context 'when preparing for mergeability fails' do
...@@ -130,9 +130,9 @@ RSpec.describe MergeRequests::AfterCreateService do ...@@ -130,9 +130,9 @@ RSpec.describe MergeRequests::AfterCreateService do
.and_raise(StandardError) .and_raise(StandardError)
end end
it 'still marks the merge request as unchecked' do it 'still checks for mergeability' do
expect(merge_request).to receive(:check_mergeability).with(async: true)
expect { execute_service }.to raise_error(StandardError) expect { execute_service }.to raise_error(StandardError)
expect(merge_request.reload).to be_unchecked
end end
context 'when early_prepare_for_mergeability feature flag is disabled' do context 'when early_prepare_for_mergeability feature flag is disabled' 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