Commit 12005471 authored by Patrick Bajao's avatar Patrick Bajao

Extract MergeRequest#open? 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 `open?` check to the
mergeability checks framework.

This is behind the `improved_mergeability_checks` feature flag.
parent 0585c0b6
...@@ -1152,8 +1152,6 @@ class MergeRequest < ApplicationRecord ...@@ -1152,8 +1152,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?
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(
merge_request: self, merge_request: self,
...@@ -1164,6 +1162,7 @@ class MergeRequest < ApplicationRecord ...@@ -1164,6 +1162,7 @@ class MergeRequest < ApplicationRecord
) )
additional_checks.execute.all?(&:success?) additional_checks.execute.all?(&:success?)
else else
return false unless open?
return false if draft? 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?
......
# frozen_string_literal: true
module MergeRequests
module Mergeability
class CheckOpenStatusService < CheckBaseService
def execute
if merge_request.open?
success
else
failure
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 = [
CheckOpenStatusService,
CheckDraftStatusService, CheckDraftStatusService,
CheckBrokenStatusService, CheckBrokenStatusService,
CheckDiscussionsStatusService, CheckDiscussionsStatusService,
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe MergeRequests::Mergeability::CheckOpenStatusService do
subject(:check_open_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(:open?).and_return(open)
end
context 'when the merge request is open' do
let(:open) { true }
it 'returns a check result with status success' do
expect(check_open_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::SUCCESS_STATUS
end
end
context 'when the merge request is not open' do
let(:open) { false }
it 'returns a check result with status failed' do
expect(check_open_status.execute.status).to eq Gitlab::MergeRequests::Mergeability::CheckResult::FAILED_STATUS
end
end
end
describe '#skip?' do
it 'returns false' do
expect(check_open_status.skip?).to eq false
end
end
describe '#cacheable?' do
it 'returns false' do
expect(check_open_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, success_result]) expect(execute).to match_array([success_result, 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