diff options
15 files changed, 178 insertions, 39 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 82ca10f4163..deddb61ca31 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -26,6 +26,10 @@ export default { type: String, required: true, }, + projectPath: { + type: String, + required: true, + }, shouldShow: { type: Boolean, required: false, @@ -94,18 +98,16 @@ export default { }, }, mounted() { - this.setEndpoint(this.endpoint); - this - .fetchDiffFiles() - .catch(() => { - createFlash(__('Something went wrong on our end. Please try again!')); - }); + this.setBaseConfig({ endpoint: this.endpoint, projectPath: this.projectPath }); + this.fetchDiffFiles().catch(() => { + createFlash(__('Fetching diff files failed. Please reload the page to try again!')); + }); }, created() { this.adjustView(); }, methods: { - ...mapActions(['setEndpoint', 'fetchDiffFiles']), + ...mapActions(['setBaseConfig', 'fetchDiffFiles']), setActive(filePath) { this.activeFile = filePath; }, diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index adcd22f7876..48ba967285f 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -1,5 +1,7 @@ <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 ParallelDiffView from './parallel_diff_view.vue'; @@ -7,6 +9,7 @@ export default { components: { InlineDiffView, ParallelDiffView, + DiffViewer, }, props: { diffFile: { @@ -15,7 +18,18 @@ export default { }, }, computed: { + ...mapState({ + projectPath: state => state.diffs.projectPath, + endpoint: state => state.diffs.endpoint, + }), ...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> @@ -23,16 +37,26 @@ export default { <template> <div class="diff-content"> <div class="diff-viewer"> - <inline-diff-view - v-if="isInlineView" - :diff-file="diffFile" - :diff-lines="diffFile.highlightedDiffLines || []" - /> - <parallel-diff-view - v-if="isParallelView" - :diff-file="diffFile" - :diff-lines="diffFile.parallelDiffLines || []" - /> + <template v-if="isTextFile"> + <inline-diff-view + v-if="isInlineView" + :diff-file="diffFile" + :diff-lines="diffFile.highlightedDiffLines || []" + /> + <parallel-diff-view + v-else-if="isParallelView" + :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> </template> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index 0ed3dc7f3ad..21376117bef 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -36,7 +36,7 @@ export default { <table :class="userColorScheme" :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> <template v-for="(line, index) in normalizedDiffLines" diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index f6840f87034..aae89109c27 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -16,6 +16,7 @@ export default function initDiffsApp(store) { return { endpoint: dataset.endpoint, + projectPath: dataset.projectPath, currentUser: convertObjectPropsToCamelCase(JSON.parse(dataset.currentUserData), { deep: true, }), @@ -31,6 +32,7 @@ export default function initDiffsApp(store) { props: { endpoint: this.endpoint, currentUser: this.currentUser, + projectPath: this.projectPath, shouldShow: this.activeTab === 'diffs', }, }); diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index f8089b314d3..bf188a44022 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -10,8 +10,9 @@ import { DIFF_VIEW_COOKIE_NAME, } from '../constants'; -export const setEndpoint = ({ commit }, endpoint) => { - commit(types.SET_ENDPOINT, endpoint); +export const setBaseConfig = ({ commit }, options) => { + const { endpoint, projectPath } = options; + commit(types.SET_BASE_CONFIG, { endpoint, projectPath }); }; export const setLoadingState = ({ commit }, state) => { @@ -86,7 +87,7 @@ export const expandAllFiles = ({ commit }) => { }; export default { - setEndpoint, + setBaseConfig, setLoadingState, fetchDiffFiles, setInlineDiffViewType, diff --git a/app/assets/javascripts/diffs/store/modules/index.js b/app/assets/javascripts/diffs/store/modules/index.js index 882a098c977..94caa131506 100644 --- a/app/assets/javascripts/diffs/store/modules/index.js +++ b/app/assets/javascripts/diffs/store/modules/index.js @@ -13,6 +13,7 @@ export default { state: { isLoading: true, endpoint: '', + basePath: '', commit: null, diffFiles: [], mergeRequestDiffs: [], diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index a65b205b8e7..63e9239dce4 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -1,4 +1,4 @@ -export const SET_ENDPOINT = 'SET_ENDPOINT'; +export const SET_BASE_CONFIG = 'SET_BASE_CONFIG'; export const SET_LOADING = 'SET_LOADING'; export const SET_DIFF_DATA = 'SET_DIFF_DATA'; export const SET_DIFF_FILES = 'SET_DIFF_FILES'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index fd9ea73e33d..339a33f8b71 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -5,8 +5,9 @@ import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } fro import * as types from './mutation_types'; export default { - [types.SET_ENDPOINT](state, endpoint) { - Object.assign(state, { endpoint }); + [types.SET_BASE_CONFIG](state, options) { + const { endpoint, projectPath } = options; + Object.assign(state, { endpoint, projectPath }); }, [types.SET_LOADING](state, isLoading) { @@ -73,7 +74,7 @@ export default { [types.EXPAND_ALL_FILES](state) { const diffFiles = []; - state.diffFiles.forEach((file) => { + state.diffFiles.forEach(file => { diffFiles.push({ ...file, collapsed: false, diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue index 2c47f5b9b35..d3cbe3c7e74 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue @@ -45,11 +45,15 @@ export default { 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() { - return `${gon.relative_url_root}/${this.projectPath}/raw/${this.oldSha}/${this.oldPath}`; + return `${this.basePath}${this.projectPath}/raw/${this.oldSha}/${this.oldPath}`; }, fullNewPath() { - return `${gon.relative_url_root}/${this.projectPath}/raw/${this.newSha}/${this.newPath}`; + return `${this.basePath}${this.projectPath}/raw/${this.newSha}/${this.newPath}`; }, }, }; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 8e8a879be88..7b8e66b6f12 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -502,6 +502,10 @@ border-bottom: 0; } +.merge-request-details .file-content.image_file img { + max-height: 50vh; +} + .diff-stats-summary-toggler { padding: 0; background-color: transparent; diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index 4fe0ae17ec5..b23baa22d8b 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -73,7 +73,8 @@ = 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?, 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 = spinner diff --git a/spec/javascripts/diffs/components/diff_content_spec.js b/spec/javascripts/diffs/components/diff_content_spec.js index 7237274eb43..dea600a783a 100644 --- a/spec/javascripts/diffs/components/diff_content_spec.js +++ b/spec/javascripts/diffs/components/diff_content_spec.js @@ -1 +1,95 @@ -// 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(); + }); + }); + }); + }); +}); diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index e61780c9928..f0bd098f698 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -12,15 +12,16 @@ import axios from '~/lib/utils/axios_utils'; import testAction from '../../helpers/vuex_action_helper'; describe('DiffsStoreActions', () => { - describe('setEndpoint', () => { - it('should set given endpoint', done => { + describe('setBaseConfig', () => { + it('should set given endpoint and project path', done => { const endpoint = '/diffs/set/endpoint'; + const projectPath = '/root/project'; testAction( - actions.setEndpoint, - endpoint, - { endpoint: '' }, - [{ type: types.SET_ENDPOINT, payload: endpoint }], + actions.setBaseConfig, + { endpoint, projectPath }, + { endpoint: '', projectPath: '' }, + [{ type: types.SET_BASE_CONFIG, payload: { endpoint, projectPath } }], [], done, ); diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 5f1a6e9def7..02836fcaeea 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -3,13 +3,15 @@ import * as types from '~/diffs/store/mutation_types'; import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants'; describe('DiffsStoreMutations', () => { - describe('SET_ENDPOINT', () => { - it('should set endpoint', () => { + describe('SET_BASE_CONFIG', () => { + it('should set endpoint and project path', () => { const state = {}; 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.projectPath).toEqual(projectPath); }); }); diff --git a/spec/javascripts/helpers/init_vue_mr_page_helper.js b/spec/javascripts/helpers/init_vue_mr_page_helper.js index 921d42a0871..05c6d587e9c 100644 --- a/spec/javascripts/helpers/init_vue_mr_page_helper.js +++ b/spec/javascripts/helpers/init_vue_mr_page_helper.js @@ -6,6 +6,7 @@ import diffFileMockData from '../diffs/mock_data/diff_file'; export default function initVueMRPage() { const diffsAppEndpoint = '/diffs/app/endpoint'; + const diffsAppProjectPath = 'testproject'; const mrEl = document.createElement('div'); mrEl.className = 'merge-request fixture-mr'; mrEl.setAttribute('data-mr-action', 'diffs'); @@ -26,6 +27,7 @@ export default function initVueMRPage() { const diffsAppEl = document.createElement('div'); diffsAppEl.id = 'js-diffs-app'; diffsAppEl.setAttribute('data-endpoint', diffsAppEndpoint); + diffsAppEl.setAttribute('data-project-path', diffsAppProjectPath); diffsAppEl.setAttribute('data-current-user-data', JSON.stringify(userDataMock)); document.body.appendChild(diffsAppEl); |