From b46ff56e52c6baded5c8b4edb90be31d31267da9 Mon Sep 17 00:00:00 2001
From: David Kim <dkim@gitlab.com>
Date: Thu, 8 Sep 2022 16:21:46 +0930
Subject: [PATCH] Use approvals only to check if the user approved
We don't need additional join to answer that question
---
app/models/concerns/approvable.rb | 2 +-
ee/app/models/approval_state.rb | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/app/models/concerns/approvable.rb b/app/models/concerns/approvable.rb
index f18a3877fe0c..1566c53217d0 100644
--- a/app/models/concerns/approvable.rb
+++ b/app/models/concerns/approvable.rb
@@ -47,7 +47,7 @@ def select_from_union(relations)
def approved_by?(user)
return false unless user
- approved_by_users.include?(user)
+ approvals.where(user: user).any?
end
def can_be_approved_by?(user)
diff --git a/ee/app/models/approval_state.rb b/ee/app/models/approval_state.rb
index a095e128905d..c28ab346c74e 100644
--- a/ee/app/models/approval_state.rb
+++ b/ee/app/models/approval_state.rb
@@ -10,7 +10,7 @@ class ApprovalState
attr_reader :merge_request, :project
- def_delegators :@merge_request, :merge_status, :approved_by_users, :approvals, :approval_feature_available?
+ def_delegators :@merge_request, :merge_status, :approved_by_users, :approvals, :approval_feature_available?, :approved_by?
alias_method :approved_approvers, :approved_by_users
def initialize(merge_request, target_branch: nil)
@@ -149,7 +149,7 @@ def can_be_approved_by?(user)
return true if unactioned_approvers.include?(user)
# Users can only approve once.
- return false if approvals.where(user: user).any?
+ return false if approved_by?(user)
# At this point, follow self-approval rules. Otherwise authors must
# have been in the list of unactioned_approvers to have been approved.
return false if !authors_can_approve? && merge_request.author == user
--
GitLab