Commit f1eb6d2d authored by Robert Hunt's avatar Robert Hunt Committed by Igor Drozdov

Add approval status checks to the MR compliance entity

- Added approval status to the MR compliance entity
- Updated the MR compliance entity rspec tests
- Added changelog
parent e8771823
...@@ -36,6 +36,7 @@ module EE ...@@ -36,6 +36,7 @@ module EE
delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true
delegate :sha, to: :base_pipeline, prefix: :base_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_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 :without_approvals, -> { left_outer_joins(:approvals).where(approvals: { id: nil }) }
scope :with_approvals, -> { joins(:approvals) } scope :with_approvals, -> { joins(:approvals) }
......
...@@ -3,6 +3,10 @@ ...@@ -3,6 +3,10 @@
class MergeRequestComplianceEntity < Grape::Entity class MergeRequestComplianceEntity < Grape::Entity
include RequestAwareEntity include RequestAwareEntity
SUCCESS_APPROVAL_STATUS = :success
WARNING_APPROVAL_STATUS = :warning
FAILED_APPROVAL_STATUS = :failed
expose :id expose :id
expose :title expose :title
expose :merged_at expose :merged_at
...@@ -21,6 +25,8 @@ class MergeRequestComplianceEntity < Grape::Entity ...@@ -21,6 +25,8 @@ class MergeRequestComplianceEntity < Grape::Entity
expose :pipeline_status, if: -> (*) { can_read_pipeline? }, with: DetailedStatusEntity expose :pipeline_status, if: -> (*) { can_read_pipeline? }, with: DetailedStatusEntity
expose :approval_status
private private
alias_method :merge_request, :object alias_method :merge_request, :object
...@@ -32,4 +38,19 @@ class MergeRequestComplianceEntity < Grape::Entity ...@@ -32,4 +38,19 @@ class MergeRequestComplianceEntity < Grape::Entity
def pipeline_status def pipeline_status
merge_request.head_pipeline.detailed_status(request.current_user) merge_request.head_pipeline.detailed_status(request.current_user)
end 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 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 ...@@ -17,7 +17,7 @@ RSpec.describe MergeRequestComplianceEntity do
it 'includes merge request attributes for compliance' do it 'includes merge request attributes for compliance' do
expect(subject).to include( 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 end
...@@ -57,5 +57,45 @@ RSpec.describe MergeRequestComplianceEntity do ...@@ -57,5 +57,45 @@ RSpec.describe MergeRequestComplianceEntity do
end end
end 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
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