Merge branch '352437-create-related-epics-table' into 'master'

Create RelatedEpicLink table and model

See merge request gitlab-org/gitlab!80499

app/models/concerns/issuable_link.rb

+# frozen_string_literal: true
+
+# == IssuableLink concern
+#
+# Contains common functionality shared between related Issues and related Epics
+#
+# Used by IssueLink, Epic::RelatedEpicLink
+#
+module IssuableLink
+  extend ActiveSupport::Concern
+
+  TYPE_RELATES_TO = 'relates_to'
+  TYPE_BLOCKS = 'blocks'
+  # we don't store is_blocked_by in the db but need it for displaying the relation
+  # from the target
+  TYPE_IS_BLOCKED_BY = 'is_blocked_by'
+
+  class_methods do
+    def inverse_link_type(type)
+      type
+    end
+  end
+
+  included do
+    validates :source, presence: true
+    validates :target, presence: true
+    validates :source, uniqueness: { scope: :target_id, message: 'is already related' }
+    validate :check_self_relation
+    validate :check_opposite_relation
+
+    enum link_type: { TYPE_RELATES_TO => 0, TYPE_BLOCKS => 1 }
+
+    private
+
+    def check_self_relation
+      return unless source && target
+
+      if source == target
+        errors.add(:source, 'cannot be related to itself')
+      end
+    end
+
+    def check_opposite_relation
+      return unless source && target
+
+      if self.class.base_class.find_by(source: target, target: source)
+        errors.add(:source, "is already related to this #{issuable_type}")
+      end
+    end
+
+    def issuable_type
+      raise NotImplementedError
+    end
+  end
+end

app/models/issue_link.rb

@@ -2,47 +2,18 @@
 
 class IssueLink < ApplicationRecord
   include FromUnion
+  include IssuableLink
 
   belongs_to :source, class_name: 'Issue'
   belongs_to :target, class_name: 'Issue'
 
-  validates :source, presence: true
-  validates :target, presence: true
-  validates :source, uniqueness: { scope: :target_id, message: 'is already related' }
-  validate :check_self_relation
-  validate :check_opposite_relation
-
   scope :for_source_issue, ->(issue) { where(source_id: issue.id) }
   scope :for_target_issue, ->(issue) { where(target_id: issue.id) }
 
-  TYPE_RELATES_TO = 'relates_to'
-  TYPE_BLOCKS = 'blocks'
-  # we don't store is_blocked_by in the db but need it for displaying the relation
-  # from the target (used in IssueLink.inverse_link_type)
-  TYPE_IS_BLOCKED_BY = 'is_blocked_by'
-
-  enum link_type: { TYPE_RELATES_TO => 0, TYPE_BLOCKS => 1 }
-
-  def self.inverse_link_type(type)
-    type
-  end
-
   private
 
-  def check_self_relation
-    return unless source && target
-
-    if source == target
-      errors.add(:source, 'cannot be related to itself')
-    end
-  end
-
-  def check_opposite_relation
-    return unless source && target
-
-    if IssueLink.find_by(source: target, target: source)
-      errors.add(:source, 'is already related to this issue')
-    end
+  def issuable_type
+    :issue
   end
 end
 

db/migrate/20220211125954_create_related_epic_links.rb

+# frozen_string_literal: true
+
+class CreateRelatedEpicLinks < Gitlab::Database::Migration[1.0]
+  def up
+    create_table :related_epic_links do |t|
+      t.references :source, index: true, foreign_key: { to_table: :epics, on_delete: :cascade }, null: false
+      t.references :target, index: true, foreign_key: { to_table: :epics, on_delete: :cascade }, null: false
+      t.timestamps_with_timezone null: false
+      t.integer :link_type, null: false, default: 0, limit: 2
+
+      t.index [:source_id, :target_id], unique: true
+    end
+  end
+
+  def down
+    drop_table :related_epic_links
+  end
+end

db/schema_migrations/20220211125954

