Prepare suggestion implementation for multi-line

Adds the groundwork needed in order to persist multi-line suggestions,
while providing the parsing strategy which will be reused for the
**Preview** as well.

3 files changed, 198 insertions, 70 deletions

diff --git a/spec/services/suggestions/apply_service_spec.rb b/spec/services/suggestions/apply_service_spec.rb
index fe85b5c9065..80b5dcac6c7 100644
--- a/spec/services/suggestions/apply_service_spec.rb
+++ b/spec/services/suggestions/apply_service_spec.rb
@@ -5,6 +5,41 @@ require 'spec_helper'
 describe Suggestions::ApplyService do
   include ProjectForksHelper
 
+  shared_examples 'successfully creates commit and updates suggestion' do
+    def apply(suggestion)
+      result = subject.execute(suggestion)
+      expect(result[:status]).to eq(:success)
+    end
+
+    it 'updates the file with the new contents' do
+      apply(suggestion)
+
+      blob = project.repository.blob_at_branch(merge_request.source_branch,
+                                               position.new_path)
+
+      expect(blob.data).to eq(expected_content)
+    end
+
+    it 'updates suggestion applied and commit_id columns' do
+      expect { apply(suggestion) }
+        .to change(suggestion, :applied)
+        .from(false).to(true)
+        .and change(suggestion, :commit_id)
+        .from(nil)
+    end
+
+    it 'created commit has users email and name' do
+      apply(suggestion)
+
+      commit = project.repository.commit
+
+      expect(user.commit_email).not_to eq(user.email)
+      expect(commit.author_email).to eq(user.commit_email)
+      expect(commit.committer_email).to eq(user.commit_email)
+      expect(commit.author_name).to eq(user.name)
+    end
+  end
+
   let(:project) { create(:project, :repository) }
   let(:user) { create(:user, :commit_email) }
 
@@ -17,9 +52,8 @@ describe Suggestions::ApplyService do
   end
 
   let(:suggestion) do
-    create(:suggestion, note: diff_note,
-                        from_content: "      raise RuntimeError, \"System commands must be given as an array of strings\"\n",
-                        to_content: "      raise RuntimeError, 'Explosion'\n      # explosion?\n")
+    create(:suggestion, :content_from_repo, note: diff_note,
+                                            to_content: "      raise RuntimeError, 'Explosion'\n      # explosion?\n")
   end
 
   subject { described_class.new(user) }
@@ -84,39 +118,7 @@ describe Suggestions::ApplyService do
         project.add_maintainer(user)
       end
 
-      it 'updates the file with the new contents' do
-        subject.execute(suggestion)
-
-        blob = project.repository.blob_at_branch(merge_request.source_branch,
-                                                 position.new_path)
-
-        expect(blob.data).to eq(expected_content)
-      end
-
-      it 'returns success status' do
-        result = subject.execute(suggestion)
-
-        expect(result[:status]).to eq(:success)
-      end
-
-      it 'updates suggestion applied and commit_id columns' do
-        expect { subject.execute(suggestion) }
-          .to change(suggestion, :applied)
-          .from(false).to(true)
-          .and change(suggestion, :commit_id)
-          .from(nil)
-      end
-
-      it 'created commit has users email and name' do
-        subject.execute(suggestion)
-
-        commit = project.repository.commit
-
-        expect(user.commit_email).not_to eq(user.email)
-        expect(commit.author_email).to eq(user.commit_email)
-        expect(commit.committer_email).to eq(user.commit_email)
-        expect(commit.author_name).to eq(user.name)
-      end
+      it_behaves_like 'successfully creates commit and updates suggestion'
 
       context 'when it fails to apply because the file was changed' do
         it 'returns error message' do
@@ -212,11 +214,13 @@ describe Suggestions::ApplyService do
         end
 
         def apply_suggestion(suggestion)
-          suggestion.note.reload
+          suggestion.reload
           merge_request.reload
           merge_request.clear_memoized_shas
 
           result = subject.execute(suggestion)
