Merge branch '233736-display-which-approval-rules-match-a-given-reviewer-backend' into 'master'

Allow applicable approval rules to be included for reviewers

See merge request gitlab-org/gitlab!46855

app/controllers/projects/merge_requests_controller.rb

@@ -99,7 +99,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
         @noteable = @merge_request
         @commits_count = @merge_request.commits_count + @merge_request.context_commits_count
         @issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar')
-        @current_user_data = UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json
+        @current_user_data = UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestCurrentUserEntity).to_json
         @show_whitespace_default = current_user.nil? || current_user.show_whitespace_in_diffs
         @file_by_file_default = Feature.enabled?(:view_diffs_file_by_file, default_enabled: true) && current_user&.view_diffs_file_by_file
         @coverage_path = coverage_reports_project_merge_request_path(@project, @merge_request, format: :json) if @merge_request.has_coverage_reports?

app/serializers/merge_request_assignee_entity.rb

-# frozen_string_literal: true
-
-class MergeRequestAssigneeEntity < ::API::Entities::UserBasic
-  expose :can_merge do |assignee, options|
-    options[:merge_request]&.can_be_merged_by?(assignee)
-  end
-end
-
-MergeRequestAssigneeEntity.prepend_if_ee('EE::MergeRequestAssigneeEntity')

app/serializers/merge_request_current_user_entity.rb

+# frozen_string_literal: true
+
+class MergeRequestCurrentUserEntity < CurrentUserEntity
+  include RequestAwareEntity
+  include BlobHelper
+  include TreeHelper
+
+  expose :can_fork do |user|
+    project && can?(user, :fork_project, request.project)
+  end
+
+  expose :can_create_merge_request do |user|
+    project && can?(user, :create_merge_request_in, project)
+  end
+
+  expose :fork_path, if: -> (*) { project } do |user|
+    params = edit_blob_fork_params("Edit")
+    project_forks_path(project, namespace_key: user.namespace.id, continue: params)
+  end
+
+  def project
+    request.respond_to?(:project) && request.project
+  end
+end

app/serializers/merge_request_reviewer_entity.rb

-# frozen_string_literal: true
-
-class MergeRequestReviewerEntity < ::API::Entities::UserBasic
-  expose :can_merge do |reviewer, options|
-    options[:merge_request]&.can_be_merged_by?(reviewer)
-  end
-end
-
-MergeRequestReviewerEntity.prepend_if_ee('EE::MergeRequestReviewerEntity')

app/serializers/merge_request_sidebar_extras_entity.rb

@@ -2,10 +2,10 @@
 
 class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity
   expose :assignees do |merge_request|
-    MergeRequestAssigneeEntity.represent(merge_request.assignees, merge_request: merge_request)
+    MergeRequestUserEntity.represent(merge_request.assignees, merge_request: merge_request)
   end
 
   expose :reviewers, if: -> (m) { m.allows_reviewers? } do |merge_request|
-    MergeRequestReviewerEntity.represent(merge_request.reviewers, merge_request: merge_request)
+    MergeRequestUserEntity.represent(merge_request.reviewers, merge_request: merge_request)
   end
 end

app/serializers/merge_request_user_entity.rb

 # frozen_string_literal: true
 
-class MergeRequestUserEntity < CurrentUserEntity
-  include RequestAwareEntity
-  include BlobHelper
-  include TreeHelper
-
-  expose :can_fork do |user|
-    can?(user, :fork_project, request.project) if project
-  end
-
-  expose :can_create_merge_request do |user|
-    project && can?(user, :create_merge_request_in, project)
-  end
-
-  expose :fork_path, if: -> (*) { project } do |user|
-    params = edit_blob_fork_params("Edit")
-    project_forks_path(project, namespace_key: user.namespace.id, continue: params)
-  end
-
-  def project
-    return false unless request.respond_to?(:project) && request.project
-
-    request.project
+class MergeRequestUserEntity < ::API::Entities::UserBasic
+  expose :can_merge do |reviewer, options|
+    options[:merge_request]&.can_be_merged_by?(reviewer)
   end
 end
+
+MergeRequestUserEntity.prepend_if_ee('EE::MergeRequestUserEntity')

app/serializers/user_entity.rb

@@ -2,3 +2,5 @@
 
 class UserEntity < API::Entities::UserPath
 end
+
+UserEntity.prepend_if_ee('EE::UserEntity')

app/serializers/user_serializer.rb

@@ -8,7 +8,7 @@ class UserSerializer < BaseSerializer
       merge_request = opts[:project].merge_requests.find_by_iid!(params[:merge_request_iid])
       preload_max_member_access(merge_request.project, Array(resource))
 
