Merge branch 'ce-3777-promote-to-epic' into 'master'

Refactoring Issues::MoveService (port of promote epics)

See merge request gitlab-org/gitlab-ce!22766

app/services/issuable/clone/attributes_rewriter.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+module Issuable
+  module Clone
+    class AttributesRewriter < ::Issuable::Clone::BaseService
+      def initialize(current_user, original_entity, new_entity)
+        @current_user = current_user
+        @original_entity = original_entity
+        @new_entity = new_entity
+      end
+
+      def execute
+        new_entity.update(milestone: cloneable_milestone, labels: cloneable_labels)
+        copy_resource_label_events
+      end
+
+      private
+
+      def cloneable_milestone
+        title = original_entity.milestone&.title
+        return unless title
+
+        params = { title: title, project_ids: new_entity.project&.id, group_ids: group&.id }
+
+        milestones = MilestonesFinder.new(params).execute
+        milestones.first
+      end
+
+      def cloneable_labels
+        params = {
+          project_id: new_entity.project&.id,
+          group_id: group&.id,
+          title: original_entity.labels.select(:title),
+          include_ancestor_groups: true
+        }
+
+        params[:only_group_labels] = true if new_parent.is_a?(Group)
+
+        LabelsFinder.new(current_user, params).execute
+      end
+
+      def copy_resource_label_events
+        original_entity.resource_label_events.find_in_batches do |batch|
+          events = batch.map do |event|
+            entity_key = new_entity.is_a?(Issue) ? 'issue_id' : 'epic_id'
+            # rubocop: disable CodeReuse/ActiveRecord
+            event.attributes
+              .except('id', 'reference', 'reference_html')
+              .merge(entity_key => new_entity.id, 'action' => ResourceLabelEvent.actions[event.action])
+            # rubocop: enable CodeReuse/ActiveRecord
+          end
+
+          Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, events)
+        end
+      end
+
+      def entity_key
+        new_entity.class.name.parameterize('_').foreign_key
+      end
+    end
+  end
+end

app/services/issuable/clone/base_service.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+module Issuable
+  module Clone
+    class BaseService < IssuableBaseService
+      attr_reader :original_entity, :new_entity
+
+      alias_method :old_project, :project
+
+      def execute(original_entity, new_project = nil)
+        @original_entity = original_entity
+
+        # Using transaction because of a high resources footprint
+        # on rewriting notes (unfolding references)
+        #
+        ActiveRecord::Base.transaction do
+          @new_entity = create_new_entity
+
+          update_new_entity
+          update_old_entity
+          create_notes
+        end
+      end
+
+      private
+
+      def update_new_entity
+        rewriters = [ContentRewriter, AttributesRewriter]
+
+        rewriters.each do |rewriter|
+          rewriter.new(current_user, original_entity, new_entity).execute
+        end
+      end
+
+      def update_old_entity
+        close_issue
+      end
+
+      def create_notes
+        add_note_from
+        add_note_to
+      end
+
+      def close_issue
+        close_service = Issues::CloseService.new(old_project, current_user)
+        close_service.execute(original_entity, notifications: false, system_note: false)
+      end
+
+      def new_parent
+        new_entity.project ? new_entity.project : new_entity.group
+      end
+
+      def group
+        if new_entity.project&.group && current_user.can?(:read_group, new_entity.project.group)
+          new_entity.project.group
+        end
+      end
+    end
+  end
+end

app/services/issuable/clone/content_rewriter.rb

@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+module Issuable
+  module Clone
+    class ContentRewriter < ::Issuable::Clone::BaseService
+      def initialize(current_user, original_entity, new_entity)
+        @current_user = current_user
+        @original_entity = original_entity
+        @new_entity = new_entity
+        @project = original_entity.project
+      end
+
+      def execute
+        rewrite_description
+        rewrite_award_emoji(original_entity, new_entity)
+        rewrite_notes
+      end
+
+      private
+
+      def rewrite_description
+        new_entity.update(description: rewrite_content(original_entity.description))
+      end
+
+      def rewrite_notes
+        original_entity.notes_with_associations.find_each do |note|
+          new_note = note.dup
+          new_params = {
+            project: new_entity.project, noteable: new_entity,
+            note: rewrite_content(new_note.note),
+            created_at: note.created_at,
+            updated_at: note.updated_at
+          }
+
+          if note.system_note_metadata
+            new_params[:system_note_metadata] = note.system_note_metadata.dup
+          end
+
+          new_note.update(new_params)
+
+          rewrite_award_emoji(note, new_note)
+        end
+      end
+
+      def rewrite_content(content)
+        return unless content
+
+        rewriters = [Gitlab::Gfm::ReferenceRewriter, Gitlab::Gfm::UploadsRewriter]
+
+        rewriters.inject(content) do |text, klass|
+          rewriter = klass.new(text, old_project, current_user)
+          rewriter.rewrite(new_parent)
+        end
+      end
+
+      def rewrite_award_emoji(old_awardable, new_awardable)
+        old_awardable.award_emoji.each do |award|
+          new_award = award.dup
+          new_award.awardable = new_awardable
+          new_award.save
+        end
+      end
+    end
+  end
+end

app/services/issues/move_service.rb

@@ -1,165 +1,66 @@
 # frozen_string_literal: true
 
 module Issues
-  class MoveService < Issues::BaseService
+  class MoveService < Issuable::Clone::BaseService
     MoveError = Class.new(StandardError)
 
