diff options
Diffstat (limited to 'app/models/diff_note.rb')
-rw-r--r-- | app/models/diff_note.rb | 40 |
1 files changed, 28 insertions, 12 deletions
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 686d06d3ee0..939d8bc4bef 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -23,6 +23,12 @@ class DiffNote < Note before_validation :set_line_code, if: :on_text?, unless: :importing? after_save :keep_around_commits, unless: :importing? + + NoteDiffFileCreationError = Class.new(StandardError) + + DIFF_LINE_NOT_FOUND_MESSAGE = "Failed to find diff line for: %{file_path}, old_line: %{old_line}, new_line: %{new_line}" + DIFF_FILE_NOT_FOUND_MESSAGE = "Failed to find diff file" + after_commit :create_diff_file, on: :create def discussion_class(*) @@ -33,7 +39,16 @@ class DiffNote < Note return unless should_create_diff_file? diff_file = fetch_diff_file + raise NoteDiffFileCreationError, DIFF_FILE_NOT_FOUND_MESSAGE unless diff_file + diff_line = diff_file.line_for_position(self.original_position) + unless diff_line + raise NoteDiffFileCreationError, DIFF_LINE_NOT_FOUND_MESSAGE % { + file_path: diff_file.file_path, + old_line: original_position.old_line, + new_line: original_position.new_line + } + end creation_params = diff_file.diff.to_hash .except(:too_large) @@ -110,19 +125,20 @@ class DiffNote < Note def fetch_diff_file return note_diff_file.raw_diff_file if note_diff_file - file = - if created_at_diff?(noteable.diff_refs) - # We're able to use the already persisted diffs (Postgres) if we're - # presenting a "current version" of the MR discussion diff. - # So no need to make an extra Gitaly diff request for it. - # As an extra benefit, the returned `diff_file` already - # has `highlighted_diff_lines` data set from Redis on - # `Diff::FileCollection::MergeRequestDiff`. - noteable.diffs(original_position.diff_options).diff_files.first - else - original_position.diff_file(repository) - end + if created_at_diff?(noteable.diff_refs) + # We're able to use the already persisted diffs (Postgres) if we're + # presenting a "current version" of the MR discussion diff. + # So no need to make an extra Gitaly diff request for it. + # As an extra benefit, the returned `diff_file` already + # has `highlighted_diff_lines` data set from Redis on + # `Diff::FileCollection::MergeRequestDiff`. + file = noteable.diffs(original_position.diff_options).diff_files.first + # if line is not found in persisted diffs, fallback and retrieve file from repository using gitaly + # This is required because of https://gitlab.com/gitlab-org/gitlab/issues/42676 + file = nil if file&.line_for_position(original_position).nil? && importing? + end + file ||= original_position.diff_file(repository) file&.unfold_diff_lines(position) file |