Merge branch 'fix-500-on-old-merge-requests' into 'master'

Fix 500 error for old (somewhat) MRs

Closes #36540

See merge request gitlab-org/gitlab-ce!14945

3 files changed, 47 insertions, 3 deletions

diff --git a/changelogs/unreleased/fix-500-on-old-merge-requests.yml b/changelogs/unreleased/fix-500-on-old-merge-requests.yml
new file mode 100644
index 00000000000..765d7466819
--- /dev/null
+++ b/changelogs/unreleased/fix-500-on-old-merge-requests.yml
@@ -0,0 +1,5 @@
+---
+title: Fix 500 errors caused by empty diffs in some discussions
+merge_request: 14945
+author: Alexander Popov
+type: fixed
diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb
index bd0a9502a5e..ccfb908bcca 100644
--- a/lib/gitlab/diff/position.rb
+++ b/lib/gitlab/diff/position.rb
@@ -94,7 +94,9 @@ module Gitlab
       end
 
       def diff_file(repository)
-        @diff_file ||= begin
+        return @diff_file if defined?(@diff_file)
+
+        @diff_file = begin
           if RequestStore.active?
             key = {
               project_id: repository.project.id,
@@ -122,8 +124,8 @@ module Gitlab
 
       def find_diff_file(repository)
         return unless diff_refs.complete?
-
-        diff_refs.compare_in(repository.project).diffs(paths: paths, expanded: true).diff_files.first
+        return unless comparison = diff_refs.compare_in(repository.project)
+        comparison.diffs(paths: paths, expanded: true).diff_files.first
       end
 
       def get_formatter_class(type)
diff --git a/spec/lib/gitlab/diff/position_spec.rb b/spec/lib/gitlab/diff/position_spec.rb
index 245f24e96d4..677eb373d22 100644
--- a/spec/lib/gitlab/diff/position_spec.rb
+++ b/spec/lib/gitlab/diff/position_spec.rb
@@ -364,6 +364,43 @@ describe Gitlab::Diff::Position do
     end
   end
 
+  describe "position for a missing ref" do
+    let(:diff_refs) do
+      Gitlab::Diff::DiffRefs.new(
+        base_sha: "not_existing_sha",
+        head_sha: "existing_sha"
+      )
+    end
+
+    subject do
+      described_class.new(
+        old_path: "files/ruby/feature.rb",
+        new_path: "files/ruby/feature.rb",
+        old_line: 3,
+        new_line: nil,
+        diff_refs: diff_refs
+      )
+    end
+
+    describe "#diff_file" do
+      it "does not raise exception" do
+        expect { subject.diff_file(project.repository) }.not_to raise_error
+      end
+    end
+
+    describe "#diff_line" do
+      it "does not raise exception" do
+        expect { subject.diff_line(project.repository) }.not_to raise_error
+      end
+    end
+
+    describe "#line_code" do
+      it "does not raise exception" do
+        expect { subject.line_code(project.repository) }.not_to raise_error
+      end
+    end
+  end
+
   describe "position for a file in the initial commit" do
     let(:commit) { project.commit("1a0b36b3cdad1d2ee32457c102a8c0b7056fa863") }