Fix NoMethodError for GitHub suggestions with stale diff positions

What does this MR do and why?

GitHub can return a review comment with a null start_line or line field once its diff position goes stale, while the paired original_start_line/original_line stay pointed at the comment's original range. SuggestionFormatter#line_count assumed the pair was always complete, so a null value crashed import for review comments containing a suggestion block.

Fall back to original_start_line/original_end_line in that case, so the suggestion range is computed from the comment's original position instead of raising or collapsing to a single line.

References

#434907 (closed)

Screenshots or screen recordings

Imported diff note

How to set up and validate locally

The script below imports a single GitHub pull request (kubernetes/kubernetes#113599) and its review comments into a throwaway GitLab project. It exercises the exact code path fixed in this MR — a review comment whose diff position has gone stale (line/start_line are null) but whose original_line/original_start_line are still set.

Ruby script

# frozen_string_literal: true
#
# One-off script to import a SINGLE GitHub pull request (with its diff and
# review comments) into a throwaway GitLab project, to manually verify the
# SuggestionFormatter original_line/original_start_line fallback added on
# branch cm/fix-434907 (https://gitlab.com/gitlab-org/gitlab/-/work_items/434907).
#
# It deliberately does NOT run GitLab's full GitHub project importer (which
# would import every issue/PR/label/comment). Instead it calls the same
# importer classes the real pipeline uses directly: RepositoryImporter (to
# get a real base..head diff, cloning the full kubernetes/kubernetes repo -
# this GDK's Gitaly is a Praefect/Gitaly Cluster setup, so there is no local
# disk path to do a cheaper targeted git fetch against), then
# PullRequestImporter and DiffNoteImporter for the one PR.
#
# Usage (from the gitlab worktree root):
#   bin/rails runner /path/to/import_pr_113599.rb
#
# Optional env vars:
#   GITHUB_TOKEN         - GitHub personal access token (raises the 60/hr
#                          unauthenticated rate limit; not required for this
#                          single PR).
#   TEST_NAMESPACE_PATH  - full path of the namespace/user to own the test
#                          project (default: your own user, falling back to
#                          "root").
#
# Re-runnable: it destroys and recreates the target project every time.

REPO = 'kubernetes/kubernetes'
PR_NUMBER = 113599
PROJECT_PATH = 'github-pr-113599-test'
GITHUB_URL = "https://github.com/#{REPO}.git".freeze

def log(msg)
  puts "[import_pr_113599] #{msg}"
end

user = User.find_by(email: 'rtomonari@gitlab.com') || User.find_by(username: 'root')
raise 'Could not find a default user to own the test project' unless user

namespace_path = ENV['TEST_NAMESPACE_PATH']
namespace = namespace_path ? Namespace.find_by_full_path(namespace_path) : user.namespace
raise "Could not find namespace #{namespace_path.inspect}" unless namespace

log("Owner: #{user.username}, namespace: #{namespace.full_path}")

if (existing = Project.find_by_full_path("#{namespace.full_path}/#{PROJECT_PATH}"))
  log("Destroying existing test project #{existing.full_path}")
  ::Projects::DestroyService.new(existing, user).execute
end

log('Creating empty test project')
project = ::Projects::CreateService.new(
  user,
  name: 'GitHub PR 113599 test',
  path: PROJECT_PATH,
  namespace_id: namespace.id,
  visibility_level: Gitlab::VisibilityLevel::PRIVATE
).execute

raise "Project creation failed: #{project.errors.full_messages}" unless project.persisted?

# Set these directly instead of via CreateService so we don't trigger the
# real (whole-project) GitHub import pipeline.
project.import_url = GITHUB_URL
project.import_type = 'github'
project.import_source = REPO
project.save!

client = Gitlab::GithubImport::Client.new(ENV['GITHUB_TOKEN'], parallel: false)

log("Fetching pull_request ##{PR_NUMBER} from #{REPO}")
pr_hash = client.pull_request(REPO, PR_NUMBER)
pull_request = Gitlab::GithubImport::Representation::PullRequest.from_api_response(pr_hash)

log('Importing the full kubernetes/kubernetes repository (this can take a while)')
Gitlab::GithubImport::Importer::RepositoryImporter.new(project, client).execute

log('Importing the pull request as a merge request')
Gitlab::GithubImport::Importer::PullRequestImporter.new(pull_request, project, client).execute

merge_request = project.merge_requests.find_by(iid: PR_NUMBER)
raise 'Merge request was not created' unless merge_request

log("Merge request created: iid=#{merge_request.iid}, id=#{merge_request.id}")

log('Importing review (diff) comments')
imported = 0
client.each_object(:pull_request_comments, REPO, PR_NUMBER) do |note_hash|
  diff_note = Gitlab::GithubImport::Representation::DiffNote.from_api_response(note_hash)
  Gitlab::GithubImport::Importer::DiffNoteImporter.new(diff_note, project, client).execute
  imported += 1
end

log("Imported #{imported} review comment(s)")
log("Done. Project: #{Gitlab.config.gitlab.url}/#{project.full_path}")
log("Merge request: #{Gitlab.config.gitlab.url}/#{project.full_path}/-/merge_requests/#{merge_request.iid}")
log('Look for the comment from "alexzielenski" with the ```suggestion block - ' \
  'its range should now be "-1+0" instead of "-0+0".')

Steps:

1. Check out this branch locally:

   git fetch origin rodrigo/434907-diff-note-nil-line
   git checkout rodrigo/434907-diff-note-nil-line

2. (Optional) Set a GitHub personal access token to avoid the 60 req/hr unauthenticated rate limit:

   export GITHUB_TOKEN=<your_token>

3. Save the script above as /tmp/import_pr_113599.rb.

4. Run it from the GitLab root with Rails runner (the repository clone step takes several minutes):

   bin/rails runner /tmp/import_pr_113599.rb

5. Open the merge request URL printed at the end of the output.

6. Find the review comment by alexzielenski that contains a ```suggestion ``` block.

Expected result (this branch): the suggestion renders correctly and the diff note is saved without error. The log line confirms the range is -1+0 (one-line suggestion) rather than -0+0.

Expected result (unpatched master): bin/rails runner raises NoMethodError and the import aborts before any diff notes are saved.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.