-      super(resource, opts.merge(merge_request: merge_request), MergeRequestAssigneeEntity)
+      super(resource, opts.merge(merge_request: merge_request), MergeRequestUserEntity)
     else
       super
     end
@@ -20,3 +20,5 @@ class UserSerializer < BaseSerializer
     project.team.max_member_access_for_user_ids(users.map(&:id))
   end
 end
+
+UserSerializer.prepend_if_ee('EE::UserSerializer')

ee/app/models/ee/list.rb

@@ -67,7 +67,7 @@ module EE
     def as_json(options = {})
       super.tap do |json|
         if options.key?(:user)
-          json[:user] = UserSerializer.new.represent(user).as_json
+          json[:user] = ::UserSerializer.new.represent(user).as_json
         end
 
         if options.key?(:milestone)

ee/app/models/ee/merge_request.rb

@@ -274,6 +274,12 @@ module EE
       (base_pipeline.security_scans.pluck(:scan_type) - actual_head_pipeline.security_scans.pluck(:scan_type)).uniq
     end
 
+    def applicable_approval_rules_for_user(user_id)
+      approval_rules.applicable_to_branch(target_branch).select do |rule|
+        rule.approvers.pluck(:id).include?(user_id)
+      end
+    end
+
     private
 
     def has_approved_license_check?

ee/app/models/ee/project.rb

@@ -432,6 +432,12 @@ module EE
       super
     end
 
+    def applicable_approval_rules_for_user(user_id, target_branch = nil)
+      visible_approval_rules(target_branch: target_branch).select do |rule|
+        rule.approvers.pluck(:id).include?(user_id)
+      end
+    end
+
     def visible_approval_rules(target_branch: nil)
       rules = strong_memoize(:visible_approval_rules) do
         Hash.new do |h, key|

ee/app/serializers/ee/merge_request_reviewer_entity.rb

-# frozen_string_literal: true
-
-module EE
-  module MergeRequestReviewerEntity
-    extend ActiveSupport::Concern
-
-    prepended do
-      expose :gitlab_employee?, as: :is_gitlab_employee, if: proc { ::Gitlab.com? && ::Feature.enabled?(:gitlab_employee_badge) }
-    end
-  end
-end

ee/app/serializers/ee/merge_request_assignee_entity.rb → ee/app/serializers/ee/merge_request_user_entity.rb

 # frozen_string_literal: true
 
 module EE
-  module MergeRequestAssigneeEntity
+  module MergeRequestUserEntity
     extend ActiveSupport::Concern
 
     prepended do
       expose :gitlab_employee?, as: :is_gitlab_employee, if: proc { ::Gitlab.com? && ::Feature.enabled?(:gitlab_employee_badge) }
+      expose :applicable_approval_rules, if: proc { |_, options| options[:project]&.feature_available?(:merge_request_approvers) && options[:approval_rules] }, using: ::EE::API::Entities::ApprovalRuleShort do |user, options|
+        options[:merge_request]&.applicable_approval_rules_for_user(user.id)
+      end
     end
   end
 end

ee/app/serializers/ee/user_entity.rb

+# frozen_string_literal: true
+
+module EE
+  module UserEntity
+    extend ActiveSupport::Concern
+
+    prepended do
+      expose :applicable_approval_rules, if: proc { |_, options| options[:project]&.feature_available?(:merge_request_approvers) && options[:approval_rules] }, using: ::EE::API::Entities::ApprovalRuleShort do |user, options|
+        options[:project]&.applicable_approval_rules_for_user(user.id, options[:target_branch])
+      end
+    end
+  end
+end

ee/app/serializers/ee/user_serializer.rb

+# frozen_string_literal: true
+
+module EE
+  module UserSerializer
+    extend ::Gitlab::Utils::Override
+
+    override :represent
+    def represent(resource, opts = {}, entity = nil)
+      opts = opts.merge(approval_rules: params[:approval_rules] == 'true') if params[:approval_rules]
+      opts = opts.merge(target_branch: params[:target_branch]) if params[:target_branch]
+
+      super(resource, opts, entity)
+    end
+  end
+end

