diff --git a/db/docs/board_assignees.yml b/db/docs/board_assignees.yml index 836caad528b74a47689a9912df6da83468c085d1..5a2c79a05834bff4ea8fe5cc5f8973b295db0346 100644 --- a/db/docs/board_assignees.yml +++ b/db/docs/board_assignees.yml @@ -8,6 +8,8 @@ description: Used by issue boards to filter issues by assignee as part of the de scope introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/2912 milestone: '10.2' -gitlab_schema: gitlab_main -sharding_key_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/514603 +gitlab_schema: gitlab_main_cell +sharding_key: + group_id: namespaces + project_id: projects table_size: small diff --git a/db/migrate/20250123194127_add_board_assignees_sharding_key.rb b/db/migrate/20250123194127_add_board_assignees_sharding_key.rb new file mode 100644 index 0000000000000000000000000000000000000000..a5cbcc4298790a335f47af4f2c16098c823d8f1e --- /dev/null +++ b/db/migrate/20250123194127_add_board_assignees_sharding_key.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddBoardAssigneesShardingKey < Gitlab::Database::Migration[2.2] + milestone '17.9' + + def change + add_column :board_assignees, :group_id, :bigint + add_column :board_assignees, :project_id, :bigint + end +end diff --git a/db/post_migrate/20250123195300_backfill_sharding_key_on_board_assignees.rb b/db/post_migrate/20250123195300_backfill_sharding_key_on_board_assignees.rb new file mode 100644 index 0000000000000000000000000000000000000000..b997c9cbf9e5016781fa71e417eb79c81f552878 --- /dev/null +++ b/db/post_migrate/20250123195300_backfill_sharding_key_on_board_assignees.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class BackfillShardingKeyOnBoardAssignees < Gitlab::Database::Migration[2.2] + BATCH_SIZE = 100 + + disable_ddl_transaction! + restrict_gitlab_migration gitlab_schema: :gitlab_main + + milestone '17.9' + + def up + each_batch(:board_assignees, of: BATCH_SIZE) do |batch| + connection.execute( + <<~SQL + UPDATE + "board_assignees" + SET + "group_id" = "boards"."group_id", + "project_id" = "boards"."project_id" + FROM + "boards" + WHERE + "board_assignees"."board_id" = "boards"."id" + AND "board_assignees"."id" IN (#{batch.select(:id).to_sql}) + SQL + ) + end + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20250123195400_add_board_assignees_project_id_index.rb b/db/post_migrate/20250123195400_add_board_assignees_project_id_index.rb new file mode 100644 index 0000000000000000000000000000000000000000..8904bbc834442cd8176161a2d37255a91b0db2d4 --- /dev/null +++ b/db/post_migrate/20250123195400_add_board_assignees_project_id_index.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddBoardAssigneesProjectIdIndex < Gitlab::Database::Migration[2.2] + INDEX_NAME = 'index_board_assignees_on_project_id' + + disable_ddl_transaction! + milestone '17.9' + + def up + add_concurrent_index :board_assignees, :project_id, name: INDEX_NAME + end + + def down + remove_concurrent_index :board_assignees, :project_id, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20250123195500_add_board_assignees_group_id_index.rb b/db/post_migrate/20250123195500_add_board_assignees_group_id_index.rb new file mode 100644 index 0000000000000000000000000000000000000000..29ad3ccc2b05ccfa36681d9f90c1299bc6fb771e --- /dev/null +++ b/db/post_migrate/20250123195500_add_board_assignees_group_id_index.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddBoardAssigneesGroupIdIndex < Gitlab::Database::Migration[2.2] + INDEX_NAME = 'index_board_assignees_on_group_id' + + disable_ddl_transaction! + milestone '17.9' + + def up + add_concurrent_index :board_assignees, :group_id, name: INDEX_NAME + end + + def down + remove_concurrent_index :board_assignees, :group_id, name: INDEX_NAME + end +end diff --git a/db/post_migrate/20250123201951_add_board_assignees_sharding_key_not_null_constraint.rb b/db/post_migrate/20250123201951_add_board_assignees_sharding_key_not_null_constraint.rb new file mode 100644 index 0000000000000000000000000000000000000000..816a76579aa3ac6d781754cd3ba66588ad46f4a2 --- /dev/null +++ b/db/post_migrate/20250123201951_add_board_assignees_sharding_key_not_null_constraint.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddBoardAssigneesShardingKeyNotNullConstraint < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '17.9' + + def up + add_multi_column_not_null_constraint(:board_assignees, :group_id, :project_id) + end + + def down + remove_multi_column_not_null_constraint(:board_assignees, :group_id, :project_id) + end +end diff --git a/db/post_migrate/20250123202635_add_board_assignees_group_id_fk.rb b/db/post_migrate/20250123202635_add_board_assignees_group_id_fk.rb new file mode 100644 index 0000000000000000000000000000000000000000..28a5a8a27c0afbd0f8ccbebb8f0d2bc57c5f8a6d --- /dev/null +++ b/db/post_migrate/20250123202635_add_board_assignees_group_id_fk.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddBoardAssigneesGroupIdFk < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '17.9' + + def up + add_concurrent_foreign_key :board_assignees, + :namespaces, + column: :group_id, + target_column: :id, + reverse_lock_order: true + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :board_assignees, column: :group_id + end + end +end diff --git a/db/post_migrate/20250123202841_add_board_assignees_project_id_fk.rb b/db/post_migrate/20250123202841_add_board_assignees_project_id_fk.rb new file mode 100644 index 0000000000000000000000000000000000000000..6ae5a7da62669dd1cadf57c08c4b3c4b2a4fa1bd --- /dev/null +++ b/db/post_migrate/20250123202841_add_board_assignees_project_id_fk.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddBoardAssigneesProjectIdFk < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '17.9' + + def up + add_concurrent_foreign_key :board_assignees, + :projects, + column: :project_id, + target_column: :id, + reverse_lock_order: true + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :board_assignees, column: :project_id + end + end +end diff --git a/db/schema_migrations/20250123194127 b/db/schema_migrations/20250123194127 new file mode 100644 index 0000000000000000000000000000000000000000..cfa65b08b18419dbb867d7e46546888697327233 --- /dev/null +++ b/db/schema_migrations/20250123194127 @@ -0,0 +1 @@ +dd61b7388f7b41ec30ae87eb30f900c717e440affbb0a843a817453759c893ff \ No newline at end of file diff --git a/db/schema_migrations/20250123195300 b/db/schema_migrations/20250123195300 new file mode 100644 index 0000000000000000000000000000000000000000..6f1b125744b32a3f2ec07041fe21e1c36838b9bb --- /dev/null +++ b/db/schema_migrations/20250123195300 @@ -0,0 +1 @@ +19b415bfd61c7f2d64f71e129490c2283c93bac7d295879e7dc4b451b7a04ed0 \ No newline at end of file diff --git a/db/schema_migrations/20250123195400 b/db/schema_migrations/20250123195400 new file mode 100644 index 0000000000000000000000000000000000000000..83267d9c42ac0a526691ebcfa9fddf1fdab1e740 --- /dev/null +++ b/db/schema_migrations/20250123195400 @@ -0,0 +1 @@ +bb55a393c82125013ebaaf13e91a51daaa4a4ff2aad015f97383d33fbe1db3a4 \ No newline at end of file diff --git a/db/schema_migrations/20250123195500 b/db/schema_migrations/20250123195500 new file mode 100644 index 0000000000000000000000000000000000000000..5b29260bf56968e7d13ba36022e1aea4e8cbb7b5 --- /dev/null +++ b/db/schema_migrations/20250123195500 @@ -0,0 +1 @@ +9820c0c7b88c3454ad3482f0af37f83c28e795618f0172cff6533c49ac17b7ee \ No newline at end of file diff --git a/db/schema_migrations/20250123201951 b/db/schema_migrations/20250123201951 new file mode 100644 index 0000000000000000000000000000000000000000..d0c3f60bc9f8b02731203fb921d91b582a2181f5 --- /dev/null +++ b/db/schema_migrations/20250123201951 @@ -0,0 +1 @@ +13ac6fc0f2cbee82eb4fda9b27c9c92a47abde9fc8512b1d3a9ec2d8e2e813d7 \ No newline at end of file diff --git a/db/schema_migrations/20250123202635 b/db/schema_migrations/20250123202635 new file mode 100644 index 0000000000000000000000000000000000000000..6e773f25c1b8739d8e6f8d51efe37bea2bda9d56 --- /dev/null +++ b/db/schema_migrations/20250123202635 @@ -0,0 +1 @@ +8aa07ff2b7917401b35fe69967c19444e49f2858af9ea390e8b1475b426adb5c \ No newline at end of file diff --git a/db/schema_migrations/20250123202841 b/db/schema_migrations/20250123202841 new file mode 100644 index 0000000000000000000000000000000000000000..03d10168135a6fb8c996c7b9a47e08bb28efbe0a --- /dev/null +++ b/db/schema_migrations/20250123202841 @@ -0,0 +1 @@ +1d22128d9daaf82a9ab7144e8a2b71724bc3c75bcbe5c46adce1b6373c561770 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 60b0c65c708b7b404b4b05ea374c022e1d3d3da0..6f025df6c4d6237163e160895fb50bfe9947f36a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9025,7 +9025,10 @@ ALTER SEQUENCE batched_background_migrations_id_seq OWNED BY batched_background_ CREATE TABLE board_assignees ( id bigint NOT NULL, board_id bigint NOT NULL, - assignee_id bigint NOT NULL + assignee_id bigint NOT NULL, + group_id bigint, + project_id bigint, + CONSTRAINT check_b56ef26337 CHECK ((num_nonnulls(group_id, project_id) = 1)) ); CREATE SEQUENCE board_assignees_id_seq @@ -30937,6 +30940,10 @@ CREATE INDEX index_board_assignees_on_assignee_id ON board_assignees USING btree CREATE UNIQUE INDEX index_board_assignees_on_board_id_and_assignee_id ON board_assignees USING btree (board_id, assignee_id); +CREATE INDEX index_board_assignees_on_group_id ON board_assignees USING btree (group_id); + +CREATE INDEX index_board_assignees_on_project_id ON board_assignees USING btree (project_id); + CREATE INDEX index_board_group_recent_visits_on_board_id ON board_group_recent_visits USING btree (board_id); CREATE INDEX index_board_group_recent_visits_on_group_id ON board_group_recent_visits USING btree (group_id); @@ -37803,6 +37810,9 @@ ALTER TABLE ONLY deployment_approvals ALTER TABLE ONLY project_pages_metadata ADD CONSTRAINT fk_0fd5b22688 FOREIGN KEY (pages_deployment_id) REFERENCES pages_deployments(id) ON DELETE SET NULL; +ALTER TABLE ONLY board_assignees + ADD CONSTRAINT fk_105c1d6d08 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY audit_events_streaming_event_type_filters ADD CONSTRAINT fk_107946dffb FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; @@ -38247,6 +38257,9 @@ ALTER TABLE ONLY approval_group_rules_protected_branches ALTER TABLE ONLY project_compliance_standards_adherence ADD CONSTRAINT fk_4fd1d9d9b0 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE SET NULL; +ALTER TABLE ONLY board_assignees + ADD CONSTRAINT fk_50159bc755 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY approval_group_rules_groups ADD CONSTRAINT fk_50edc8134e FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; diff --git a/ee/app/models/board_assignee.rb b/ee/app/models/board_assignee.rb index d18d0a4678298c3adea5bf8c23cb80e181fe9e73..d3c32655a8e7ea742dba1c0e83514992cffdd752 100644 --- a/ee/app/models/board_assignee.rb +++ b/ee/app/models/board_assignee.rb @@ -3,7 +3,23 @@ class BoardAssignee < ApplicationRecord belongs_to :board belongs_to :assignee, class_name: 'User' + belongs_to :group + belongs_to :project validates :board, presence: true validates :assignee, presence: true + validates :group, presence: true, unless: :project + validates :project, presence: true, unless: :group + validates :group, absence: { + message: ->(_object, _data) { _("can't be specified if a project was already provided") } + }, if: :project + + before_validation :ensure_group_or_project + + private + + def ensure_group_or_project + self.group_id ||= board&.group_id + self.project_id ||= board&.project_id + end end diff --git a/ee/spec/models/board_assignee_spec.rb b/ee/spec/models/board_assignee_spec.rb index d488a4a480cce6cae42849c84939a73367c37d3a..bb6d3f91017bf905b997af3119445e41fe3a1062 100644 --- a/ee/spec/models/board_assignee_spec.rb +++ b/ee/spec/models/board_assignee_spec.rb @@ -11,5 +11,64 @@ describe 'validations' do it { is_expected.to validate_presence_of(:board) } it { is_expected.to validate_presence_of(:assignee) } + + describe 'group presence' do + it { is_expected.to validate_presence_of(:group) } + + context 'when project is present' do + subject { described_class.new(project: build_stubbed(:project)) } + + it { is_expected.not_to validate_presence_of(:group) } + end + end + + describe 'project presence' do + it { is_expected.to validate_presence_of(:project) } + + context 'when group is present' do + subject { described_class.new(group: build_stubbed(:group)) } + + it { is_expected.not_to validate_presence_of(:project) } + end + end + + describe 'group and project mutually exclusive' do + context 'when project is present' do + it 'validates that project and group are mutually exclusive' do + expect(described_class.new(project: build_stubbed(:project))).to validate_absence_of(:group) + .with_message(_("can't be specified if a project was already provided")) + end + end + + context 'when project is not present' do + it { is_expected.not_to validate_absence_of(:group) } + end + end + end + + describe 'ensure_group_or_project' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + context 'when board belongs to a group' do + let_it_be(:board) { create(:board, group: group) } + + it 'sets group_id from the parent board' do + board_assignee = described_class.create!(board: board, assignee: user) + + expect(board_assignee.group_id).to eq(board.group_id) + end + end + + context 'when board belongs to a project' do + let_it_be(:board) { create(:board, project: project) } + + it 'sets project_id from the parent board' do + board_assignee = described_class.create!(board: board, assignee: user) + + expect(board_assignee.project_id).to eq(board.project_id) + end + end end end diff --git a/spec/migrations/20250123195300_backfill_sharding_key_on_board_assignees_spec.rb b/spec/migrations/20250123195300_backfill_sharding_key_on_board_assignees_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ce1848db7cfb97be6ff52b9ead92a9c0b30d9a38 --- /dev/null +++ b/spec/migrations/20250123195300_backfill_sharding_key_on_board_assignees_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe BackfillShardingKeyOnBoardAssignees, migration: :gitlab_main, feature_category: :team_planning do + let(:organization) { table(:organizations).create!(name: 'organization', path: 'organization') } + let(:namespace) { table(:namespaces).create!(name: "namespace", path: "namespace", organization_id: organization.id) } + let(:project) do + table(:projects).create!( + namespace_id: namespace.id, + project_namespace_id: namespace.id, + organization_id: organization.id + ) + end + + let(:board1) { table(:boards).create!(group_id: namespace.id) } + let(:board2) { table(:boards).create!(project_id: project.id) } + let(:user1) { table(:users).create!(username: 'user1', projects_limit: 2, email: 'user1@mail.com') } + let(:user2) { table(:users).create!(username: 'user2', projects_limit: 2, email: 'user2@mail.com') } + + before do + [board1, board2].each do |board| + [user1, user2].each { |user| table(:board_assignees).create!(assignee_id: user.id, board_id: board.id) } + end + + stub_const("#{described_class}::BATCH_SIZE", 2) + end + + describe '#up' do + it 'updates records in batches' do + expect do + migrate! + end.to make_queries_matching(/UPDATE\s+"board_assignees"/, 2) + end + + it 'sets group_id or project_id in every record' do + expect { migrate! }.to change { + table(:board_assignees).order(:id).pluck(:group_id, :project_id) + }.from( + Array.new(4) { [nil, nil] } + ).to( + [ + [namespace.id, nil], + [namespace.id, nil], + [nil, project.id], + [nil, project.id] + ] + ) + end + end +end