From 14c983fb696b7b75065df2b5defd458e563444e3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 22 Feb 2016 11:00:40 +0100 Subject: [PATCH] Add implementation that rewrites issue notes when moving --- app/services/issues/move_service.rb | 23 ++++++-- spec/services/issues/move_service_spec.rb | 67 ++++++++++++++++++----- 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index 935bd8a87f7..2cba6147429 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -15,9 +15,19 @@ module Issues def execute return unless move? + # New issue tasks + # open_new_issue rewrite_notes + add_moved_from_note + + # Old issue tasks + # close_old_issue + add_moved_to_note + + # Notifications + # notify_participants @issue_new @@ -40,25 +50,28 @@ module Issues def open_new_issue @issue_new.project = @project_new @issue_new.save! - - add_note_moved_from end def rewrite_notes + @issue_old.notes.find_each do |note| + note_new = note.dup + note_new.project = @project_new + note_new.noteable = @issue_new + note_new.save! + end end def close_old_issue - add_note_moved_to end def notify_participants end - def add_note_moved_from + def add_moved_from_note SystemNoteService.noteable_moved(:from, @issue_new, @project_new, @issue_old, @current_user) end - def add_note_moved_to + def add_moved_to_note SystemNoteService.noteable_moved(:to, @issue_old, @project_old, @issue_new, @current_user) end end diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 412a817208e..6667840ec6e 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -2,15 +2,15 @@ require 'spec_helper' describe Issues::MoveService, services: true do let(:user) { create(:user) } - let(:issue) { create(:issue, title: 'Some issue', description: 'Some issue description') } - let(:current_project) { issue.project } + let(:title) { 'Some issue' } + let(:description) { 'Some issue description' } + let(:old_issue) { create(:issue, title: title, description: description) } + let(:current_project) { old_issue.project } let(:new_project) { create(:project) } let(:move_params) { { 'move_to_project_id' => new_project.id } } - let(:move_service) { described_class.new(current_project, user, move_params, issue) } + let(:move_service) { described_class.new(current_project, user, move_params, old_issue) } - before do - current_project.team << [user, :master] - end + before { current_project.team << [user, :master] } context 'issue movable' do describe '#move?' do @@ -19,18 +19,57 @@ describe Issues::MoveService, services: true do end describe '#execute' do - let!(:new_issue) { move_service.execute } - - it 'should create a new issue in a new project' do - expect(new_issue.project).to eq new_project + shared_context 'issue move executed' do + let!(:new_issue) { move_service.execute } end - it 'should add system note to old issue' do - expect(issue.notes.last.note).to match /^Moved to/ + context 'generic issue' do + include_context 'issue move executed' + + it 'creates a new issue in a new project' do + expect(new_issue.project).to eq new_project + end + + it 'rewrites issue title' do + expect(new_issue.title).to eq title + end + + it 'rewrites issue description' do + expect(new_issue.description).to include description + end + + it 'adds system note to old issue at the end' do + expect(old_issue.notes.last.note).to match /^Moved to/ + end + + it 'adds system note to new issue at the end' do + expect(new_issue.notes.last.note).to match /^Moved from/ + end end - it 'should add system note to new issue' do - expect(new_issue.notes.last.note).to match /^Moved from/ + context 'notes exist' do + let(:note_contents) do + ['Some system note 1', 'Some comment', 'Some system note 2'] + end + + before do + note_params = { noteable: old_issue, project: current_project, author: user} + create(:system_note, note_params.merge(note: note_contents.first)) + create(:note, note_params.merge(note: note_contents.second)) + create(:system_note, note_params.merge(note: note_contents.third)) + end + + include_context 'issue move executed' + + let(:new_notes) { new_issue.notes.order('id ASC').pluck(:note) } + + it 'rewrites existing system notes in valid order' do + expect(new_notes.first(3)).to eq note_contents + end + + it 'adds a system note about move after rewritten notes' do + expect(new_notes.last).to match /^Moved from/ + end end end end