+73feefe409b9c0f4ea373d0c3f13690df0086fbc4fc212595e959ad65fcc27b1
\ No newline at end of file

db/structure.sql

@@ -19241,6 +19241,24 @@ CREATE SEQUENCE redirect_routes_id_seq
 
 ALTER SEQUENCE redirect_routes_id_seq OWNED BY redirect_routes.id;
 
+CREATE TABLE related_epic_links (
+    id bigint NOT NULL,
+    source_id bigint NOT NULL,
+    target_id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    link_type smallint DEFAULT 0 NOT NULL
+);
+
+CREATE SEQUENCE related_epic_links_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE related_epic_links_id_seq OWNED BY related_epic_links.id;
+
 CREATE TABLE release_links (
     id bigint NOT NULL,
     release_id integer NOT NULL,
@@ -22327,6 +22345,8 @@ ALTER TABLE ONLY raw_usage_data ALTER COLUMN id SET DEFAULT nextval('raw_usage_d
 
 ALTER TABLE ONLY redirect_routes ALTER COLUMN id SET DEFAULT nextval('redirect_routes_id_seq'::regclass);
 
+ALTER TABLE ONLY related_epic_links ALTER COLUMN id SET DEFAULT nextval('related_epic_links_id_seq'::regclass);
+
 ALTER TABLE ONLY release_links ALTER COLUMN id SET DEFAULT nextval('release_links_id_seq'::regclass);
 
 ALTER TABLE ONLY releases ALTER COLUMN id SET DEFAULT nextval('releases_id_seq'::regclass);
@@ -24254,6 +24274,9 @@ ALTER TABLE ONLY raw_usage_data
 ALTER TABLE ONLY redirect_routes
     ADD CONSTRAINT redirect_routes_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY related_epic_links
+    ADD CONSTRAINT related_epic_links_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY release_links
     ADD CONSTRAINT release_links_pkey PRIMARY KEY (id);
 
@@ -27704,6 +27727,12 @@ CREATE UNIQUE INDEX index_redirect_routes_on_path_unique_text_pattern_ops ON red
 
 CREATE INDEX index_redirect_routes_on_source_type_and_source_id ON redirect_routes USING btree (source_type, source_id);
 
+CREATE INDEX index_related_epic_links_on_source_id ON related_epic_links USING btree (source_id);
+
+CREATE UNIQUE INDEX index_related_epic_links_on_source_id_and_target_id ON related_epic_links USING btree (source_id, target_id);
+
+CREATE INDEX index_related_epic_links_on_target_id ON related_epic_links USING btree (target_id);
+
 CREATE UNIQUE INDEX index_release_links_on_release_id_and_name ON release_links USING btree (release_id, name);
 
 CREATE UNIQUE INDEX index_release_links_on_release_id_and_url ON release_links USING btree (release_id, url);
@@ -30334,6 +30363,9 @@ ALTER TABLE ONLY packages_debian_group_distributions
 ALTER TABLE ONLY packages_conan_file_metadata
     ADD CONSTRAINT fk_rails_0afabd9328 FOREIGN KEY (package_file_id) REFERENCES packages_package_files(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY related_epic_links
+    ADD CONSTRAINT fk_rails_0b72027748 FOREIGN KEY (target_id) REFERENCES epics(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY ci_build_pending_states
     ADD CONSTRAINT fk_rails_0bbbfeaf9d FOREIGN KEY (build_id) REFERENCES ci_builds(id) ON DELETE CASCADE;
 
@@ -31468,6 +31500,9 @@ ALTER TABLE ONLY group_deploy_keys_groups
 ALTER TABLE ONLY merge_request_user_mentions
     ADD CONSTRAINT fk_rails_c440b9ea31 FOREIGN KEY (note_id) REFERENCES notes(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY related_epic_links
+    ADD CONSTRAINT fk_rails_c464534def FOREIGN KEY (source_id) REFERENCES epics(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY boards_epic_board_recent_visits
     ADD CONSTRAINT fk_rails_c4dcba4a3e FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
 

ee/app/models/epic/related_epic_link.rb

+# frozen_string_literal: true
+
+class Epic::RelatedEpicLink < ApplicationRecord
+  include IssuableLink
+
+  belongs_to :source, class_name: 'Epic'
+  belongs_to :target, class_name: 'Epic'
+
+  self.table_name = 'related_epic_links'
+
+  private
+
+  def issuable_type
+    :epic
+  end
+end

ee/spec/factories/related_epic_links.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :related_epic_link, class: 'Epic::RelatedEpicLink' do
+    source factory: :epic
+    target factory: :epic
+  end
+end

ee/spec/models/epic/related_epic_link_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Epic::RelatedEpicLink do
+  it_behaves_like 'issuable link' do
+    let_it_be_with_reload(:issuable_link) { create(:related_epic_link) }
+    let_it_be(:issuable) { create(:epic) }
+    let(:issuable_class) { 'Epic' }
+    let(:issuable_link_factory) { :related_epic_link }
+  end
+end

lib/gitlab/database/gitlab_schemas.yml

@@ -442,6 +442,7 @@ push_event_payloads: :gitlab_main
 push_rules: :gitlab_main
 raw_usage_data: :gitlab_main
 redirect_routes: :gitlab_main
+related_epic_links: :gitlab_main
 release_links: :gitlab_main
 releases: :gitlab_main
 remote_mirrors: :gitlab_main

spec/models/concerns/issuable_link_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IssuableLink do
+  let(:test_class) do
+    Class.new(ApplicationRecord) do
+      include IssuableLink
+
+      self.table_name = 'issue_links'
+
+      belongs_to :source, class_name: 'Issue'
+      belongs_to :target, class_name: 'Issue'
+
+      def self.name
+        'TestClass'
+      end
+    end
+  end
+
+  describe '.inverse_link_type' do
+    it 'returns the inverse type of link' do
+      expect(test_class.inverse_link_type('relates_to')).to eq('relates_to')
+      expect(test_class.inverse_link_type('is_blocked_by')).to eq('is_blocked_by')
+      expect(test_class.inverse_link_type('blocks')).to eq('blocks')
+    end
+  end
+
+  describe '.issuable_type' do
+    let_it_be(:source_issue) { create(:issue) }
+    let_it_be(:target_issue) { create(:issue) }
+
+    before do
+      test_class.create!(source: source_issue, target: target_issue)
+    end
+
+    context 'when opposite relation already exists' do
+      it 'raises NotImplementedError when performing validations' do
+        instance = test_class.new(source: target_issue, target: source_issue)
+
+        expect { instance.save! }.to raise_error(NotImplementedError)
+      end
+    end
+  end
+end

spec/models/issue_link_spec.rb

@@ -3,57 +3,38 @@
 require 'spec_helper'
 
 RSpec.describe IssueLink do
-  describe 'Associations' do
-    it { is_expected.to belong_to(:source).class_name('Issue') }
-    it { is_expected.to belong_to(:target).class_name('Issue') }
+  it_behaves_like 'issuable link' do
+    let_it_be_with_reload(:issuable_link) { create(:issue_link) }
+    let_it_be(:issuable) { create(:issue) }
+    let(:issuable_class) { 'Issue' }
+    let(:issuable_link_factory) { :issue_link }
   end
 
-  describe 'link_type' do
-    it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1) }
+  describe 'Scopes' do
+    let_it_be(:issue1) { create(:issue) }
+    let_it_be(:issue2) { create(:issue) }
 
-    it 'provides the "related" as default link_type' do
-      expect(create(:issue_link).link_type).to eq 'relates_to'
-    end
-  end
-
-  describe 'Validation' do
-    subject { create :issue_link }
-
-    it { is_expected.to validate_presence_of(:source) }
-    it { is_expected.to validate_presence_of(:target) }
-    it do
-      is_expected.to validate_uniqueness_of(:source)
-                       .scoped_to(:target_id)
-                       .with_message(/already related/)
-    end
+    describe '.for_source_issue' do
+      it 'includes linked issues for source issue' do
+        source_issue = create(:issue)
+        issue_link_1 = create(:issue_link, source: source_issue, target: issue1)
+        issue_link_2 = create(:issue_link, source: source_issue, target: issue2)
 
-    it 'is not valid if an opposite link already exists' do
-      issue_link = build(:issue_link, source: subject.target, target: subject.source)
+        result = described_class.for_source_issue(source_issue)
 
-      expect(issue_link).to be_invalid
-      expect(issue_link.errors[:source]).to include('is already related to this issue')
+        expect(result).to contain_exactly(issue_link_1, issue_link_2)
+      end
     end
 
-    context 'when it relates to itself' do
-      let(:issue) { create :issue }
-
-      context 'cannot be validated' do
-        it 'does not invalidate object with self relation error' do
-          issue_link = build :issue_link, source: issue, target: nil
-
-          issue_link.valid?
-
-          expect(issue_link.errors[:source]).to be_empty
-        end
-      end
+    describe '.for_target_issue' do
+      it 'includes linked issues for target issue' do
+        target_issue = create(:issue)
+        issue_link_1 = create(:issue_link, source: issue1, target: target_issue)
+        issue_link_2 = create(:issue_link, source: issue2, target: target_issue)
 
-      context 'can be invalidated' do
-        it 'invalidates object' do
-          issue_link = build :issue_link, source: issue, target: issue
+        result = described_class.for_target_issue(target_issue)
 
-          expect(issue_link).to be_invalid
-          expect(issue_link.errors[:source]).to include('cannot be related to itself')
-        end
+        expect(result).to contain_exactly(issue_link_1, issue_link_2)
       end
     end
   end

spec/support/shared_examples/models/issuable_link_shared_examples.rb

+# frozen_string_literal: true
+
+# This shared example requires the following variables
+# issuable_link
+# issuable
+# issuable_class
+# issuable_link_factory
+RSpec.shared_examples 'issuable link' do
+  describe 'Associations' do
+    it { is_expected.to belong_to(:source).class_name(issuable.class.name) }
+    it { is_expected.to belong_to(:target).class_name(issuable.class.name) }
+  end
+
+  describe 'Validation' do
+    subject { issuable_link }
+
+    it { is_expected.to validate_presence_of(:source) }
+    it { is_expected.to validate_presence_of(:target) }
+    it do
+      is_expected.to validate_uniqueness_of(:source)
+                       .scoped_to(:target_id)
+                       .with_message(/already related/)
+    end
+
+    it 'is not valid if an opposite link already exists' do
+      issuable_link = create_issuable_link(subject.target, subject.source)
+
+      expect(issuable_link).to be_invalid
+      expect(issuable_link.errors[:source]).to include("is already related to this #{issuable.class.name.downcase}")
+    end
+
+    context 'when it relates to itself' do
+      context 'when target is nil' do
+        it 'does not invalidate object with self relation error' do
+          issuable_link = create_issuable_link(issuable, nil)
+
+          issuable_link.valid?
+
+          expect(issuable_link.errors[:source]).to be_empty
+        end
+      end
+
+      context 'when source and target are present' do
+        it 'invalidates object' do
+          issuable_link = create_issuable_link(issuable, issuable)
+
+          expect(issuable_link).to be_invalid
+          expect(issuable_link.errors[:source]).to include('cannot be related to itself')
+        end
+      end
+    end
+
+    def create_issuable_link(source, target)
+      build(issuable_link_factory, source: source, target: target)
+    end
+  end
+
+  describe '.link_type' do
+    it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1) }
+
+    it 'provides the "related" as default link_type' do
+      expect(issuable_link.link_type).to eq 'relates_to'
+    end
+  end
+end