-    def execute(issue, new_project)
+    def execute(issue, target_project)
-      @old_issue = issue
+      @target_project = target_project
-      @old_project = @project
-      @new_project = new_project
 
-      unless issue.can_move?(current_user, new_project)
+      unless issue.can_move?(current_user, @target_project)
         raise MoveError, 'Cannot move issue due to insufficient permissions!'
       end
 
-      if @project == new_project
+      if @project == @target_project
         raise MoveError, 'Cannot move issue to project it originates from!'
       end
 
-      # Using transaction because of a high resources footprint
+      super
-      # on rewriting notes (unfolding references)
-      #
-      ActiveRecord::Base.transaction do
-        @new_issue = create_new_issue
-
-        update_new_issue
-        update_old_issue
-      end
 
       notify_participants
 
-      @new_issue
+      new_entity
     end
 
     private
 
-    def update_new_issue
+    def update_old_entity
-      rewrite_notes
+      super
-      copy_resource_label_events
-      rewrite_issue_award_emoji
-      add_note_moved_from
-    end
 
-    def update_old_issue
-      add_note_moved_to
-      close_issue
       mark_as_moved
     end
 
-    def create_new_issue
+    def create_new_entity
-      new_params = { id: nil, iid: nil, label_ids: cloneable_label_ids,
+      new_params = {
-                     milestone_id: cloneable_milestone_id,
+                     id: nil,
-                     project: @new_project, author: @old_issue.author,
+                     iid: nil,
-                     description: rewrite_content(@old_issue.description),
+                     project: @target_project,
-                     assignee_ids: @old_issue.assignee_ids }
+                     author: original_entity.author,
+                     assignee_ids: original_entity.assignee_ids
+                   }
 
-      new_params = @old_issue.serializable_hash.symbolize_keys.merge(new_params)
+      new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params)
-      CreateService.new(@new_project, @current_user, new_params).execute
+      CreateService.new(@target_project, @current_user, new_params).execute
-    end
-
-    # rubocop: disable CodeReuse/ActiveRecord
-    def cloneable_label_ids
-      params = {
-        project_id: @new_project.id,
-        title: @old_issue.labels.pluck(:title),
-        include_ancestor_groups: true
-      }
-
-      LabelsFinder.new(current_user, params).execute.pluck(:id)
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    def cloneable_milestone_id
-      title = @old_issue.milestone&.title
-      return unless title
-
-      if @new_project.group && can?(current_user, :read_group, @new_project.group)
-        group_id = @new_project.group.id
-      end
-
-      params =
-        { title: title, project_ids: @new_project.id, group_ids: group_id }
-
-      milestones = MilestonesFinder.new(params).execute
-      milestones.first&.id
-    end
-
-    def rewrite_notes
-      @old_issue.notes_with_associations.find_each do |note|
-        new_note = note.dup
-        new_params = { project: @new_project, noteable: @new_issue,
-                       note: rewrite_content(new_note.note),
-                       created_at: note.created_at,
-                       updated_at: note.updated_at }
-
-        new_note.update(new_params)
-
-        rewrite_award_emoji(note, new_note)
-      end
-    end
-
-    # rubocop: disable CodeReuse/ActiveRecord
-    def copy_resource_label_events
-      @old_issue.resource_label_events.find_in_batches do |batch|
-        events = batch.map do |event|
-          event.attributes
-            .except('id', 'reference', 'reference_html')
-            .merge('issue_id' => @new_issue.id, 'action' => ResourceLabelEvent.actions[event.action])
-        end
-
-        Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, events)
-      end
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    def rewrite_issue_award_emoji
-      rewrite_award_emoji(@old_issue, @new_issue)
-    end
-
-    def rewrite_award_emoji(old_awardable, new_awardable)
-      old_awardable.award_emoji.each do |award|
-        new_award = award.dup
-        new_award.awardable = new_awardable
-        new_award.save
-      end
-    end
-
-    def rewrite_content(content)
-      return unless content
-
-      rewriters = [Gitlab::Gfm::ReferenceRewriter,
-                   Gitlab::Gfm::UploadsRewriter]
-
-      rewriters.inject(content) do |text, klass|
-        rewriter = klass.new(text, @old_project, @current_user)
-        rewriter.rewrite(@new_project)
-      end
-    end
-
-    def close_issue
-      close_service = CloseService.new(@old_project, @current_user)
-      close_service.execute(@old_issue, notifications: false, system_note: false)
-    end
-
-    def add_note_moved_from
-      SystemNoteService.noteable_moved(@new_issue, @new_project,
-                                       @old_issue, @current_user,
-                                       direction: :from)
-    end
-
-    def add_note_moved_to
-      SystemNoteService.noteable_moved(@old_issue, @old_project,
-                                       @new_issue, @current_user,
-                                       direction: :to)
     end
 
     def mark_as_moved
-      @old_issue.update(moved_to: @new_issue)
+      original_entity.update(moved_to: new_entity)
     end
 
     def notify_participants
-      notification_service.async.issue_moved(@old_issue, @new_issue, @current_user)
+      notification_service.async.issue_moved(original_entity, new_entity, @current_user)
+    end
+
+    def add_note_from
+      SystemNoteService.noteable_moved(new_entity, @target_project,
+                                       original_entity, current_user,
+                                       direction: :from)
+    end
+
+    def add_note_to
+      SystemNoteService.noteable_moved(original_entity, old_project,
+                                       new_entity, current_user,
+                                       direction: :to)
     end
   end
 end

