Merge branch '273544-add-group-mr-approval-settings-update-api' into 'master'

Add API to update group MR approval settings

See merge request gitlab-org/gitlab!50814

ee/app/models/group_merge_request_approval_setting.rb

 # 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

ee/app/services/merge_request_approval_settings/update_service.rb

+# 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

ee/lib/api/group_merge_request_approval_settings.rb

@@ -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

ee/spec/models/group_merge_request_approval_setting_spec.rb

@@ -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

ee/spec/requests/api/group_merge_request_approval_settings_spec.rb

@@ -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

ee/spec/services/merge_request_approval_settings/update_service_spec.rb

+# 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