Merge branch 'link-types' into 'master'

Added issue link type

See merge request gitlab-org/gitlab!20617

db/migrate/20191121193110_add_issue_links_type.rb

+# frozen_string_literal: true
+
+class AddIssueLinksType < ActiveRecord::Migration[5.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default :issue_links, :link_type, :integer, default: 0, limit: 2
+  end
+
+  def down
+    remove_column :issue_links, :link_type
+  end
+end

db/schema.rb

@@ -2049,6 +2049,7 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do
     t.integer "target_id", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.integer "link_type", limit: 2, default: 0, null: false
     t.index ["source_id", "target_id"], name: "index_issue_links_on_source_id_and_target_id", unique: true
     t.index ["source_id"], name: "index_issue_links_on_source_id"
     t.index ["target_id"], name: "index_issue_links_on_target_id"

ee/app/controllers/concerns/issuable_links.rb

@@ -28,7 +28,7 @@ module IssuableLinks
   end
 
   def create_params
-    params.slice(:issuable_references)
+    params.permit(issuable_references: [])
   end
 
   def create_service

ee/app/controllers/projects/issue_links_controller.rb

@@ -40,5 +40,9 @@ module Projects
     def link
       @link ||= IssueLink.find(params[:id])
     end
+
+    def create_params
+      params.permit(:link_type, issuable_references: [])
+    end
   end
 end

ee/app/models/ee/issue.rb

@@ -92,13 +92,15 @@ module EE
 
     def related_issues(current_user, preload: nil)
       related_issues = ::Issue
-                           .select(['issues.*', 'issue_links.id AS issue_link_id'])
-                           .joins("INNER JOIN issue_links ON
-                                 (issue_links.source_id = issues.id AND issue_links.target_id = #{id})
-                                 OR
-                                 (issue_links.target_id = issues.id AND issue_links.source_id = #{id})")
-                           .preload(preload)
-                           .reorder('issue_link_id')
+        .select(['issues.*', 'issue_links.id AS issue_link_id',
+                 'issue_links.link_type as issue_link_type_value',
+                 'issue_links.target_id as issue_link_source_id'])
+        .joins("INNER JOIN issue_links ON
+               (issue_links.source_id = issues.id AND issue_links.target_id = #{id})
+               OR
+               (issue_links.target_id = issues.id AND issue_links.source_id = #{id})")
+        .preload(preload)
+        .reorder('issue_link_id')
 
       cross_project_filter = -> (issues) { issues.where(project: project) }
       Ability.issues_readable_by_user(related_issues,
@@ -129,6 +131,15 @@ module EE
       !!promoted_to_epic_id
     end
 
+    def issue_link_type
+      return unless respond_to?(:issue_link_type_value) && respond_to?(:issue_link_source_id)
+
+      type = IssueLink.link_types.key(issue_link_type_value) || IssueLink::TYPE_RELATES_TO
+      return type if issue_link_source_id == id
+
+      IssueLink.inverse_link_type(type)
+    end
+
     class_methods do
       # override
       def sort_by_attribute(method, excluded_labels: [])

ee/app/models/issue_link.rb

@@ -12,6 +12,23 @@ class IssueLink < ApplicationRecord
   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'
+  TYPE_IS_BLOCKED_BY = 'is_blocked_by'
+
+  enum link_type: { TYPE_RELATES_TO => 0, TYPE_BLOCKS => 1, TYPE_IS_BLOCKED_BY => 2 }
+
+  def self.inverse_link_type(type)
+    case type
+    when TYPE_BLOCKS
+      TYPE_IS_BLOCKED_BY
+    when TYPE_IS_BLOCKED_BY
+      TYPE_BLOCKS
+    else
+      type
+    end
+  end
+
   private
 
   def check_self_relation

ee/app/serializers/linked_project_issue_entity.rb

@@ -11,6 +11,10 @@ class LinkedProjectIssueEntity < LinkedIssueEntity
     end
   end
 
+  expose :link_type, if: -> (issue, _) { Feature.enabled?(:issue_link_types, issue.project) } do |issue|
+    issue.issue_link_type
+  end
+
   private
 
   def can_admin_issue_link_on_current_project?

ee/app/services/issue_links/create_service.rb

@@ -3,7 +3,13 @@
 module IssueLinks
   class CreateService < IssuableLinks::CreateService
     def relate_issuables(referenced_issue)
-      link = IssueLink.create(source: issuable, target: referenced_issue)
+      attrs = { source: issuable, target: referenced_issue }
+
+      if ::Feature.enabled?(:issue_link_types, issuable.project) && params[:link_type].present?
+        attrs[:link_type] = params[:link_type]
+      end
+
+      link = IssueLink.create(attrs)
 
       yield if link.persisted?
     end

ee/changelogs/unreleased/link-types.yml

+---
+title: Added migration for issue link types.
+merge_request: 20617
+author:
+type: added

ee/spec/controllers/projects/issue_links_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::IssueLinksController do
+  let_it_be(:namespace) { create(:group, :public) }
+  let_it_be(:project)   { create(:project, :public, namespace: namespace) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:issue1) { create(:issue, project: project) }
+  let_it_be(:issue2) { create(:issue, project: project) }
+
+  describe 'GET #index' do
+    let_it_be(:issue_link) { create(:issue_link, source: issue1, target: issue2, link_type: 'is_blocked_by') }
+
+    def get_link(user, issue)
+      sign_in(user)
+
+      params = {
+        namespace_id: issue.project.namespace.to_param,
+        project_id: issue.project,
+        issue_id: issue.iid
+      }
+
+      get :index, params: params, as: :json
+    end
+
+    before do
+      stub_licensed_features(related_issues: true)
+      project.add_developer(user)
+    end
+
+    context 'when issue_link_types is enabled' do
+      before do
+        stub_feature_flags(issue_link_types: true)
+      end
+
+      it 'returns success response' do
+        get_link(user, issue1)
+
+        expect(response).to have_gitlab_http_status(200)
+
+        link = json_response.first
+        expect(link['id']).to eq(issue2.id)
+        expect(link['link_type']).to eq('is_blocked_by')
+      end
+    end
+
+    context 'when issue_link_types is disabled' do
+      before do
+        stub_feature_flags(issue_link_types: false)
+      end
+
+      it 'does not return issue_link_type' do
+        get_link(user, issue1)
+
+        expect(response).to have_gitlab_http_status(200)
+
+        link = json_response.first
+        expect(link['id']).to eq(issue2.id)
+        expect(link).not_to include('link_type')
+      end
+    end
+  end
+
+  describe 'POST #create' do
+    def create_link(user, issue, target)
+      sign_in(user)
+
+      post_params = {
+        namespace_id: issue.project.namespace.to_param,
+        project_id: issue.project,
+        issue_id: issue.iid,
+        issuable_references: [target.to_reference],
+        link_type: 'is_blocked_by'
+      }
+
+      post :create, params: post_params, as: :json
+    end
+
+    context 'when related issues are available on the project' do
+      before do
+        project.add_developer(user)
+        stub_licensed_features(related_issues: true)
+        stub_feature_flags(link_types: true)
+      end
+
+      it 'returns success response' do
+        create_link(user, issue1, issue2)
+
+        expect(response).to have_gitlab_http_status(200)
+
+        link = json_response['issuables'].first
+        expect(link['id']).to eq(issue2.id)
+        expect(link['link_type']).to eq('is_blocked_by')
+      end
+    end
+
+    context 'when related issues are not available on the project' do
+      before do
+        stub_licensed_features(related_issues: false)
+      end
+
+      it 'returns 403' do
+        create_link(user, issue1, issue2)
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+    end
+  end
+end

ee/spec/models/issue_link_spec.rb

@@ -8,6 +8,14 @@ describe IssueLink do
     it { is_expected.to belong_to(:target).class_name('Issue') }
   end
 
+  describe 'link_type' do
+    it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1, is_blocked_by: 2) }
+
+    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 }
 
@@ -42,4 +50,12 @@ describe IssueLink do
       end
     end
   end
+
+  describe '.inverse_link_type' do
+    it 'returns reverse type of link' do
+      expect(described_class.inverse_link_type('relates_to')).to eq 'relates_to'
+      expect(described_class.inverse_link_type('blocks')).to eq 'is_blocked_by'
+      expect(described_class.inverse_link_type('is_blocked_by')).to eq 'blocks'
+    end
+  end
 end

ee/spec/models/issue_spec.rb

@@ -161,6 +161,13 @@ describe Issue do
           .to contain_exactly(authorized_issue_b, authorized_issue_c)
     end
 
+    it 'returns issues with valid issue_link_type' do
+      link_types = authorized_issue_a.related_issues(user).map(&:issue_link_type)
+
+      expect(link_types).not_to be_empty
+      expect(link_types).not_to include(nil)
+    end
+
     describe 'when a user cannot read cross project' do
       it 'only returns issues within the same project' do
         expect(Ability).to receive(:allowed?).with(user, :read_all_resources, :global).and_call_original
@@ -510,4 +517,33 @@ describe Issue do
       it { is_expected.to contain_exactly(design_a, design_c) }
     end
   end
+
+  describe "#issue_link_type" do
+    let(:issue) { build(:issue) }
+
+    it 'returns nil for a regular issue' do
+      expect(issue.issue_link_type).to be_nil
+    end
+
+    where(:id, :issue_link_source_id, :issue_link_type_value, :expected) do
+      1 | 1   | 0 | 'relates_to'
+      1 | 1   | 1 | 'blocks'
+      1 | 2   | 3 | 'relates_to'
+      1 | 2   | 1 | 'is_blocked_by'
+      1 | 2   | 2 | 'blocks'
+    end
+
+    with_them do
+      let(:issue) { build(:issue) }
+      subject { issue.issue_link_type }
+
+      before do
+        allow(issue).to receive(:id).and_return(id)
+        allow(issue).to receive(:issue_link_source_id).and_return(issue_link_source_id)
+        allow(issue).to receive(:issue_link_type_value).and_return(issue_link_type_value)
+      end
+
+      it { is_expected.to eq(expected) }
+    end
+  end
 end

ee/spec/serializers/linked_project_issue_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe LinkedProjectIssueEntity do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:issue_link) { create(:issue_link) }
+
+  let(:request) { double('request') }
+  let(:related_issue) { issue_link.source.related_issues(user).first }
+  let(:entity) { described_class.new(related_issue, request: request, current_user: user) }
+
+  before do
+    allow(request).to receive(:current_user).and_return(user)
+    allow(request).to receive(:issuable).and_return(issue_link.source)
+    issue_link.target.project.add_developer(user)
+  end
+
+  describe 'issue_link_type' do
+    context 'when issue_link_types is enabled' do
+      before do
+        stub_feature_flags(issue_link_types: true)
+      end
+
+      it { expect(entity.as_json).to include(link_type: 'relates_to') }
+    end
+
+    context 'when issue_link_types is disabled' do
+      before do
+        stub_feature_flags(issue_link_types: false)
+      end
+
+      it { expect(entity.as_json).not_to include(:link_type) }
+    end
+  end
+end

ee/spec/services/issue_links/create_service_spec.rb

@@ -87,7 +87,7 @@ describe IssueLinks::CreateService do
       let(:another_project_issue_ref) { another_project_issue.to_reference(project) }
 
       let(:params) do
-        { issuable_references: [issue_a_ref, another_project_issue_ref] }
+        { issuable_references: [issue_a_ref, another_project_issue_ref], link_type: 'is_blocked_by' }
       end
 
       before do
@@ -97,8 +97,8 @@ describe IssueLinks::CreateService do
       it 'creates relationships' do
         expect { subject }.to change(IssueLink, :count).from(0).to(2)
 
-        expect(IssueLink.find_by!(target: issue_a)).to have_attributes(source: issue)
-        expect(IssueLink.find_by!(target: another_project_issue)).to have_attributes(source: issue)
+        expect(IssueLink.find_by!(target: issue_a)).to have_attributes(source: issue, link_type: 'is_blocked_by')
+        expect(IssueLink.find_by!(target: another_project_issue)).to have_attributes(source: issue, link_type: 'is_blocked_by')
       end
 
       it 'returns success status' do
@@ -120,6 +120,19 @@ describe IssueLinks::CreateService do
 
         subject
       end
+
+      context 'when issue_link_types is disabled' do
+        before do
+          stub_feature_flags(issue_link_types: false)
+        end
+
+        it 'creates links with default type' do
+          expect { subject }.to change(IssueLink, :count).from(0).to(2)
+
+          expect(IssueLink.find_by!(target: issue_a)).to have_attributes(source: issue, link_type: 'relates_to')
+          expect(IssueLink.find_by!(target: another_project_issue)).to have_attributes(source: issue, link_type: 'relates_to')
+        end
+      end
     end
 
     context 'when reference of any already related issue is present' do