Commit 8f926dc9 authored by David Kim's avatar David Kim Committed by Kerri Miller

Allow applicable approval rules to be included for reviewers

parent 667edbac
...@@ -99,7 +99,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -99,7 +99,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@noteable = @merge_request @noteable = @merge_request
@commits_count = @merge_request.commits_count + @merge_request.context_commits_count @commits_count = @merge_request.commits_count + @merge_request.context_commits_count
@issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar') @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 @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 @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? @coverage_path = coverage_reports_project_merge_request_path(@project, @merge_request, format: :json) if @merge_request.has_coverage_reports?
......
# 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')
# 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
# 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')
...@@ -2,10 +2,10 @@ ...@@ -2,10 +2,10 @@
class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity
expose :assignees do |merge_request| expose :assignees do |merge_request|
MergeRequestAssigneeEntity.represent(merge_request.assignees, merge_request: merge_request) MergeRequestUserEntity.represent(merge_request.assignees, merge_request: merge_request)
end end
expose :reviewers, if: -> (m) { m.allows_reviewers? } do |merge_request| 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
end end
# frozen_string_literal: true # frozen_string_literal: true
class MergeRequestUserEntity < CurrentUserEntity class MergeRequestUserEntity < ::API::Entities::UserBasic
include RequestAwareEntity expose :can_merge do |reviewer, options|
include BlobHelper options[:merge_request]&.can_be_merged_by?(reviewer)
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
end end
end end
MergeRequestUserEntity.prepend_if_ee('EE::MergeRequestUserEntity')
...@@ -2,3 +2,5 @@ ...@@ -2,3 +2,5 @@
class UserEntity < API::Entities::UserPath class UserEntity < API::Entities::UserPath
end end
UserEntity.prepend_if_ee('EE::UserEntity')
...@@ -8,7 +8,7 @@ class UserSerializer < BaseSerializer ...@@ -8,7 +8,7 @@ class UserSerializer < BaseSerializer
merge_request = opts[:project].merge_requests.find_by_iid!(params[:merge_request_iid]) merge_request = opts[:project].merge_requests.find_by_iid!(params[:merge_request_iid])
preload_max_member_access(merge_request.project, Array(resource)) 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 else
super super
end end
...@@ -20,3 +20,5 @@ class UserSerializer < BaseSerializer ...@@ -20,3 +20,5 @@ class UserSerializer < BaseSerializer
project.team.max_member_access_for_user_ids(users.map(&:id)) project.team.max_member_access_for_user_ids(users.map(&:id))
end end
end end
UserSerializer.prepend_if_ee('EE::UserSerializer')
...@@ -67,7 +67,7 @@ module EE ...@@ -67,7 +67,7 @@ module EE
def as_json(options = {}) def as_json(options = {})
super.tap do |json| super.tap do |json|
if options.key?(:user) if options.key?(:user)
json[:user] = UserSerializer.new.represent(user).as_json json[:user] = ::UserSerializer.new.represent(user).as_json
end end
if options.key?(:milestone) if options.key?(:milestone)
......
...@@ -274,6 +274,12 @@ module EE ...@@ -274,6 +274,12 @@ module EE
(base_pipeline.security_scans.pluck(:scan_type) - actual_head_pipeline.security_scans.pluck(:scan_type)).uniq (base_pipeline.security_scans.pluck(:scan_type) - actual_head_pipeline.security_scans.pluck(:scan_type)).uniq
end 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 private
def has_approved_license_check? def has_approved_license_check?
......
...@@ -432,6 +432,12 @@ module EE ...@@ -432,6 +432,12 @@ module EE
super super
end 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) def visible_approval_rules(target_branch: nil)
rules = strong_memoize(:visible_approval_rules) do rules = strong_memoize(:visible_approval_rules) do
Hash.new do |h, key| Hash.new do |h, key|
......
# 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
# frozen_string_literal: true # frozen_string_literal: true
module EE module EE
module MergeRequestAssigneeEntity module MergeRequestUserEntity
extend ActiveSupport::Concern extend ActiveSupport::Concern
prepended do prepended do
expose :gitlab_employee?, as: :is_gitlab_employee, if: proc { ::Gitlab.com? && ::Feature.enabled?(:gitlab_employee_badge) } 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 end
end end
# 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
# 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
# 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
# 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
...@@ -15,7 +15,7 @@ RSpec.describe MergeRequestUserEntity do ...@@ -15,7 +15,7 @@ RSpec.describe MergeRequestUserEntity do
subject { entity.as_json } subject { entity.as_json }
it 'exposes needed attributes' do 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 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