From 2e305bfc9c05b6bdc6a2cc195377a3ec9cb0d6b7 Mon Sep 17 00:00:00 2001 From: Eugenia Grieff <egrieff@gitlab.com> Date: Mon, 16 Oct 2023 08:07:35 +0000 Subject: [PATCH] Add default related link restrictions Adds restrictions for predefined work item types. Changelog: added --- app/models/work_items/type.rb | 1 + ...ate_work_item_related_link_restrictions.rb | 5 ++ ...ate_work_item_related_link_restrictions.rb | 5 ++ ...dd_work_items_related_link_restrictions.rb | 80 +++++++++++++++++++ db/schema_migrations/20231005131445 | 1 + doc/development/work_items.md | 11 +++ .../related_links_restrictions_importer.rb | 74 +++++++++++++++++ ...ork_item_related_link_restrictions_spec.rb | 9 +++ ...ork_item_related_link_restrictions_spec.rb | 9 +++ .../work_items/related_link_restrictions.rb | 5 ++ ...elated_links_restrictions_importer_spec.rb | 10 +++ ...rk_items_related_link_restrictions_spec.rb | 37 +++++++++ .../related_link_restriction_spec.rb | 5 ++ spec/models/work_items/type_spec.rb | 3 + .../work_item_types_helper.rb | 3 + spec/support/helpers/test_env.rb | 1 + ...item_related_link_restrictions_importer.rb | 39 +++++++++ 17 files changed, 298 insertions(+) create mode 100644 db/fixtures/development/51_create_work_item_related_link_restrictions.rb create mode 100644 db/fixtures/production/040_create_work_item_related_link_restrictions.rb create mode 100644 db/post_migrate/20231005131445_add_work_items_related_link_restrictions.rb create mode 100644 db/schema_migrations/20231005131445 create mode 100644 lib/gitlab/database_importers/work_items/related_links_restrictions_importer.rb create mode 100644 spec/db/development/create_work_item_related_link_restrictions_spec.rb create mode 100644 spec/db/production/create_work_item_related_link_restrictions_spec.rb create mode 100644 spec/lib/gitlab/database_importers/work_items/related_links_restrictions_importer_spec.rb create mode 100644 spec/migrations/add_work_items_related_link_restrictions_spec.rb create mode 100644 spec/support/shared_examples/work_item_related_link_restrictions_importer.rb diff --git a/app/models/work_items/type.rb b/app/models/work_items/type.rb index b7ceeecbc7faf..4ccef4c93d38d 100644 --- a/app/models/work_items/type.rb +++ b/app/models/work_items/type.rb @@ -73,6 +73,7 @@ def self.default_by_type(type) Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.upsert_types Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter.upsert_restrictions + Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter.upsert_restrictions find_by(namespace_id: nil, base_type: type) end diff --git a/db/fixtures/development/51_create_work_item_related_link_restrictions.rb b/db/fixtures/development/51_create_work_item_related_link_restrictions.rb new file mode 100644 index 0000000000000..04440451d3738 --- /dev/null +++ b/db/fixtures/development/51_create_work_item_related_link_restrictions.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +Gitlab::Seeder.quiet do + Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter.upsert_restrictions +end diff --git a/db/fixtures/production/040_create_work_item_related_link_restrictions.rb b/db/fixtures/production/040_create_work_item_related_link_restrictions.rb new file mode 100644 index 0000000000000..04440451d3738 --- /dev/null +++ b/db/fixtures/production/040_create_work_item_related_link_restrictions.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +Gitlab::Seeder.quiet do + Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter.upsert_restrictions +end diff --git a/db/post_migrate/20231005131445_add_work_items_related_link_restrictions.rb b/db/post_migrate/20231005131445_add_work_items_related_link_restrictions.rb new file mode 100644 index 0000000000000..e26f6a367618a --- /dev/null +++ b/db/post_migrate/20231005131445_add_work_items_related_link_restrictions.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +class AddWorkItemsRelatedLinkRestrictions < Gitlab::Database::Migration[2.1] + RELATED = 0 + BLOCKS = 1 + + class WorkItemType < MigrationRecord + self.table_name = 'work_item_types' + end + + class RelatedLinkRestriction < MigrationRecord + self.table_name = 'work_item_related_link_restrictions' + end + + restrict_gitlab_migration gitlab_schema: :gitlab_main + disable_ddl_transaction! + + # rubocop:disable Metrics/AbcSize + def up + epic = WorkItemType.find_by_name_and_namespace_id('Epic', nil) + issue = WorkItemType.find_by_name_and_namespace_id('Issue', nil) + task = WorkItemType.find_by_name_and_namespace_id('Task', nil) + objective = WorkItemType.find_by_name_and_namespace_id('Objective', nil) + key_result = WorkItemType.find_by_name_and_namespace_id('Key Result', nil) + + unless epic && issue && task && objective && key_result + Gitlab::AppLogger.warn('Default WorkItemType records are missing, not adding RelatedLinkRestrictions.') + + return + end + + restrictions = [ + { source_type_id: epic.id, target_type_id: epic.id, link_type: RELATED }, + { source_type_id: epic.id, target_type_id: issue.id, link_type: RELATED }, + { source_type_id: epic.id, target_type_id: task.id, link_type: RELATED }, + { source_type_id: epic.id, target_type_id: objective.id, link_type: RELATED }, + { source_type_id: epic.id, target_type_id: key_result.id, link_type: RELATED }, + { source_type_id: issue.id, target_type_id: issue.id, link_type: RELATED }, + { source_type_id: issue.id, target_type_id: task.id, link_type: RELATED }, + { source_type_id: issue.id, target_type_id: objective.id, link_type: RELATED }, + { source_type_id: issue.id, target_type_id: key_result.id, link_type: RELATED }, + { source_type_id: task.id, target_type_id: task.id, link_type: RELATED }, + { source_type_id: task.id, target_type_id: objective.id, link_type: RELATED }, + { source_type_id: task.id, target_type_id: key_result.id, link_type: RELATED }, + { source_type_id: objective.id, target_type_id: objective.id, link_type: RELATED }, + { source_type_id: objective.id, target_type_id: key_result.id, link_type: RELATED }, + { source_type_id: key_result.id, target_type_id: key_result.id, link_type: RELATED }, + { source_type_id: epic.id, target_type_id: epic.id, link_type: BLOCKS }, + { source_type_id: epic.id, target_type_id: issue.id, link_type: BLOCKS }, + { source_type_id: epic.id, target_type_id: task.id, link_type: BLOCKS }, + { source_type_id: epic.id, target_type_id: objective.id, link_type: BLOCKS }, + { source_type_id: epic.id, target_type_id: key_result.id, link_type: BLOCKS }, + { source_type_id: issue.id, target_type_id: issue.id, link_type: BLOCKS }, + { source_type_id: issue.id, target_type_id: epic.id, link_type: BLOCKS }, + { source_type_id: issue.id, target_type_id: task.id, link_type: BLOCKS }, + { source_type_id: issue.id, target_type_id: objective.id, link_type: BLOCKS }, + { source_type_id: issue.id, target_type_id: key_result.id, link_type: BLOCKS }, + { source_type_id: task.id, target_type_id: task.id, link_type: BLOCKS }, + { source_type_id: task.id, target_type_id: epic.id, link_type: BLOCKS }, + { source_type_id: task.id, target_type_id: issue.id, link_type: BLOCKS }, + { source_type_id: task.id, target_type_id: objective.id, link_type: BLOCKS }, + { source_type_id: task.id, target_type_id: key_result.id, link_type: BLOCKS }, + { source_type_id: objective.id, target_type_id: objective.id, link_type: BLOCKS }, + { source_type_id: objective.id, target_type_id: key_result.id, link_type: BLOCKS }, + { source_type_id: key_result.id, target_type_id: key_result.id, link_type: BLOCKS }, + { source_type_id: key_result.id, target_type_id: objective.id, link_type: BLOCKS } + ] + + RelatedLinkRestriction.upsert_all( + restrictions, + unique_by: :index_work_item_link_restrictions_on_source_link_type_target + ) + end + # rubocop:enable Metrics/AbcSize + + def down + # Until this point the restrictions table was empty so we can delete all records when migrating down + RelatedLinkRestriction.delete_all + end +end diff --git a/db/schema_migrations/20231005131445 b/db/schema_migrations/20231005131445 new file mode 100644 index 0000000000000..5bc8e37f95279 --- /dev/null +++ b/db/schema_migrations/20231005131445 @@ -0,0 +1 @@ +564a5cf5e449653a4492cfc25a010d7fb4cf7d4bc30c2c414f7a667ae147e7aa \ No newline at end of file diff --git a/doc/development/work_items.md b/doc/development/work_items.md index 927b5a0a46d2b..73993b1d9ee05 100644 --- a/doc/development/work_items.md +++ b/doc/development/work_items.md @@ -246,6 +246,17 @@ Keep the following in mind when you write your migration: work item type can have children and of what type. Also, you should specify the hierarchy depth for work items of the same type. By default a cross-hierarchy (cross group or project) relationship is disabled when creating new restrictions but it can be enabled by specifying a value for `cross_hierarchy_enabled`. +- Optional. Create linked item restrictions. + - Similarly to the `Hierarchy` widget, the `Linked items` widget also supports rules defining which work item types can be + linked to other types. A restriction can specify if the source type can be related to or blocking a target type. Current restrictions: + + | Type | Can be related to | Can block | Can be blocked by | + |------------|------------------------------------------|------------------------------------------|------------------------------------------| + | Epic | Epic, issue, task, objective, key result | Epic, issue, task, objective, key result | Epic, issue, task | + | Issue | Epic, issue, task, objective, key result | Epic, issue, task, objective, key result | Epic, issue, task | + | Task | Epic, issue, task, objective, key result | Epic, issue, task, objective, key result | Epic, issue, task | + | Objective | Epic, issue, task, objective, key result | Objective, key result | Epic, issue, task, objective, key result | + | Key result | Epic, issue, task, objective, key result | Objective, key result | Epic, issue, task, objective, key result | ##### Example of adding a ticket work item diff --git a/lib/gitlab/database_importers/work_items/related_links_restrictions_importer.rb b/lib/gitlab/database_importers/work_items/related_links_restrictions_importer.rb new file mode 100644 index 0000000000000..692764bd16d6c --- /dev/null +++ b/lib/gitlab/database_importers/work_items/related_links_restrictions_importer.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module Gitlab + module DatabaseImporters + module WorkItems + module RelatedLinksRestrictionsImporter + # This importer populates the default link restrictions for the base work item types that support this feature. + # These rules are documented in https://docs.gitlab.com/ee/development/work_items.html#write-a-database-migration + + # rubocop:disable Metrics/AbcSize + def self.upsert_restrictions + epic = find_or_create_type(::WorkItems::Type::TYPE_NAMES[:epic]) + issue = find_or_create_type(::WorkItems::Type::TYPE_NAMES[:issue]) + task = find_or_create_type(::WorkItems::Type::TYPE_NAMES[:task]) + objective = find_or_create_type(::WorkItems::Type::TYPE_NAMES[:objective]) + key_result = find_or_create_type(::WorkItems::Type::TYPE_NAMES[:key_result]) + + restrictions = [ + # Source can relate to target and target can relate to source + { source_type_id: epic.id, target_type_id: epic.id, link_type: 0 }, + { source_type_id: epic.id, target_type_id: issue.id, link_type: 0 }, + { source_type_id: epic.id, target_type_id: task.id, link_type: 0 }, + { source_type_id: epic.id, target_type_id: objective.id, link_type: 0 }, + { source_type_id: epic.id, target_type_id: key_result.id, link_type: 0 }, + { source_type_id: issue.id, target_type_id: issue.id, link_type: 0 }, + { source_type_id: issue.id, target_type_id: task.id, link_type: 0 }, + { source_type_id: issue.id, target_type_id: objective.id, link_type: 0 }, + { source_type_id: issue.id, target_type_id: key_result.id, link_type: 0 }, + { source_type_id: task.id, target_type_id: task.id, link_type: 0 }, + { source_type_id: task.id, target_type_id: objective.id, link_type: 0 }, + { source_type_id: task.id, target_type_id: key_result.id, link_type: 0 }, + { source_type_id: objective.id, target_type_id: objective.id, link_type: 0 }, + { source_type_id: objective.id, target_type_id: key_result.id, link_type: 0 }, + { source_type_id: key_result.id, target_type_id: key_result.id, link_type: 0 }, + # Source can block target and target can be blocked by source + { source_type_id: epic.id, target_type_id: epic.id, link_type: 1 }, + { source_type_id: epic.id, target_type_id: issue.id, link_type: 1 }, + { source_type_id: epic.id, target_type_id: task.id, link_type: 1 }, + { source_type_id: epic.id, target_type_id: objective.id, link_type: 1 }, + { source_type_id: epic.id, target_type_id: key_result.id, link_type: 1 }, + { source_type_id: issue.id, target_type_id: issue.id, link_type: 1 }, + { source_type_id: issue.id, target_type_id: epic.id, link_type: 1 }, + { source_type_id: issue.id, target_type_id: task.id, link_type: 1 }, + { source_type_id: issue.id, target_type_id: objective.id, link_type: 1 }, + { source_type_id: issue.id, target_type_id: key_result.id, link_type: 1 }, + { source_type_id: task.id, target_type_id: task.id, link_type: 1 }, + { source_type_id: task.id, target_type_id: epic.id, link_type: 1 }, + { source_type_id: task.id, target_type_id: issue.id, link_type: 1 }, + { source_type_id: task.id, target_type_id: objective.id, link_type: 1 }, + { source_type_id: task.id, target_type_id: key_result.id, link_type: 1 }, + { source_type_id: objective.id, target_type_id: objective.id, link_type: 1 }, + { source_type_id: objective.id, target_type_id: key_result.id, link_type: 1 }, + { source_type_id: key_result.id, target_type_id: key_result.id, link_type: 1 }, + { source_type_id: key_result.id, target_type_id: objective.id, link_type: 1 } + ] + + ::WorkItems::RelatedLinkRestriction.upsert_all( + restrictions, + unique_by: :index_work_item_link_restrictions_on_source_link_type_target + ) + end + # rubocop:enable Metrics/AbcSize + + def self.find_or_create_type(name) + type = ::WorkItems::Type.find_by_name_and_namespace_id(name, nil) + return type if type + + Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.upsert_types + ::WorkItems::Type.find_by_name_and_namespace_id(name, nil) + end + end + end + end +end diff --git a/spec/db/development/create_work_item_related_link_restrictions_spec.rb b/spec/db/development/create_work_item_related_link_restrictions_spec.rb new file mode 100644 index 0000000000000..5a7ade3c83e16 --- /dev/null +++ b/spec/db/development/create_work_item_related_link_restrictions_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Create work item related links restrictions in development', feature_category: :portfolio_management do + subject { load Rails.root.join('db/fixtures/development/51_create_work_item_related_link_restrictions.rb') } + + it_behaves_like 'work item related links restrictions importer' +end diff --git a/spec/db/production/create_work_item_related_link_restrictions_spec.rb b/spec/db/production/create_work_item_related_link_restrictions_spec.rb new file mode 100644 index 0000000000000..e3147593327e0 --- /dev/null +++ b/spec/db/production/create_work_item_related_link_restrictions_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Create work item related links restrictions in production', feature_category: :portfolio_management do + subject { load Rails.root.join('db/fixtures/production/040_create_work_item_related_link_restrictions.rb') } + + it_behaves_like 'work item related links restrictions importer' +end diff --git a/spec/factories/work_items/related_link_restrictions.rb b/spec/factories/work_items/related_link_restrictions.rb index b1e2547f00428..c0e4f188b5f99 100644 --- a/spec/factories/work_items/related_link_restrictions.rb +++ b/spec/factories/work_items/related_link_restrictions.rb @@ -5,5 +5,10 @@ source_type { association :work_item_type, :default } target_type { association :work_item_type, :default } link_type { 0 } + + initialize_with do + WorkItems::RelatedLinkRestriction + .find_or_initialize_by(source_type: source_type, target_type: target_type, link_type: link_type) + end end end diff --git a/spec/lib/gitlab/database_importers/work_items/related_links_restrictions_importer_spec.rb b/spec/lib/gitlab/database_importers/work_items/related_links_restrictions_importer_spec.rb new file mode 100644 index 0000000000000..39d02922acc81 --- /dev/null +++ b/spec/lib/gitlab/database_importers/work_items/related_links_restrictions_importer_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter, + feature_category: :portfolio_management do + subject { described_class.upsert_restrictions } + + it_behaves_like 'work item related links restrictions importer' +end diff --git a/spec/migrations/add_work_items_related_link_restrictions_spec.rb b/spec/migrations/add_work_items_related_link_restrictions_spec.rb new file mode 100644 index 0000000000000..e1e0b4c35ff7e --- /dev/null +++ b/spec/migrations/add_work_items_related_link_restrictions_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe AddWorkItemsRelatedLinkRestrictions, :migration, feature_category: :portfolio_management do + let!(:restrictions) { table(:work_item_related_link_restrictions) } + let!(:work_item_types) { table(:work_item_types) } + + # These rules are documented in https://docs.gitlab.com/ee/development/work_items.html#write-a-database-migration + it 'creates default restrictions' do + restrictions.delete_all + + reversible_migration do |migration| + migration.before -> { + expect(restrictions.count).to eq(0) + } + + migration.after -> { + expect(restrictions.count).to eq(34) + } + end + end + + context 'when work item types are missing' do + before do + work_item_types.delete_all + end + + it 'does not add restrictions' do + expect(Gitlab::AppLogger).to receive(:warn) + .with('Default WorkItemType records are missing, not adding RelatedLinkRestrictions.') + + expect { migrate! }.not_to change { restrictions.count } + end + end +end diff --git a/spec/models/work_items/related_link_restriction_spec.rb b/spec/models/work_items/related_link_restriction_spec.rb index 1dc2286c0bf61..764ada53f8b10 100644 --- a/spec/models/work_items/related_link_restriction_spec.rb +++ b/spec/models/work_items/related_link_restriction_spec.rb @@ -9,6 +9,11 @@ end describe 'validations' do + before do + # delete seeded records to prevent non-unique record error + described_class.delete_all + end + subject { build(:related_link_restriction) } it { is_expected.to validate_presence_of(:source_type) } diff --git a/spec/models/work_items/type_spec.rb b/spec/models/work_items/type_spec.rb index e4d2ccdfc5ae8..7f836ce4e9013 100644 --- a/spec/models/work_items/type_spec.rb +++ b/spec/models/work_items/type_spec.rb @@ -83,6 +83,8 @@ expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).not_to receive(:upsert_types).and_call_original expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).not_to receive(:upsert_widgets) expect(Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter).not_to receive(:upsert_restrictions) + expect(Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter) + .not_to receive(:upsert_restrictions) expect(subject).to eq(default_issue_type) end @@ -96,6 +98,7 @@ expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).to receive(:upsert_types).and_call_original expect(Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter).to receive(:upsert_widgets) expect(Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter).to receive(:upsert_restrictions) + expect(Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter).to receive(:upsert_restrictions) expect(subject).to eq(default_issue_type) end diff --git a/spec/support/helpers/migrations_helpers/work_item_types_helper.rb b/spec/support/helpers/migrations_helpers/work_item_types_helper.rb index 5f1f7869bf8c3..9d114ae82b105 100644 --- a/spec/support/helpers/migrations_helpers/work_item_types_helper.rb +++ b/spec/support/helpers/migrations_helpers/work_item_types_helper.rb @@ -6,6 +6,9 @@ def reset_work_item_types Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.upsert_types WorkItems::HierarchyRestriction.reset_column_information Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter.upsert_restrictions + return unless WorkItems::RelatedLinkRestriction.table_exists? + + Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter.upsert_restrictions end end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index b95adb3fe4dfb..fb663011e8f29 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -396,6 +396,7 @@ def forked_repo_bundle_path def seed_db Gitlab::DatabaseImporters::WorkItems::BaseTypeImporter.upsert_types Gitlab::DatabaseImporters::WorkItems::HierarchyRestrictionsImporter.upsert_restrictions + Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter.upsert_restrictions end private diff --git a/spec/support/shared_examples/work_item_related_link_restrictions_importer.rb b/spec/support/shared_examples/work_item_related_link_restrictions_importer.rb new file mode 100644 index 0000000000000..935ad2ba472a3 --- /dev/null +++ b/spec/support/shared_examples/work_item_related_link_restrictions_importer.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'work item related links restrictions importer' do + shared_examples_for 'adds restrictions' do + it "adds all restrictions if they don't exist" do + expect { subject }.to change { WorkItems::RelatedLinkRestriction.count }.from(0).to(34) + end + end + + context 'when restrictions are missing' do + before do + WorkItems::RelatedLinkRestriction.delete_all + end + + it_behaves_like 'adds restrictions' + end + + context 'when base types are missing' do + before do + WorkItems::Type.delete_all + end + + it_behaves_like 'adds restrictions' + end + + context 'when some restrictions are missing' do + before do + Gitlab::DatabaseImporters::WorkItems::RelatedLinksRestrictionsImporter.upsert_restrictions + WorkItems::RelatedLinkRestriction.limit(1).delete_all + end + + it 'inserts missing restrictions and does nothing if some already existed' do + expect { subject }.to make_queries_matching(/INSERT/, 1).and( + change { WorkItems::RelatedLinkRestriction.count }.by(1) + ) + expect(WorkItems::RelatedLinkRestriction.count).to eq(34) + end + end +end -- GitLab