Commit 0f576619 authored by Tan Le's avatar Tan Le

Add API to update group MR approval settings

The new PUT API is limited to user with owner or admin role.

PUT /groups/:id/merge_request_approval_setting
parent c1fb673a
# frozen_string_literal: true
class GroupMergeRequestApprovalSetting < ApplicationRecord
self.primary_key = :group_id
belongs_to :group, inverse_of: :group_merge_request_approval_setting
validates :group, presence: true
validates :allow_author_approval, inclusion: { in: [true, false], message: 'must be a boolean value' }
self.primary_key = :group_id
scope :find_or_initialize_by_group, ->(group) {
find_or_initialize_by(group: group)
}
end
# frozen_string_literal: true
module MergeRequestApprovalSettings
class UpdateService < BaseContainerService
def execute
return ServiceResponse.error(message: 'Insufficient permissions') unless allowed?
setting = GroupMergeRequestApprovalSetting.find_or_initialize_by_group(container)
setting.assign_attributes(params)
if setting.save
ServiceResponse.success(payload: setting)
else
ServiceResponse.error(message: setting.errors.messages)
end
end
private
def allowed?
can?(current_user, :admin_merge_request_approval_settings, container)
end
end
end
......@@ -24,6 +24,28 @@ module API
present user_group.group_merge_request_approval_setting,
with: ::API::Entities::GroupMergeRequestApprovalSetting
end
desc 'Update existing merge request approval setting' do
detail 'This feature is gated by the :group_merge_request_approval_settings_feature_flag'
success EE::API::Entities::GroupMergeRequestApprovalSetting
end
params do
optional :allow_author_approval, type: Boolean, desc: 'Allow authors to self-approve merge requests'
end
put do
authorize! :admin_merge_request_approval_settings, user_group
setting_params = declared_params(include_missing: false)
response = ::MergeRequestApprovalSettings::UpdateService
.new(container: user_group, current_user: current_user, params: setting_params).execute
if response.success?
present response.payload, with: ::API::Entities::GroupMergeRequestApprovalSetting
else
render_api_error!(response.message, :bad_request)
end
end
end
end
end
......
......@@ -16,4 +16,20 @@ RSpec.describe GroupMergeRequestApprovalSetting do
it { is_expected.not_to allow_value(nil).for(:allow_author_approval) }
it { is_expected.to allow_value(true, false).for(:allow_author_approval) }
end
describe '.find_or_initialize_by_group' do
let_it_be(:group) { create(:group) }
subject { described_class.find_or_initialize_by_group(group) }
context 'no existing setting' do
it { is_expected.to be_a_new_record }
end
context 'existing setting' do
let_it_be(:setting) { create(:group_merge_request_approval_setting, group: group) }
it { is_expected.to eq(setting) }
end
end
end
......@@ -64,4 +64,73 @@ RSpec.describe API::GroupMergeRequestApprovalSettings do
end
end
end
describe 'PUT /groups/:id/merge_request_approval_setting' do
let(:params) { { allow_author_approval: true } }
context 'when feature flag is disabled' do
before do
stub_feature_flags(group_merge_request_approval_settings_feature_flag: false)
end
it 'returns 404 status' do
put api(url, user), params: params
expect(response).to have_gitlab_http_status(:not_found)
end
end
context 'when feature flag is enabled' do
before do
allow(Ability).to receive(:allowed?).and_call_original
stub_feature_flags(group_merge_request_approval_settings_feature_flag: true)
end
context 'when the user is authorised' do
before do
allow(Ability).to receive(:allowed?)
.with(user, :admin_merge_request_approval_settings, group)
.and_return(true)
end
it 'returns 200 status with correct response body', :aggregate_failures do
put api(url, user), params: params
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['allow_author_approval']).to eq(true)
end
it 'matches the response schema' do
put api(url, user), params: params
expect(response).to match_response_schema('public_api/v4/group_merge_request_approval_settings', dir: 'ee')
end
context 'when update fails' do
let(:params) { { allow_author_approval: nil } }
it 'returns 400 status', :aggregate_failures do
put api(url, user), params: params
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).to eq('allow_author_approval' => ['must be a boolean value'])
end
end
end
context 'when the user is not authorised' do
before do
allow(Ability).to receive(:allowed?)
.with(user, :admin_merge_request_approval_settings, group)
.and_return(false)
end
it 'returns 403 status' do
put api(url, user), params: params
expect(response).to have_gitlab_http_status(:forbidden)
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe MergeRequestApprovalSettings::UpdateService do
let_it_be_with_reload(:group) { create(:group) }
let_it_be(:user) { create(:user) }
let(:params) { { allow_author_approval: true } }
subject(:service) do
described_class.new(
container: group,
current_user: user,
params: params
)
end
describe 'execute' do
context 'user does not have permissions' do
before do
allow(service).to receive(:can?).with(user, :admin_merge_request_approval_settings, group).and_return(false)
end
it 'responds with an error response', :aggregate_failures do
response = subject.execute
expect(response.status).to eq(:error)
expect(response.message).to eq('Insufficient permissions')
end
end
context 'user has permissions' do
before do
allow(service).to receive(:can?).with(user, :admin_merge_request_approval_settings, group).and_return(true)
end
it 'creates a new setting' do
expect { subject.execute }
.to change { group.group_merge_request_approval_setting }
.from(nil).to(be_instance_of(GroupMergeRequestApprovalSetting))
end
it 'responds with a successful service response', :aggregate_failures do
response = subject.execute
expect(response).to be_success
expect(response.payload.allow_author_approval).to be(true)
end
context 'when group has an existing setting' do
let_it_be(:group) { create(:group) }
let_it_be(:existing_setting) { create(:group_merge_request_approval_setting, group: group) }
it 'does not create a new setting' do
expect { subject.execute }
.to change { GroupMergeRequestApprovalSetting.count }.by(0)
.and change { existing_setting.reload.allow_author_approval }.to(true)
end
it 'responds with a successful service response', :aggregate_failures do
response = subject.execute
expect(response).to be_success
expect(response.payload.allow_author_approval).to be(true)
end
end
context 'when saving fails' do
let(:params) { { allow_author_approval: nil } }
it 'responds with an error service response', :aggregate_failures do
response = subject.execute
expect(response).to be_error
expect(response.message).to eq(allow_author_approval: ['must be a boolean value'])
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