app/uploaders/file_uploader.rb

@@ -149,9 +149,9 @@ class FileUploader < GitlabUploader
 
   # return a new uploader with a file copy on another project
   def self.copy_to(uploader, to_project)
-    moved = uploader.dup.tap do |u|
+    moved = self.new(to_project)
-      u.model = to_project
+    moved.object_store = uploader.object_store
-    end
+    moved.filename = uploader.filename
 
     moved.copy_file(uploader.file)
     moved

lib/gitlab/gfm/reference_rewriter.rb

@@ -31,19 +31,19 @@ module Gitlab
     class ReferenceRewriter
       RewriteError = Class.new(StandardError)
 
-      def initialize(text, source_project, current_user)
+      def initialize(text, source_parent, current_user)
         @text = text
-        @source_project = source_project
+        @source_parent = source_parent
         @current_user = current_user
         @original_html = markdown(text)
         @pattern = Gitlab::ReferenceExtractor.references_pattern
       end
 
-      def rewrite(target_project)
+      def rewrite(target_parent)
         return @text unless needs_rewrite?
 
         @text.gsub(@pattern) do |reference|
-          unfold_reference(reference, Regexp.last_match, target_project)
+          unfold_reference(reference, Regexp.last_match, target_parent)
         end
       end
 
@@ -53,14 +53,14 @@ module Gitlab
 
       private
 
-      def unfold_reference(reference, match, target_project)
+      def unfold_reference(reference, match, target_parent)
         before = @text[0...match.begin(0)]
         after = @text[match.end(0)..-1]
 
         referable = find_referable(reference)
         return reference unless referable
 
-        cross_reference = build_cross_reference(referable, target_project)
+        cross_reference = build_cross_reference(referable, target_parent)
         return reference if reference == cross_reference
 
         if cross_reference.nil?
@@ -72,17 +72,17 @@ module Gitlab
       end
 
       def find_referable(reference)
-        extractor = Gitlab::ReferenceExtractor.new(@source_project,
+        extractor = Gitlab::ReferenceExtractor.new(@source_parent,
                                                    @current_user)
         extractor.analyze(reference)
         extractor.all.first
       end
 
-      def build_cross_reference(referable, target_project)
+      def build_cross_reference(referable, target_parent)
         if referable.respond_to?(:project)
-          referable.to_reference(target_project)
+          referable.to_reference(target_parent)
         else
-          referable.to_reference(@source_project, target_project: target_project)
+          referable.to_reference(@source_parent, target_project: target_parent)
         end
       end
 
@@ -91,7 +91,7 @@ module Gitlab
       end
 
       def markdown(text)
-        Banzai.render(text, project: @source_project, no_original_data: true)
+        Banzai.render(text, project: @source_parent, no_original_data: true)
       end
     end
   end

lib/gitlab/gfm/uploads_rewriter.rb

@@ -16,14 +16,15 @@ module Gitlab
         @pattern = FileUploader::MARKDOWN_PATTERN
       end
 
-      def rewrite(target_project)
+      def rewrite(target_parent)
         return @text unless needs_rewrite?
 
         @text.gsub(@pattern) do |markdown|
           file = find_file(@source_project, $~[:secret], $~[:file])
           break markdown unless file.try(:exists?)
 
-          moved = FileUploader.copy_to(file, target_project)
+          klass = target_parent.is_a?(Namespace) ? NamespaceFileUploader : FileUploader
+          moved = klass.copy_to(file, target_parent)
           moved.markdown_link
         end
       end

lib/gitlab/quick_actions/command_definition.rb

@@ -2,13 +2,14 @@ module Gitlab
   module QuickActions
     class CommandDefinition
       attr_accessor :name, :aliases, :description, :explanation, :params,
-        :condition_block, :parse_params_block, :action_block
+        :condition_block, :parse_params_block, :action_block, :warning
 
       def initialize(name, attributes = {})
         @name = name
 
         @aliases = attributes[:aliases] || []
         @description = attributes[:description] || ''
+        @warning = attributes[:warning] || ''
         @explanation = attributes[:explanation] || ''
         @params = attributes[:params] || []
         @condition_block = attributes[:condition_block]
@@ -33,11 +34,13 @@ module Gitlab
       def explain(context, arg)
         return unless available?(context)
 
-        if explanation.respond_to?(:call)
+        message = if explanation.respond_to?(:call)
-          execute_block(explanation, context, arg)
+                    execute_block(explanation, context, arg)
-        else
+                  else
-          explanation
+                    explanation
-        end
+                  end
+
+        warning.empty? ? message : "#{message} (#{warning})"
       end
 
       def execute(context, arg)
@@ -61,6 +64,7 @@ module Gitlab
           name: name,
           aliases: aliases,
           description: desc,
+          warning: warning,
           params: prms
         }
       end

lib/gitlab/quick_actions/dsl.rb

@@ -31,6 +31,10 @@ module Gitlab
           @description = block_given? ? block : text
         end
 
+        def warning(message = '')
+          @warning = message
+        end
+
         # Allows to define params for the next quick action.
         # These params are shown in the autocomplete menu.
         #
@@ -133,6 +137,7 @@ module Gitlab
             name,
             aliases: aliases,
             description: @description,
