Merge branch 'osw-fetch-latest-version-when-creating-suggestions' into 'master'

Enforce using the latest diff version when creating suggestions

Closes gitlab-ee#9876

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

2 files changed, 52 insertions, 6 deletions

diff --git a/spec/services/suggestions/apply_service_spec.rb b/spec/services/suggestions/apply_service_spec.rb
index 8e77d582eb4..fe85b5c9065 100644
--- a/spec/services/suggestions/apply_service_spec.rb
+++ b/spec/services/suggestions/apply_service_spec.rb
@@ -362,6 +362,17 @@ describe Suggestions::ApplyService do
       project.add_maintainer(user)
     end
 
+    context 'diff file was not found' do
+      it 'returns error message' do
+        expect(suggestion.note).to receive(:latest_diff_file) { nil }
+
+        result = subject.execute(suggestion)
+
+        expect(result).to eq(message: 'The file was not found',
+                             status: :error)
+      end
+    end
+
     context 'suggestion was already applied' do
       it 'returns success status' do
         result = subject.execute(suggestion)
diff --git a/spec/services/suggestions/create_service_spec.rb b/spec/services/suggestions/create_service_spec.rb
index f1142c88a69..1b4b15b8eaa 100644
--- a/spec/services/suggestions/create_service_spec.rb
+++ b/spec/services/suggestions/create_service_spec.rb
@@ -9,14 +9,18 @@ describe Suggestions::CreateService do
                            target_project: project_with_repo)
   end
 
-  let(:position) do
-    Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
-                               new_path: "files/ruby/popen.rb",
-                               old_line: nil,
-                               new_line: 14,
-                               diff_refs: merge_request.diff_refs)
+  def build_position(args = {})
+    default_args = { old_path: "files/ruby/popen.rb",
+                     new_path: "files/ruby/popen.rb",
+                     old_line: nil,
+                     new_line: 14,
+                     diff_refs: merge_request.diff_refs }
+
+    Gitlab::Diff::Position.new(default_args.merge(args))
   end
 
+  let(:position) { build_position }
+
   let(:markdown) do
     <<-MARKDOWN.strip_heredoc
         ```suggestion
@@ -74,6 +78,21 @@ describe Suggestions::CreateService do
       end
     end
 
+    context 'should not create suggestions' do
+      let(:note) do
+        create(:diff_note_on_merge_request, project: project_with_repo,
+                                            noteable: merge_request,
+                                            position: position,
+                                            note: markdown)
+      end
+
+      it 'creates no suggestion when diff file is not found' do
+        expect(note).to receive(:latest_diff_file) { nil }
+
+        expect { subject.execute }.not_to change(Suggestion, :count)
+      end
+    end
+
     context 'should create suggestions' do
       let(:note) do
         create(:diff_note_on_merge_request, project: project_with_repo,
@@ -104,6 +123,22 @@ describe Suggestions::CreateService do
           expect(suggestion_2).to have_attributes(from_content: "    vars = {\n",
                                                   to_content: "  xpto\n  baz\n")
         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) }
+
+          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
+
+            subject.execute
+          end
+        end
       end
     end
   end