From 310b417b0f2336e5e7a9b44da2fabf5d4abb0cb4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 14 Mar 2016 08:25:37 +0100 Subject: [PATCH] Preserve original author when moving issue This also wrapps entire process into transation, as rewriting references may have large memory footprint. --- app/services/issues/move_service.rb | 38 +++++++++++------------ spec/services/issues/move_service_spec.rb | 8 ++--- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index de12ea5c172..d1bded5c99f 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -4,7 +4,7 @@ module Issues super(project, current_user, params) @issue_old = issue - @issue_new = nil + @issue_new = issue.dup @project_old = @project @project_new = Project.find(new_project_id) if new_project_id end @@ -12,16 +12,18 @@ module Issues def execute return unless move? - # New issue tasks - # - open_new_issue - rewrite_notes - add_moved_from_note + ActiveRecord::Base.transaction do + # New issue tasks + # + open_new_issue + rewrite_notes + add_moved_from_note - # Old issue tasks - # - add_moved_to_note - close_old_issue + # Old issue tasks + # + add_moved_to_note + close_old_issue + end # Notifications # @@ -42,16 +44,12 @@ module Issues end def open_new_issue - create_service = CreateService.new(@project_new, current_user, new_issue_params) - @issue_new = create_service.execute - end - - def new_issue_params - new_params = { id: nil, iid: nil, milestone_id: nil, label_ids: [], - project_id: @project_new.id, - description: rewrite_references(@issue_old) } - - params.merge(new_params) + @issue_new.iid = nil + @issue_new.project = @project_new + @issue_new.labels = [] + @issue_new.milestone = nil + @issue_new.description = rewrite_references(@issue_old) + @issue_new.save! end def rewrite_notes diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index d516bd4830a..4fcfceaddd4 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -74,13 +74,13 @@ describe Issues::MoveService, services: true do expect(new_issue.persisted?).to be true end - it 'persist all changes' do + it 'persists all changes' do expect(old_issue.changed?).to be false - expect(new_issue.changed?).to be false + expect(new_issue.persisted?).to be true end - it 'changes author' do - expect(new_issue.author).to eq user + it 'preserves author' do + expect(new_issue.author).to eq author end it 'removes data that is invalid in new context' do