Commit 812f5106 authored by Mark Chao's avatar Mark Chao

Add ApprovalState and ApprovalWrappedRule

ApprovalWrappedRule acts as common interface for project/MR rules.
ApprovalState allows easier invalidation.
parent 1ad788f5
# frozen_string_literal: true
require 'forwardable'
# A state object to centralize logic related to various approval related states.
# This reduce interface footprint on MR and allows easier cache invalidation.
class ApprovalState
extend Forwardable
include ::Gitlab::Utils::StrongMemoize
attr_reader :merge_request, :project
def_delegators :@merge_request, :merge_status, :approved_by_users
alias_method :approved_approvers, :approved_by_users
def initialize(merge_request)
@merge_request = merge_request
@project = merge_request.target_project
end
# Excludes the author if 'self-approval' isn't explicitly enabled on project settings.
def self.filter_author(users, merge_request)
return users unless merge_request.author_id
return users if merge_request.target_project.merge_requests_author_approval?
if users.is_a?(ActiveRecord::Relation) && !users.loaded?
users.where.not(id: merge_request.author_id)
else
users.dup
users.delete(merge_request.author)
users
end
end
def wrapped_approval_rules
strong_memoize(:wrapped_approval_rules) do
regular_rules + code_owner_rules
end
end
def approval_rules_overwritten?
merge_request.approval_rules.regular.exists?
end
def approval_needed?
return false unless project.feature_available?(:merge_request_approvers)
!approved?
end
def approved?
strong_memoize(:approved) do
wrapped_approval_rules.all?(&:approved?)
end
end
alias_method :any_approver_allowed?, :approved?
# Number of approvals remaining (excluding existing approvals) before the MR is
# considered approved.
def approvals_left
strong_memoize(:approvals_left) do
wrapped_approval_rules.sum(&:approvals_left)
end
end
def approval_rules_left
wrapped_approval_rules.reject(&:approved?)
end
def approvers
strong_memoize(:approvers) { filtered_approvers(target: :approvers) }
end
# @param regular [Boolean]
# @param code_owner [Boolean]
# @param target [:approvers, :users]
# @param unactioned [Boolean]
def filtered_approvers(regular: true, code_owner: true, target: :approvers, unactioned: false)
rules = []
rules.concat(regular_rules) if regular
rules.concat(code_owner_rules) if code_owner
users = rules.flat_map(&target)
users.uniq!
users -= approved_approvers if unactioned
self.class.filter_author(users, merge_request)
end
# approvers_left
def unactioned_approvers
strong_memoize(:unactioned_approvers) { approvers - approved_approvers }
end
def can_approve?(user)
return false unless user
# The check below considers authors being able to approve the MR.
# That is, they're included/excluded from that list accordingly.
return true if unactioned_approvers.include?(user)
# We can safely unauthorize authors if it reaches this guard clause.
return false if user == merge_request.author
return false unless user.can?(:update_merge_request, merge_request)
any_approver_allowed? && merge_request.approvals.where(user: user).empty?
end
def has_approved?(user)
return false unless user
approved_approvers.include?(user)
end
def authors_can_approve?
project.merge_requests_author_approval?
end
# TODO: remove after #1979 is closed
# This is a temporary method for backward compatibility
# before introduction of approval rules.
# This avoids re-queries.
def first_regular_rule
strong_memoize(:first_regular_rule) do
regular_rules.first
end
end
private
def regular_rules
strong_memoize(:regular_rules) do
rule_source = approval_rules_overwritten? ? merge_request : project
rules = rule_source.approval_rules.regular
unless project.feature_available?(:multiple_approval_rules)
rules = rules.order(id: :asc).limit(1)
end
wrap_rules(rules)
end
end
def code_owner_rules
strong_memoize(:code_owner_rules) do
wrap_rules(merge_request.approval_rules.code_owner)
end
end
def wrap_rules(rules)
rules.map { |rule| ApprovalWrappedRule.new(merge_request, rule) }
end
end
# frozen_string_literal: true
# A common state computation interface to wrap around ApprovalRuleLike models
class ApprovalWrappedRule
extend Forwardable
include Gitlab::Utils::StrongMemoize
attr_reader :merge_request
attr_reader :approval_rule
def_delegators :@approval_rule, :id, :name, :users, :groups, :approvals_required, :code_owner
def initialize(merge_request, approval_rule)
@merge_request = merge_request
@approval_rule = approval_rule
end
def project
@merge_request.target_project
end
def approvers
ApprovalState.filter_author(@approval_rule.approvers, merge_request)
end
# @return [Array<User>] all approvers related to this rule
#
# This is dynamically calculated when MR is open, but is persisted in DB after MR is merged.
#
# After merge, the approval state should no longer change.
# Persisting instead of recomputing dynamically guarantees this even
# if project level rule ever adds/removes approvers after the merge.
#
# For open MRs, it is still dynamically calculated instead of persisted because
# - Additional complexity to add update hooks
# - DB updating many MRs for one project rule change is inefficient
def approved_approvers
return approval_rule.approved_approvers if merge_request.merged?
strong_memoize(:approved_approvers) do
overall_approver_ids = merge_request.approvals.map(&:user_id)
approvers.select do |approver|
overall_approver_ids.include?(approver.id)
end
end
end
def approved?
strong_memoize(:approved) do
approvals_left <= 0 || unactioned_approvers.size <= 0
end
end
# Number of approvals remaining (excluding existing approvals)
# before the rule is considered approved.
#
# If there are fewer potential approvers than approvals left,
# users should either reduce `approvals_required`
# and/or allow MR authors to approve their own merge
# requests (in case only one approval is needed).
def approvals_left
strong_memoize(:approvals_left) do
approvals_left_count = approvals_required - approved_approvers.size
[approvals_left_count, 0].max
end
end
def unactioned_approvers
approvers - approved_approvers
end
end
......@@ -7,6 +7,10 @@ module Approvable
# such as approver_groups and target_project in presenters
include ::VisibleApprovable
def approval_state
@approval_state ||= ApprovalState.new(self)
end
def approval_needed?
approvals_required&.nonzero?
end
......
......@@ -80,5 +80,6 @@ module VisibleApprovable
clear_memoization(:approvers_left)
clear_memoization(:all_approvers_including_groups)
clear_memoization(:approval_state)
end
end
This diff is collapsed.
# frozen_string_literal: true
require 'spec_helper'
describe ApprovalWrappedRule do
let(:merge_request) { create(:merge_request) }
let(:rule) { create(:approval_merge_request_rule, merge_request: merge_request, approvals_required: approvals_required) }
let(:approvals_required) { 0 }
let(:approver1) { create(:user) }
let(:approver2) { create(:user) }
let(:approver3) { create(:user) }
subject { described_class.new(merge_request, rule) }
describe '#project' do
it 'returns merge request project' do
expect(subject.project).to eq(merge_request.target_project)
end
end
describe '#approvals_left' do
before do
create(:approval, merge_request: merge_request, user: approver1)
create(:approval, merge_request: merge_request, user: approver2)
rule.users << approver1
rule.users << approver2
end
context 'when approvals_required is greater than approved approver count' do
let(:approvals_required) { 8 }
it 'returns approvals still needed' do
expect(subject.approvals_left).to eq(6)
end
end
context 'when approvals_required is less than approved approver count' do
let(:approvals_required) { 1 }
it 'returns zero' do
expect(subject.approvals_left).to eq(0)
end
end
end
describe '#approved?' do
before do
create(:approval, merge_request: merge_request, user: approver1)
rule.users << approver1
end
context 'when approvals left is zero' do
let(:approvals_required) { 1 }
it 'returns true' do
expect(subject.approved?).to eq(true)
end
end
context 'when approvals left is not zero, but there is still unactioned approvers' do
let(:approvals_required) { 99 }
before do
rule.users << approver2
end
it 'returns false' do
expect(subject.approved?).to eq(false)
end
end
context 'when approvals left is not zero, but there is no unactioned approvers' do
let(:approvals_required) { 99 }
it 'returns true' do
expect(subject.approved?).to eq(true)
end
end
end
describe '#approved_approvers' do
context 'when some approvers has made the approvals' do
before do
create(:approval, merge_request: merge_request, user: approver1)
create(:approval, merge_request: merge_request, user: approver2)
rule.users = [approver1, approver3]
end
it 'returns approved approvers' do
expect(subject.approved_approvers).to contain_exactly(approver1)
end
end
context 'when merged' do
let(:merge_request) { create(:merged_merge_request) }
before do
rule.approved_approvers << approver3
end
it 'returns approved approvers from database' do
expect(subject.approved_approvers).to contain_exactly(approver3)
end
end
end
describe '#unactioned_approvers' do
context 'when some approvers has not approved yet' do
before do
create(:approval, merge_request: merge_request, user: approver1)
rule.users = [approver1, approver2]
end
it 'returns unactioned approvers' do
expect(subject.unactioned_approvers).to contain_exactly(approver2)
end
end
context 'when merged' do
let(:merge_request) { create(:merged_merge_request) }
before do
rule.approved_approvers << approver3
rule.users = [approver1, approver3]
end
it 'returns approved approvers from database' do
expect(subject.unactioned_approvers).to contain_exactly(approver1)
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