Add API to query group MR approval settings

This MR adds supported migration, model and GET API to manage group
merge request approval settings.

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

GET /groups/:id/merge_request_approval_setting

changelogs/unreleased/273544-add-group-mr-approval-settings-api.yml

+---
+title: Add group MR approval settings table
+merge_request: 50256
+author:
+type: added

db/migrate/20201217070530_add_group_merge_request_approval_settings.rb

+# frozen_string_literal: true
+
+class AddGroupMergeRequestApprovalSettings < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      create_table :group_merge_request_approval_settings, id: false do |t|
+        t.timestamps_with_timezone null: false
+        t.references :group, references: :namespaces, primary_key: true, default: nil, index: false,
+          foreign_key: { to_table: :namespaces, on_delete: :cascade }
+        t.boolean :allow_author_approval, null: false, default: false
+      end
+    end
+  end
+
+  def down
+    with_lock_retries do
+      drop_table :group_merge_request_approval_settings
+    end
+  end
+end

db/schema_migrations/20201217070530

+59e40a24e8422e64bc85c4d54e6da923512017bac6a167350affeff241046e9f
\ No newline at end of file

db/structure.sql

@@ -12949,6 +12949,13 @@ CREATE SEQUENCE group_import_states_group_id_seq
 
 ALTER SEQUENCE group_import_states_group_id_seq OWNED BY group_import_states.group_id;
 
