Merge branch 'id-fix-approvals-for-ee-without-license' into 'master'

[RUN AS-IF-FOSS] Fix merge request approvals for EE without license

See merge request gitlab-org/gitlab!37246

app/models/concerns/approvable_base.rb

@@ -13,4 +13,8 @@ module ApprovableBase
 
     approved_by_users.include?(user)
   end
+
+  def can_be_approved_by?(user)
+    user && !approved_by?(user) && user.can?(:approve_merge_request, self)
+  end
 end

changelogs/unreleased/id-fix-approvals-for-ee-without-license.yml

+---
+title: Fix merge request approvals for EE without a license
+merge_request: 37246
+author:
+type: fixed

ee/app/models/approval_state.rb

@@ -114,6 +114,8 @@ class ApprovalState
   end
 
   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.can?(:approve_merge_request, merge_request)
 

ee/spec/models/approval_state_spec.rb

@@ -760,6 +760,16 @@ RSpec.describe ApprovalState do
           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
 
     describe '#authors_can_approve?' do

lib/api/entities/merge_request_approvals.rb

@@ -8,8 +8,7 @@ module API
       end
 
       expose :user_can_approve do |merge_request, options|
-        !merge_request.approved_by?(options[:current_user]) &&
-          options[:current_user].can?(:approve_merge_request, merge_request)
+        merge_request.can_be_approved_by?(options[:current_user])
       end
 
       expose :approved do |merge_request|

spec/models/concerns/approvable_base_spec.rb

@@ -3,10 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe ApprovableBase do
-  describe '#approved_by?' do
-    let(:merge_request) { create(:merge_request) }
-    let(:user) { create(:user) }
+  let(:merge_request) { create(:merge_request) }
+  let(:user) { create(:user) }
 
+  describe '#approved_by?' do
     subject { merge_request.approved_by?(user) }
 
     context 'when a user has not approved' do
@@ -31,4 +31,32 @@ RSpec.describe ApprovableBase do
       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