Merge branch 'feature/issue-move' into 'master'

Ability to move issue to another project

Tasks:

- [x] Create scaffold of service that will move issue to another project.
- [x] Close old issue, add system note about moving issue to a new project.
- [x] Create a new issue, add system note about issue being moved from old project.
- [x] Check if issue can be moved to another project before executing service
- [x] Check permissions when moving an issue (`:admin_issue` ability)
- [x] Display select box for a new project when editing an issue
- [x] Show only projects that issue can be moved into in that select box
- [x] Add project select handler, helper and some permission filters to it
- [x] Preserve as much information as possible, including author
- [x] Prepare mechanisms that unfolds local references in issue description
- [x] Rewrite issue description with references unfolding and add some specs for it
- [x] Rewrite all system notes and comments attached to issue that is being moved
- [x] Update `Label` so that is was able to create cross reference labels (separate MR)
- [x] Add notifications about moving issue to another project
- [x] Display confirmation alert/message when issue move has been requested
- [x] Make it possible to undo selecting project where issue will be moved to
- [x] Add column to issue, that will indicate if it has been moved to another project
- [x] Do not allow to move issue that has been already moved
- [x] Write top-to-bottom feature spec in RSpec instead of Spinach

UI:

![issue_move_ui](/uploads/b3c6b563362c1fded9082cc0f51e5a74/issue_move_ui.png)

![issue_move_tooltip](/uploads/2ab913b06f52df1cafde9abe89bd9cb8/issue_move_tooltip.png)

Closes #3024

See merge request !2831

7 files changed, 539 insertions, 0 deletions

