diff options
author | Robert Speicher <rspeicher@gmail.com> | 2021-01-20 13:34:23 -0600 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2021-01-20 13:34:23 -0600 |
commit | 6438df3a1e0fb944485cebf07976160184697d72 (patch) | |
tree | 00b09bfd170e77ae9391b1a2f5a93ef6839f2597 /app/assets/javascripts/diffs | |
parent | 42bcd54d971da7ef2854b896a7b34f4ef8601067 (diff) | |
download | gitlab-ce-6438df3a1e0fb944485cebf07976160184697d72.tar.gz |
Add latest changes from gitlab-org/gitlab@13-8-stable-eev13.8.0-rc42
Diffstat (limited to 'app/assets/javascripts/diffs')
32 files changed, 706 insertions, 302 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 7827c78b658..32822fe1fe8 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -124,6 +124,16 @@ export default { required: false, default: false, }, + defaultSuggestionCommitMessage: { + type: String, + required: false, + default: '', + }, + mrReviews: { + type: Object, + required: false, + default: () => ({}), + }, }, data() { const treeWidth = @@ -136,19 +146,17 @@ export default { }, computed: { ...mapState({ - isLoading: state => state.diffs.isLoading, - isBatchLoading: state => state.diffs.isBatchLoading, - diffFiles: state => state.diffs.diffFiles, - diffViewType: state => state.diffs.diffViewType, - mergeRequestDiffs: state => state.diffs.mergeRequestDiffs, - mergeRequestDiff: state => state.diffs.mergeRequestDiff, - commit: state => state.diffs.commit, - renderOverflowWarning: state => state.diffs.renderOverflowWarning, - numTotalFiles: state => state.diffs.realSize, - numVisibleFiles: state => state.diffs.size, - plainDiffPath: state => state.diffs.plainDiffPath, - emailPatchPath: state => state.diffs.emailPatchPath, - retrievingBatches: state => state.diffs.retrievingBatches, + isLoading: (state) => state.diffs.isLoading, + isBatchLoading: (state) => state.diffs.isBatchLoading, + diffFiles: (state) => state.diffs.diffFiles, + diffViewType: (state) => state.diffs.diffViewType, + commit: (state) => state.diffs.commit, + renderOverflowWarning: (state) => state.diffs.renderOverflowWarning, + numTotalFiles: (state) => state.diffs.realSize, + numVisibleFiles: (state) => state.diffs.size, + plainDiffPath: (state) => state.diffs.plainDiffPath, + emailPatchPath: (state) => state.diffs.emailPatchPath, + retrievingBatches: (state) => state.diffs.retrievingBatches, }), ...mapState('diffs', [ 'showTreeList', @@ -161,7 +169,12 @@ export default { 'hasConflicts', 'viewDiffsFileByFile', ]), - ...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']), + ...mapGetters('diffs', [ + 'whichCollapsedTypes', + 'isParallelView', + 'currentDiffIndex', + 'fileReviews', + ]), ...mapGetters(['isNotesFetched', 'getNoteableData']), diffs() { if (!this.viewDiffsFileByFile) { @@ -176,17 +189,16 @@ export default { return this.currentUser.can_fork === true && this.currentUser.can_create_merge_request; }, renderDiffFiles() { - return ( - this.diffFiles.length > 0 || - (this.startVersion && - this.startVersion.version_index === this.mergeRequestDiff.version_index) - ); + return this.diffFiles.length > 0; + }, + renderFileTree() { + return this.renderDiffFiles && this.showTreeList; }, hideFileStats() { return this.treeWidth <= TREE_HIDE_STATS_WIDTH; }, isLimitedContainer() { - return !this.showTreeList && !this.isParallelView && !this.isFluidLayout; + return !this.renderFileTree && !this.isParallelView && !this.isFluidLayout; }, isDiffHead() { return parseBoolean(getParameterByName('diff_head')); @@ -249,7 +261,7 @@ export default { this.adjustView(); }, isLoading: 'adjustView', - showTreeList: 'adjustView', + renderFileTree: 'adjustView', }, mounted() { this.setBaseConfig({ @@ -261,6 +273,8 @@ export default { dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, viewDiffsFileByFile: fileByFile(this.fileByFileUserPreference), + defaultSuggestionCommitMessage: this.defaultSuggestionCommitMessage, + mrReviews: this.mrReviews || {}, }); if (this.shouldShow) { @@ -270,12 +284,7 @@ export default { const id = window?.location?.hash; if (id && id.indexOf('#note') !== 0) { - this.setHighlightedRow( - id - .split('diff-content') - .pop() - .slice(1), - ); + this.setHighlightedRow(id.split('diff-content').pop().slice(1)); } }, beforeCreate() { @@ -393,10 +402,7 @@ export default { }, setDiscussions() { requestIdleCallback( - () => - this.assignDiscussionsToDiff() - .then(this.$nextTick) - .then(this.startTaskList), + () => this.assignDiscussionsToDiff().then(this.$nextTick).then(this.startTaskList), { timeout: 1000 }, ); }, @@ -425,7 +431,7 @@ export default { } }); - if (this.commit && this.glFeatures.mrCommitNeighborNav) { + if (this.commit) { Mousetrap.bind('c', () => this.moveToNeighboringCommit({ direction: 'next' })); Mousetrap.bind('x', () => this.moveToNeighboringCommit({ direction: 'previous' })); } @@ -464,7 +470,6 @@ export default { <div v-if="isLoading || !isTreeLoaded" class="loading"><gl-loading-icon size="lg" /></div> <div v-else id="diffs" :class="{ active: shouldShow }" class="diffs tab-pane"> <compare-versions - :merge-request-diffs="mergeRequestDiffs" :is-limited-container="isLimitedContainer" :diff-files-count-text="numTotalFiles" /> @@ -492,7 +497,7 @@ export default { class="files d-flex gl-mt-2" > <div - v-if="showTreeList" + v-if="renderFileTree" :style="{ width: `${treeWidth}px` }" class="diff-tree-list js-diff-tree-list px-3 pr-md-0" > @@ -519,6 +524,7 @@ export default { v-for="(file, index) in diffs" :key="file.newPath" :file="file" + :reviewed="fileReviews[index]" :is-first-file="index === 0" :is-last-file="index === diffs.length - 1" :help-page-path="helpPagePath" diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index a548354f257..af19f90ee77 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -6,7 +6,7 @@ import { GlButtonGroup, GlButton, GlTooltipDirective, GlIcon } 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 ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; +import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; @@ -39,7 +39,7 @@ import { setUrlParams } from '../../lib/utils/url_utility'; export default { components: { UserAvatarLink, - ClipboardButton, + ModalCopyButton, TimeAgoTooltip, CommitPipelineStatus, GlButtonGroup, @@ -142,16 +142,13 @@ export default { data-testid="commit-sha-short-id" v-text="commit.short_id" /> - <clipboard-button + <modal-copy-button :text="commit.id" :title="__('Copy commit SHA')" class="input-group-text" /> </gl-button-group> - <div - v-if="hasNeighborCommits && glFeatures.mrCommitNeighborNav" - class="commit-nav-buttons ml-3" - > + <div v-if="hasNeighborCommits" class="commit-nav-buttons ml-3"> <gl-button-group> <gl-button :href="previousCommitUrl" diff --git a/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue b/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue index da34a7ee19b..2c249f71091 100644 --- a/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue +++ b/app/assets/javascripts/diffs/components/compare_dropdown_layout.vue @@ -16,7 +16,7 @@ export default { }, computed: { selectedVersionName() { - return this.versions.find(x => x.selected)?.versionName || ''; + return this.versions.find((x) => x.selected)?.versionName || ''; }, }, }; diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index f3cc359a679..489278fd6ef 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -22,10 +22,6 @@ export default { GlTooltip: GlTooltipDirective, }, props: { - mergeRequestDiffs: { - type: Array, - required: true, - }, isLimitedContainer: { type: Boolean, required: false, @@ -44,6 +40,7 @@ export default { 'diffCompareDropdownSourceVersions', ]), ...mapState('diffs', [ + 'diffFiles', 'commit', 'showTreeList', 'startVersion', @@ -51,12 +48,15 @@ export default { 'addedLines', 'removedLines', ]), - showDropdowns() { - return !this.commit && this.mergeRequestDiffs.length; - }, toggleFileBrowserTitle() { return this.showTreeList ? __('Hide file browser') : __('Show file browser'); }, + hasChanges() { + return this.diffFiles.length > 0; + }, + hasSourceVersions() { + return this.diffCompareDropdownSourceVersions.length > 0; + }, }, created() { this.CENTERED_LIMITED_CONTAINER_CLASSES = CENTERED_LIMITED_CONTAINER_CLASSES; @@ -82,6 +82,7 @@ export default { }" > <gl-button + v-if="hasChanges" v-gl-tooltip.hover variant="default" icon="file-tree" @@ -90,8 +91,12 @@ export default { :selected="showTreeList" @click="setShowTreeList({ showTreeList: !showTreeList })" /> + <div v-if="commit"> + {{ __('Viewing commit') }} + <gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link> + </div> <gl-sprintf - v-if="showDropdowns" + v-else-if="hasSourceVersions" class="d-flex align-items-center compare-versions-container" :message="s__('MergeRequest|Compare %{target} and %{source}')" > @@ -109,11 +114,7 @@ export default { /> </template> </gl-sprintf> - <div v-else-if="commit"> - {{ __('Viewing commit') }} - <gl-link :href="commit.commit_url" class="monospace">{{ commit.short_id }}</gl-link> - </div> - <div class="inline-parallel-buttons d-none d-md-flex ml-auto"> + <div v-if="hasChanges" class="inline-parallel-buttons d-none d-md-flex ml-auto"> <diff-stats :diff-files-count-text="diffFilesCountText" :added-lines="addedLines" diff --git a/app/assets/javascripts/diffs/components/diff_content.vue b/app/assets/javascripts/diffs/components/diff_content.vue index f938ea368d8..f4e2571dd09 100644 --- a/app/assets/javascripts/diffs/components/diff_content.vue +++ b/app/assets/javascripts/diffs/components/diff_content.vue @@ -50,7 +50,7 @@ export default { }, computed: { ...mapState({ - projectPath: state => state.diffs.projectPath, + projectPath: (state) => state.diffs.projectPath, }), ...mapGetters('diffs', [ 'isInlineView', diff --git a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue index 2401e12e4f6..2d1a7237122 100644 --- a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue @@ -13,7 +13,7 @@ const EXPAND_DOWN = 2; const lineNumberByViewType = (viewType, diffLine) => { const numberGetters = { - [INLINE_DIFF_VIEW_TYPE]: line => line?.new_line, + [INLINE_DIFF_VIEW_TYPE]: (line) => line?.new_line, }; const numberGetter = numberGetters[viewType]; return numberGetter && numberGetter(diffLine); @@ -56,7 +56,7 @@ export default { }, computed: { ...mapState({ - diffFiles: state => state.diffs.diffFiles, + diffFiles: (state) => state.diffs.diffFiles, }), canExpandUp() { return !this.isBottom; diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index ed94cabe124..e613b684345 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -37,6 +37,11 @@ export default { type: Object, required: true, }, + reviewed: { + type: Boolean, + required: false, + default: false, + }, isFirstFile: { type: Boolean, required: false, @@ -134,7 +139,7 @@ export default { return !this.isCollapsed || this.automaticallyCollapsed; }, showWarning() { - return this.isCollapsed && (this.automaticallyCollapsed && !this.viewDiffsFileByFile); + return this.isCollapsed && this.automaticallyCollapsed && !this.viewDiffsFileByFile; }, showContent() { return !this.isCollapsed && !this.isFileTooLarge; @@ -205,7 +210,7 @@ export default { await this.$nextTick(); - eventsForThisFile.forEach(event => { + eventsForThisFile.forEach((event) => { eventHub.$emit(event); }); }, diff --git a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue index 439319f487c..f62b31734c5 100644 --- a/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue +++ b/app/assets/javascripts/diffs/components/diff_gutter_avatars.vue @@ -29,7 +29,7 @@ export default { return this.discussions.reduce((acc, note) => acc.concat(note.notes), []); }, notesInGutter() { - return this.allDiscussions.slice(0, COUNT_OF_AVATARS_IN_GUTTER).map(n => ({ + return this.allDiscussions.slice(0, COUNT_OF_AVATARS_IN_GUTTER).map((n) => ({ note: n.note, author: n.author, })); diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue index 172a2bdde7d..463b7f5cff4 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -56,10 +56,11 @@ export default { }, computed: { ...mapState({ - noteableData: state => state.notes.noteableData, - diffViewType: state => state.diffs.diffViewType, + diffViewType: ({ diffs }) => diffs.diffViewType, + showSuggestPopover: ({ diffs }) => diffs.showSuggestPopover, + noteableData: ({ notes }) => notes.noteableData, + selectedCommentPosition: ({ notes }) => notes.selectedCommentPosition, }), - ...mapState('diffs', ['showSuggestPopover']), ...mapGetters('diffs', ['getDiffFileByHash', 'diffLines']), ...mapGetters([ 'isLoggedIn', @@ -126,6 +127,10 @@ export default { this.initAutoSave(this.noteableData, keys); } + + if (this.selectedCommentPosition) { + this.commentLineStart = this.selectedCommentPosition.start; + } }, methods: { ...mapActions('diffs', [ diff --git a/app/assets/javascripts/diffs/components/diff_row.vue b/app/assets/javascripts/diffs/components/diff_row.vue index c0719e2a7d9..db03da966c3 100644 --- a/app/assets/javascripts/diffs/components/diff_row.vue +++ b/app/assets/javascripts/diffs/components/diff_row.vue @@ -1,7 +1,16 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; import { GlTooltipDirective, GlIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui'; -import { CONTEXT_LINE_CLASS_NAME, PARALLEL_DIFF_VIEW_TYPE } from '../constants'; +import { + CONTEXT_LINE_CLASS_NAME, + PARALLEL_DIFF_VIEW_TYPE, + CONFLICT_MARKER_OUR, + CONFLICT_MARKER_THEIR, + CONFLICT_OUR, + CONFLICT_THEIR, + CONFLICT_MARKER, +} from '../constants'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import DiffGutterAvatars from './diff_gutter_avatars.vue'; import * as utils from './diff_row_utils'; @@ -14,6 +23,7 @@ export default { GlTooltip: GlTooltipDirective, SafeHtml, }, + mixins: [glFeatureFlagsMixin()], props: { fileHash: { type: String, @@ -37,6 +47,15 @@ export default { required: false, default: false, }, + index: { + type: Number, + required: true, + }, + }, + data() { + return { + dragging: false, + }; }, computed: { ...mapGetters('diffs', ['fileLineCoverage']), @@ -44,26 +63,40 @@ export default { ...mapState({ isHighlighted(state) { const line = this.line.left?.line_code ? this.line.left : this.line.right; - return utils.isHighlighted(state, line, this.isCommented); + return utils.isHighlighted(state, line, false); }, }), classNameMap() { return { [CONTEXT_LINE_CLASS_NAME]: this.line.isContextLineLeft, - [PARALLEL_DIFF_VIEW_TYPE]: true, + [PARALLEL_DIFF_VIEW_TYPE]: !this.inline, + commented: this.isCommented, }; }, parallelViewLeftLineType() { - return utils.parallelViewLeftLineType(this.line, this.isHighlighted); + return utils.parallelViewLeftLineType(this.line, this.isHighlighted || this.isCommented); }, - coverageState() { + coverageStateLeft() { + if (!this.inline || !this.line.left) return {}; + return this.fileLineCoverage(this.filePath, this.line.left.new_line); + }, + coverageStateRight() { + if (!this.line.right) return {}; return this.fileLineCoverage(this.filePath, this.line.right.new_line); }, classNameMapCellLeft() { - return utils.classNameMapCell(this.line.left, this.isHighlighted, this.isLoggedIn); + return utils.classNameMapCell({ + line: this.line.left, + hll: this.isHighlighted || this.isCommented, + isLoggedIn: this.isLoggedIn, + }); }, classNameMapCellRight() { - return utils.classNameMapCell(this.line.right, this.isHighlighted, this.isLoggedIn); + return utils.classNameMapCell({ + line: this.line.right, + hll: this.isHighlighted || this.isCommented, + isLoggedIn: this.isLoggedIn, + }); }, addCommentTooltipLeft() { return utils.addCommentTooltip(this.line.left); @@ -71,6 +104,12 @@ export default { addCommentTooltipRight() { return utils.addCommentTooltip(this.line.right); }, + emptyCellRightClassMap() { + return { conflict_their: this.line.left?.type === CONFLICT_OUR }; + }, + emptyCellLeftClassMap() { + return { conflict_our: this.line.right?.type === CONFLICT_THEIR }; + }, shouldRenderCommentButton() { return ( this.isLoggedIn && @@ -80,6 +119,9 @@ export default { !this.line.hasDiscussionsRight ); }, + isLeftConflictMarker() { + return [CONFLICT_MARKER_OUR, CONFLICT_MARKER_THEIR].includes(this.line.left?.type); + }, }, mounted() { this.scrollToLineIfNeededParallel(this.line); @@ -98,7 +140,9 @@ export default { const table = line.closest('.diff-table'); table.classList.remove('left-side-selected', 'right-side-selected'); - const [lineClass] = ['left-side', 'right-side'].filter(name => line.classList.contains(name)); + const [lineClass] = ['left-side', 'right-side'].filter((name) => + line.classList.contains(name), + ); if (lineClass) { table.classList.add(`${lineClass}-selected`); @@ -107,37 +151,75 @@ export default { handleCommentButton(line) { this.showCommentForm({ lineCode: line.line_code, fileHash: this.fileHash }); }, + conflictText(line) { + return line.type === CONFLICT_MARKER_THEIR + ? this.$options.THEIR_CHANGES + : this.$options.OUR_CHANGES; + }, + onDragEnd() { + this.dragging = false; + if (!this.glFeatures.dragCommentSelection) return; + + this.$emit('stopdragging'); + }, + onDragEnter(line, index) { + if (!this.glFeatures.dragCommentSelection) return; + + this.$emit('enterdragging', { ...line, index }); + }, + onDragStart(line) { + this.$root.$emit('bv::hide::tooltip'); + this.dragging = true; + this.$emit('startdragging', line); + }, }, + OUR_CHANGES: 'HEAD//our changes', + THEIR_CHANGES: 'origin//their changes', + CONFLICT_MARKER, + CONFLICT_MARKER_THEIR, + CONFLICT_OUR, + CONFLICT_THEIR, }; </script> <template> <div :class="classNameMap" class="diff-grid-row diff-tr line_holder"> - <div class="diff-grid-left left-side"> - <template v-if="line.left"> + <div + data-testid="left-side" + class="diff-grid-left left-side" + @dragover.prevent + @dragenter="onDragEnter(line.left, index)" + @dragend="onDragEnd" + > + <template v-if="line.left && line.left.type !== $options.CONFLICT_MARKER"> <div :class="classNameMapCellLeft" data-testid="leftLineNumber" - class="diff-td diff-line-num old_line" + class="diff-td diff-line-num" > - <span - v-if="shouldRenderCommentButton" - v-gl-tooltip - data-testid="leftCommentButton" - class="add-diff-note tooltip-wrapper" - :title="addCommentTooltipLeft" - > - <button - type="button" - class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" - :disabled="line.left.commentsDisabled" - @click="handleCommentButton(line.left)" + <template v-if="!isLeftConflictMarker"> + <span + v-if="shouldRenderCommentButton" + v-gl-tooltip + data-testid="leftCommentButton" + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltipLeft" > - <gl-icon :size="12" name="comment" /> - </button> - </span> + <button + :draggable="glFeatures.dragCommentSelection" + type="button" + class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + :class="{ 'gl-cursor-grab': dragging }" + :disabled="line.left.commentsDisabled" + @click="handleCommentButton(line.left)" + @dragstart="onDragStart({ ...line.left, index })" + > + <gl-icon :size="12" name="comment" /> + </button> + </span> + </template> <a - v-if="line.left.old_line" + v-if="line.left.old_line && line.left.type !== $options.CONFLICT_THEIR" :data-linenumber="line.left.old_line" :href="line.lineHrefOld" @click="setHighlightedRow(line.lineCode)" @@ -157,52 +239,87 @@ export default { " /> </div> - <div v-if="inline" :class="classNameMapCellLeft" class="diff-td diff-line-num old_line"> + <div v-if="inline" :class="classNameMapCellLeft" class="diff-td diff-line-num"> <a - v-if="line.left.new_line" + v-if="line.left.new_line && line.left.type !== $options.CONFLICT_OUR" :data-linenumber="line.left.new_line" :href="line.lineHrefOld" @click="setHighlightedRow(line.lineCode)" > </a> </div> - <div :class="parallelViewLeftLineType" class="diff-td line-coverage left-side"></div> + <div + v-gl-tooltip.hover + :title="coverageStateLeft.text" + :class="[...parallelViewLeftLineType, coverageStateLeft.class]" + class="diff-td line-coverage left-side" + ></div> <div :id="line.left.line_code" :key="line.left.line_code" - v-safe-html="line.left.rich_text" - :class="parallelViewLeftLineType" - class="diff-td line_content with-coverage parallel left-side" + :class="[parallelViewLeftLineType, { parallel: !inline }]" + class="diff-td line_content with-coverage left-side" data-testid="leftContent" @mousedown="handleParallelLineMouseDown" - ></div> + > + <strong v-if="isLeftConflictMarker">{{ conflictText(line.left) }}</strong> + <span v-else v-safe-html="line.left.rich_text"></span> + </div> </template> - <template v-else> - <div data-testid="leftEmptyCell" class="diff-td diff-line-num old_line empty-cell"></div> - <div v-if="inline" class="diff-td diff-line-num old_line empty-cell"></div> - <div class="diff-td line-coverage left-side empty-cell"></div> - <div class="diff-td line_content with-coverage parallel left-side empty-cell"></div> + <template v-else-if="!inline || (line.left && line.left.type === $options.CONFLICT_MARKER)"> + <div + data-testid="leftEmptyCell" + class="diff-td diff-line-num old_line empty-cell" + :class="emptyCellLeftClassMap" + > + + </div> + <div + v-if="inline" + class="diff-td diff-line-num old_line empty-cell" + :class="emptyCellLeftClassMap" + ></div> + <div + class="diff-td line-coverage left-side empty-cell" + :class="emptyCellLeftClassMap" + ></div> + <div + class="diff-td line_content with-coverage left-side empty-cell" + :class="[emptyCellLeftClassMap, { parallel: !inline }]" + ></div> </template> </div> - <div v-if="!inline" class="diff-grid-right right-side"> + <div + v-if="!inline" + data-testid="right-side" + class="diff-grid-right right-side" + @dragover.prevent + @dragenter="onDragEnter(line.right, index)" + @dragend="onDragEnd" + > <template v-if="line.right"> <div :class="classNameMapCellRight" class="diff-td diff-line-num new_line"> - <span - v-if="shouldRenderCommentButton" - v-gl-tooltip - data-testid="rightCommentButton" - class="add-diff-note tooltip-wrapper" - :title="addCommentTooltipRight" - > - <button - type="button" - class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" - :disabled="line.right.commentsDisabled" - @click="handleCommentButton(line.right)" + <template v-if="line.right.type !== $options.CONFLICT_MARKER_THEIR"> + <span + v-if="shouldRenderCommentButton" + v-gl-tooltip + data-testid="rightCommentButton" + class="add-diff-note tooltip-wrapper" + :title="addCommentTooltipRight" > - <gl-icon :size="12" name="comment" /> - </button> - </span> + <button + :draggable="glFeatures.dragCommentSelection" + type="button" + class="add-diff-note note-button js-add-diff-note-button qa-diff-comment" + :class="{ 'gl-cursor-grab': dragging }" + :disabled="line.right.commentsDisabled" + @click="handleCommentButton(line.right)" + @dragstart="onDragStart({ ...line.right, index })" + > + <gl-icon :size="12" name="comment" /> + </button> + </span> + </template> <a v-if="line.right.new_line" :data-linenumber="line.right.new_line" @@ -226,8 +343,12 @@ export default { </div> <div v-gl-tooltip.hover - :title="coverageState.text" - :class="[line.right.type, coverageState.class, { hll: isHighlighted }]" + :title="coverageStateRight.text" + :class="[ + line.right.type, + coverageStateRight.class, + { hll: isHighlighted, hll: isCommented }, + ]" class="diff-td line-coverage right-side" ></div> <div @@ -238,17 +359,38 @@ export default { line.right.type, { hll: isHighlighted, + hll: isCommented, + parallel: !inline, }, ]" - class="diff-td line_content with-coverage parallel right-side" + class="diff-td line_content with-coverage right-side" @mousedown="handleParallelLineMouseDown" - ></div> + > + <strong v-if="line.right.type === $options.CONFLICT_MARKER_THEIR">{{ + conflictText(line.right) + }}</strong> + <span v-else v-safe-html="line.right.rich_text"></span> + </div> </template> <template v-else> - <div data-testid="rightEmptyCell" class="diff-td diff-line-num old_line empty-cell"></div> - <div class="diff-td diff-line-num old_line empty-cell"></div> - <div class="diff-td line-coverage right-side empty-cell"></div> - <div class="diff-td line_content with-coverage parallel right-side empty-cell"></div> + <div + data-testid="rightEmptyCell" + class="diff-td diff-line-num old_line empty-cell" + :class="emptyCellRightClassMap" + ></div> + <div + v-if="inline" + class="diff-td diff-line-num old_line empty-cell" + :class="emptyCellRightClassMap" + ></div> + <div + class="diff-td line-coverage right-side empty-cell" + :class="emptyCellRightClassMap" + ></div> + <div + class="diff-td line_content with-coverage right-side empty-cell" + :class="[emptyCellRightClassMap, { parallel: !inline }]" + ></div> </template> </div> </div> diff --git a/app/assets/javascripts/diffs/components/diff_row_utils.js b/app/assets/javascripts/diffs/components/diff_row_utils.js index d5491d3cd56..7606c39ad37 100644 --- a/app/assets/javascripts/diffs/components/diff_row_utils.js +++ b/app/assets/javascripts/diffs/components/diff_row_utils.js @@ -15,27 +15,27 @@ export const isHighlighted = (state, line, isCommented) => { return lineCode ? lineCode === state.diffs.highlightedRow : false; }; -export const isContextLine = type => type === CONTEXT_LINE_TYPE; +export const isContextLine = (type) => type === CONTEXT_LINE_TYPE; -export const isMatchLine = type => type === MATCH_LINE_TYPE; +export const isMatchLine = (type) => type === MATCH_LINE_TYPE; -export const isMetaLine = type => +export const isMetaLine = (type) => [OLD_NO_NEW_LINE_TYPE, NEW_NO_NEW_LINE_TYPE, EMPTY_CELL_TYPE].includes(type); export const shouldRenderCommentButton = (isLoggedIn, isCommentButtonRendered) => { return isCommentButtonRendered && isLoggedIn; }; -export const hasDiscussions = line => line?.discussions?.length > 0; +export const hasDiscussions = (line) => line?.discussions?.length > 0; -export const lineHref = line => `#${line?.line_code || ''}`; +export const lineHref = (line) => `#${line?.line_code || ''}`; -export const lineCode = line => { +export const lineCode = (line) => { if (!line) return undefined; return line.line_code || line.left?.line_code || line.right?.line_code; }; -export const classNameMapCell = (line, hll, isLoggedIn, isHover) => { +export const classNameMapCell = ({ line, hll, isLoggedIn, isHover }) => { if (!line) return []; const { type } = line; @@ -44,15 +44,19 @@ export const classNameMapCell = (line, hll, isLoggedIn, isHover) => { { hll, [LINE_HOVER_CLASS_NAME]: isLoggedIn && isHover && !isContextLine(type) && !isMetaLine(type), + old_line: line.type === 'old', + new_line: line.type === 'new', }, ]; }; -export const addCommentTooltip = line => { +export const addCommentTooltip = (line) => { let tooltip; if (!line) return tooltip; - tooltip = __('Add a comment to this line'); + tooltip = gon.drag_comment_selection + ? __('Add a comment to this line or drag for multiple lines') + : __('Add a comment to this line'); const brokenSymlinks = line.commentsDisabled; if (brokenSymlinks) { @@ -84,7 +88,7 @@ export const shouldShowCommentButton = (hover, context, meta, discussions) => { return hover && !context && !meta && !discussions; }; -export const mapParallel = content => line => { +export const mapParallel = (content) => (line) => { let { left, right } = line; // Dicussions/Comments @@ -137,7 +141,7 @@ export const mapParallel = content => line => { }; // TODO: Delete this function when unifiedDiffComponents FF is removed -export const mapInline = content => line => { +export const mapInline = (content) => (line) => { // Discussions/Comments const renderCommentRow = line.hasForm || (line.discussions?.length && line.discussionsExpanded); diff --git a/app/assets/javascripts/diffs/components/diff_view.vue b/app/assets/javascripts/diffs/components/diff_view.vue index 84429f62a1c..79800f835f4 100644 --- a/app/assets/javascripts/diffs/components/diff_view.vue +++ b/app/assets/javascripts/diffs/components/diff_view.vue @@ -1,5 +1,5 @@ <script> -import { mapGetters, mapState } from 'vuex'; +import { mapGetters, mapState, mapActions } from 'vuex'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; import DraftNote from '~/batch_comments/components/draft_note.vue'; import DiffRow from './diff_row.vue'; @@ -35,6 +35,12 @@ export default { default: false, }, }, + data() { + return { + dragStart: null, + updatedLineRange: null, + }; + }, computed: { ...mapGetters('diffs', ['commitId']), ...mapState({ @@ -52,12 +58,39 @@ export default { }, }, methods: { + ...mapActions(['setSelectedCommentPosition']), + ...mapActions('diffs', ['showCommentForm']), showCommentLeft(line) { return !this.inline || line.left; }, showCommentRight(line) { return !this.inline || (line.right && !line.left); }, + onStartDragging(line) { + this.dragStart = line; + }, + onDragOver(line) { + if (line.chunk !== this.dragStart.chunk) return; + + let start = this.dragStart; + let end = line; + + if (this.dragStart.index >= line.index) { + start = line; + end = this.dragStart; + } + + this.updatedLineRange = { start, end }; + + this.setSelectedCommentPosition(this.updatedLineRange); + }, + onStopDragging() { + this.showCommentForm({ + lineCode: this.updatedLineRange?.end?.line_code, + fileHash: this.diffFile.file_hash, + }); + this.dragStart = null; + }, }, userColorScheme: window.gon.user_color_scheme, }; @@ -94,6 +127,10 @@ export default { :is-bottom="index + 1 === diffLinesLength" :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" :inline="inline" + :index="index" + @enterdragging="onDragOver" + @startdragging="onStartDragging" + @stopdragging="onStopDragging" /> <div v-if="line.renderCommentRow" diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue index 2d8ffb047ca..014b1ebe54b 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -72,7 +72,12 @@ export default { return this.fileLineCoverage(this.filePath, this.line.new_line); }, classNameMapCell() { - return classNameMapCell(this.line, this.isHighlighted, this.isLoggedIn, this.isHover); + return classNameMapCell({ + line: this.line, + hll: this.isHighlighted, + isLoggedIn: this.isLoggedIn, + isHover: this.isHover, + }); }, addCommentTooltip() { return addCommentTooltip(this.line); diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index 05f5461054f..28485a2fdac 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -58,9 +58,9 @@ export default { class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view" > <colgroup> - <col style="width: 50px;" /> - <col style="width: 50px;" /> - <col style="width: 8px;" /> + <col style="width: 50px" /> + <col style="width: 50px" /> + <col style="width: 8px" /> <col /> </colgroup> <tbody> diff --git a/app/assets/javascripts/diffs/components/no_changes.vue b/app/assets/javascripts/diffs/components/no_changes.vue index a640dcb0a90..e0fdbf6ac3a 100644 --- a/app/assets/javascripts/diffs/components/no_changes.vue +++ b/app/assets/javascripts/diffs/components/no_changes.vue @@ -14,7 +14,31 @@ export default { }, }, computed: { + ...mapGetters('diffs', [ + 'diffCompareDropdownTargetVersions', + 'diffCompareDropdownSourceVersions', + ]), ...mapGetters(['getNoteableData']), + selectedSourceVersion() { + return this.diffCompareDropdownSourceVersions.find((x) => x.selected); + }, + sourceName() { + if (!this.selectedSourceVersion || this.selectedSourceVersion.isLatestVersion) { + return this.getNoteableData.source_branch; + } + + return this.selectedSourceVersion.versionName; + }, + selectedTargetVersion() { + return this.diffCompareDropdownTargetVersions.find((x) => x.selected); + }, + targetName() { + if (!this.selectedTargetVersion || this.selectedTargetVersion.version_index < 0) { + return this.getNoteableData.target_branch; + } + + return this.selectedTargetVersion.versionName || ''; + }, }, }; </script> @@ -26,14 +50,16 @@ export default { </div> <div class="col-12"> <div class="text-content text-center"> - <gl-sprintf :message="__('No changes between %{sourceBranch} and %{targetBranch}')"> - <template #sourceBranch> - <span class="ref-name">{{ getNoteableData.source_branch }}</span> - </template> - <template #targetBranch> - <span class="ref-name">{{ getNoteableData.target_branch }}</span> - </template> - </gl-sprintf> + <div data-testid="no-changes-message"> + <gl-sprintf :message="__('No changes between %{source} and %{target}')"> + <template #source> + <span class="ref-name">{{ sourceName }}</span> + </template> + <template #target> + <span class="ref-name">{{ targetName }}</span> + </template> + </gl-sprintf> + </div> <div class="text-center"> <gl-button :href="getNoteableData.new_blob_path" variant="success" category="primary">{{ __('Create commit') diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue index 13cd0651ff2..47eecef2385 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -68,20 +68,20 @@ export default { return this.fileLineCoverage(this.filePath, this.line.right.new_line); }, classNameMapCellLeft() { - return utils.classNameMapCell( - this.line.left, - this.isHighlighted, - this.isLoggedIn, - this.isLeftHover, - ); + return utils.classNameMapCell({ + line: this.line.left, + hll: this.isHighlighted, + isLoggedIn: this.isLoggedIn, + isHover: this.isLeftHover, + }); }, classNameMapCellRight() { - return utils.classNameMapCell( - this.line.right, - this.isHighlighted, - this.isLoggedIn, - this.isRightHover, - ); + return utils.classNameMapCell({ + line: this.line.right, + hll: this.isHighlighted, + isLoggedIn: this.isLoggedIn, + isHover: this.isRightHover, + }); }, addCommentTooltipLeft() { return utils.addCommentTooltip(this.line.left); @@ -112,8 +112,8 @@ export default { mounted() { this.scrollToLineIfNeededParallel(this.line); this.unwatchShouldShowCommentButton = this.$watch( - vm => [vm.shouldShowCommentButtonLeft, vm.shouldShowCommentButtonRight].join(), - newVal => { + (vm) => [vm.shouldShowCommentButtonLeft, vm.shouldShowCommentButtonRight].join(), + (newVal) => { if (newVal) { this.isCommentButtonRendered = true; this.unwatchShouldShowCommentButton(); @@ -150,7 +150,7 @@ export default { const table = line.closest('table'); table.removeClass('left-side-selected right-side-selected'); - const [lineClass] = ['left-side', 'right-side'].filter(name => line.hasClass(name)); + const [lineClass] = ['left-side', 'right-side'].filter((name) => line.hasClass(name)); if (lineClass) { table.addClass(`${lineClass}-selected`); diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 67b599fe163..21e0bf18dbf 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -57,11 +57,11 @@ export default { class="code diff-wrap-lines js-syntax-highlight text-file" > <colgroup> - <col style="width: 50px;" /> - <col style="width: 8px;" /> + <col style="width: 50px" /> + <col style="width: 8px" /> <col /> - <col style="width: 50px;" /> - <col style="width: 8px;" /> + <col style="width: 50px" /> + <col style="width: 8px" /> <col /> </colgroup> <tbody> diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index d03d450b12d..1a258695fa0 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -35,7 +35,7 @@ export default { } return this.allBlobs.reduce((acc, folder) => { - const tree = folder.tree.filter(f => f.path.toLowerCase().indexOf(search) >= 0); + const tree = folder.tree.filter((f) => f.path.toLowerCase().indexOf(search) >= 0); if (tree.length) { return acc.concat({ diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 07e27bd8e47..7080348ee7d 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -109,3 +109,9 @@ export const EVT_PERF_MARK_FILE_TREE_END = 'mr:diffs:perf:fileTreeEnd'; export const EVT_PERF_MARK_DIFF_FILES_START = 'mr:diffs:perf:filesStart'; export const EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN = 'mr:diffs:perf:firstFileShown'; export const EVT_PERF_MARK_DIFF_FILES_END = 'mr:diffs:perf:filesEnd'; + +export const CONFLICT_OUR = 'conflict_our'; +export const CONFLICT_THEIR = 'conflict_their'; +export const CONFLICT_MARKER = 'conflict_marker'; +export const CONFLICT_MARKER_OUR = 'conflict_marker_our'; +export const CONFLICT_MARKER_THEIR = 'conflict_marker_their'; diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 587220488be..4e0720c645a 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -5,6 +5,10 @@ import { parseBoolean } from '~/lib/utils/common_utils'; import FindFile from '~/vue_shared/components/file_finder/index.vue'; import eventHub from '../notes/event_hub'; import diffsApp from './components/app.vue'; + +import { getDerivedMergeRequestInformation } from './utils/merge_request'; +import { getReviewsForMergeRequest } from './utils/file_reviews'; + import { TREE_LIST_STORAGE_KEY, DIFF_WHITESPACE_COOKIE_NAME } from './constants'; export default function initDiffsApp(store) { @@ -79,11 +83,12 @@ export default function initDiffsApp(store) { showSuggestPopover: parseBoolean(dataset.showSuggestPopover), showWhitespaceDefault: parseBoolean(dataset.showWhitespaceDefault), viewDiffsFileByFile: parseBoolean(dataset.fileByFileDefault), + defaultSuggestionCommitMessage: dataset.defaultSuggestionCommitMessage, }; }, computed: { ...mapState({ - activeTab: state => state.page.activeTab, + activeTab: (state) => state.page.activeTab, }), }, created() { @@ -102,6 +107,8 @@ export default function initDiffsApp(store) { ...mapActions('diffs', ['setRenderTreeList', 'setShowWhitespace']), }, render(createElement) { + const { mrPath } = getDerivedMergeRequestInformation({ endpoint: this.endpoint }); + return createElement('diffs-app', { props: { endpoint: this.endpoint, @@ -117,6 +124,8 @@ export default function initDiffsApp(store) { dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, fileByFileUserPreference: this.viewDiffsFileByFile, + defaultSuggestionCommitMessage: this.defaultSuggestionCommitMessage, + mrReviews: getReviewsForMergeRequest(mrPath), }, }); }, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 5b410051705..e95e9ac3ee4 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -50,6 +50,8 @@ import { } from '../constants'; import { diffViewerModes } from '~/ide/constants'; import { isCollapsed } from '../utils/diff_file'; +import { getDerivedMergeRequestInformation } from '../utils/merge_request'; +import { markFileReview, setReviewsForMergeRequest } from '../utils/file_reviews'; export const setBaseConfig = ({ commit }, options) => { const { @@ -60,7 +62,9 @@ export const setBaseConfig = ({ commit }, options) => { projectPath, dismissEndpoint, showSuggestPopover, + defaultSuggestionCommitMessage, viewDiffsFileByFile, + mrReviews, } = options; commit(types.SET_BASE_CONFIG, { endpoint, @@ -70,7 +74,9 @@ export const setBaseConfig = ({ commit }, options) => { projectPath, dismissEndpoint, showSuggestPopover, + defaultSuggestionCommitMessage, viewDiffsFileByFile, + mrReviews, }); }; @@ -123,7 +129,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { // We need to check that the currentDiffFileId points to a file that exists if ( state.currentDiffFileId && - !state.diffFiles.some(f => f.file_hash === state.currentDiffFileId) && + !state.diffFiles.some((f) => f.file_hash === state.currentDiffFileId) && !isNoteLink ) { commit(types.VIEW_DIFF_FILE, state.diffFiles[0].file_hash); @@ -131,11 +137,11 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { if (state.diffFiles?.length) { // eslint-disable-next-line promise/catch-or-return,promise/no-nesting - import('~/code_navigation').then(m => + import('~/code_navigation').then((m) => m.default({ blobs: state.diffFiles - .filter(f => f.code_navigation_path) - .map(f => ({ + .filter((f) => f.code_navigation_path) + .map((f) => ({ path: f.new_path, codeNavigationPath: f.code_navigation_path, })), @@ -157,7 +163,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { return pagination.next_page; }) - .then(nextPage => nextPage && getBatch(nextPage)) + .then((nextPage) => nextPage && getBatch(nextPage)) .catch(() => commit(types.SET_RETRIEVING_BATCHES, false)); return getBatch() @@ -207,7 +213,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { export const fetchCoverageFiles = ({ commit, state }) => { const coveragePoll = new Poll({ resource: { - getCoverageReports: endpoint => axios.get(endpoint), + getCoverageReports: (endpoint) => axios.get(endpoint), }, data: state.endpointCoverage, method: 'getCoverageReports', @@ -242,8 +248,8 @@ export const assignDiscussionsToDiff = ( const hash = getLocationHash(); discussions - .filter(discussion => discussion.diff_discussion) - .forEach(discussion => { + .filter((discussion) => discussion.diff_discussion) + .forEach((discussion) => { commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { discussion, diffPositionByLineCode, @@ -270,10 +276,10 @@ export const toggleLineDiscussions = ({ commit }, options) => { }; export const renderFileForDiscussionId = ({ commit, rootState, state }, discussionId) => { - const discussion = rootState.notes.discussions.find(d => d.id === discussionId); + const discussion = rootState.notes.discussions.find((d) => d.id === discussionId); if (discussion && discussion.diff_file) { - const file = state.diffFiles.find(f => f.file_hash === discussion.diff_file.file_hash); + const file = state.diffFiles.find((f) => f.file_hash === discussion.diff_file.file_hash); if (file) { if (!file.renderIt) { @@ -299,11 +305,12 @@ export const renderFileForDiscussionId = ({ commit, rootState, state }, discussi export const startRenderDiffsQueue = ({ state, commit }) => { const checkItem = () => - new Promise(resolve => { + new Promise((resolve) => { const nextFile = state.diffFiles.find( - file => + (file) => !file.renderIt && - (file.viewer && (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text)), + file.viewer && + (!isCollapsed(file) || file.viewer.name !== diffViewerModes.text), ); if (nextFile) { @@ -357,7 +364,7 @@ export const loadMoreLines = ({ commit }, options) => { params.from_merge_request = true; - return axios.get(endpoint, { params }).then(res => { + return axios.get(endpoint, { params }).then((res) => { const contextLines = res.data || []; commit(types.ADD_CONTEXT_LINES, { @@ -398,7 +405,7 @@ export const loadCollapsedDiff = ({ commit, getters, state }, file) => w: state.showWhitespace ? '0' : '1', }, }) - .then(res => { + .then((res) => { commit(types.ADD_COLLAPSED_DIFFS, { file, data: res.data, @@ -421,7 +428,7 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => { const shouldCloseAll = getters.diffHasAllExpandedDiscussions(diff); const shouldExpandAll = getters.diffHasAllCollapsedDiscussions(diff); - discussions.forEach(discussion => { + discussions.forEach((discussion) => { const data = { discussionId: discussion.id }; if (shouldCloseAll) { @@ -435,13 +442,13 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => { export const toggleFileDiscussionWrappers = ({ commit }, diff) => { const discussionWrappersExpanded = allDiscussionWrappersExpanded(diff); const lineCodesWithDiscussions = new Set(); - const lineHasDiscussion = line => Boolean(line?.discussions.length); - const registerDiscussionLine = line => lineCodesWithDiscussions.add(line.line_code); + const lineHasDiscussion = (line) => Boolean(line?.discussions.length); + const registerDiscussionLine = (line) => lineCodesWithDiscussions.add(line.line_code); diff[INLINE_DIFF_LINES_KEY].filter(lineHasDiscussion).forEach(registerDiscussionLine); if (lineCodesWithDiscussions.size) { - Array.from(lineCodesWithDiscussions).forEach(lineCode => { + Array.from(lineCodesWithDiscussions).forEach((lineCode) => { commit(types.TOGGLE_LINE_DISCUSSIONS, { fileHash: diff.file_hash, expanded: !discussionWrappersExpanded, @@ -459,8 +466,8 @@ export const saveDiffDiscussion = ({ state, dispatch }, { note, formData }) => { }); return dispatch('saveNote', postData, { root: true }) - .then(result => dispatch('updateDiscussion', result.discussion, { root: true })) - .then(discussion => dispatch('assignDiscussionsToDiff', [discussion])) + .then((result) => dispatch('updateDiscussion', result.discussion, { root: true })) + .then((discussion) => dispatch('assignDiscussionsToDiff', [discussion])) .then(() => dispatch('updateResolvableDiscussionsCounts', null, { root: true })) .then(() => dispatch('closeDiffFileCommentForm', formData.diffFile.file_hash)) .catch(() => createFlash(s__('MergeRequests|Saving the comment failed'))); @@ -560,7 +567,7 @@ export const setExpandedDiffLines = ({ commit }, { file, data }) => { }); commit(types.TOGGLE_DIFF_FILE_RENDERING_MORE, file.file_path); - const idleCb = t => { + const idleCb = (t) => { const startIndex = index; while ( @@ -608,7 +615,7 @@ export const fetchFullDiff = ({ commit, dispatch }, file) => .catch(() => dispatch('receiveFullDiffError', file.file_path)); export const toggleFullDiff = ({ dispatch, commit, getters, state }, filePath) => { - const file = state.diffFiles.find(f => f.file_path === filePath); + const file = state.diffFiles.find((f) => f.file_path === filePath); commit(types.REQUEST_FULL_DIFF, filePath); @@ -719,7 +726,7 @@ export const setCurrentDiffFileIdFromNote = ({ commit, state, rootGetters }, not const fileHash = rootGetters.getDiscussion(note.discussion_id).diff_file?.file_hash; - if (fileHash && state.diffFiles.some(f => f.file_hash === fileHash)) { + if (fileHash && state.diffFiles.some((f) => f.file_hash === fileHash)) { commit(types.VIEW_DIFF_FILE, fileHash); } }; @@ -741,3 +748,13 @@ export const setFileByFile = ({ commit }, { fileByFile }) => { mergeUrlParams({ [DIFF_FILE_BY_FILE_COOKIE_NAME]: fileViewMode }, window.location.href), ); }; + +export function reviewFile({ commit, state, getters }, { file, reviewed = true }) { + const { mrPath } = getDerivedMergeRequestInformation({ endpoint: file.load_collapsed_diff_url }); + const reviews = setReviewsForMergeRequest( + mrPath, + markFileReview(getters.fileReviews(state), file, reviewed), + ); + + commit(types.SET_MR_FILE_REVIEWS, reviews); +} diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index baf54188932..a167b6d4694 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -1,5 +1,6 @@ import { __, n__ } from '~/locale'; import { parallelizeDiffLines } from './utils'; +import { isFileReviewed } from '../utils/file_reviews'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE, @@ -8,13 +9,13 @@ import { export * from './getters_versions_dropdowns'; -export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE; +export const isParallelView = (state) => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE; -export const isInlineView = state => state.diffViewType === INLINE_DIFF_VIEW_TYPE; +export const isInlineView = (state) => state.diffViewType === INLINE_DIFF_VIEW_TYPE; -export const whichCollapsedTypes = state => { - const automatic = state.diffFiles.some(file => file.viewer?.automaticallyCollapsed); - const manual = state.diffFiles.some(file => file.viewer?.manuallyCollapsed); +export const whichCollapsedTypes = (state) => { + const automatic = state.diffFiles.some((file) => file.viewer?.automaticallyCollapsed); + const manual = state.diffFiles.some((file) => file.viewer?.manuallyCollapsed); return { any: automatic || manual, @@ -23,18 +24,18 @@ export const whichCollapsedTypes = state => { }; }; -export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null); +export const commitId = (state) => (state.commit && state.commit.id ? state.commit.id : null); /** * Checks if the diff has all discussions expanded * @param {Object} diff * @returns {Boolean} */ -export const diffHasAllExpandedDiscussions = (state, getters) => diff => { +export const diffHasAllExpandedDiscussions = (state, getters) => (diff) => { const discussions = getters.getDiffFileDiscussions(diff); return ( - (discussions && discussions.length && discussions.every(discussion => discussion.expanded)) || + (discussions && discussions.length && discussions.every((discussion) => discussion.expanded)) || false ); }; @@ -44,11 +45,13 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => { * @param {Object} diff * @returns {Boolean} */ -export const diffHasAllCollapsedDiscussions = (state, getters) => diff => { +export const diffHasAllCollapsedDiscussions = (state, getters) => (diff) => { const discussions = getters.getDiffFileDiscussions(diff); return ( - (discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) || + (discussions && + discussions.length && + discussions.every((discussion) => !discussion.expanded)) || false ); }; @@ -58,9 +61,9 @@ export const diffHasAllCollapsedDiscussions = (state, getters) => diff => { * @param {Object} diff * @returns {Boolean} */ -export const diffHasExpandedDiscussions = () => diff => { - return diff[INLINE_DIFF_LINES_KEY].filter(l => l.discussions.length >= 1).some( - l => l.discussionsExpanded, +export const diffHasExpandedDiscussions = () => (diff) => { + return diff[INLINE_DIFF_LINES_KEY].filter((l) => l.discussions.length >= 1).some( + (l) => l.discussionsExpanded, ); }; @@ -69,8 +72,8 @@ export const diffHasExpandedDiscussions = () => diff => { * @param {Boolean} diff * @returns {Boolean} */ -export const diffHasDiscussions = () => diff => { - return diff[INLINE_DIFF_LINES_KEY].some(l => l.discussions.length >= 1); +export const diffHasDiscussions = () => (diff) => { + return diff[INLINE_DIFF_LINES_KEY].some((l) => l.discussions.length >= 1); }; /** @@ -78,22 +81,22 @@ export const diffHasDiscussions = () => diff => { * @param {Object} diff * @returns {Array} */ -export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) => diff => +export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) => (diff) => rootGetters.discussions.filter( - discussion => discussion.diff_discussion && discussion.diff_file.file_hash === diff.file_hash, + (discussion) => discussion.diff_discussion && discussion.diff_file.file_hash === diff.file_hash, ) || []; -export const getDiffFileByHash = state => fileHash => - state.diffFiles.find(file => file.file_hash === fileHash); +export const getDiffFileByHash = (state) => (fileHash) => + state.diffFiles.find((file) => file.file_hash === fileHash); -export const flatBlobsList = state => - Object.values(state.treeEntries).filter(f => f.type === 'blob'); +export const flatBlobsList = (state) => + Object.values(state.treeEntries).filter((f) => f.type === 'blob'); export const allBlobs = (state, getters) => getters.flatBlobsList.reduce((acc, file) => { const { parentPath } = file; - if (parentPath && !acc.some(f => f.path === parentPath)) { + if (parentPath && !acc.some((f) => f.path === parentPath)) { acc.push({ path: parentPath, isHeader: true, @@ -101,13 +104,13 @@ export const allBlobs = (state, getters) => }); } - acc.find(f => f.path === parentPath).tree.push(file); + acc.find((f) => f.path === parentPath).tree.push(file); return acc; }, []); -export const getCommentFormForDiffFile = state => fileHash => - state.commentForms.find(form => form.fileHash === fileHash); +export const getCommentFormForDiffFile = (state) => (fileHash) => + state.commentForms.find((form) => form.fileHash === fileHash); /** * Returns the test coverage hits for a specific line of a given file @@ -115,7 +118,7 @@ export const getCommentFormForDiffFile = state => fileHash => * @param {number} line * @returns {number} */ -export const fileLineCoverage = state => (file, line) => { +export const fileLineCoverage = (state) => (file, line) => { if (!state.coverageFiles.files) return {}; const fileCoverage = state.coverageFiles.files[file]; if (!fileCoverage) return {}; @@ -136,10 +139,13 @@ export const fileLineCoverage = state => (file, line) => { * Returns index of a currently selected diff in diffFiles * @returns {number} */ -export const currentDiffIndex = state => - Math.max(0, state.diffFiles.findIndex(diff => diff.file_hash === state.currentDiffFileId)); +export const currentDiffIndex = (state) => + Math.max( + 0, + state.diffFiles.findIndex((diff) => diff.file_hash === state.currentDiffFileId), + ); -export const diffLines = state => (file, unifiedDiffComponents) => { +export const diffLines = (state) => (file, unifiedDiffComponents) => { if (!unifiedDiffComponents && state.diffViewType === INLINE_DIFF_VIEW_TYPE) { return null; } @@ -149,3 +155,7 @@ export const diffLines = state => (file, unifiedDiffComponents) => { state.diffViewType === INLINE_DIFF_VIEW_TYPE, ); }; + +export function fileReviews(state) { + return state.diffFiles.map((file) => isFileReviewed(state.mrReviews, file)); +} diff --git a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js index 135b1c61ef5..3f33b0c900e 100644 --- a/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js +++ b/app/assets/javascripts/diffs/store/getters_versions_dropdowns.js @@ -2,10 +2,10 @@ import { __, n__, sprintf } from '~/locale'; import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; import { DIFF_COMPARE_BASE_VERSION_INDEX, DIFF_COMPARE_HEAD_VERSION_INDEX } from '../constants'; -export const selectedTargetIndex = state => +export const selectedTargetIndex = (state) => state.startVersion?.version_index || DIFF_COMPARE_BASE_VERSION_INDEX; -export const selectedSourceIndex = state => state.mergeRequestDiff.version_index; +export const selectedSourceIndex = (state) => state.mergeRequestDiff.version_index; export const diffCompareDropdownTargetVersions = (state, getters) => { // startVersion only exists if the user has selected a version other @@ -40,7 +40,7 @@ export const diffCompareDropdownTargetVersions = (state, getters) => { selected: isHeadSelected, }; // Appended properties here are to make the compare_dropdown_layout easier to reason about - const formatVersion = v => { + const formatVersion = (v) => { return { href: v.compare_path, versionName: sprintf(__(`version %{versionIndex}`), { versionIndex: v.version_index }), @@ -53,19 +53,23 @@ export const diffCompareDropdownTargetVersions = (state, getters) => { ...state.mergeRequestDiffs.slice(1).map(formatVersion), baseVersion, state.mergeRequestDiff.head_version_path && headVersion, - ].filter(a => a); + ].filter((a) => a); }; export const diffCompareDropdownSourceVersions = (state, getters) => { // Appended properties here are to make the compare_dropdown_layout easier to reason about - return state.mergeRequestDiffs.map((v, i) => ({ - ...v, - href: v.version_path, - commitsText: n__(`%d commit,`, `%d commits,`, v.commits_count), - versionName: - i === 0 + return state.mergeRequestDiffs.map((v, i) => { + const isLatestVersion = i === 0; + + return { + ...v, + href: v.version_path, + commitsText: n__(`%d commit,`, `%d commits,`, v.commits_count), + isLatestVersion, + versionName: isLatestVersion ? __('latest version') : sprintf(__(`version %{versionIndex}`), { versionIndex: v.version_index }), - selected: v.version_index === getters.selectedSourceIndex, - })); + selected: v.version_index === getters.selectedSourceIndex, + }; + }); }; diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index c331e52c887..aa89c74cef0 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -45,4 +45,6 @@ export default () => ({ fileFinderVisible: false, dismissEndpoint: '', showSuggestPopover: true, + defaultSuggestionCommitMessage: '', + mrReviews: {}, }); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 30097239aaa..4641731c4b6 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -7,6 +7,8 @@ export const SET_DIFF_METADATA = 'SET_DIFF_METADATA'; export const SET_DIFF_DATA_BATCH = 'SET_DIFF_DATA_BATCH'; export const SET_DIFF_FILES = 'SET_DIFF_FILES'; +export const SET_MR_FILE_REVIEWS = 'SET_MR_FILE_REVIEWS'; + export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE'; export const SET_COVERAGE_DATA = 'SET_COVERAGE_DATA'; export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 19122c3096f..06f0f2c3dfb 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -36,7 +36,9 @@ export default { projectPath, dismissEndpoint, showSuggestPopover, + defaultSuggestionCommitMessage, viewDiffsFileByFile, + mrReviews, } = options; Object.assign(state, { endpoint, @@ -46,7 +48,9 @@ export default { projectPath, dismissEndpoint, showSuggestPopover, + defaultSuggestionCommitMessage, viewDiffsFileByFile, + mrReviews, }); }, @@ -103,11 +107,11 @@ export default { }, [types.TOGGLE_LINE_HAS_FORM](state, { lineCode, fileHash, hasForm }) { - const diffFile = state.diffFiles.find(f => f.file_hash === fileHash); + const diffFile = state.diffFiles.find((f) => f.file_hash === fileHash); if (!diffFile) return; - diffFile[INLINE_DIFF_LINES_KEY].find(l => l.line_code === lineCode).hasForm = hasForm; + diffFile[INLINE_DIFF_LINES_KEY].find((l) => l.line_code === lineCode).hasForm = hasForm; }, [types.ADD_CONTEXT_LINES](state, options) { @@ -123,7 +127,7 @@ export default { bottom, isExpandDown, nextLineNumbers, - ).map(line => { + ).map((line) => { const lineCode = line.type === 'match' ? `${fileHash}_${line.meta_data.old_pos}_${line.meta_data.new_pos}_match` @@ -147,8 +151,8 @@ export default { [types.ADD_COLLAPSED_DIFFS](state, { file, data }) { const files = prepareDiffData({ diff: data }); - const [newFileData] = files.filter(f => f.file_hash === file.file_hash); - const selectedFile = state.diffFiles.find(f => f.file_hash === file.file_hash); + const [newFileData] = files.filter((f) => f.file_hash === file.file_hash); + const selectedFile = state.diffFiles.find((f) => f.file_hash === file.file_hash); Object.assign(selectedFile, { ...newFileData }); }, @@ -157,9 +161,9 @@ export default { const discussionLineCodes = [discussion.line_code, ...(discussion.line_codes || [])]; const fileHash = discussion.diff_file.file_hash; - const lineCheck = line => + const lineCheck = (line) => discussionLineCodes.some( - discussionLineCode => + (discussionLineCode) => line.line_code === discussionLineCode && isDiscussionApplicableToLine({ discussion, @@ -177,26 +181,26 @@ export default { : [], }); - const setDiscussionsExpanded = line => { + const setDiscussionsExpanded = (line) => { const isLineNoteTargeted = line.discussions && line.discussions.some( - disc => disc.notes && disc.notes.find(note => hash === `note_${note.id}`), + (disc) => disc.notes && disc.notes.find((note) => hash === `note_${note.id}`), ); return { ...line, discussionsExpanded: line.discussions && line.discussions.length - ? line.discussions.some(disc => !disc.resolved) || isLineNoteTargeted + ? line.discussions.some((disc) => !disc.resolved) || isLineNoteTargeted : false, }; }; - state.diffFiles.forEach(file => { + state.diffFiles.forEach((file) => { if (file.file_hash === fileHash) { if (file[INLINE_DIFF_LINES_KEY].length) { - file[INLINE_DIFF_LINES_KEY].forEach(line => { + file[INLINE_DIFF_LINES_KEY].forEach((line) => { Object.assign( line, setDiscussionsExpanded(lineCheck(line) ? mapDiscussions(line) : line), @@ -206,7 +210,7 @@ export default { if (!file[INLINE_DIFF_LINES_KEY].length) { const newDiscussions = (file.discussions || []) - .filter(d => d.id !== discussion.id) + .filter((d) => d.id !== discussion.id) .concat(discussion); Object.assign(file, { @@ -218,26 +222,26 @@ export default { }, [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { - const selectedFile = state.diffFiles.find(f => f.file_hash === fileHash); + const selectedFile = state.diffFiles.find((f) => f.file_hash === fileHash); if (selectedFile) { - updateLineInFile(selectedFile, lineCode, line => + updateLineInFile(selectedFile, lineCode, (line) => Object.assign(line, { - discussions: line.discussions.filter(discussion => discussion.notes.length), + discussions: line.discussions.filter((discussion) => discussion.notes.length), }), ); if (selectedFile.discussions && selectedFile.discussions.length) { selectedFile.discussions = selectedFile.discussions.filter( - discussion => discussion.notes.length, + (discussion) => discussion.notes.length, ); } } }, [types.TOGGLE_LINE_DISCUSSIONS](state, { fileHash, lineCode, expanded }) { - const selectedFile = state.diffFiles.find(f => f.file_hash === fileHash); + const selectedFile = state.diffFiles.find((f) => f.file_hash === fileHash); - updateLineInFile(selectedFile, lineCode, line => { + updateLineInFile(selectedFile, lineCode, (line) => { Object.assign(line, { discussionsExpanded: expanded }); }); }, @@ -260,7 +264,7 @@ export default { [types.UPDATE_DIFF_FILE_COMMENT_FORM](state, formData) { const { fileHash } = formData; - state.commentForms = state.commentForms.map(form => { + state.commentForms = state.commentForms.map((form) => { if (form.fileHash === fileHash) { return { ...formData, @@ -271,7 +275,7 @@ export default { }); }, [types.CLOSE_DIFF_FILE_COMMENT_FORM](state, fileHash) { - state.commentForms = state.commentForms.filter(form => form.fileHash !== fileHash); + state.commentForms = state.commentForms.filter((form) => form.fileHash !== fileHash); }, [types.SET_HIGHLIGHTED_ROW](state, lineCode) { state.highlightedRow = lineCode; @@ -311,7 +315,7 @@ export default { state, { filePath, collapsed, trigger = DIFF_FILE_AUTOMATIC_COLLAPSE }, ) { - const file = state.diffFiles.find(f => f.file_path === filePath); + const file = state.diffFiles.find((f) => f.file_path === filePath); if (file && file.viewer) { if (trigger === DIFF_FILE_MANUAL_COLLAPSE) { @@ -328,17 +332,17 @@ export default { } }, [types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) { - const file = state.diffFiles.find(f => f.file_path === filePath); + const file = state.diffFiles.find((f) => f.file_path === filePath); file[INLINE_DIFF_LINES_KEY] = lines; }, [types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, line }) { - const file = state.diffFiles.find(f => f.file_path === filePath); + const file = state.diffFiles.find((f) => f.file_path === filePath); file[INLINE_DIFF_LINES_KEY].push(line); }, [types.TOGGLE_DIFF_FILE_RENDERING_MORE](state, filePath) { - const file = state.diffFiles.find(f => f.file_path === filePath); + const file = state.diffFiles.find((f) => f.file_path === filePath); file.renderingLines = !file.renderingLines; }, @@ -353,4 +357,7 @@ export default { [types.SET_FILE_BY_FILE](state, fileByFile) { state.viewDiffsFileByFile = fileByFile; }, + [types.SET_MR_FILE_REVIEWS](state, newReviews) { + state.mrReviews = newReviews; + }, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 1839df12c96..c52da558be2 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -15,13 +15,23 @@ import { INLINE_DIFF_LINES_KEY, SHOW_WHITESPACE, NO_SHOW_WHITESPACE, + CONFLICT_OUR, + CONFLICT_THEIR, + CONFLICT_MARKER, + CONFLICT_MARKER_OUR, + CONFLICT_MARKER_THEIR, } from '../constants'; import { prepareRawDiffFile } from '../utils/diff_file'; -export const isAdded = line => ['new', 'new-nonewline'].includes(line.type); -export const isRemoved = line => ['old', 'old-nonewline'].includes(line.type); -export const isUnchanged = line => !line.type; -export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includes(line.type); +export const isAdded = (line) => ['new', 'new-nonewline'].includes(line.type); +export const isRemoved = (line) => ['old', 'old-nonewline'].includes(line.type); +export const isUnchanged = (line) => !line.type; +export const isMeta = (line) => ['match', 'new-nonewline', 'old-nonewline'].includes(line.type); +export const isConflictMarker = (line) => + [CONFLICT_MARKER_OUR, CONFLICT_MARKER_THEIR].includes(line.type); +export const isConflictSeperator = (line) => line.type === CONFLICT_MARKER; +export const isConflictOur = (line) => line.type === CONFLICT_OUR; +export const isConflictTheir = (line) => line.type === CONFLICT_THEIR; /** * Pass in the inline diff lines array which gets converted @@ -42,12 +52,22 @@ export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includ export const parallelizeDiffLines = (diffLines, inline) => { let freeRightIndex = null; + let conflictStartIndex = -1; const lines = []; + // `chunk` is used for dragging to select diff lines + // we are restricting commenting to only lines that appear between + // "expansion rows". Here equal chunks are lines grouped together + // inbetween expansion rows. + let chunk = 0; + for (let i = 0, diffLinesLength = diffLines.length, index = 0; i < diffLinesLength; i += 1) { const line = diffLines[i]; + line.chunk = chunk; + + if (isMeta(line)) chunk += 1; - if (isRemoved(line) || inline) { + if (isRemoved(line) || isConflictOur(line) || inline) { lines.push({ [LINE_POSITION_LEFT]: line, [LINE_POSITION_RIGHT]: null, @@ -58,7 +78,7 @@ export const parallelizeDiffLines = (diffLines, inline) => { freeRightIndex = index; } index += 1; - } else if (isAdded(line)) { + } else if (isAdded(line) || isConflictTheir(line)) { if (freeRightIndex !== null) { // If an old line came before this without a line on the right, this // line can be put to the right of it. @@ -77,15 +97,28 @@ export const parallelizeDiffLines = (diffLines, inline) => { freeRightIndex = null; index += 1; } - } else if (isMeta(line) || isUnchanged(line)) { - // line in the right panel is the same as in the left one - lines.push({ - [LINE_POSITION_LEFT]: line, - [LINE_POSITION_RIGHT]: line, - }); + } else if ( + isMeta(line) || + isUnchanged(line) || + isConflictMarker(line) || + (isConflictSeperator(line) && inline) + ) { + if (conflictStartIndex <= 0) { + // line in the right panel is the same as in the left one + lines.push({ + [LINE_POSITION_LEFT]: line, + [LINE_POSITION_RIGHT]: !inline && line, + }); - freeRightIndex = null; - index += 1; + if (!inline && isConflictMarker(line)) { + conflictStartIndex = index; + } + freeRightIndex = null; + index += 1; + } else { + lines[conflictStartIndex][LINE_POSITION_RIGHT] = line; + conflictStartIndex = -1; + } } } @@ -93,10 +126,10 @@ export const parallelizeDiffLines = (diffLines, inline) => { }; export function findDiffFile(files, match, matchKey = 'file_hash') { - return files.find(file => file[matchKey] === match); + return files.find((file) => file[matchKey] === match); } -export const getReversePosition = linePosition => { +export const getReversePosition = (linePosition) => { if (linePosition === LINE_POSITION_RIGHT) { return LINE_POSITION_LEFT; } @@ -173,7 +206,7 @@ export const findIndexInInlineLines = (lines, lineNumbers) => { const { oldLineNumber, newLineNumber } = lineNumbers; return lines.findIndex( - line => line.old_line === oldLineNumber && line.new_line === newLineNumber, + (line) => line.old_line === oldLineNumber && line.new_line === newLineNumber, ); }; @@ -346,7 +379,7 @@ export function prepareLineForRenamedFile({ line, diffFile, index = 0 }) { function prepareDiffFileLines(file) { const inlineLines = file[INLINE_DIFF_LINES_KEY]; - inlineLines.forEach(line => prepareLine(line, file)); // WARNING: In-Place Mutations! + inlineLines.forEach((line) => prepareLine(line, file)); // WARNING: In-Place Mutations! Object.assign(file, { inlineLinesCount: inlineLines.length, @@ -400,7 +433,7 @@ export function getDiffPositionByLineCode(diffFiles) { let lines = []; lines = diffFiles.reduce((acc, diffFile) => { - diffFile[INLINE_DIFF_LINES_KEY].forEach(line => { + diffFile[INLINE_DIFF_LINES_KEY].forEach((line) => { acc.push({ file: diffFile, line }); }); @@ -447,21 +480,21 @@ export function isDiscussionApplicableToLine({ discussion, diffPosition, latestD ...(discussion.positions || []), ]; - const removeLineRange = position => { + const removeLineRange = (position) => { const { line_range: pNotUsed, ...positionNoLineRange } = position; return positionNoLineRange; }; return discussionPositions .map(removeLineRange) - .some(position => isEqual(position, diffPositionCopy)); + .some((position) => isEqual(position, diffPositionCopy)); } // eslint-disable-next-line return latestDiff && discussion.active && line_code === discussion.line_code; } -export const getLowestSingleFolder = folder => { +export const getLowestSingleFolder = (folder) => { const getFolder = (blob, start = []) => blob.tree.reduce( (acc, file) => { @@ -493,8 +526,8 @@ export const getLowestSingleFolder = folder => { }; }; -export const flattenTree = tree => { - const flatten = blobTree => +export const flattenTree = (tree) => { + const flatten = (blobTree) => blobTree.reduce((acc, file) => { const blob = file; let treeToFlatten = blob.tree; @@ -516,7 +549,7 @@ export const flattenTree = tree => { return flatten(tree); }; -export const generateTreeList = files => { +export const generateTreeList = (files) => { const { treeEntries, tree } = files.reduce( (acc, file) => { const split = file.new_path.split('/'); @@ -566,8 +599,8 @@ export const generateTreeList = files => { return { treeEntries, tree: flattenTree(tree) }; }; -export const getDiffMode = diffFile => { - const diffModeKey = Object.keys(diffModes).find(key => diffFile[`${key}_file`]); +export const getDiffMode = (diffFile) => { + const diffModeKey = Object.keys(diffModes).find((key) => diffFile[`${key}_file`]); return ( diffModes[diffModeKey] || (diffFile.viewer && @@ -615,11 +648,11 @@ export const convertExpandLines = ({ return lines; }; -export const idleCallback = cb => requestIdleCallback(cb); +export const idleCallback = (cb) => requestIdleCallback(cb); function getLinesFromFileByLineCode(file, lineCode) { const inlineLines = file[INLINE_DIFF_LINES_KEY]; - const matchesCode = line => line.line_code === lineCode; + const matchesCode = (line) => line.line_code === lineCode; return inlineLines.filter(matchesCode); } @@ -628,15 +661,15 @@ export const updateLineInFile = (selectedFile, lineCode, updateFn) => { getLinesFromFileByLineCode(selectedFile, lineCode).forEach(updateFn); }; -export const allDiscussionWrappersExpanded = diff => { +export const allDiscussionWrappersExpanded = (diff) => { let discussionsExpanded = true; - const changeExpandedResult = line => { + const changeExpandedResult = (line) => { if (line && line.discussions.length) { discussionsExpanded = discussionsExpanded && line.discussionsExpanded; } }; - diff[INLINE_DIFF_LINES_KEY].forEach(line => { + diff[INLINE_DIFF_LINES_KEY].forEach((line) => { changeExpandedResult(line); }); diff --git a/app/assets/javascripts/diffs/utils/diff_file.js b/app/assets/javascripts/diffs/utils/diff_file.js index 69d0e49e501..ce0398e75fc 100644 --- a/app/assets/javascripts/diffs/utils/diff_file.js +++ b/app/assets/javascripts/diffs/utils/diff_file.js @@ -4,11 +4,12 @@ import { DIFF_FILE_MANUAL_COLLAPSE, DIFF_FILE_AUTOMATIC_COLLAPSE, } from '../constants'; +import { getDerivedMergeRequestInformation } from './merge_request'; import { uuids } from './uuids'; function fileSymlinkInformation(file, fileList) { - const duplicates = fileList.filter(iteratedFile => iteratedFile.file_hash === file.file_hash); - const includesSymlink = duplicates.some(iteratedFile => { + const duplicates = fileList.filter((iteratedFile) => iteratedFile.file_hash === file.file_hash); + const includesSymlink = duplicates.some((iteratedFile) => { return [iteratedFile.a_mode, iteratedFile.b_mode].includes(DIFF_FILE_SYMLINK_MODE); }); const brokenSymlinkScenario = duplicates.length > 1 && includesSymlink; @@ -34,8 +35,12 @@ function collapsed(file) { } function identifier(file) { + const { userOrGroup, project, id } = getDerivedMergeRequestInformation({ + endpoint: file.load_collapsed_diff_url, + }); + return uuids({ - seeds: [file.file_identifier_hash, file.blob?.id], + seeds: [userOrGroup, project, id, file.file_identifier_hash, file.blob?.id], })[0]; } @@ -48,10 +53,10 @@ export function prepareRawDiffFile({ file, allFiles, meta = false }) { }, }; - // It's possible, but not confirmed, that `content_sha` isn't available sometimes + // It's possible, but not confirmed, that `blob.id` isn't available sometimes // See: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49506#note_464692057 // We don't want duplicate IDs if that's the case, so we just don't assign an ID - if (!meta && file.blob?.id) { + if (!meta && file.blob?.id && file.load_collapsed_diff_url) { additionalProperties.id = identifier(file); } diff --git a/app/assets/javascripts/diffs/utils/file_reviews.js b/app/assets/javascripts/diffs/utils/file_reviews.js new file mode 100644 index 00000000000..0047955643a --- /dev/null +++ b/app/assets/javascripts/diffs/utils/file_reviews.js @@ -0,0 +1,61 @@ +function getFileReviewsKey(mrPath) { + return `${mrPath}-file-reviews`; +} + +export function getReviewsForMergeRequest(mrPath) { + const reviewsForMr = localStorage.getItem(getFileReviewsKey(mrPath)); + let reviews = {}; + + if (reviewsForMr) { + try { + reviews = JSON.parse(reviewsForMr); + } catch (err) { + reviews = {}; + } + } + + return reviews; +} + +export function setReviewsForMergeRequest(mrPath, reviews) { + localStorage.setItem(getFileReviewsKey(mrPath), JSON.stringify(reviews)); + + return reviews; +} + +export function isFileReviewed(reviews, file) { + const fileReviews = reviews[file.file_identifier_hash]; + + return file?.id && fileReviews?.length ? new Set(fileReviews).has(file.id) : false; +} + +export function reviewable(file) { + return Boolean(file.id) && Boolean(file.file_identifier_hash); +} + +export function markFileReview(reviews, file, reviewed = true) { + const usableReviews = { ...(reviews || {}) }; + let updatedReviews = usableReviews; + let fileReviews; + + if (reviewable(file)) { + fileReviews = new Set([...(usableReviews[file.file_identifier_hash] || [])]); + + if (reviewed) { + fileReviews.add(file.id); + } else { + fileReviews.delete(file.id); + } + + updatedReviews = { + ...usableReviews, + [file.file_identifier_hash]: Array.from(fileReviews), + }; + + if (updatedReviews[file.file_identifier_hash].length === 0) { + delete updatedReviews[file.file_identifier_hash]; + } + } + + return updatedReviews; +} diff --git a/app/assets/javascripts/diffs/utils/merge_request.js b/app/assets/javascripts/diffs/utils/merge_request.js new file mode 100644 index 00000000000..edb4304f558 --- /dev/null +++ b/app/assets/javascripts/diffs/utils/merge_request.js @@ -0,0 +1,20 @@ +const endpointRE = /^(\/?(.+?)\/(.+?)\/-\/merge_requests\/(\d+)).*$/i; + +export function getDerivedMergeRequestInformation({ endpoint } = {}) { + let mrPath; + let userOrGroup; + let project; + let id; + const matches = endpointRE.exec(endpoint); + + if (matches) { + [, mrPath, userOrGroup, project, id] = matches; + } + + return { + mrPath, + userOrGroup, + project, + id, + }; +} diff --git a/app/assets/javascripts/diffs/utils/uuids.js b/app/assets/javascripts/diffs/utils/uuids.js index 12448350e62..1fe5f9f6499 100644 --- a/app/assets/javascripts/diffs/utils/uuids.js +++ b/app/assets/javascripts/diffs/utils/uuids.js @@ -11,7 +11,7 @@ * @typedef {String} UUIDv4 */ -import MersenneTwister from 'mersenne-twister'; +import { MersenneTwister } from 'fast-mersenne-twister'; import stringHash from 'string-hash'; import { isString } from 'lodash'; import { v4 } from 'uuid'; @@ -49,7 +49,7 @@ function randomValuesForUuid(prng) { const buffer = new ArrayBuffer(4); const view = new DataView(buffer); - view.setUint32(0, prng.random_int()); + view.setUint32(0, prng.randomNumber()); randomValues.push(view.getUint8(0), view.getUint8(1), view.getUint8(2), view.getUint8(3)); } diff --git a/app/assets/javascripts/diffs/workers/tree_worker.js b/app/assets/javascripts/diffs/workers/tree_worker.js index 415c463fd19..2fa1934439e 100644 --- a/app/assets/javascripts/diffs/workers/tree_worker.js +++ b/app/assets/javascripts/diffs/workers/tree_worker.js @@ -2,7 +2,7 @@ import { sortTree } from '~/ide/stores/utils'; import { generateTreeList } from '../store/utils'; // eslint-disable-next-line no-restricted-globals -self.addEventListener('message', e => { +self.addEventListener('message', (e) => { const { data } = e; if (data === undefined) { |