diff --git a/app/services/issuable_links/create_service.rb b/app/services/issuable_links/create_service.rb index 81685f81afaefb00be0223b68d3bc3badbb5f8f8..64a64f50ded70a018e325e3701794a315b33c751 100644 --- a/app/services/issuable_links/create_service.rb +++ b/app/services/issuable_links/create_service.rb @@ -36,6 +36,20 @@ def execute success end + # rubocop: disable CodeReuse/ActiveRecord + def relate_issuables(referenced_issuable) + link = link_class.find_or_initialize_by(source: issuable, target: referenced_issuable) + + set_link_type(link) + + if link.changed? && link.save + create_notes(referenced_issuable) + end + + link + end + # rubocop: enable CodeReuse/ActiveRecord + private def render_conflict_error? @@ -96,6 +110,23 @@ def extractor_context {} end + def issuables_assigned_message + _('%{issuable}(s) already assigned' % { issuable: target_issuable_type.capitalize }) + end + + def issuables_not_found_message + _('No matching %{issuable} found. Make sure that you are adding a valid %{issuable} URL.' % { issuable: target_issuable_type }) + end + + def target_issuable_type + :issue + end + + def create_notes(referenced_issuable) + SystemNoteService.relate_issuable(issuable, referenced_issuable, current_user) + SystemNoteService.relate_issuable(referenced_issuable, issuable, current_user) + end + def linkable_issuables(objects) raise NotImplementedError end @@ -104,16 +135,12 @@ def previous_related_issuables raise NotImplementedError end - def relate_issuables(referenced_object) + def link_class raise NotImplementedError end - def issuables_assigned_message - _("Issue(s) already assigned") - end - - def issuables_not_found_message - _("No matching issue found. Make sure that you are adding a valid issue URL.") + def set_link_type(_link) + # no-op end end end diff --git a/app/services/issue_links/create_service.rb b/app/services/issue_links/create_service.rb index a022d3e0bcfa4daf3fd03bf8d5ae548ad08a821b..1c6621ce0a1937ab22870e0bd78c7413520e4b46 100644 --- a/app/services/issue_links/create_service.rb +++ b/app/services/issue_links/create_service.rb @@ -2,44 +2,25 @@ module IssueLinks class CreateService < IssuableLinks::CreateService - # rubocop: disable CodeReuse/ActiveRecord - def relate_issuables(referenced_issue) - link = IssueLink.find_or_initialize_by(source: issuable, target: referenced_issue) - - set_link_type(link) - - if link.changed? && link.save - create_notes(referenced_issue) - end - - link - end - # rubocop: enable CodeReuse/ActiveRecord - def linkable_issuables(issues) @linkable_issuables ||= begin issues.select { |issue| can?(current_user, :admin_issue_link, issue) } end end - def create_notes(referenced_issue) - SystemNoteService.relate_issue(issuable, referenced_issue, current_user) - SystemNoteService.relate_issue(referenced_issue, issuable, current_user) - end - def previous_related_issuables @related_issues ||= issuable.related_issues(current_user).to_a end private - def set_link_type(_link) - # EE only - end - def track_event track_incident_action(current_user, issuable, :incident_relate) end + + def link_class + IssueLink + end end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 8d77f03c0d9549f7f5e1f8eb3ce6ba718d662796..9db39a5e17414a26f38e41116c1f0eb8cefc7d66 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -49,8 +49,8 @@ def change_issuable_contacts(issuable, project, author, added_count, removed_cou ::SystemNotes::IssuablesService.new(noteable: issuable, project: project, author: author).change_issuable_contacts(added_count, removed_count) end - def relate_issue(noteable, noteable_ref, user) - ::SystemNotes::IssuablesService.new(noteable: noteable, project: noteable.project, author: user).relate_issue(noteable_ref) + def relate_issuable(noteable, noteable_ref, user) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: noteable.project, author: user).relate_issuable(noteable_ref) end def unrelate_issuable(noteable, noteable_ref, user) diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index ce592f8d4bbd32bc59b45c7942a3070e29d0eb8a..89212288a6bab9091070e4aed9fc177f4e396a6a 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -10,8 +10,9 @@ class IssuablesService < ::SystemNotes::BaseService # "marked this issue as related to gitlab-foss#9001" # # Returns the created Note object - def relate_issue(noteable_ref) - body = "marked this issue as related to #{noteable_ref.to_reference(noteable.project)}" + def relate_issuable(noteable_ref) + issuable_type = noteable.to_ability_name.humanize(capitalize: false) + body = "marked this #{issuable_type} as related to #{noteable_ref.to_reference(noteable.resource_parent)}" issue_activity_counter.track_issue_related_action(author: author) if noteable.is_a?(Issue) diff --git a/ee/app/controllers/groups/epics/related_epic_links_controller.rb b/ee/app/controllers/groups/epics/related_epic_links_controller.rb index 0bc58cd75db24ed1a0e879c9777f18e9cf93dc47..4af0e51ec6e0320385e06463e77ef88ab4e5fbe8 100644 --- a/ee/app/controllers/groups/epics/related_epic_links_controller.rb +++ b/ee/app/controllers/groups/epics/related_epic_links_controller.rb @@ -10,7 +10,7 @@ class Groups::Epics::RelatedEpicLinksController < Groups::ApplicationController before_action :check_epics_available! before_action :check_related_epics_available! before_action :authorize_related_epic_link_association!, only: [:destroy] - before_action :authorize_admin!, only: [:destroy] + before_action :authorize_admin!, only: [:create, :destroy] feature_category :portfolio_management urgency :default @@ -37,7 +37,15 @@ def destroy_service Epics::RelatedEpicLinks::DestroyService.new(link, current_user) end + def create_service + Epics::RelatedEpicLinks::CreateService.new(epic, current_user, create_params) + end + def ensure_related_epics_enabled! render_404 unless Feature.enabled?(:related_epics_widget, epic&.group, default_enabled: :yaml) end + + def create_params + params.permit(:link_type, issuable_references: []) + end end diff --git a/ee/app/serializers/epics/related_epic_entity.rb b/ee/app/serializers/epics/related_epic_entity.rb index 05a8bd1d4b5a2502f9d2a8fed2ce5fd99dadbe3e..e4a0bbc12979eb004c9e9d21f85ce7a33c857a6e 100644 --- a/ee/app/serializers/epics/related_epic_entity.rb +++ b/ee/app/serializers/epics/related_epic_entity.rb @@ -2,10 +2,17 @@ module Epics class RelatedEpicEntity < Grape::Entity + include Gitlab::Utils::StrongMemoize include RequestAwareEntity expose :id, :confidential, :title, :state, :created_at, :closed_at + expose :relation_path do |related_epic| + if can_admin_related_epic_links?(related_epic) + group_epic_related_epic_link_path(issuable.group, issuable.iid, related_epic.related_epic_link_id) + end + end + expose :reference do |related_epic| related_epic.to_reference(request.issuable.group) end @@ -17,5 +24,17 @@ class RelatedEpicEntity < Grape::Entity expose :link_type do |related_epic| related_epic.epic_link_type end + + private + + def can_admin_related_epic_links?(epic) + user = request.current_user + + Ability.allowed?(user, :admin_related_epic_link, issuable) && Ability.allowed?(user, :admin_epic, epic) + end + + def issuable + request.issuable + end end end diff --git a/ee/app/services/ee/issuable_links/create_service.rb b/ee/app/services/ee/issuable_links/create_service.rb index 9dba3a8aaa9548644f24946fd44b25758de42273..fb366389dd6c0438270217c55b507ce1b5a2e239 100644 --- a/ee/app/services/ee/issuable_links/create_service.rb +++ b/ee/app/services/ee/issuable_links/create_service.rb @@ -24,6 +24,19 @@ def link_issuables(objects) def affected_epics(_issues) [] end + + override :set_link_type + def set_link_type(link) + return unless params[:link_type].present? + + # `blocked_by` links are treated as `blocks` links where source and target is swapped. + if params[:link_type] == ::IssuableLink::TYPE_IS_BLOCKED_BY + link.source, link.target = link.target, link.source + link.link_type = ::IssuableLink::TYPE_BLOCKS + else + link.link_type = params[:link_type] + end + end end end end diff --git a/ee/app/services/ee/issue_links/create_service.rb b/ee/app/services/ee/issue_links/create_service.rb index 1f146b01e43a1ba59df495a8185427a1f3f99af1..2038651355c13a3b37b6ee9460f7e5b2118cc4ab 100644 --- a/ee/app/services/ee/issue_links/create_service.rb +++ b/ee/app/services/ee/issue_links/create_service.rb @@ -13,18 +13,6 @@ def execute private - def set_link_type(link) - return unless params[:link_type].present? - - # `blocked_by` links are treated as `blocks` links where source and target is swapped. - if params[:link_type] == ::IssueLink::TYPE_IS_BLOCKED_BY - link.source, link.target = link.target, link.source - link.link_type = ::IssueLink::TYPE_BLOCKS - else - link.link_type = params[:link_type] - end - end - def link_type_available? # `blocked_by` is allowed as a param and handled in set_link_type return true unless [::IssueLink::TYPE_BLOCKS, ::IssueLink::TYPE_IS_BLOCKED_BY].include?(params[:link_type]) diff --git a/ee/app/services/epics/related_epic_links/create_service.rb b/ee/app/services/epics/related_epic_links/create_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..bf4ca1a890673b7c31f163d45d94b0644f961e6b --- /dev/null +++ b/ee/app/services/epics/related_epic_links/create_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Epics::RelatedEpicLinks + class CreateService < IssuableLinks::CreateService + def linkable_issuables(epics) + @linkable_issuables ||= begin + epics.select { |epic| can?(current_user, :admin_epic, epic) } + end + end + + def previous_related_issuables + @related_epics ||= issuable.related_epics(current_user).to_a + end + + private + + def references(extractor) + extractor.epics + end + + def extractor_context + { group: issuable.group } + end + + def target_issuable_type + :epic + end + + def link_class + Epic::RelatedEpicLink + end + end +end diff --git a/ee/app/services/epics/related_epic_links/destroy_service.rb b/ee/app/services/epics/related_epic_links/destroy_service.rb index ed7db8b253529bcfbbda670425795995a828eb4c..38f034d05e61367940d04939c00928dfbbcad4b7 100644 --- a/ee/app/services/epics/related_epic_links/destroy_service.rb +++ b/ee/app/services/epics/related_epic_links/destroy_service.rb @@ -6,7 +6,7 @@ class DestroyService < ::IssuableLinks::DestroyService private def permission_to_remove_relation? - can?(current_user, :admin_related_epic_link, source) && can?(current_user, :admin_related_epic_link, target) + can?(current_user, :admin_related_epic_link, source) && can?(current_user, :admin_epic, target) end def track_event diff --git a/ee/config/routes/group.rb b/ee/config/routes/group.rb index 033916ead93536ca5f7aac03fafda945ccd3b791..96249cc6c71f9ac4fbaae31e5a6e3055f15bf97e 100644 --- a/ee/config/routes/group.rb +++ b/ee/config/routes/group.rb @@ -123,7 +123,7 @@ scope module: :epics do resources :notes, only: [:index, :create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } - resources :related_epic_links, only: [:index, :destroy] + resources :related_epic_links, only: [:index, :create, :destroy] end collection do diff --git a/ee/spec/controllers/projects/issues_controller_spec.rb b/ee/spec/controllers/projects/issues_controller_spec.rb index 9edb064d5d7c8f0e733b389db398e9e5ce35cd2b..d75f20b39b07285a44a1c407f116b45ed4a1220f 100644 --- a/ee/spec/controllers/projects/issues_controller_spec.rb +++ b/ee/spec/controllers/projects/issues_controller_spec.rb @@ -249,7 +249,7 @@ def send_request context 'with a related system note' do let(:confidential_issue) { create(:issue, :confidential, project: project) } - let!(:system_note) { SystemNoteService.relate_issue(issue, confidential_issue, user) } + let!(:system_note) { SystemNoteService.relate_issuable(issue, confidential_issue, user) } shared_examples 'user can see confidential issue' do |access_level| context "when a user is a #{access_level}" do diff --git a/ee/spec/fixtures/api/schemas/entities/related_epic.json b/ee/spec/fixtures/api/schemas/entities/related_epic.json index 5effbc2b97578f51d4cad307391f5b8341b7f520..42312c5c00be733362ee169104258f2b69b2095c 100644 --- a/ee/spec/fixtures/api/schemas/entities/related_epic.json +++ b/ee/spec/fixtures/api/schemas/entities/related_epic.json @@ -1,30 +1,28 @@ { "type": "object", - "allOf": [ - { - "required" : [ - "id", - "confidential", - "title", - "state", - "created_at", - "closed_at", - "reference", - "path", - "link_type" - ], - "properties" : { - "id": { "type": "integer" }, - "confidential": { "type": "boolean" }, - "title": { "type": "string" }, - "state": { "type": "string" }, - "created_at": { "type": "string", "format": "date-time" }, - "closed_at": { "type": ["string", "null"], "format": "date-time" }, - "reference": { "type": "string" }, - "path": { "type": "string" }, - "link_type": { "type": "string" } - }, - "additionalProperties": false - } - ] + "properties" : { + "id": { "type": "integer" }, + "confidential": { "type": "boolean" }, + "title": { "type": "string" }, + "state": { "type": "string" }, + "created_at": { "type": "string", "format": "date-time" }, + "closed_at": { "type": ["string", "null"], "format": "date-time" }, + "relation_path": { "type": ["string", "null"] }, + "reference": { "type": "string" }, + "path": { "type": "string" }, + "link_type": { "type": "string" } + }, + "required" : [ + "id", + "confidential", + "title", + "state", + "created_at", + "closed_at", + "relation_path", + "reference", + "path", + "link_type" + ], + "additionalProperties": false } diff --git a/ee/spec/requests/groups/epics/related_epic_links_controller_spec.rb b/ee/spec/requests/groups/epics/related_epic_links_controller_spec.rb index 9ea095f0a783e2b15197b2039967a2ec0bf0d334..a216226c5cefe7c16f2ea925e8fa69712bd44fb6 100644 --- a/ee/spec/requests/groups/epics/related_epic_links_controller_spec.rb +++ b/ee/spec/requests/groups/epics/related_epic_links_controller_spec.rb @@ -6,8 +6,10 @@ let_it_be(:user) { create(:user) } let_it_be(:epic) { create(:epic) } let_it_be(:epic2) { create(:epic, group: epic.group) } - let_it_be(:epic_link1) { create(:related_epic_link, source: epic, target: epic2) } + let_it_be(:epic3) { create(:epic, group: epic.group) } + let_it_be(:epic_link1) { create(:related_epic_link, source: epic, target: epic3) } let_it_be(:epic_link2) { create(:related_epic_link, source: epic) } + let_it_be(:listing_service) { Epics::RelatedEpicLinks::ListService } before do stub_licensed_features(epics: true, related_epics: true) @@ -113,4 +115,84 @@ def do_request end end end + + describe 'POST /groups/*group_id/-/epics/:epic_id/related_epic_links' do + let(:issuable_references) { [epic2.to_reference(full: true)] } + + subject(:request) do + post group_epic_related_epic_links_path(related_epics_params(issuable_references: issuable_references)) + end + + before do + epic.group.add_developer(user) + epic2.group.add_developer(user) + login_as user + end + + context 'with success' do + it 'returns JSON response' do + request + + list_service_response = listing_service.new(epic, user).execute + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq('message' => nil, + 'issuables' => list_service_response.as_json) + end + + it 'delegates the creation of the related epic link to Epics::RelatedEpicLinks::CreateService' do + expect_next_instance_of(Epics::RelatedEpicLinks::CreateService) do |service| + expect(service).to receive(:execute).once.and_call_original + end + + request + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'creates a new Epic::RelatedEpicLink record' do + expect { request }.to change { Epic::RelatedEpicLink.count }.by(1) + end + + it 'returns correct relation path in response' do + request + related_epic_link = Epic::RelatedEpicLink.find_by(source: epic, target: epic2) + + expect(json_response['issuables'].last) + .to include('relation_path' => "/groups/#{epic.group.path}/-/epics/#{epic.iid}/related_epic_links/#{related_epic_link&.id}") + end + end + + context 'with failure' do + context 'when unauthorized' do + it 'returns 403' do + epic.group.add_guest(user) + + request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when failing service result' do + let(:issuable_references) { ["##{non_existing_record_iid}"] } + + it 'returns failure JSON' do + request + + list_service_response = listing_service.new(epic, user).execute + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response).to eq('message' => 'No matching epic found. Make sure that you are adding a valid epic URL.', 'issuables' => list_service_response.as_json) + end + end + + it_behaves_like 'a not available action' + end + end + + def related_epics_params(opts = {}) + opts.reverse_merge(group_id: epic.group, + epic_id: epic.iid, + format: :json) + end end diff --git a/ee/spec/serializers/epics/related_epic_entity_spec.rb b/ee/spec/serializers/epics/related_epic_entity_spec.rb index 783d826635737e465568b58137df34bec3bb96be..c05318da6a2c25c768811261c9781acde5a530f5 100644 --- a/ee/spec/serializers/epics/related_epic_entity_spec.rb +++ b/ee/spec/serializers/epics/related_epic_entity_spec.rb @@ -5,8 +5,9 @@ RSpec.describe Epics::RelatedEpicEntity do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } + let_it_be(:group_2) { create(:group) } let_it_be(:source) { create(:epic, group: group) } - let_it_be(:target) { create(:epic, group: group) } + let_it_be(:target) { create(:epic, group: group_2) } let_it_be(:epic_link) { create(:related_epic_link, source: source, target: target) } let(:request) { EntityRequest.new(issuable: epic_link.source, current_user: user) } @@ -18,9 +19,35 @@ group.add_developer(user) end - describe '#as_json' do - it 'matches json schema' do - expect(entity.to_json).to match_schema('entities/related_epic', dir: 'ee') + describe '#to_json' do + context 'when user can admin_epic on target epic group' do + before do + group_2.add_reporter(user) + end + + it 'matches json schema' do + expect(entity.to_json).to match_schema('entities/related_epic', dir: 'ee') + end + + it 'returns relation_path' do + path = Gitlab::Routing.url_helpers.group_epic_related_epic_link_path(source.group, source.iid, epic_link.id) + + expect(entity.as_json[:relation_path]).to eq(path) + end + end + + context 'when user cannot admin_epic on target epic group' do + before do + group_2.add_guest(user) + end + + it 'matches json schema' do + expect(entity.to_json).to match_schema('entities/related_epic', dir: 'ee') + end + + it 'returns null relation_path' do + expect(entity.as_json[:relation_path]).to eq(nil) + end end end end diff --git a/ee/spec/services/ee/issue_links/create_service_spec.rb b/ee/spec/services/ee/issue_links/create_service_spec.rb index ef7af2a958260db27abadd16c713708ed3c9cf9c..2be9f8883812c3ac29d00e6660ee85f9416b5d45 100644 --- a/ee/spec/services/ee/issue_links/create_service_spec.rb +++ b/ee/spec/services/ee/issue_links/create_service_spec.rb @@ -50,27 +50,15 @@ end end - 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, link_type: 'blocks') - expect(IssueLink.find_by!(target: another_project_issue)).to have_attributes(source: issue, link_type: 'blocks') - end - it 'returns success status' do is_expected.to eq(status: :success) end - context 'when blocked_by relation is used' do - let(:params) do - { issuable_references: [issue_a_ref], link_type: 'is_blocked_by' } - end - - it 'creates creates `blocks` relation with swapped source and target' do - expect { subject }.to change(IssueLink, :count).from(0).to(1) - - expect(IssueLink.find_by!(source: issue_a)).to have_attributes(target: issue, link_type: 'blocks') - end + it_behaves_like 'issuable link creation with blocking link_type' do + let(:issuable_link_class) { IssueLink } + let(:issuable) { issue } + let(:issuable2) { issue_a } + let(:issuable3) { another_project_issue } end end diff --git a/ee/spec/services/ee/system_notes/issuables_service_spec.rb b/ee/spec/services/ee/system_notes/issuables_service_spec.rb index 28047a3e520ddcd0a1ee2696aa48b8fcec17d0de..de1826637cde41b3a593a50b17cee1c1777c6aea 100644 --- a/ee/spec/services/ee/system_notes/issuables_service_spec.rb +++ b/ee/spec/services/ee/system_notes/issuables_service_spec.rb @@ -87,6 +87,17 @@ subject end end + + describe '#relate_issuable' do + let(:noteable) { epic } + let(:target) { create(:epic) } + + it 'creates system notes when relating epics' do + result = service.relate_issuable(target) + + expect(result.note).to eq("marked this epic as related to #{target.to_reference(target.group, full: true)}") + end + end end describe '#unrelate_issuable' do diff --git a/ee/spec/services/epics/related_epic_links/create_service_spec.rb b/ee/spec/services/epics/related_epic_links/create_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ebded0d95bbd9dc8a500cb51a69edeee3ea46e93 --- /dev/null +++ b/ee/spec/services/epics/related_epic_links/create_service_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Epics::RelatedEpicLinks::CreateService do + describe '#execute' do + let_it_be(:user) { create :user } + let_it_be(:group) { create :group } + let_it_be(:issuable) { create :epic, group: group } + let_it_be(:issuable2) { create :epic, group: group } + let_it_be(:guest_issuable) { create :epic } + let_it_be(:another_group) { create :group } + let_it_be(:issuable3) { create :epic, group: another_group } + let_it_be(:issuable_a) { create :epic, group: group } + let_it_be(:issuable_b) { create :epic, group: group } + let_it_be(:issuable_link) { create :related_epic_link, source: issuable, target: issuable_b, link_type: IssuableLink::TYPE_RELATES_TO } + + let(:issuable_parent) { issuable.group } + let(:issuable_type) { :epic } + let(:issuable_link_class) { Epic::RelatedEpicLink } + let(:params) { {} } + + before do + stub_licensed_features(epics: true, related_epics: true) + group.add_developer(user) + guest_issuable.group.add_guest(user) + another_group.add_developer(user) + end + + it_behaves_like 'issuable link creation' + it_behaves_like 'issuable link creation with blocking link_type' do + let(:params) do + { issuable_references: [issuable2.to_reference, issuable3.to_reference(issuable3.group, full: true)] } + end + end + + context 'when related_epics is not available for target epic' do + let(:params) do + { issuable_references: [issuable3.to_reference(issuable3.group, full: true)] } + end + + subject { described_class.new(issuable, user, params).execute } + + before do + stub_licensed_features(epics: true, related_epics: false) + allow(issuable.group).to receive(:licensed_feature_available?).with(:related_epics).and_return(true) + end + + it 'creates relationships' do + expect { subject }.to change(issuable_link_class, :count).by(1) + + expect(issuable_link_class.find_by!(target: issuable3)).to have_attributes(source: issuable, link_type: 'relates_to') + end + end + end +end diff --git a/ee/spec/support/shared_examples/services/issuable_links/create_links_with_link_type.rb b/ee/spec/support/shared_examples/services/issuable_links/create_links_with_link_type.rb new file mode 100644 index 0000000000000000000000000000000000000000..e3445836d6c68794a1973533f98f634c9d0fd5f3 --- /dev/null +++ b/ee/spec/support/shared_examples/services/issuable_links/create_links_with_link_type.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +shared_examples 'issuable link creation with blocking link_type' do + subject { described_class.new(issuable, user, params).execute } + + context 'when is_blocked_by relation is used' do + before do + params[:link_type] = 'is_blocked_by' + end + + it 'creates `blocks` relation with swapped source and target' do + expect { subject }.to change(issuable_link_class, :count).by(2) + + expect(issuable_link_class.find_by!(source: issuable2)).to have_attributes(target: issuable, link_type: 'blocks') + expect(issuable_link_class.find_by!(source: issuable3)).to have_attributes(target: issuable, link_type: 'blocks') + end + end + + context 'when blocks relation is used' do + before do + params[:link_type] = 'blocks' + end + + it 'creates `blocks` relation' do + expect { subject }.to change(issuable_link_class, :count).by(2) + + expect(issuable_link_class.find_by!(target: issuable2)).to have_attributes(source: issuable, link_type: 'blocks') + expect(issuable_link_class.find_by!(target: issuable3)).to have_attributes(source: issuable, link_type: 'blocks') + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 093e8c9ea0e6b77bb8e3ef72756d375786062cca..e7743b8c6b5426cca46edca6d7a8b671b22272e1 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -691,6 +691,9 @@ msgstr "" msgid "%{issuableType} will be removed! Are you sure?" msgstr "" +msgid "%{issuable}(s) already assigned" +msgstr "" + msgid "%{issueType} actions" msgstr "" @@ -20495,9 +20498,6 @@ msgstr "" msgid "Issue weight" msgstr "" -msgid "Issue(s) already assigned" -msgstr "" - msgid "IssueAnalytics|Age" msgstr "" @@ -24859,7 +24859,7 @@ msgstr "" msgid "No matches found" msgstr "" -msgid "No matching issue found. Make sure that you are adding a valid issue URL." +msgid "No matching %{issuable} found. Make sure that you are adding a valid %{issuable} URL." msgstr "" msgid "No matching labels" diff --git a/spec/services/issue_links/create_service_spec.rb b/spec/services/issue_links/create_service_spec.rb index 1bca717acb723acc01d1d735a6b48aa121cff21a..9cb5980716a231de7829dba620acc31d6bc3e8b5 100644 --- a/spec/services/issue_links/create_service_spec.rb +++ b/spec/services/issue_links/create_service_spec.rb @@ -4,180 +4,42 @@ RSpec.describe IssueLinks::CreateService do describe '#execute' do - let(:namespace) { create :namespace } - let(:project) { create :project, namespace: namespace } - let(:issue) { create :issue, project: project } - let(:user) { create :user } - let(:params) do - {} - end + let_it_be(:user) { create :user } + let_it_be(:namespace) { create :namespace } + let_it_be(:project) { create :project, namespace: namespace } + let_it_be(:issuable) { create :issue, project: project } + let_it_be(:issuable2) { create :issue, project: project } + let_it_be(:guest_issuable) { create :issue } + let_it_be(:another_project) { create :project, namespace: project.namespace } + let_it_be(:issuable3) { create :issue, project: another_project } + let_it_be(:issuable_a) { create :issue, project: project } + let_it_be(:issuable_b) { create :issue, project: project } + let_it_be(:issuable_link) { create :issue_link, source: issuable, target: issuable_b, link_type: IssueLink::TYPE_RELATES_TO } + + let(:issuable_parent) { issuable.project } + let(:issuable_type) { :issue } + let(:issuable_link_class) { IssueLink } + let(:params) { {} } before do project.add_developer(user) + guest_issuable.project.add_guest(user) + another_project.add_developer(user) end - subject { described_class.new(issue, user, params).execute } + it_behaves_like 'issuable link creation' - context 'when the reference list is empty' do - let(:params) do - { issuable_references: [] } - end + context 'when target is an incident' do + let_it_be(:issue) { create(:incident, project: project) } - it 'returns error' do - is_expected.to eq(message: 'No matching issue found. Make sure that you are adding a valid issue URL.', status: :error, http_status: 404) - end - end - - context 'when Issue not found' do let(:params) do - { issuable_references: ["##{non_existing_record_iid}"] } - end - - it 'returns error' do - is_expected.to eq(message: 'No matching issue found. Make sure that you are adding a valid issue URL.', status: :error, http_status: 404) + { issuable_references: [issuable2.to_reference, issuable3.to_reference(another_project)] } end - it 'no relationship is created' do - expect { subject }.not_to change(IssueLink, :count) - end - end - - context 'when user has no permission to target project Issue' do - let(:target_issuable) { create :issue } - - let(:params) do - { issuable_references: [target_issuable.to_reference(project)] } - end - - it 'returns error' do - target_issuable.project.add_guest(user) - - is_expected.to eq(message: 'No matching issue found. Make sure that you are adding a valid issue URL.', status: :error, http_status: 404) - end - - it 'no relationship is created' do - expect { subject }.not_to change(IssueLink, :count) - end - end - - context 'source and target are the same issue' do - let(:params) do - { issuable_references: [issue.to_reference] } - end - - it 'does not create notes' do - expect(SystemNoteService).not_to receive(:relate_issue) - - subject - end - - it 'no relationship is created' do - expect { subject }.not_to change(IssueLink, :count) - end - end - - context 'when there is an issue to relate' do - let(:issue_a) { create :issue, project: project } - let(:another_project) { create :project, namespace: project.namespace } - let(:another_project_issue) { create :issue, project: another_project } - - let(:issue_a_ref) { issue_a.to_reference } - let(:another_project_issue_ref) { another_project_issue.to_reference(project) } - - let(:params) do - { issuable_references: [issue_a_ref, another_project_issue_ref] } - end - - before do - another_project.add_developer(user) - end - - 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, link_type: 'relates_to') - expect(IssueLink.find_by!(target: another_project_issue)).to have_attributes(source: issue, link_type: 'relates_to') - end - - it 'returns success status' do - is_expected.to eq(status: :success) - end - - it 'creates notes' do - # First two-way relation notes - expect(SystemNoteService).to receive(:relate_issue) - .with(issue, issue_a, user) - expect(SystemNoteService).to receive(:relate_issue) - .with(issue_a, issue, user) - - # Second two-way relation notes - expect(SystemNoteService).to receive(:relate_issue) - .with(issue, another_project_issue, user) - expect(SystemNoteService).to receive(:relate_issue) - .with(another_project_issue, issue, user) - - subject - end - - context 'issue is an incident' do - let(:issue) { create(:incident, project: project) } - - it_behaves_like 'an incident management tracked event', :incident_management_incident_relate do - let(:current_user) { user } - end - end - end - - context 'when reference of any already related issue is present' do - let(:issue_a) { create :issue, project: project } - let(:issue_b) { create :issue, project: project } - let(:issue_c) { create :issue, project: project } - - before do - create :issue_link, source: issue, target: issue_b, link_type: IssueLink::TYPE_RELATES_TO - create :issue_link, source: issue, target: issue_c, link_type: IssueLink::TYPE_RELATES_TO - end - - let(:params) do - { - issuable_references: [ - issue_a.to_reference, - issue_b.to_reference, - issue_c.to_reference - ], - link_type: IssueLink::TYPE_RELATES_TO - } - end - - it 'creates notes only for new relations' do - expect(SystemNoteService).to receive(:relate_issue).with(issue, issue_a, anything) - expect(SystemNoteService).to receive(:relate_issue).with(issue_a, issue, anything) - expect(SystemNoteService).not_to receive(:relate_issue).with(issue, issue_b, anything) - expect(SystemNoteService).not_to receive(:relate_issue).with(issue_b, issue, anything) - expect(SystemNoteService).not_to receive(:relate_issue).with(issue, issue_c, anything) - expect(SystemNoteService).not_to receive(:relate_issue).with(issue_c, issue, anything) - - subject - end - end - - context 'when there are invalid references' do - let(:issue_a) { create :issue, project: project } - - let(:params) do - { issuable_references: [issue.to_reference, issue_a.to_reference] } - end - - it 'creates links only for valid references' do - expect { subject }.to change { IssueLink.count }.by(1) - end + subject { described_class.new(issue, user, params).execute } - it 'returns error status' do - expect(subject).to eq( - status: :error, - http_status: 422, - message: "#{issue.to_reference} cannot be added: cannot be related to itself" - ) + it_behaves_like 'an incident management tracked event', :incident_management_incident_relate do + let(:current_user) { user } end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index e0528aace5adbb5c07b3e753f2153e57c1128daf..c322ec35e867d7e5e3a653b0ccf762244217d812 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -100,7 +100,7 @@ end end - describe '.relate_issue' do + describe '.relate_issuable' do let(:noteable_ref) { double } let(:noteable) { double } @@ -110,10 +110,10 @@ it 'calls IssuableService' do expect_next_instance_of(::SystemNotes::IssuablesService) do |service| - expect(service).to receive(:relate_issue).with(noteable_ref) + expect(service).to receive(:relate_issuable).with(noteable_ref) end - described_class.relate_issue(noteable, noteable_ref, double) + described_class.relate_issuable(noteable, noteable_ref, double) end end diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb index 767f5d10c4bf6261d2f48d27f8151f956f05088c..5bc7ea82976d592afd421f88bb6e1e8186f26ffc 100644 --- a/spec/services/system_notes/issuables_service_spec.rb +++ b/spec/services/system_notes/issuables_service_spec.rb @@ -14,10 +14,10 @@ let(:service) { described_class.new(noteable: noteable, project: project, author: author) } - describe '#relate_issue' do + describe '#relate_issuable' do let(:noteable_ref) { create(:issue) } - subject { service.relate_issue(noteable_ref) } + subject { service.relate_issuable(noteable_ref) } it_behaves_like 'a system note' do let(:action) { 'relate' } diff --git a/spec/support/shared_examples/services/issuable_links/create_links_shared_examples.rb b/spec/support/shared_examples/services/issuable_links/create_links_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..6146aae6b9ba09bfe4085025b5feb85a949a61f5 --- /dev/null +++ b/spec/support/shared_examples/services/issuable_links/create_links_shared_examples.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +shared_examples 'issuable link creation' do + describe '#execute' do + subject { described_class.new(issuable, user, params).execute } + + context 'when the reference list is empty' do + let(:params) do + { issuable_references: [] } + end + + it 'returns error' do + is_expected.to eq(message: "No matching #{issuable_type} found. Make sure that you are adding a valid #{issuable_type} URL.", status: :error, http_status: 404) + end + end + + context 'when Issuable not found' do + let(:params) do + { issuable_references: ["##{non_existing_record_iid}"] } + end + + it 'returns error' do + is_expected.to eq(message: "No matching #{issuable_type} found. Make sure that you are adding a valid #{issuable_type} URL.", status: :error, http_status: 404) + end + + it 'no relationship is created' do + expect { subject }.not_to change(issuable_link_class, :count) + end + end + + context 'when user has no permission to target issuable' do + let(:params) do + { issuable_references: [guest_issuable.to_reference(issuable_parent)] } + end + + it 'returns error' do + is_expected.to eq(message: "No matching #{issuable_type} found. Make sure that you are adding a valid #{issuable_type} URL.", status: :error, http_status: 404) + end + + it 'no relationship is created' do + expect { subject }.not_to change(issuable_link_class, :count) + end + end + + context 'source and target are the same issuable' do + let(:params) do + { issuable_references: [issuable.to_reference] } + end + + it 'does not create notes' do + expect(SystemNoteService).not_to receive(:relate_issuable) + + subject + end + + it 'no relationship is created' do + expect { subject }.not_to change(issuable_link_class, :count) + end + end + + context 'when there is an issuable to relate' do + let(:params) do + { issuable_references: [issuable2.to_reference, issuable3.to_reference(issuable_parent)] } + end + + it 'creates relationships' do + expect { subject }.to change(issuable_link_class, :count).by(2) + + expect(issuable_link_class.find_by!(target: issuable2)).to have_attributes(source: issuable, link_type: 'relates_to') + expect(issuable_link_class.find_by!(target: issuable3)).to have_attributes(source: issuable, link_type: 'relates_to') + end + + it 'returns success status' do + is_expected.to eq(status: :success) + end + + it 'creates notes' do + # First two-way relation notes + expect(SystemNoteService).to receive(:relate_issuable) + .with(issuable, issuable2, user) + expect(SystemNoteService).to receive(:relate_issuable) + .with(issuable2, issuable, user) + + # Second two-way relation notes + expect(SystemNoteService).to receive(:relate_issuable) + .with(issuable, issuable3, user) + expect(SystemNoteService).to receive(:relate_issuable) + .with(issuable3, issuable, user) + + subject + end + end + + context 'when reference of any already related issue is present' do + let(:params) do + { + issuable_references: [ + issuable_a.to_reference, + issuable_b.to_reference + ], + link_type: IssueLink::TYPE_RELATES_TO + } + end + + it 'creates notes only for new relations' do + expect(SystemNoteService).to receive(:relate_issuable).with(issuable, issuable_a, anything) + expect(SystemNoteService).to receive(:relate_issuable).with(issuable_a, issuable, anything) + expect(SystemNoteService).not_to receive(:relate_issuable).with(issuable, issuable_b, anything) + expect(SystemNoteService).not_to receive(:relate_issuable).with(issuable_b, issuable, anything) + + subject + end + end + + context 'when there are invalid references' do + let(:params) do + { issuable_references: [issuable.to_reference, issuable_a.to_reference] } + end + + it 'creates links only for valid references' do + expect { subject }.to change { issuable_link_class.count }.by(1) + end + + it 'returns error status' do + expect(subject).to eq( + status: :error, + http_status: 422, + message: "#{issuable.to_reference} cannot be added: cannot be related to itself" + ) + end + end + end +end