Merge branch 'osw-ignore-diff-header-when-persisting-diff-hunk' into 'master'

Adjust insufficient diff hunks being persisted on NoteDiffFile

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

changelogs/unreleased/osw-ignore-diff-header-when-persisting-diff-hunk.yml

+---
+title: Adjust insufficient diff hunks being persisted on NoteDiffFile
+merge_request:
+author:
+type: fixed

lib/gitlab/diff/file.rb

@@ -78,9 +78,12 @@ module Gitlab
 
       # Returns the raw diff content up to the given line index
       def diff_hunk(diff_line)
-        # Adding 2 because of the @@ diff header and Enum#take should consider
-        # an extra line, because we're passing an index.
-        raw_diff.each_line.take(diff_line.index + 2).join
+        diff_line_index = diff_line.index
+        # @@ (match) header is not kept if it's found in the top of the file,
+        # therefore we should keep an extra line on this scenario.
+        diff_line_index += 1 unless diff_lines.first.match?
+
+        diff_lines.select { |line| line.index <= diff_line_index }.map(&:text).join("\n")
       end
 
       def old_sha

lib/gitlab/diff/line.rb

@@ -53,6 +53,10 @@ module Gitlab
         %w[match new-nonewline old-nonewline].include?(type)
       end
 
+      def match?
+        type == :match
+      end
+
       def discussable?
         !meta?
       end

spec/lib/gitlab/diff/file_spec.rb

@@ -470,56 +470,69 @@ describe Gitlab::Diff::File do
   end
 
   describe '#diff_hunk' do
+    context 'when first line is a match' do
       let(:raw_diff) do
-      <<EOS
-@@ -6,12 +6,18 @@ module Popen
+        <<~EOS
+          --- a/files/ruby/popen.rb
+          +++ b/files/ruby/popen.rb
+          @@ -6,12 +6,18 @@ module Popen
 
              def popen(cmd, path=nil)
                unless cmd.is_a?(Array)
--      raise "System commands must be given as an array of strings"
-+      raise RuntimeError, "System commands must be given as an array of strings"
-     end
-
-     path ||= Dir.pwd
--    vars = { "PWD" => path }
--    options = { chdir: path }
-+
-+    vars = {
-+      "PWD" => path
-+    }
-+
-+    options = {
-+      chdir: path
-+    }
-
-     unless File.directory?(path)
-       FileUtils.mkdir_p(path)
-@@ -19,6 +25,7 @@ module Popen
-
-     @cmd_output = ""
-     @cmd_status = 0
-+
-     Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
-       @cmd_output << stdout.read
-       @cmd_output << stderr.read
-EOS
+          -      raise "System commands must be given as an array of strings"
+          +      raise RuntimeError, "System commands must be given as an array of strings"
+               end
+        EOS
       end
 
       it 'returns raw diff up to given line index' do
         allow(diff_file).to receive(:raw_diff) { raw_diff }
-      diff_line = instance_double(Gitlab::Diff::Line, index: 5)
+        diff_line = instance_double(Gitlab::Diff::Line, index: 4)
 
-      diff_hunk = <<EOS
-@@ -6,12 +6,18 @@ module Popen
+        diff_hunk = <<~EOS
+          @@ -6,12 +6,18 @@ module Popen
 
              def popen(cmd, path=nil)
                unless cmd.is_a?(Array)
--      raise "System commands must be given as an array of strings"
-+      raise RuntimeError, "System commands must be given as an array of strings"
+          -      raise "System commands must be given as an array of strings"
+          +      raise RuntimeError, "System commands must be given as an array of strings"
+        EOS
+
+        expect(diff_file.diff_hunk(diff_line)).to eq(diff_hunk.strip)
+      end
+    end
+
+    context 'when first line is not a match' do
+      let(:raw_diff) do
+        <<~EOS
+          @@ -1,4 +1,4 @@
+          -Copyright (c) 2011-2017 GitLab B.V.
+          +Copyright (c) 2011-2019 GitLab B.V.
+
+          With regard to the GitLab Software:
+
+          @@ -9,17 +9,21 @@ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+          copies of the Software, and to permit persons to whom the Software is
+          furnished to do so, subject to the following conditions:
+        EOS
       end
-EOS
 
-      expect(diff_file.diff_hunk(diff_line)).to eq(diff_hunk)
+      it 'returns raw diff up to given line index' do
+        allow(diff_file).to receive(:raw_diff) { raw_diff }
+        diff_line = instance_double(Gitlab::Diff::Line, index: 5)
+
+        diff_hunk = <<~EOS
+          -Copyright (c) 2011-2017 GitLab B.V.
+          +Copyright (c) 2011-2019 GitLab B.V.
+
+          With regard to the GitLab Software:
+
+          @@ -9,17 +9,21 @@ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+          copies of the Software, and to permit persons to whom the Software is
+        EOS
+
+        expect(diff_file.diff_hunk(diff_line)).to eq(diff_hunk.strip)
+      end
     end
   end
 end