diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
12 files changed, 275 insertions, 28 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 072bcaaad97..941365d9d1d 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -7,6 +7,7 @@ import createFlash from '~/flash'; import PanelResizer from '~/vue_shared/components/panel_resizer.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { isSingleViewStyle } from '~/helpers/diffs_helper'; +import { updateHistory } from '~/lib/utils/url_utility'; import eventHub from '../../notes/event_hub'; import CompareVersions from './compare_versions.vue'; import DiffFile from './diff_file.vue'; @@ -140,6 +141,20 @@ export default { }, }, watch: { + commit(newCommit, oldCommit) { + const commitChangedAfterRender = newCommit && !this.isLoading; + const commitIsDifferent = oldCommit && newCommit.id !== oldCommit.id; + const url = window?.location ? String(window.location) : ''; + + if (commitChangedAfterRender && commitIsDifferent) { + updateHistory({ + title: document.title, + url: url.replace(oldCommit.id, newCommit.id), + }); + this.refetchDiffData(); + this.adjustView(); + } + }, diffViewType() { if (this.needsReload() || this.needsFirstLoad()) { this.refetchDiffData(); @@ -209,6 +224,7 @@ export default { methods: { ...mapActions(['startTaskList']), ...mapActions('diffs', [ + 'moveToNeighboringCommit', 'setBaseConfig', 'fetchDiffFiles', 'fetchDiffFilesMeta', @@ -329,9 +345,16 @@ export default { break; } }); + + if (this.commit && this.glFeatures.mrCommitNeighborNav) { + Mousetrap.bind('c', () => this.moveToNeighboringCommit({ direction: 'next' })); + Mousetrap.bind('x', () => this.moveToNeighboringCommit({ direction: 'previous' })); + } }, removeEventListeners() { Mousetrap.unbind(['[', 'k', ']', 'j']); + Mousetrap.unbind('c'); + Mousetrap.unbind('x'); }, jumpToFile(step) { const targetIndex = this.currentDiffIndex + step; diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index 9d4edd84f25..ee93ca020e8 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -1,10 +1,18 @@ <script> +import { mapActions } from 'vuex'; +import { GlButtonGroup, GlButton, GlIcon, GlTooltipDirective } from '@gitlab/ui'; + +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; + import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import Icon from '~/vue_shared/components/icon.vue'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; + import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; + import initUserPopovers from '../../user_popovers'; +import { setUrlParams } from '../../lib/utils/url_utility'; /** * CommitItem @@ -18,7 +26,16 @@ import initUserPopovers from '../../user_popovers'; * coexist, but there is an issue to remove the duplication. * https://gitlab.com/gitlab-org/gitlab-foss/issues/51613 * + * EXCEPTION WARNING + * 1. The commit navigation buttons (next neighbor, previous neighbor) + * are not duplicated because: + * - We don't have the same data available on the Rails side (yet, + * without backend work) + * - This Vue component should always be what's used when in the + * context of an MR diff, so the HAML should never have any idea + * about navigating among commits. */ + export default { components: { UserAvatarLink, @@ -26,7 +43,14 @@ export default { ClipboardButton, TimeAgoTooltip, CommitPipelineStatus, + GlButtonGroup, + GlButton, + GlIcon, + }, + directives: { + GlTooltip: GlTooltipDirective, }, + mixins: [glFeatureFlagsMixin()], props: { commit: { type: Object, @@ -54,12 +78,28 @@ export default { authorAvatar() { return this.author.avatar_url || this.commit.author_gravatar_url; }, + nextCommitUrl() { + return this.commit.next_commit_id + ? setUrlParams({ commit_id: this.commit.next_commit_id }) + : ''; + }, + previousCommitUrl() { + return this.commit.prev_commit_id + ? setUrlParams({ commit_id: this.commit.prev_commit_id }) + : ''; + }, + hasNeighborCommits() { + return this.commit.next_commit_id || this.commit.prev_commit_id; + }, }, created() { this.$nextTick(() => { initUserPopovers(this.$el.querySelectorAll('.js-user-link')); }); }, + methods: { + ...mapActions('diffs', ['moveToNeighboringCommit']), + }, }; </script> @@ -123,6 +163,41 @@ export default { class="btn btn-default" /> </div> + <div + v-if="hasNeighborCommits && glFeatures.mrCommitNeighborNav" + class="commit-nav-buttons ml-3" + > + <gl-button-group> + <gl-button + :href="previousCommitUrl" + :disabled="!commit.prev_commit_id" + @click.prevent="moveToNeighboringCommit({ direction: 'previous' })" + > + <span + v-if="!commit.prev_commit_id" + v-gl-tooltip + class="h-100 w-100 position-absolute" + :title="__('You\'re at the first commit')" + ></span> + <gl-icon name="chevron-left" /> + {{ __('Prev') }} + </gl-button> + <gl-button + :href="nextCommitUrl" + :disabled="!commit.next_commit_id" + @click.prevent="moveToNeighboringCommit({ direction: 'next' })" + > + <span + v-if="!commit.next_commit_id" + v-gl-tooltip + class="h-100 w-100 position-absolute" + :title="__('You\'re at the last commit')" + ></span> + {{ __('Next') }} + <gl-icon name="chevron-right" /> + </gl-button> + </gl-button-group> + </div> </div> </div> </li> diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue index b0460bacff2..b6a0724c201 100644 --- a/app/assets/javascripts/diffs/components/diff_discussions.vue +++ b/app/assets/javascripts/diffs/components/diff_discussions.vue @@ -85,9 +85,11 @@ export default { :help-page-path="helpPagePath" @noteDeleted="deleteNoteHandler" > - <span v-if="renderAvatarBadge" slot="avatar-badge" class="badge badge-pill"> - {{ index + 1 }} - </span> + <template v-if="renderAvatarBadge" #avatar-badge> + <span class="badge badge-pill"> + {{ index + 1 }} + </span> + </template> </noteable-discussion> </ul> </div> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 82ca3749ac1..54852b113ae 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,6 +1,6 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; -import { escape as esc } from 'lodash'; +import { escape } from 'lodash'; import { GlLoadingIcon } from '@gitlab/ui'; import { __, sprintf } from '~/locale'; import createFlash from '~/flash'; @@ -46,7 +46,7 @@ export default { return sprintf( __('You can %{linkStart}view the blob%{linkEnd} instead.'), { - linkStart: `<a href="${esc(this.file.view_path)}">`, + linkStart: `<a href="${escape(this.file.view_path)}">`, linkEnd: '</a>', }, false, diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index d601c3769a3..61bbf13aa53 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -1,5 +1,5 @@ <script> -import { escape as esc } from 'lodash'; +import { escape } from 'lodash'; import { mapActions, mapGetters } from 'vuex'; import { GlDeprecatedButton, GlTooltipDirective, GlLoadingIcon } from '@gitlab/ui'; import { polyfillSticky } from '~/lib/utils/sticky'; @@ -91,7 +91,7 @@ export default { return this.expanded ? 'chevron-down' : 'chevron-right'; }, viewFileButtonText() { - const truncatedContentSha = esc(truncateSha(this.diffFile.content_sha)); + const truncatedContentSha = escape(truncateSha(this.diffFile.content_sha)); return sprintf( s__('MergeRequests|View file @ %{commitId}'), { commitId: truncatedContentSha }, @@ -99,7 +99,7 @@ export default { ); }, viewReplacedFileButtonText() { - const truncatedBaseSha = esc(truncateSha(this.diffFile.diff_refs.base_sha)); + const truncatedBaseSha = escape(truncateSha(this.diffFile.diff_refs.base_sha)); return sprintf( s__('MergeRequests|View replaced file @ %{commitId}'), { diff --git a/app/assets/javascripts/diffs/components/edit_button.vue b/app/assets/javascripts/diffs/components/edit_button.vue index 91e296f8572..21fdb19287d 100644 --- a/app/assets/javascripts/diffs/components/edit_button.vue +++ b/app/assets/javascripts/diffs/components/edit_button.vue @@ -1,5 +1,6 @@ <script> import { GlTooltipDirective, GlDeprecatedButton } from '@gitlab/ui'; +import { __ } from '~/locale'; import Icon from '~/vue_shared/components/icon.vue'; export default { @@ -13,7 +14,8 @@ export default { props: { editPath: { type: String, - required: true, + required: false, + default: '', }, canCurrentUserFork: { type: Boolean, @@ -25,6 +27,18 @@ export default { default: false, }, }, + computed: { + tooltipTitle() { + if (this.isDisabled) { + return __("Can't edit as source branch was deleted"); + } + + return __('Edit file'); + }, + isDisabled() { + return !this.editPath; + }, + }, methods: { handleEditClick(evt) { if (this.canCurrentUserFork && !this.canModifyBlob) { @@ -37,13 +51,15 @@ export default { </script> <template> - <gl-deprecated-button - v-gl-tooltip.top - :href="editPath" - :title="__('Edit file')" - class="js-edit-blob" - @click.native="handleEditClick" - > - <icon name="pencil" /> - </gl-deprecated-button> + <span v-gl-tooltip.top :title="tooltipTitle"> + <gl-deprecated-button + :href="editPath" + :disabled="isDisabled" + :class="{ 'cursor-not-allowed': isDisabled }" + class="rounded-0 js-edit-blob" + @click.native="handleEditClick" + > + <icon name="pencil" /> + </gl-deprecated-button> + </span> </template> diff --git a/app/assets/javascripts/diffs/components/no_changes.vue b/app/assets/javascripts/diffs/components/no_changes.vue index 5fd68471094..94c2695a945 100644 --- a/app/assets/javascripts/diffs/components/no_changes.vue +++ b/app/assets/javascripts/diffs/components/no_changes.vue @@ -1,6 +1,6 @@ <script> import { mapGetters } from 'vuex'; -import { escape as esc } from 'lodash'; +import { escape } from 'lodash'; import { GlDeprecatedButton } from '@gitlab/ui'; import { __, sprintf } from '~/locale'; @@ -24,8 +24,8 @@ export default { { ref_start: '<span class="ref-name">', ref_end: '</span>', - source_branch: esc(this.getNoteableData.source_branch), - target_branch: esc(this.getNoteableData.target_branch), + source_branch: escape(this.getNoteableData.source_branch), + target_branch: escape(this.getNoteableData.target_branch), }, false, ); diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 93c242e32ac..1975d6996a5 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -16,6 +16,7 @@ import { idleCallback, allDiscussionWrappersExpanded, prepareDiffData, + prepareLineForRenamedFile, } from './utils'; import * as types from './mutation_types'; import { @@ -627,6 +628,42 @@ export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => { } }; +export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { diffFile }) { + return axios + .get(diffFile.context_lines_path, { + params: { + full: true, + from_merge_request: true, + }, + }) + .then(({ data }) => { + const lines = data.map((line, index) => + prepareLineForRenamedFile({ + diffViewType: state.diffViewType, + line, + diffFile, + index, + }), + ); + + commit(types.SET_DIFF_FILE_VIEWER, { + filePath: diffFile.file_path, + viewer: { + ...diffFile.alternate_viewer, + collapsed: false, + }, + }); + commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: diffFile.file_path, lines }); + + dispatch('startRenderDiffsQueue'); + }) + .catch(error => { + dispatch('receiveFullDiffError', diffFile.file_path); + + throw error; + }); +} + export const setFileCollapsed = ({ commit }, { filePath, collapsed }) => commit(types.SET_FILE_COLLAPSED, { filePath, collapsed }); @@ -642,5 +679,48 @@ export const setSuggestPopoverDismissed = ({ commit, state }) => createFlash(s__('MergeRequest|Error dismissing suggestion popover. Please try again.')); }); +export function changeCurrentCommit({ dispatch, commit, state }, { commitId }) { + /* eslint-disable @gitlab/require-i18n-strings */ + if (!commitId) { + return Promise.reject(new Error('`commitId` is a required argument')); + } else if (!state.commit) { + return Promise.reject(new Error('`state` must already contain a valid `commit`')); + } + /* eslint-enable @gitlab/require-i18n-strings */ + + // this is less than ideal, see: https://gitlab.com/gitlab-org/gitlab/-/issues/215421 + const commitRE = new RegExp(state.commit.id, 'g'); + + commit(types.SET_DIFF_FILES, []); + commit(types.SET_BASE_CONFIG, { + ...state, + endpoint: state.endpoint.replace(commitRE, commitId), + endpointBatch: state.endpointBatch.replace(commitRE, commitId), + endpointMetadata: state.endpointMetadata.replace(commitRE, commitId), + }); + + return dispatch('fetchDiffFilesMeta'); +} + +export function moveToNeighboringCommit({ dispatch, state }, { direction }) { + const previousCommitId = state.commit?.prev_commit_id; + const nextCommitId = state.commit?.next_commit_id; + const canMove = { + next: !state.isLoading && nextCommitId, + previous: !state.isLoading && previousCommitId, + }; + let commitId; + + if (direction === 'next' && canMove.next) { + commitId = nextCommitId; + } else if (direction === 'previous' && canMove.previous) { + commitId = previousCommitId; + } + + if (commitId) { + dispatch('changeCurrentCommit', { commitId }); + } +} + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js index acc8874dad8..1e8e736c028 100644 --- a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js +++ b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js @@ -40,10 +40,7 @@ export const diffCompareDropdownTargetVersions = (state, getters) => { }; }; - if (gon.features?.diffCompareWithHead) { - return [...state.mergeRequestDiffs.slice(1).map(formatVersion), baseVersion, headVersion]; - } - return [...state.mergeRequestDiffs.slice(1).map(formatVersion), baseVersion]; + return [...state.mergeRequestDiffs.slice(1).map(formatVersion), baseVersion, headVersion]; }; export const diffCompareDropdownSourceVersions = (state, getters) => { diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 699c61b3ddd..4b1dbc34902 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -41,6 +41,8 @@ export const SET_CURRENT_VIEW_DIFF_FILE_LINES = 'SET_CURRENT_VIEW_DIFF_FILE_LINE export const ADD_CURRENT_VIEW_DIFF_FILE_LINES = 'ADD_CURRENT_VIEW_DIFF_FILE_LINES'; export const TOGGLE_DIFF_FILE_RENDERING_MORE = 'TOGGLE_DIFF_FILE_RENDERING_MORE'; +export const SET_DIFF_FILE_VIEWER = 'SET_DIFF_FILE_VIEWER'; + export const SET_SHOW_SUGGEST_POPOVER = 'SET_SHOW_SUGGEST_POPOVER'; export const TOGGLE_LINE_DISCUSSIONS = 'TOGGLE_LINE_DISCUSSIONS'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 104686993a8..7e89d041c21 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -383,6 +383,11 @@ export default { file.renderingLines = !file.renderingLines; }, + [types.SET_DIFF_FILE_VIEWER](state, { filePath, viewer }) { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + + file.viewer = viewer; + }, [types.SET_SHOW_SUGGEST_POPOVER](state) { state.showSuggestPopover = false; }, diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index dd8dec49a37..2be71c77087 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -233,7 +233,7 @@ export function trimFirstCharOfLineContent(line = {}) { // eslint-disable-next-line no-param-reassign delete line.text; - const parsedLine = Object.assign({}, line); + const parsedLine = { ...line }; if (line.rich_text) { const firstChar = parsedLine.rich_text.charAt(0); @@ -303,6 +303,42 @@ function prepareLine(line) { } } +export function prepareLineForRenamedFile({ line, diffViewType, diffFile, index = 0 }) { + /* + Renamed files are a little different than other diffs, which + is why this is distinct from `prepareDiffFileLines` below. + + We don't get any of the diff file context when we get the diff + (so no "inline" vs. "parallel", no "line_code", etc.). + + We can also assume that both the left and the right of each line + (for parallel diff view type) are identical, because the file + is renamed, not modified. + + This should be cleaned up as part of the effort around flattening our data + ==> https://gitlab.com/groups/gitlab-org/-/epics/2852#note_304803402 + */ + const lineNumber = index + 1; + const cleanLine = { + ...line, + line_code: `${diffFile.file_hash}_${lineNumber}_${lineNumber}`, + new_line: lineNumber, + old_line: lineNumber, + }; + + prepareLine(cleanLine); // WARNING: In-Place Mutations! + + if (diffViewType === PARALLEL_DIFF_VIEW_TYPE) { + return { + left: { ...cleanLine }, + right: { ...cleanLine }, + line_code: cleanLine.line_code, + }; + } + + return cleanLine; +} + function prepareDiffFileLines(file) { const inlineLines = file.highlighted_diff_lines; const parallelLines = file.parallel_diff_lines; @@ -437,7 +473,11 @@ export function getDiffPositionByLineCode(diffFiles, useSingleDiffStyle) { // This method will check whether the discussion is still applicable // to the diff line in question regarding different versions of the MR export function isDiscussionApplicableToLine({ discussion, diffPosition, latestDiff }) { - const { line_code, ...diffPositionCopy } = diffPosition; + const { line_code, ...dp } = diffPosition; + // Removing `line_range` from diffPosition because the backend does not + // yet consistently return this property. This check can be removed, + // once this is addressed. see https://gitlab.com/gitlab-org/gitlab/-/issues/213010 + const { line_range: dpNotUsed, ...diffPositionCopy } = dp; if (discussion.original_position && discussion.position) { const discussionPositions = [ @@ -446,7 +486,14 @@ export function isDiscussionApplicableToLine({ discussion, diffPosition, latestD ...(discussion.positions || []), ]; - return discussionPositions.some(position => isEqual(position, diffPositionCopy)); + const removeLineRange = position => { + const { line_range: pNotUsed, ...positionNoLineRange } = position; + return positionNoLineRange; + }; + + return discussionPositions + .map(removeLineRange) + .some(position => isEqual(position, diffPositionCopy)); } // eslint-disable-next-line |