Delegate a single discussion to a new issue

Delegate a discussion in a merge request into a new issue.
The discussion wil be marked as resolved and a system note will be
added linking to the newly created issue.

3 files changed, 200 insertions, 36 deletions

diff --git a/spec/services/issues/base_service_spec.rb b/spec/services/issues/base_service_spec.rb
new file mode 100644
index 00000000000..bfeefe3bcc5
--- /dev/null
+++ b/spec/services/issues/base_service_spec.rb
@@ -0,0 +1,112 @@
+require 'spec_helper.rb'
+
+describe Issues::BaseService, services: true do
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+
+  before do
+    project.team << [user, :developer]
+  end
+
+  describe "for resolving discussions" do
+    let(:discussion) { Discussion.new([create(:diff_note_on_merge_request, project: project, note: "Almost done")]) }
+    let(:merge_request) { discussion.noteable }
+    let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "other") }
+
+    describe "#for_single_discussion" do
+      it "is true when only a discussion is passed" do
+        service = described_class.new(project, user, discussion_to_resolve: discussion)
+
+        expect(service.for_single_discussion?).to be_truthy
+      end
+
+      it "is true when matching merge request and discussion are passed" do
+        service = described_class.new(
+          project,
+          user,
+          discussion_to_resolve: discussion,
+          merge_request_for_resolving_discussions: merge_request
+        )
+
+        expect(service.for_single_discussion?).to be_truthy
+      end
+
+      it "is false when a discussion and another merge request are passed" do
+        service = described_class.new(
+          project,
+          user,
+          discussion_to_resolve: discussion,
+          merge_request_for_resolving_discussions: other_merge_request
+        )
+
+        expect(service.for_single_discussion?).to be_falsy
+      end
+    end
+
+    describe "#for_all_discussions_in_a_merge_request" do
+      it "is true when only a merge request is passed" do
+        service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request)
+
+        expect(service.for_all_discussions_in_a_merge_request?).to be_truthy
+      end
+
+      it "is false when matching merge request and discussion are passed" do
+        service = described_class.new(
+          project,
+          user,
+          discussion_to_resolve: discussion,
+          merge_request_for_resolving_discussions: merge_request
+        )
+
+        expect(service.for_all_discussions_in_a_merge_request?).to be_falsy
+      end
+    end
+
+    describe "#discussions_to_resolve" do
+      it "only contains a single discussion when only a discussion is passed" do
+        service = described_class.new(project, user, discussion_to_resolve: discussion)
+
+        expect(service.discussions_to_resolve).to contain_exactly(discussion)
+      end
+
+      it "is contains a single discussion when matching merge request and discussion are passed" do
+        service = described_class.new(
+          project,
+          user,
+          discussion_to_resolve: discussion,
+          merge_request_for_resolving_discussions: merge_request
+        )
+
+        expect(service.discussions_to_resolve).to contain_exactly(discussion)
+      end
+
+      it "contains all discussions when only a merge request is passed" do
+        second_discussion = Discussion.new([create(:diff_note_on_merge_request,
+                                                  noteable: merge_request,
+                                                  project: merge_request.target_project,
+                                                  line_number: 15)])
+        service = described_class.new(
+          project,
+          user,
+          merge_request_for_resolving_discussions: merge_request
+        )
+        # We need to compare discussion id's because the Discussion-objects are rebuilt
+        # which causes the object-id's not to be different.
+        discussion_ids = service.discussions_to_resolve.map(&:id)
+
+        expect(discussion_ids).to contain_exactly(discussion.id, second_discussion.id)
+      end
+
+      it "is empty when a discussion and another merge request are passed" do
+        service = described_class.new(
+          project,
+          user,
+          discussion_to_resolve: discussion,
+          merge_request_for_resolving_discussions: other_merge_request
+        )
+
+        expect(service.discussions_to_resolve).to be_empty
+      end
+    end
+  end
+end
diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb
index 09807e5d35b..cca7cdbdd2d 100644
--- a/spec/services/issues/build_service_spec.rb
+++ b/spec/services/issues/build_service_spec.rb
@@ -8,23 +8,36 @@ describe Issues::BuildService, services: true do
     project.team << [user, :developer]
   end
 
+  context 'for a single discussion' do
+    describe '#execute' do
+      it 'references the noteable title in the issue title' do
+        merge_request = create(:merge_request, title: "Hello world", source_project: project)
+        discussion = Discussion.new([create(:note_on_merge_request, project: project, noteable: merge_request)])
+        service = described_class.new(project, user, discussion_to_resolve: discussion)
+
+        issue = service.execute
+
+        expect(issue.title).to include('Hello world')
+      end
+
+      it 'adds the note content to the description' do
+        discussion = Discussion.new([create(:note_on_merge_request, project: project, note: "Almost done")])
+        service = described_class.new(project, user, discussion_to_resolve: discussion)
+
+        issue = service.execute
+
+        expect(issue.description).to include('Almost done')
+      end
+    end
+  end
+
   context 'for discussions in a merge request' do
     let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
     let(:issue) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute }
 
