diff options
20 files changed, 685 insertions, 53 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 4d33ad23f39..a5125c3d077 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -5,7 +5,7 @@ import { polyfillSticky } from '~/lib/utils/sticky'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import Icon from '~/vue_shared/components/icon.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue'; -import { GlTooltipDirective } from '@gitlab/ui'; +import { GlButton, GlTooltipDirective, GlTooltip, GlLoadingIcon } from '@gitlab/ui'; import { truncateSha } from '~/lib/utils/text_utility'; import { __, s__, sprintf } from '~/locale'; import { diffViewerModes } from '~/ide/constants'; @@ -14,6 +14,9 @@ import DiffStats from './diff_stats.vue'; export default { components: { + GlTooltip, + GlLoadingIcon, + GlButton, ClipboardButton, EditButton, Icon, @@ -125,12 +128,15 @@ export default { isModeChanged() { return this.diffFile.viewer.name === diffViewerModes.mode_changed; }, + showExpandDiffToFullFileEnabled() { + return gon.features.expandDiffFullFile && !this.diffFile.is_fully_expanded; + }, }, mounted() { polyfillSticky(this.$refs.header); }, methods: { - ...mapActions('diffs', ['toggleFileDiscussions']), + ...mapActions('diffs', ['toggleFileDiscussions', 'toggleFullDiff']), handleToggleFile(e, checkTarget) { if ( !checkTarget || @@ -240,12 +246,30 @@ export default { v-html="viewReplacedFileButtonText" > </a> - <a + <gl-tooltip :target="() => $refs.viewButton" placement="bottom"> + <span v-html="viewFileButtonText"></span> + </gl-tooltip> + <gl-button + ref="viewButton" :href="diffFile.view_path" - class="btn view-file js-view-file-button" - v-html="viewFileButtonText" + target="blank" + class="view-file js-view-file-button" > - </a> + <icon name="external-link" /> + </gl-button> + <gl-button + v-if="showExpandDiffToFullFileEnabled" + class="expand-file js-expand-file" + @click="toggleFullDiff(diffFile.file_path)" + > + <template v-if="diffFile.isShowingFullFile"> + {{ s__('MRDiff|Show changes only') }} + </template> + <template v-else> + {{ s__('MRDiff|Show full file') }} + </template> + <gl-loading-icon v-if="diffFile.isLoadingFullFile" inline /> + </gl-button> <a v-if="diffFile.external_url" diff --git a/app/assets/javascripts/diffs/components/edit_button.vue b/app/assets/javascripts/diffs/components/edit_button.vue index 803f23b9170..f0cc5de4b33 100644 --- a/app/assets/javascripts/diffs/components/edit_button.vue +++ b/app/assets/javascripts/diffs/components/edit_button.vue @@ -1,5 +1,15 @@ <script> +import { GlTooltipDirective, GlButton } from '@gitlab/ui'; +import Icon from '~/vue_shared/components/icon.vue'; + export default { + components: { + GlButton, + Icon, + }, + directives: { + GlTooltip: GlTooltipDirective, + }, props: { editPath: { type: String, @@ -27,5 +37,13 @@ export default { </script> <template> - <a :href="editPath" class="btn btn-default js-edit-blob" @click="handleEditClick"> Edit </a> + <gl-button + v-gl-tooltip.bottom + :href="editPath" + :title="__('Edit file')" + class="js-edit-blob" + @click.native="handleEditClick" + > + <icon name="pencil" /> + </gl-button> </template> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 7002655ea49..6f380fe6ece 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -42,3 +42,8 @@ export const INITIAL_TREE_WIDTH = 320; export const MIN_TREE_WIDTH = 240; export const MAX_TREE_WIDTH = 400; export const TREE_HIDE_STATS_WIDTH = 260; + +export const OLD_LINE_KEY = 'old_line'; +export const NEW_LINE_KEY = 'new_line'; +export const TYPE_KEY = 'type'; +export const LEFT_LINE_KEY = 'left'; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index c40775c3259..57ddc923a3e 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -309,5 +309,40 @@ export const cacheTreeListWidth = (_, size) => { localStorage.setItem(TREE_LIST_WIDTH_STORAGE_KEY, size); }; +export const requestFullDiff = ({ commit }, filePath) => commit(types.REQUEST_FULL_DIFF, filePath); +export const receiveFullDiffSucess = ({ commit }, { filePath, data }) => + commit(types.RECEIVE_FULL_DIFF_SUCCESS, { filePath, data }); +export const receiveFullDiffError = ({ commit }, filePath) => { + commit(types.RECEIVE_FULL_DIFF_ERROR, filePath); + createFlash(s__('MergeRequest|Error loading full diff. Please try again.')); +}; + +export const fetchFullDiff = ({ dispatch }, file) => + axios + .get(file.context_lines_path, { + params: { + full: true, + from_merge_request: true, + }, + }) + .then(({ data }) => dispatch('receiveFullDiffSucess', { filePath: file.file_path, data })) + .then(() => scrollToElement(`#${file.file_hash}`)) + .catch(() => dispatch('receiveFullDiffError', file.file_path)); + +export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => { + const file = state.diffFiles.find(f => f.file_path === filePath); + + dispatch('requestFullDiff', filePath); + + if (file.isShowingFullFile) { + dispatch('loadCollapsedDiff', file) + .then(() => dispatch('assignDiscussionsToDiff', getters.getDiffFileDiscussions(file))) + .then(() => scrollToElement(`#${file.file_hash}`)) + .catch(() => dispatch('receiveFullDiffError', filePath)); + } else { + dispatch('fetchFullDiff', file); + } +}; + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 71ad108ce88..b441b1de451 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -23,3 +23,7 @@ export const SET_TREE_DATA = 'SET_TREE_DATA'; export const SET_RENDER_TREE_LIST = 'SET_RENDER_TREE_LIST'; export const SET_SHOW_WHITESPACE = 'SET_SHOW_WHITESPACE'; export const TOGGLE_FILE_FINDER_VISIBLE = 'TOGGLE_FILE_FINDER_VISIBLE'; + +export const REQUEST_FULL_DIFF = 'REQUEST_FULL_DIFF'; +export const RECEIVE_FULL_DIFF_SUCCESS = 'RECEIVE_FULL_DIFF_SUCCESS'; +export const RECEIVE_FULL_DIFF_ERROR = 'RECEIVE_FULL_DIFF_ERROR'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 5a27388863c..45187d93fef 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -6,8 +6,10 @@ import { addContextLines, prepareDiffData, isDiscussionApplicableToLine, + convertExpandLines, } from './utils'; import * as types from './mutation_types'; +import { OLD_LINE_KEY, NEW_LINE_KEY, TYPE_KEY, LEFT_LINE_KEY } from '../constants'; export default { [types.SET_BASE_CONFIG](state, options) { @@ -248,4 +250,54 @@ export default { [types.TOGGLE_FILE_FINDER_VISIBLE](state, visible) { state.fileFinderVisible = visible; }, + [types.REQUEST_FULL_DIFF](state, filePath) { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + + file.isLoadingFullFile = true; + }, + [types.RECEIVE_FULL_DIFF_ERROR](state, filePath) { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + + file.isLoadingFullFile = false; + }, + [types.RECEIVE_FULL_DIFF_SUCCESS](state, { filePath, data }) { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + + file.isShowingFullFile = true; + file.isLoadingFullFile = false; + + file.highlighted_diff_lines = convertExpandLines({ + diffLines: file.highlighted_diff_lines, + typeKey: [TYPE_KEY], + oldLineKey: [OLD_LINE_KEY], + newLineKey: [NEW_LINE_KEY], + data, + mapLine: ({ line, oldLine, newLine }) => ({ + ...line, + old_line: oldLine, + new_line: newLine, + line_code: `${file.file_hash}_${oldLine}_${newLine}`, + }), + }); + + file.parallel_diff_lines = convertExpandLines({ + diffLines: file.parallel_diff_lines, + typeKey: [LEFT_LINE_KEY, TYPE_KEY], + oldLineKey: [LEFT_LINE_KEY, OLD_LINE_KEY], + newLineKey: [LEFT_LINE_KEY, NEW_LINE_KEY], + data, + mapLine: ({ line, oldLine, newLine }) => ({ + left: { + ...line, + old_line: oldLine, + line_code: `${file.file_hash}_${oldLine}_${newLine}`, + }, + right: { + ...line, + new_line: newLine, + line_code: `${file.file_hash}_${newLine}_${oldLine}`, + }, + }), + }); + }, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 247d1e65fea..27a79369a24 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -15,8 +15,8 @@ import { TREE_TYPE, } from '../constants'; -export function findDiffFile(files, hash) { - return files.filter(file => file.file_hash === hash)[0]; +export function findDiffFile(files, match, matchKey = 'file_hash') { + return files.find(file => file[matchKey] === match); } export const getReversePosition = linePosition => { @@ -250,6 +250,8 @@ export function prepareDiffData(diffData) { renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, collapsed: file.viewer.name === diffViewerModes.text && showingLines > MAX_LINES_TO_BE_RENDERED, + isShowingFullFile: false, + isLoadingFullFile: false, discussions: [], }); } @@ -411,3 +413,37 @@ export const getDiffMode = diffFile => { diffModes.replaced ); }; + +export const convertExpandLines = ({ + diffLines, + data, + typeKey, + oldLineKey, + newLineKey, + mapLine, +}) => { + const dataLength = data.length; + + return diffLines.reduce((acc, line, i) => { + if (_.property(typeKey)(line) === 'match') { + const beforeLine = diffLines[i - 1]; + const afterLine = diffLines[i + 1]; + const beforeLineIndex = _.property(newLineKey)(beforeLine) || 0; + const afterLineIndex = _.property(newLineKey)(afterLine) - 1 || dataLength; + + acc.push( + ...data.slice(beforeLineIndex, afterLineIndex).map((l, index) => ({ + ...mapLine({ + line: { ...l, hasForm: false, discussions: [] }, + oldLine: (_.property(oldLineKey)(beforeLine) || 0) + index + 1, + newLine: (_.property(newLineKey)(beforeLine) || 0) + index + 1, + }), + })), + ); + } else { + acc.push(line); + } + + return acc; + }, []); +}; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 0dbbe9e4c25..e50db5310a6 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -53,6 +53,10 @@ background-color: $gray-normal; } + a { + color: $gray-700; + } + svg { vertical-align: middle; top: -1px; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 46a44841c31..2903f7d705b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -18,6 +18,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo before_action only: [:show] do push_frontend_feature_flag(:diff_tree_filtering, default_enabled: true) + push_frontend_feature_flag(:expand_diff_full_file) end def index diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb index 01ee7af37ed..13711070a46 100644 --- a/app/serializers/diff_file_entity.rb +++ b/app/serializers/diff_file_entity.rb @@ -7,7 +7,7 @@ class DiffFileEntity < DiffFileBaseEntity expose :added_lines expose :removed_lines - expose :load_collapsed_diff_url, if: -> (diff_file, options) { diff_file.viewer.collapsed? && options[:merge_request] } do |diff_file| + expose :load_collapsed_diff_url, if: -> (diff_file, options) { options[:merge_request] } do |diff_file| merge_request = options[:merge_request] project = merge_request.target_project @@ -57,6 +57,10 @@ class DiffFileEntity < DiffFileBaseEntity diff_file.diff_lines_for_serializer end + expose :is_fully_expanded, if: -> (diff_file, _) { Feature.enabled?(:expand_diff_full_file) && diff_file.text? } do |diff_file| + diff_file.fully_expanded? + end + # Used for parallel diffs expose :parallel_diff_lines, using: DiffLineParallelEntity, if: -> (diff_file, _) { diff_file.text? } end diff --git a/changelogs/unreleased/expand-diff-to-full-file.yml b/changelogs/unreleased/expand-diff-to-full-file.yml new file mode 100644 index 00000000000..f41a6be22e8 --- /dev/null +++ b/changelogs/unreleased/expand-diff-to-full-file.yml @@ -0,0 +1,5 @@ +--- +title: Allow expanding a diff to display full file +merge_request: 24406 +author: +type: added diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index c9d89d56884..88340f044c0 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -329,6 +329,16 @@ module Gitlab lines end + def fully_expanded? + return true if binary? + + lines = diff_lines_for_serializer + + return true if lines.nil? + + lines.none? { |line| line.type.to_s == 'match' } + end + private def total_blob_lines(blob) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index ec36341be61..052b081599a 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2929,6 +2929,9 @@ msgstr "" msgid "Edit environment" msgstr "" +msgid "Edit file" +msgstr "" + msgid "Edit files in the editor and commit changes here" msgstr "" @@ -4521,6 +4524,12 @@ msgstr "" msgid "Logs" msgstr "" +msgid "MRDiff|Show changes only" +msgstr "" + +msgid "MRDiff|Show full file" +msgstr "" + msgid "Make sure you're logged into the account that owns the projects you'd like to import." msgstr "" @@ -4689,6 +4698,9 @@ msgstr "" msgid "MergeRequest| %{paragraphStart}changed the description %{descriptionChangedTimes} times %{timeDifferenceMinutes}%{paragraphEnd}" msgstr "" +msgid "MergeRequest|Error loading full diff. Please try again." +msgstr "" + msgid "MergeRequest|Filter files" msgstr "" diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index aead98dae23..b35f985126c 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -26,7 +26,7 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js visit project_merge_request_path(target_project, merge_request) click_link 'Changes' wait_for_requests - first('.js-file-title').click_link 'Edit' + first('.js-file-title').find('.js-edit-blob').click wait_for_requests end diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index 1201f066d2f..66c5b17b825 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -23,6 +23,9 @@ describe('diff_file_header', () => { }); beforeEach(() => { + gon.features = { + expandDiffFullFile: true, + }; const diffFile = diffDiscussionMock.diff_file; diffFile.added_lines = 2; @@ -382,7 +385,7 @@ describe('diff_file_header', () => { props.diffFile.edit_path = '/'; vm = mountComponentWithStore(Component, { props, store }); - expect(vm.$el.querySelector('.js-edit-blob')).toContainText('Edit'); + expect(vm.$el.querySelector('.js-edit-blob')).not.toBe(null); }); it('should not show edit button when file is deleted', () => { @@ -576,4 +579,66 @@ describe('diff_file_header', () => { }); }); }); + + describe('expand full file button', () => { + beforeEach(() => { + props.addMergeRequestButtons = true; + props.diffFile.edit_path = '/'; + }); + + it('does not render button', () => { + vm = mountComponentWithStore(Component, { props, store }); + + expect(vm.$el.querySelector('.js-expand-file')).toBe(null); + }); + + it('renders button', () => { + props.diffFile.is_fully_expanded = false; + + vm = mountComponentWithStore(Component, { props, store }); + + expect(vm.$el.querySelector('.js-expand-file')).not.toBe(null); + }); + + it('shows fully expanded text', () => { + props.diffFile.is_fully_expanded = false; + props.diffFile.isShowingFullFile = true; + + vm = mountComponentWithStore(Component, { props, store }); + + expect(vm.$el.querySelector('.js-expand-file').textContent).toContain('Show changes only'); + }); + + it('shows expand text', () => { + props.diffFile.is_fully_expanded = false; + + vm = mountComponentWithStore(Component, { props, store }); + + expect(vm.$el.querySelector('.js-expand-file').textContent).toContain('Show full file'); + }); + + it('renders loading icon', () => { + props.diffFile.is_fully_expanded = false; + props.diffFile.isLoadingFullFile = true; + + vm = mountComponentWithStore(Component, { props, store }); + + expect(vm.$el.querySelector('.js-expand-file .loading-container')).not.toBe(null); + }); + + it('calls toggleFullDiff on click', () => { + props.diffFile.is_fully_expanded = false; + + vm = mountComponentWithStore(Component, { props, store }); + + spyOn(vm.$store, 'dispatch').and.stub(); + + vm.$el.querySelector('.js-expand-file').click(); + + expect(vm.$store.dispatch).toHaveBeenCalledWith( + 'diffs/toggleFullDiff', + props.diffFile.file_path, + ); + }); + }); }); diff --git a/spec/javascripts/diffs/mock_data/diff_discussions.js b/spec/javascripts/diffs/mock_data/diff_discussions.js index 4a091b4580b..fd5dd611383 100644 --- a/spec/javascripts/diffs/mock_data/diff_discussions.js +++ b/spec/javascripts/diffs/mock_data/diff_discussions.js @@ -288,6 +288,7 @@ export default { external_storage: null, old_path_html: 'CHANGELOG_OLD', new_path_html: 'CHANGELOG', + is_fully_expanded: true, context_lines_path: '/gitlab-org/gitlab-test/blob/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG/diff', highlighted_diff_lines: [ diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index e47c7906fcb..070bfb2ccd0 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -30,6 +30,11 @@ import actions, { setRenderTreeList, setShowWhitespace, setRenderIt, + requestFullDiff, + receiveFullDiffSucess, + receiveFullDiffError, + fetchFullDiff, + toggleFullDiff, } from '~/diffs/store/actions'; import eventHub from '~/notes/event_hub'; import * as types from '~/diffs/store/mutation_types'; @@ -847,4 +852,129 @@ describe('DiffsStoreActions', () => { testAction(setRenderIt, 'file', {}, [{ type: types.RENDER_FILE, payload: 'file' }], [], done); }); }); + + describe('requestFullDiff', () => { + it('commits REQUEST_FULL_DIFF', done => { + testAction( + requestFullDiff, + 'file', + {}, + [{ type: types.REQUEST_FULL_DIFF, payload: 'file' }], + [], + done, + ); + }); + }); + + describe('receiveFullDiffSucess', () => { + it('commits REQUEST_FULL_DIFF', done => { + testAction( + receiveFullDiffSucess, + { filePath: 'test', data: 'test' }, + {}, + [{ type: types.RECEIVE_FULL_DIFF_SUCCESS, payload: { filePath: 'test', data: 'test' } }], + [], + done, + ); + }); + }); + + describe('receiveFullDiffError', () => { + it('commits REQUEST_FULL_DIFF', done => { + testAction( + receiveFullDiffError, + 'file', + {}, + [{ type: types.RECEIVE_FULL_DIFF_ERROR, payload: 'file' }], + [], + done, + ); + }); + }); + + describe('fetchFullDiff', () => { + let mock; + let scrollToElementSpy; + + beforeEach(() => { + scrollToElementSpy = spyOnDependency(actions, 'scrollToElement').and.stub(); + + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.restore(); + }); + + describe('success', () => { + beforeEach(() => { + mock.onGet(`${gl.TEST_HOST}/context`).replyOnce(200, ['test']); + }); + + it('dispatches receiveFullDiffSucess', done => { + testAction( + fetchFullDiff, + { context_lines_path: `${gl.TEST_HOST}/context`, file_path: 'test', file_hash: 'test' }, + null, + [], + [{ type: 'receiveFullDiffSucess', payload: { filePath: 'test', data: ['test'] } }], + done, + ); + }); + + it('scrolls to element', done => { + fetchFullDiff( + { dispatch() {} }, + { context_lines_path: `${gl.TEST_HOST}/context`, file_path: 'test', file_hash: 'test' }, + ) + .then(() => { + expect(scrollToElementSpy).toHaveBeenCalledWith('#test'); + + done(); + }) + .catch(done.fail); + }); + }); + + describe('error', () => { + beforeEach(() => { + mock.onGet(`${gl.TEST_HOST}/context`).replyOnce(500); + }); + + it('dispatches receiveFullDiffError', done => { + testAction( + fetchFullDiff, + { context_lines_path: `${gl.TEST_HOST}/context`, file_path: 'test', file_hash: 'test' }, + null, + [], + [{ type: 'receiveFullDiffError', payload: 'test' }], + done, + ); + }); + }); + }); + + describe('toggleFullDiff', () => { + let state; + + beforeEach(() => { + state = { + diffFiles: [{ file_path: 'test', isShowingFullFile: false }], + }; + }); + + it('dispatches fetchFullDiff when file is not expanded', done => { + testAction( + toggleFullDiff, + 'test', + state, + [], + [ + { type: 'requestFullDiff', payload: 'test' }, + { type: 'fetchFullDiff', payload: state.diffFiles[0] }, + ], + done, + ); + }); + }); }); diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 09ee691b602..270e7d75666 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -680,4 +680,66 @@ describe('DiffsStoreMutations', () => { expect(state.showWhitespace).toBe(false); }); }); + + describe('REQUEST_FULL_DIFF', () => { + it('sets isLoadingFullFile to true', () => { + const state = { + diffFiles: [{ file_path: 'test', isLoadingFullFile: false }], + }; + + mutations[types.REQUEST_FULL_DIFF](state, 'test'); + + expect(state.diffFiles[0].isLoadingFullFile).toBe(true); + }); + }); + + describe('RECEIVE_FULL_DIFF_ERROR', () => { + it('sets isLoadingFullFile to false', () => { + const state = { + diffFiles: [{ file_path: 'test', isLoadingFullFile: true }], + }; + + mutations[types.RECEIVE_FULL_DIFF_ERROR](state, 'test'); + + expect(state.diffFiles[0].isLoadingFullFile).toBe(false); + }); + }); + + describe('RECEIVE_FULL_DIFF_SUCCESS', () => { + it('sets isLoadingFullFile to false', () => { + const state = { + diffFiles: [ + { + file_path: 'test', + isLoadingFullFile: true, + isShowingFullFile: false, + highlighted_diff_lines: [], + parallel_diff_lines: [], + }, + ], + }; + + mutations[types.RECEIVE_FULL_DIFF_SUCCESS](state, { filePath: 'test', data: [] }); + + expect(state.diffFiles[0].isLoadingFullFile).toBe(false); + }); + + it('sets isShowingFullFile to true', () => { + const state = { + diffFiles: [ + { + file_path: 'test', + isLoadingFullFile: true, + isShowingFullFile: false, + highlighted_diff_lines: [], + parallel_diff_lines: [], + }, + ], + }; + + mutations[types.RECEIVE_FULL_DIFF_SUCCESS](state, { filePath: 'test', data: [] }); + + expect(state.diffFiles[0].isShowingFullFile).toBe(true); + }); + }); }); diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js index 599ea9cd420..1f877910125 100644 --- a/spec/javascripts/diffs/store/utils_spec.js +++ b/spec/javascripts/diffs/store/utils_spec.js @@ -781,4 +781,49 @@ describe('DiffsStoreUtils', () => { ]); }); }); + + describe('convertExpandLines', () => { + it('converts expanded lines to normal lines', () => { + const diffLines = [ + { + type: 'match', + old_line: 1, + new_line: 1, + }, + { + type: '', + old_line: 2, + new_line: 2, + }, + ]; + + const lines = utils.convertExpandLines({ + diffLines, + data: [{ text: 'expanded' }], + typeKey: 'type', + oldLineKey: 'old_line', + newLineKey: 'new_line', + mapLine: ({ line, oldLine, newLine }) => ({ + ...line, + old_line: oldLine, + new_line: newLine, + }), + }); + + expect(lines).toEqual([ + { + text: 'expanded', + new_line: 1, + old_line: 1, + discussions: [], + hasForm: false, + }, + { + type: '', + old_line: 2, + new_line: 2, + }, + ]); + }); + }); }); diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 862590268ca..611c3e946ed 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -8,6 +8,47 @@ describe Gitlab::Diff::File do let(:diff) { commit.raw_diffs.first } let(:diff_file) { described_class.new(diff, diff_refs: commit.diff_refs, repository: project.repository) } + def create_file(file_name, content) + Files::CreateService.new( + project, + project.owner, + commit_message: 'Update', + start_branch: branch_name, + branch_name: branch_name, + file_path: file_name, + file_content: content + ).execute + + project.commit(branch_name).diffs.diff_files.first + end + + def update_file(file_name, content) + Files::UpdateService.new( + project, + project.owner, + commit_message: 'Update', + start_branch: branch_name, + branch_name: branch_name, + file_path: file_name, + file_content: content + ).execute + + project.commit(branch_name).diffs.diff_files.first + end + + def delete_file(file_name) + Files::DeleteService.new( + project, + project.owner, + commit_message: 'Update', + start_branch: branch_name, + branch_name: branch_name, + file_path: file_name + ).execute + + project.commit(branch_name).diffs.diff_files.first + end + describe '#diff_lines' do let(:diff_lines) { diff_file.diff_lines } @@ -675,47 +716,6 @@ describe Gitlab::Diff::File do end let(:branch_name) { 'master' } - def create_file(file_name, content) - Files::CreateService.new( - project, - project.owner, - commit_message: 'Update', - start_branch: branch_name, - branch_name: branch_name, - file_path: file_name, - file_content: content - ).execute - - project.commit(branch_name).diffs.diff_files.first - end - - def update_file(file_name, content) - Files::UpdateService.new( - project, - project.owner, - commit_message: 'Update', - start_branch: branch_name, - branch_name: branch_name, - file_path: file_name, - file_content: content - ).execute - - project.commit(branch_name).diffs.diff_files.first - end - - def delete_file(file_name) - Files::DeleteService.new( - project, - project.owner, - commit_message: 'Update', - start_branch: branch_name, - branch_name: branch_name, - file_path: file_name - ).execute - - project.commit(branch_name).diffs.diff_files.first - end - context 'when empty file is created' do it 'returns true' do diff_file = create_file('empty.md', '') @@ -751,4 +751,123 @@ describe Gitlab::Diff::File do end end end + + describe '#fully_expanded?' do + let(:project) do + create(:project, :custom_repo, files: {}) + end + let(:branch_name) { 'master' } + + context 'when empty file is created' do + it 'returns true' do + diff_file = create_file('empty.md', '') + + expect(diff_file.fully_expanded?).to be_truthy + end + end + + context 'when empty file is deleted' do + it 'returns true' do + create_file('empty.md', '') + diff_file = delete_file('empty.md') + + expect(diff_file.fully_expanded?).to be_truthy + end + end + + context 'when short file with last line removed' do + it 'returns true' do + create_file('with-content.md', (1..3).to_a.join("\n")) + diff_file = update_file('with-content.md', (1..2).to_a.join("\n")) + + expect(diff_file.fully_expanded?).to be_truthy + end + end + + context 'when a single line is added to empty file' do + it 'returns true' do + create_file('empty.md', '') + diff_file = update_file('empty.md', 'new content') + + expect(diff_file.fully_expanded?).to be_truthy + end + end + + context 'when single line file is changed' do + it 'returns true' do + create_file('file.md', 'foo') + diff_file = update_file('file.md', 'bar') + + expect(diff_file.fully_expanded?).to be_truthy + end + end + + context 'when long file is changed' do + before do + create_file('file.md', (1..999).to_a.join("\n")) + end + + context 'when first line is removed' do + it 'returns true' do + diff_file = update_file('file.md', (2..999).to_a.join("\n")) + + expect(diff_file.fully_expanded?).to be_falsey + end + end + + context 'when last line is removed' do + it 'returns true' do + diff_file = update_file('file.md', (1..998).to_a.join("\n")) + + expect(diff_file.fully_expanded?).to be_falsey + end + end + + context 'when first and last lines are removed' do + it 'returns false' do + diff_file = update_file('file.md', (2..998).to_a.join("\n")) + + expect(diff_file.fully_expanded?).to be_falsey + end + end + + context 'when first and last lines are changed' do + it 'returns false' do + content = (2..998).to_a + content.prepend('a') + content.append('z') + content = content.join("\n") + + diff_file = update_file('file.md', content) + + expect(diff_file.fully_expanded?).to be_falsey + end + end + + context 'when every line are changed' do + it 'returns true' do + diff_file = update_file('file.md', "hi\n" * 999) + + expect(diff_file.fully_expanded?).to be_truthy + end + end + + context 'when all contents are cleared' do + it 'returns true' do + diff_file = update_file('file.md', "") + + expect(diff_file.fully_expanded?).to be_truthy + end + end + + context 'when file is binary' do + it 'returns true' do + diff_file = update_file('file.md', (1..998).to_a.join("\n")) + allow(diff_file).to receive(:binary?).and_return(true) + + expect(diff_file.fully_expanded?).to be_truthy + end + end + end + end end |