diff --git a/db/docs/merge_requests_approval_rules.yml b/db/docs/merge_requests_approval_rules.yml new file mode 100644 index 0000000000000000000000000000000000000000..fc3ebd5900c59b062f6f3b2cfe05297767c47bfb --- /dev/null +++ b/db/docs/merge_requests_approval_rules.yml @@ -0,0 +1,13 @@ +--- +table_name: merge_requests_approval_rules +classes: + - MergeRequests::ApprovalRule +feature_categories: + - code_review_workflow +description: Main table that stores information about approval rules v2. +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179839 +milestone: '17.9' +gitlab_schema: gitlab_main_cell +sharding_key: + group_id: namespaces + project_id: projects diff --git a/db/migrate/20250123151650_create_merge_requests_approval_rules.rb b/db/migrate/20250123151650_create_merge_requests_approval_rules.rb new file mode 100644 index 0000000000000000000000000000000000000000..700e18d4dedbd27af17244d2e1b31bdb20f44e1c --- /dev/null +++ b/db/migrate/20250123151650_create_merge_requests_approval_rules.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class CreateMergeRequestsApprovalRules < Gitlab::Database::Migration[2.2] + milestone '17.9' + + def change + create_table :merge_requests_approval_rules do |t| # -- Migration/EnsureFactoryForTable false positive + t.text :name, limit: 255, null: false + t.integer :approvals_required, null: false, default: 0 + t.integer :rule_type, null: false, default: 0, limit: 2 + t.integer :origin, null: false, default: 0, limit: 2 + t.bigint :project_id, null: true + t.bigint :group_id, null: true + t.bigint :source_rule_id, null: true + t.index :project_id + t.index :group_id + t.index :source_rule_id + t.timestamps_with_timezone null: false + end + end +end diff --git a/db/migrate/20250205094214_add_merge_requests_approval_rules_source_rule_fk.rb b/db/migrate/20250205094214_add_merge_requests_approval_rules_source_rule_fk.rb new file mode 100644 index 0000000000000000000000000000000000000000..03a6a9ba8a790b1a0f0dd23a3d70348e45bc3ba3 --- /dev/null +++ b/db/migrate/20250205094214_add_merge_requests_approval_rules_source_rule_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesSourceRuleFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules, :merge_requests_approval_rules, column: :source_rule_id, + on_delete: :nullify + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules, column: :source_rule_id + end + end +end diff --git a/db/migrate/20250205094243_add_merge_requests_approval_rules_project_fk.rb b/db/migrate/20250205094243_add_merge_requests_approval_rules_project_fk.rb new file mode 100644 index 0000000000000000000000000000000000000000..40a1ee3859b19e5ac50cef90fe2543fa8ebeca7d --- /dev/null +++ b/db/migrate/20250205094243_add_merge_requests_approval_rules_project_fk.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesProjectFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules, :projects, column: :project_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules, column: :project_id + end + end +end diff --git a/db/migrate/20250205094302_add_merge_requests_approval_rules_group_fk.rb b/db/migrate/20250205094302_add_merge_requests_approval_rules_group_fk.rb new file mode 100644 index 0000000000000000000000000000000000000000..b558885fda687321841ebb37833c409168a8447f --- /dev/null +++ b/db/migrate/20250205094302_add_merge_requests_approval_rules_group_fk.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesGroupFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules, :namespaces, column: :group_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules, column: :group_id + end + end +end diff --git a/db/migrate/20250205094331_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb b/db/migrate/20250205094331_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb new file mode 100644 index 0000000000000000000000000000000000000000..b34c7ea12a4260fe9fece5baf23be698002a956a --- /dev/null +++ b/db/migrate/20250205094331_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesMultiColumnNotNullConstraint < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_multi_column_not_null_constraint(:merge_requests_approval_rules, :group_id, :project_id) + end + + def down + remove_multi_column_not_null_constraint(:merge_requests_approval_rules, :group_id, :project_id) + end +end diff --git a/db/schema_migrations/20250123151650 b/db/schema_migrations/20250123151650 new file mode 100644 index 0000000000000000000000000000000000000000..6fbb499a26496d4b1f800227a9761a6fc305859c --- /dev/null +++ b/db/schema_migrations/20250123151650 @@ -0,0 +1 @@ +d149047a5d5fa4fa8242fde7f0d266cebef8d56e0db282d745c6da6e0f9f1c1d \ No newline at end of file diff --git a/db/schema_migrations/20250205094214 b/db/schema_migrations/20250205094214 new file mode 100644 index 0000000000000000000000000000000000000000..d8a44e1c1b5c2e94b313c88748803e90b95ced9b --- /dev/null +++ b/db/schema_migrations/20250205094214 @@ -0,0 +1 @@ +b22251b9e81427177253b4121f677e6a690378b55cc2836e3da0019b71822629 \ No newline at end of file diff --git a/db/schema_migrations/20250205094243 b/db/schema_migrations/20250205094243 new file mode 100644 index 0000000000000000000000000000000000000000..45f04e8079f5f08ae379cd6147ae115aea39689c --- /dev/null +++ b/db/schema_migrations/20250205094243 @@ -0,0 +1 @@ +6d8e72cc35740ab5214c24b9d2094bce9289edd5c999ef89d4c2aa9b4d949a07 \ No newline at end of file diff --git a/db/schema_migrations/20250205094302 b/db/schema_migrations/20250205094302 new file mode 100644 index 0000000000000000000000000000000000000000..b54934da8ad24ee2d7e1dc122015e80f12c6b34c --- /dev/null +++ b/db/schema_migrations/20250205094302 @@ -0,0 +1 @@ +89018d03700f67c3d24a34d03529697d7e75dcffad09c6234239a7c661746071 \ No newline at end of file diff --git a/db/schema_migrations/20250205094331 b/db/schema_migrations/20250205094331 new file mode 100644 index 0000000000000000000000000000000000000000..c5eb507d3afba10ee8960e911afc33d200451ecf --- /dev/null +++ b/db/schema_migrations/20250205094331 @@ -0,0 +1 @@ +660df89c5562b36729e4f93d57ad28b761a275105a806eb91258fb01f3b3703f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index aa7db723ee4426082ee29a3cc2e3325900275f9d..5aa852900c5cd05a26b1b3813f7524f17d4267d9 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15885,6 +15885,30 @@ CREATE TABLE merge_requests ( CONSTRAINT check_970d272570 CHECK ((lock_version IS NOT NULL)) ); +CREATE TABLE merge_requests_approval_rules ( + id bigint NOT NULL, + name text NOT NULL, + approvals_required integer DEFAULT 0 NOT NULL, + rule_type smallint DEFAULT 0 NOT NULL, + origin smallint DEFAULT 0 NOT NULL, + project_id bigint, + group_id bigint, + source_rule_id bigint, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + CONSTRAINT check_ba7b03c61a CHECK ((num_nonnulls(group_id, project_id) = 1)), + CONSTRAINT check_c7c36145b7 CHECK ((char_length(name) <= 255)) +); + +CREATE SEQUENCE merge_requests_approval_rules_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_requests_approval_rules_id_seq OWNED BY merge_requests_approval_rules.id; + CREATE TABLE merge_requests_closing_issues ( id bigint NOT NULL, merge_request_id bigint NOT NULL, @@ -25102,6 +25126,8 @@ ALTER TABLE ONLY merge_request_user_mentions ALTER COLUMN id SET DEFAULT nextval ALTER TABLE ONLY merge_requests ALTER COLUMN id SET DEFAULT nextval('merge_requests_id_seq'::regclass); +ALTER TABLE ONLY merge_requests_approval_rules ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_id_seq'::regclass); + ALTER TABLE ONLY merge_requests_closing_issues ALTER COLUMN id SET DEFAULT nextval('merge_requests_closing_issues_id_seq'::regclass); ALTER TABLE ONLY merge_requests_compliance_violations ALTER COLUMN id SET DEFAULT nextval('merge_requests_compliance_violations_id_seq'::regclass); @@ -27651,6 +27677,9 @@ ALTER TABLE ONLY merge_request_reviewers ALTER TABLE ONLY merge_request_user_mentions ADD CONSTRAINT merge_request_user_mentions_pkey PRIMARY KEY (id); +ALTER TABLE ONLY merge_requests_approval_rules + ADD CONSTRAINT merge_requests_approval_rules_pkey PRIMARY KEY (id); + ALTER TABLE ONLY merge_requests_closing_issues ADD CONSTRAINT merge_requests_closing_issues_pkey PRIMARY KEY (id); @@ -32990,6 +33019,12 @@ CREATE INDEX index_merge_request_reviewers_on_user_id ON merge_request_reviewers CREATE UNIQUE INDEX index_merge_request_user_mentions_on_note_id ON merge_request_user_mentions USING btree (note_id) WHERE (note_id IS NOT NULL); +CREATE INDEX index_merge_requests_approval_rules_on_group_id ON merge_requests_approval_rules USING btree (group_id); + +CREATE INDEX index_merge_requests_approval_rules_on_project_id ON merge_requests_approval_rules USING btree (project_id); + +CREATE INDEX index_merge_requests_approval_rules_on_source_rule_id ON merge_requests_approval_rules USING btree (source_rule_id); + CREATE INDEX index_merge_requests_closing_issues_on_issue_id ON merge_requests_closing_issues USING btree (issue_id); CREATE INDEX index_merge_requests_closing_issues_on_merge_request_id ON merge_requests_closing_issues USING btree (merge_request_id); @@ -37875,6 +37910,9 @@ ALTER TABLE ONLY cluster_agent_url_configurations ALTER TABLE ONLY incident_management_escalation_rules ADD CONSTRAINT fk_0314ee86eb FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules + ADD CONSTRAINT fk_03983bf729 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY audit_events_instance_google_cloud_logging_configurations ADD CONSTRAINT fk_03a15ca4fa FOREIGN KEY (stream_destination_id) REFERENCES audit_events_instance_external_streaming_destinations(id) ON DELETE SET NULL; @@ -38745,6 +38783,9 @@ ALTER TABLE ONLY scan_result_policies ALTER TABLE ONLY catalog_resource_versions ADD CONSTRAINT fk_7ad8849db4 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules + ADD CONSTRAINT fk_7af76dbd21 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY issue_customer_relations_contacts ADD CONSTRAINT fk_7b92f835bb FOREIGN KEY (contact_id) REFERENCES customer_relations_contacts(id) ON DELETE CASCADE; @@ -39690,6 +39731,9 @@ ALTER TABLE ONLY application_settings ALTER TABLE ONLY issuable_severities ADD CONSTRAINT fk_f9df19ecb6 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules + ADD CONSTRAINT fk_fa5b38e373 FOREIGN KEY (source_rule_id) REFERENCES merge_requests_approval_rules(id) ON DELETE SET NULL; + ALTER TABLE ONLY clusters_managed_resources ADD CONSTRAINT fk_fad3c3b2e2 FOREIGN KEY (environment_id) REFERENCES environments(id) ON DELETE CASCADE; diff --git a/ee/app/models/merge_requests/approval_rule.rb b/ee/app/models/merge_requests/approval_rule.rb new file mode 100644 index 0000000000000000000000000000000000000000..46538828aea773d3fd9b8e9d0a87d563089eb6e0 --- /dev/null +++ b/ee/app/models/merge_requests/approval_rule.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module MergeRequests + class ApprovalRule < ApplicationRecord + self.table_name = 'merge_requests_approval_rules' + + validate :ensure_single_sharding_key + + with_options validate: true do + enum :rule_type, { regular: 0, code_owner: 1, report_approver: 2, any_approver: 3 }, default: :regular + enum :origin, { group: 0, project: 1, merge_request: 2 }, prefix: :originates_from + end + + private + + def ensure_single_sharding_key + return errors.add(:base, "Must have either `group_id` or `project_id`") if no_sharding_key? + + errors.add(:base, "Cannot have both `group_id` and `project_id`") if multiple_sharding_keys? + end + + def sharding_keys + [group_id, project_id] + end + + def no_sharding_key? + sharding_keys.all?(&:blank?) + end + + def multiple_sharding_keys? + sharding_keys.all?(&:present?) + end + end +end diff --git a/ee/spec/factories/merge_requests/approval_rules.rb b/ee/spec/factories/merge_requests/approval_rules.rb new file mode 100644 index 0000000000000000000000000000000000000000..a56d74503f402a283ff9c9d109b20fde33ba907f --- /dev/null +++ b/ee/spec/factories/merge_requests/approval_rules.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_requests_approval_rule, class: 'MergeRequests::ApprovalRule' do + sequence(:name) { |n| "Approval Rule #{n}" } + approvals_required { 2 } + rule_type { 0 } + origin { 0 } + + trait :with_source_rule do + association :source_rule, factory: :merge_requests_approval_rule + end + end +end diff --git a/ee/spec/models/merge_requests/approval_rule_spec.rb b/ee/spec/models/merge_requests/approval_rule_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f038a559295d654fdb4c621919dbff59e73ac829 --- /dev/null +++ b/ee/spec/models/merge_requests/approval_rule_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::ApprovalRule, type: :model, feature_category: :code_review_workflow do + let_it_be(:project) { create(:project) } + let_it_be(:group) { create(:group) } + let(:attributes) { {} } + + subject(:rule) { build(:merge_requests_approval_rule, attributes) } + + describe 'validations' do + describe 'sharding key validation' do + context 'with group_id' do + let(:attributes) { { group_id: group.id } } + + it { is_expected.to be_valid } + end + + context 'with project_id' do + let(:attributes) { { project_id: project.id } } + + it { is_expected.to be_valid } + end + + context 'without project_id or group_id' do + it { is_expected.not_to be_valid } + + it 'has the correct error message' do + rule.valid? + expect(rule.errors[:base]).to contain_exactly("Must have either `group_id` or `project_id`") + end + end + + context 'with both project_id and group_id' do + let(:attributes) { { project_id: project.id, group_id: group.id } } + + it { is_expected.not_to be_valid } + + it 'has the correct error message' do + rule.valid? + expect(rule.errors[:base]).to contain_exactly("Cannot have both `group_id` and `project_id`") + end + end + end + end +end