+            warning: @warning,
             explanation: @explanation,
             params: @params,
             condition_block: @condition_block,
@@ -150,6 +155,7 @@ module Gitlab
           @explanation = nil
           @params = nil
           @condition_block = nil
+          @warning = nil
           @parse_params_block = nil
         end
       end

spec/lib/gitlab/quick_actions/command_definition_spec.rb

@@ -210,6 +210,19 @@ describe Gitlab::QuickActions::CommandDefinition do
       end
     end
 
+    context 'when warning is set' do
+      before do
+        subject.explanation = 'Explanation'
+        subject.warning = 'dangerous!'
+      end
+
+      it 'returns this static string' do
+        result = subject.explain({}, nil)
+
+        expect(result).to eq 'Explanation (dangerous!)'
+      end
+    end
+
     context 'when the explanation is dynamic' do
       before do
         subject.explanation = proc { |arg| "Dynamic #{arg}" }

spec/lib/gitlab/quick_actions/dsl_spec.rb

@@ -12,6 +12,7 @@ describe Gitlab::QuickActions::Dsl do
 
       params 'The first argument'
       explanation 'Static explanation'
+      warning 'Possible problem!'
       command :explanation_with_aliases, :once, :first do |arg|
         arg
       end
@@ -64,6 +65,7 @@ describe Gitlab::QuickActions::Dsl do
       expect(no_args_def.condition_block).to be_nil
       expect(no_args_def.action_block).to be_a_kind_of(Proc)
       expect(no_args_def.parse_params_block).to be_nil
+      expect(no_args_def.warning).to eq('')
 
       expect(explanation_with_aliases_def.name).to eq(:explanation_with_aliases)
       expect(explanation_with_aliases_def.aliases).to eq([:once, :first])
@@ -73,6 +75,7 @@ describe Gitlab::QuickActions::Dsl do
       expect(explanation_with_aliases_def.condition_block).to be_nil
       expect(explanation_with_aliases_def.action_block).to be_a_kind_of(Proc)
       expect(explanation_with_aliases_def.parse_params_block).to be_nil
+      expect(explanation_with_aliases_def.warning).to eq('Possible problem!')
 
       expect(dynamic_description_def.name).to eq(:dynamic_description)
       expect(dynamic_description_def.aliases).to eq([])
@@ -82,6 +85,7 @@ describe Gitlab::QuickActions::Dsl do
       expect(dynamic_description_def.condition_block).to be_nil
       expect(dynamic_description_def.action_block).to be_a_kind_of(Proc)
       expect(dynamic_description_def.parse_params_block).to be_nil
+      expect(dynamic_description_def.warning).to eq('')
 
       expect(cc_def.name).to eq(:cc)
       expect(cc_def.aliases).to eq([])
@@ -91,6 +95,7 @@ describe Gitlab::QuickActions::Dsl do
       expect(cc_def.condition_block).to be_nil
       expect(cc_def.action_block).to be_nil
       expect(cc_def.parse_params_block).to be_nil
+      expect(cc_def.warning).to eq('')
 
       expect(cond_action_def.name).to eq(:cond_action)
       expect(cond_action_def.aliases).to eq([])
@@ -100,6 +105,7 @@ describe Gitlab::QuickActions::Dsl do
       expect(cond_action_def.condition_block).to be_a_kind_of(Proc)
       expect(cond_action_def.action_block).to be_a_kind_of(Proc)
       expect(cond_action_def.parse_params_block).to be_nil
+      expect(cond_action_def.warning).to eq('')
 
       expect(with_params_parsing_def.name).to eq(:with_params_parsing)
       expect(with_params_parsing_def.aliases).to eq([])
@@ -109,6 +115,7 @@ describe Gitlab::QuickActions::Dsl do
       expect(with_params_parsing_def.condition_block).to be_nil
       expect(with_params_parsing_def.action_block).to be_a_kind_of(Proc)
       expect(with_params_parsing_def.parse_params_block).to be_a_kind_of(Proc)
+      expect(with_params_parsing_def.warning).to eq('')
 
       expect(substitution_def.name).to eq(:something)
       expect(substitution_def.aliases).to eq([])
@@ -118,6 +125,7 @@ describe Gitlab::QuickActions::Dsl do
       expect(substitution_def.condition_block).to be_nil
       expect(substitution_def.action_block.call('text')).to eq('text Some complicated thing you want in here')
       expect(substitution_def.parse_params_block).to be_nil
+      expect(substitution_def.warning).to eq('')
     end
   end
 end

