From da6716ab79872f3a6519c984c666f4f330b6b374 Mon Sep 17 00:00:00 2001 From: ghinfey <“ghinfey@gitlab.com”> Date: Tue, 26 Sep 2023 11:17:16 +0100 Subject: [PATCH 1/2] Add DB tables to allow group approval rules - approval_group_rules - approval_group_rules_users - approval_group_rules_groups - approval_group_rules_protected_branches Changelog: changed MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132651 EE: true --- ...092944_add_approval_groups_rules_groups.rb | 22 +++++++++++++++++++ ...6093004_add_approval_groups_rules_users.rb | 18 +++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 db/migrate/20230926092944_add_approval_groups_rules_groups.rb create mode 100644 db/migrate/20230926093004_add_approval_groups_rules_users.rb diff --git a/db/migrate/20230926092944_add_approval_groups_rules_groups.rb b/db/migrate/20230926092944_add_approval_groups_rules_groups.rb new file mode 100644 index 00000000000000..03faf9a0533d71 --- /dev/null +++ b/db/migrate/20230926092944_add_approval_groups_rules_groups.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddApprovalGroupsRulesGroups < Gitlab::Database::Migration[2.1] + # frozen_string_literal: true + INDEX_RULE_GROUP = 'idx_on_approval_group_rules_groups_rule_group' + + def up + create_table :approval_group_rules_groups do |t| + t.bigint :approval_group_rule_id, null: false + t.bigint :group_id, null: false, index: true + + t.index [:approval_group_rule_id, :group_id], unique: true, name: INDEX_RULE_GROUP + end + end + + def down + drop_table :approval_group_rules_groups + end +end diff --git a/db/migrate/20230926093004_add_approval_groups_rules_users.rb b/db/migrate/20230926093004_add_approval_groups_rules_users.rb new file mode 100644 index 00000000000000..4488f7e53482b8 --- /dev/null +++ b/db/migrate/20230926093004_add_approval_groups_rules_users.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddApprovalGroupsRulesUsers < Gitlab::Database::Migration[2.1] + INDEX_RULE_USER = 'idx_on_approval_group_rules_users_rule_user' + + def up + create_table :approval_group_rules_users do |t| + t.bigint :approval_group_rule_id, null: false + t.bigint :user_id, null: false, index: true + + t.index [:approval_group_rule_id, :user_id], unique: true, name: INDEX_RULE_USER + end + end + + def down + drop_table :approval_group_rules_users + end +end -- GitLab From e9f7c65a14f7f0b2fb13159383907b44ea5b3cab Mon Sep 17 00:00:00 2001 From: ghinfey <“ghinfey@gitlab.com”> Date: Thu, 28 Sep 2023 10:25:30 +0100 Subject: [PATCH 2/2] Add approval group rule model Contributes to allowing group approval rules https://gitlab.com/gitlab-org/gitlab/-/issues/325646 Changelog: added MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132862 EE: true --- ...092944_add_approval_groups_rules_groups.rb | 22 ------------ ...6093004_add_approval_groups_rules_users.rb | 18 ---------- .../approval_rules/approval_group_rule.rb | 23 ++++++++++++ ee/app/models/ee/group.rb | 1 + ee/spec/factories/approval_rules.rb | 6 ++++ .../approval_group_rule_spec.rb | 36 +++++++++++++++++++ ee/spec/models/ee/group_spec.rb | 1 + locale/gitlab.pot | 3 ++ 8 files changed, 70 insertions(+), 40 deletions(-) delete mode 100644 db/migrate/20230926092944_add_approval_groups_rules_groups.rb delete mode 100644 db/migrate/20230926093004_add_approval_groups_rules_users.rb create mode 100644 ee/app/models/approval_rules/approval_group_rule.rb create mode 100644 ee/spec/models/approval_rules/approval_group_rule_spec.rb diff --git a/db/migrate/20230926092944_add_approval_groups_rules_groups.rb b/db/migrate/20230926092944_add_approval_groups_rules_groups.rb deleted file mode 100644 index 03faf9a0533d71..00000000000000 --- a/db/migrate/20230926092944_add_approval_groups_rules_groups.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddApprovalGroupsRulesGroups < Gitlab::Database::Migration[2.1] - # frozen_string_literal: true - INDEX_RULE_GROUP = 'idx_on_approval_group_rules_groups_rule_group' - - def up - create_table :approval_group_rules_groups do |t| - t.bigint :approval_group_rule_id, null: false - t.bigint :group_id, null: false, index: true - - t.index [:approval_group_rule_id, :group_id], unique: true, name: INDEX_RULE_GROUP - end - end - - def down - drop_table :approval_group_rules_groups - end -end diff --git a/db/migrate/20230926093004_add_approval_groups_rules_users.rb b/db/migrate/20230926093004_add_approval_groups_rules_users.rb deleted file mode 100644 index 4488f7e53482b8..00000000000000 --- a/db/migrate/20230926093004_add_approval_groups_rules_users.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class AddApprovalGroupsRulesUsers < Gitlab::Database::Migration[2.1] - INDEX_RULE_USER = 'idx_on_approval_group_rules_users_rule_user' - - def up - create_table :approval_group_rules_users do |t| - t.bigint :approval_group_rule_id, null: false - t.bigint :user_id, null: false, index: true - - t.index [:approval_group_rule_id, :user_id], unique: true, name: INDEX_RULE_USER - end - end - - def down - drop_table :approval_group_rules_users - end -end diff --git a/ee/app/models/approval_rules/approval_group_rule.rb b/ee/app/models/approval_rules/approval_group_rule.rb new file mode 100644 index 00000000000000..c68d4c865736e0 --- /dev/null +++ b/ee/app/models/approval_rules/approval_group_rule.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module ApprovalRules + class ApprovalGroupRule < ApplicationRecord + include ApprovalRuleLike + + enum rule_type: { + regular: 1, + code_owner: 2, + report_approver: 3, + any_approver: 4 + } + + belongs_to :group, inverse_of: :approval_rules + has_and_belongs_to_many :protected_branches + + validates :name, uniqueness: { scope: [:group_id, :rule_type] } + validates :rule_type, uniqueness: { + scope: :group_id, + message: proc { _('any-approver for the group already exists') } + }, if: :any_approver? + end +end diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 21a3f63c38f3ba..b5367b48fd5536 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -82,6 +82,7 @@ module Group belongs_to :file_template_project, class_name: "Project" belongs_to :push_rule, inverse_of: :group + has_many :approval_rules, class_name: 'ApprovalRules::ApprovalGroupRule', inverse_of: :group delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true diff --git a/ee/spec/factories/approval_rules.rb b/ee/spec/factories/approval_rules.rb index 433e1f8211e26c..62c7121d096d2b 100644 --- a/ee/spec/factories/approval_rules.rb +++ b/ee/spec/factories/approval_rules.rb @@ -102,4 +102,10 @@ approval_project_rule user end + + factory :approval_group_rule, class: 'ApprovalRules::ApprovalGroupRule' do + group + sequence(:name) { |n| "#{ApprovalRuleLike::DEFAULT_NAME}-#{n}" } + rule_type { :regular } + end end diff --git a/ee/spec/models/approval_rules/approval_group_rule_spec.rb b/ee/spec/models/approval_rules/approval_group_rule_spec.rb new file mode 100644 index 00000000000000..1ac8f0b9c7c987 --- /dev/null +++ b/ee/spec/models/approval_rules/approval_group_rule_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ApprovalRules::ApprovalGroupRule, feature_category: :source_code_management do + subject { build(:approval_group_rule) } + + describe 'validations' do + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to([:group_id, :rule_type]) } + it { is_expected.to validate_numericality_of(:approvals_required).is_less_than_or_equal_to(100) } + it { is_expected.to validate_numericality_of(:approvals_required).is_greater_than_or_equal_to(0) } + end + + describe 'associations' do + it { is_expected.to belong_to(:group).inverse_of(:approval_rules) } + it { is_expected.to belong_to(:security_orchestration_policy_configuration) } + it { is_expected.to belong_to(:scan_result_policy_read) } + it { is_expected.to have_and_belong_to_many(:users) } + it { is_expected.to have_and_belong_to_many(:groups) } + it { is_expected.to have_and_belong_to_many(:protected_branches) } + end + + describe 'any_approver rules' do + let_it_be(:group) { create(:group) } + + let(:rule) { build(:approval_group_rule, group: group, rule_type: :any_approver) } + + it 'allows to create only one any_approver rule', :aggregate_failures do + create(:approval_group_rule, group: group, rule_type: :any_approver) + + expect(rule).not_to be_valid + expect(rule.errors.messages).to eq(rule_type: ['any-approver for the group already exists']) + end + end +end diff --git a/ee/spec/models/ee/group_spec.rb b/ee/spec/models/ee/group_spec.rb index dd25558506a80f..c533baf8148427 100644 --- a/ee/spec/models/ee/group_spec.rb +++ b/ee/spec/models/ee/group_spec.rb @@ -31,6 +31,7 @@ it { is_expected.to have_many(:repository_storage_moves) } it { is_expected.to have_many(:iterations) } it { is_expected.to have_many(:iterations_cadences) } + it { is_expected.to have_many(:approval_rules).class_name('ApprovalRules::ApprovalGroupRule').inverse_of(:group) } it { is_expected.to have_many(:epic_board_recent_visits).inverse_of(:group) } it { is_expected.to have_many(:external_audit_event_destinations) } it { is_expected.to have_many(:google_cloud_logging_configurations) } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7fff677c22c4d4..212c2552cf770a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -55641,6 +55641,9 @@ msgstr "" msgid "and" msgstr "" +msgid "any-approver for the group already exists" +msgstr "" + msgid "any-approver for the merge request already exists" msgstr "" -- GitLab