Commit 190626f8 authored by David Kim's avatar David Kim Committed by Markus Koller

Allow multiple reviewers on Starter+

- One reviewer is allowed on CE
- Multiple reviewer is allowed on Starter and up
parent 915ac9c6
......@@ -1690,6 +1690,10 @@ class MergeRequest < ApplicationRecord
Feature.enabled?(:merge_request_reviewers, project)
end
def allows_multiple_reviewers?
false
end
private
def with_rebase_lock
......
......@@ -110,6 +110,10 @@ module MergeRequests
return
end
unless merge_request.allows_multiple_reviewers?
params[:reviewer_ids] = params[:reviewer_ids].first(1)
end
reviewer_ids = params[:reviewer_ids].select { |reviewer_id| user_can_read?(merge_request, reviewer_id) }
if params[:reviewer_ids].map(&:to_s) == [IssuableFinder::Params::NONE]
......
......@@ -120,6 +120,10 @@ module EE
project.feature_available?(:multiple_merge_request_assignees)
end
def allows_multiple_reviewers?
project.feature_available?(:multiple_merge_request_reviewers)
end
def visible_blocking_merge_requests(user)
Ability.merge_requests_readable_by_user(blocking_merge_requests, user)
end
......
......@@ -29,6 +29,7 @@ class License < ApplicationRecord
multiple_issue_assignees
multiple_ldap_servers
multiple_merge_request_assignees
multiple_merge_request_reviewers
project_merge_request_analytics
protected_refs_for_users
push_rules
......
......@@ -118,6 +118,24 @@ RSpec.describe MergeRequest do
end
end
describe '#allows_multiple_reviewers?' do
it 'returns false without license' do
stub_licensed_features(multiple_merge_request_reviewers: false)
merge_request = build_stubbed(:merge_request)
expect(merge_request.allows_multiple_reviewers?).to be(false)
end
it 'returns true when licensed' do
stub_licensed_features(multiple_merge_request_reviewers: true)
merge_request = build(:merge_request)
expect(merge_request.allows_multiple_reviewers?).to be(true)
end
end
describe '#participants' do
context 'with approval rule' do
before do
......
......@@ -51,6 +51,10 @@ RSpec.describe MergeRequests::CreateService do
it_behaves_like 'new issuable with scoped labels' do
let(:parent) { project }
end
it_behaves_like 'service with multiple reviewers' do
let(:execute) { service.execute }
end
end
describe '#execute with blocking merge requests', :clean_gitlab_redis_shared_state do
......
......@@ -36,6 +36,11 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
let(:parent) { project }
end
it_behaves_like 'service with multiple reviewers' do
let(:opts) { {} }
let(:execute) { update_merge_request(opts) }
end
def update_merge_request(opts)
described_class.new(project, user, opts).execute(merge_request)
end
......
......@@ -96,3 +96,38 @@ RSpec.shared_examples 'merge validation hooks' do |args|
end
end
end
RSpec.shared_examples 'service with multiple reviewers' do
context 'with multiple reviewer assignments' do
let(:opts) { super().merge(reviewer_ids_param) }
let(:reviewer_ids_param) { { reviewer_ids: [reviewer1.id, reviewer2.id] } }
let(:reviewer1) { create(:user) }
let(:reviewer2) { create(:user) }
before do
stub_feature_flags(merge_request_reviewer: true)
project.add_developer(reviewer1)
project.add_developer(reviewer2)
end
context 'with multiple_merge_request_reviewers feature on' do
before do
stub_licensed_features(multiple_merge_request_reviewers: true)
end
it 'allows multiple reviewers' do
expect(execute.reviewers).to contain_exactly(reviewer1, reviewer2)
end
end
context 'with multiple_merge_request_reviewers feature off' do
before do
stub_licensed_features(multiple_merge_request_reviewers: false)
end
it 'only allows one reviewer' do
expect(execute.reviewers).to contain_exactly(reviewer1)
end
end
end
end
......@@ -13,11 +13,10 @@ RSpec.shared_examples 'reviewer_ids filter' do
end
context 'with reviewer_ids' do
let(:reviewer_ids_param) { { reviewer_ids: [reviewer1.id, reviewer2.id, reviewer3.id] } }
let(:reviewer_ids_param) { { reviewer_ids: [reviewer1.id, reviewer2.id] } }
let(:reviewer1) { create(:user) }
let(:reviewer2) { create(:user) }
let(:reviewer3) { create(:user) }
context 'when the current user can admin the merge_request' do
context 'when merge_request_reviewer feature is enabled' do
......@@ -25,14 +24,13 @@ RSpec.shared_examples 'reviewer_ids filter' do
stub_feature_flags(merge_request_reviewer: true)
end
context 'with reviewers who can read the merge_request' do
context 'with a reviewer who can read the merge_request' do
before do
project.add_developer(reviewer1)
project.add_developer(reviewer2)
end
it 'contains reviewers who can read the merge_request' do
expect(execute.reviewers).to contain_exactly(reviewer1, reviewer2)
expect(execute.reviewers).to contain_exactly(reviewer1)
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