-    def position_on_line(line_number)
-      Gitlab::Diff::Position.new(
-        old_path: "files/ruby/popen.rb",
-        new_path: "files/ruby/popen.rb",
-        old_line: nil,
-        new_line: line_number,
-        diff_refs: merge_request.diff_refs
-      )
-    end
-
     describe '#items_for_discussions' do
       it 'has an item for each discussion' do
-        create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, position: position_on_line(13))
+        create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, line_number: 13)
         service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request)
 
         service.execute
@@ -82,7 +95,7 @@ describe Issues::BuildService, services: true do
 
       describe 'with multiple discussions' do
         before do
-          create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, position: position_on_line(15))
+          create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, line_number: 15)
         end
 
         it 'mentions all the authors in the description' do
@@ -99,7 +112,7 @@ describe Issues::BuildService, services: true do
         end
 
         it 'mentions additional notes' do
-          create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, position: position_on_line(15))
+          create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, line_number: 15)
 
           expect(issue.description).to include('(+2 comments)')
         end
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 6045d00ff09..4a5780f257a 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -140,46 +140,85 @@ describe Issues::CreateService, services: true do
 
     it_behaves_like 'new issuable record that supports slash commands'
 
-    context 'for a merge request' do
+    context 'resolving discussions' do
       let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
       let(:merge_request) { discussion.noteable }
       let(:project) { merge_request.source_project }
-      let(:opts) { { merge_request_for_resolving_discussions: merge_request } }
 
       before do
         project.team << [user, :master]
       end
 
-      it 'resolves the discussion for the merge request' do
-        described_class.new(project, user, opts).execute
-        discussion.first_note.reload
+      describe 'for a single discussion' do
+        let(:opts) { { discussion_to_resolve: discussion } }
 
-        expect(discussion.resolved?).to be(true)
-      end
+        it 'resolves the discussion' do
+          described_class.new(project, user, opts).execute
+          discussion.first_note.reload
 
-      it 'added a system note to the discussion' do
-        described_class.new(project, user, opts).execute
+          expect(discussion.resolved?).to be(true)
+        end
 
-        reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first
+        it 'added a system note to the discussion' do
+          described_class.new(project, user, opts).execute
 
-        expect(reloaded_discussion.last_note.system).to eq(true)
-      end
+          reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first
+
+          expect(reloaded_discussion.last_note.system).to eq(true)
+        end
+
+        it 'assigns the title and description for the issue' do
+          issue = described_class.new(project, user, opts).execute
+
+          expect(issue.title).not_to be_nil
+          expect(issue.description).not_to be_nil
+        end
 
-      it 'assigns the title and description for the issue' do
-        issue = described_class.new(project, user, opts).execute
+        it 'can set nil explicitly to the title and description' do
+          issue = described_class.new(project, user,
+                                      merge_request_for_resolving_discussions: merge_request,
+                                      description: nil,
+                                      title: nil).execute
 
-        expect(issue.title).not_to be_nil
-        expect(issue.description).not_to be_nil
+          expect(issue.description).to be_nil
+          expect(issue.title).to be_nil
+        end
       end
 
-      it 'can set nil explicityly to the title and description' do
-        issue = described_class.new(project, user,
-                                    merge_request_for_resolving_discussions: merge_request,
-                                    description: nil,
-                                    title: nil).execute
+      describe 'for a merge request' do
+        let(:opts) { { merge_request_for_resolving_discussions: merge_request } }
+
+        it 'resolves the discussion' do
+          described_class.new(project, user, opts).execute
+          discussion.first_note.reload
 
-        expect(issue.description).to be_nil
-        expect(issue.title).to be_nil
+          expect(discussion.resolved?).to be(true)
+        end
+
+        it 'added a system note to the discussion' do
+          described_class.new(project, user, opts).execute
+
+          reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first
+
+          expect(reloaded_discussion.last_note.system).to eq(true)
+        end
+
+        it 'assigns the title and description for the issue' do
+          issue = described_class.new(project, user, opts).execute
+
+          expect(issue.title).not_to be_nil
+          expect(issue.description).not_to be_nil
+        end
+
+        it 'can set nil explicitly to the title and description' do
+          issue = described_class.new(project, user,
+                                      merge_request_for_resolving_discussions: merge_request,
+                                      description: nil,
+                                      title: nil).execute
+
+          expect(issue.description).to be_nil
+          expect(issue.title).to be_nil
+        end
       end
     end