Commit 970cdfdd authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch 'osw-fix-lfs-images-not-rendering' into 'master'

Fix LFS uploaded images not being rendered

Closes #50617

See merge request gitlab-org/gitlab-ce!22092
parents 1bd08177 caf10464
...@@ -28,7 +28,7 @@ export default { ...@@ -28,7 +28,7 @@ export default {
return diffModes[diffModeKey] || diffModes.replaced; return diffModes[diffModeKey] || diffModes.replaced;
}, },
isTextFile() { isTextFile() {
return this.diffFile.text; return this.diffFile.viewer.name === 'text';
}, },
}, },
}; };
......
...@@ -116,6 +116,10 @@ class DiffFileEntity < Grape::Entity ...@@ -116,6 +116,10 @@ class DiffFileEntity < Grape::Entity
project_blob_path(project, tree_join(diff_file.content_sha, diff_file.new_path)) project_blob_path(project, tree_join(diff_file.content_sha, diff_file.new_path))
end 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| 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_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 image_replaced = diff_file.old_content_sha && diff_file.old_content_sha != diff_file.content_sha
......
# 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
---
title: Fix LFS uploaded images not being rendered
merge_request: 22092
author:
type: fixed
{
"type": "object",
"required": ["name"],
"properties": {
"name": { "type": ["string"] }
},
"additionalProperties": false
}
...@@ -8,13 +8,12 @@ import diffFileMockData from '../mock_data/diff_file'; ...@@ -8,13 +8,12 @@ import diffFileMockData from '../mock_data/diff_file';
describe('DiffContent', () => { describe('DiffContent', () => {
const Component = Vue.extend(DiffContentComponent); const Component = Vue.extend(DiffContentComponent);
let vm; let vm;
const getDiffFileMock = () => Object.assign({}, diffFileMockData);
beforeEach(() => { beforeEach(() => {
vm = mountComponentWithStore(Component, { vm = mountComponentWithStore(Component, {
store, store,
props: { props: {
diffFile: getDiffFileMock(), diffFile: JSON.parse(JSON.stringify(diffFileMockData)),
}, },
}); });
}); });
...@@ -43,7 +42,7 @@ describe('DiffContent', () => { ...@@ -43,7 +42,7 @@ describe('DiffContent', () => {
describe('Non-Text diffs', () => { describe('Non-Text diffs', () => {
beforeEach(() => { beforeEach(() => {
vm.diffFile.text = false; vm.diffFile.viewer.name = 'image';
}); });
describe('image diff', () => { describe('image diff', () => {
......
...@@ -6,11 +6,10 @@ import diffFileMockData from '../mock_data/diff_file'; ...@@ -6,11 +6,10 @@ import diffFileMockData from '../mock_data/diff_file';
describe('DiffFile', () => { describe('DiffFile', () => {
let vm; let vm;
const getDiffFileMock = () => Object.assign({}, diffFileMockData);
beforeEach(() => { beforeEach(() => {
vm = createComponentWithStore(Vue.extend(DiffFileComponent), store, { vm = createComponentWithStore(Vue.extend(DiffFileComponent), store, {
file: getDiffFileMock(), file: JSON.parse(JSON.stringify(diffFileMockData)),
canCurrentUserFork: false, canCurrentUserFork: false,
}).$mount(); }).$mount();
}); });
...@@ -18,7 +17,7 @@ describe('DiffFile', () => { ...@@ -18,7 +17,7 @@ describe('DiffFile', () => {
describe('template', () => { describe('template', () => {
it('should render component with file header, file content components', () => { it('should render component with file header, file content components', () => {
const el = vm.$el; const el = vm.$el;
const { fileHash, filePath } = diffFileMockData; const { fileHash, filePath } = vm.file;
expect(el.id).toEqual(fileHash); expect(el.id).toEqual(fileHash);
expect(el.classList.contains('diff-file')).toEqual(true); expect(el.classList.contains('diff-file')).toEqual(true);
......
...@@ -23,6 +23,9 @@ export default { ...@@ -23,6 +23,9 @@ export default {
aMode: '100644', aMode: '100644',
bMode: '100644', bMode: '100644',
text: true, text: true,
viewer: {
name: 'text',
},
addedLines: 2, addedLines: 2,
removedLines: 0, removedLines: 0,
diffRefs: { diffRefs: {
......
...@@ -26,6 +26,11 @@ describe DiffFileEntity do ...@@ -26,6 +26,11 @@ describe DiffFileEntity do
) )
end 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 # Converted diff files from GitHub import does not contain blob file
# and content sha. # and content sha.
context 'when diff file does not have a blob and content sha' do context 'when diff file does not have a blob and content sha' do
......
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment