Merge branch 'kassio/github-importer-format-suggestions' into 'master'

GithubImporter: Format diff note suggestions to the gitlab format

See merge request gitlab-org/gitlab!71411

lib/gitlab/github_import/representation/diff_note.rb

@@ -40,7 +40,9 @@ module Gitlab
             note: note.body,
             created_at: note.created_at,
             updated_at: note.updated_at,
-            note_id: note.id
+            note_id: note.id,
+            end_line: note.line,
+            start_line: note.start_line
           }
 
           new(hash)
@@ -83,6 +85,14 @@ module Gitlab
           }
         end
 
+        def note
+          @note ||= DiffNotes::SuggestionFormatter.formatted_note_for(
+            note: attributes[:note],
+            start_line: attributes[:start_line],
+            end_line: attributes[:end_line]
+          )
+        end
+
         def github_identifiers
           {
             note_id: note_id,

lib/gitlab/github_import/representation/diff_notes/suggestion_formatter.rb

+# frozen_string_literal: true
+
+# This class replaces Github markdown suggestion tag with
+# a Gitlab suggestion tag. The difference between
+# Github's and Gitlab's suggestion tags is that Gitlab
+# includes the range of the suggestion in the tag, while Github
+# uses other note attributes to position the suggestion.
+module Gitlab
+  module GithubImport
+    module Representation
+      module DiffNotes
+        class SuggestionFormatter
+          # A github suggestion:
+          # - the ```suggestion tag must be the first text of the line
+          #   - it might have up to 3 spaces before the ```suggestion tag
+          # - extra text on the ```suggestion tag line will be ignored
+          GITHUB_SUGGESTION = /^\ {,3}(?<suggestion>```suggestion\b).*(?<eol>\R)/.freeze
+
+          def self.formatted_note_for(...)
+            new(...).formatted_note
+          end
+
+          def initialize(note:, start_line: nil, end_line: nil)
+            @note = note
+            @start_line = start_line
+            @end_line = end_line
+          end
+
+          def formatted_note
+            if contains_suggestion?
+              note.gsub(
+                GITHUB_SUGGESTION,
+                "\\k<suggestion>:#{suggestion_range}\\k<eol>"
+              )
+            else
+              note
+            end
+          end
+
+          private
+
+          attr_reader :note, :start_line, :end_line
+
+          def contains_suggestion?
+            note.to_s.match?(GITHUB_SUGGESTION)
+          end
+
+          # Github always saves the comment on the _last_ line of the range.
+          # Therefore, the diff hunk will always be related to lines before
+          # the comment itself.
+          def suggestion_range
+            "-#{line_count}+0"
+          end
+
+          def line_count
+            if start_line.present?
+              end_line - start_line
+            else
+              0
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/github_import/importer/diff_notes_importer_spec.rb

@@ -15,10 +15,18 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNotesImporter do
       original_commit_id: 'original123abc',
       diff_hunk: "@@ -1 +1 @@\n-Hello\n+Hello world",
       user: double(:user, id: 4, login: 'alice'),
-      body: 'Hello world',
       created_at: Time.zone.now,
       updated_at: Time.zone.now,
-      id: 1
+      line: 23,
+      start_line: nil,
+      id: 1,
+      body: <<~BODY
+        Hello World
+
+        ```suggestion
+        sug1
+        ```
+      BODY
     )
   end
 

spec/lib/gitlab/github_import/representation/diff_note_spec.rb

@@ -73,6 +73,8 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do
         body: 'Hello world',
         created_at: created_at,
         updated_at: updated_at,
+        line: 23,
+        start_line: nil,
         id: 1
       )
     end
@@ -90,47 +92,70 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do
 
       expect(note.author).to be_nil
     end
-  end
 
-  describe '.from_json_hash' do
-    it_behaves_like 'a DiffNote' do
-      let(:hash) do
-        {
-          'noteable_type' => 'MergeRequest',
-          'noteable_id' => 42,
-          'file_path' => 'README.md',
-          'commit_id' => '123abc',
-          'original_commit_id' => 'original123abc',
-          'diff_hunk' => hunk,
-          'author' => { 'id' => 4, 'login' => 'alice' },
-          'note' => 'Hello world',
-          'created_at' => created_at.to_s,
-          'updated_at' => updated_at.to_s,
-          'note_id' => 1
-        }
-      end
+    it 'formats a suggestion in the note body' do
+      allow(response)
+        .to receive(:body)
+        .and_return <<~BODY
+      ```suggestion
+      Hello World
+      ```
+      BODY
 
-      let(:note) { described_class.from_json_hash(hash) }
+      note = described_class.from_api_response(response)
+
+      expect(note.note).to eq <<~BODY
+      ```suggestion:-0+0
+      Hello World
+      ```
+      BODY
     end
+  end
 
-    it 'does not convert the author if it was not specified' do
-      hash = {
+  describe '.from_json_hash' do
+    let(:hash) do
+      {
         'noteable_type' => 'MergeRequest',
         'noteable_id' => 42,
         'file_path' => 'README.md',
         'commit_id' => '123abc',
         'original_commit_id' => 'original123abc',
         'diff_hunk' => hunk,
+        'author' => { 'id' => 4, 'login' => 'alice' },
         'note' => 'Hello world',
         'created_at' => created_at.to_s,
         'updated_at' => updated_at.to_s,
         'note_id' => 1
       }
+    end
+
+    it_behaves_like 'a DiffNote' do
+      let(:note) { described_class.from_json_hash(hash) }
+    end
+
+    it 'does not convert the author if it was not specified' do
+      hash.delete('author')
 
       note = described_class.from_json_hash(hash)
 
       expect(note.author).to be_nil
     end
+
+    it 'formats a suggestion in the note body' do
+      hash['note'] = <<~BODY
+      ```suggestion
+      Hello World
+      ```
+      BODY
+
+      note = described_class.from_json_hash(hash)
+
+      expect(note.note).to eq <<~BODY
+      ```suggestion:-0+0
+      Hello World
+      ```
+      BODY
+    end
   end
 
   describe '#line_code' do
@@ -181,4 +206,54 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote do
       expect(note.github_identifiers).to eq(github_identifiers)
     end
   end
+
+  describe '#note' do
+    it 'returns the given note' do
+      hash = {
+        'note': 'simple text'
+      }
+
+      note = described_class.new(hash)
+
+      expect(note.note).to eq 'simple text'
+    end
+
+    it 'returns the suggestion formatted in the note' do
+      hash = {
+        'note': <<~BODY
+        ```suggestion
+        Hello World
+        ```
+        BODY
+      }
+
+      note = described_class.new(hash)
+
+      expect(note.note).to eq <<~BODY
+      ```suggestion:-0+0
+      Hello World
+      ```
+      BODY
+    end
+
+    it 'returns the multi-line suggestion formatted in the note' do
+      hash = {
+        'start_line': 20,
+        'end_line': 23,
+        'note': <<~BODY
+        ```suggestion
+        Hello World
+        ```
+        BODY
+      }
+
+      note = described_class.new(hash)
+
+      expect(note.note).to eq <<~BODY
+      ```suggestion:-3+0
+      Hello World
+      ```
+      BODY
+    end
+  end
 end

spec/lib/gitlab/github_import/representation/diff_notes/suggestion_formatter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GithubImport::Representation::DiffNotes::SuggestionFormatter do
+  it 'does nothing when there is any text before the suggestion tag' do
+    note = <<~BODY
+    looks like```suggestion but it isn't
+    ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note)).to eq(note)
+  end
+
+  it 'handles nil value for note' do
+    note = nil
+
+    expect(described_class.formatted_note_for(note: note)).to eq(note)
+  end
+
+  it 'does not allow over 3 leading spaces for valid suggestion' do
+    note = <<~BODY
+      Single-line suggestion
+          ```suggestion
+      sug1
+      ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note)).to eq(note)
+  end
+
+  it 'allows up to 3 leading spaces' do
+    note = <<~BODY
+      Single-line suggestion
+         ```suggestion
+      sug1
+      ```
+    BODY
+
+    expected = <<~BODY
+      Single-line suggestion
+      ```suggestion:-0+0
+      sug1
+      ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note)).to eq(expected)
+  end
+
+  it 'does nothing when there is any text without space after the suggestion tag' do
+    note = <<~BODY
+    ```suggestionbut it isn't
+    ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note)).to eq(note)
+  end
+
+  it 'formats single-line suggestions' do
+    note = <<~BODY
+      Single-line suggestion
+      ```suggestion
+      sug1
+      ```
+    BODY
+
+    expected = <<~BODY
+      Single-line suggestion
+      ```suggestion:-0+0
+      sug1
+      ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note)).to eq(expected)
+  end
+
+  it 'ignores text after suggestion tag on the same line' do
+    note = <<~BODY
+    looks like
+    ```suggestion text to be ignored
+    suggestion
+    ```
+    BODY
+
+    expected = <<~BODY
+    looks like
+    ```suggestion:-0+0
+    suggestion
+    ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note)).to eq(expected)
+  end
+
+  it 'formats multiple single-line suggestions' do
+    note = <<~BODY
+      Single-line suggestion
+      ```suggestion
+      sug1
+      ```
+      OR
+      ```suggestion
+      sug2
+      ```
+    BODY
+
+    expected = <<~BODY
+      Single-line suggestion
+      ```suggestion:-0+0
+      sug1
+      ```
+      OR
+      ```suggestion:-0+0
+      sug2
+      ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note)).to eq(expected)
+  end
+
+  it 'formats multi-line suggestions' do
+    note = <<~BODY
+      Multi-line suggestion
+      ```suggestion
+      sug1
+      ```
+    BODY
+
+    expected = <<~BODY
+      Multi-line suggestion
+      ```suggestion:-2+0
+      sug1
+      ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note, start_line: 6, end_line: 8)).to eq(expected)
+  end
+
+  it 'formats multiple multi-line suggestions' do
+    note = <<~BODY
+      Multi-line suggestion
+      ```suggestion
+      sug1
+      ```
+      OR
+      ```suggestion
+      sug2
+      ```
+    BODY
+
+    expected = <<~BODY
+      Multi-line suggestion
+      ```suggestion:-2+0
+      sug1
+      ```
+      OR
+      ```suggestion:-2+0
+      sug2
+      ```
+    BODY
+
+    expect(described_class.formatted_note_for(note: note, start_line: 6, end_line: 8)).to eq(expected)
+  end
+end