spec/services/issuable/clone/attributes_rewriter_spec.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Issuable::Clone::AttributesRewriter do
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+  let(:project1) { create(:project, :public, group: group) }
+  let(:project2) { create(:project, :public, group: group) }
+  let(:original_issue) { create(:issue, project: project1) }
+  let(:new_issue) { create(:issue, project: project2) }
+
+  subject { described_class.new(user, original_issue, new_issue) }
+
+  context 'setting labels' do
+    it 'sets labels present in the new project and group labels' do
+      project1_label_1 = create(:label, title: 'label1', project: project1)
+      project1_label_2 = create(:label, title: 'label2', project: project1)
+      project2_label_1 = create(:label, title: 'label1', project: project2)
+      group_label = create(:group_label, title: 'group_label', group: group)
+      create(:label, title: 'label3', project: project2)
+
+      original_issue.update(labels: [project1_label_1, project1_label_2, group_label])
+
+      subject.execute
+
+      expect(new_issue.reload.labels).to match_array([project2_label_1, group_label])
+    end
+
+    it 'does not set any labels when not used on the original issue' do
+      subject.execute
+
+      expect(new_issue.reload.labels).to be_empty
+    end
+
+    it 'copies the resource label events' do
+      resource_label_events = create_list(:resource_label_event, 2, issue: original_issue)
+
+      subject.execute
+
+      expected = resource_label_events.map(&:label_id)
+
+      expect(new_issue.resource_label_events.map(&:label_id)).to match_array(expected)
+    end
+  end
+
+  context 'setting milestones' do
+    it 'sets milestone to nil when old issue milestone is not in the new project' do
+      milestone = create(:milestone, title: 'milestone', project: project1)
+
+      original_issue.update(milestone: milestone)
+
+      subject.execute
+
+      expect(new_issue.reload.milestone).to be_nil
+    end
+
+    it 'copies the milestone when old issue milestone title is in the new project' do
+      milestone_project1 = create(:milestone, title: 'milestone', project: project1)
+      milestone_project2 = create(:milestone, title: 'milestone', project: project2)
+
+      original_issue.update(milestone: milestone_project1)
+
+      subject.execute
+
+      expect(new_issue.reload.milestone).to eq(milestone_project2)
+    end
+
+    it 'copies the milestone when old issue milestone is a group milestone' do
+      milestone = create(:milestone, title: 'milestone', group: group)
+
+      original_issue.update(milestone: milestone)
+
+      subject.execute
+
+      expect(new_issue.reload.milestone).to eq(milestone)
+    end
+  end
+end

spec/services/issuable/clone/content_rewriter_spec.rb

@@ -0,0 +1,153 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Issuable::Clone::ContentRewriter do
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+  let(:project1) { create(:project, :public, group: group) }
+  let(:project2) { create(:project, :public, group: group) }
+
+  let(:other_issue) { create(:issue, project: project1) }
+  let(:merge_request) { create(:merge_request) }
+
+  subject { described_class.new(user, original_issue, new_issue)}
+
+  let(:description) { 'Simple text' }
+  let(:original_issue) { create(:issue, description: description, project: project1) }
+  let(:new_issue) { create(:issue, project: project2) }
+
+  context 'rewriting award emojis' do
+    it 'copies the award emojis' do
+      create(:award_emoji, awardable: original_issue, name: 'thumbsup')
+      create(:award_emoji, awardable: original_issue, name: 'thumbsdown')
+
+      expect { subject.execute }.to change { AwardEmoji.count }.by(2)
+
+      expect(new_issue.award_emoji.map(&:name)).to match_array(%w(thumbsup thumbsdown))
+    end
+  end
+
+  context 'rewriting description' do
+    before do
+      subject.execute
+    end
+
+    context 'when description is a simple text' do
+      it 'does not rewrite the description' do
+        expect(new_issue.reload.description).to eq(original_issue.description)
+      end
+    end
+
+    context 'when description contains a local reference' do
+      let(:description) { "See ##{other_issue.iid}" }
+
+      it 'rewrites the local reference correctly' do
+        expected_description = "See #{project1.path}##{other_issue.iid}"
+
+        expect(new_issue.reload.description).to eq(expected_description)
+      end
+    end
+
+    context 'when description contains a cross reference' do
+      let(:description) { "See #{merge_request.project.full_path}!#{merge_request.iid}" }
+
+      it 'rewrites the cross reference correctly' do
+        expected_description = "See #{merge_request.project.full_path}!#{merge_request.iid}"
+
+        expect(new_issue.reload.description).to eq(expected_description)
+      end
+    end
+
+    context 'when description contains a user reference' do
+      let(:description) { "FYU #{user.to_reference}" }
+
+      it 'works with a user reference' do
+        expect(new_issue.reload.description).to eq("FYU #{user.to_reference}")
+      end
+    end
+
+    context 'when description contains uploads' do
+      let(:uploader) { build(:file_uploader, project: project1) }
+      let(:description) { "Text and #{uploader.markdown_link}" }
+
+      it 'rewrites uploads in the description' do
+        upload = Upload.last
+
+        expect(new_issue.description).not_to eq(description)
+        expect(new_issue.description).to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/)
+        expect(upload.secret).not_to eq(uploader.secret)
+        expect(new_issue.description).to include(upload.secret)
+        expect(new_issue.description).to include(upload.path)
+      end
+    end
+  end
+
+  context 'rewriting notes' do
+    context 'simple notes' do
+      let!(:notes) do
+        [
+          create(:note, noteable: original_issue, project: project1,
+                        created_at: 2.weeks.ago, updated_at: 1.week.ago),
+          create(:note, noteable: original_issue, project: project1),
+          create(:note, system: true, noteable: original_issue, project: project1)
+        ]
+      end
+      let!(:system_note_metadata) { create(:system_note_metadata, note: notes.last) }
+      let!(:award_emoji) { create(:award_emoji, awardable: notes.first, name: 'thumbsup')}
+
+      before do
+        subject.execute
+      end
+
+      it 'rewrites existing notes in valid order' do
+        expect(new_issue.notes.order('id ASC').pluck(:note).first(3)).to eq(notes.map(&:note))
+      end
+
+      it 'copies all the issue notes' do
+        expect(new_issue.notes.count).to eq(3)
+      end
+
+      it 'does not change the note attributes' do
+        subject.execute
+
+        new_note = new_issue.notes.first
+
+        expect(new_note.note).to eq(notes.first.note)
+        expect(new_note.author).to eq(notes.first.author)
+      end
+
+      it 'copies the award emojis' do
+        subject.execute
+
+        new_note = new_issue.notes.first
+        new_note.award_emoji.first.name = 'thumbsup'
+      end
+
+      it 'copies system_note_metadata for system note' do
+        new_note = new_issue.notes.last
+
+        expect(new_note.system_note_metadata.action).to eq(system_note_metadata.action)
+        expect(new_note.system_note_metadata.id).not_to eq(system_note_metadata.id)
+      end
+    end
+
+    context 'notes with reference' do
+      let(:text) do
+        "See ##{other_issue.iid} and #{merge_request.project.full_path}!#{merge_request.iid}"
+      end
+      let!(:note) { create(:note, noteable: original_issue, note: text, project: project1) }
+
+      it 'rewrites the references correctly' do
+        subject.execute
+
+        new_note = new_issue.notes.first
+
+        expected_text = "See #{other_issue.project.path}##{other_issue.iid} and #{merge_request.project.full_path}!#{merge_request.iid}"
+
+        expect(new_note.note).to eq(expected_text)
+        expect(new_note.author).to eq(note.author)
+      end
+    end
+  end
+end

