Commit b740ff9a authored by Igor Drozdov's avatar Igor Drozdov

Fix merge request approvals for EE without license

Call FOSS method when EE doesn't have a license
parent 497620d5
...@@ -13,4 +13,8 @@ module ApprovableBase ...@@ -13,4 +13,8 @@ module ApprovableBase
approved_by_users.include?(user) approved_by_users.include?(user)
end end
def can_be_approved_by?(user)
user && !approved_by?(user) && user.can?(:approve_merge_request, self)
end
end end
---
title: Fix merge request approvals for EE without a license
merge_request: 37246
author:
type: fixed
...@@ -114,6 +114,8 @@ class ApprovalState ...@@ -114,6 +114,8 @@ class ApprovalState
end end
def can_approve?(user) def can_approve?(user)
return merge_request.can_be_approved_by?(user) unless merge_request.approval_feature_available?
return false unless user return false unless user
return false unless user.can?(:approve_merge_request, merge_request) return false unless user.can?(:approve_merge_request, merge_request)
......
...@@ -760,6 +760,16 @@ RSpec.describe ApprovalState do ...@@ -760,6 +760,16 @@ RSpec.describe ApprovalState do
end end
end end
end end
context 'when approval feature is disabled' do
it 'delegates the call to merge request' do
stub_licensed_features(merge_request_approvers: false)
expect(merge_request).to receive(:can_be_approved_by?).with(approver1)
subject.can_approve?(approver1)
end
end
end end
describe '#authors_can_approve?' do describe '#authors_can_approve?' do
......
...@@ -8,8 +8,7 @@ module API ...@@ -8,8 +8,7 @@ module API
end end
expose :user_can_approve do |merge_request, options| expose :user_can_approve do |merge_request, options|
!merge_request.approved_by?(options[:current_user]) && merge_request.can_be_approved_by?(options[:current_user])
options[:current_user].can?(:approve_merge_request, merge_request)
end end
expose :approved do |merge_request| expose :approved do |merge_request|
......
...@@ -3,10 +3,10 @@ ...@@ -3,10 +3,10 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe ApprovableBase do RSpec.describe ApprovableBase do
describe '#approved_by?' do let(:merge_request) { create(:merge_request) }
let(:merge_request) { create(:merge_request) } let(:user) { create(:user) }
let(:user) { create(:user) }
describe '#approved_by?' do
subject { merge_request.approved_by?(user) } subject { merge_request.approved_by?(user) }
context 'when a user has not approved' do context 'when a user has not approved' do
...@@ -31,4 +31,32 @@ RSpec.describe ApprovableBase do ...@@ -31,4 +31,32 @@ RSpec.describe ApprovableBase do
end end
end end
end end
describe '#can_be_approved_by?' do
subject { merge_request.can_be_approved_by?(user) }
before do
merge_request.project.add_developer(user)
end
it 'returns true' do
is_expected.to be_truthy
end
context 'when a user has approved' do
let!(:approval) { create(:approval, merge_request: merge_request, user: user) }
it 'returns false' do
is_expected.to be_falsy
end
end
context 'when a user is nil' do
let(:user) { nil }
it 'returns false' do
is_expected.to be_falsy
end
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