From 9882802a8b70e998ff6850a3d096b3b52730ab85 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 16 Feb 2016 11:47:00 +0100
Subject: [PATCH] Improve system notes that are added when issue is moved
---
app/services/issues/move_service.rb | 4 +-
app/services/system_note_service.rb | 25 +++++++----
spec/services/issues/move_service_spec.rb | 6 ++-
spec/services/system_note_service_spec.rb | 51 +++++++++++++++++++----
4 files changed, 66 insertions(+), 20 deletions(-)
diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
index 49ec9609e1f..e2ab06ac332 100644
--- a/app/services/issues/move_service.rb
+++ b/app/services/issues/move_service.rb
@@ -4,6 +4,7 @@ module Issues
@issue_old = issue_old
@issue_new = issue_old.dup
@project_new = project_new
+ @project_old = @project
open_new_issue
rewrite_notes
@@ -33,10 +34,11 @@ module Issues
end
def add_note_moved_from
+ SystemNoteService.noteable_moved(:from, @issue_new, @project_new, @issue_old, @current_user)
end
def add_note_moved_to
- SystemNoteService.issue_moved_to_another_project(@issue_old, @project, @project_new, @current_user)
+ SystemNoteService.noteable_moved(:to, @issue_old, @project_old, @issue_new, @current_user)
end
end
end
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 95379d260f1..a8d6555dec3 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -3,6 +3,7 @@
# Used for creating system notes (e.g., when a user references a merge request
# from an issue, an issue's assignee changes, an issue is closed, etc.)
class SystemNoteService
+ extend GitlabMarkdownHelper
# Called when commits are added to a Merge Request
#
# noteable - Noteable object
@@ -388,20 +389,26 @@ class SystemNoteService
create_note(noteable: noteable, project: project, author: author, note: body)
end
- # Called when issue has been moved to another project
+ # Called when noteable has been moved to another project
#
- # issue - Issue that has been moved to another project
- # project_from - Source project of the issue
- # project_to - Destination project for the issue
- # author - User performing the move
+ # direction - symbol, :to or :from
+ # noteable - Noteable object
+ # noteable_ref - Referenced noteable
+ # author - User performing the move
#
# Example Note text:
#
- # "This issue has been moved to SomeNamespace / SomeProject"
+ # "Moved to project_new/#11"
#
# Returns the created Note object
- def self.issue_moved_to_another_project(issue, project_from, project_to, author)
- body = "This issue has been moved to #{project_to.to_reference} by #{author.to_reference}"
- create_note(noteable: issue, project: project_from, author: author, note: body)
+ def self.noteable_moved(direction, noteable, project, noteable_ref, author)
+ unless [:to, :from].include?(direction)
+ raise StandardError, "Invalid direction `#{direction}`"
+ end
+
+ cross_reference = cross_project_reference(noteable_ref.project, noteable_ref)
+
+ body = "Moved #{direction} #{cross_reference}"
+ create_note(noteable: noteable, project: project, author: author, note: body)
end
end
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 25dacb2068b..569e155e617 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -20,7 +20,11 @@ describe Issues::MoveService, services: true do
end
it 'should add system note to old issue' do
- expect(issue.notes.last.note).to match /This issue has been moved to/
+ expect(issue.notes.last.note).to match /^Moved to/
+ end
+
+ it 'should add system note to new issue' do
+ expect(new_issue.notes.last.note).to match /^Moved from/
end
end
end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 9074d3dbe19..e0ae49ab37c 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -441,23 +441,56 @@ describe SystemNoteService, services: true do
end
end
- describe '.issue_moved_to_another_project' do
+ describe '.noteable_moved' do
+ let(:new_project) { create(:project) }
+ let(:new_noteable) { create(:issue, project: new_project) }
+
subject do
- described_class.issue_moved_to_another_project(noteable, project, new_project, author)
+ described_class.noteable_moved(direction, noteable, project, new_noteable, author)
end
- let(:new_project) { create(:project) }
+ shared_examples 'cross project mentionable' do
+ include GitlabMarkdownHelper
+
+ it 'should contain cross reference to new noteable' do
+ expect(subject.note).to include cross_project_reference(new_project, new_noteable)
+ end
+
+ it 'should mention referenced noteable' do
+ expect(subject.note).to include new_noteable.to_reference
+ end
+
+ it 'should mention referenced project' do
+ expect(subject.note).to include new_project.to_reference
+ end
+ end
+
+ context 'moved to' do
+ let(:direction) { :to }
+
+ it_behaves_like 'cross project mentionable'
- it 'should notify about issue being moved' do
- expect(subject.note).to match /This issue has been moved to/
+ it 'should notify about noteable being moved to' do
+ expect(subject.note).to match /Moved to/
+ end
end
- it 'should mention destination project' do
- expect(subject.note).to include new_project.to_reference
+ context 'moved from' do
+ let(:direction) { :from }
+
+ it_behaves_like 'cross project mentionable'
+
+ it 'should notify about noteable being moved from' do
+ expect(subject.note).to match /Moved from/
+ end
end
- it 'should mention author of that change' do
- expect(subject.note).to include author.to_reference
+ context 'invalid direction' do
+ let (:direction) { :invalid }
+
+ it 'should raise error' do
+ expect { subject }.to raise_error StandardError, /Invalid direction/
+ end
end
end
--
GitLab