Merge branch '1979-2-0' into 'master'

Add ApprovalState

See merge request gitlab-org/gitlab-ee!8974

ee/app/models/approval_state.rb

+# 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, :approvals
+  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 if merge_request.target_project.merge_requests_author_approval?
+
+    if users.is_a?(ActiveRecord::Relation) && !users.loaded?
+      users.where.not(id: merge_request.authors)
+    else
+      users - merge_request.authors
+    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)
+
+    overall_approvals_required > 0 || wrapped_approval_rules.any? { |rule| rule.approvals_required > 0 }
+  end
+
+  def overall_approvals_required
+    @overall_approvals_required ||= project.approvals_before_merge
+  end
+
+  def approved?
+    strong_memoize(:approved) do
+      (overall_approvals_required == 0 || approvals.size >= overall_approvals_required) && wrapped_approval_rules.all?(&:approved?)
+    end
+  end
+
+  def any_approver_allowed?
+    approved? || overall_approvals_required > approvers.size
+  end
+
+  # 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 merge_request.authors.include?(user)
+    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

ee/app/models/approval_wrapped_rule.rb

+# 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

ee/app/models/concerns/approvable.rb

@@ -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

ee/app/models/concerns/visible_approvable.rb

@@ -80,5 +80,6 @@ module VisibleApprovable
 
     clear_memoization(:approvers_left)
     clear_memoization(:all_approvers_including_groups)
+    clear_memoization(:approval_state)
   end
 end

ee/app/models/license.rb

@@ -55,6 +55,7 @@ class License < ActiveRecord::Base
     ldap_group_sync_filter
     multiple_clusters
     multiple_group_issue_boards
+    multiple_approval_rules
     merge_request_performance_metrics
     object_storage
     group_saml

ee/spec/models/approval_wrapped_rule_spec.rb

+# 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