Merge branch 'fix-old-mr-diffs' into 'master'9-5-stable-patch-2

Fix old MR diffs

Closes #36516

See merge request !13744

3 files changed, 70 insertions, 5 deletions

diff --git a/changelogs/unreleased/fix-old-mr-diffs.yml b/changelogs/unreleased/fix-old-mr-diffs.yml
new file mode 100644
index 00000000000..b0a011cf354
--- /dev/null
+++ b/changelogs/unreleased/fix-old-mr-diffs.yml
@@ -0,0 +1,6 @@
+---
+title: Show un-highlighted text diffs when we do not have references to the correct
+  blobs
+merge_request:
+author:
+type: fixed
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index 6d7de52cb80..1dabd4ebdd0 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -186,7 +186,10 @@ module Gitlab
       end
 
       def content_changed?
-        old_blob && new_blob && old_blob.id != new_blob.id
+        return blobs_changed? if diff_refs
+        return false if new_file? || deleted_file? || renamed_file?
+
+        text? && diff_lines.any?
       end
 
       def different_type?
@@ -225,6 +228,10 @@ module Gitlab
 
       private
 
+      def blobs_changed?
+        old_blob && new_blob && old_blob.id != new_blob.id
+      end
+
       def simple_viewer_class
         return DiffViewer::NotDiffable unless diffable?
 
@@ -250,6 +257,8 @@ module Gitlab
           DiffViewer::Renamed
         elsif mode_changed?
           DiffViewer::ModeChanged
+        else
+          DiffViewer::NoPreview
         end
       end
 
diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb
index d3d841b0668..c91895cedc3 100644
--- a/spec/lib/gitlab/diff/file_spec.rb
+++ b/spec/lib/gitlab/diff/file_spec.rb
@@ -15,6 +15,17 @@ describe Gitlab::Diff::File do
     it { expect(diff_lines.first).to be_kind_of(Gitlab::Diff::Line) }
   end
 
+  describe '#highlighted_diff_lines' do
+    it 'highlights the diff and memoises the result' do
+      expect(Gitlab::Diff::Highlight).to receive(:new)
+                                           .with(diff_file, repository: project.repository)
+                                           .once
+                                           .and_call_original
+
+      diff_file.highlighted_diff_lines
+    end
+  end
+
   describe '#mode_changed?' do
     it { expect(diff_file.mode_changed?).to be_falsey }
   end
@@ -122,8 +133,20 @@ describe Gitlab::Diff::File do
         let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
         let(:diff_file) { commit.diffs.diff_file_with_new_path('files/images/6049019_460s.jpg') }
 
-        it 'returns true' do
-          expect(diff_file.content_changed?).to be_truthy
+        context 'when the blobs are different' do
+          it 'returns true' do
+            expect(diff_file.content_changed?).to be_truthy
+          end
+        end
+
+        context 'when there are no diff refs' do
+          before do
+            allow(diff_file).to receive(:diff_refs).and_return(nil)
+          end
+
+          it 'returns false' do
+            expect(diff_file.content_changed?).to be_falsey
+          end
         end
       end
 
@@ -131,8 +154,20 @@ describe Gitlab::Diff::File do
         let(:commit) { project.commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') }
         let(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') }
 
-        it 'returns true' do
-          expect(diff_file.content_changed?).to be_truthy
+        context 'when the blobs are different' do
+          it 'returns true' do
+            expect(diff_file.content_changed?).to be_truthy
+          end
+        end
+
+        context 'when there are no diff refs' do
+          before do
+            allow(diff_file).to receive(:diff_refs).and_return(nil)
+          end
+
+          it 'returns true' do
+            expect(diff_file.content_changed?).to be_truthy
+          end
         end
       end
     end
@@ -270,6 +305,21 @@ describe Gitlab::Diff::File do
         expect(diff_file.simple_viewer).to be_a(DiffViewer::ModeChanged)
       end
     end
+
+    context 'when no other conditions apply' do
+      before do
+        allow(diff_file).to receive(:content_changed?).and_return(false)
+        allow(diff_file).to receive(:new_file?).and_return(false)
+        allow(diff_file).to receive(:deleted_file?).and_return(false)
+        allow(diff_file).to receive(:renamed_file?).and_return(false)
+        allow(diff_file).to receive(:mode_changed?).and_return(false)
+        allow(diff_file).to receive(:raw_text?).and_return(false)
+      end
+
+      it 'returns a No Preview viewer' do
+        expect(diff_file.simple_viewer).to be_a(DiffViewer::NoPreview)
+      end
+    end
   end
 
   describe '#rich_viewer' do