ee/spec/serializers/ee/user_serializer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe UserSerializer do
+  let_it_be(:user1) { create(:user) }
+  let_it_be(:user2) { create(:user) }
+  let_it_be(:merge_request) { create(:merge_request) }
+  let_it_be(:project) { merge_request.project }
+
+  let(:serializer) { described_class.new(options) }
+
+  shared_examples 'user without applicable_approval_rules' do
+    it 'returns a user without applicable_approval_rules' do
+      serialized_user1, serialized_user2 = serializer.represent([user1, user2], project: project).as_json
+
+      expect(serialized_user1.keys).not_to include('applicable_approval_rules')
+      expect(serialized_user2.keys).not_to include('applicable_approval_rules')
+    end
+  end
+
+  before do
+    stub_licensed_features(multiple_approval_rules: true)
+  end
+
+  context 'with merge_request_iid' do
+    let(:options) { { merge_request_iid: merge_request.iid } }
+
+    context 'without approval_rules' do
+      it_behaves_like 'user without applicable_approval_rules'
+    end
+
+    context 'with approval_rules' do
+      let(:options) { super().merge(approval_rules: 'true') }
+
+      let!(:approval_merge_request_rule) do
+        create(:approval_merge_request_rule, name: 'Merge Request Rule', merge_request: merge_request, users: [user1])
+      end
+
+      it 'returns users with applicable_approval_rules' do
+        serialized_user1, serialized_user2 = serializer.represent([user1, user2], project: project).as_json
+
+        expect(serialized_user1).to include(
+          'id' => user1.id,
+          'applicable_approval_rules' => [
+            { 'id' => approval_merge_request_rule.id, 'name' => 'Merge Request Rule', 'rule_type' => 'regular' }
+          ]
+        )
+        expect(serialized_user2).to include('id' => user2.id, 'applicable_approval_rules' => [])
+      end
+    end
+  end
+
+  context 'without merge_request_iid' do
+    let(:options) { {} }
+
+    context 'wsee/spec/serializers/ee/user_serializer_spec.rbthout approval_rules' do
+      it_behaves_like 'user without applicable_approval_rules'
+    end
+
+    context 'with approval_rules' do
+      let(:options) { super().merge(approval_rules: 'true') }
+
+      let!(:protected_branch) { create(:protected_branch, project: project, name: 'my_branch') }
+      let!(:approval_project_rule) do
+        create(:approval_project_rule, name: 'Project Rule', project: project, users: [user1], protected_branches: [protected_branch])
+      end
+
+      it 'returns users with applicable_approval_rules' do
+        serialized_user1, serialized_user2 = serializer.represent([user1, user2], project: project).as_json
+
+        expect(serialized_user1).to include(
+          'id' => user1.id,
+          'applicable_approval_rules' => [
+            { 'id' => approval_project_rule.id, 'name' => 'Project Rule', 'rule_type' => 'regular' }
+          ]
+        )
+        expect(serialized_user2).to include('id' => user2.id, 'applicable_approval_rules' => [])
+      end
+
+      context 'with target_branch' do
+        let(:options) { super().merge(target_branch: 'my_branch') }
+
+        it 'returns users with applicable_approval_rules' do
+          serialized_user1, serialized_user2 = serializer.represent([user1, user2], project: project).as_json
+
+          expect(serialized_user1).to include(
+            'id' => user1.id,
+            'applicable_approval_rules' => [
+              { 'id' => approval_project_rule.id, 'name' => 'Project Rule', 'rule_type' => 'regular' }
+            ]
+          )
+          expect(serialized_user2).to include('id' => user2.id, 'applicable_approval_rules' => [])
+        end
+      end
+
+      context 'with unknown target_branch' do
+        let(:options) { super().merge(target_branch: 'unknown_branch') }
+
+        it 'returns users with applicable_approval_rules' do
+          serialized_user1, serialized_user2 = serializer.represent([user1, user2], project: project).as_json
+
+          expect(serialized_user1).to include('id' => user1.id, 'applicable_approval_rules' => [])
+          expect(serialized_user2).to include('id' => user2.id, 'applicable_approval_rules' => [])
+        end
+      end
+    end
+  end
+end

spec/serializers/merge_request_current_user_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequestCurrentUserEntity do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :repository) }
+  let(:request) { EntityRequest.new(project: project, current_user: user) }
+
+  let(:entity) do
+    described_class.new(user, request: request)
+  end
+
+  context 'as json' do
+    subject { entity.as_json }
+
+    it 'exposes needed attributes' do
+      expect(subject).to include(:can_fork, :can_create_merge_request, :fork_path)
+    end
+  end
+end

spec/serializers/merge_request_user_entity_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe MergeRequestUserEntity do
     subject { entity.as_json }
 
     it 'exposes needed attributes' do
-      expect(subject).to include(:can_fork, :can_create_merge_request, :fork_path)
+      expect(subject).to include(:id, :name, :username, :state, :avatar_url, :web_url, :can_merge)
     end
   end
 end