From 48d498a4eb03e42179f773183c99d0ee3598bc77 Mon Sep 17 00:00:00 2001
From: Igor Drozdov <idrozdov@gitlab.com>
Date: Fri, 24 May 2019 13:43:16 +0300
Subject: Fix removing empty lines via suggestions

Before this fix, a suggestion which just removes
an empty line wasn't appliable
---
 .../id-bug-suggested-changes-remove-empty-line.yml |  5 ++
 lib/gitlab/diff/suggestion.rb                      |  2 +
 spec/services/suggestions/apply_service_spec.rb    | 68 +++++++++++++++++++---
 spec/services/suggestions/create_service_spec.rb   | 20 +++++++
 4 files changed, 88 insertions(+), 7 deletions(-)
 create mode 100644 changelogs/unreleased/id-bug-suggested-changes-remove-empty-line.yml

diff --git a/changelogs/unreleased/id-bug-suggested-changes-remove-empty-line.yml b/changelogs/unreleased/id-bug-suggested-changes-remove-empty-line.yml
new file mode 100644
index 00000000000..5e30ab17842
--- /dev/null
+++ b/changelogs/unreleased/id-bug-suggested-changes-remove-empty-line.yml
@@ -0,0 +1,5 @@
+---
+title: Allow to remove empty lines via suggestions
+merge_request: 28703
+author:
+type: fixed
diff --git a/lib/gitlab/diff/suggestion.rb b/lib/gitlab/diff/suggestion.rb
index 027c7a31bcf..4a3ac2106e2 100644
--- a/lib/gitlab/diff/suggestion.rb
+++ b/lib/gitlab/diff/suggestion.rb
@@ -33,6 +33,8 @@ module Gitlab
       end
 
       def to_content
+        return "" if @text.blank?
+
         # The parsed suggestion doesn't have information about the correct
         # ending characters (we may have a line break, or not), so we take
         # this information from the last line being changed (last
diff --git a/spec/services/suggestions/apply_service_spec.rb b/spec/services/suggestions/apply_service_spec.rb
index 7732767137c..bdbcb0fdb07 100644
--- a/spec/services/suggestions/apply_service_spec.rb
+++ b/spec/services/suggestions/apply_service_spec.rb
@@ -5,6 +5,16 @@ require 'spec_helper'
 describe Suggestions::ApplyService do
   include ProjectForksHelper
 
+  def build_position(args = {})
+    default_args = { old_path: "files/ruby/popen.rb",
+                     new_path: "files/ruby/popen.rb",
+                     old_line: nil,
+                     new_line: 9,
+                     diff_refs: merge_request.diff_refs }
+
+    Gitlab::Diff::Position.new(default_args.merge(args))
+  end
+
   shared_examples 'successfully creates commit and updates suggestion' do
     def apply(suggestion)
       result = subject.execute(suggestion)
@@ -43,13 +53,7 @@ describe Suggestions::ApplyService do
   let(:project) { create(:project, :repository) }
   let(:user) { create(:user, :commit_email) }
 
-  let(:position) do
-    Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
-                               new_path: "files/ruby/popen.rb",
-                               old_line: nil,
-                               new_line: 9,
-                               diff_refs: merge_request.diff_refs)
-  end
+  let(:position) { build_position }
 
   let(:diff_note) do
     create(:diff_note_on_merge_request, noteable: merge_request, position: position, project: project)
@@ -333,6 +337,56 @@ describe Suggestions::ApplyService do
 
         it_behaves_like 'successfully creates commit and updates suggestion'
       end
+
+      context 'remove an empty line suggestion' do
+        let(:expected_content) do
+          <<~CONTENT
+            require 'fileutils'
+            require 'open3'
+
+            module Popen
+              extend self
+
+              def popen(cmd, path=nil)
+                unless cmd.is_a?(Array)
+                  raise RuntimeError, "System commands must be given as an array of strings"
+                end
+
+                path ||= Dir.pwd
+                vars = {
+                  "PWD" => path
+                }
+
+                options = {
+                  chdir: path
+                }
+
+                unless File.directory?(path)
+                  FileUtils.mkdir_p(path)
+                end
+
+                @cmd_output = ""
+                @cmd_status = 0
+
+                Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
+                  @cmd_output << stdout.read
+                  @cmd_output << stderr.read
+                  @cmd_status = wait_thr.value.exitstatus
+                end
+
+                return @cmd_output, @cmd_status
+              end
+            end
+          CONTENT
+        end
+
+        let(:position) { build_position(new_line: 13) }
+        let(:suggestion) do
+          create(:suggestion, :content_from_repo, note: diff_note, to_content: "")
+        end
+
+        it_behaves_like 'successfully creates commit and updates suggestion'
+      end
     end
 
     context 'fork-project' do
diff --git a/spec/services/suggestions/create_service_spec.rb b/spec/services/suggestions/create_service_spec.rb
index ce4990a34a4..ccd44e615a8 100644
--- a/spec/services/suggestions/create_service_spec.rb
+++ b/spec/services/suggestions/create_service_spec.rb
@@ -151,6 +151,26 @@ describe Suggestions::CreateService do
           subject.execute
         end
       end
+
+      context 'when a patch removes an empty line' do
+        let(:markdown) do
+          <<-MARKDOWN.strip_heredoc
+              ```suggestion
+              ```
+          MARKDOWN
+        end
+        let(:position) { build_position(new_line: 13) }
+
+        it 'creates an appliable suggestion' do
+          subject.execute
+
+          suggestion = note.suggestions.last
+
+          expect(suggestion).to be_appliable
+          expect(suggestion.from_content).to eq("\n")
+          expect(suggestion.to_content).to eq("")
+        end
+      end
     end
   end
 end
-- 
cgit v1.2.1