Fallback to creating a note if DiffNote fails to import

2 files changed, 56 insertions, 4 deletions

diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb
index 10a2bcabb60..133b8f763ec 100644
--- a/lib/gitlab/bitbucket_server_import/importer.rb
+++ b/lib/gitlab/bitbucket_server_import/importer.rb
@@ -216,31 +216,49 @@ module Gitlab
       def import_inline_comments(inline_comments, merge_request)
         inline_comments.each do |comment|
           position = build_position(merge_request, comment)
-          parent = build_diff_note(merge_request, comment, position)
+          parent = create_diff_note(merge_request, comment, position)
 
           next unless parent&.persisted?
 
           discussion_id = parent.discussion_id
 
           comment.comments.each do |reply|
-            build_diff_note(merge_request, reply, position, discussion_id)
+            create_diff_note(merge_request, reply, position, discussion_id)
           end
         end
       end
 
-      def build_diff_note(merge_request, comment, position, discussion_id = nil)
+      def create_diff_note(merge_request, comment, position, discussion_id = nil)
         attributes = pull_request_comment_attributes(comment)
         attributes.merge!(
           position: position,
           type: 'DiffNote')
         attributes[:discussion_id] = discussion_id if discussion_id
 
-        merge_request.notes.create!(attributes)
+        note = merge_request.notes.build(attributes)
+
+        if note.valid?
+          note.save
+          return note
+        end
+
+        # Fallback to a regular comment
+        create_fallback_diff_note(merge_request, comment)
       rescue StandardError => e
         errors << { type: :pull_request, id: comment.id, errors: e.message }
         nil
       end
 
+      # Bitbucket Server supports the ability to comment on any line, not just the
+      # line in the diff. If we can't add the note as a DiffNote, fallback to creating
+      # a regular note.
+      def create_fallback_diff_note(merge_request, comment)
+        attributes = pull_request_comment_attributes(comment)
+        attributes[:note] = "Comment on file: #{comment.file_path}, old position: #{comment.old_pos}, new_position: #{comment.new_pos}\n\n" + attributes[:note]
+
+        merge_request.notes.create!(attributes)
+      end
+
       def build_position(merge_request, pr_comment)
         params = {
           diff_refs: merge_request.diff_refs,
diff --git a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb
index 89abb19592f..44d5b551838 100644
--- a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb
+++ b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb
@@ -168,6 +168,40 @@ describe Gitlab::BitbucketServerImport::Importer do
       expect(reply_note.position.new_line).to eq(inline_note.new_pos)
     end
 
+    it 'falls back to comments if diff comments fail to validate' do
+      # https://gitlab.com/gitlab-org/gitlab-test/compare/c1acaa58bbcbc3eafe538cb8274ba387047b69f8...5937ac0a7beb003549fc5fd26fc247ad
+      inline_note = instance_double(
+        BitbucketServer::Representation::PullRequestComment,
+        file_type: 'REMOVED',
+        from_sha: sample.commits.first,
+        to_sha: sample.commits.last,
+        file_path: '.gitmodules',
+        old_pos: 8,
+        new_pos: 9,
+        note: 'This is a note with an invalid line position.',
+        author_email: project.owner.email,
+        comments: [],
+        created_at: now,
+        updated_at: now)
+
+      inline_comment = instance_double(
+        BitbucketServer::Representation::Activity,
+        comment?: true,
+        inline_comment?: true,
+        merge_event?: false,
+        comment: inline_note)
+
+      expect(subject.client).to receive(:activities).and_return([inline_comment])
+
+      expect { subject.execute }.to change { MergeRequest.count }.by(1)
+
+      merge_request = MergeRequest.first
+      expect(merge_request.notes.count).to eq(1)
+      note = merge_request.notes.first
+
+      expect(note.note).to start_with('Comment on file:')
+    end
+
     it 'restores branches of inaccessible SHAs' do
     end
   end