Add latest changes from gitlab-org/gitlab@master

2 files changed, 114 insertions, 62 deletions

diff --git a/lib/gitlab/diff/rendered/notebook/diff_file.rb b/lib/gitlab/diff/rendered/notebook/diff_file.rb
index 68011555c3c..99631d90ce6 100644
--- a/lib/gitlab/diff/rendered/notebook/diff_file.rb
+++ b/lib/gitlab/diff/rendered/notebook/diff_file.rb
@@ -3,9 +3,10 @@ module Gitlab
   module Diff
     module Rendered
       module Notebook
-        include Gitlab::Utils::StrongMemoize
-
         class DiffFile < Gitlab::Diff::File
+          include Gitlab::Diff::Rendered::Notebook::DiffFileHelper
+          include Gitlab::Utils::StrongMemoize
+
           RENDERED_TIMEOUT_BACKGROUND = 10.seconds
           RENDERED_TIMEOUT_FOREGROUND = 1.5.seconds
           BACKGROUND_EXECUTION = 'background'
@@ -14,7 +15,6 @@ module Gitlab
           LOG_IPYNBDIFF_TIMEOUT = 'IPYNB_DIFF_TIMEOUT'
           LOG_IPYNBDIFF_INVALID = 'IPYNB_DIFF_INVALID'
           LOG_IPYNBDIFF_TRUNCATED = 'IPYNB_DIFF_TRUNCATED'
-          EMBEDDED_IMAGE_PATTERN = '    ![](data:image'
 
           attr_reader :source_diff
 
@@ -51,7 +51,8 @@ module Gitlab
 
           def highlighted_diff_lines
             @highlighted_diff_lines ||= begin
-              removal_line_maps, addition_line_maps = compute_end_start_map
+              removal_line_maps, addition_line_maps = map_diff_block_to_source_line(
+                source_diff.highlighted_diff_lines, source_diff.new_file?, source_diff.deleted_file?)
               Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight.map do |line|
                 mutate_line(line, addition_line_maps, removal_line_maps)
               end
@@ -90,55 +91,6 @@ module Gitlab
             diff
           end
 