+          expect(result[:status]).to eq(:success)
+
           refresh = MergeRequests::RefreshService.new(project, user)
           refresh.execute(merge_request.diff_head_sha,
                           suggestion.commit_id,
@@ -241,7 +245,7 @@ describe Suggestions::ApplyService do
 
           suggestion_2_changes = { old_line: 24,
                                    new_line: 31,
-                                   from_content: "       @cmd_output << stderr.read\n",
+                                   from_content: "      @cmd_output << stderr.read\n",
                                    to_content: "# v2 change\n",
                                    path: path }
 
@@ -368,7 +372,18 @@ describe Suggestions::ApplyService do
 
         result = subject.execute(suggestion)
 
-        expect(result).to eq(message: 'The file was not found',
+        expect(result).to eq(message: 'Suggestion is not appliable',
+                             status: :error)
+      end
+    end
+
+    context 'suggestion is eligible to be outdated' do
+      it 'returns error message' do
+        expect(suggestion).to receive(:outdated?) { true }
+
+        result = subject.execute(suggestion)
+
+        expect(result).to eq(message: 'Suggestion is not appliable',
                              status: :error)
       end
     end
diff --git a/spec/services/suggestions/create_service_spec.rb b/spec/services/suggestions/create_service_spec.rb
index 1b4b15b8eaa..ce4990a34a4 100644
--- a/spec/services/suggestions/create_service_spec.rb
+++ b/spec/services/suggestions/create_service_spec.rb
@@ -40,6 +40,14 @@ describe Suggestions::CreateService do
         ```thing
           this is not a suggestion, it's a thing
         ```
+
+        ```suggestion:-3+2
+          # multi-line suggestion 1
+        ```
+
+        ```suggestion:-5
+          # multi-line suggestion 1
+        ```
     MARKDOWN
   end
 
@@ -54,7 +62,7 @@ describe Suggestions::CreateService do
         end
 
         it 'does not try to parse suggestions' do
-          expect(Banzai::SuggestionsParser).not_to receive(:parse)
+          expect(Gitlab::Diff::SuggestionsParser).not_to receive(:parse)
 
           subject.execute
         end
@@ -71,7 +79,7 @@ describe Suggestions::CreateService do
         it 'does not try to parse suggestions' do
           allow(note).to receive(:on_text?) { false }
 
-          expect(Banzai::SuggestionsParser).not_to receive(:parse)
+          expect(Gitlab::Diff::SuggestionsParser).not_to receive(:parse)
 
           subject.execute
         end
@@ -87,7 +95,9 @@ describe Suggestions::CreateService do
       end
 
       it 'creates no suggestion when diff file is not found' do
-        expect(note).to receive(:latest_diff_file) { nil }
+        expect_next_instance_of(DiffNote) do |diff_note|
+          expect(diff_note).to receive(:latest_diff_file).twice { nil }
+        end
 
         expect { subject.execute }.not_to change(Suggestion, :count)
       end
@@ -101,43 +111,44 @@ describe Suggestions::CreateService do
                                             note: markdown)
       end
 
-      context 'single line suggestions' do
-        it 'persists suggestion records' do
-          expect { subject.execute }
-            .to change { note.suggestions.count }
-            .from(0)
-            .to(2)
-        end
+      let(:expected_suggestions) do
+        Gitlab::Diff::SuggestionsParser.parse(markdown,
+                                              project: note.project,
+                                              position: note.position)
+      end
 
-        it 'persists original from_content lines and suggested lines' do
-          subject.execute
+      it 'persists suggestion records' do
+        expect { subject.execute }.to change { note.suggestions.count }
+          .from(0).to(expected_suggestions.size)
+      end
 
-          suggestions = note.suggestions.order(:relative_order)
+      it 'persists suggestions data correctly' do
+        subject.execute
 
-          suggestion_1 = suggestions.first
-          suggestion_2 = suggestions.last
+        suggestions = note.suggestions.order(:relative_order)
 
-          expect(suggestion_1).to have_attributes(from_content: "    vars = {\n",
-                                                  to_content: "  foo\n  bar\n")
+        suggestions.zip(expected_suggestions) do |suggestion, expected_suggestion|
+          expected_data = expected_suggestion.to_hash
 
-          expect(suggestion_2).to have_attributes(from_content: "    vars = {\n",
-                                                  to_content: "  xpto\n  baz\n")
+          expect(suggestion.from_content).to eq(expected_data[:from_content])
+          expect(suggestion.to_content).to eq(expected_data[:to_content])
+          expect(suggestion.lines_above).to eq(expected_data[:lines_above])
+          expect(suggestion.lines_below).to eq(expected_data[:lines_below])
         end
+      end
 
-        context 'outdated position note' do
-          let!(:outdated_diff) { merge_request.merge_request_diff }
-          let!(:latest_diff) { merge_request.create_merge_request_diff }
-          let(:outdated_position) { build_position(diff_refs: outdated_diff.diff_refs) }
-          let(:position) { build_position(diff_refs: latest_diff.diff_refs) }
+      context 'outdated position note' do
+        let!(:outdated_diff) { merge_request.merge_request_diff }
+        let!(:latest_diff) { merge_request.create_merge_request_diff }
+        let(:outdated_position) { build_position(diff_refs: outdated_diff.diff_refs) }
+        let(:position) { build_position(diff_refs: latest_diff.diff_refs) }
 
-          it 'uses the correct position when creating the suggestion' do
-            expect(note.position)
-              .to receive(:diff_file)
-              .with(project_with_repo.repository)
-              .and_call_original
+        it 'uses the correct position when creating the suggestion' do
+          expect(Gitlab::Diff::SuggestionsParser).to receive(:parse)
+            .with(note.note, project: note.project, position: note.position)
+            .and_call_original
 
-            subject.execute
-          end
+          subject.execute
         end
       end
     end
diff --git a/spec/services/suggestions/outdate_service_spec.rb b/spec/services/suggestions/outdate_service_spec.rb
new file mode 100644
index 00000000000..bcc627013d8
--- /dev/null
+++ b/spec/services/suggestions/outdate_service_spec.rb
@@ -0,0 +1,102 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Suggestions::OutdateService do
+  describe '#execute' do
+    let(:merge_request) { create(:merge_request) }
+    let(:project) { merge_request.target_project }
+    let(:user) { merge_request.author }
+    let(:file_path) { 'files/ruby/popen.rb' }
+    let(:branch_name) { project.default_branch }
+    let(:diff_file) { suggestion.diff_file }
+    let(:position) { build_position(file_path, comment_line) }
+    let(:note) do
+      create(:diff_note_on_merge_request, noteable: merge_request,
+                                          position: position,
+                                          project: project)
+    end
+
+    def build_position(path, line)
+      Gitlab::Diff::Position.new(old_path: path,
+                                 new_path: path,
+                                 old_line: nil,
+                                 new_line: line,
+                                 diff_refs: merge_request.diff_refs)
+    end
+
+    def commit_changes(file_path, new_content)
+      params = {
+        file_path: file_path,
+        commit_message: "Update File",
+        file_content: new_content,
+        start_project: project,
+        start_branch: project.default_branch,
+        branch_name: branch_name
+      }
+
+      Files::UpdateService.new(project, user, params).execute
+    end
+
+    def update_file_line(diff_file, change_line, content)
+      new_lines = diff_file.new_blob.data.lines
+      new_lines[change_line..change_line] = content
+      result = commit_changes(diff_file.file_path, new_lines.join)
+      newrev = result[:result]
+
+      expect(result[:status]).to eq(:success)
+      expect(newrev).to be_present
+
+      # Ensure all memoized data is cleared in order
+      # to generate the new merge_request_diff.
+      MergeRequest.find(merge_request.id).reload_diff(user)
+
+      note.reload
+    end
+
+    before do
+      project.add_maintainer(user)
+    end
+
+    subject { described_class.new.execute(merge_request) }
+
+    context 'when there is a change within multi-line suggestion range' do
+      let(:comment_line) { 9 }
+      let(:lines_above) { 8 } # suggesting to change lines 1..9
+      let(:change_line) { 2 } # line 2 is within the range
+      let!(:suggestion) do
+        create(:suggestion, :content_from_repo, note: note, lines_above: lines_above)
+      end
+
+      it 'updates the outdatable suggestion record' do
+        update_file_line(diff_file, change_line, "# foo\nbar\n")
+
+        # Make sure note is still active
+        expect(note.active?).to be(true)
+
+        expect { subject }.to change { suggestion.reload.outdated }
+          .from(false).to(true)
+      end
+    end
+
+    context 'when there is no change within multi-line suggestion range' do
+      let(:comment_line) { 9 }
+      let(:lines_above) { 3 } # suggesting to change lines 6..9
+      let(:change_line) { 2 } # line 2 is not within the range
+      let!(:suggestion) do
+        create(:suggestion, :content_from_repo, note: note, lines_above: lines_above)
+      end
+
+      subject { described_class.new.execute(merge_request) }
+
+      it 'does not outdates suggestion record' do
+        update_file_line(diff_file, change_line, "# foo\nbar\n")
+
+        # Make sure note is still active
+        expect(note.active?).to be(true)
+
+        expect { subject }.not_to change { suggestion.reload.outdated }.from(false)
+      end
+    end
+  end
+end