From b09d7755c3e4cb656b5be49aa04246120a2352cc Mon Sep 17 00:00:00 2001
From: "Sincheol (David) Kim" <dkim@gitlab.com>
Date: Fri, 23 Sep 2022 15:12:40 +0000
Subject: [PATCH] Rename can_be_approved_by? to be more descriptive
`can_be_approved_by?` can be confusing since we have a policy check
called :approve_merge_request and it can be called like
`user.can?(:approve_merge_request, merge_request, user)`
---
app/models/concerns/approvable.rb | 4 +-
.../merge_request_noteable_entity.rb | 2 +-
.../merge_requests/approval_service.rb | 6 +-
ee/app/models/approval_state.rb | 2 +-
.../approval_wrapped_any_approver_rule.rb | 2 +-
ee/app/models/concerns/ee/approvable.rb | 6 +-
ee/lib/ee/api/entities/approval_state.rb | 2 +-
ee/spec/models/approval_state_spec.rb | 140 +++++++++---------
...approval_wrapped_any_approver_rule_spec.rb | 2 +-
ee/spec/models/ee/approvable_spec.rb | 10 +-
lib/api/entities/merge_request_approvals.rb | 2 +-
lib/api/merge_requests.rb | 2 +-
.../quick_actions/merge_request_actions.rb | 4 +-
spec/models/concerns/approvable_spec.rb | 8 +-
14 files changed, 96 insertions(+), 96 deletions(-)
diff --git a/app/models/concerns/approvable.rb b/app/models/concerns/approvable.rb
index 1566c53217d06..55e138d84fba5 100644
--- a/app/models/concerns/approvable.rb
+++ b/app/models/concerns/approvable.rb
@@ -50,11 +50,11 @@ def approved_by?(user)
approvals.where(user: user).any?
end
- def can_be_approved_by?(user)
+ def eligible_for_approval_by?(user)
user && !approved_by?(user) && user.can?(:approve_merge_request, self)
end
- def can_be_unapproved_by?(user)
+ def eligible_for_unapproval_by?(user)
user && approved_by?(user) && user.can?(:approve_merge_request, self)
end
end
diff --git a/app/serializers/merge_request_noteable_entity.rb b/app/serializers/merge_request_noteable_entity.rb
index 29bd26c3a15c1..07d7d19d1f3e1 100644
--- a/app/serializers/merge_request_noteable_entity.rb
+++ b/app/serializers/merge_request_noteable_entity.rb
@@ -51,7 +51,7 @@ class MergeRequestNoteableEntity < IssuableEntity
end
expose :can_approve do |merge_request|
- merge_request.can_be_approved_by?(current_user)
+ merge_request.eligible_for_approval_by?(current_user)
end
end
diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb
index 64ae33c9b1528..5761e34caffa4 100644
--- a/app/services/merge_requests/approval_service.rb
+++ b/app/services/merge_requests/approval_service.rb
@@ -3,7 +3,7 @@
module MergeRequests
class ApprovalService < MergeRequests::BaseService
def execute(merge_request)
- return unless can_be_approved?(merge_request)
+ return unless eligible_for_approval?(merge_request)
approval = merge_request.approvals.new(user: current_user)
@@ -28,8 +28,8 @@ def execute(merge_request)
private
- def can_be_approved?(merge_request)
- merge_request.can_be_approved_by?(current_user)
+ def eligible_for_approval?(merge_request)
+ merge_request.eligible_for_approval_by?(current_user)
end
def save_approval(approval)
diff --git a/ee/app/models/approval_state.rb b/ee/app/models/approval_state.rb
index c28ab346c74e3..116fc53f2966a 100644
--- a/ee/app/models/approval_state.rb
+++ b/ee/app/models/approval_state.rb
@@ -143,7 +143,7 @@ def suggested_approvers(current_user:)
filter_approvers(rules.flat_map(&:approvers), unactioned: true)
end
- def can_be_approved_by?(user)
+ def eligible_for_approval_by?(user)
return false unless user
return false unless user.can?(:approve_merge_request, merge_request)
diff --git a/ee/app/models/approval_wrapped_any_approver_rule.rb b/ee/app/models/approval_wrapped_any_approver_rule.rb
index 25da50ed17044..543b4bef90df5 100644
--- a/ee/app/models/approval_wrapped_any_approver_rule.rb
+++ b/ee/app/models/approval_wrapped_any_approver_rule.rb
@@ -19,7 +19,7 @@ def approved?
def commented_approvers
strong_memoize(:commented_approvers) do
merge_request.user_note_authors.select do |user|
- merge_request.can_be_approved_by?(user)
+ merge_request.eligible_for_approval_by?(user)
end
end
end
diff --git a/ee/app/models/concerns/ee/approvable.rb b/ee/app/models/concerns/ee/approvable.rb
index 01d2ae3fc6ea1..43ffd21d5e520 100644
--- a/ee/app/models/concerns/ee/approvable.rb
+++ b/ee/app/models/concerns/ee/approvable.rb
@@ -66,11 +66,11 @@ def approver_group_ids=(value)
end
end
- override :can_be_approved_by?
- def can_be_approved_by?(user)
+ override :eligible_for_approval_by?
+ def eligible_for_approval_by?(user)
return super unless approval_feature_available?
- approval_state.can_be_approved_by?(user)
+ approval_state.eligible_for_approval_by?(user)
end
end
end
diff --git a/ee/lib/ee/api/entities/approval_state.rb b/ee/lib/ee/api/entities/approval_state.rb
index 0a300a6e87392..84cf3cbd63f80 100644
--- a/ee/lib/ee/api/entities/approval_state.rb
+++ b/ee/lib/ee/api/entities/approval_state.rb
@@ -55,7 +55,7 @@ class ApprovalState < Grape::Entity
end
expose :user_can_approve do |approval_state, options|
- approval_state.can_be_approved_by?(options[:current_user])
+ approval_state.eligible_for_approval_by?(options[:current_user])
end
expose :approval_rules_left, using: ApprovalRuleShort
diff --git a/ee/spec/models/approval_state_spec.rb b/ee/spec/models/approval_state_spec.rb
index e1802db550eaa..41270ea0761f2 100644
--- a/ee/spec/models/approval_state_spec.rb
+++ b/ee/spec/models/approval_state_spec.rb
@@ -99,10 +99,10 @@ def users(amount)
end
shared_examples_for 'a MR that all members with write access can approve' do
- it { expect(subject.can_be_approved_by?(developer)).to be_truthy }
- it { expect(subject.can_be_approved_by?(reporter)).to be_falsey }
- it { expect(subject.can_be_approved_by?(stranger)).to be_falsey }
- it { expect(subject.can_be_approved_by?(nil)).to be_falsey }
+ it { expect(subject.eligible_for_approval_by?(developer)).to be_truthy }
+ it { expect(subject.eligible_for_approval_by?(reporter)).to be_falsey }
+ it { expect(subject.eligible_for_approval_by?(stranger)).to be_falsey }
+ it { expect(subject.eligible_for_approval_by?(nil)).to be_falsey }
end
shared_context 'project members' do
@@ -450,7 +450,7 @@ def create_unapproved_rule
end
end
- describe '#can_be_approved_by?' do
+ describe '#eligible_for_approval_by?' do
shared_examples_for 'authors self-approval authorization' do
context 'when authors are authorized to approve their own MRs' do
before do
@@ -458,13 +458,13 @@ def create_unapproved_rule
end
it 'allows the author to approve the MR if within the approvers list' do
- expect(subject.can_be_approved_by?(author)).to be_truthy
+ expect(subject.eligible_for_approval_by?(author)).to be_truthy
end
end
context 'when authors are not authorized to approve their own MRs' do
it 'does not allow the author to approve the MR' do
- expect(subject.can_be_approved_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
end
end
end
@@ -490,13 +490,13 @@ def create_unapproved_rule
end
it 'allows the author to approve the MR if within the approvers list' do
- expect(subject.can_be_approved_by?(author)).to be_truthy
+ expect(subject.eligible_for_approval_by?(author)).to be_truthy
end
it 'allows the author to approve the MR if not within the approvers list' do
allow(subject).to receive(:approvers).and_return([])
- expect(subject.can_be_approved_by?(author)).to be_truthy
+ expect(subject.eligible_for_approval_by?(author)).to be_truthy
end
context 'when the author has approved the MR already' do
@@ -505,7 +505,7 @@ def create_unapproved_rule
end
it 'does not allow the author to approve the MR again' do
- expect(subject.can_be_approved_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
end
end
end
@@ -518,13 +518,13 @@ def create_unapproved_rule
it 'allows the author to approve the MR if within the approvers list' do
allow(subject).to receive(:approvers).and_return([author])
- expect(subject.can_be_approved_by?(author)).to be_truthy
+ expect(subject.eligible_for_approval_by?(author)).to be_truthy
end
it 'does not allow the author to approve the MR if not within the approvers list' do
allow(subject).to receive(:approvers).and_return([])
- expect(subject.can_be_approved_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
end
end
@@ -536,13 +536,13 @@ def create_unapproved_rule
it 'allows the committer to approve the MR if within the approvers list' do
allow(subject).to receive(:approvers).and_return([committer])
- expect(subject.can_be_approved_by?(committer)).to be_truthy
+ expect(subject.eligible_for_approval_by?(committer)).to be_truthy
end
it 'allows the committer to approve the MR if not within the approvers list' do
allow(subject).to receive(:approvers).and_return([])
- expect(subject.can_be_approved_by?(committer)).to be_truthy
+ expect(subject.eligible_for_approval_by?(committer)).to be_truthy
end
context 'when the committer has approved the MR already' do
@@ -551,7 +551,7 @@ def create_unapproved_rule
end
it 'does not allow the committer to approve the MR again' do
- expect(subject.can_be_approved_by?(committer)).to be_falsey
+ expect(subject.eligible_for_approval_by?(committer)).to be_falsey
end
end
end
@@ -564,13 +564,13 @@ def create_unapproved_rule
it 'allows the committer to approve the MR if within the approvers list' do
allow(subject).to receive(:approvers).and_return([committer])
- expect(subject.can_be_approved_by?(committer)).to be_truthy
+ expect(subject.eligible_for_approval_by?(committer)).to be_truthy
end
it 'does not allow the committer to approve the MR if not within the approvers list' do
allow(subject).to receive(:approvers).and_return([])
- expect(subject.can_be_approved_by?(committer)).to be_falsey
+ expect(subject.eligible_for_approval_by?(committer)).to be_falsey
end
end
@@ -592,13 +592,13 @@ def create_unapproved_rule
it 'allows the user to approve the MR if within the approvers list' do
allow(subject).to receive(:approvers).and_return([user])
- expect(subject.can_be_approved_by?(user)).to be_truthy
+ expect(subject.eligible_for_approval_by?(user)).to be_truthy
end
it 'does not allow the user to approve the MR if not within the approvers list' do
allow(subject).to receive(:approvers).and_return([])
- expect(subject.can_be_approved_by?(user)).to be_falsey
+ expect(subject.eligible_for_approval_by?(user)).to be_falsey
end
end
@@ -613,13 +613,13 @@ def create_unapproved_rule
it 'allows the user to approve the MR if within the approvers list' do
allow(subject).to receive(:approvers).and_return([user])
- expect(subject.can_be_approved_by?(user)).to be_truthy
+ expect(subject.eligible_for_approval_by?(user)).to be_truthy
end
it 'does not allow the user to approve the MR if not within the approvers list' do
allow(subject).to receive(:approvers).and_return([])
- expect(subject.can_be_approved_by?(user)).to be_falsey
+ expect(subject.eligible_for_approval_by?(user)).to be_falsey
end
end
end
@@ -635,7 +635,7 @@ def create_unapproved_rule
it_behaves_like 'a MR that all members with write access can approve'
it 'does not allow a logged-out user to approve the MR' do
- expect(subject.can_be_approved_by?(nil)).to be_falsey
+ expect(subject.eligible_for_approval_by?(nil)).to be_falsey
end
it 'is not approved' do
@@ -660,11 +660,11 @@ def create_unapproved_rule
end
it 'allows any other other approver to approve the MR' do
- expect(subject.can_be_approved_by?(approver)).to be_truthy
+ expect(subject.eligible_for_approval_by?(approver)).to be_truthy
end
it 'does not allow a logged-out user to approve the MR' do
- expect(subject.can_be_approved_by?(nil)).to be_falsey
+ expect(subject.eligible_for_approval_by?(nil)).to be_falsey
end
context 'when self-approval is disabled and all of the valid approvers have approved the MR' do
@@ -680,12 +680,12 @@ def create_unapproved_rule
end
it 'does not allow the author to approve the MR' do
- expect(subject.can_be_approved_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
end
it 'does not allow the approvers to approve the MR again' do
- expect(subject.can_be_approved_by?(approver)).to be_falsey
- expect(subject.can_be_approved_by?(approver2)).to be_falsey
+ expect(subject.eligible_for_approval_by?(approver)).to be_falsey
+ expect(subject.eligible_for_approval_by?(approver2)).to be_falsey
end
end
@@ -701,14 +701,14 @@ def create_unapproved_rule
end
it 'does not allow the approvers to approve the MR again' do
- expect(subject.can_be_approved_by?(author)).to be_falsey
- expect(subject.can_be_approved_by?(approver2)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(approver2)).to be_falsey
end
it 'allows any other project member with write access to approve the MR' do
- expect(subject.can_be_approved_by?(reporter)).to be_falsey
- expect(subject.can_be_approved_by?(stranger)).to be_falsey
- expect(subject.can_be_approved_by?(nil)).to be_falsey
+ expect(subject.eligible_for_approval_by?(reporter)).to be_falsey
+ expect(subject.eligible_for_approval_by?(stranger)).to be_falsey
+ expect(subject.eligible_for_approval_by?(nil)).to be_falsey
end
end
@@ -739,14 +739,14 @@ def create_unapproved_rule
end
it 'allows anyone with write access except for author to approve the MR' do
- expect(subject.can_be_approved_by?(developer)).to be_truthy
- expect(subject.can_be_approved_by?(approver)).to be_truthy
- expect(subject.can_be_approved_by?(approver2)).to be_truthy
+ expect(subject.eligible_for_approval_by?(developer)).to be_truthy
+ expect(subject.eligible_for_approval_by?(approver)).to be_truthy
+ expect(subject.eligible_for_approval_by?(approver2)).to be_truthy
- expect(subject.can_be_approved_by?(author)).to be_falsey
- expect(subject.can_be_approved_by?(reporter)).to be_falsey
- expect(subject.can_be_approved_by?(stranger)).to be_falsey
- expect(subject.can_be_approved_by?(nil)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(reporter)).to be_falsey
+ expect(subject.eligible_for_approval_by?(stranger)).to be_falsey
+ expect(subject.eligible_for_approval_by?(nil)).to be_falsey
end
end
end
@@ -1173,7 +1173,7 @@ def create_rules
end
end
- describe '#can_be_approved_by?' do
+ describe '#eligible_for_approval_by?' do
shared_examples_for 'authors self-approval authorization' do
context 'when authors are authorized to approve their own MRs' do
before do
@@ -1181,13 +1181,13 @@ def create_rules
end
it 'allows the author to approve the MR if within the approvers list' do
- expect(subject.can_be_approved_by?(author)).to be_truthy
+ expect(subject.eligible_for_approval_by?(author)).to be_truthy
end
end
context 'when authors are not authorized to approve their own MRs' do
it 'does not allow the author to approve the MR' do
- expect(subject.can_be_approved_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
end
end
end
@@ -1205,13 +1205,13 @@ def create_rules
it 'returns true when authors can approve' do
project.update!(merge_requests_author_approval: true)
- expect(subject.can_be_approved_by?(author)).to be true
+ expect(subject.eligible_for_approval_by?(author)).to be true
end
it 'returns false when authors cannot approve' do
project.update!(merge_requests_author_approval: false)
- expect(subject.can_be_approved_by?(author)).to be false
+ expect(subject.eligible_for_approval_by?(author)).to be false
end
end
@@ -1219,13 +1219,13 @@ def create_rules
it 'returns true when authors can approve' do
project.update!(merge_requests_author_approval: true)
- expect(subject.can_be_approved_by?(author)).to be true
+ expect(subject.eligible_for_approval_by?(author)).to be true
end
it 'returns false when authors cannot approve' do
project.update!(merge_requests_author_approval: false)
- expect(subject.can_be_approved_by?(author)).to be false
+ expect(subject.eligible_for_approval_by?(author)).to be false
end
end
end
@@ -1245,13 +1245,13 @@ def create_rules
it 'return true when committers can approve' do
project.update!(merge_requests_disable_committers_approval: false)
- expect(subject.can_be_approved_by?(user)).to be true
+ expect(subject.eligible_for_approval_by?(user)).to be true
end
it 'return false when committers cannot approve' do
project.update!(merge_requests_disable_committers_approval: true)
- expect(subject.can_be_approved_by?(user)).to be false
+ expect(subject.eligible_for_approval_by?(user)).to be false
end
end
@@ -1259,13 +1259,13 @@ def create_rules
it 'return true when committers can approve' do
project.update!(merge_requests_disable_committers_approval: false)
- expect(subject.can_be_approved_by?(user)).to be true
+ expect(subject.eligible_for_approval_by?(user)).to be true
end
it 'return false when committers cannot approve' do
project.update!(merge_requests_disable_committers_approval: true)
- expect(subject.can_be_approved_by?(user)).to be false
+ expect(subject.eligible_for_approval_by?(user)).to be false
end
end
end
@@ -1284,7 +1284,7 @@ def create_rules
end
it 'does not allow a logged-out user to approve the MR' do
- expect(subject.can_be_approved_by?(nil)).to be_falsey
+ expect(subject.eligible_for_approval_by?(nil)).to be_falsey
end
it 'is not approved' do
@@ -1308,11 +1308,11 @@ def create_rules
end
it 'allows any other other approver to approve the MR' do
- expect(subject.can_be_approved_by?(approver)).to be_truthy
+ expect(subject.eligible_for_approval_by?(approver)).to be_truthy
end
it 'does not allow a logged-out user to approve the MR' do
- expect(subject.can_be_approved_by?(nil)).to be_falsey
+ expect(subject.eligible_for_approval_by?(nil)).to be_falsey
end
context 'when self-approval is disabled and all of the valid approvers have approved the MR' do
@@ -1328,12 +1328,12 @@ def create_rules
end
it 'does not allow the author to approve the MR' do
- expect(subject.can_be_approved_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
end
it 'does not allow the approvers to approve the MR again' do
- expect(subject.can_be_approved_by?(approver)).to be_falsey
- expect(subject.can_be_approved_by?(approver2)).to be_falsey
+ expect(subject.eligible_for_approval_by?(approver)).to be_falsey
+ expect(subject.eligible_for_approval_by?(approver2)).to be_falsey
end
end
@@ -1349,14 +1349,14 @@ def create_rules
end
it 'does not allow the approvers to approve the MR again' do
- expect(subject.can_be_approved_by?(author)).to be_falsey
- expect(subject.can_be_approved_by?(approver2)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(approver2)).to be_falsey
end
it 'allows any other project member with write access to approve the MR' do
- expect(subject.can_be_approved_by?(reporter)).to be_falsey
- expect(subject.can_be_approved_by?(stranger)).to be_falsey
- expect(subject.can_be_approved_by?(nil)).to be_falsey
+ expect(subject.eligible_for_approval_by?(reporter)).to be_falsey
+ expect(subject.eligible_for_approval_by?(stranger)).to be_falsey
+ expect(subject.eligible_for_approval_by?(nil)).to be_falsey
end
end
@@ -1387,15 +1387,15 @@ def create_rules
end
it 'allows anyone with write access except for author to approve the MR' do
- expect(subject.can_be_approved_by?(developer)).to be_truthy
- expect(subject.can_be_approved_by?(approver)).to be_truthy
- expect(subject.can_be_approved_by?(approver2)).to be_truthy
+ expect(subject.eligible_for_approval_by?(developer)).to be_truthy
+ expect(subject.eligible_for_approval_by?(approver)).to be_truthy
+ expect(subject.eligible_for_approval_by?(approver2)).to be_truthy
- expect(subject.can_be_approved_by?(author)).to be_falsey
- expect(subject.can_be_approved_by?(reporter)).to be_falsey
- expect(subject.can_be_approved_by?(guest)).to be_falsey
- expect(subject.can_be_approved_by?(stranger)).to be_falsey
- expect(subject.can_be_approved_by?(nil)).to be_falsey
+ expect(subject.eligible_for_approval_by?(author)).to be_falsey
+ expect(subject.eligible_for_approval_by?(reporter)).to be_falsey
+ expect(subject.eligible_for_approval_by?(guest)).to be_falsey
+ expect(subject.eligible_for_approval_by?(stranger)).to be_falsey
+ expect(subject.eligible_for_approval_by?(nil)).to be_falsey
end
context 'when an approver does not have access to the merge request', :sidekiq_inline do
@@ -1404,7 +1404,7 @@ def create_rules
end
it 'the user cannot approver' do
- expect(subject.can_be_approved_by?(developer)).to be_falsey
+ expect(subject.eligible_for_approval_by?(developer)).to be_falsey
end
end
end
diff --git a/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb b/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb
index d38299120a52c..72b846dd755cd 100644
--- a/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb
+++ b/ee/spec/models/approval_wrapped_any_approver_rule_spec.rb
@@ -57,7 +57,7 @@
create(:note, project: merge_request.project, noteable: merge_request, author: approver1)
create(:system_note, project: merge_request.project, noteable: merge_request, author: approver2)
- allow(merge_request).to receive(:can_be_approved_by?).and_return(true)
+ allow(merge_request).to receive(:eligible_for_approval_by?).and_return(true)
expect(subject.commented_approvers).to include(approver1)
expect(subject.commented_approvers).not_to include(approver2)
diff --git a/ee/spec/models/ee/approvable_spec.rb b/ee/spec/models/ee/approvable_spec.rb
index f58c2a2f3c2ab..fcd9d889a10f4 100644
--- a/ee/spec/models/ee/approvable_spec.rb
+++ b/ee/spec/models/ee/approvable_spec.rb
@@ -48,16 +48,16 @@
it { is_expected.to delegate_method(method).to(:approval_state) }
end
- describe '#can_be_approved_by?' do
+ describe '#eligible_for_approval_by?' do
context 'when merge_request_approvers feature is enabled' do
before do
stub_licensed_features(merge_request_approvers: true)
end
it 'delegates the call to merge request' do
- expect(subject.approval_state).to receive(:can_be_approved_by?).with(author)
+ expect(subject.approval_state).to receive(:eligible_for_approval_by?).with(author)
- subject.can_be_approved_by?(author)
+ subject.eligible_for_approval_by?(author)
end
end
@@ -67,9 +67,9 @@
end
it 'delegates the call to merge request' do
- expect(subject.approval_state).not_to receive(:can_be_approved_by?).with(author)
+ expect(subject.approval_state).not_to receive(:eligible_for_approval_by?).with(author)
- subject.can_be_approved_by?(author)
+ subject.eligible_for_approval_by?(author)
end
end
end
diff --git a/lib/api/entities/merge_request_approvals.rb b/lib/api/entities/merge_request_approvals.rb
index 0c464844ae7eb..6810952b2fcfa 100644
--- a/lib/api/entities/merge_request_approvals.rb
+++ b/lib/api/entities/merge_request_approvals.rb
@@ -8,7 +8,7 @@ class MergeRequestApprovals < Grape::Entity
end
expose :user_can_approve do |merge_request, options|
- merge_request.can_be_approved_by?(options[:current_user])
+ merge_request.eligible_for_approval_by?(options[:current_user])
end
expose :approved do |merge_request|
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 1dc0e1f0d2213..800954a4311e4 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -560,7 +560,7 @@ def recheck_mergeability_of(merge_requests:)
put ':id/merge_requests/:merge_request_iid/reset_approvals', feature_category: :code_review, urgency: :low do
merge_request = find_project_merge_request(params[:merge_request_iid])
- unauthorized! unless current_user.bot? && merge_request.can_be_approved_by?(current_user)
+ unauthorized! unless current_user.bot? && merge_request.eligible_for_approval_by?(current_user)
merge_request.approvals.delete_all
diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb
index 3cb01db1491c6..e68c5fdb11bee 100644
--- a/lib/gitlab/quick_actions/merge_request_actions.rb
+++ b/lib/gitlab/quick_actions/merge_request_actions.rb
@@ -188,7 +188,7 @@ module MergeRequestActions
explanation { _('Approve the current merge request.') }
types MergeRequest
condition do
- quick_action_target.persisted? && quick_action_target.can_be_approved_by?(current_user)
+ quick_action_target.persisted? && quick_action_target.eligible_for_approval_by?(current_user)
end
command :approve do
success = ::MergeRequests::ApprovalService.new(project: quick_action_target.project, current_user: current_user).execute(quick_action_target)
@@ -202,7 +202,7 @@ module MergeRequestActions
explanation { _('Unapprove the current merge request.') }
types MergeRequest
condition do
- quick_action_target.persisted? && quick_action_target.can_be_unapproved_by?(current_user)
+ quick_action_target.persisted? && quick_action_target.eligible_for_unapproval_by?(current_user)
end
command :unapprove do
success = ::MergeRequests::RemoveApprovalService.new(project: quick_action_target.project, current_user: current_user).execute(quick_action_target)
diff --git a/spec/models/concerns/approvable_spec.rb b/spec/models/concerns/approvable_spec.rb
index 1ddd9b3edca8f..25a4f51cd8261 100644
--- a/spec/models/concerns/approvable_spec.rb
+++ b/spec/models/concerns/approvable_spec.rb
@@ -32,8 +32,8 @@
end
end
- describe '#can_be_approved_by?' do
- subject { merge_request.can_be_approved_by?(user) }
+ describe '#eligible_for_approval_by?' do
+ subject { merge_request.eligible_for_approval_by?(user) }
before do
merge_request.project.add_developer(user) if user
@@ -60,8 +60,8 @@
end
end
- describe '#can_be_unapproved_by?' do
- subject { merge_request.can_be_unapproved_by?(user) }
+ describe '#eligible_for_unapproval_by?' do
+ subject { merge_request.eligible_for_unapproval_by?(user) }
before do
merge_request.project.add_developer(user) if user
--
GitLab