From c734b4260ba188af6b39b000a5aaec7715e1cdd4 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Fri, 31 Jan 2025 16:27:19 +0000 Subject: [PATCH 1/4] Add v2 approval rule associations to user and user group Adds new approval rule associations to user and user group for new approval rule architecture. Changelog: changed EE: true --- ...equests_approval_rules_approver_groups.yml | 12 +++ .../merge_requests_approval_rules_users.yml | 13 ++++ ...requests_approval_rules_approver_groups.rb | 22 ++++++ ...ate_merge_requests_approval_rules_users.rb | 26 +++++++ ..._rules_approver_groups_approval_rule_fk.rb | 17 ++++ ...approval_rules_approver_groups_group_fk.rb | 17 ++++ ...e_requests_approval_rules_users_user_fk.rb | 16 ++++ ...equests_approval_rules_users_project_fk.rb | 16 ++++ ...s_approval_rules_users_approval_rule_fk.rb | 17 ++++ ..._requests_approval_rules_users_group_fk.rb | 16 ++++ ..._users_multi_column_not_null_constraint.rb | 14 ++++ db/schema_migrations/20250123151841 | 1 + db/schema_migrations/20250123151859 | 1 + db/schema_migrations/20250214133158 | 1 + db/schema_migrations/20250214133235 | 1 + db/schema_migrations/20250214134117 | 1 + db/schema_migrations/20250214134138 | 1 + db/schema_migrations/20250214134202 | 1 + db/schema_migrations/20250225112059 | 1 + db/schema_migrations/20250225112409 | 1 + db/structure.sql | 77 +++++++++++++++++++ ee/app/models/merge_requests/approval_rule.rb | 6 ++ .../approval_rules_approver_group.rb | 10 +++ .../merge_requests/approval_rules_user.rb | 22 ++++++ .../approval_rules_approver_groups.rb | 8 ++ .../merge_requests/approval_rules_users.rb | 8 ++ .../approval_rules_approver_group_spec.rb | 10 +++ .../approval_rules_user_spec.rb | 55 +++++++++++++ 28 files changed, 391 insertions(+) create mode 100644 db/docs/merge_requests_approval_rules_approver_groups.yml create mode 100644 db/docs/merge_requests_approval_rules_users.yml create mode 100644 db/migrate/20250123151841_create_merge_requests_approval_rules_approver_groups.rb create mode 100644 db/migrate/20250123151859_create_merge_requests_approval_rules_users.rb create mode 100644 db/migrate/20250214133158_add_merge_requests_approval_rules_approver_groups_approval_rule_fk.rb create mode 100644 db/migrate/20250214133235_add_merge_requests_approval_rules_approver_groups_group_fk.rb create mode 100644 db/migrate/20250214134117_add_merge_requests_approval_rules_users_user_fk.rb create mode 100644 db/migrate/20250214134138_add_merge_requests_approval_rules_users_project_fk.rb create mode 100644 db/migrate/20250214134202_add_merge_requests_approval_rules_users_approval_rule_fk.rb create mode 100644 db/migrate/20250225112059_add_merge_requests_approval_rules_users_group_fk.rb create mode 100644 db/migrate/20250225112409_add_merge_requests_approval_rules_users_multi_column_not_null_constraint.rb create mode 100644 db/schema_migrations/20250123151841 create mode 100644 db/schema_migrations/20250123151859 create mode 100644 db/schema_migrations/20250214133158 create mode 100644 db/schema_migrations/20250214133235 create mode 100644 db/schema_migrations/20250214134117 create mode 100644 db/schema_migrations/20250214134138 create mode 100644 db/schema_migrations/20250214134202 create mode 100644 db/schema_migrations/20250225112059 create mode 100644 db/schema_migrations/20250225112409 create mode 100644 ee/app/models/merge_requests/approval_rules_approver_group.rb create mode 100644 ee/app/models/merge_requests/approval_rules_user.rb create mode 100644 ee/spec/factories/merge_requests/approval_rules_approver_groups.rb create mode 100644 ee/spec/factories/merge_requests/approval_rules_users.rb create mode 100644 ee/spec/models/merge_requests/approval_rules_approver_group_spec.rb create mode 100644 ee/spec/models/merge_requests/approval_rules_user_spec.rb diff --git a/db/docs/merge_requests_approval_rules_approver_groups.yml b/db/docs/merge_requests_approval_rules_approver_groups.yml new file mode 100644 index 00000000000000..65161adf4bee94 --- /dev/null +++ b/db/docs/merge_requests_approval_rules_approver_groups.yml @@ -0,0 +1,12 @@ +--- +table_name: merge_requests_approval_rules_approver_groups +classes: +- MergeRequests::ApprovalRulesApproverGroup +feature_categories: +- code_review_workflow +description: Stores approval groups for approval rules v2 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179877 +milestone: '17.10' +gitlab_schema: gitlab_main_cell +sharding_key: + group_id: namespaces diff --git a/db/docs/merge_requests_approval_rules_users.yml b/db/docs/merge_requests_approval_rules_users.yml new file mode 100644 index 00000000000000..096697ec2d987f --- /dev/null +++ b/db/docs/merge_requests_approval_rules_users.yml @@ -0,0 +1,13 @@ +--- +table_name: merge_requests_approval_rules_users +classes: + - MergeRequests::ApprovalRulesUser +feature_categories: + - code_review_workflow +description: Stores approval users for approval rules v2 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179877 +milestone: '17.10' +gitlab_schema: gitlab_main_cell +sharding_key: + group_id: namespaces + project_id: projects diff --git a/db/migrate/20250123151841_create_merge_requests_approval_rules_approver_groups.rb b/db/migrate/20250123151841_create_merge_requests_approval_rules_approver_groups.rb new file mode 100644 index 00000000000000..600d390e4f0e28 --- /dev/null +++ b/db/migrate/20250123151841_create_merge_requests_approval_rules_approver_groups.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class CreateMergeRequestsApprovalRulesApproverGroups < Gitlab::Database::Migration[2.2] + milestone '17.10' + + def change + create_table :merge_requests_approval_rules_approver_groups do |t| + t.bigint :approval_rule_id, null: false + t.bigint :group_id, null: false + t.index :group_id + + t.timestamps_with_timezone null: false + end + + add_index( + :merge_requests_approval_rules_approver_groups, + %i[approval_rule_id group_id], + unique: true, + name: 'index_mrs_ars_approver_groups_on_ar_id_and_group_id' + ) + end +end diff --git a/db/migrate/20250123151859_create_merge_requests_approval_rules_users.rb b/db/migrate/20250123151859_create_merge_requests_approval_rules_users.rb new file mode 100644 index 00000000000000..c4a66163a6843f --- /dev/null +++ b/db/migrate/20250123151859_create_merge_requests_approval_rules_users.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class CreateMergeRequestsApprovalRulesUsers < Gitlab::Database::Migration[2.2] + milestone '17.10' + + def change + create_table :merge_requests_approval_rules_users do |t| + t.bigint :approval_rule_id, null: false + t.bigint :user_id, null: false + t.bigint :project_id, null: true + t.bigint :group_id, null: true + t.index :user_id + t.index :project_id + t.index :group_id + + t.timestamps_with_timezone null: false + end + + add_index( + :merge_requests_approval_rules_users, + %i[approval_rule_id user_id], + unique: true, + name: 'index_mrs_ars_users_on_ar_id_and_user_id' + ) + end +end diff --git a/db/migrate/20250214133158_add_merge_requests_approval_rules_approver_groups_approval_rule_fk.rb b/db/migrate/20250214133158_add_merge_requests_approval_rules_approver_groups_approval_rule_fk.rb new file mode 100644 index 00000000000000..0542cb9b6680d8 --- /dev/null +++ b/db/migrate/20250214133158_add_merge_requests_approval_rules_approver_groups_approval_rule_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesApproverGroupsApprovalRuleFk < Gitlab::Database::Migration[2.2] + milestone '17.10' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_approver_groups, :merge_requests_approval_rules, + column: :approval_rule_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_approver_groups, column: :approval_rule_id + end + end +end diff --git a/db/migrate/20250214133235_add_merge_requests_approval_rules_approver_groups_group_fk.rb b/db/migrate/20250214133235_add_merge_requests_approval_rules_approver_groups_group_fk.rb new file mode 100644 index 00000000000000..699c8f6c37a6be --- /dev/null +++ b/db/migrate/20250214133235_add_merge_requests_approval_rules_approver_groups_group_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesApproverGroupsGroupFk < Gitlab::Database::Migration[2.2] + milestone '17.10' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_approver_groups, :namespaces, column: :group_id, + on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_approver_groups, column: :group_id + end + end +end diff --git a/db/migrate/20250214134117_add_merge_requests_approval_rules_users_user_fk.rb b/db/migrate/20250214134117_add_merge_requests_approval_rules_users_user_fk.rb new file mode 100644 index 00000000000000..b475091f798e94 --- /dev/null +++ b/db/migrate/20250214134117_add_merge_requests_approval_rules_users_user_fk.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesUsersUserFk < Gitlab::Database::Migration[2.2] + milestone '17.10' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_users, :users, column: :user_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_users, column: :user_id + end + end +end diff --git a/db/migrate/20250214134138_add_merge_requests_approval_rules_users_project_fk.rb b/db/migrate/20250214134138_add_merge_requests_approval_rules_users_project_fk.rb new file mode 100644 index 00000000000000..2335a31fd18ef2 --- /dev/null +++ b/db/migrate/20250214134138_add_merge_requests_approval_rules_users_project_fk.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesUsersProjectFk < Gitlab::Database::Migration[2.2] + milestone '17.10' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_users, :projects, column: :project_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_users, column: :project_id + end + end +end diff --git a/db/migrate/20250214134202_add_merge_requests_approval_rules_users_approval_rule_fk.rb b/db/migrate/20250214134202_add_merge_requests_approval_rules_users_approval_rule_fk.rb new file mode 100644 index 00000000000000..2bb57d551d17f5 --- /dev/null +++ b/db/migrate/20250214134202_add_merge_requests_approval_rules_users_approval_rule_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesUsersApprovalRuleFk < Gitlab::Database::Migration[2.2] + milestone '17.10' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_users, :merge_requests_approval_rules, + column: :approval_rule_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_users, column: :approval_rule_id + end + end +end diff --git a/db/migrate/20250225112059_add_merge_requests_approval_rules_users_group_fk.rb b/db/migrate/20250225112059_add_merge_requests_approval_rules_users_group_fk.rb new file mode 100644 index 00000000000000..cace63dd3214e5 --- /dev/null +++ b/db/migrate/20250225112059_add_merge_requests_approval_rules_users_group_fk.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesUsersGroupFk < Gitlab::Database::Migration[2.2] + milestone '17.10' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_users, :namespaces, column: :group_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_users, column: :group_id + end + end +end diff --git a/db/migrate/20250225112409_add_merge_requests_approval_rules_users_multi_column_not_null_constraint.rb b/db/migrate/20250225112409_add_merge_requests_approval_rules_users_multi_column_not_null_constraint.rb new file mode 100644 index 00000000000000..4970f1621ad54f --- /dev/null +++ b/db/migrate/20250225112409_add_merge_requests_approval_rules_users_multi_column_not_null_constraint.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesUsersMultiColumnNotNullConstraint < Gitlab::Database::Migration[2.2] + milestone '17.10' + disable_ddl_transaction! + + def up + add_multi_column_not_null_constraint(:merge_requests_approval_rules_users, :group_id, :project_id) + end + + def down + remove_multi_column_not_null_constraint(:merge_requests_approval_rules_users, :group_id, :project_id) + end +end diff --git a/db/schema_migrations/20250123151841 b/db/schema_migrations/20250123151841 new file mode 100644 index 00000000000000..9f18b936142829 --- /dev/null +++ b/db/schema_migrations/20250123151841 @@ -0,0 +1 @@ +3dddd1d5c9f162ebc7e1f1628dc0dbff5a6104587dbf3df962b15a1b47e44aff \ No newline at end of file diff --git a/db/schema_migrations/20250123151859 b/db/schema_migrations/20250123151859 new file mode 100644 index 00000000000000..72ffdd36a09635 --- /dev/null +++ b/db/schema_migrations/20250123151859 @@ -0,0 +1 @@ +0231ed1f8496a8da0aa9e7941c188c15e94291d0462bcc4575c369ba7b5af2a5 \ No newline at end of file diff --git a/db/schema_migrations/20250214133158 b/db/schema_migrations/20250214133158 new file mode 100644 index 00000000000000..6f782e65b76f31 --- /dev/null +++ b/db/schema_migrations/20250214133158 @@ -0,0 +1 @@ +71751d3c5b490ec202be7fc4370216c047944454284df1ce9e774ea218cb6cc1 \ No newline at end of file diff --git a/db/schema_migrations/20250214133235 b/db/schema_migrations/20250214133235 new file mode 100644 index 00000000000000..110f55f6761fad --- /dev/null +++ b/db/schema_migrations/20250214133235 @@ -0,0 +1 @@ +0a852fc3ae3e8fc892dda967727b0ce7629b38c2ce84c050e4c5d886de6d43dd \ No newline at end of file diff --git a/db/schema_migrations/20250214134117 b/db/schema_migrations/20250214134117 new file mode 100644 index 00000000000000..b3d95d79c3f60a --- /dev/null +++ b/db/schema_migrations/20250214134117 @@ -0,0 +1 @@ +af99cf986e829d045cf5e5ad11b72d40378cb90a956a97c1549db8806953a05a \ No newline at end of file diff --git a/db/schema_migrations/20250214134138 b/db/schema_migrations/20250214134138 new file mode 100644 index 00000000000000..94829569314e5c --- /dev/null +++ b/db/schema_migrations/20250214134138 @@ -0,0 +1 @@ +3b18c1137fa082dddc82012f80750ec36266568d11bc8bd7160d8327d1153267 \ No newline at end of file diff --git a/db/schema_migrations/20250214134202 b/db/schema_migrations/20250214134202 new file mode 100644 index 00000000000000..393f746b24b7a7 --- /dev/null +++ b/db/schema_migrations/20250214134202 @@ -0,0 +1 @@ +fe84c9693c07d7b5b2b771094693597ed538dbc67c1f22d4204ae807f0fc9078 \ No newline at end of file diff --git a/db/schema_migrations/20250225112059 b/db/schema_migrations/20250225112059 new file mode 100644 index 00000000000000..2c12efce18973c --- /dev/null +++ b/db/schema_migrations/20250225112059 @@ -0,0 +1 @@ +f7604e3c541ec4b7b5e186c76365908c7c26035ea69f003cdf27d84d937e8180 \ No newline at end of file diff --git a/db/schema_migrations/20250225112409 b/db/schema_migrations/20250225112409 new file mode 100644 index 00000000000000..447d5a5a211d7d --- /dev/null +++ b/db/schema_migrations/20250225112409 @@ -0,0 +1 @@ +68a2b468a90e4116867a7460660177301bd506107d2fafc41f0b63d18f7e5f8a \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0f80bc724b0fd0..35bd3a6033588d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16404,6 +16404,23 @@ CREATE TABLE merge_requests_approval_rules ( CONSTRAINT check_c7c36145b7 CHECK ((char_length(name) <= 255)) ); +CREATE TABLE merge_requests_approval_rules_approver_groups ( + id bigint NOT NULL, + approval_rule_id bigint NOT NULL, + group_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL +); + +CREATE SEQUENCE merge_requests_approval_rules_approver_groups_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_requests_approval_rules_approver_groups_id_seq OWNED BY merge_requests_approval_rules_approver_groups.id; + CREATE TABLE merge_requests_approval_rules_groups ( id bigint NOT NULL, approval_rule_id bigint NOT NULL, @@ -16465,6 +16482,26 @@ CREATE SEQUENCE merge_requests_approval_rules_projects_id_seq ALTER SEQUENCE merge_requests_approval_rules_projects_id_seq OWNED BY merge_requests_approval_rules_projects.id; +CREATE TABLE merge_requests_approval_rules_users ( + id bigint NOT NULL, + approval_rule_id bigint NOT NULL, + user_id bigint NOT NULL, + project_id bigint, + group_id bigint, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + CONSTRAINT check_cac54cf7e3 CHECK ((num_nonnulls(group_id, project_id) = 1)) +); + +CREATE SEQUENCE merge_requests_approval_rules_users_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_requests_approval_rules_users_id_seq OWNED BY merge_requests_approval_rules_users.id; + CREATE TABLE merge_requests_closing_issues ( id bigint NOT NULL, merge_request_id bigint NOT NULL, @@ -25829,12 +25866,16 @@ ALTER TABLE ONLY merge_requests ALTER COLUMN id SET DEFAULT nextval('merge_reque 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_approval_rules_approver_groups ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_approver_groups_id_seq'::regclass); + ALTER TABLE ONLY merge_requests_approval_rules_groups ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_groups_id_seq'::regclass); ALTER TABLE ONLY merge_requests_approval_rules_merge_requests ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_merge_requests_id_seq'::regclass); ALTER TABLE ONLY merge_requests_approval_rules_projects ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_projects_id_seq'::regclass); +ALTER TABLE ONLY merge_requests_approval_rules_users ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_users_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); @@ -28372,6 +28413,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_approver_groups + ADD CONSTRAINT merge_requests_approval_rules_approver_groups_pkey PRIMARY KEY (id); + ALTER TABLE ONLY merge_requests_approval_rules_groups ADD CONSTRAINT merge_requests_approval_rules_groups_pkey PRIMARY KEY (id); @@ -28384,6 +28428,9 @@ ALTER TABLE ONLY merge_requests_approval_rules ALTER TABLE ONLY merge_requests_approval_rules_projects ADD CONSTRAINT merge_requests_approval_rules_projects_pkey PRIMARY KEY (id); +ALTER TABLE ONLY merge_requests_approval_rules_users + ADD CONSTRAINT merge_requests_approval_rules_users_pkey PRIMARY KEY (id); + ALTER TABLE ONLY merge_requests_closing_issues ADD CONSTRAINT merge_requests_closing_issues_pkey PRIMARY KEY (id); @@ -33849,6 +33896,8 @@ 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_approver_groups_on_group_id ON merge_requests_approval_rules_approver_groups USING btree (group_id); + CREATE INDEX index_merge_requests_approval_rules_groups_on_group_id ON merge_requests_approval_rules_groups USING btree (group_id); CREATE INDEX index_merge_requests_approval_rules_on_group_id ON merge_requests_approval_rules USING btree (group_id); @@ -33857,6 +33906,12 @@ CREATE INDEX index_merge_requests_approval_rules_on_project_id ON merge_requests 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_approval_rules_users_on_group_id ON merge_requests_approval_rules_users USING btree (group_id); + +CREATE INDEX index_merge_requests_approval_rules_users_on_project_id ON merge_requests_approval_rules_users USING btree (project_id); + +CREATE INDEX index_merge_requests_approval_rules_users_on_user_id ON merge_requests_approval_rules_users USING btree (user_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); @@ -34035,12 +34090,16 @@ CREATE INDEX index_mrs_approval_rules_mrs_on_project_id ON merge_requests_approv CREATE INDEX index_mrs_approval_rules_projects_on_project_id ON merge_requests_approval_rules_projects USING btree (project_id); +CREATE UNIQUE INDEX index_mrs_ars_approver_groups_on_ar_id_and_group_id ON merge_requests_approval_rules_approver_groups USING btree (approval_rule_id, group_id); + CREATE UNIQUE INDEX index_mrs_ars_groups_on_ar_id_and_group_id ON merge_requests_approval_rules_groups USING btree (approval_rule_id, group_id); CREATE UNIQUE INDEX index_mrs_ars_mrs_on_ar_id_and_mr_id ON merge_requests_approval_rules_merge_requests USING btree (approval_rule_id, merge_request_id); CREATE UNIQUE INDEX index_mrs_ars_projects_on_ar_id_and_project_id ON merge_requests_approval_rules_projects USING btree (approval_rule_id, project_id); +CREATE UNIQUE INDEX index_mrs_ars_users_on_ar_id_and_user_id ON merge_requests_approval_rules_users USING btree (approval_rule_id, user_id); + CREATE INDEX index_namespace_admin_notes_on_namespace_id ON namespace_admin_notes USING btree (namespace_id); CREATE UNIQUE INDEX index_namespace_aggregation_schedules_on_namespace_id ON namespace_aggregation_schedules USING btree (namespace_id); @@ -39149,6 +39208,9 @@ ALTER TABLE ONLY zoekt_enabled_namespaces ALTER TABLE ONLY import_placeholder_memberships ADD CONSTRAINT fk_1f4659deee FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_approver_groups + ADD CONSTRAINT fk_1f8729ebf4 FOREIGN KEY (approval_rule_id) REFERENCES merge_requests_approval_rules(id) ON DELETE CASCADE; + ALTER TABLE ONLY epics ADD CONSTRAINT fk_1fbed67632 FOREIGN KEY (start_date_sourcing_milestone_id) REFERENCES milestones(id) ON DELETE SET NULL; @@ -39704,6 +39766,9 @@ ALTER TABLE p_ci_builds ALTER TABLE ONLY routes ADD CONSTRAINT fk_679ff8213d FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE NOT VALID; +ALTER TABLE ONLY merge_requests_approval_rules_approver_groups + ADD CONSTRAINT fk_67fa93ad4b FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY ai_conversation_messages ADD CONSTRAINT fk_68774ec148 FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; @@ -39743,6 +39808,9 @@ ALTER TABLE ONLY deploy_tokens ALTER TABLE ONLY oauth_openid_requests ADD CONSTRAINT fk_7092424b77 FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_users + ADD CONSTRAINT fk_709cb411cb FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY bulk_import_failures ADD CONSTRAINT fk_70f30b02fd FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; @@ -40010,6 +40078,9 @@ ALTER TABLE ONLY audit_events_streaming_group_namespace_filters ALTER TABLE ONLY compliance_requirements ADD CONSTRAINT fk_8f5fb77fc7 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_users + ADD CONSTRAINT fk_8fd456df22 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY catalog_resource_component_last_usages ADD CONSTRAINT fk_909d62907f FOREIGN KEY (component_id) REFERENCES catalog_resource_components(id) ON DELETE CASCADE; @@ -40259,6 +40330,9 @@ ALTER TABLE ONLY design_management_designs_versions ALTER TABLE ONLY project_deletion_schedules ADD CONSTRAINT fk_b11f7e2219 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_users + ADD CONSTRAINT fk_b12b222020 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ALTER TABLE ONLY project_deletion_schedules ADD CONSTRAINT fk_b140171ca8 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; @@ -40550,6 +40624,9 @@ ALTER TABLE ONLY subscription_user_add_on_assignments ALTER TABLE ONLY project_mirror_data ADD CONSTRAINT fk_d1aad367d7 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_users + ADD CONSTRAINT fk_d1c23df23f FOREIGN KEY (approval_rule_id) REFERENCES merge_requests_approval_rules(id) ON DELETE CASCADE; + ALTER TABLE ONLY environments ADD CONSTRAINT fk_d1c8c1da6a FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/app/models/merge_requests/approval_rule.rb b/ee/app/models/merge_requests/approval_rule.rb index 8f002d21655a8e..5329815957c2b0 100644 --- a/ee/app/models/merge_requests/approval_rule.rb +++ b/ee/app/models/merge_requests/approval_rule.rb @@ -28,6 +28,12 @@ class ApprovalRule < ApplicationRecord has_one :approval_rules_merge_request, inverse_of: :approval_rule has_one :merge_request, through: :approval_rules_merge_request + has_many :approval_rules_users + has_many :users, through: :approval_rules_users + + has_many :approval_rules_approver_groups + has_many :approver_groups, through: :approval_rules_approver_groups, source: :group + validate :ensure_single_sharding_key with_options validate: true do diff --git a/ee/app/models/merge_requests/approval_rules_approver_group.rb b/ee/app/models/merge_requests/approval_rules_approver_group.rb new file mode 100644 index 00000000000000..3eaa95c5b1ce07 --- /dev/null +++ b/ee/app/models/merge_requests/approval_rules_approver_group.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module MergeRequests + class ApprovalRulesApproverGroup < ApplicationRecord + self.table_name = 'merge_requests_approval_rules_approver_groups' + + belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' + belongs_to :group + end +end diff --git a/ee/app/models/merge_requests/approval_rules_user.rb b/ee/app/models/merge_requests/approval_rules_user.rb new file mode 100644 index 00000000000000..f2dec7229f53ce --- /dev/null +++ b/ee/app/models/merge_requests/approval_rules_user.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module MergeRequests + class ApprovalRulesUser < ApplicationRecord + self.table_name = 'merge_requests_approval_rules_users' + + belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' + belongs_to :user + + before_validation :set_sharding_key + + def set_sharding_key + if approval_rule.origin == 'group' + self.group_id = approval_rule.group_id + + return + end + + self.project_id = approval_rule.project_id + end + end +end diff --git a/ee/spec/factories/merge_requests/approval_rules_approver_groups.rb b/ee/spec/factories/merge_requests/approval_rules_approver_groups.rb new file mode 100644 index 00000000000000..0dd53bb01d0312 --- /dev/null +++ b/ee/spec/factories/merge_requests/approval_rules_approver_groups.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_requests_approval_rules_approver_group, class: 'MergeRequests::ApprovalRulesApproverGroup' do + association :approval_rule, factory: :merge_requests_approval_rule + association :group, factory: :group + end +end diff --git a/ee/spec/factories/merge_requests/approval_rules_users.rb b/ee/spec/factories/merge_requests/approval_rules_users.rb new file mode 100644 index 00000000000000..545576397c3f6a --- /dev/null +++ b/ee/spec/factories/merge_requests/approval_rules_users.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_requests_approval_rules_user, class: 'MergeRequests::ApprovalRulesUser' do + association :approval_rule, factory: :merge_requests_approval_rule + association :user, factory: :user + end +end diff --git a/ee/spec/models/merge_requests/approval_rules_approver_group_spec.rb b/ee/spec/models/merge_requests/approval_rules_approver_group_spec.rb new file mode 100644 index 00000000000000..0cb13db7288951 --- /dev/null +++ b/ee/spec/models/merge_requests/approval_rules_approver_group_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::ApprovalRulesApproverGroup, type: :model, feature_category: :code_review_workflow do + describe 'associations' do + it { is_expected.to belong_to(:approval_rule) } + it { is_expected.to belong_to(:group) } + end +end diff --git a/ee/spec/models/merge_requests/approval_rules_user_spec.rb b/ee/spec/models/merge_requests/approval_rules_user_spec.rb new file mode 100644 index 00000000000000..c1f949b117fe5a --- /dev/null +++ b/ee/spec/models/merge_requests/approval_rules_user_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::ApprovalRulesUser, type: :model, feature_category: :code_review_workflow do + describe '#set_sharding_key' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let(:approval_rule) { create(:merge_requests_approval_rule, origin: 0, group_id: group.id) } + + subject(:approval_rules_user) do + create(:merge_requests_approval_rules_user, user: user, approval_rule: approval_rule) + end + + describe '#set_sharding_key' do + context 'when approval rule origin is group' do + it 'sets the group_id' do + expect(approval_rules_user.group_id).to eq(group.id) + end + + it 'does not set the project_id' do + expect(approval_rules_user.project_id).to be_nil + end + end + + context 'when rule origin is not group' do + let_it_be(:project) { create(:project) } + let(:origin) { 1 } + let(:approval_rule) { create(:merge_requests_approval_rule, origin: origin, project_id: project.id) } + + context 'when approval rule origin is project' do + it 'sets the project_id' do + expect(approval_rules_user.project_id).to eq(project.id) + end + + it 'does not set the group_id' do + expect(approval_rules_user.group_id).to be_nil + end + end + + context 'when approval rule origin is merge request' do + let(:origin) { 2 } + + it 'sets the project_id' do + expect(approval_rules_user.project_id).to eq(project.id) + end + + it 'does not set the group_id' do + expect(approval_rules_user.group_id).to be_nil + end + end + end + end + end +end -- GitLab From 976b13f4817b1c209713058ebc2ee79048176662 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Thu, 27 Feb 2025 09:13:31 +0000 Subject: [PATCH 2/4] Use correct method for enum --- ee/app/models/merge_requests/approval_rules_user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/models/merge_requests/approval_rules_user.rb b/ee/app/models/merge_requests/approval_rules_user.rb index f2dec7229f53ce..8a16e335f71e36 100644 --- a/ee/app/models/merge_requests/approval_rules_user.rb +++ b/ee/app/models/merge_requests/approval_rules_user.rb @@ -10,7 +10,7 @@ class ApprovalRulesUser < ApplicationRecord before_validation :set_sharding_key def set_sharding_key - if approval_rule.origin == 'group' + if approval_rule.originates_from_group? self.group_id = approval_rule.group_id return -- GitLab From 667d6e0c7dcaffcc2c01b60888c5589ae2f159f3 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Wed, 5 Mar 2025 09:29:41 +0000 Subject: [PATCH 3/4] Review changes: change from users to approver_users --- ...equests_approval_rules_approver_users.yml} | 6 +- ...requests_approval_rules_approver_users.rb} | 8 +- ...e_requests_approval_rules_users_user_fk.rb | 5 +- ...equests_approval_rules_users_project_fk.rb | 5 +- ...s_approval_rules_users_approval_rule_fk.rb | 4 +- ..._requests_approval_rules_users_group_fk.rb | 5 +- ..._users_multi_column_not_null_constraint.rb | 4 +- db/structure.sql | 88 +++++++++---------- ...ser.rb => approval_rules_approver_user.rb} | 4 +- .../merge_requests/approval_rules.rb | 13 ++- ...rs.rb => approval_rules_approver_users.rb} | 2 +- ...b => approval_rules_approver_user_spec.rb} | 12 +-- 12 files changed, 85 insertions(+), 71 deletions(-) rename db/docs/{merge_requests_approval_rules_users.yml => merge_requests_approval_rules_approver_users.yml} (61%) rename db/migrate/{20250123151859_create_merge_requests_approval_rules_users.rb => 20250123151859_create_merge_requests_approval_rules_approver_users.rb} (61%) rename ee/app/models/merge_requests/{approval_rules_user.rb => approval_rules_approver_user.rb} (76%) rename ee/spec/factories/merge_requests/{approval_rules_users.rb => approval_rules_approver_users.rb} (86%) rename ee/spec/models/merge_requests/{approval_rules_user_spec.rb => approval_rules_approver_user_spec.rb} (81%) diff --git a/db/docs/merge_requests_approval_rules_users.yml b/db/docs/merge_requests_approval_rules_approver_users.yml similarity index 61% rename from db/docs/merge_requests_approval_rules_users.yml rename to db/docs/merge_requests_approval_rules_approver_users.yml index 096697ec2d987f..084da085100d62 100644 --- a/db/docs/merge_requests_approval_rules_users.yml +++ b/db/docs/merge_requests_approval_rules_approver_users.yml @@ -1,10 +1,10 @@ --- -table_name: merge_requests_approval_rules_users +table_name: merge_requests_approval_rules_approver_users classes: - - MergeRequests::ApprovalRulesUser + - MergeRequests::ApprovalRulesApproverUser feature_categories: - code_review_workflow -description: Stores approval users for approval rules v2 +description: Stores approver users for approval rules v2 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179877 milestone: '17.10' gitlab_schema: gitlab_main_cell diff --git a/db/migrate/20250123151859_create_merge_requests_approval_rules_users.rb b/db/migrate/20250123151859_create_merge_requests_approval_rules_approver_users.rb similarity index 61% rename from db/migrate/20250123151859_create_merge_requests_approval_rules_users.rb rename to db/migrate/20250123151859_create_merge_requests_approval_rules_approver_users.rb index c4a66163a6843f..23255e8dbe2fb4 100644 --- a/db/migrate/20250123151859_create_merge_requests_approval_rules_users.rb +++ b/db/migrate/20250123151859_create_merge_requests_approval_rules_approver_users.rb @@ -1,23 +1,23 @@ # frozen_string_literal: true -class CreateMergeRequestsApprovalRulesUsers < Gitlab::Database::Migration[2.2] +class CreateMergeRequestsApprovalRulesApproverUsers < Gitlab::Database::Migration[2.2] milestone '17.10' def change - create_table :merge_requests_approval_rules_users do |t| + create_table :merge_requests_approval_rules_approver_users do |t| t.bigint :approval_rule_id, null: false t.bigint :user_id, null: false t.bigint :project_id, null: true t.bigint :group_id, null: true t.index :user_id - t.index :project_id + t.index :project_id, name: 'index_mrs_approval_rules_approver_users_on_project_id' t.index :group_id t.timestamps_with_timezone null: false end add_index( - :merge_requests_approval_rules_users, + :merge_requests_approval_rules_approver_users, %i[approval_rule_id user_id], unique: true, name: 'index_mrs_ars_users_on_ar_id_and_user_id' diff --git a/db/migrate/20250214134117_add_merge_requests_approval_rules_users_user_fk.rb b/db/migrate/20250214134117_add_merge_requests_approval_rules_users_user_fk.rb index b475091f798e94..5b0c1c1afa90ee 100644 --- a/db/migrate/20250214134117_add_merge_requests_approval_rules_users_user_fk.rb +++ b/db/migrate/20250214134117_add_merge_requests_approval_rules_users_user_fk.rb @@ -5,12 +5,13 @@ class AddMergeRequestsApprovalRulesUsersUserFk < Gitlab::Database::Migration[2.2 disable_ddl_transaction! def up - add_concurrent_foreign_key :merge_requests_approval_rules_users, :users, column: :user_id, on_delete: :cascade + add_concurrent_foreign_key :merge_requests_approval_rules_approver_users, :users, column: :user_id, + on_delete: :cascade end def down with_lock_retries do - remove_foreign_key :merge_requests_approval_rules_users, column: :user_id + remove_foreign_key :merge_requests_approval_rules_approver_users, column: :user_id end end end diff --git a/db/migrate/20250214134138_add_merge_requests_approval_rules_users_project_fk.rb b/db/migrate/20250214134138_add_merge_requests_approval_rules_users_project_fk.rb index 2335a31fd18ef2..7d6043024ad51e 100644 --- a/db/migrate/20250214134138_add_merge_requests_approval_rules_users_project_fk.rb +++ b/db/migrate/20250214134138_add_merge_requests_approval_rules_users_project_fk.rb @@ -5,12 +5,13 @@ class AddMergeRequestsApprovalRulesUsersProjectFk < Gitlab::Database::Migration[ disable_ddl_transaction! def up - add_concurrent_foreign_key :merge_requests_approval_rules_users, :projects, column: :project_id, on_delete: :cascade + add_concurrent_foreign_key :merge_requests_approval_rules_approver_users, :projects, column: :project_id, + on_delete: :cascade end def down with_lock_retries do - remove_foreign_key :merge_requests_approval_rules_users, column: :project_id + remove_foreign_key :merge_requests_approval_rules_approver_users, column: :project_id end end end diff --git a/db/migrate/20250214134202_add_merge_requests_approval_rules_users_approval_rule_fk.rb b/db/migrate/20250214134202_add_merge_requests_approval_rules_users_approval_rule_fk.rb index 2bb57d551d17f5..807affa8945512 100644 --- a/db/migrate/20250214134202_add_merge_requests_approval_rules_users_approval_rule_fk.rb +++ b/db/migrate/20250214134202_add_merge_requests_approval_rules_users_approval_rule_fk.rb @@ -5,13 +5,13 @@ class AddMergeRequestsApprovalRulesUsersApprovalRuleFk < Gitlab::Database::Migra disable_ddl_transaction! def up - add_concurrent_foreign_key :merge_requests_approval_rules_users, :merge_requests_approval_rules, + add_concurrent_foreign_key :merge_requests_approval_rules_approver_users, :merge_requests_approval_rules, column: :approval_rule_id, on_delete: :cascade end def down with_lock_retries do - remove_foreign_key :merge_requests_approval_rules_users, column: :approval_rule_id + remove_foreign_key :merge_requests_approval_rules_approver_users, column: :approval_rule_id end end end diff --git a/db/migrate/20250225112059_add_merge_requests_approval_rules_users_group_fk.rb b/db/migrate/20250225112059_add_merge_requests_approval_rules_users_group_fk.rb index cace63dd3214e5..04bd8117d25285 100644 --- a/db/migrate/20250225112059_add_merge_requests_approval_rules_users_group_fk.rb +++ b/db/migrate/20250225112059_add_merge_requests_approval_rules_users_group_fk.rb @@ -5,12 +5,13 @@ class AddMergeRequestsApprovalRulesUsersGroupFk < Gitlab::Database::Migration[2. disable_ddl_transaction! def up - add_concurrent_foreign_key :merge_requests_approval_rules_users, :namespaces, column: :group_id, on_delete: :cascade + add_concurrent_foreign_key :merge_requests_approval_rules_approver_users, :namespaces, column: :group_id, + on_delete: :cascade end def down with_lock_retries do - remove_foreign_key :merge_requests_approval_rules_users, column: :group_id + remove_foreign_key :merge_requests_approval_rules_approver_users, column: :group_id end end end diff --git a/db/migrate/20250225112409_add_merge_requests_approval_rules_users_multi_column_not_null_constraint.rb b/db/migrate/20250225112409_add_merge_requests_approval_rules_users_multi_column_not_null_constraint.rb index 4970f1621ad54f..e7d408a90de5f5 100644 --- a/db/migrate/20250225112409_add_merge_requests_approval_rules_users_multi_column_not_null_constraint.rb +++ b/db/migrate/20250225112409_add_merge_requests_approval_rules_users_multi_column_not_null_constraint.rb @@ -5,10 +5,10 @@ class AddMergeRequestsApprovalRulesUsersMultiColumnNotNullConstraint < Gitlab::D disable_ddl_transaction! def up - add_multi_column_not_null_constraint(:merge_requests_approval_rules_users, :group_id, :project_id) + add_multi_column_not_null_constraint(:merge_requests_approval_rules_approver_users, :group_id, :project_id) end def down - remove_multi_column_not_null_constraint(:merge_requests_approval_rules_users, :group_id, :project_id) + remove_multi_column_not_null_constraint(:merge_requests_approval_rules_approver_users, :group_id, :project_id) end end diff --git a/db/structure.sql b/db/structure.sql index 35bd3a6033588d..28da8ed7bbf4d3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16421,6 +16421,26 @@ CREATE SEQUENCE merge_requests_approval_rules_approver_groups_id_seq ALTER SEQUENCE merge_requests_approval_rules_approver_groups_id_seq OWNED BY merge_requests_approval_rules_approver_groups.id; +CREATE TABLE merge_requests_approval_rules_approver_users ( + id bigint NOT NULL, + approval_rule_id bigint NOT NULL, + user_id bigint NOT NULL, + project_id bigint, + group_id bigint, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + CONSTRAINT check_ccdbd0e37e CHECK ((num_nonnulls(group_id, project_id) = 1)) +); + +CREATE SEQUENCE merge_requests_approval_rules_approver_users_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_requests_approval_rules_approver_users_id_seq OWNED BY merge_requests_approval_rules_approver_users.id; + CREATE TABLE merge_requests_approval_rules_groups ( id bigint NOT NULL, approval_rule_id bigint NOT NULL, @@ -16482,26 +16502,6 @@ CREATE SEQUENCE merge_requests_approval_rules_projects_id_seq ALTER SEQUENCE merge_requests_approval_rules_projects_id_seq OWNED BY merge_requests_approval_rules_projects.id; -CREATE TABLE merge_requests_approval_rules_users ( - id bigint NOT NULL, - approval_rule_id bigint NOT NULL, - user_id bigint NOT NULL, - project_id bigint, - group_id bigint, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, - CONSTRAINT check_cac54cf7e3 CHECK ((num_nonnulls(group_id, project_id) = 1)) -); - -CREATE SEQUENCE merge_requests_approval_rules_users_id_seq - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE merge_requests_approval_rules_users_id_seq OWNED BY merge_requests_approval_rules_users.id; - CREATE TABLE merge_requests_closing_issues ( id bigint NOT NULL, merge_request_id bigint NOT NULL, @@ -25868,14 +25868,14 @@ ALTER TABLE ONLY merge_requests_approval_rules ALTER COLUMN id SET DEFAULT nextv ALTER TABLE ONLY merge_requests_approval_rules_approver_groups ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_approver_groups_id_seq'::regclass); +ALTER TABLE ONLY merge_requests_approval_rules_approver_users ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_approver_users_id_seq'::regclass); + ALTER TABLE ONLY merge_requests_approval_rules_groups ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_groups_id_seq'::regclass); ALTER TABLE ONLY merge_requests_approval_rules_merge_requests ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_merge_requests_id_seq'::regclass); ALTER TABLE ONLY merge_requests_approval_rules_projects ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_projects_id_seq'::regclass); -ALTER TABLE ONLY merge_requests_approval_rules_users ALTER COLUMN id SET DEFAULT nextval('merge_requests_approval_rules_users_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); @@ -28416,6 +28416,9 @@ ALTER TABLE ONLY merge_request_user_mentions ALTER TABLE ONLY merge_requests_approval_rules_approver_groups ADD CONSTRAINT merge_requests_approval_rules_approver_groups_pkey PRIMARY KEY (id); +ALTER TABLE ONLY merge_requests_approval_rules_approver_users + ADD CONSTRAINT merge_requests_approval_rules_approver_users_pkey PRIMARY KEY (id); + ALTER TABLE ONLY merge_requests_approval_rules_groups ADD CONSTRAINT merge_requests_approval_rules_groups_pkey PRIMARY KEY (id); @@ -28428,9 +28431,6 @@ ALTER TABLE ONLY merge_requests_approval_rules ALTER TABLE ONLY merge_requests_approval_rules_projects ADD CONSTRAINT merge_requests_approval_rules_projects_pkey PRIMARY KEY (id); -ALTER TABLE ONLY merge_requests_approval_rules_users - ADD CONSTRAINT merge_requests_approval_rules_users_pkey PRIMARY KEY (id); - ALTER TABLE ONLY merge_requests_closing_issues ADD CONSTRAINT merge_requests_closing_issues_pkey PRIMARY KEY (id); @@ -33898,6 +33898,10 @@ CREATE UNIQUE INDEX index_merge_request_user_mentions_on_note_id ON merge_reques CREATE INDEX index_merge_requests_approval_rules_approver_groups_on_group_id ON merge_requests_approval_rules_approver_groups USING btree (group_id); +CREATE INDEX index_merge_requests_approval_rules_approver_users_on_group_id ON merge_requests_approval_rules_approver_users USING btree (group_id); + +CREATE INDEX index_merge_requests_approval_rules_approver_users_on_user_id ON merge_requests_approval_rules_approver_users USING btree (user_id); + CREATE INDEX index_merge_requests_approval_rules_groups_on_group_id ON merge_requests_approval_rules_groups USING btree (group_id); CREATE INDEX index_merge_requests_approval_rules_on_group_id ON merge_requests_approval_rules USING btree (group_id); @@ -33906,12 +33910,6 @@ CREATE INDEX index_merge_requests_approval_rules_on_project_id ON merge_requests 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_approval_rules_users_on_group_id ON merge_requests_approval_rules_users USING btree (group_id); - -CREATE INDEX index_merge_requests_approval_rules_users_on_project_id ON merge_requests_approval_rules_users USING btree (project_id); - -CREATE INDEX index_merge_requests_approval_rules_users_on_user_id ON merge_requests_approval_rules_users USING btree (user_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); @@ -34084,6 +34082,8 @@ CREATE INDEX index_mr_metrics_on_target_project_id_merged_at_nulls_last ON merge CREATE INDEX index_mr_metrics_on_target_project_id_merged_at_time_to_merge ON merge_request_metrics USING btree (target_project_id, merged_at, created_at) WHERE (merged_at > created_at); +CREATE INDEX index_mrs_approval_rules_approver_users_on_project_id ON merge_requests_approval_rules_approver_users USING btree (project_id); + CREATE INDEX index_mrs_approval_rules_mrs_on_mr_id ON merge_requests_approval_rules_merge_requests USING btree (merge_request_id); CREATE INDEX index_mrs_approval_rules_mrs_on_project_id ON merge_requests_approval_rules_merge_requests USING btree (project_id); @@ -34098,7 +34098,7 @@ CREATE UNIQUE INDEX index_mrs_ars_mrs_on_ar_id_and_mr_id ON merge_requests_appro CREATE UNIQUE INDEX index_mrs_ars_projects_on_ar_id_and_project_id ON merge_requests_approval_rules_projects USING btree (approval_rule_id, project_id); -CREATE UNIQUE INDEX index_mrs_ars_users_on_ar_id_and_user_id ON merge_requests_approval_rules_users USING btree (approval_rule_id, user_id); +CREATE UNIQUE INDEX index_mrs_ars_users_on_ar_id_and_user_id ON merge_requests_approval_rules_approver_users USING btree (approval_rule_id, user_id); CREATE INDEX index_namespace_admin_notes_on_namespace_id ON namespace_admin_notes USING btree (namespace_id); @@ -39490,6 +39490,9 @@ ALTER TABLE ONLY security_orchestration_policy_rule_schedules ALTER TABLE ONLY abuse_reports ADD CONSTRAINT fk_3fe6467b93 FOREIGN KEY (assignee_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY merge_requests_approval_rules_approver_users + ADD CONSTRAINT fk_4025feea5b FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; + ALTER TABLE ONLY protected_environment_approval_rules ADD CONSTRAINT fk_405568b491 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; @@ -39646,6 +39649,9 @@ ALTER TABLE ONLY abuse_report_notes ALTER TABLE ONLY approval_merge_request_rules ADD CONSTRAINT fk_5822f009ea FOREIGN KEY (security_orchestration_policy_configuration_id) REFERENCES security_orchestration_policy_configurations(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_approver_users + ADD CONSTRAINT fk_582e5f36e8 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY deploy_keys_projects ADD CONSTRAINT fk_58a901ca7e FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; @@ -39808,9 +39814,6 @@ ALTER TABLE ONLY deploy_tokens ALTER TABLE ONLY oauth_openid_requests ADD CONSTRAINT fk_7092424b77 FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; -ALTER TABLE ONLY merge_requests_approval_rules_users - ADD CONSTRAINT fk_709cb411cb FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; - ALTER TABLE ONLY bulk_import_failures ADD CONSTRAINT fk_70f30b02fd FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; @@ -39829,6 +39832,9 @@ ALTER TABLE ONLY packages_conan_package_references ALTER TABLE ONLY subscription_user_add_on_assignments ADD CONSTRAINT fk_724c2df9a8 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_approver_users + ADD CONSTRAINT fk_725cca295c FOREIGN KEY (approval_rule_id) REFERENCES merge_requests_approval_rules(id) ON DELETE CASCADE; + ALTER TABLE ONLY zentao_tracker_data ADD CONSTRAINT fk_72a0e59cd8 FOREIGN KEY (instance_integration_id) REFERENCES instance_integrations(id) ON DELETE CASCADE; @@ -39982,6 +39988,9 @@ ALTER TABLE ONLY import_export_uploads ALTER TABLE ONLY packages_npm_metadata ADD CONSTRAINT fk_83625a27c0 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_approver_users + ADD CONSTRAINT fk_836efc3006 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY push_rules ADD CONSTRAINT fk_83b29894de FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; @@ -40078,9 +40087,6 @@ ALTER TABLE ONLY audit_events_streaming_group_namespace_filters ALTER TABLE ONLY compliance_requirements ADD CONSTRAINT fk_8f5fb77fc7 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; -ALTER TABLE ONLY merge_requests_approval_rules_users - ADD CONSTRAINT fk_8fd456df22 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; - ALTER TABLE ONLY catalog_resource_component_last_usages ADD CONSTRAINT fk_909d62907f FOREIGN KEY (component_id) REFERENCES catalog_resource_components(id) ON DELETE CASCADE; @@ -40330,9 +40336,6 @@ ALTER TABLE ONLY design_management_designs_versions ALTER TABLE ONLY project_deletion_schedules ADD CONSTRAINT fk_b11f7e2219 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; -ALTER TABLE ONLY merge_requests_approval_rules_users - ADD CONSTRAINT fk_b12b222020 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; - ALTER TABLE ONLY project_deletion_schedules ADD CONSTRAINT fk_b140171ca8 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; @@ -40624,9 +40627,6 @@ ALTER TABLE ONLY subscription_user_add_on_assignments ALTER TABLE ONLY project_mirror_data ADD CONSTRAINT fk_d1aad367d7 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; -ALTER TABLE ONLY merge_requests_approval_rules_users - ADD CONSTRAINT fk_d1c23df23f FOREIGN KEY (approval_rule_id) REFERENCES merge_requests_approval_rules(id) ON DELETE CASCADE; - ALTER TABLE ONLY environments ADD CONSTRAINT fk_d1c8c1da6a FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/app/models/merge_requests/approval_rules_user.rb b/ee/app/models/merge_requests/approval_rules_approver_user.rb similarity index 76% rename from ee/app/models/merge_requests/approval_rules_user.rb rename to ee/app/models/merge_requests/approval_rules_approver_user.rb index 8a16e335f71e36..1fa00195ea6c94 100644 --- a/ee/app/models/merge_requests/approval_rules_user.rb +++ b/ee/app/models/merge_requests/approval_rules_approver_user.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true module MergeRequests - class ApprovalRulesUser < ApplicationRecord - self.table_name = 'merge_requests_approval_rules_users' + class ApprovalRulesApproverUser < ApplicationRecord + self.table_name = 'merge_requests_approval_rules_approver_users' belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' belongs_to :user diff --git a/ee/spec/factories/merge_requests/approval_rules.rb b/ee/spec/factories/merge_requests/approval_rules.rb index a56d74503f402a..0abcabbfbb6a30 100644 --- a/ee/spec/factories/merge_requests/approval_rules.rb +++ b/ee/spec/factories/merge_requests/approval_rules.rb @@ -5,10 +5,21 @@ 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 + + trait :from_group do + origin { 0 } + end + + trait :from_project do + origin { 1 } + end + + trait :from_merge_request do + origin { 2 } + end end end diff --git a/ee/spec/factories/merge_requests/approval_rules_users.rb b/ee/spec/factories/merge_requests/approval_rules_approver_users.rb similarity index 86% rename from ee/spec/factories/merge_requests/approval_rules_users.rb rename to ee/spec/factories/merge_requests/approval_rules_approver_users.rb index 545576397c3f6a..dafce8ef10a4c0 100644 --- a/ee/spec/factories/merge_requests/approval_rules_users.rb +++ b/ee/spec/factories/merge_requests/approval_rules_approver_users.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :merge_requests_approval_rules_user, class: 'MergeRequests::ApprovalRulesUser' do + factory :merge_requests_approval_rules_user, class: 'MergeRequests::ApprovalRulesApproverUser' do association :approval_rule, factory: :merge_requests_approval_rule association :user, factory: :user end diff --git a/ee/spec/models/merge_requests/approval_rules_user_spec.rb b/ee/spec/models/merge_requests/approval_rules_approver_user_spec.rb similarity index 81% rename from ee/spec/models/merge_requests/approval_rules_user_spec.rb rename to ee/spec/models/merge_requests/approval_rules_approver_user_spec.rb index c1f949b117fe5a..a2cae5d9ff4505 100644 --- a/ee/spec/models/merge_requests/approval_rules_user_spec.rb +++ b/ee/spec/models/merge_requests/approval_rules_approver_user_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' -RSpec.describe MergeRequests::ApprovalRulesUser, type: :model, feature_category: :code_review_workflow do +RSpec.describe MergeRequests::ApprovalRulesApproverUser, type: :model, feature_category: :code_review_workflow do describe '#set_sharding_key' do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } - let(:approval_rule) { create(:merge_requests_approval_rule, origin: 0, group_id: group.id) } + let(:approval_rule) { create(:merge_requests_approval_rule, :from_group, group_id: group.id) } subject(:approval_rules_user) do create(:merge_requests_approval_rules_user, user: user, approval_rule: approval_rule) @@ -23,10 +23,10 @@ end end - context 'when rule origin is not group' do + context 'when approval rule origin is not group' do let_it_be(:project) { create(:project) } - let(:origin) { 1 } - let(:approval_rule) { create(:merge_requests_approval_rule, origin: origin, project_id: project.id) } + let(:origin) { :from_project } + let(:approval_rule) { create(:merge_requests_approval_rule, origin, project_id: project.id) } context 'when approval rule origin is project' do it 'sets the project_id' do @@ -39,7 +39,7 @@ end context 'when approval rule origin is merge request' do - let(:origin) { 2 } + let(:origin) { :from_merge_request } it 'sets the project_id' do expect(approval_rules_user.project_id).to eq(project.id) -- GitLab From 61b5dddaca5fa8ee96778fa6f5ac3dea5a68034a Mon Sep 17 00:00:00 2001 From: ghinfey Date: Mon, 10 Mar 2025 09:31:16 +0000 Subject: [PATCH 4/4] review suggestions to approver user model --- .../models/merge_requests/approval_rules_approver_user.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ee/app/models/merge_requests/approval_rules_approver_user.rb b/ee/app/models/merge_requests/approval_rules_approver_user.rb index 1fa00195ea6c94..cbdd4e92a460e1 100644 --- a/ee/app/models/merge_requests/approval_rules_approver_user.rb +++ b/ee/app/models/merge_requests/approval_rules_approver_user.rb @@ -9,12 +9,10 @@ class ApprovalRulesApproverUser < ApplicationRecord before_validation :set_sharding_key - def set_sharding_key - if approval_rule.originates_from_group? - self.group_id = approval_rule.group_id + private - return - end + def set_sharding_key + return self.group_id = approval_rule.group_id if approval_rule.originates_from_group? self.project_id = approval_rule.project_id end -- GitLab