Commit cc4d5f0a authored by Stan Hu's avatar Stan Hu

Merge branch 'id-bug-suggested-changes-remove-empty-line' into 'master'

Append endline only if suggestion present

Closes #58566

See merge request gitlab-org/gitlab-ce!28703
parents 6da066c1 c92db3db
---
title: Allow removal of empty lines via suggestions
merge_request: 28703
author:
type: fixed
...@@ -33,6 +33,8 @@ module Gitlab ...@@ -33,6 +33,8 @@ module Gitlab
end end
def to_content def to_content
return "" if @text.blank?
# The parsed suggestion doesn't have information about the correct # The parsed suggestion doesn't have information about the correct
# ending characters (we may have a line break, or not), so we take # ending characters (we may have a line break, or not), so we take
# this information from the last line being changed (last # this information from the last line being changed (last
......
...@@ -5,6 +5,16 @@ require 'spec_helper' ...@@ -5,6 +5,16 @@ require 'spec_helper'
describe Suggestions::ApplyService do describe Suggestions::ApplyService do
include ProjectForksHelper 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 shared_examples 'successfully creates commit and updates suggestion' do
def apply(suggestion) def apply(suggestion)
result = subject.execute(suggestion) result = subject.execute(suggestion)
...@@ -43,13 +53,7 @@ describe Suggestions::ApplyService do ...@@ -43,13 +53,7 @@ describe Suggestions::ApplyService do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:user) { create(:user, :commit_email) } let(:user) { create(:user, :commit_email) }
let(:position) do let(:position) { build_position }
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(:diff_note) do let(:diff_note) do
create(:diff_note_on_merge_request, noteable: merge_request, position: position, project: project) create(:diff_note_on_merge_request, noteable: merge_request, position: position, project: project)
...@@ -333,6 +337,56 @@ describe Suggestions::ApplyService do ...@@ -333,6 +337,56 @@ describe Suggestions::ApplyService do
it_behaves_like 'successfully creates commit and updates suggestion' it_behaves_like 'successfully creates commit and updates suggestion'
end 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 end
context 'fork-project' do context 'fork-project' do
......
...@@ -151,6 +151,26 @@ describe Suggestions::CreateService do ...@@ -151,6 +151,26 @@ describe Suggestions::CreateService do
subject.execute subject.execute
end end
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 end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment