Commit e23943c6 authored by Jan Provaznik's avatar Jan Provaznik Committed by Robert Speicher

Added issue link type

We can now distinguish between blocking, blocked_by and related
issues.
parent 6a83747f
# 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
...@@ -2049,6 +2049,7 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do ...@@ -2049,6 +2049,7 @@ ActiveRecord::Schema.define(version: 2019_12_08_071112) do
t.integer "target_id", null: false t.integer "target_id", null: false
t.datetime "created_at" t.datetime "created_at"
t.datetime "updated_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", "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 ["source_id"], name: "index_issue_links_on_source_id"
t.index ["target_id"], name: "index_issue_links_on_target_id" t.index ["target_id"], name: "index_issue_links_on_target_id"
......
...@@ -28,7 +28,7 @@ module IssuableLinks ...@@ -28,7 +28,7 @@ module IssuableLinks
end end
def create_params def create_params
params.slice(:issuable_references) params.permit(issuable_references: [])
end end
def create_service def create_service
......
...@@ -40,5 +40,9 @@ module Projects ...@@ -40,5 +40,9 @@ module Projects
def link def link
@link ||= IssueLink.find(params[:id]) @link ||= IssueLink.find(params[:id])
end end
def create_params
params.permit(:link_type, issuable_references: [])
end
end end
end end
...@@ -92,7 +92,9 @@ module EE ...@@ -92,7 +92,9 @@ module EE
def related_issues(current_user, preload: nil) def related_issues(current_user, preload: nil)
related_issues = ::Issue related_issues = ::Issue
.select(['issues.*', 'issue_links.id AS 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 .joins("INNER JOIN issue_links ON
(issue_links.source_id = issues.id AND issue_links.target_id = #{id}) (issue_links.source_id = issues.id AND issue_links.target_id = #{id})
OR OR
...@@ -129,6 +131,15 @@ module EE ...@@ -129,6 +131,15 @@ module EE
!!promoted_to_epic_id !!promoted_to_epic_id
end 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 class_methods do
# override # override
def sort_by_attribute(method, excluded_labels: []) def sort_by_attribute(method, excluded_labels: [])
......
...@@ -12,6 +12,23 @@ class IssueLink < ApplicationRecord ...@@ -12,6 +12,23 @@ class IssueLink < ApplicationRecord
scope :for_source_issue, ->(issue) { where(source_id: issue.id) } scope :for_source_issue, ->(issue) { where(source_id: issue.id) }
scope :for_target_issue, ->(issue) { where(target_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 private
def check_self_relation def check_self_relation
......
...@@ -11,6 +11,10 @@ class LinkedProjectIssueEntity < LinkedIssueEntity ...@@ -11,6 +11,10 @@ class LinkedProjectIssueEntity < LinkedIssueEntity
end end
end end
expose :link_type, if: -> (issue, _) { Feature.enabled?(:issue_link_types, issue.project) } do |issue|
issue.issue_link_type
end
private private
def can_admin_issue_link_on_current_project? def can_admin_issue_link_on_current_project?
......
...@@ -3,7 +3,13 @@ ...@@ -3,7 +3,13 @@
module IssueLinks module IssueLinks
class CreateService < IssuableLinks::CreateService class CreateService < IssuableLinks::CreateService
def relate_issuables(referenced_issue) 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? yield if link.persisted?
end end
......
---
title: Added migration for issue link types.
merge_request: 20617
author:
type: added
# 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
...@@ -8,6 +8,14 @@ describe IssueLink do ...@@ -8,6 +8,14 @@ describe IssueLink do
it { is_expected.to belong_to(:target).class_name('Issue') } it { is_expected.to belong_to(:target).class_name('Issue') }
end 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 describe 'Validation' do
subject { create :issue_link } subject { create :issue_link }
...@@ -42,4 +50,12 @@ describe IssueLink do ...@@ -42,4 +50,12 @@ describe IssueLink do
end end
end 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 end
...@@ -161,6 +161,13 @@ describe Issue do ...@@ -161,6 +161,13 @@ describe Issue do
.to contain_exactly(authorized_issue_b, authorized_issue_c) .to contain_exactly(authorized_issue_b, authorized_issue_c)
end 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 describe 'when a user cannot read cross project' do
it 'only returns issues within the same 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 expect(Ability).to receive(:allowed?).with(user, :read_all_resources, :global).and_call_original
...@@ -510,4 +517,33 @@ describe Issue do ...@@ -510,4 +517,33 @@ describe Issue do
it { is_expected.to contain_exactly(design_a, design_c) } it { is_expected.to contain_exactly(design_a, design_c) }
end end
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 end
# 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
...@@ -87,7 +87,7 @@ describe IssueLinks::CreateService do ...@@ -87,7 +87,7 @@ describe IssueLinks::CreateService do
let(:another_project_issue_ref) { another_project_issue.to_reference(project) } let(:another_project_issue_ref) { another_project_issue.to_reference(project) }
let(:params) do 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 end
before do before do
...@@ -97,8 +97,8 @@ describe IssueLinks::CreateService do ...@@ -97,8 +97,8 @@ describe IssueLinks::CreateService do
it 'creates relationships' do it 'creates relationships' do
expect { subject }.to change(IssueLink, :count).from(0).to(2) 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: 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) expect(IssueLink.find_by!(target: another_project_issue)).to have_attributes(source: issue, link_type: 'is_blocked_by')
end end
it 'returns success status' do it 'returns success status' do
...@@ -120,6 +120,19 @@ describe IssueLinks::CreateService do ...@@ -120,6 +120,19 @@ describe IssueLinks::CreateService do
subject subject
end 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 end
context 'when reference of any already related issue is present' do context 'when reference of any already related issue is present' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment