Fix LFS uploaded images not being rendered

10 files changed, 56 insertions, 7 deletions

diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue
index 02d5be1821b..fb5556e3cd7 100644
--- a/app/assets/javascripts/diffs/components/diff_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_content.vue
@@ -28,7 +28,7 @@ export default {
       return diffModes[diffModeKey] || diffModes.replaced;
     },
     isTextFile() {
-      return this.diffFile.text;
+      return this.diffFile.viewer.name === 'text';
     },
   },
 };
diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb
index c193ed10fef..63ea8e8f95f 100644
--- a/app/serializers/diff_file_entity.rb
+++ b/app/serializers/diff_file_entity.rb
@@ -116,6 +116,10 @@ class DiffFileEntity < Grape::Entity
     project_blob_path(project, tree_join(diff_file.content_sha, diff_file.new_path))
   end
 
+  expose :viewer, using: DiffViewerEntity do |diff_file|
+    diff_file.rich_viewer || diff_file.simple_viewer
+  end
+
   expose :replaced_view_path, if: -> (_, options) { options[:merge_request] } do |diff_file|
     image_diff = diff_file.rich_viewer && diff_file.rich_viewer.partial_name == 'image'
     image_replaced = diff_file.old_content_sha && diff_file.old_content_sha != diff_file.content_sha
diff --git a/app/serializers/diff_viewer_entity.rb b/app/serializers/diff_viewer_entity.rb
new file mode 100644
index 00000000000..27fba03cb3f
--- /dev/null
+++ b/app/serializers/diff_viewer_entity.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class DiffViewerEntity < Grape::Entity
+  # Partial name refers directly to a Rails feature, let's avoid
+  # using this on the frontend.
+  expose :partial_name, as: :name
+end
diff --git a/changelogs/unreleased/osw-fix-lfs-images-not-rendering.yml b/changelogs/unreleased/osw-fix-lfs-images-not-rendering.yml
new file mode 100644
index 00000000000..5dde22d3158
--- /dev/null
+++ b/changelogs/unreleased/osw-fix-lfs-images-not-rendering.yml
@@ -0,0 +1,5 @@
+---
+title: Fix LFS uploaded images not being rendered
+merge_request: 22092
+author:
+type: fixed
diff --git a/spec/fixtures/api/schemas/entities/diff_viewer.json b/spec/fixtures/api/schemas/entities/diff_viewer.json
new file mode 100644
index 00000000000..19780f49a88
--- /dev/null
+++ b/spec/fixtures/api/schemas/entities/diff_viewer.json
@@ -0,0 +1,8 @@
+{
+  "type": "object",
+  "required": ["name"],
+  "properties": {
+    "name": { "type": ["string"] }
+  },
+  "additionalProperties": false
+}
diff --git a/spec/javascripts/diffs/components/diff_content_spec.js b/spec/javascripts/diffs/components/diff_content_spec.js
index dea600a783a..67f7b569f47 100644
--- a/spec/javascripts/diffs/components/diff_content_spec.js
+++ b/spec/javascripts/diffs/components/diff_content_spec.js
@@ -8,13 +8,12 @@ import diffFileMockData from '../mock_data/diff_file';
 describe('DiffContent', () => {
   const Component = Vue.extend(DiffContentComponent);
   let vm;
-  const getDiffFileMock = () => Object.assign({}, diffFileMockData);
 
   beforeEach(() => {
     vm = mountComponentWithStore(Component, {
       store,
       props: {
-        diffFile: getDiffFileMock(),
+        diffFile: JSON.parse(JSON.stringify(diffFileMockData)),
       },
     });
   });
@@ -43,7 +42,7 @@ describe('DiffContent', () => {
 
   describe('Non-Text diffs', () => {
     beforeEach(() => {
-      vm.diffFile.text = false;
+      vm.diffFile.viewer.name = 'image';
     });
 
     describe('image diff', () => {
diff --git a/spec/javascripts/diffs/components/diff_file_spec.js b/spec/javascripts/diffs/components/diff_file_spec.js
index 2a52cd2b179..13859f43e98 100644
--- a/spec/javascripts/diffs/components/diff_file_spec.js
+++ b/spec/javascripts/diffs/components/diff_file_spec.js
@@ -6,11 +6,10 @@ import diffFileMockData from '../mock_data/diff_file';
 
 describe('DiffFile', () => {
   let vm;
-  const getDiffFileMock = () => Object.assign({}, diffFileMockData);
 
   beforeEach(() => {
     vm = createComponentWithStore(Vue.extend(DiffFileComponent), store, {
-      file: getDiffFileMock(),
+      file: JSON.parse(JSON.stringify(diffFileMockData)),
       canCurrentUserFork: false,
     }).$mount();
   });
@@ -18,7 +17,7 @@ describe('DiffFile', () => {
   describe('template', () => {
     it('should render component with file header, file content components', () => {
       const el = vm.$el;
-      const { fileHash, filePath } = diffFileMockData;
+      const { fileHash, filePath } = vm.file;
 
       expect(el.id).toEqual(fileHash);
       expect(el.classList.contains('diff-file')).toEqual(true);
diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js
index 2aa2f8f3528..d7bc0dbe431 100644
--- a/spec/javascripts/diffs/mock_data/diff_file.js
+++ b/spec/javascripts/diffs/mock_data/diff_file.js
@@ -23,6 +23,9 @@ export default {
   aMode: '100644',
   bMode: '100644',
   text: true,
+  viewer: {
+    name: 'text',
+  },
   addedLines: 2,
   removedLines: 0,
   diffRefs: {
diff --git a/spec/serializers/diff_file_entity_spec.rb b/spec/serializers/diff_file_entity_spec.rb
index 3d90ce44dfb..7497b8f27bd 100644
--- a/spec/serializers/diff_file_entity_spec.rb
+++ b/spec/serializers/diff_file_entity_spec.rb
@@ -26,6 +26,11 @@ describe DiffFileEntity do
       )
     end
 
+    it 'includes viewer' do
+      expect(subject[:viewer].with_indifferent_access)
+          .to match_schema('entities/diff_viewer')
+    end
+
     # Converted diff files from GitHub import does not contain blob file
     # and content sha.
     context 'when diff file does not have a blob and content sha' do
diff --git a/spec/serializers/diff_viewer_entity_spec.rb b/spec/serializers/diff_viewer_entity_spec.rb
new file mode 100644
index 00000000000..66ac6ef2adc
--- /dev/null
+++ b/spec/serializers/diff_viewer_entity_spec.rb
@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+describe DiffViewerEntity do
+  include RepoHelpers
+
+  let(:project) { create(:project, :repository) }
+  let(:repository) { project.repository }
+  let(:commit) { project.commit(sample_commit.id) }
+  let(:diff_refs) { commit.diff_refs }
+  let(:diff) { commit.raw_diffs.first }
+  let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) }
+  let(:viewer) { diff_file.simple_viewer }
+
+  subject { described_class.new(viewer).as_json }
+
+  it 'serializes diff file viewer' do
+    expect(subject.with_indifferent_access).to match_schema('entities/diff_viewer')
+  end
+end