Commit 53f77cfe authored by Tim Zallmann's avatar Tim Zallmann Committed by Clement Ho

Added Diff Viewer to new VUE based MR page

parent 869d6450
...@@ -26,6 +26,10 @@ export default { ...@@ -26,6 +26,10 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
projectPath: {
type: String,
required: true,
},
shouldShow: { shouldShow: {
type: Boolean, type: Boolean,
required: false, required: false,
...@@ -94,18 +98,16 @@ export default { ...@@ -94,18 +98,16 @@ export default {
}, },
}, },
mounted() { mounted() {
this.setEndpoint(this.endpoint); this.setBaseConfig({ endpoint: this.endpoint, projectPath: this.projectPath });
this this.fetchDiffFiles().catch(() => {
.fetchDiffFiles() createFlash(__('Fetching diff files failed. Please reload the page to try again!'));
.catch(() => { });
createFlash(__('Something went wrong on our end. Please try again!'));
});
}, },
created() { created() {
this.adjustView(); this.adjustView();
}, },
methods: { methods: {
...mapActions(['setEndpoint', 'fetchDiffFiles']), ...mapActions(['setBaseConfig', 'fetchDiffFiles']),
setActive(filePath) { setActive(filePath) {
this.activeFile = filePath; this.activeFile = filePath;
}, },
......
<script> <script>
import { mapGetters } from 'vuex'; import { mapGetters, mapState } from 'vuex';
import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
import { diffModes } from '~/ide/constants';
import InlineDiffView from './inline_diff_view.vue'; import InlineDiffView from './inline_diff_view.vue';
import ParallelDiffView from './parallel_diff_view.vue'; import ParallelDiffView from './parallel_diff_view.vue';
...@@ -7,6 +9,7 @@ export default { ...@@ -7,6 +9,7 @@ export default {
components: { components: {
InlineDiffView, InlineDiffView,
ParallelDiffView, ParallelDiffView,
DiffViewer,
}, },
props: { props: {
diffFile: { diffFile: {
...@@ -15,7 +18,18 @@ export default { ...@@ -15,7 +18,18 @@ export default {
}, },
}, },
computed: { computed: {
...mapState({
projectPath: state => state.diffs.projectPath,
endpoint: state => state.diffs.endpoint,
}),
...mapGetters(['isInlineView', 'isParallelView']), ...mapGetters(['isInlineView', 'isParallelView']),
diffMode() {
const diffModeKey = Object.keys(diffModes).find(key => this.diffFile[`${key}File`]);
return diffModes[diffModeKey] || diffModes.replaced;
},
isTextFile() {
return this.diffFile.text;
},
}, },
}; };
</script> </script>
...@@ -23,16 +37,26 @@ export default { ...@@ -23,16 +37,26 @@ export default {
<template> <template>
<div class="diff-content"> <div class="diff-content">
<div class="diff-viewer"> <div class="diff-viewer">
<inline-diff-view <template v-if="isTextFile">
v-if="isInlineView" <inline-diff-view
:diff-file="diffFile" v-if="isInlineView"
:diff-lines="diffFile.highlightedDiffLines || []" :diff-file="diffFile"
/> :diff-lines="diffFile.highlightedDiffLines || []"
<parallel-diff-view />
v-if="isParallelView" <parallel-diff-view
:diff-file="diffFile" v-else-if="isParallelView"
:diff-lines="diffFile.parallelDiffLines || []" :diff-file="diffFile"
/> :diff-lines="diffFile.parallelDiffLines || []"
/>
</template>
<diff-viewer
v-else
:diff-mode="diffMode"
:new-path="diffFile.newPath"
:new-sha="diffFile.diffRefs.headSha"
:old-path="diffFile.oldPath"
:old-sha="diffFile.diffRefs.baseSha"
:project-path="projectPath"/>
</div> </div>
</div> </div>
</template> </template>
...@@ -36,7 +36,7 @@ export default { ...@@ -36,7 +36,7 @@ export default {
<table <table
:class="userColorScheme" :class="userColorScheme"
:data-commit-id="commitId" :data-commit-id="commitId"
class="code diff-wrap-lines js-syntax-highlight text-file"> class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view">
<tbody> <tbody>
<template <template
v-for="(line, index) in normalizedDiffLines" v-for="(line, index) in normalizedDiffLines"
......
...@@ -16,6 +16,7 @@ export default function initDiffsApp(store) { ...@@ -16,6 +16,7 @@ export default function initDiffsApp(store) {
return { return {
endpoint: dataset.endpoint, endpoint: dataset.endpoint,
projectPath: dataset.projectPath,
currentUser: convertObjectPropsToCamelCase(JSON.parse(dataset.currentUserData), { currentUser: convertObjectPropsToCamelCase(JSON.parse(dataset.currentUserData), {
deep: true, deep: true,
}), }),
...@@ -31,6 +32,7 @@ export default function initDiffsApp(store) { ...@@ -31,6 +32,7 @@ export default function initDiffsApp(store) {
props: { props: {
endpoint: this.endpoint, endpoint: this.endpoint,
currentUser: this.currentUser, currentUser: this.currentUser,
projectPath: this.projectPath,
shouldShow: this.activeTab === 'diffs', shouldShow: this.activeTab === 'diffs',
}, },
}); });
......
...@@ -10,8 +10,9 @@ import { ...@@ -10,8 +10,9 @@ import {
DIFF_VIEW_COOKIE_NAME, DIFF_VIEW_COOKIE_NAME,
} from '../constants'; } from '../constants';
export const setEndpoint = ({ commit }, endpoint) => { export const setBaseConfig = ({ commit }, options) => {
commit(types.SET_ENDPOINT, endpoint); const { endpoint, projectPath } = options;
commit(types.SET_BASE_CONFIG, { endpoint, projectPath });
}; };
export const setLoadingState = ({ commit }, state) => { export const setLoadingState = ({ commit }, state) => {
...@@ -86,7 +87,7 @@ export const expandAllFiles = ({ commit }) => { ...@@ -86,7 +87,7 @@ export const expandAllFiles = ({ commit }) => {
}; };
export default { export default {
setEndpoint, setBaseConfig,
setLoadingState, setLoadingState,
fetchDiffFiles, fetchDiffFiles,
setInlineDiffViewType, setInlineDiffViewType,
......
...@@ -13,6 +13,7 @@ export default { ...@@ -13,6 +13,7 @@ export default {
state: { state: {
isLoading: true, isLoading: true,
endpoint: '', endpoint: '',
basePath: '',
commit: null, commit: null,
diffFiles: [], diffFiles: [],
mergeRequestDiffs: [], mergeRequestDiffs: [],
......
export const SET_ENDPOINT = 'SET_ENDPOINT'; export const SET_BASE_CONFIG = 'SET_BASE_CONFIG';
export const SET_LOADING = 'SET_LOADING'; export const SET_LOADING = 'SET_LOADING';
export const SET_DIFF_DATA = 'SET_DIFF_DATA'; export const SET_DIFF_DATA = 'SET_DIFF_DATA';
export const SET_DIFF_FILES = 'SET_DIFF_FILES'; export const SET_DIFF_FILES = 'SET_DIFF_FILES';
......
...@@ -5,8 +5,9 @@ import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } fro ...@@ -5,8 +5,9 @@ import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } fro
import * as types from './mutation_types'; import * as types from './mutation_types';
export default { export default {
[types.SET_ENDPOINT](state, endpoint) { [types.SET_BASE_CONFIG](state, options) {
Object.assign(state, { endpoint }); const { endpoint, projectPath } = options;
Object.assign(state, { endpoint, projectPath });
}, },
[types.SET_LOADING](state, isLoading) { [types.SET_LOADING](state, isLoading) {
...@@ -73,7 +74,7 @@ export default { ...@@ -73,7 +74,7 @@ export default {
[types.EXPAND_ALL_FILES](state) { [types.EXPAND_ALL_FILES](state) {
const diffFiles = []; const diffFiles = [];
state.diffFiles.forEach((file) => { state.diffFiles.forEach(file => {
diffFiles.push({ diffFiles.push({
...file, ...file,
collapsed: false, collapsed: false,
......
...@@ -45,11 +45,15 @@ export default { ...@@ -45,11 +45,15 @@ export default {
return DownloadDiffViewer; return DownloadDiffViewer;
} }
}, },
basePath() {
// We might get the project path from rails with the relative url already setup
return this.projectPath.indexOf('/') === 0 ? '' : `${gon.relative_url_root}/`;
},
fullOldPath() { fullOldPath() {
return `${gon.relative_url_root}/${this.projectPath}/raw/${this.oldSha}/${this.oldPath}`; return `${this.basePath}${this.projectPath}/raw/${this.oldSha}/${this.oldPath}`;
}, },
fullNewPath() { fullNewPath() {
return `${gon.relative_url_root}/${this.projectPath}/raw/${this.newSha}/${this.newPath}`; return `${this.basePath}${this.projectPath}/raw/${this.newSha}/${this.newPath}`;
}, },
}, },
}; };
......
...@@ -502,6 +502,10 @@ ...@@ -502,6 +502,10 @@
border-bottom: 0; border-bottom: 0;
} }
.merge-request-details .file-content.image_file img {
max-height: 50vh;
}
.diff-stats-summary-toggler { .diff-stats-summary-toggler {
padding: 0; padding: 0;
background-color: transparent; background-color: transparent;
......
...@@ -73,7 +73,8 @@ ...@@ -73,7 +73,8 @@
= render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request) = render 'projects/commit/pipelines_list', disable_initialization: true, endpoint: pipelines_project_merge_request_path(@project, @merge_request)
#js-diffs-app.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked?, #js-diffs-app.diffs.tab-pane{ data: { "is-locked" => @merge_request.discussion_locked?,
endpoint: diffs_project_merge_request_path(@project, @merge_request, 'json', request.query_parameters), endpoint: diffs_project_merge_request_path(@project, @merge_request, 'json', request.query_parameters),
current_user_data: UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json } } current_user_data: UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json,
project_path: project_path(@merge_request.project)} }
.mr-loading-status .mr-loading-status
= spinner = spinner
......
// TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034 import Vue from 'vue';
import DiffContentComponent from '~/diffs/components/diff_content.vue';
import store from '~/mr_notes/stores';
import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import { GREEN_BOX_IMAGE_URL, RED_BOX_IMAGE_URL } from 'spec/test_constants';
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(),
},
});
});
describe('text based files', () => {
it('should render diff inline view', done => {
vm.$store.state.diffs.diffViewType = 'inline';
vm.$nextTick(() => {
expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(1);
done();
});
});
it('should render diff parallel view', done => {
vm.$store.state.diffs.diffViewType = 'parallel';
vm.$nextTick(() => {
expect(vm.$el.querySelectorAll('.parallel').length).toEqual(18);
done();
});
});
});
describe('Non-Text diffs', () => {
beforeEach(() => {
vm.diffFile.text = false;
});
describe('image diff', () => {
beforeEach(() => {
vm.diffFile.newPath = GREEN_BOX_IMAGE_URL;
vm.diffFile.newSha = 'DEF';
vm.diffFile.oldPath = RED_BOX_IMAGE_URL;
vm.diffFile.oldSha = 'ABC';
vm.diffFile.viewPath = '';
});
it('should have image diff view in place', done => {
vm.$nextTick(() => {
expect(vm.$el.querySelectorAll('.js-diff-inline-view').length).toEqual(0);
expect(vm.$el.querySelectorAll('.diff-viewer .image').length).toEqual(1);
done();
});
});
});
describe('file diff', () => {
it('should have download buttons in place', done => {
const el = vm.$el;
vm.diffFile.newPath = 'test.abc';
vm.diffFile.newSha = 'DEF';
vm.diffFile.oldPath = 'test.abc';
vm.diffFile.oldSha = 'ABC';
vm.$nextTick(() => {
expect(el.querySelectorAll('.js-diff-inline-view').length).toEqual(0);
expect(el.querySelector('.deleted .file-info').textContent.trim()).toContain('test.abc');
expect(el.querySelector('.deleted .btn.btn-default').textContent.trim()).toContain(
'Download',
);
expect(el.querySelector('.added .file-info').textContent.trim()).toContain('test.abc');
expect(el.querySelector('.added .btn.btn-default').textContent.trim()).toContain(
'Download',
);
done();
});
});
});
});
});
...@@ -12,15 +12,16 @@ import axios from '~/lib/utils/axios_utils'; ...@@ -12,15 +12,16 @@ import axios from '~/lib/utils/axios_utils';
import testAction from '../../helpers/vuex_action_helper'; import testAction from '../../helpers/vuex_action_helper';
describe('DiffsStoreActions', () => { describe('DiffsStoreActions', () => {
describe('setEndpoint', () => { describe('setBaseConfig', () => {
it('should set given endpoint', done => { it('should set given endpoint and project path', done => {
const endpoint = '/diffs/set/endpoint'; const endpoint = '/diffs/set/endpoint';
const projectPath = '/root/project';
testAction( testAction(
actions.setEndpoint, actions.setBaseConfig,
endpoint, { endpoint, projectPath },
{ endpoint: '' }, { endpoint: '', projectPath: '' },
[{ type: types.SET_ENDPOINT, payload: endpoint }], [{ type: types.SET_BASE_CONFIG, payload: { endpoint, projectPath } }],
[], [],
done, done,
); );
......
...@@ -3,13 +3,15 @@ import * as types from '~/diffs/store/mutation_types'; ...@@ -3,13 +3,15 @@ import * as types from '~/diffs/store/mutation_types';
import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants';
describe('DiffsStoreMutations', () => { describe('DiffsStoreMutations', () => {
describe('SET_ENDPOINT', () => { describe('SET_BASE_CONFIG', () => {
it('should set endpoint', () => { it('should set endpoint and project path', () => {
const state = {}; const state = {};
const endpoint = '/diffs/endpoint'; const endpoint = '/diffs/endpoint';
const projectPath = '/root/project';
mutations[types.SET_ENDPOINT](state, endpoint); mutations[types.SET_BASE_CONFIG](state, { endpoint, projectPath });
expect(state.endpoint).toEqual(endpoint); expect(state.endpoint).toEqual(endpoint);
expect(state.projectPath).toEqual(projectPath);
}); });
}); });
......
...@@ -6,6 +6,7 @@ import diffFileMockData from '../diffs/mock_data/diff_file'; ...@@ -6,6 +6,7 @@ import diffFileMockData from '../diffs/mock_data/diff_file';
export default function initVueMRPage() { export default function initVueMRPage() {
const diffsAppEndpoint = '/diffs/app/endpoint'; const diffsAppEndpoint = '/diffs/app/endpoint';
const diffsAppProjectPath = 'testproject';
const mrEl = document.createElement('div'); const mrEl = document.createElement('div');
mrEl.className = 'merge-request fixture-mr'; mrEl.className = 'merge-request fixture-mr';
mrEl.setAttribute('data-mr-action', 'diffs'); mrEl.setAttribute('data-mr-action', 'diffs');
...@@ -26,6 +27,7 @@ export default function initVueMRPage() { ...@@ -26,6 +27,7 @@ export default function initVueMRPage() {
const diffsAppEl = document.createElement('div'); const diffsAppEl = document.createElement('div');
diffsAppEl.id = 'js-diffs-app'; diffsAppEl.id = 'js-diffs-app';
diffsAppEl.setAttribute('data-endpoint', diffsAppEndpoint); diffsAppEl.setAttribute('data-endpoint', diffsAppEndpoint);
diffsAppEl.setAttribute('data-project-path', diffsAppProjectPath);
diffsAppEl.setAttribute('data-current-user-data', JSON.stringify(userDataMock)); diffsAppEl.setAttribute('data-current-user-data', JSON.stringify(userDataMock));
document.body.appendChild(diffsAppEl); document.body.appendChild(diffsAppEl);
......
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