spec/services/issues/move_service_spec.rb

@@ -10,11 +10,9 @@ describe Issues::MoveService do
   let(:sub_group_2) { create(:group, :private, parent: group) }
   let(:old_project) { create(:project, namespace: sub_group_1) }
   let(:new_project) { create(:project, namespace: sub_group_2) }
-  let(:milestone1) { create(:milestone, project_id: old_project.id, title: 'v9.0') }
 
   let(:old_issue) do
-    create(:issue, title: title, description: description,
+    create(:issue, title: title, description: description, project: old_project, author: author)
-                   project: old_project, author: author, milestone: milestone1)
   end
 
   subject(:move_service) do
@@ -25,16 +23,6 @@ describe Issues::MoveService do
     before do
       old_project.add_reporter(user)
       new_project.add_reporter(user)
-
-      labels = Array.new(2) { |x| "label%d" % (x + 1) }
-
-      labels.each do |label|
-        old_issue.labels << create(:label,
-          project_id: old_project.id,
-          title: label)
-
-        new_project.labels << create(:label, title: label)
-      end
     end
   end
 
@@ -48,91 +36,6 @@ describe Issues::MoveService do
     context 'issue movable' do
       include_context 'user can move issue'
 
-      context 'move to new milestone'  do
-        let(:new_issue) { move_service.execute(old_issue, new_project) }
-
-        context 'project milestone' do
-          let!(:milestone2) do
-            create(:milestone, project_id: new_project.id, title: 'v9.0')
-          end
-
-          it 'assigns milestone to new issue' do
-            expect(new_issue.reload.milestone.title).to eq 'v9.0'
-            expect(new_issue.reload.milestone).to eq(milestone2)
-          end
-        end
-
-        context 'group milestones' do
-          let!(:group) { create(:group, :private) }
-          let!(:group_milestone_1) do
-            create(:milestone, group_id: group.id, title: 'v9.0_group')
-          end
-
-          before do
-            old_issue.update(milestone: group_milestone_1)
-            old_project.update(namespace: group)
-            new_project.update(namespace: group)
-
-            group.add_users([user], GroupMember::DEVELOPER)
-          end
-
-          context 'when moving to a project of the same group' do
-            it 'keeps the same group milestone' do
-              expect(new_issue.reload.project).to eq(new_project)
-              expect(new_issue.reload.milestone).to eq(group_milestone_1)
-            end
-          end
-
-          context 'when moving to a project of a different group' do
-            let!(:group_2) { create(:group, :private) }
-
-            let!(:group_milestone_2) do
-              create(:milestone, group_id: group_2.id, title: 'v9.0_group')
-            end
-
-            before do
-              old_issue.update(milestone: group_milestone_1)
-              new_project.update(namespace: group_2)
-
-              group_2.add_users([user], GroupMember::DEVELOPER)
-            end
-
-            it 'assigns to new group milestone of same title' do
-              expect(new_issue.reload.project).to eq(new_project)
-              expect(new_issue.reload.milestone).to eq(group_milestone_2)
-            end
-          end
-        end
-      end
-
-      context 'issue with group labels', :nested_groups do
-        it 'assigns group labels to new issue' do
-          label = create(:group_label, group: group)
-          label_issue = create(:labeled_issue, description: description, project: old_project,
-                                               milestone: milestone1, labels: [label])
-          old_project.add_reporter(user)
-          new_project.add_reporter(user)
-
-          new_issue = move_service.execute(label_issue, new_project)
-
-          expect(new_issue).to have_attributes(
-            project: new_project,
-            labels: include(label)
-          )
-        end
-      end
-
-      context 'issue with resource label events' do
-        it 'assigns resource label events to new issue' do
-          old_issue.resource_label_events = create_list(:resource_label_event, 2, issue: old_issue)
-
-          new_issue = move_service.execute(old_issue, new_project)
-
-          expected = old_issue.resource_label_events.map(&:label_id)
-          expect(new_issue.resource_label_events.map(&:label_id)).to match_array(expected)
-        end
-      end
-
       context 'generic issue' do
         include_context 'issue move executed'
 