-          def strip_diff_frontmatter(diff_content)
-            diff_content.scan(/.*\n/)[2..]&.join('') if diff_content.present?
-          end
-
-          def transformed_line_to_source(transformed_line, transformed_blocks)
-            transformed_blocks.empty? ? 0 : ( transformed_blocks[transformed_line - 1][:source_line] || -1 ) + 1
-          end
-
-          def mutate_line(line, addition_line_maps, removal_line_maps)
-            line.new_pos = transformed_line_to_source(line.new_pos, notebook_diff.to.blocks)
-            line.old_pos = transformed_line_to_source(line.old_pos, notebook_diff.from.blocks)
-
-            line.old_pos = addition_line_maps[line.new_pos] if line.old_pos == 0 && line.new_pos != 0
-            line.new_pos = removal_line_maps[line.old_pos] if line.new_pos == 0 && line.old_pos != 0
-
-            # Lines that do not appear on the original diff should not be commentable
-            line.type = "#{line.type || 'unchanged'}-nomappinginraw" unless addition_line_maps[line.new_pos] || removal_line_maps[line.old_pos]
-
-            line.line_code = line_code(line)
-
-            line.rich_text = image_as_rich_text(line)
-
-            line
-          end
-
-          def compute_end_start_map
-            # line_codes are used for assigning notes to diffs, and these depend on the line on the new version and the
-            # line that would have been that one in the previous version. However, since we do a transformation on the
-            # file, that map gets lost. To overcome this, we look at the original source lines and build two maps:
-            # - For additions, we look at the latest line change for that line and pick the old line for that id
-            # - For removals, we look at the first line in the old version, and pick the first line on the new version
-            #
-            #
-            # The caveat here is that we can't have notes on lines that are not a translation of a line in the source
-            # diff
-            #
-            # (gitlab/diff/file.rb:75)
-
-            removals = {}
-            additions = {}
-
-            source_diff.highlighted_diff_lines.each do |line|
-              removals[line.old_pos] = line.new_pos unless source_diff.new_file?
-              additions[line.new_pos] = line.old_pos unless source_diff.deleted_file?
-            end
-
-            [removals, additions]
-          end
-
           def rendered_timeout
             @rendered_timeout ||= Gitlab::Metrics.counter(
               :ipynb_semantic_diff_timeouts_total,
@@ -156,16 +108,27 @@ module Gitlab
             nil
           end
 
-          def image_as_rich_text(line)
-            # Strip the initial +, -, or space for the diff context
-            line_text = line.text[1..]
+          def compute_line_numbers(transformed_old_pos, transformed_new_pos, addition_line_maps, removal_line_maps)
+            new_pos = map_transformed_line_to_source(transformed_new_pos, notebook_diff.to.blocks)
+            old_pos = map_transformed_line_to_source(transformed_old_pos, notebook_diff.from.blocks)
 
-            if line_text.starts_with?(EMBEDDED_IMAGE_PATTERN)
-              image_body = line_text.delete_prefix(EMBEDDED_IMAGE_PATTERN).delete_suffix(')')
-              "<img src=\"data:image#{CGI.escapeHTML(image_body)}\">".html_safe
-            else
-              line.rich_text
-            end
+            old_pos = addition_line_maps[new_pos] if old_pos == 0 && new_pos != 0
+            new_pos = removal_line_maps[old_pos] if new_pos == 0 && old_pos != 0
+
+            [old_pos, new_pos]
+          end
+
+          def mutate_line(line, addition_line_maps, removal_line_maps)
+            line.old_pos, line.new_pos = compute_line_numbers(line.old_pos, line.new_pos, addition_line_maps, removal_line_maps)
+
+            # Lines that do not appear on the original diff should not be commentable
+            line.type = "#{line.type || 'unchanged'}-nomappinginraw" unless addition_line_maps[line.new_pos] || removal_line_maps[line.old_pos]
+
+            line.line_code = line_code(line)
+
+            line.rich_text = image_as_rich_text(line.text) || line.rich_text
+
+            line
           end
         end
       end
diff --git a/lib/gitlab/diff/rendered/notebook/diff_file_helper.rb b/lib/gitlab/diff/rendered/notebook/diff_file_helper.rb
new file mode 100644
index 00000000000..822b3771276
--- /dev/null
+++ b/lib/gitlab/diff/rendered/notebook/diff_file_helper.rb
@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+module Gitlab
+  module Diff
+    module Rendered
+      module Notebook
+        module DiffFileHelper
+          EMBEDDED_IMAGE_PATTERN = '    ![](data:image'
+
+          def strip_diff_frontmatter(diff_content)
+            diff_content.scan(/.*\n/)[2..]&.join('') if diff_content.present?
+          end
+
+          def map_transformed_line_to_source(transformed_line, transformed_blocks)
+            transformed_blocks.empty? ? 0 : ( transformed_blocks[transformed_line - 1][:source_line] || -1 ) + 1
+          end
+
+          # line_codes are used for assigning notes to diffs, and these depend on the line on the new version and the
+          # line that would have been that one in the previous version. However, since we do a transformation on the
+          # file, that mapping gets lost. To overcome this, we look at the original source lines and build two maps:
+          # - For additions, we look at the latest line change for that line and pick the old line for that id
+          # - For removals, we look at the first line in the old version, and pick the first line on the new version
+          #
+          # Note: ipynb files never change the first or last line (open and closure of the
+          # json object), unless the file is removed or deleted
+          #
+          # Example: Additions and removals
+          # Old:   New:
+          # A      A
+          # B      D
+          # C      E
+          # F      F
+          #
+          # Diff:
+          # 1  A A 1 | line code: 1_1
+          # 2 -B     | line code: 2_2 -> new line is what it is after been without the removal, 2
+          # 3 -C     | line code: 3_2
+          #   +  D 2 | line code: 4_2 -> old line is what would have been before the addition, 4
+          #   +  E 3 | line code: 4_3
+          # 4 F  F 4 | line code: 4_4
+          #
+          # Example: only additions
+          # Old:   New:
+          # A      A
+          # F      B
+          #        C
+          #        F
+          #
+          # Diff:
+          #  A A | line code: 1_1
+          # +  B | line code: 2_2 -> old line is the next after the additions, 2
+          # +  C | line code: 2_3
+          #  F F | line code: 2_4
+          #
+          # Example: only removals
+          # Old:   New:
+          # A      A
+          # B      F
+          # C
+          # F
+          #
+          # Diff:
+          #  A A | line code: 1_1
+          # -B   | line code: 2_2 -> new line is what it is after been without the removal, 2
+          # -C   | line code: 3_2
+          #  F F | line code: 4_2
+          def map_diff_block_to_source_line(lines, file_added, file_deleted)
+            removals = {}
+            additions = {}
+
+            lines.each do |line|
+              removals[line.old_pos] = line.new_pos unless file_added
+              additions[line.new_pos] = line.old_pos unless file_deleted
+            end
+
+            [removals, additions]
+          end
+
+          def image_as_rich_text(line_text)
+            return unless line_text[1..].starts_with?(EMBEDDED_IMAGE_PATTERN)
+
+            image_body = line_text[1..].delete_prefix(EMBEDDED_IMAGE_PATTERN).delete_suffix(')')
+
+            "<img src=\"data:image#{CGI.escapeHTML(image_body)}\">".html_safe
+          end
+        end
+      end
+    end
+  end
+end