Commit 0b774a6d authored by Felipe Artur's avatar Felipe Artur Committed by Sean McGivern

Resolve: When you add an issue to an epic that is already in the epic, you get...

Resolve: When you add an issue to an epic that is already in the epic, you get erroneous system notes
parent 98a8e4d6
......@@ -42,9 +42,17 @@ module EpicIssues
end
def linkable_issues(issues)
return [] unless can?(current_user, :admin_epic, issuable.group)
@linkable_issues ||= begin
return [] unless can?(current_user, :admin_epic, issuable.group)
issues.select { |issue| issuable_group_descendants.include?(issue.project.group) }
issues.select do |issue|
issuable_group_descendants.include?(issue.project.group) && !previous_related_issues.include?(issue)
end
end
end
def previous_related_issues
@related_issues ||= issuable.issues.to_a
end
def issuable_group_descendants
......
......@@ -7,7 +7,14 @@ module IssuableLinks
end
def execute
if referenced_issues.blank?
# If ALL referenced issues are already assigned to the given epic it renders a conflict status,
# otherwise create issue links for the issues which
# are still not assigned and return success message.
if render_conflict_error?
return error('Issue(s) already assigned', 409)
end
if render_not_found_error?
return error('No Issue found for given params', 404)
end
......@@ -17,8 +24,18 @@ module IssuableLinks
private
def render_conflict_error?
referenced_issues.present? && (referenced_issues - previous_related_issues).empty?
end
def render_not_found_error?
linkable_issues(referenced_issues).empty?
end
def create_issue_links
referenced_issues.each do |referenced_issue|
issues = linkable_issues(referenced_issues)
issues.each do |referenced_issue|
relate_issues(referenced_issue) do |params|
create_notes(referenced_issue, params)
end
......@@ -29,15 +46,13 @@ module IssuableLinks
@referenced_issues ||= begin
target_issue = params[:target_issue]
issues = if params[:issue_references].present?
extract_issues_from_references
elsif target_issue
[target_issue]
else
[]
end
linkable_issues(issues)
if params[:issue_references].present?
extract_issues_from_references
elsif target_issue
[target_issue]
else
[]
end
end
end
......@@ -59,6 +74,10 @@ module IssuableLinks
raise NotImplementedError
end
def previous_related_issues
raise NotImplementedError
end
def relate_issues(referenced_issue)
raise NotImplementedError
end
......
......@@ -7,12 +7,18 @@ module IssueLinks
end
def linkable_issues(issues)
issues.select { |issue| can?(current_user, :admin_issue_link, issue) }
@linkable_issues ||= begin
issues.select { |issue| can?(current_user, :admin_issue_link, issue) }
end
end
def create_notes(referenced_issue, params)
SystemNoteService.relate_issue(issuable, referenced_issue, current_user)
SystemNoteService.relate_issue(referenced_issue, issuable, current_user)
end
def previous_related_issues
@related_issues ||= issuable.related_issues(current_user).to_a
end
end
end
---
title: Do not allow to assign an issue to an epic twice
merge_request: 8004
author:
type: fixed
......@@ -24,7 +24,7 @@ describe EpicIssues::CreateService do
it 'creates a new relationship and updates epic' do
expect(epic).to receive(:update_start_and_due_dates)
expect { subject }.to change(EpicIssue, :count).from(1).to(2)
expect { subject }.to change(EpicIssue, :count).by(1)
expect(created_link).to have_attributes(epic: epic)
end
......@@ -39,13 +39,13 @@ describe EpicIssues::CreateService do
expect(subject).to eq(status: :success)
end
it 'creates 2 system notes' do
expect { subject }.to change { Note.count }.from(0).to(2)
it 'creates 1 system note for epic and 1 system note for issue' do
expect { subject }.to change { Note.count }.by(2)
end
it 'creates a note for epic correctly' do
subject
note = Note.find_by(noteable_id: epic.id, noteable_type: 'Epic')
note = Note.where(noteable_id: epic.id, noteable_type: 'Epic').last
expect(note.note).to eq("added issue #{issue.to_reference(epic.group)}")
expect(note.author).to eq(user)
......@@ -199,6 +199,36 @@ describe EpicIssues::CreateService do
include_examples 'returns an error'
end
context 'when assigning issue(s) to the same epic' do
before do
group.add_developer(user)
assign_issue([valid_reference])
epic.reload
end
subject { assign_issue([valid_reference]) }
it 'no relationship is created' do
expect { subject }.not_to change { EpicIssue.count }
end
it 'does not create notes' do
expect { subject }.not_to change { Note.count }
end
it 'returns an error' do
expect(subject).to eq(message: 'Issue(s) already assigned', status: :error, http_status: 409)
end
context 'when at least one of the issues is still not assigned to the epic' do
let(:valid_reference) { issue2.to_reference(full: true) }
subject { assign_issue([valid_reference, issue.to_reference(full: true)]) }
include_examples 'returns success'
end
end
context 'when an issue is already assigned to another epic' do
before do
group.add_developer(user)
......
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