diff --git a/app/models/concerns/approvable.rb b/app/models/concerns/approvable.rb index 1566c53217d0620686982fbfab89622b841eaf7c..55e138d84fba53f997bf0250d934bca422b78f12 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 29bd26c3a15c14b76b8f3afeb21c8dcc2fe0764a..07d7d19d1f3e12a4745458557148b56d0d9bac65 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 64ae33c9b1528aa5603a84a3262bf325fd7ba6c4..5761e34caffa47517dd5e3dd71187705c43f8e35 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 c28ab346c74e3e17a58e7c05f89d3174629e2504..116fc53f2966ad36993a2701fd176e92cd05ca31 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 25da50ed17044de3c115e38697d52492d047a7fa..543b4bef90df52d0c1e832402e9a7c926641db57 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 01d2ae3fc6ea18aac1cbd722c6c0c5f42e63721f..43ffd21d5e52097307a2149377171f2f3facedb9 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 0a300a6e87392f0725a58ad40dffa36c5b41269a..84cf3cbd63f80e82313f99b8ae490134e567cd9e 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 e1802db550eaa2ccddfa27c44adc46395f84914f..41270ea0761f28f698df5adc9b76f2dbbd8b2a00 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 d38299120a52c4ca89e2527df2f01adbde3774bd..72b846dd755cd4dfb3416439d1d07238caf22dc6 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 f58c2a2f3c2ab7067743e8bfb20100ee959c52aa..fcd9d889a10f48bfdac2a41030496cb3d127d748 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 0c464844ae7ebf8077b5344c1d4b10693a123c16..6810952b2fcfa1896460d7ae2d028ca74597afa2 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 1dc0e1f0d2213f5f743daa858e696262a3d336d0..800954a4311e439566fc9614266990e69155f30a 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 3cb01db1491c6990627a5e1798b5b14ebf106c98..e68c5fdb11beebd1149e3b0be449a2e7d72991b2 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 1ddd9b3edca8f3ce79f5b0d0862fa6caf522d6bb..25a4f51cd826142ad90e6c66870b771ec5b25677 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