Commit fb36e7af authored by Igor Drozdov's avatar Igor Drozdov

Merge branch '217939-add-mr-approval-settings-to-mr-entity' into 'master'

Add MR approval status to MR compliance entity

See merge request gitlab-org/gitlab!36824
parents 43fc0ec5 f1eb6d2d
......@@ -36,6 +36,7 @@ module EE
delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true
delegate :sha, to: :base_pipeline, prefix: :base_pipeline, allow_nil: true
delegate :merge_requests_author_approval?, to: :target_project, allow_nil: true
delegate :merge_requests_disable_committers_approval?, to: :target_project, allow_nil: true
scope :without_approvals, -> { left_outer_joins(:approvals).where(approvals: { id: nil }) }
scope :with_approvals, -> { joins(:approvals) }
......
......@@ -3,6 +3,10 @@
class MergeRequestComplianceEntity < Grape::Entity
include RequestAwareEntity
SUCCESS_APPROVAL_STATUS = :success
WARNING_APPROVAL_STATUS = :warning
FAILED_APPROVAL_STATUS = :failed
expose :id
expose :title
expose :merged_at
......@@ -21,6 +25,8 @@ class MergeRequestComplianceEntity < Grape::Entity
expose :pipeline_status, if: -> (*) { can_read_pipeline? }, with: DetailedStatusEntity
expose :approval_status
private
alias_method :merge_request, :object
......@@ -32,4 +38,19 @@ class MergeRequestComplianceEntity < Grape::Entity
def pipeline_status
merge_request.head_pipeline.detailed_status(request.current_user)
end
def approval_status
# All these checks should be false for this to pass as a success
# If any are true then there is a violation of the separation of duties
checks = [
merge_request.authors_can_approve?,
merge_request.committers_can_approve?,
merge_request.approvals_required < 2
]
return FAILED_APPROVAL_STATUS if checks.all?
return WARNING_APPROVAL_STATUS if checks.any?
SUCCESS_APPROVAL_STATUS
end
end
---
title: Add MR approval status to the MR compliance entity
merge_request: 36824
author:
type: added
......@@ -17,7 +17,7 @@ RSpec.describe MergeRequestComplianceEntity do
it 'includes merge request attributes for compliance' do
expect(subject).to include(
:id, :title, :merged_at, :milestone, :path, :issuable_reference, :author, :approved_by_users
:id, :title, :merged_at, :milestone, :path, :issuable_reference, :author, :approved_by_users, :approval_status
)
end
......@@ -57,5 +57,45 @@ RSpec.describe MergeRequestComplianceEntity do
end
end
end
context 'with an approval status' do
let_it_be(:committers_approval_enabled) { false }
let_it_be(:authors_approval_enabled) { false }
let_it_be(:approvals_required) { 2 }
shared_examples 'the approval status' do
before do
allow(merge_request).to receive(:authors_can_approve?).and_return(authors_approval_enabled)
allow(merge_request).to receive(:committers_can_approve?).and_return(committers_approval_enabled)
allow(merge_request).to receive(:approvals_required).and_return(approvals_required)
end
it 'is correct' do
expect(subject[:approval_status]).to eq(status)
end
end
context 'all approval checks pass' do
let_it_be(:status) { :success }
it_behaves_like 'the approval status'
end
context 'only some of the approval checks pass' do
let_it_be(:authors_approval_enabled) { true }
let_it_be(:status) { :warning }
it_behaves_like 'the approval status'
end
context 'none of the approval checks pass' do
let_it_be(:committers_approval_enabled) { true }
let_it_be(:authors_approval_enabled) { true }
let_it_be(:approvals_required) { 0 }
let_it_be(:status) { :failed }
it_behaves_like 'the approval status'
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