From 32aebac9e452247b1f829d16f634bdc9565825e6 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Fri, 31 Jan 2025 12:18:09 +0000 Subject: [PATCH 1/8] Add v2 approval rule table and model Adds new approval rule table and model for new approval rule architecture. Changelog: changed MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179839 EE: true --- ...oval_rules_multi_column_not_null_constraint.rb | 15 +++++++++++++++ db/schema_migrations/20250130110721 | 1 + ee/app/models/ee/group.rb | 2 ++ ee/app/models/ee/project.rb | 2 ++ spec/lib/gitlab/import_export/all_models.yml | 2 ++ 5 files changed, 22 insertions(+) create mode 100644 db/migrate/20250130110721_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb create mode 100644 db/schema_migrations/20250130110721 diff --git a/db/migrate/20250130110721_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb b/db/migrate/20250130110721_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb new file mode 100644 index 00000000000000..24e0e203b532c4 --- /dev/null +++ b/db/migrate/20250130110721_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb @@ -0,0 +1,15 @@ +# 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/20250130110721 b/db/schema_migrations/20250130110721 new file mode 100644 index 00000000000000..7a1e2fbe886aeb --- /dev/null +++ b/db/schema_migrations/20250130110721 @@ -0,0 +1 @@ +4804fd7719ef467850550e1a488d544ec412f83e51335a367eca119ba550161c \ No newline at end of file diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 13de06a8d0d8fc..a3c9f638555296 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -99,6 +99,8 @@ module Group has_many :security_exclusions, class_name: 'Security::GroupSecurityExclusion' + has_many :v2_approval_rules_from_group_origin, class_name: 'MergeRequests::ApprovalRule', inverse_of: :source_group + delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true delegate :repository_read_only, diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index d3448eca50ff28..96007512310c5e 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -217,6 +217,8 @@ def lock_for_confirmation!(id) has_many :project_control_compliance_statuses, class_name: 'ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus' + has_many :v2_approval_rules_from_project_origin, class_name: 'MergeRequests::ApprovalRule', inverse_of: :source_project + elastic_index_dependant_association :issues, on_change: :visibility_level elastic_index_dependant_association :issues, on_change: :archived elastic_index_dependant_association :work_items, on_change: :visibility_level diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index b2bff3b96eda89..45e952cb1d85f7 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -255,6 +255,7 @@ merge_requests: - approver_users - approver_groups - approved_by_users +- v2_approval_rules_from_project_origin - draft_notes - merge_train_car - blocks_as_blocker @@ -769,6 +770,7 @@ project: - approval_merge_request_rules - approval_merge_request_rule_sources - approval_project_rules +- v2_approval_rules_from_project_origin - approvers - approver_users - audit_events -- GitLab From 957c549f2ac1d4b93a2a78aa358239c7b17e43c4 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Tue, 4 Feb 2025 11:26:20 +0000 Subject: [PATCH 2/8] Remove association for sharding --- ee/app/models/ee/group.rb | 2 -- ee/app/models/ee/project.rb | 2 -- spec/lib/gitlab/import_export/all_models.yml | 2 -- 3 files changed, 6 deletions(-) diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index a3c9f638555296..13de06a8d0d8fc 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -99,8 +99,6 @@ module Group has_many :security_exclusions, class_name: 'Security::GroupSecurityExclusion' - has_many :v2_approval_rules_from_group_origin, class_name: 'MergeRequests::ApprovalRule', inverse_of: :source_group - delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true delegate :repository_read_only, diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 96007512310c5e..d3448eca50ff28 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -217,8 +217,6 @@ def lock_for_confirmation!(id) has_many :project_control_compliance_statuses, class_name: 'ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus' - has_many :v2_approval_rules_from_project_origin, class_name: 'MergeRequests::ApprovalRule', inverse_of: :source_project - elastic_index_dependant_association :issues, on_change: :visibility_level elastic_index_dependant_association :issues, on_change: :archived elastic_index_dependant_association :work_items, on_change: :visibility_level diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 45e952cb1d85f7..b2bff3b96eda89 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -255,7 +255,6 @@ merge_requests: - approver_users - approver_groups - approved_by_users -- v2_approval_rules_from_project_origin - draft_notes - merge_train_car - blocks_as_blocker @@ -770,7 +769,6 @@ project: - approval_merge_request_rules - approval_merge_request_rule_sources - approval_project_rules -- v2_approval_rules_from_project_origin - approvers - approver_users - audit_events -- GitLab From 8e5ca9cc12b8e26bb006917c02c5720d33a4f0bf Mon Sep 17 00:00:00 2001 From: ghinfey Date: Wed, 5 Feb 2025 10:44:54 +0000 Subject: [PATCH 3/8] Add foreign keys in seperate migrations --- ...oval_rules_multi_column_not_null_constraint.rb | 15 --------------- db/schema_migrations/20250130110721 | 1 - 2 files changed, 16 deletions(-) delete mode 100644 db/migrate/20250130110721_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb delete mode 100644 db/schema_migrations/20250130110721 diff --git a/db/migrate/20250130110721_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb b/db/migrate/20250130110721_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb deleted file mode 100644 index 24e0e203b532c4..00000000000000 --- a/db/migrate/20250130110721_add_merge_requests_approval_rules_multi_column_not_null_constraint.rb +++ /dev/null @@ -1,15 +0,0 @@ -# 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/20250130110721 b/db/schema_migrations/20250130110721 deleted file mode 100644 index 7a1e2fbe886aeb..00000000000000 --- a/db/schema_migrations/20250130110721 +++ /dev/null @@ -1 +0,0 @@ -4804fd7719ef467850550e1a488d544ec412f83e51335a367eca119ba550161c \ No newline at end of file -- GitLab From 2965c7584c50d384fed875482b7f64f2f1246b77 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Fri, 31 Jan 2025 15:17:04 +0000 Subject: [PATCH 4/8] Add v2 approval rule associations to project group and mr Adds new approval rule associations to project group and merge request for new approval rule architecture. Changelog: changed EE: true --- .../merge_requests_approval_rules_groups.yml | 12 +++++++++ ...requests_approval_rules_merge_requests.yml | 19 ++++++++++++++ ...merge_requests_approval_rules_projects.yml | 12 +++++++++ ...te_merge_requests_approval_rules_groups.rb | 23 +++++++++++++++++ ..._requests_approval_rules_merge_requests.rb | 25 +++++++++++++++++++ ..._merge_requests_approval_rules_projects.rb | 24 ++++++++++++++++++ db/schema_migrations/20250123151708 | 1 + db/schema_migrations/20250123151726 | 1 + db/schema_migrations/20250123151745 | 1 + ee/app/models/ee/group.rb | 3 +++ ee/app/models/ee/merge_request.rb | 5 ++++ ee/app/models/ee/project.rb | 3 +++ ee/app/models/merge_requests/approval_rule.rb | 20 +++++++++++++++ .../merge_requests/approval_rules_group.rb | 10 ++++++++ .../approval_rules_merge_request.rb | 10 ++++++++ .../merge_requests/approval_rules_project.rb | 10 ++++++++ .../merge_requests/approval_rules_groups.rb | 8 ++++++ .../approval_rules_merge_requests.rb | 8 ++++++ .../merge_requests/approval_rules_projects.rb | 8 ++++++ .../approval_rules_group_spec.rb | 10 ++++++++ .../approval_rules_merge_request_spec.rb | 10 ++++++++ .../approval_rules_project_spec.rb | 10 ++++++++ spec/lib/gitlab/import_export/all_models.yml | 6 +++++ 23 files changed, 239 insertions(+) create mode 100644 db/docs/merge_requests_approval_rules_groups.yml create mode 100644 db/docs/merge_requests_approval_rules_merge_requests.yml create mode 100644 db/docs/merge_requests_approval_rules_projects.yml create mode 100644 db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb create mode 100644 db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb create mode 100644 db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb create mode 100644 db/schema_migrations/20250123151708 create mode 100644 db/schema_migrations/20250123151726 create mode 100644 db/schema_migrations/20250123151745 create mode 100644 ee/app/models/merge_requests/approval_rules_group.rb create mode 100644 ee/app/models/merge_requests/approval_rules_merge_request.rb create mode 100644 ee/app/models/merge_requests/approval_rules_project.rb create mode 100644 ee/spec/factories/merge_requests/approval_rules_groups.rb create mode 100644 ee/spec/factories/merge_requests/approval_rules_merge_requests.rb create mode 100644 ee/spec/factories/merge_requests/approval_rules_projects.rb create mode 100644 ee/spec/models/merge_requests/approval_rules_group_spec.rb create mode 100644 ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb create mode 100644 ee/spec/models/merge_requests/approval_rules_project_spec.rb diff --git a/db/docs/merge_requests_approval_rules_groups.yml b/db/docs/merge_requests_approval_rules_groups.yml new file mode 100644 index 00000000000000..cdc7d0a6fd580a --- /dev/null +++ b/db/docs/merge_requests_approval_rules_groups.yml @@ -0,0 +1,12 @@ +--- +table_name: merge_requests_approval_rules_groups +classes: + - MergeRequests::ApprovalRulesGroup +feature_categories: + - code_review_workflow +description: Stores relationship between approval rules v2 and groups +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179257 +milestone: '17.9' +gitlab_schema: gitlab_main_cell +sharding_key: + group_id: namespaces diff --git a/db/docs/merge_requests_approval_rules_merge_requests.yml b/db/docs/merge_requests_approval_rules_merge_requests.yml new file mode 100644 index 00000000000000..d4c33aa7b67731 --- /dev/null +++ b/db/docs/merge_requests_approval_rules_merge_requests.yml @@ -0,0 +1,19 @@ +--- +table_name: merge_requests_approval_rules_merge_requests +classes: + - MergeRequests::ApprovalRulesMergeRequest +feature_categories: + - code_review_workflow +description: Stores relationship between approval rules v2 and merge requests +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179257 +milestone: '17.9' +gitlab_schema: gitlab_main_cell +desired_sharding_key: + project_id: + references: projects + backfill_via: + parent: + foreign_key: merge_request_id + table: merge_requests + sharding_key: target_project_id + belongs_to: merge_request diff --git a/db/docs/merge_requests_approval_rules_projects.yml b/db/docs/merge_requests_approval_rules_projects.yml new file mode 100644 index 00000000000000..97c93a5b8a9727 --- /dev/null +++ b/db/docs/merge_requests_approval_rules_projects.yml @@ -0,0 +1,12 @@ +--- +table_name: merge_requests_approval_rules_projects +classes: + - MergeRequests::ApprovalRulesProject +feature_categories: + - code_review_workflow +description: Stores relationship between approval rules v2 and projects +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179257 +milestone: '17.9' +gitlab_schema: gitlab_main_cell +sharding_key: + project_id: projects diff --git a/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb b/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb new file mode 100644 index 00000000000000..88e3be65db90bb --- /dev/null +++ b/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class CreateMergeRequestsApprovalRulesGroups < Gitlab::Database::Migration[2.2] + milestone '17.9' + + def change + create_table :merge_requests_approval_rules_groups do |t| # -- Migration/EnsureFactoryForTable false positive + t.belongs_to( + :approval_rule, + foreign_key: { to_table: :merge_requests_approval_rules }, + index: { name: 'index_approval_rules_groups_on_approval_rule_id' }, + null: false + ) + t.belongs_to( + :group, + foreign_key: { to_table: :namespaces, on_delete: :cascade }, + null: false + ) + + t.timestamps_with_timezone null: false + end + end +end diff --git a/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb b/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb new file mode 100644 index 00000000000000..e9a03a7b2b2846 --- /dev/null +++ b/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class CreateMergeRequestsApprovalRulesMergeRequests < Gitlab::Database::Migration[2.2] + milestone '17.9' + + def change + create_table :merge_requests_approval_rules_merge_requests do |t| # Migration/EnsureFactoryForTable false positive + t.belongs_to( + :approval_rule, + foreign_key: { to_table: :merge_requests_approval_rules }, + index: { name: 'index_approval_rules_merge_requests_on_approval_rule_id' }, + null: false + ) + + t.belongs_to( + :merge_request, + foreign_key: { to_table: :merge_requests, on_delete: :cascade }, + index: { name: 'index_approval_rules_merge_requests_on_merge_request_id' }, + null: false + ) + + t.timestamps_with_timezone null: false + end + end +end diff --git a/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb b/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb new file mode 100644 index 00000000000000..9ffbaabdb0d180 --- /dev/null +++ b/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class CreateMergeRequestsApprovalRulesProjects < Gitlab::Database::Migration[2.2] + milestone '17.9' + + def change + create_table :merge_requests_approval_rules_projects do |t| # -- Migration/EnsureFactoryForTable false positive + t.belongs_to( + :approval_rule, + foreign_key: { to_table: :merge_requests_approval_rules }, + index: { name: 'index_approval_rules_project_on_approval_rule_id' }, + null: false + ) + t.belongs_to( + :project, + foreign_key: { to_table: :projects, on_delete: :cascade }, + index: { name: 'index_approval_rules_project_on_project_id' }, + null: false + ) + + t.timestamps_with_timezone null: false + end + end +end diff --git a/db/schema_migrations/20250123151708 b/db/schema_migrations/20250123151708 new file mode 100644 index 00000000000000..87a1ad0236b30a --- /dev/null +++ b/db/schema_migrations/20250123151708 @@ -0,0 +1 @@ +620def37e032e7fa463c4f1ecff0aacde33a5b9bd7a56c2f6ead418b3f4ce828 \ No newline at end of file diff --git a/db/schema_migrations/20250123151726 b/db/schema_migrations/20250123151726 new file mode 100644 index 00000000000000..0d54c5d1bc08d4 --- /dev/null +++ b/db/schema_migrations/20250123151726 @@ -0,0 +1 @@ +e5a26f1a31603cf3ac570824874afbeac3bb970ca079bdaaebbf9cbe13155230 \ No newline at end of file diff --git a/db/schema_migrations/20250123151745 b/db/schema_migrations/20250123151745 new file mode 100644 index 00000000000000..2ce7ec05eaa1d5 --- /dev/null +++ b/db/schema_migrations/20250123151745 @@ -0,0 +1 @@ +d6fc925f97583a4509894202b2e529ae64138f2cceef76e0d0c4dcaafe70f45a \ No newline at end of file diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 13de06a8d0d8fc..7a559d063c0fb1 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -99,6 +99,9 @@ module Group has_many :security_exclusions, class_name: 'Security::GroupSecurityExclusion' + has_many :v2_approval_group_rules, class_name: 'MergeRequests::ApprovalRulesGroup', inverse_of: :group + has_many :v2_approval_rules, through: :v2_approval_group_rules, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule + delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true delegate :repository_read_only, diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index fcbf51d524998f..4aaa2604b2d243 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -101,6 +101,11 @@ def set_applicable_when_copying_rules(applicable_ids) has_many :merge_request_stage_events, class_name: 'Analytics::CycleAnalytics::MergeRequestStageEvent' + has_many :v2_approval_merge_request_rules, class_name: 'MergeRequests::ApprovalRulesMergeRequest', + inverse_of: :merge_request + has_many :v2_approval_rules, through: :v2_approval_merge_request_rules, + class_name: 'MergeRequests::ApprovalRule', source: :approval_rule + delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true delegate :sha, to: :base_pipeline, prefix: :base_pipeline, allow_nil: true delegate :wrapped_approval_rules, :invalid_approvers_rules, to: :approval_state diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index d3448eca50ff28..0f209db88e8b59 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -217,6 +217,9 @@ def lock_for_confirmation!(id) has_many :project_control_compliance_statuses, class_name: 'ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus' + has_many :v2_approval_project_rules, class_name: 'MergeRequests::ApprovalRulesProject', inverse_of: :project + has_many :v2_approval_rules, through: :v2_approval_project_rules, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule + elastic_index_dependant_association :issues, on_change: :visibility_level elastic_index_dependant_association :issues, on_change: :archived elastic_index_dependant_association :work_items, on_change: :visibility_level diff --git a/ee/app/models/merge_requests/approval_rule.rb b/ee/app/models/merge_requests/approval_rule.rb index 46538828aea773..9951c6777192b4 100644 --- a/ee/app/models/merge_requests/approval_rule.rb +++ b/ee/app/models/merge_requests/approval_rule.rb @@ -4,6 +4,26 @@ module MergeRequests class ApprovalRule < ApplicationRecord self.table_name = 'merge_requests_approval_rules' + # If we allow overriding in subgroups we can add groups association + has_many :approval_rules_groups + has_many :groups, through: :approval_rules_groups + + # When this originated_from_merge_request there's only one merge_request + has_one :approval_rules_group, inverse_of: :approval_rule + has_one :group, through: :approval_rules_group + + # When this originated_from_group there are multiple projects + has_many :approval_rules_projects + has_many :projects, through: :approval_rules_projects + + # When this originated_from_merge_request there's only one merge_request + has_one :approval_rules_merge_request, inverse_of: :approval_rule + has_one :merge_request, through: :approval_rules_merge_request + + # When this originated_from_project there are multiple merge_requests + has_many :approval_rules_merge_requests + has_many :merge_requests, through: :approval_rules_merge_requests + validate :ensure_single_sharding_key with_options validate: true do diff --git a/ee/app/models/merge_requests/approval_rules_group.rb b/ee/app/models/merge_requests/approval_rules_group.rb new file mode 100644 index 00000000000000..c2d524c77aea9f --- /dev/null +++ b/ee/app/models/merge_requests/approval_rules_group.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module MergeRequests + class ApprovalRulesGroup < ApplicationRecord + self.table_name = 'merge_requests_approval_rules_groups' + + belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' + belongs_to :group + end +end diff --git a/ee/app/models/merge_requests/approval_rules_merge_request.rb b/ee/app/models/merge_requests/approval_rules_merge_request.rb new file mode 100644 index 00000000000000..6ba10a234ac04b --- /dev/null +++ b/ee/app/models/merge_requests/approval_rules_merge_request.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module MergeRequests + class ApprovalRulesMergeRequest < ApplicationRecord + self.table_name = 'merge_requests_approval_rules_merge_requests' + + belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' + belongs_to :merge_request + end +end diff --git a/ee/app/models/merge_requests/approval_rules_project.rb b/ee/app/models/merge_requests/approval_rules_project.rb new file mode 100644 index 00000000000000..cebfec1e6b354c --- /dev/null +++ b/ee/app/models/merge_requests/approval_rules_project.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module MergeRequests + class ApprovalRulesProject < ApplicationRecord + self.table_name = 'merge_requests_approval_rules_projects' + + belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' + belongs_to :project + end +end diff --git a/ee/spec/factories/merge_requests/approval_rules_groups.rb b/ee/spec/factories/merge_requests/approval_rules_groups.rb new file mode 100644 index 00000000000000..fb8212dd5ea6cc --- /dev/null +++ b/ee/spec/factories/merge_requests/approval_rules_groups.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_requests_approval_rules_group, class: 'MergeRequests::ApprovalRulesGroup' 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_merge_requests.rb b/ee/spec/factories/merge_requests/approval_rules_merge_requests.rb new file mode 100644 index 00000000000000..b221902638aaa6 --- /dev/null +++ b/ee/spec/factories/merge_requests/approval_rules_merge_requests.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_requests_approval_rules_merge_request, class: 'MergeRequests::ApprovalRulesMergeRequest' do + association :approval_rule, factory: :merge_requests_approval_rule + association :merge_request, factory: :merge_request + end +end diff --git a/ee/spec/factories/merge_requests/approval_rules_projects.rb b/ee/spec/factories/merge_requests/approval_rules_projects.rb new file mode 100644 index 00000000000000..de9d478f357fff --- /dev/null +++ b/ee/spec/factories/merge_requests/approval_rules_projects.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :merge_requests_approval_rules_project, class: 'MergeRequests::ApprovalRulesProject' do + association :approval_rule, factory: :merge_requests_approval_rule + association :project, factory: :project + end +end diff --git a/ee/spec/models/merge_requests/approval_rules_group_spec.rb b/ee/spec/models/merge_requests/approval_rules_group_spec.rb new file mode 100644 index 00000000000000..b86fa692d11307 --- /dev/null +++ b/ee/spec/models/merge_requests/approval_rules_group_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::ApprovalRulesGroup, 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_merge_request_spec.rb b/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb new file mode 100644 index 00000000000000..4b9037cb93489c --- /dev/null +++ b/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::ApprovalRulesMergeRequest, 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(:merge_request) } + end +end diff --git a/ee/spec/models/merge_requests/approval_rules_project_spec.rb b/ee/spec/models/merge_requests/approval_rules_project_spec.rb new file mode 100644 index 00000000000000..cd8575ef2ea18a --- /dev/null +++ b/ee/spec/models/merge_requests/approval_rules_project_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::ApprovalRulesProject, 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(:project) } + end +end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index b2bff3b96eda89..d9505bb561e03a 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -255,6 +255,9 @@ merge_requests: - approver_users - approver_groups - approved_by_users +- v2_approval_rules +- v2_approval_project_rules +- v2_approval_merge_request_rules - draft_notes - merge_train_car - blocks_as_blocker @@ -769,6 +772,9 @@ project: - approval_merge_request_rules - approval_merge_request_rule_sources - approval_project_rules +- v2_approval_rules +- v2_approval_project_rules +- v2_approval_merge_request_rules - approvers - approver_users - audit_events -- GitLab From 39089c28dde21872ebb885b1e8405cdf1557dcfc Mon Sep 17 00:00:00 2001 From: ghinfey Date: Thu, 6 Feb 2025 17:03:22 +0000 Subject: [PATCH 5/8] Create seperate migrations for fks --- ...te_merge_requests_approval_rules_groups.rb | 16 +--- ..._requests_approval_rules_merge_requests.rb | 18 +--- ..._merge_requests_approval_rules_projects.rb | 17 +--- ..._approval_rules_groups_approval_rule_fk.rb | 17 ++++ ...requests_approval_rules_groups_group_fk.rb | 17 ++++ ...pproval_rules_projects_approval_rule_fk.rb | 17 ++++ ...ests_approval_rules_projects_project_fk.rb | 17 ++++ ...sts_approval_rules_mrs_approval_rule_fk.rb | 17 ++++ ...merge_requests_approval_rules_mrs_mr_fk.rb | 18 ++++ db/schema_migrations/20250206143903 | 1 + db/schema_migrations/20250206143924 | 1 + db/schema_migrations/20250206144004 | 1 + db/schema_migrations/20250206144027 | 1 + db/schema_migrations/20250206144049 | 1 + db/schema_migrations/20250206144110 | 1 + db/structure.sql | 96 +++++++++++++++++++ spec/lib/gitlab/database/sharding_key_spec.rb | 3 +- 17 files changed, 219 insertions(+), 40 deletions(-) create mode 100644 db/migrate/20250206143903_add_merge_requests_approval_rules_groups_approval_rule_fk.rb create mode 100644 db/migrate/20250206143924_add_merge_requests_approval_rules_groups_group_fk.rb create mode 100644 db/migrate/20250206144004_add_merge_requests_approval_rules_projects_approval_rule_fk.rb create mode 100644 db/migrate/20250206144027_add_merge_requests_approval_rules_projects_project_fk.rb create mode 100644 db/migrate/20250206144049_add_merge_requests_approval_rules_mrs_approval_rule_fk.rb create mode 100644 db/migrate/20250206144110_add_merge_requests_approval_rules_mrs_mr_fk.rb create mode 100644 db/schema_migrations/20250206143903 create mode 100644 db/schema_migrations/20250206143924 create mode 100644 db/schema_migrations/20250206144004 create mode 100644 db/schema_migrations/20250206144027 create mode 100644 db/schema_migrations/20250206144049 create mode 100644 db/schema_migrations/20250206144110 diff --git a/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb b/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb index 88e3be65db90bb..53b3b725003458 100644 --- a/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb +++ b/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb @@ -5,18 +5,10 @@ class CreateMergeRequestsApprovalRulesGroups < Gitlab::Database::Migration[2.2] def change create_table :merge_requests_approval_rules_groups do |t| # -- Migration/EnsureFactoryForTable false positive - t.belongs_to( - :approval_rule, - foreign_key: { to_table: :merge_requests_approval_rules }, - index: { name: 'index_approval_rules_groups_on_approval_rule_id' }, - null: false - ) - t.belongs_to( - :group, - foreign_key: { to_table: :namespaces, on_delete: :cascade }, - null: false - ) - + t.bigint :approval_rule_id, null: false + t.bigint :group_id, null: false + t.index :approval_rule_id + t.index :group_id t.timestamps_with_timezone null: false end end diff --git a/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb b/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb index e9a03a7b2b2846..70dc4ba004ea49 100644 --- a/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb +++ b/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb @@ -5,20 +5,10 @@ class CreateMergeRequestsApprovalRulesMergeRequests < Gitlab::Database::Migratio def change create_table :merge_requests_approval_rules_merge_requests do |t| # Migration/EnsureFactoryForTable false positive - t.belongs_to( - :approval_rule, - foreign_key: { to_table: :merge_requests_approval_rules }, - index: { name: 'index_approval_rules_merge_requests_on_approval_rule_id' }, - null: false - ) - - t.belongs_to( - :merge_request, - foreign_key: { to_table: :merge_requests, on_delete: :cascade }, - index: { name: 'index_approval_rules_merge_requests_on_merge_request_id' }, - null: false - ) - + t.bigint :approval_rule_id, null: false + t.bigint :merge_request_id, null: false + t.index :approval_rule_id, name: 'index_mrs_approval_rules_mrs_on_approval_rule_id' + t.index :merge_request_id, name: 'index_mrs_approval_rules_mrs_on_mr_id' t.timestamps_with_timezone null: false end end diff --git a/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb b/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb index 9ffbaabdb0d180..649f71135cfd59 100644 --- a/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb +++ b/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb @@ -5,19 +5,10 @@ class CreateMergeRequestsApprovalRulesProjects < Gitlab::Database::Migration[2.2 def change create_table :merge_requests_approval_rules_projects do |t| # -- Migration/EnsureFactoryForTable false positive - t.belongs_to( - :approval_rule, - foreign_key: { to_table: :merge_requests_approval_rules }, - index: { name: 'index_approval_rules_project_on_approval_rule_id' }, - null: false - ) - t.belongs_to( - :project, - foreign_key: { to_table: :projects, on_delete: :cascade }, - index: { name: 'index_approval_rules_project_on_project_id' }, - null: false - ) - + t.bigint :approval_rule_id, null: false + t.bigint :project_id, null: false + t.index :approval_rule_id, name: 'index_mrs_approval_rules_projects_on_approval_rule_id' + t.index :project_id, name: 'index_mrs_approval_rules_projects_on_project_id' t.timestamps_with_timezone null: false end end diff --git a/db/migrate/20250206143903_add_merge_requests_approval_rules_groups_approval_rule_fk.rb b/db/migrate/20250206143903_add_merge_requests_approval_rules_groups_approval_rule_fk.rb new file mode 100644 index 00000000000000..205b9a21c4596a --- /dev/null +++ b/db/migrate/20250206143903_add_merge_requests_approval_rules_groups_approval_rule_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesGroupsApprovalRuleFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_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_groups, column: :approval_rule_id + end + end +end diff --git a/db/migrate/20250206143924_add_merge_requests_approval_rules_groups_group_fk.rb b/db/migrate/20250206143924_add_merge_requests_approval_rules_groups_group_fk.rb new file mode 100644 index 00000000000000..35db8abf4702f8 --- /dev/null +++ b/db/migrate/20250206143924_add_merge_requests_approval_rules_groups_group_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesGroupsGroupFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_groups, :namespaces, column: :group_id, + on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_groups, column: :group_id + end + end +end diff --git a/db/migrate/20250206144004_add_merge_requests_approval_rules_projects_approval_rule_fk.rb b/db/migrate/20250206144004_add_merge_requests_approval_rules_projects_approval_rule_fk.rb new file mode 100644 index 00000000000000..740ca025d72897 --- /dev/null +++ b/db/migrate/20250206144004_add_merge_requests_approval_rules_projects_approval_rule_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesProjectsApprovalRuleFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_projects, :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_projects, column: :approval_rule_id + end + end +end diff --git a/db/migrate/20250206144027_add_merge_requests_approval_rules_projects_project_fk.rb b/db/migrate/20250206144027_add_merge_requests_approval_rules_projects_project_fk.rb new file mode 100644 index 00000000000000..70dea81b3e8c42 --- /dev/null +++ b/db/migrate/20250206144027_add_merge_requests_approval_rules_projects_project_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesProjectsProjectFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_projects, :projects, column: :project_id, + on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_projects, column: :project_id + end + end +end diff --git a/db/migrate/20250206144049_add_merge_requests_approval_rules_mrs_approval_rule_fk.rb b/db/migrate/20250206144049_add_merge_requests_approval_rules_mrs_approval_rule_fk.rb new file mode 100644 index 00000000000000..eb877d49cb85a3 --- /dev/null +++ b/db/migrate/20250206144049_add_merge_requests_approval_rules_mrs_approval_rule_fk.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesMrsApprovalRuleFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_merge_requests, :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_merge_requests, column: :approval_rule_id + end + end +end diff --git a/db/migrate/20250206144110_add_merge_requests_approval_rules_mrs_mr_fk.rb b/db/migrate/20250206144110_add_merge_requests_approval_rules_mrs_mr_fk.rb new file mode 100644 index 00000000000000..222f73bdce3d29 --- /dev/null +++ b/db/migrate/20250206144110_add_merge_requests_approval_rules_mrs_mr_fk.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesMrsMrFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_merge_requests, :merge_requests, + column: :merge_request_id, + on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_merge_requests, column: :merge_request_id + end + end +end diff --git a/db/schema_migrations/20250206143903 b/db/schema_migrations/20250206143903 new file mode 100644 index 00000000000000..fc7b302235973f --- /dev/null +++ b/db/schema_migrations/20250206143903 @@ -0,0 +1 @@ +56101662487483f6866c2c1f2cd0ee66d1fa12d08c0da12bd550d389e37f6bf7 \ No newline at end of file diff --git a/db/schema_migrations/20250206143924 b/db/schema_migrations/20250206143924 new file mode 100644 index 00000000000000..724ba0d42f826b --- /dev/null +++ b/db/schema_migrations/20250206143924 @@ -0,0 +1 @@ +4b740e775e43ed2143b7060b5e5e1730aeb7309cc83e4d76a5316cc84e80893b \ No newline at end of file diff --git a/db/schema_migrations/20250206144004 b/db/schema_migrations/20250206144004 new file mode 100644 index 00000000000000..8ccb8a71b94f95 --- /dev/null +++ b/db/schema_migrations/20250206144004 @@ -0,0 +1 @@ +31f9cc1659cd680b16a742eddeb361c4eb059388b1af0136d0d5a648a6fb8cca \ No newline at end of file diff --git a/db/schema_migrations/20250206144027 b/db/schema_migrations/20250206144027 new file mode 100644 index 00000000000000..99653db12ffd2c --- /dev/null +++ b/db/schema_migrations/20250206144027 @@ -0,0 +1 @@ +08d74afd22511618546821c8cc53473ec3a9f6ec4be3c80a20086e1a2e71da37 \ No newline at end of file diff --git a/db/schema_migrations/20250206144049 b/db/schema_migrations/20250206144049 new file mode 100644 index 00000000000000..3cbc804941579b --- /dev/null +++ b/db/schema_migrations/20250206144049 @@ -0,0 +1 @@ +1611b7751ef4fa7476f9ffeab5263f9e0e85f89ab21e3028b2d5245dd38f31db \ No newline at end of file diff --git a/db/schema_migrations/20250206144110 b/db/schema_migrations/20250206144110 new file mode 100644 index 00000000000000..49eba2314f5c5a --- /dev/null +++ b/db/schema_migrations/20250206144110 @@ -0,0 +1 @@ +cd304e944ce13f05dc95d6c74a604bca579a04b08d9abf0fd143ae03c00f1390 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index bee8bc9fad2f1d..e921e598874b89 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16280,6 +16280,23 @@ CREATE TABLE merge_requests_approval_rules ( CONSTRAINT check_c7c36145b7 CHECK ((char_length(name) <= 255)) ); +CREATE TABLE merge_requests_approval_rules_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_groups_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_requests_approval_rules_groups_id_seq OWNED BY merge_requests_approval_rules_groups.id; + CREATE SEQUENCE merge_requests_approval_rules_id_seq START WITH 1 INCREMENT BY 1 @@ -16289,6 +16306,40 @@ CREATE SEQUENCE merge_requests_approval_rules_id_seq ALTER SEQUENCE merge_requests_approval_rules_id_seq OWNED BY merge_requests_approval_rules.id; +CREATE TABLE merge_requests_approval_rules_merge_requests ( + id bigint NOT NULL, + approval_rule_id bigint NOT NULL, + merge_request_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_merge_requests_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_requests_approval_rules_merge_requests_id_seq OWNED BY merge_requests_approval_rules_merge_requests.id; + +CREATE TABLE merge_requests_approval_rules_projects ( + id bigint NOT NULL, + approval_rule_id bigint NOT NULL, + project_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_projects_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE merge_requests_approval_rules_projects_id_seq OWNED BY merge_requests_approval_rules_projects.id; + CREATE TABLE merge_requests_closing_issues ( id bigint NOT NULL, merge_request_id bigint NOT NULL, @@ -25536,6 +25587,12 @@ 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_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_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); @@ -28100,9 +28157,18 @@ 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_groups + ADD CONSTRAINT merge_requests_approval_rules_groups_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY merge_requests_approval_rules_merge_requests + ADD CONSTRAINT merge_requests_approval_rules_merge_requests_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_approval_rules_projects + ADD CONSTRAINT merge_requests_approval_rules_projects_pkey PRIMARY KEY (id); + ALTER TABLE ONLY merge_requests_closing_issues ADD CONSTRAINT merge_requests_closing_issues_pkey PRIMARY KEY (id); @@ -33500,6 +33566,10 @@ 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_groups_on_approval_rule_id ON merge_requests_approval_rules_groups USING btree (approval_rule_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); CREATE INDEX index_merge_requests_approval_rules_on_project_id ON merge_requests_approval_rules USING btree (project_id); @@ -33678,6 +33748,14 @@ 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_mrs_on_approval_rule_id ON merge_requests_approval_rules_merge_requests USING btree (approval_rule_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_projects_on_approval_rule_id ON merge_requests_approval_rules_projects USING btree (approval_rule_id); + +CREATE INDEX index_mrs_approval_rules_projects_on_project_id ON merge_requests_approval_rules_projects USING btree (project_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); @@ -38543,6 +38621,9 @@ ALTER TABLE ONLY observability_traces_issues_connections ALTER TABLE ONLY merge_request_assignment_events ADD CONSTRAINT fk_08f7602bfd FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_groups + ADD CONSTRAINT fk_094b4086a3 FOREIGN KEY (approval_rule_id) REFERENCES merge_requests_approval_rules(id) ON DELETE CASCADE; + ALTER TABLE ONLY catalog_resource_component_last_usages ADD CONSTRAINT fk_094c686785 FOREIGN KEY (component_project_id) REFERENCES projects(id) ON DELETE CASCADE; @@ -39023,6 +39104,9 @@ ALTER TABLE ONLY incident_management_timeline_events ALTER TABLE ONLY todos ADD CONSTRAINT fk_45054f9c45 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_projects + ADD CONSTRAINT fk_451a9dfe93 FOREIGN KEY (approval_rule_id) REFERENCES merge_requests_approval_rules(id) ON DELETE CASCADE; + ALTER TABLE ONLY security_policy_requirements ADD CONSTRAINT fk_458f7f5ad5 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; @@ -39140,6 +39224,9 @@ ALTER TABLE ONLY approval_merge_request_rules ALTER TABLE ONLY deploy_keys_projects ADD CONSTRAINT fk_58a901ca7e FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_groups + ADD CONSTRAINT fk_59068f09e5 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY oauth_access_grants ADD CONSTRAINT fk_59cdb2323c FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; @@ -39173,6 +39260,9 @@ ALTER TABLE ONLY dast_scanner_profiles_builds ALTER TABLE ONLY protected_environment_deploy_access_levels ADD CONSTRAINT fk_5d9b05a7e9 FOREIGN KEY (protected_environment_project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_merge_requests + ADD CONSTRAINT fk_5ddc4a2f7b FOREIGN KEY (approval_rule_id) REFERENCES merge_requests_approval_rules(id) ON DELETE CASCADE; + ALTER TABLE ONLY issue_assignees ADD CONSTRAINT fk_5e0c8d9154 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; @@ -39311,6 +39401,9 @@ ALTER TABLE ONLY index_statuses ALTER TABLE ONLY abuse_report_notes ADD CONSTRAINT fk_74e1990397 FOREIGN KEY (abuse_report_id) REFERENCES abuse_reports(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_merge_requests + ADD CONSTRAINT fk_74e3466397 FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; + ALTER TABLE ONLY software_license_policies ADD CONSTRAINT fk_74f6d8328a FOREIGN KEY (custom_software_license_id) REFERENCES custom_software_licenses(id) ON DELETE CASCADE; @@ -39752,6 +39845,9 @@ ALTER TABLE ONLY ml_experiments ALTER TABLE ONLY merge_request_metrics ADD CONSTRAINT fk_ae440388cc FOREIGN KEY (latest_closed_by_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY merge_requests_approval_rules_projects + ADD CONSTRAINT fk_af4078336f FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY analytics_cycle_analytics_group_stages ADD CONSTRAINT fk_analytics_cycle_analytics_group_stages_group_value_stream_id FOREIGN KEY (group_value_stream_id) REFERENCES analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE; diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index 246f0541560d8d..9d6bb0a9770972 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -12,7 +12,8 @@ 'merge_request_diff_commits_b5377a7a34', # has a desired sharding key instead 'merge_request_diff_files_99208b8fac', # has a desired sharding key instead 'p_ci_pipeline_variables', # has a desired sharding key instead - 'web_hook_logs_daily' # temporary copy of web_hook_logs + 'web_hook_logs_daily', # temporary copy of web_hook_logs + 'merge_requests_approval_rules_merge_requests' # has a desired sharding key instead ] end -- GitLab From cb00da2ff32ffd022e5d0effc4e3b527ed9f22f2 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Mon, 10 Feb 2025 09:12:24 +0000 Subject: [PATCH 6/8] Review changes add unique indexes --- ...te_merge_requests_approval_rules_groups.rb | 8 +++++++- ..._requests_approval_rules_merge_requests.rb | 8 +++++++- ..._merge_requests_approval_rules_projects.rb | 8 +++++++- db/structure.sql | 12 ++++++------ ee/app/models/ee/group.rb | 3 ++- ee/app/models/ee/merge_request.rb | 3 ++- ee/app/models/ee/project.rb | 3 ++- .../merge_requests/approval_rules_group.rb | 2 ++ .../approval_rules_merge_request.rb | 2 ++ .../merge_requests/approval_rules_project.rb | 2 ++ .../approval_rules_group_spec.rb | 18 ++++++++++++++++++ .../approval_rules_merge_request_spec.rb | 19 +++++++++++++++++++ .../approval_rules_project_spec.rb | 18 ++++++++++++++++++ spec/lib/gitlab/import_export/all_models.yml | 8 ++++---- 14 files changed, 98 insertions(+), 16 deletions(-) diff --git a/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb b/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb index 53b3b725003458..e873138551f8ea 100644 --- a/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb +++ b/db/migrate/20250123151708_create_merge_requests_approval_rules_groups.rb @@ -7,9 +7,15 @@ def change create_table :merge_requests_approval_rules_groups do |t| # -- Migration/EnsureFactoryForTable false positive t.bigint :approval_rule_id, null: false t.bigint :group_id, null: false - t.index :approval_rule_id t.index :group_id t.timestamps_with_timezone null: false end + + add_index( + :merge_requests_approval_rules_groups, + %i[approval_rule_id group_id], + unique: true, + name: 'index_mrs_ars_groups_on_ar_id_and_group_id' + ) end end diff --git a/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb b/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb index 70dc4ba004ea49..656c7af067be99 100644 --- a/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb +++ b/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb @@ -7,9 +7,15 @@ def change create_table :merge_requests_approval_rules_merge_requests do |t| # Migration/EnsureFactoryForTable false positive t.bigint :approval_rule_id, null: false t.bigint :merge_request_id, null: false - t.index :approval_rule_id, name: 'index_mrs_approval_rules_mrs_on_approval_rule_id' t.index :merge_request_id, name: 'index_mrs_approval_rules_mrs_on_mr_id' t.timestamps_with_timezone null: false end + + add_index( + :merge_requests_approval_rules_merge_requests, + %i[approval_rule_id merge_request_id], + unique: true, + name: 'index_mrs_ars_mrs_on_ar_id_and_mr_id' + ) end end diff --git a/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb b/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb index 649f71135cfd59..b87ec1ace48ae0 100644 --- a/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb +++ b/db/migrate/20250123151745_create_merge_requests_approval_rules_projects.rb @@ -7,9 +7,15 @@ def change create_table :merge_requests_approval_rules_projects do |t| # -- Migration/EnsureFactoryForTable false positive t.bigint :approval_rule_id, null: false t.bigint :project_id, null: false - t.index :approval_rule_id, name: 'index_mrs_approval_rules_projects_on_approval_rule_id' t.index :project_id, name: 'index_mrs_approval_rules_projects_on_project_id' t.timestamps_with_timezone null: false end + + add_index( + :merge_requests_approval_rules_projects, + %i[approval_rule_id project_id], + unique: true, + name: 'index_mrs_ars_projects_on_ar_id_and_project_id' + ) end end diff --git a/db/structure.sql b/db/structure.sql index e921e598874b89..e0f8bbaac6d496 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -33566,8 +33566,6 @@ 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_groups_on_approval_rule_id ON merge_requests_approval_rules_groups USING btree (approval_rule_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); @@ -33748,14 +33746,16 @@ 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_mrs_on_approval_rule_id ON merge_requests_approval_rules_merge_requests USING btree (approval_rule_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_projects_on_approval_rule_id ON merge_requests_approval_rules_projects USING btree (approval_rule_id); - 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_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 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); diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 7a559d063c0fb1..49d1d53fd7ea3a 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -99,7 +99,8 @@ module Group has_many :security_exclusions, class_name: 'Security::GroupSecurityExclusion' - has_many :v2_approval_group_rules, class_name: 'MergeRequests::ApprovalRulesGroup', inverse_of: :group + # WIP v2 approval rules as part of https://gitlab.com/groups/gitlab-org/-/epics/12955 + has_many :v2_approval_rules_groups, class_name: 'MergeRequests::ApprovalRulesGroup', inverse_of: :group has_many :v2_approval_rules, through: :v2_approval_group_rules, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 4aaa2604b2d243..7534bf906540da 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -101,7 +101,8 @@ def set_applicable_when_copying_rules(applicable_ids) has_many :merge_request_stage_events, class_name: 'Analytics::CycleAnalytics::MergeRequestStageEvent' - has_many :v2_approval_merge_request_rules, class_name: 'MergeRequests::ApprovalRulesMergeRequest', + # WIP v2 approval rules as part of https://gitlab.com/groups/gitlab-org/-/epics/12955 + has_many :v2_approval_rules_merge_requests, class_name: 'MergeRequests::ApprovalRulesMergeRequest', inverse_of: :merge_request has_many :v2_approval_rules, through: :v2_approval_merge_request_rules, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 0f209db88e8b59..e79aa8d7dfcb35 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -217,7 +217,8 @@ def lock_for_confirmation!(id) has_many :project_control_compliance_statuses, class_name: 'ComplianceManagement::ComplianceFramework::ProjectControlComplianceStatus' - has_many :v2_approval_project_rules, class_name: 'MergeRequests::ApprovalRulesProject', inverse_of: :project + # WIP v2 approval rules as part of https://gitlab.com/groups/gitlab-org/-/epics/12955 + has_many :v2_approval_rules_projects, class_name: 'MergeRequests::ApprovalRulesProject', inverse_of: :project has_many :v2_approval_rules, through: :v2_approval_project_rules, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule elastic_index_dependant_association :issues, on_change: :visibility_level diff --git a/ee/app/models/merge_requests/approval_rules_group.rb b/ee/app/models/merge_requests/approval_rules_group.rb index c2d524c77aea9f..2f5b08c306cdb5 100644 --- a/ee/app/models/merge_requests/approval_rules_group.rb +++ b/ee/app/models/merge_requests/approval_rules_group.rb @@ -6,5 +6,7 @@ class ApprovalRulesGroup < ApplicationRecord belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' belongs_to :group + + validates :group_id, uniqueness: { scope: :approval_rule_id } end end diff --git a/ee/app/models/merge_requests/approval_rules_merge_request.rb b/ee/app/models/merge_requests/approval_rules_merge_request.rb index 6ba10a234ac04b..e09954bfea8699 100644 --- a/ee/app/models/merge_requests/approval_rules_merge_request.rb +++ b/ee/app/models/merge_requests/approval_rules_merge_request.rb @@ -6,5 +6,7 @@ class ApprovalRulesMergeRequest < ApplicationRecord belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' belongs_to :merge_request + + validates :merge_request_id, uniqueness: { scope: :approval_rule_id } end end diff --git a/ee/app/models/merge_requests/approval_rules_project.rb b/ee/app/models/merge_requests/approval_rules_project.rb index cebfec1e6b354c..dbe99c628f2daf 100644 --- a/ee/app/models/merge_requests/approval_rules_project.rb +++ b/ee/app/models/merge_requests/approval_rules_project.rb @@ -6,5 +6,7 @@ class ApprovalRulesProject < ApplicationRecord belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' belongs_to :project + + validates :project_id, uniqueness: { scope: :approval_rule_id } end end diff --git a/ee/spec/models/merge_requests/approval_rules_group_spec.rb b/ee/spec/models/merge_requests/approval_rules_group_spec.rb index b86fa692d11307..870caa6b9799d7 100644 --- a/ee/spec/models/merge_requests/approval_rules_group_spec.rb +++ b/ee/spec/models/merge_requests/approval_rules_group_spec.rb @@ -7,4 +7,22 @@ it { is_expected.to belong_to(:approval_rule) } it { is_expected.to belong_to(:group) } end + + describe 'validations' do + context 'when adding the same group to an approval rule' do + let(:group) { create(:group) } + let(:approval_rule) { create(:merge_requests_approval_rule, group_id: group.id) } + + before do + create(:merge_requests_approval_rules_group, approval_rule: approval_rule, group: group) + end + + it 'is not valid' do + duplicate_approval_rules_group = build(:merge_requests_approval_rules_group, approval_rule: approval_rule, + group_id: group.id) + expect(duplicate_approval_rules_group).not_to be_valid + expect(duplicate_approval_rules_group.errors[:group_id]).to include('has already been taken') + end + end + end end diff --git a/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb b/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb index 4b9037cb93489c..eb65bc61fdb3ca 100644 --- a/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb +++ b/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb @@ -7,4 +7,23 @@ it { is_expected.to belong_to(:approval_rule) } it { is_expected.to belong_to(:merge_request) } end + + describe 'validations' do + context 'when adding the same merge request to an approval rule' do + let(:merge_request) { create(:merge_request) } + let(:group) { create(:group) } + let(:approval_rule) { create(:merge_requests_approval_rule, group_id: group.id) } + + before do + create(:merge_requests_approval_rules_merge_request, approval_rule: approval_rule, merge_request: merge_request) + end + + it 'is not valid' do + duplicate_approval_rules_merge_request = build(:merge_requests_approval_rules_merge_request, + approval_rule: approval_rule, merge_request_id: merge_request.id) + expect(duplicate_approval_rules_merge_request).not_to be_valid + expect(duplicate_approval_rules_merge_request.errors[:merge_request_id]).to include('has already been taken') + end + end + end end diff --git a/ee/spec/models/merge_requests/approval_rules_project_spec.rb b/ee/spec/models/merge_requests/approval_rules_project_spec.rb index cd8575ef2ea18a..eeb9ee379d6da7 100644 --- a/ee/spec/models/merge_requests/approval_rules_project_spec.rb +++ b/ee/spec/models/merge_requests/approval_rules_project_spec.rb @@ -7,4 +7,22 @@ it { is_expected.to belong_to(:approval_rule) } it { is_expected.to belong_to(:project) } end + + describe 'validations' do + context 'when adding the same project to an approval rule' do + let_it_be(:project) { create(:project) } + let(:approval_rule) { create(:merge_requests_approval_rule, project_id: project.id) } + + before do + create(:merge_requests_approval_rules_project, approval_rule: approval_rule, project: project) + end + + it 'is not valid' do + duplicate_approval_rules_project = build(:merge_requests_approval_rules_project, approval_rule: approval_rule, + project_id: project.id) + expect(duplicate_approval_rules_project).not_to be_valid + expect(duplicate_approval_rules_project.errors[:project_id]).to include('has already been taken') + end + end + end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index d9505bb561e03a..4f97cbcbb3fb6d 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -256,8 +256,8 @@ merge_requests: - approver_groups - approved_by_users - v2_approval_rules -- v2_approval_project_rules -- v2_approval_merge_request_rules +- v2_approval_rules_projects +- v2_approval_rules_merge_requests - draft_notes - merge_train_car - blocks_as_blocker @@ -773,8 +773,8 @@ project: - approval_merge_request_rule_sources - approval_project_rules - v2_approval_rules -- v2_approval_project_rules -- v2_approval_merge_request_rules +- v2_approval_rules_projects +- v2_approval_rules_merge_requests - approvers - approver_users - audit_events -- GitLab From 0030372fdc4b20f9fc78b6b70ce6c5512e3ce030 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Tue, 11 Feb 2025 14:36:22 +0000 Subject: [PATCH 7/8] Add project fk to approval rules mr join table --- .../merge_requests_approval_rules_groups.yml | 2 +- ..._requests_approval_rules_merge_requests.yml | 13 +++---------- .../merge_requests_approval_rules_projects.yml | 2 +- ...e_requests_approval_rules_merge_requests.rb | 2 ++ ...e_requests_approval_rules_mrs_project_fk.rb | 18 ++++++++++++++++++ db/schema_migrations/20250211141110 | 1 + db/structure.sql | 6 ++++++ ee/app/models/ee/group.rb | 2 +- ee/app/models/ee/merge_request.rb | 2 +- ee/app/models/ee/project.rb | 2 +- .../approval_rules_merge_request_spec.rb | 5 +++-- spec/lib/gitlab/database/sharding_key_spec.rb | 3 +-- 12 files changed, 39 insertions(+), 19 deletions(-) create mode 100644 db/migrate/20250211141110_add_merge_requests_approval_rules_mrs_project_fk.rb create mode 100644 db/schema_migrations/20250211141110 diff --git a/db/docs/merge_requests_approval_rules_groups.yml b/db/docs/merge_requests_approval_rules_groups.yml index cdc7d0a6fd580a..b8a52e7811f911 100644 --- a/db/docs/merge_requests_approval_rules_groups.yml +++ b/db/docs/merge_requests_approval_rules_groups.yml @@ -5,7 +5,7 @@ classes: feature_categories: - code_review_workflow description: Stores relationship between approval rules v2 and groups -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179257 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179861 milestone: '17.9' gitlab_schema: gitlab_main_cell sharding_key: diff --git a/db/docs/merge_requests_approval_rules_merge_requests.yml b/db/docs/merge_requests_approval_rules_merge_requests.yml index d4c33aa7b67731..ec7130a8a9ada9 100644 --- a/db/docs/merge_requests_approval_rules_merge_requests.yml +++ b/db/docs/merge_requests_approval_rules_merge_requests.yml @@ -5,15 +5,8 @@ classes: feature_categories: - code_review_workflow description: Stores relationship between approval rules v2 and merge requests -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179257 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179861 milestone: '17.9' gitlab_schema: gitlab_main_cell -desired_sharding_key: - project_id: - references: projects - backfill_via: - parent: - foreign_key: merge_request_id - table: merge_requests - sharding_key: target_project_id - belongs_to: merge_request +sharding_key: + project_id: projects diff --git a/db/docs/merge_requests_approval_rules_projects.yml b/db/docs/merge_requests_approval_rules_projects.yml index 97c93a5b8a9727..bbf00b7348c476 100644 --- a/db/docs/merge_requests_approval_rules_projects.yml +++ b/db/docs/merge_requests_approval_rules_projects.yml @@ -5,7 +5,7 @@ classes: feature_categories: - code_review_workflow description: Stores relationship between approval rules v2 and projects -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179257 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179861 milestone: '17.9' gitlab_schema: gitlab_main_cell sharding_key: diff --git a/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb b/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb index 656c7af067be99..a67fe56a775879 100644 --- a/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb +++ b/db/migrate/20250123151726_create_merge_requests_approval_rules_merge_requests.rb @@ -7,7 +7,9 @@ def change create_table :merge_requests_approval_rules_merge_requests do |t| # Migration/EnsureFactoryForTable false positive t.bigint :approval_rule_id, null: false t.bigint :merge_request_id, null: false + t.bigint :project_id, null: false t.index :merge_request_id, name: 'index_mrs_approval_rules_mrs_on_mr_id' + t.index :project_id, name: 'index_mrs_approval_rules_mrs_on_project_id' t.timestamps_with_timezone null: false end diff --git a/db/migrate/20250211141110_add_merge_requests_approval_rules_mrs_project_fk.rb b/db/migrate/20250211141110_add_merge_requests_approval_rules_mrs_project_fk.rb new file mode 100644 index 00000000000000..430ff31f244243 --- /dev/null +++ b/db/migrate/20250211141110_add_merge_requests_approval_rules_mrs_project_fk.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddMergeRequestsApprovalRulesMrsProjectFk < Gitlab::Database::Migration[2.2] + milestone '17.9' + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_requests_approval_rules_merge_requests, :projects, + column: :project_id, + on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :merge_requests_approval_rules_merge_requests, column: :project_id + end + end +end diff --git a/db/schema_migrations/20250211141110 b/db/schema_migrations/20250211141110 new file mode 100644 index 00000000000000..c1b01af91f496c --- /dev/null +++ b/db/schema_migrations/20250211141110 @@ -0,0 +1 @@ +69fd6b83d6c739006134ada0d7019b9c04ea5d6ae34b3dc4906546a2c3070110 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e0f8bbaac6d496..9bd03b158bbbd4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16310,6 +16310,7 @@ CREATE TABLE merge_requests_approval_rules_merge_requests ( id bigint NOT NULL, approval_rule_id bigint NOT NULL, merge_request_id bigint NOT NULL, + project_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL ); @@ -33748,6 +33749,8 @@ CREATE INDEX index_mr_metrics_on_target_project_id_merged_at_time_to_merge ON me 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); + 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_groups_on_ar_id_and_group_id ON merge_requests_approval_rules_groups USING btree (approval_rule_id, group_id); @@ -38783,6 +38786,9 @@ ALTER TABLE ONLY agent_project_authorizations ALTER TABLE ONLY ai_agent_version_attachments ADD CONSTRAINT fk_1d4253673b FOREIGN KEY (ai_vectorizable_file_id) REFERENCES ai_vectorizable_files(id) ON DELETE CASCADE; +ALTER TABLE ONLY merge_requests_approval_rules_merge_requests + ADD CONSTRAINT fk_1d49645a27 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY design_management_versions ADD CONSTRAINT fk_1dccb304f8 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 49d1d53fd7ea3a..33ac105e9ba893 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -101,7 +101,7 @@ module Group # WIP v2 approval rules as part of https://gitlab.com/groups/gitlab-org/-/epics/12955 has_many :v2_approval_rules_groups, class_name: 'MergeRequests::ApprovalRulesGroup', inverse_of: :group - has_many :v2_approval_rules, through: :v2_approval_group_rules, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule + has_many :v2_approval_rules, through: :v2_approval_rules_groups, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule delegate :deleting_user, :marked_for_deletion_on, to: :deletion_schedule, allow_nil: true diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 7534bf906540da..83dcc54275cddc 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -104,7 +104,7 @@ def set_applicable_when_copying_rules(applicable_ids) # WIP v2 approval rules as part of https://gitlab.com/groups/gitlab-org/-/epics/12955 has_many :v2_approval_rules_merge_requests, class_name: 'MergeRequests::ApprovalRulesMergeRequest', inverse_of: :merge_request - has_many :v2_approval_rules, through: :v2_approval_merge_request_rules, + has_many :v2_approval_rules, through: :v2_approval_rules_merge_requests, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule delegate :sha, to: :head_pipeline, prefix: :head_pipeline, allow_nil: true diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index e79aa8d7dfcb35..843d569b729388 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -219,7 +219,7 @@ def lock_for_confirmation!(id) # WIP v2 approval rules as part of https://gitlab.com/groups/gitlab-org/-/epics/12955 has_many :v2_approval_rules_projects, class_name: 'MergeRequests::ApprovalRulesProject', inverse_of: :project - has_many :v2_approval_rules, through: :v2_approval_project_rules, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule + has_many :v2_approval_rules, through: :v2_approval_rules_projects, class_name: 'MergeRequests::ApprovalRule', source: :approval_rule elastic_index_dependant_association :issues, on_change: :visibility_level elastic_index_dependant_association :issues, on_change: :archived diff --git a/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb b/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb index eb65bc61fdb3ca..c2d0ebf27bc8a0 100644 --- a/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb +++ b/ee/spec/models/merge_requests/approval_rules_merge_request_spec.rb @@ -15,12 +15,13 @@ let(:approval_rule) { create(:merge_requests_approval_rule, group_id: group.id) } before do - create(:merge_requests_approval_rules_merge_request, approval_rule: approval_rule, merge_request: merge_request) + create(:merge_requests_approval_rules_merge_request, approval_rule: approval_rule, + merge_request: merge_request, project_id: merge_request.project_id) end it 'is not valid' do duplicate_approval_rules_merge_request = build(:merge_requests_approval_rules_merge_request, - approval_rule: approval_rule, merge_request_id: merge_request.id) + approval_rule: approval_rule, merge_request_id: merge_request.id, project_id: merge_request.project_id) expect(duplicate_approval_rules_merge_request).not_to be_valid expect(duplicate_approval_rules_merge_request.errors[:merge_request_id]).to include('has already been taken') end diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index 9d6bb0a9770972..246f0541560d8d 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -12,8 +12,7 @@ 'merge_request_diff_commits_b5377a7a34', # has a desired sharding key instead 'merge_request_diff_files_99208b8fac', # has a desired sharding key instead 'p_ci_pipeline_variables', # has a desired sharding key instead - 'web_hook_logs_daily', # temporary copy of web_hook_logs - 'merge_requests_approval_rules_merge_requests' # has a desired sharding key instead + 'web_hook_logs_daily' # temporary copy of web_hook_logs ] end -- GitLab From ed8388d1f6eb33a841698c7beb96b889e2ee60b3 Mon Sep 17 00:00:00 2001 From: ghinfey Date: Wed, 12 Feb 2025 12:49:55 +0000 Subject: [PATCH 8/8] add has one project relation and update comments --- ee/app/models/merge_requests/approval_rule.rb | 18 +++++++++++------- .../approval_rules_merge_request.rb | 9 ++++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/ee/app/models/merge_requests/approval_rule.rb b/ee/app/models/merge_requests/approval_rule.rb index 9951c6777192b4..8f002d21655a8e 100644 --- a/ee/app/models/merge_requests/approval_rule.rb +++ b/ee/app/models/merge_requests/approval_rule.rb @@ -4,26 +4,30 @@ module MergeRequests class ApprovalRule < ApplicationRecord self.table_name = 'merge_requests_approval_rules' - # If we allow overriding in subgroups we can add groups association + # If we allow overriding in subgroups there can be multiple groups has_many :approval_rules_groups has_many :groups, through: :approval_rules_groups - # When this originated_from_merge_request there's only one merge_request + # When this originated from group there is only one group has_one :approval_rules_group, inverse_of: :approval_rule has_one :group, through: :approval_rules_group - # When this originated_from_group there are multiple projects + # When this originated from group there are multiple projects has_many :approval_rules_projects has_many :projects, through: :approval_rules_projects - # When this originated_from_merge_request there's only one merge_request - has_one :approval_rules_merge_request, inverse_of: :approval_rule - has_one :merge_request, through: :approval_rules_merge_request + # When this originated from project there is only one project + has_one :approval_rules_project + has_one :project, through: :approval_rules_project - # When this originated_from_project there are multiple merge_requests + # When this originated from group or project there are multiple merge_requests has_many :approval_rules_merge_requests has_many :merge_requests, through: :approval_rules_merge_requests + # When this originated from merge_request there is only one merge_request + has_one :approval_rules_merge_request, inverse_of: :approval_rule + has_one :merge_request, through: :approval_rules_merge_request + validate :ensure_single_sharding_key with_options validate: true do diff --git a/ee/app/models/merge_requests/approval_rules_merge_request.rb b/ee/app/models/merge_requests/approval_rules_merge_request.rb index e09954bfea8699..add783e2d3d6cf 100644 --- a/ee/app/models/merge_requests/approval_rules_merge_request.rb +++ b/ee/app/models/merge_requests/approval_rules_merge_request.rb @@ -5,8 +5,15 @@ class ApprovalRulesMergeRequest < ApplicationRecord self.table_name = 'merge_requests_approval_rules_merge_requests' belongs_to :approval_rule, class_name: 'MergeRequests::ApprovalRule' - belongs_to :merge_request + belongs_to :merge_request, class_name: 'MergeRequest' validates :merge_request_id, uniqueness: { scope: :approval_rule_id } + before_create :set_project_id + + private + + def set_project_id + self.project_id = merge_request.source_project.id + end end end -- GitLab