diff --git a/spec/features/issues/move_spec.rb b/spec/features/issues/move_spec.rb
new file mode 100644
index 00000000000..6fda0c31866
--- /dev/null
+++ b/spec/features/issues/move_spec.rb
@@ -0,0 +1,87 @@
+require 'rails_helper'
+
+feature 'issue move to another project' do
+  let(:user) { create(:user) }
+  let(:old_project) { create(:project) }
+  let(:text) { 'Some issue description' }
+
+  let(:issue) do
+    create(:issue, description: text, project: old_project, author: user)
+  end
+
+  background { login_as(user) }
+
+  context 'user does not have permission to move issue' do
+    background do
+      old_project.team << [user, :guest]
+
+      edit_issue(issue)
+    end
+
+    scenario 'moving issue to another project not allowed' do
+      expect(page).to have_no_select('move_to_project_id')
+    end
+  end
+
+  context 'user has permission to move issue' do
+    let!(:mr) { create(:merge_request, source_project: old_project) }
+    let(:new_project) { create(:project) }
+    let(:text) { 'Text with !1' }
+    let(:cross_reference) { old_project.to_reference }
+
+    background do
+      old_project.team << [user, :reporter]
+      new_project.team << [user, :reporter]
+
+      edit_issue(issue)
+    end
+
+    scenario 'moving issue to another project' do
+      select(new_project.name_with_namespace, from: 'move_to_project_id')
+      click_button('Save changes')
+
+      expect(current_url).to include project_path(new_project)
+
+      page.within('.issue') do
+        expect(page).to have_content("Text with #{cross_reference}!1")
+        expect(page).to have_content("Moved from #{cross_reference}#1")
+        expect(page).to have_content(issue.title)
+      end
+    end
+
+    context 'projects user does not have permission to move issue to exist' do
+      let!(:private_project) { create(:project, :private) }
+      let(:another_project) { create(:project) }
+      background { another_project.team << [user, :guest] }
+
+      scenario 'browsing projects in projects select' do
+        options = [ '', 'No project', new_project.name_with_namespace ]
+        expect(page).to have_select('move_to_project_id', options: options)
+      end
+    end
+
+    context 'issue has been already moved' do
+      let(:new_issue) { create(:issue, project: new_project) }
+      let(:issue) do
+        create(:issue, project: old_project, author: user, moved_to: new_issue)
+      end
+
+      scenario 'user wants to move issue that has already been moved' do
+        expect(page).to have_no_select('move_to_project_id')
+      end
+    end
+  end
+
+  def edit_issue(issue)
+    visit issue_path(issue)
+    page.within('.issuable-header') { click_link 'Edit' }
+  end
+
+  def issue_path(issue)
+    namespace_project_issue_path(issue.project.namespace, issue.project, issue)
+  end
+
+  def project_path(project)
+    namespace_project_path(new_project.namespace, new_project)
+  end
+end
diff --git a/spec/lib/gitlab/gfm/reference_rewriter_spec.rb b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
new file mode 100644
index 00000000000..0a7ca3ec848
--- /dev/null
+++ b/spec/lib/gitlab/gfm/reference_rewriter_spec.rb
@@ -0,0 +1,89 @@
+require 'spec_helper'
+
+describe Gitlab::Gfm::ReferenceRewriter do
+  let(:text) { 'some text' }
+  let(:old_project) { create(:project) }
+  let(:new_project) { create(:project) }
+  let(:user) { create(:user) }
+
+  before { old_project.team << [user, :guest] }
+
+  describe '#rewrite' do
+    subject do
+      described_class.new(text, old_project, user).rewrite(new_project)
+    end
+
+    context 'multiple issues and merge requests referenced' do
+      let!(:issue_first) { create(:issue, project: old_project) }
+      let!(:issue_second) { create(:issue, project: old_project) }
+      let!(:merge_request) { create(:merge_request, source_project: old_project) }
+
+      context 'plain text description' do
+        let(:text) { 'Description that references #1, #2 and !1' }
+
+        it { is_expected.to include issue_first.to_reference(new_project) }
+        it { is_expected.to include issue_second.to_reference(new_project) }
+        it { is_expected.to include merge_request.to_reference(new_project) }
+      end
+
+      context 'description with ignored elements' do
+        let(:text) do
+          "Hi. This references #1, but not `#2`\n" +
+          '<pre>and not !1</pre>'
+        end
+
+        it { is_expected.to include issue_first.to_reference(new_project) }
+        it { is_expected.to_not include issue_second.to_reference(new_project) }
+        it { is_expected.to_not include merge_request.to_reference(new_project) }
+      end
+
+      context 'description ambigous elements' do
+        context 'url' do
+          let(:url) { 'http://gitlab.com/#1' }
+          let(:text) { "This references #1, but not #{url}" }
+
+          it { is_expected.to include url }
+        end
+
+        context 'code' do
+          let(:text) { "#1, but not `[#1]`" }
+          it { is_expected.to eq "#{issue_first.to_reference(new_project)}, but not `[#1]`" }
+        end
+
+        context 'code reverse' do
+          let(:text) { "not `#1`, but #1" }
+          it { is_expected.to eq "not `#1`, but #{issue_first.to_reference(new_project)}" }
+        end
+
+        context 'code in random order' do
+          let(:text) { "#1, `#1`, #1, `#1`" }
+          let(:ref) { issue_first.to_reference(new_project) }
+
+          it { is_expected.to eq "#{ref}, `#1`, #{ref}, `#1`" }
+        end
+
+        context 'description with labels' do
+          let!(:label) { create(:label, id: 123, name: 'test', project: old_project) }
+          let(:project_ref) { old_project.to_reference }
+
+          context 'label referenced by id' do
+            let(:text) { '#1 and ~123' }
+            it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~123} }
+          end
+
+          context 'label referenced by text' do
+            let(:text) { '#1 and ~"test"' }
+            it { is_expected.to eq %Q{#{project_ref}#1 and #{project_ref}~123} }
+          end
+        end
+      end
+
+      context 'reference contains milestone' do
+        let(:milestone) { create(:milestone) }
+        let(:text) { "milestone ref: #{milestone.to_reference}" }
+
+        it { is_expected.to eq text }
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index 65af37e24f1..7c617723e6d 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -124,4 +124,24 @@ describe Gitlab::ReferenceExtractor, lib: true do
       expect(extracted).to match_array([issue])
     end
   end
+
+  describe '#all' do
+    let(:issue) { create(:issue, project: project) }
+    let(:label) { create(:label, project: project) }
+    let(:text) { "Ref. #{issue.to_reference} and #{label.to_reference}" }
+
+    before do
+      project.team << [project.creator, :developer]
+      subject.analyze(text)
+    end
+
+    it 'returns all referables' do
+      expect(subject.all).to match_array([issue, label])
+    end
+  end
+
+  describe '.references_pattern' do
+    subject { described_class.references_pattern }
+    it { is_expected.to be_kind_of Regexp }
+  end
 end
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index f910424d85b..9b47acfe0cd 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -158,6 +158,33 @@ describe Notify do
             is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
           end
         end
+
+        describe 'moved to another project' do
+          let(:new_issue) { create(:issue) }
+          subject { Notify.issue_moved_email(recipient, issue, new_issue, current_user) }
+
+          it_behaves_like 'an answer to an existing thread', 'issue'
+          it_behaves_like 'it should show Gmail Actions View Issue link'
+          it_behaves_like 'an unsubscribeable thread'
+
+          it 'contains description about action taken' do
+            is_expected.to have_body_text 'Issue was moved to another project'
+          end
+
+          it 'has the correct subject' do
+            is_expected.to have_subject /#{issue.title} \(##{issue.iid}\)/i
+          end
+
+          it 'contains link to new issue' do
+            new_issue_url = namespace_project_issue_path(new_issue.project.namespace,
+                                                         new_issue.project, new_issue)
+            is_expected.to have_body_text new_issue_url
+          end
+
+          it 'contains a link to the original issue' do
+            is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
+          end
+        end
       end
 
       context 'for merge requests' do
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 540a62eb1f8..05e4233243d 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -130,6 +130,56 @@ describe Issue, models: true do
     end
   end
 
+  describe '#can_move?' do
+    let(:user) { create(:user) }
+    let(:issue) { create(:issue) }
+    subject { issue.can_move?(user) }
+
+    context 'user is not a member of project issue belongs to' do
+      it { is_expected.to eq false}
+    end
+
+    context 'user is reporter in project issue belongs to' do
+      let(:project) { create(:project) }
+      let(:issue) { create(:issue, project: project) }
+
+      before { project.team << [user, :reporter] }
+
+      it { is_expected.to eq true }
+
+      context 'checking destination project also' do
+        subject { issue.can_move?(user, to_project) }
+        let(:to_project) { create(:project) }
+
+        context 'destination project allowed' do
+          before { to_project.team << [user, :reporter] }
+          it { is_expected.to eq true }
+        end
+
+        context 'destination project not allowed' do
+          before { to_project.team << [user, :guest] }
+          it { is_expected.to eq false }
+        end
+      end
+    end
+  end
+
+  describe '#moved?' do
+    let(:issue) { create(:issue) }
+    subject { issue.moved? }
+
+    context 'issue not moved' do
+      it { is_expected.to eq false }
+    end
+
+    context 'issue already moved' do
+      let(:moved_to_issue) { create(:issue) }
+      let(:issue) { create(:issue, moved_to: moved_to_issue) }
+
+      it { is_expected.to eq true }
+    end
+  end
+
   describe '#related_branches' do
     it "selects the right branches" do
       allow(subject.project.repository).to receive(:branch_names).
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
new file mode 100644
index 00000000000..14cc20e529a
--- /dev/null
+++ b/spec/services/issues/move_service_spec.rb
@@ -0,0 +1,213 @@
+require 'spec_helper'
+
+describe Issues::MoveService, services: true do
+  let(:user) { create(:user) }
+  let(:author) { create(:user) }
+  let(:title) { 'Some issue' }
+  let(:description) { 'Some issue description' }
+  let(:old_project) { create(:project) }
+  let(:new_project) { create(:project) }
+
+  let(:old_issue) do
+    create(:issue, title: title, description: description,
+                   project: old_project, author: author)
+  end
+
+  let(:move_service) do
+    described_class.new(old_project, user)
+  end
+
+  shared_context 'user can move issue' do
+    before do
+      old_project.team << [user, :reporter]
+      new_project.team << [user, :reporter]
+    end
+  end
+
+  describe '#execute' do
+    shared_context 'issue move executed' do
+      let!(:new_issue) { move_service.execute(old_issue, new_project) }
+    end
+
+    context 'issue movable' do
+      include_context 'user can move issue'
+
+      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 eq 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
+
+        it 'closes old issue' do
+          expect(old_issue.closed?).to be true
+        end
+
+        it 'persists new issue' do
+          expect(new_issue.persisted?).to be true
+        end
+
+        it 'persists all changes' do
+          expect(old_issue.changed?).to be false
+          expect(new_issue.changed?).to be false
+        end
+
+        it 'preserves author' do
+          expect(new_issue.author).to eq author
+        end
+
+        it 'removes data that is invalid in new context' do
+          expect(new_issue.milestone).to be_nil
+          expect(new_issue.labels).to be_empty
+        end
+
+        it 'creates a new internal id for issue' do
+          expect(new_issue.iid).to be 1
+        end
+
+        it 'marks issue as moved' do
+          expect(old_issue.moved?).to eq true
+          expect(old_issue.moved_to).to eq new_issue
+        end
+      end
+
+      context 'issue with notes' do
+        context 'notes without references' do
+          let(:notes_params) do
+            [{ system: false, note: 'Some comment 1' },
+             { system: true, note: 'Some system note' },
+             { system: false, note: 'Some comment 2' }]
+          end
+
+          let(:notes_contents) { notes_params.map { |n| n[:note] } }
+
+          before do
+            note_params = { noteable: old_issue, project: old_project, author: author }
+            notes_params.each do |note|
+              create(:note, note_params.merge(note))
+            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 '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
+
+          it 'preserves time when note has been created at' do
+            expect(old_issue.notes.first.created_at)
+              .to eq new_issue.notes.first.created_at
+          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}!1"
+          end
+        end
+      end
+
+      describe 'rewritting references' do
+        include_context 'issue move executed'
+
+        context 'issue reference' 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 #{old_project.to_reference}#{another_issue.to_reference}"
+          end
+        end
+      end
+
+      context 'moving to same project' do
+        let(:new_project) { old_project }
+
+        it 'raises error' do
+          expect { move_service.execute(old_issue, new_project) }
+            .to raise_error(StandardError, /Cannot move issue/)
+        end
+      end
+    end
+
+    describe 'move permissions' do
+      let(:move) { move_service.execute(old_issue, new_project) }
+
+      context 'user is reporter in both projects' do
+        include_context 'user can move issue'
+        it { expect { move }.to_not raise_error }
+      end
+
+      context 'user is reporter only in new project' do
+        before { new_project.team << [user, :reporter] }
+        it { expect { move }.to raise_error(StandardError, /permissions/) }
+      end
+
+      context 'user is reporter only in old project' do
+        before { old_project.team << [user, :reporter] }
+        it { expect { move }.to raise_error(StandardError, /permissions/) }
+      end
+
+      context 'user is reporter in one project and guest in another' do
+        before do
+          new_project.team << [user, :guest]
+          old_project.team << [user, :reporter]
+        end
+
+        it { expect { move }.to raise_error(StandardError, /permissions/) }
+      end
+
+      context 'issue has already been moved' do
+        include_context 'user can move issue'
+
+        let(:moved_to_issue) { create(:issue) }
+
+        let(:old_issue) do
+          create(:issue, project: old_project, author: author,
+                         moved_to: moved_to_issue)
+        end
+
+        it { expect { move }.to raise_error(StandardError, /permissions/) }
+      end
+    end
+  end
+end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 8e6292014d4..240eae10052 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -453,6 +453,59 @@ describe SystemNoteService, services: true do
     end
   end
 
+  describe '.noteable_moved' do
+    let(:new_project) { create(:project) }
+    let(:new_noteable) { create(:issue, project: new_project) }
+
+    subject do
+      described_class.noteable_moved(noteable, project, new_noteable, author, direction: direction)
+    end
+
+    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 noteable being moved to' do
+        expect(subject.note).to match /Moved to/
+      end
+    end
+
+    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
+
+    context 'invalid direction' do
+      let(:direction) { :invalid }
+
+      it 'should raise error' do
+        expect { subject }.to raise_error StandardError, /Invalid direction/
+      end
+    end
+  end
+
   include JiraServiceHelper
 
   describe 'JIRA integration' do