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

Add API to query group MR approval settings

See merge request gitlab-org/gitlab!50256

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