Commit ae139028 authored by Patrick Bajao's avatar Patrick Bajao

Extract draft status check to mergeability framework

As part of the effort of improving the checks we have to determine
if a MR is in a mergeable state, this moves the draft check to the
mergeability checks framework.

This is behind the improved_mergeability_checks feature flag.
parent ab754b70
...@@ -1153,7 +1153,6 @@ class MergeRequest < ApplicationRecord ...@@ -1153,7 +1153,6 @@ class MergeRequest < ApplicationRecord
# rubocop: disable CodeReuse/ServiceClass # rubocop: disable CodeReuse/ServiceClass
def mergeable_state?(skip_ci_check: false, skip_discussions_check: false) def mergeable_state?(skip_ci_check: false, skip_discussions_check: false)
return false unless open? return false unless open?
return false if work_in_progress?
if Feature.enabled?(:improved_mergeability_checks, self.project, default_enabled: :yaml) if Feature.enabled?(:improved_mergeability_checks, self.project, default_enabled: :yaml)
additional_checks = MergeRequests::Mergeability::RunChecksService.new( additional_checks = MergeRequests::Mergeability::RunChecksService.new(
...@@ -1165,6 +1164,7 @@ class MergeRequest < ApplicationRecord ...@@ -1165,6 +1164,7 @@ class MergeRequest < ApplicationRecord
) )
additional_checks.execute.all?(&:success?) additional_checks.execute.all?(&:success?)
else else
return false if draft?
return false if broken? return false if broken?
return false unless skip_discussions_check || mergeable_discussions_state? return false unless skip_discussions_check || mergeable_discussions_state?
return false unless skip_ci_check || mergeable_ci_state? return false unless skip_ci_check || mergeable_ci_state?
......
# frozen_string_literal: true
module MergeRequests
module Mergeability
class CheckDraftStatusService < CheckBaseService
def execute
if merge_request.draft?
failure
else
success
end
end
def skip?
false
end
def cacheable?
false
end
end
end
end
...@@ -7,6 +7,7 @@ module MergeRequests ...@@ -7,6 +7,7 @@ module MergeRequests
# We want to have the cheapest checks first in the list, # We want to have the cheapest checks first in the list,
# that way we can fail fast before running the more expensive ones # that way we can fail fast before running the more expensive ones
CHECKS = [ CHECKS = [
CheckDraftStatusService,
CheckBrokenStatusService, CheckBrokenStatusService,
CheckDiscussionsStatusService, CheckDiscussionsStatusService,
CheckCiStatusService CheckCiStatusService
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe MergeRequests::Mergeability::CheckDraftStatusService do
subject(:check_draft_status) { described_class.new(merge_request: merge_request, params: {}) }
let(:merge_request) { build(:merge_request) }
describe '#execute' do
before do
expect(merge_request).to receive(:draft?).and_return(draft)
end
context 'when the merge request is a draft' do
let(:draft) { true }
it 'returns a check result with status failed' do
expect(check_draft_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS
end
end
context 'when the merge request is not a draft' do
let(:draft) { false }
it 'returns a check result with status success' do
expect(check_draft_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS
end
end
end
describe '#skip?' do
it 'returns false' do
expect(check_draft_status.skip?).to eq false
end
end
describe '#cacheable?' do
it 'returns false' do
expect(check_draft_status.cacheable?).to eq false
end
end
end
...@@ -47,7 +47,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do ...@@ -47,7 +47,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do
expect(service).not_to receive(:execute) expect(service).not_to receive(:execute)
end end
expect(execute).to match_array([success_result, success_result]) expect(execute).to match_array([success_result, success_result, success_result])
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