Commit a6d3efc5 authored by Jan Provaznik's avatar Jan Provaznik

Fix link deletion

When an issue or epic link is deleted, check that the link id belongs to
the issuable specified in the API URL.

Changelog: fixed
parent ea08a31e
...@@ -29,6 +29,8 @@ module IssuableLink ...@@ -29,6 +29,8 @@ module IssuableLink
validate :check_self_relation validate :check_self_relation
validate :check_opposite_relation validate :check_opposite_relation
scope :for_source_or_target, ->(issuable) { where(source: issuable).or(where(target: issuable)) }
enum link_type: { TYPE_RELATES_TO => 0, TYPE_BLOCKS => 1 } enum link_type: { TYPE_RELATES_TO => 0, TYPE_BLOCKS => 1 }
private private
......
...@@ -92,13 +92,14 @@ module API ...@@ -92,13 +92,14 @@ module API
requires :related_epic_link_id, type: Integer, desc: 'The ID of a related epic link' requires :related_epic_link_id, type: Integer, desc: 'The ID of a related epic link'
end end
delete ':id/epics/:epic_iid/related_epics/:related_epic_link_id' do delete ':id/epics/:epic_iid/related_epics/:related_epic_link_id' do
find_permissioned_epic!(params[:epic_iid]) epic = find_permissioned_epic!(params[:epic_iid])
epic_link = ::Epic::RelatedEpicLink
epic_link = ::Epic::RelatedEpicLink.find(declared_params[:related_epic_link_id]) .for_source_or_target(epic)
.find(declared_params[:related_epic_link_id])
result = ::Epics::RelatedEpicLinks::DestroyService result = ::Epics::RelatedEpicLinks::DestroyService
.new(epic_link, current_user) .new(epic_link, current_user)
.execute .execute
if result[:status] == :success if result[:status] == :success
present epic_link, with: Entities::RelatedEpicLink present epic_link, with: Entities::RelatedEpicLink
......
...@@ -261,6 +261,19 @@ RSpec.describe API::RelatedEpicLinks do ...@@ -261,6 +261,19 @@ RSpec.describe API::RelatedEpicLinks do
it_behaves_like 'not found resource', 'No Related Epic Link found' it_behaves_like 'not found resource', 'No Related Epic Link found'
end end
context 'when related_epic_link_id belongs to a different epic' do
let_it_be(:other_epic) { create(:epic, group: target_group) }
let_it_be(:other_epic_link) { create(:related_epic_link, source: other_epic, target: target_epic) }
subject { delete api("/groups/#{group.id}/epics/#{epic.iid}/related_epics/#{other_epic_link.id}", user) }
before do
target_group.add_reporter(user)
end
it_behaves_like 'not found resource', '404 Not found'
end
context 'when user can relate epics' do context 'when user can relate epics' do
before do before do
target_group.add_reporter(user) target_group.add_reporter(user)
......
...@@ -67,14 +67,16 @@ module API ...@@ -67,14 +67,16 @@ module API
requires :issue_link_id, type: Integer, desc: 'The ID of an issue link' requires :issue_link_id, type: Integer, desc: 'The ID of an issue link'
end end
delete ':id/issues/:issue_iid/links/:issue_link_id' do delete ':id/issues/:issue_iid/links/:issue_link_id' do
issue_link = IssueLink.find(declared_params[:issue_link_id]) issue = find_project_issue(params[:issue_iid])
issue_link = IssueLink
.for_source_or_target(issue)
.find(declared_params[:issue_link_id])
find_project_issue(params[:issue_iid])
find_project_issue(issue_link.target.iid.to_s, issue_link.target.project_id.to_s) find_project_issue(issue_link.target.iid.to_s, issue_link.target.project_id.to_s)
result = ::IssueLinks::DestroyService result = ::IssueLinks::DestroyService
.new(issue_link, current_user) .new(issue_link, current_user)
.execute .execute
if result[:status] == :success if result[:status] == :success
present issue_link, with: Entities::IssueLink present issue_link, with: Entities::IssueLink
......
...@@ -205,16 +205,30 @@ RSpec.describe API::IssueLinks do ...@@ -205,16 +205,30 @@ RSpec.describe API::IssueLinks do
end end
context 'when user has ability to delete the issue link' do context 'when user has ability to delete the issue link' do
let_it_be(:target_issue) { create(:issue, project: project) }
before do
project.add_reporter(user)
end
it 'returns 200' do it 'returns 200' do
target_issue = create(:issue, project: project)
issue_link = create(:issue_link, source: issue, target: target_issue) issue_link = create(:issue_link, source: issue, target: target_issue)
project.add_reporter(user)
delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user) delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/issue_link') expect(response).to match_response_schema('public_api/v4/issue_link')
end end
it 'returns 404 when the issue link does not belong to the specified issue' do
other_issue = create(:issue, project: project)
issue_link = create(:issue_link, source: other_issue, target: target_issue)
delete api("/projects/#{project.id}/issues/#{issue.iid}/links/#{issue_link.id}", user)
expect(response).to have_gitlab_http_status(:not_found)
expect(json_response['message']).to eq('404 Not found')
end
end end
end end
end end
......
...@@ -55,6 +55,19 @@ RSpec.shared_examples 'issuable link' do ...@@ -55,6 +55,19 @@ RSpec.shared_examples 'issuable link' do
end end
end end
describe 'scopes' do
describe '.for_source_or_target' do
it 'returns only links where id is either source or target id' do
link1 = create(issuable_link_factory, source: issuable_link.source)
link2 = create(issuable_link_factory, target: issuable_link.source)
# unrelated link, should not be included in result list
create(issuable_link_factory) # rubocop: disable Rails/SaveBang
expect(described_class.for_source_or_target(issuable_link.source_id)).to match_array([issuable_link, link1, link2])
end
end
end
describe '.link_type' do describe '.link_type' do
it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1) } it { is_expected.to define_enum_for(:link_type).with_values(relates_to: 0, blocks: 1) }
......
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