+CREATE TABLE group_merge_request_approval_settings (
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    group_id bigint NOT NULL,
+    allow_author_approval boolean DEFAULT false NOT NULL
+);
+
 CREATE TABLE group_wiki_repositories (
     shard_id bigint NOT NULL,
     group_id bigint NOT NULL,
@@ -19680,6 +19687,9 @@ ALTER TABLE ONLY group_group_links
 ALTER TABLE ONLY group_import_states
     ADD CONSTRAINT group_import_states_pkey PRIMARY KEY (group_id);
 
+ALTER TABLE ONLY group_merge_request_approval_settings
+    ADD CONSTRAINT group_merge_request_approval_settings_pkey PRIMARY KEY (group_id);
+
 ALTER TABLE ONLY group_wiki_repositories
     ADD CONSTRAINT group_wiki_repositories_pkey PRIMARY KEY (group_id);
 
@@ -24345,6 +24355,9 @@ ALTER TABLE ONLY merge_request_blocks
 ALTER TABLE ONLY merge_request_reviewers
     ADD CONSTRAINT fk_rails_3704a66140 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY group_merge_request_approval_settings
+    ADD CONSTRAINT fk_rails_37b6b4cdba FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY analytics_cycle_analytics_project_stages
     ADD CONSTRAINT fk_rails_3829e49b66 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

ee/app/models/ee/group.rb

@@ -44,6 +44,7 @@ module EE
       has_many :provisioned_users, through: :provisioned_user_details, source: :user
       has_many :cycle_analytics_stages, class_name: 'Analytics::CycleAnalytics::GroupStage'
       has_many :value_streams, class_name: 'Analytics::CycleAnalytics::GroupValueStream'
+      has_one :group_merge_request_approval_setting, inverse_of: :group
 
       has_one :deletion_schedule, class_name: 'GroupDeletionSchedule'
       delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true

ee/app/models/group_merge_request_approval_setting.rb

+# frozen_string_literal: true
+
+class GroupMergeRequestApprovalSetting < ApplicationRecord
+  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
+end

ee/app/models/license.rb

@@ -88,6 +88,7 @@ class License < ApplicationRecord
     group_forking_protection
     group_ip_restriction
     group_merge_request_analytics
+    group_merge_request_approval_settings
     group_milestone_project_releases
     group_project_templates
     group_repository_analytics

ee/app/policies/ee/group_policy.rb

@@ -111,6 +111,10 @@ module EE
         @subject.feature_available?(:push_rules)
       end
 
+      condition(:group_merge_request_approval_settings_enabled) do
+        @subject.feature_available?(:group_merge_request_approval_settings)
+      end
+
       condition(:over_storage_limit, scope: :subject) { @subject.over_storage_limit? }
 
       rule { public_group | logged_in_viewable }.policy do
@@ -255,6 +259,10 @@ module EE
         enable :admin_group_credentials_inventory
       end
 
+      rule { (admin | owner) & group_merge_request_approval_settings_enabled }.policy do
+        enable :admin_merge_request_approval_settings
+      end
+
       rule { needs_new_sso_session }.policy do
         prevent :read_group
       end

ee/config/feature_flags/development/group_merge_request_approval_settings_feature_flag.yml

+---
+name: group_merge_request_approval_settings_feature_flag
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50256
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/247921
+milestone: '13.8'
+type: development
+group: group::compliance
+default_enabled: false

ee/lib/api/entities/group_merge_request_approval_setting.rb

+# frozen_string_literal: true
+
+module API
+  module Entities
+    class GroupMergeRequestApprovalSetting < Grape::Entity
+      expose :allow_author_approval
+    end
+  end
+end

ee/lib/api/group_merge_request_approval_settings.rb

+# frozen_string_literal: true
+
+module API
+  class GroupMergeRequestApprovalSettings < ::API::Base
+    feature_category :source_code_management
+
+    before do
+      authenticate!
+      not_found! unless ::Feature.enabled?(:group_merge_request_approval_settings_feature_flag, user_group)
+    end
+
+    params do
+      requires :id, type: String, desc: 'The ID of a group'
+    end
+    resource :groups, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
+      segment ':id/merge_request_approval_setting' do
+        desc 'Get group merge request approval setting' do
+          detail 'This feature is gated by the :group_merge_request_approval_settings_feature_flag'
+          success ::API::Entities::GroupMergeRequestApprovalSetting
+        end
+        get do
+          authorize! :admin_merge_request_approval_settings, user_group
+
+          present user_group.group_merge_request_approval_setting,
+            with: ::API::Entities::GroupMergeRequestApprovalSetting
+        end
+      end
+    end
+  end
+end

ee/lib/ee/api/api.rb

@@ -29,6 +29,7 @@ module EE
         mount ::API::GroupPushRule
         mount ::API::MergeTrains
         mount ::API::GroupHooks
+        mount ::API::GroupMergeRequestApprovalSettings
         mount ::API::Scim
         mount ::API::ManagedLicenses
         mount ::API::ProjectApprovals

ee/spec/factories/group_merge_request_approval_settings.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :group_merge_request_approval_setting do
+    group
+  end
+end

ee/spec/fixtures/api/schemas/public_api/v4/group_merge_request_approval_settings.json

+{
+  "type": "object",
+  "properties": {
+    "allow_author_approval": { "type": "boolean" }
+  },
+  "additionalProperties": false
+}

ee/spec/models/group_merge_request_approval_setting_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GroupMergeRequestApprovalSetting do
+  describe 'Associations' do
+    it { is_expected.to belong_to :group }
+  end
+
+  describe 'Validations' do
+    let_it_be(:setting) { create(:group_merge_request_approval_setting) }
+
+    subject { setting }
+
+    it { is_expected.to validate_presence_of(:group) }
+    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
+end

ee/spec/models/group_spec.rb

@@ -26,6 +26,7 @@ RSpec.describe Group do
     it { is_expected.to have_many(:epic_boards).inverse_of(:group) }
     it { is_expected.to have_many(:provisioned_user_details).inverse_of(:provisioned_by_group) }
     it { is_expected.to have_many(:provisioned_users) }
+    it { is_expected.to have_one(:group_merge_request_approval_setting) }
 
     it_behaves_like 'model with wiki' do
       let(:container) { create(:group, :nested, :wiki_repo) }

ee/spec/policies/group_policy_spec.rb

@@ -1341,5 +1341,36 @@ RSpec.describe GroupPolicy do
 
       it_behaves_like 'read_group_release_stats permissions'
     end
+
+    describe ':admin_merge_request_approval_settings' do
+      using RSpec::Parameterized::TableSyntax
+
+      let(:policy) { :admin_merge_request_approval_settings }
+
+      where(:role, :licensed, :allowed) do
+        :guest      | true  | false
+        :guest      | false | false
+        :reporter   | true  | false
+        :reporter   | false | false
+        :developer  | true  | false
+        :developer  | false | false
+        :maintainer | true  | false
+        :maintainer | false | false
+        :owner      | true  | true
+        :owner      | false | false
+        :admin      | true  | true
+        :admin      | false | false
+      end
+
+      with_them do
+        let(:current_user) { public_send(role) }
+
+        before do
+          stub_licensed_features(group_merge_request_approval_settings: licensed)
+        end
+
+        it { is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy)) }
+      end
+    end
   end
 end

ee/spec/requests/api/group_merge_request_approval_settings_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe API::GroupMergeRequestApprovalSettings do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:setting) { create(:group_merge_request_approval_setting, group: group) }
+
+  let(:url) { "/groups/#{group.id}/merge_request_approval_setting" }
+
+  describe 'GET /groups/:id/merge_request_approval_settings' do
+    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
+        get api(url, user)
+
+        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
+          get api(url, user)
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['allow_author_approval']).to eq(false)
+        end
+
+        it 'matches the response schema' do
+          get api(url, user)
+
+          expect(response).to match_response_schema('public_api/v4/group_merge_request_approval_settings', dir: 'ee')
+        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
+          get api(url, user)
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+    end
+  end
+end