diff --git a/app/services/work_items/parent_links/destroy_service.rb b/app/services/work_items/parent_links/destroy_service.rb index 78705823162850e1a67c34e5716ae4d1c2318f15..3059450d67179eb74a0c70e21b3e5e941ba2a507 100644 --- a/app/services/work_items/parent_links/destroy_service.rb +++ b/app/services/work_items/parent_links/destroy_service.rb @@ -5,9 +5,10 @@ module ParentLinks class DestroyService < IssuableLinks::DestroyService attr_reader :link, :current_user, :parent, :child - def initialize(link, user) + def initialize(link, user, params = {}) @link = link @current_user = user + @params = params @parent = link.work_item_parent @child = link.work_item end diff --git a/ee/app/policies/ee/work_item_policy.rb b/ee/app/policies/ee/work_item_policy.rb index f2ded0d8fb15dcdf14989ff6fca948c744743015..397e88eaaf2117859eb362a4c2526ace4e359057 100644 --- a/ee/app/policies/ee/work_item_policy.rb +++ b/ee/app/policies/ee/work_item_policy.rb @@ -19,6 +19,7 @@ module WorkItemPolicy prevent :create_todo prevent :update_subscription prevent :create_requirement_test_report + prevent :admin_parent_link end end end diff --git a/ee/app/services/ee/work_items/parent_links/create_service.rb b/ee/app/services/ee/work_items/parent_links/create_service.rb index ba125480011f56304e4fe590be1d37a45aec2487..24f37c6056d8381ace8e40b038c2fb26d7c7a86c 100644 --- a/ee/app/services/ee/work_items/parent_links/create_service.rb +++ b/ee/app/services/ee/work_items/parent_links/create_service.rb @@ -11,7 +11,14 @@ module CreateService override :create_notes_and_resource_event def create_notes_and_resource_event(work_item, _link) - return if work_item.synced_epic.present? + return if params.fetch(:synced_work_item, false) + + super + end + + override :can_admin_link? + def can_admin_link?(work_item) + return true if params.fetch(:synced_work_item, false) super end diff --git a/ee/app/services/ee/work_items/parent_links/destroy_service.rb b/ee/app/services/ee/work_items/parent_links/destroy_service.rb index 1829a6ebf3169a25ab52a7ea2b2e2fa9c9125a22..427befa6075121cae10d96c1cb48318119c67cbd 100644 --- a/ee/app/services/ee/work_items/parent_links/destroy_service.rb +++ b/ee/app/services/ee/work_items/parent_links/destroy_service.rb @@ -11,7 +11,14 @@ module DestroyService override :create_notes def create_notes - return if child.synced_epic.present? + return if params.fetch(:synced_work_item, false) + + super + end + + override :permission_to_remove_relation? + def permission_to_remove_relation? + return true if params.fetch(:synced_work_item, false) super end diff --git a/ee/app/services/epics/epic_links/create_service.rb b/ee/app/services/epics/epic_links/create_service.rb index 3dff05ae4141e3b41e7dfd7bddb71d3b4f674d50..2e6a4a19a58ac38e95b86068182ec12e89f31bd7 100644 --- a/ee/app/services/epics/epic_links/create_service.rb +++ b/ee/app/services/epics/epic_links/create_service.rb @@ -123,7 +123,7 @@ def create_synced_work_item_link!(child_epic) return true unless issuable.work_item && child_epic.work_item response = ::WorkItems::ParentLinks::CreateService - .new(issuable.work_item, current_user, { target_issuable: child_epic.work_item }) + .new(issuable.work_item, current_user, { target_issuable: child_epic.work_item, synced_work_item: true }) .execute if response[:status] == :success diff --git a/ee/app/services/epics/epic_links/destroy_service.rb b/ee/app/services/epics/epic_links/destroy_service.rb index 94de900c32995ab48395cacc74dedc990383ce61..1c2454d8f8a9c067f9cf79db75f749035d232daa 100644 --- a/ee/app/services/epics/epic_links/destroy_service.rb +++ b/ee/app/services/epics/epic_links/destroy_service.rb @@ -45,7 +45,8 @@ def destroy_work_item_parent_link! parent_link = child_epic.work_item.parent_link return unless parent_link.present? - service_response = ::WorkItems::ParentLinks::DestroyService.new(parent_link, current_user).execute + service_response = ::WorkItems::ParentLinks::DestroyService + .new(parent_link, current_user, { synced_work_item: true }).execute return if service_response[:status] == :success synced_work_item_error!(service_response[:message]) diff --git a/ee/spec/policies/work_item_policy_spec.rb b/ee/spec/policies/work_item_policy_spec.rb index 99ae6301dc96f94077f67dcbd967a602e3736910..4ac9eb7698e0f11eab53ebdbf5830d04400a487f 100644 --- a/ee/spec/policies/work_item_policy_spec.rb +++ b/ee/spec/policies/work_item_policy_spec.rb @@ -16,7 +16,8 @@ def permissions(user, work_item) it 'does not allow modifying issue' do expect(permissions(reporter, work_item)).to be_disallowed( :admin_work_item_link, :admin_work_item, :update_work_item, :set_work_item_metadata, - :create_note, :award_emoji, :create_todo, :update_subscription, :create_requirement_test_report + :create_note, :award_emoji, :create_todo, :update_subscription, :create_requirement_test_report, + :admin_parent_link ) end end diff --git a/ee/spec/services/work_items/parent_links/create_service_spec.rb b/ee/spec/services/work_items/parent_links/create_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..21ac93344f53402342c85c6fe437975d81a3fd7f --- /dev/null +++ b/ee/spec/services/work_items/parent_links/create_service_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::ParentLinks::CreateService, feature_category: :portfolio_management do + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group).tap { |g| g.add_reporter(user) } } + let_it_be(:work_item1) { create(:work_item, :epic, namespace: group) } + let_it_be(:work_item2) { create(:work_item, :epic, namespace: group) } + let_it_be(:with_synced_epic) { create(:epic, :with_synced_work_item, group: group).work_item } + + let(:params) { { issuable_references: [child_work_item] } } + + subject(:create_link) { described_class.new(parent_work_item, user, params).execute } + + shared_examples 'does not create relationship' do + it 'no relationship is created' do + expect { create_link }.not_to change { WorkItems::ParentLink.count } + end + + it 'returns error' do + is_expected.to eq({ + http_status: 404, + status: :error, + message: 'No matching work item found. Make sure that you are adding a valid work item ID.' + }) + end + end + + context "when work items don't have a synced epic" do + let_it_be(:parent_work_item) { work_item1 } + let_it_be(:child_work_item) { work_item2 } + + it 'relationship is created' do + expect { create_link }.to change { WorkItems::ParentLink.count }.by(1) + end + end + + context 'when parent work item has a synced epic' do + let_it_be(:parent_work_item) { with_synced_epic } + let_it_be(:child_work_item) { work_item2 } + + it_behaves_like 'does not create relationship' + + context 'when synced_work_item param is true' do + let(:params) { { issuable_references: [child_work_item], synced_work_item: true } } + + it 'relationship is created' do + expect { create_link }.to change { WorkItems::ParentLink.count }.by(1) + end + end + end + + context 'when child work item has a synced epic' do + let_it_be(:parent_work_item) { work_item1 } + let_it_be(:child_work_item) { with_synced_epic } + + it_behaves_like 'does not create relationship' + + context 'when synced_work_item param is true' do + let(:params) { { issuable_references: [child_work_item], synced_work_item: true } } + + it 'relationship is created' do + expect { create_link }.to change { WorkItems::ParentLink.count }.by(1) + end + end + end + + context 'when work items have a synced epic' do + let_it_be(:parent_work_item) { with_synced_epic } + let_it_be(:child_work_item) { create(:epic, :with_synced_work_item, group: group).work_item } + + it_behaves_like 'does not create relationship' + + context 'when synced_work_item param is true' do + let(:params) { { issuable_references: [child_work_item], synced_work_item: true } } + + it 'relationship is created' do + expect { create_link }.to change { WorkItems::ParentLink.count }.by(1) + end + end + end + end +end diff --git a/ee/spec/services/work_items/parent_links/destroy_service_spec.rb b/ee/spec/services/work_items/parent_links/destroy_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..697236c81d75ae2232607e38ec57035cca2fd867 --- /dev/null +++ b/ee/spec/services/work_items/parent_links/destroy_service_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::ParentLinks::DestroyService, feature_category: :team_planning do + describe '#execute' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group).tap { |g| g.add_reporter(user) } } + let_it_be(:work_item1) { create(:work_item, :epic, namespace: group) } + let_it_be(:work_item2) { create(:work_item, :epic, namespace: group) } + let_it_be(:with_synced_epic1) { create(:epic, :with_synced_work_item, group: group).work_item } + let_it_be(:with_synced_epic2) { create(:epic, :with_synced_work_item, group: group).work_item } + + let(:params) { {} } + + subject(:destroy_link) { described_class.new(parent_link, user, params).execute } + + shared_examples 'does not remove relationship' do + it 'does not remove relation', :aggregate_failures do + expect { destroy_link }.to not_change { WorkItems::ParentLink.count }.from(1) + .and not_change { WorkItems::ResourceLinkEvent.count } + expect(SystemNoteService).not_to receive(:unrelate_work_item) + end + + it 'returns error message' do + is_expected.to eq(message: 'No Work Item Link found', status: :error, http_status: 404) + end + end + + context "when work items doesn't have a synced epic" do + let_it_be(:parent_link) { create(:parent_link, work_item: work_item1, work_item_parent: work_item2) } + + it 'removes relation', :aggregate_failures do + expect { destroy_link }.to change { WorkItems::ParentLink.count }.by(-1) + end + end + + context "when parent work item has a synced epic" do + let_it_be(:parent_link) { create(:parent_link, work_item: with_synced_epic1, work_item_parent: work_item1) } + + it_behaves_like 'does not remove relationship' + + context 'when synced_work_item param is true' do + let(:params) { { synced_work_item: true } } + + it 'removed relationship' do + expect { destroy_link }.to change { WorkItems::ParentLink.count }.by(-1) + end + end + end + + context 'when child work item has a synced epic' do + let_it_be(:parent_link) { create(:parent_link, work_item: work_item1, work_item_parent: with_synced_epic1) } + + it_behaves_like 'does not remove relationship' + + context 'when synced_work_item param is true' do + let(:params) { { synced_work_item: true } } + + it 'removed relationship' do + expect { destroy_link }.to change { WorkItems::ParentLink.count }.by(-1) + end + end + end + + context 'when work items have a synced epic' do + let_it_be(:parent_link) do + create(:parent_link, work_item: with_synced_epic1, work_item_parent: with_synced_epic2) + end + + it_behaves_like 'does not remove relationship' + + context 'when synced_work_item param is true' do + let(:params) { { synced_work_item: true } } + + it 'removed relationship' do + expect { destroy_link }.to change { WorkItems::ParentLink.count }.by(-1) + end + end + end + end +end