@@ -140,18 +43,6 @@ describe Issues::MoveService do
           expect(new_issue.project).to eq new_project
         end
 
-        it 'assign labels to new issue' do
-          expected_label_titles = new_issue.reload.labels.map(&:title)
-          expect(expected_label_titles).to include 'label1'
-          expect(expected_label_titles).to include 'label2'
-          expect(expected_label_titles.size).to eq 2
-
-          new_issue.labels.each do |label|
-            expect(new_project.labels).to include(label)
-            expect(old_project.labels).not_to include(label)
-          end
-        end
-
         it 'rewrites issue title' do
           expect(new_issue.title).to eq title
         end
@@ -203,140 +94,25 @@ describe Issues::MoveService do
         end
       end
 
-      context 'issue with notes' do
+      context 'issue with assignee' do
-        context 'notes without references' do
+        let(:assignee) { create(:user) }
-          let(:notes_params) do
-            [{ system: false, note: 'Some comment 1' },
-             { system: true, note: 'Some system note' },
-             { system: false, note: 'Some comment 2' }]
-          end
-          let(:award_names) { %w(thumbsup thumbsdown facepalm) }
-          let(:notes_contents) { notes_params.map { |n| n[:note] } }
 
-          before do
+        before do
-            note_params = { noteable: old_issue, project: old_project, author: author }
+          old_issue.assignees = [assignee]
-            notes_params.each_with_index do |note, index|
-              new_note = create(:note, note_params.merge(note))
-              award_emoji_params = { awardable: new_note, name: award_names[index] }
-              create(:award_emoji, award_emoji_params)
-            end
-          end
-
-          include_context 'issue move executed'
-
-          let(:all_notes) { new_issue.notes.order('id ASC') }
-          let(:system_notes) { all_notes.system }
-          let(:user_notes) { all_notes.user }
-
-          it 'rewrites existing notes in valid order' do
-            expect(all_notes.pluck(:note).first(3)).to eq notes_contents
-          end
-
-          it 'creates new emojis for the new notes' do
-            expect(all_notes.map(&:award_emoji).to_a.flatten.map(&:name)).to eq award_names
-          end
-
-          it 'adds a system note about move after rewritten notes' do
-            expect(system_notes.last.note).to match /^moved from/
-          end
-
-          it 'preserves orignal author of comment' do
-            expect(user_notes.pluck(:author_id)).to all(eq(author.id))
-          end
         end
 
-        context 'note that has been updated' do
+        it 'preserves assignee with access to the new issue' do
-          let!(:note) do
+          new_project.add_reporter(assignee)
-            create(:note, noteable: old_issue, project: old_project,
-                          author: author, updated_at: Date.yesterday,
-                          created_at: Date.yesterday)
-          end
 
-          include_context 'issue move executed'
+          new_issue = move_service.execute(old_issue, new_project)
 
-          it 'preserves time when note has been created at' do
+          expect(new_issue.assignees).to eq([assignee])
-            expect(new_issue.notes.first.created_at).to eq note.created_at
-          end
-
-          it 'preserves time when note has been updated at' do
-            expect(new_issue.notes.first.updated_at).to eq note.updated_at
-          end
         end
 
-        context 'issue with assignee' do
+        it 'ignores assignee without access to the new issue' do
-          let(:assignee) { create(:user) }
+          new_issue = move_service.execute(old_issue, new_project)
 
-          before do
+          expect(new_issue.assignees).to be_empty
-            old_issue.assignees = [assignee]
-          end
-
-          it 'preserves assignee with access to the new issue' do
-            new_project.add_reporter(assignee)
-
-            new_issue = move_service.execute(old_issue, new_project)
-
-            expect(new_issue.assignees).to eq([assignee])
-          end
-
-          it 'ignores assignee without access to the new issue' do
-            new_issue = move_service.execute(old_issue, new_project)
-
-            expect(new_issue.assignees).to be_empty
-          end
-        end
-
-        context 'notes with references' do
-          before do
-            create(:merge_request, source_project: old_project)
-            create(:note, noteable: old_issue, project: old_project, author: author,
-                          note: 'Note with reference to merge request !1')
-          end
-
-          include_context 'issue move executed'
-          let(:new_note) { new_issue.notes.first }
-
-          it 'rewrites references using a cross reference to old project' do
-            expect(new_note.note)
-              .to eq "Note with reference to merge request #{old_project.to_reference(new_project)}!1"
-          end
-        end
-
-        context 'issue description with uploads' do
-          let(:uploader) { build(:file_uploader, project: old_project) }
-          let(:description) { "Text and #{uploader.markdown_link}" }
-
-          include_context 'issue move executed'
-
-          it 'rewrites uploads in description' do
-            expect(new_issue.description).not_to eq description
-            expect(new_issue.description)
-              .to match(/Text and #{FileUploader::MARKDOWN_PATTERN}/)
-            expect(new_issue.description).not_to include uploader.secret
-          end
-        end
-      end
-
-      describe 'rewriting references' do
-        include_context 'issue move executed'
-
-        context 'issue references' do
-          let(:another_issue) { create(:issue, project: old_project) }
-          let(:description) { "Some description #{another_issue.to_reference}" }
-
-          it 'rewrites referenced issues creating cross project reference' do
-            expect(new_issue.description)
-              .to eq "Some description #{another_issue.to_reference(new_project)}"
-          end
-        end
-
-        context "user references" do
-          let(:another_issue) { create(:issue, project: old_project) }
-          let(:description) { "Some description #{user.to_reference}" }
-
-          it "doesn't throw any errors for issues containing user references" do
-            expect(new_issue.description)
-              .to eq "Some description #{user.to_reference}"
-          end
         end
       end
 
@@ -416,25 +192,5 @@ describe Issues::MoveService do
         it { expect { move }.to raise_error(StandardError, /permissions/) }
       end
     end
-
-    context 'movable issue with no assigned labels' do
-      before do
-        old_project.add_reporter(user)
-        new_project.add_reporter(user)
-
-        labels = Array.new(2) { |x| "label%d" % (x + 1) }
-
-        labels.each do |label|
-          new_project.labels << create(:label, title: label)
-        end
-      end
-
-      include_context 'issue move executed'
-
-      it 'does not assign labels to new issue' do
-        expected_label_titles = new_issue.reload.labels.map(&:title)
-        expect(expected_label_titles.size).to eq 0
-      end
-    end
   end
 end

spec/uploaders/file_uploader_spec.rb

@@ -81,19 +81,24 @@ describe FileUploader do
   end
 
   describe 'copy_to' do
+    let(:new_project) { create(:project) }
+    let(:moved) { described_class.copy_to(subject, new_project) }
+
     shared_examples 'returns a valid uploader' do
       describe 'returned uploader' do
-        let(:new_project) { create(:project) }
-        let(:moved) { described_class.copy_to(subject, new_project) }
-
         it 'generates a new secret' do
           expect(subject).to be
           expect(described_class).to receive(:generate_secret).once.and_call_original
           expect(moved).to be
         end
 
-        it 'create new upload' do
+        it 'creates new upload correctly' do
-          expect(moved.upload).not_to eq(subject.upload)
+          upload = moved.upload
+
+          expect(upload).not_to eq(subject.upload)
+          expect(upload.model).to eq(new_project)
+          expect(upload.uploader).to eq('FileUploader')
+          expect(upload.secret).not_to eq(subject.upload.secret)
         end
 
         it 'copies the file' do
@@ -111,6 +116,12 @@ describe FileUploader do
       end
 
       include_examples 'returns a valid uploader'
+
+      it 'copies the file to the correct location' do
+        expect(moved.upload.path).to eq("#{moved.upload.secret}/dk.png")
+        expect(moved.file.path).to end_with("public/uploads/#{new_project.disk_path}/#{moved.upload.secret}/dk.png")
+        expect(moved.filename).to eq('dk.png')
+      end
     end
 
     context 'files are stored remotely' do
@@ -121,6 +132,12 @@ describe FileUploader do
       end
 
       include_examples 'returns a valid uploader'
+
+      it 'copies the file to the correct location' do
+        expect(moved.upload.path).to eq("#{new_project.disk_path}/#{moved.upload.secret}/dk.png")
+        expect(moved.file.path).to eq("#{new_project.disk_path}/#{moved.upload.secret}/dk.png")
+        expect(moved.filename).to eq('dk.png')
+      end
     end
   end
 

spec/uploaders/namespace_file_uploader_spec.rb

@@ -55,4 +55,62 @@ describe NamespaceFileUploader do
     it_behaves_like "migrates", to_store: described_class::Store::REMOTE
     it_behaves_like "migrates", from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL
   end
+
+  describe 'copy_to' do
+    let(:group) { create(:group) }
+    let(:moved) { described_class.copy_to(subject, group) }
+
+    shared_examples 'returns a valid uploader' do
+      it 'generates a new secret' do
+        expect(subject).to be
+        expect(described_class).to receive(:generate_secret).once.and_call_original
+        expect(moved).to be
+      end
+
+      it 'creates new upload correctly' do
+        upload = moved.upload
+
+        expect(upload).not_to eq(subject.upload)
+        expect(upload.model).to eq(group)
+        expect(upload.uploader).to eq('NamespaceFileUploader')
+        expect(upload.secret).not_to eq(subject.upload.secret)
+      end
+
+      it 'copies the file' do
+        expect(subject.file).to exist
+        expect(moved.file).to exist
+        expect(subject.file).not_to eq(moved.file)
+        expect(subject.object_store).to eq(moved.object_store)
+      end
+    end
+
+    context 'files are stored locally' do
+      before do
+        subject.store!(fixture_file_upload('spec/fixtures/dk.png'))
+      end
+
+      include_examples 'returns a valid uploader'
+
+      it 'copies the file to the correct location' do
+        expect(moved.upload.path).to eq("#{moved.upload.secret}/dk.png")
+        expect(moved.file.path).to end_with("system/namespace/#{group.id}/#{moved.upload.secret}/dk.png")
+        expect(moved.filename).to eq('dk.png')
+      end
+    end
+
+    context 'files are stored remotely' do
+      before do
+        stub_uploads_object_storage
+        subject.store!(fixture_file_upload('spec/fixtures/dk.png'))
+        subject.migrate!(ObjectStorage::Store::REMOTE)
+      end
+
+      include_examples 'returns a valid uploader'
+
+      it 'copies the file to the correct location' do
+        expect(moved.file.path).to eq("namespace/#{group.id}/#{moved.upload.secret}/dk.png")
+        expect(moved.filename).to eq('dk.png')
+      end
+    end
+  end
 end