diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
18 files changed, 464 insertions, 138 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 9ccba88f7e6..9b3db78724d 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -45,7 +45,9 @@ import { import diffsEventHub from '../event_hub'; import { reviewStatuses } from '../utils/file_reviews'; import { diffsApp } from '../utils/performance'; +import { updateChangesTabCount } from '../utils/merge_request'; import { queueRedisHllEvents } from '../utils/queue_events'; +import FindingsDrawer from './shared/findings_drawer.vue'; import CollapsedFilesWarning from './collapsed_files_warning.vue'; import CommitWidget from './commit_widget.vue'; import CompareVersions from './compare_versions.vue'; @@ -59,6 +61,7 @@ import PreRenderer from './pre_renderer.vue'; export default { name: 'DiffsApp', components: { + FindingsDrawer, DynamicScroller, DynamicScrollerItem, PreRenderer, @@ -199,6 +202,7 @@ export default { numTotalFiles: 'realSize', numVisibleFiles: 'size', }), + ...mapState('findingsDrawer', ['activeDrawer']), ...mapState('diffs', [ 'showTreeList', 'isLoading', @@ -233,6 +237,7 @@ export default { 'flatBlobsList', ]), ...mapGetters(['isNotesFetched', 'getNoteableData']), + ...mapGetters('findingsDrawer', ['activeDrawer']), diffs() { if (!this.viewDiffsFileByFile) { return this.diffFiles; @@ -248,6 +253,9 @@ export default { renderDiffFiles() { return this.flatBlobsList.length > 0; }, + diffsIncomplete() { + return this.flatBlobsList.length !== this.diffFiles.length; + }, renderFileTree() { return this.renderDiffFiles && this.showTreeList; }, @@ -308,6 +316,11 @@ export default { diffViewType() { this.adjustView(); }, + viewDiffsFileByFile(newViewFileByFile) { + if (!newViewFileByFile && this.diffsIncomplete && this.glFeatures.singleFileFileByFile) { + this.refetchDiffData({ refetchMeta: false }); + } + }, shouldShow() { // When the shouldShow property changed to true, the route is rendered for the first time // and if we have the isLoading as true this means we didn't fetch the data @@ -337,8 +350,6 @@ export default { mrReviews: this.rehydratedMrReviews, }); - this.interfaceWithDOM(); - if (this.endpointCodequality) { this.setCodequalityEndpoint(this.endpointCodequality); } @@ -426,41 +437,48 @@ export default { 'setCodequalityEndpoint', 'fetchDiffFilesMeta', 'fetchDiffFilesBatch', + 'fetchFileByFile', 'fetchCoverageFiles', 'fetchCodequality', + 'rereadNoteHash', 'startRenderDiffsQueue', 'assignDiscussionsToDiff', 'setHighlightedRow', 'cacheTreeListWidth', - 'scrollToFile', + 'goToFile', 'setShowTreeList', 'navigateToDiffFileIndex', 'setFileByFile', 'disableVirtualScroller', ]), + ...mapActions('findingsDrawer', ['setDrawer']), + closeDrawer() { + this.setDrawer({}); + }, subscribeToEvents() { notesEventHub.$once('fetchDiffData', this.fetchData); notesEventHub.$on('refetchDiffData', this.refetchDiffData); + if (this.glFeatures.singleFileFileByFile) { + diffsEventHub.$on('diffFilesModified', this.setDiscussions); + notesEventHub.$on('fetchedNotesData', this.rereadNoteHash); + } }, unsubscribeFromEvents() { + if (this.glFeatures.singleFileFileByFile) { + notesEventHub.$off('fetchedNotesData', this.rereadNoteHash); + diffsEventHub.$off('diffFilesModified', this.setDiscussions); + } notesEventHub.$off('refetchDiffData', this.refetchDiffData); notesEventHub.$off('fetchDiffData', this.fetchData); }, - interfaceWithDOM() { - this.diffsTab = document.querySelector('.js-diffs-tab'); - }, - updateChangesTabCount() { - const badge = this.diffsTab.querySelector('.gl-badge'); - - if (this.diffsTab && badge) { - badge.textContent = this.diffFilesLength; - } - }, navigateToDiffFileNumber(number) { - this.navigateToDiffFileIndex(number - 1); + this.navigateToDiffFileIndex({ + index: number - 1, + singleFile: this.glFeatures.singleFileFileByFile, + }); }, - refetchDiffData() { - this.fetchData(false); + refetchDiffData({ refetchMeta = true } = {}) { + this.fetchData({ toggleTree: false, fetchMeta: refetchMeta }); }, needsReload() { return this.diffFiles.length && isSingleViewStyle(this.diffFiles[0]); @@ -468,42 +486,52 @@ export default { needsFirstLoad() { return !this.diffFiles.length; }, - fetchData(toggleTree = true) { - this.fetchDiffFilesMeta() - .then((data) => { - let realSize = 0; - - if (data) { - realSize = data.real_size; - } - - this.diffFilesLength = parseInt(realSize, 10) || 0; - if (toggleTree) { - this.setTreeDisplay(); - } - - this.updateChangesTabCount(); - }) - .catch(() => { - createAlert({ - message: __('Something went wrong on our end. Please try again!'), + fetchData({ toggleTree = true, fetchMeta = true } = {}) { + if (fetchMeta) { + this.fetchDiffFilesMeta() + .then((data) => { + let realSize = 0; + + if (data) { + realSize = data.real_size; + + if (this.viewDiffsFileByFile && this.glFeatures.singleFileFileByFile) { + this.fetchFileByFile(); + } + } + + this.diffFilesLength = parseInt(realSize, 10) || 0; + if (toggleTree) { + this.setTreeDisplay(); + } + + updateChangesTabCount({ + count: this.diffFilesLength, + }); + }) + .catch(() => { + createAlert({ + message: __('Something went wrong on our end. Please try again!'), + }); }); - }); + } - this.fetchDiffFilesBatch() - .then(() => { - if (toggleTree) this.setTreeDisplay(); - // Guarantee the discussions are assigned after the batch finishes. - // Just watching the length of the discussions or the diff files - // isn't enough, because with split diff loading, neither will - // change when loading the other half of the diff files. - this.setDiscussions(); - }) - .catch(() => { - createAlert({ - message: __('Something went wrong on our end. Please try again!'), + if (!this.viewDiffsFileByFile || !this.glFeatures.singleFileFileByFile) { + this.fetchDiffFilesBatch() + .then(() => { + if (toggleTree) this.setTreeDisplay(); + // Guarantee the discussions are assigned after the batch finishes. + // Just watching the length of the discussions or the diff files + // isn't enough, because with split diff loading, neither will + // change when loading the other half of the diff files. + this.setDiscussions(); + }) + .catch(() => { + createAlert({ + message: __('Something went wrong on our end. Please try again!'), + }); }); - }); + } if (this.endpointCoverage) { this.fetchCoverageFiles(); @@ -579,7 +607,10 @@ export default { jumpToFile(step) { const targetIndex = this.currentDiffIndex + step; if (targetIndex >= 0 && targetIndex < this.flatBlobsList.length) { - this.scrollToFile({ path: this.flatBlobsList[targetIndex].path }); + this.goToFile({ + path: this.flatBlobsList[targetIndex].path, + singleFile: this.glFeatures.singleFileFileByFile, + }); } }, setTreeDisplay() { @@ -640,6 +671,11 @@ export default { <template> <div v-show="shouldShow"> + <findings-drawer + v-if="glFeatures.codeQualityInlineDrawer" + :drawer="activeDrawer" + @close="closeDrawer" + /> <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 :diff-files-count-text="numTotalFiles" /> diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index 1857ff557e6..d050f2fb9ae 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -1,5 +1,5 @@ <script> -import { GlButtonGroup, GlButton, GlTooltipDirective } from '@gitlab/ui'; +import { GlButtonGroup, GlButton, GlTooltipDirective, GlFormCheckbox } from '@gitlab/ui'; import SafeHtml from '~/vue_shared/directives/safe_html'; import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; @@ -30,6 +30,7 @@ export default { CommitPipelineStatus, GlButtonGroup, GlButton, + GlFormCheckbox, }, directives: { GlTooltip: GlTooltipDirective, @@ -117,12 +118,11 @@ export default { </div> <div> <div class="d-flex float-left align-items-center align-self-start"> - <input + <gl-form-checkbox v-if="isSelectable" - class="gl-mr-3" - type="checkbox" :checked="checked" - @change="$emit('handleCheckboxChange', $event.target.checked)" + class="gl-mt-3" + @change="$emit('handleCheckboxChange', !checked)" /> <user-avatar-link :link-href="authorUrl" diff --git a/app/assets/javascripts/diffs/components/diff_code_quality.vue b/app/assets/javascripts/diffs/components/diff_code_quality.vue index 5392c631c14..f3f05e3d9d9 100644 --- a/app/assets/javascripts/diffs/components/diff_code_quality.vue +++ b/app/assets/javascripts/diffs/components/diff_code_quality.vue @@ -1,27 +1,19 @@ <script> -import { GlButton, GlIcon } from '@gitlab/ui'; -import { SEVERITY_CLASSES, SEVERITY_ICONS } from '~/ci/reports/codequality_report/constants'; +import { GlButton } from '@gitlab/ui'; import { NEW_CODE_QUALITY_FINDINGS } from '../i18n'; +import DiffCodeQualityItem from './diff_code_quality_item.vue'; export default { i18n: { newFindings: NEW_CODE_QUALITY_FINDINGS, }, - components: { GlButton, GlIcon }, + components: { GlButton, DiffCodeQualityItem }, props: { codeQuality: { type: Array, required: true, }, }, - methods: { - severityClass(severity) { - return SEVERITY_CLASSES[severity] || SEVERITY_CLASSES.unknown; - }, - severityIcon(severity) { - return SEVERITY_ICONS[severity] || SEVERITY_ICONS.unknown; - }, - }, }; </script> @@ -37,23 +29,11 @@ export default { {{ $options.i18n.newFindings }} </h4> <ul class="gl-list-style-none gl-mb-0 gl-p-0"> - <li + <diff-code-quality-item v-for="finding in codeQuality" :key="finding.description" - class="gl-pt-1 gl-pb-1 gl-font-regular gl-display-flex" - > - <span class="gl-mr-3"> - <gl-icon - :size="12" - :name="severityIcon(finding.severity)" - :class="severityClass(finding.severity)" - class="codequality-severity-icon" - /> - </span> - <span> - <span class="severity-copy">{{ finding.severity }}</span> - {{ finding.description }} - </span> - </li> + :finding="finding" + /> </ul> <gl-button data-testid="diff-codequality-close" diff --git a/app/assets/javascripts/diffs/components/diff_code_quality_item.vue b/app/assets/javascripts/diffs/components/diff_code_quality_item.vue new file mode 100644 index 00000000000..eede110f46c --- /dev/null +++ b/app/assets/javascripts/diffs/components/diff_code_quality_item.vue @@ -0,0 +1,54 @@ +<script> +import { GlLink, GlIcon } from '@gitlab/ui'; +import { mapActions } from 'vuex'; +import { SEVERITY_CLASSES, SEVERITY_ICONS } from '~/ci/reports/codequality_report/constants'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; + +export default { + components: { GlLink, GlIcon }, + mixins: [glFeatureFlagsMixin()], + props: { + finding: { + type: Object, + required: true, + }, + }, + methods: { + severityClass(severity) { + return SEVERITY_CLASSES[severity] || SEVERITY_CLASSES.unknown; + }, + severityIcon(severity) { + return SEVERITY_ICONS[severity] || SEVERITY_ICONS.unknown; + }, + toggleDrawer() { + this.setDrawer(this.finding); + }, + ...mapActions('findingsDrawer', ['setDrawer']), + }, +}; +</script> + +<template> + <li class="gl-py-1 gl-font-regular gl-display-flex"> + <span class="gl-mr-3"> + <gl-icon + :size="12" + :name="severityIcon(finding.severity)" + :class="severityClass(finding.severity)" + class="codequality-severity-icon" + /> + </span> + <span + v-if="glFeatures.codeQualityInlineDrawer" + data-testid="description-button-section" + class="gl-display-flex" + > + <gl-link category="primary" variant="link" @click="toggleDrawer"> + {{ finding.severity }} - {{ finding.description }}</gl-link + > + </span> + <span v-else data-testid="description-plain-text" class="gl-display-flex"> + {{ finding.severity }} - {{ finding.description }} + </span> + </li> +</template> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index c19174dda8a..a58178eaef7 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -209,7 +209,11 @@ export default { if (this.hasDiff) { this.postRender(); - } else if (this.viewDiffsFileByFile && !this.isCollapsed) { + } else if ( + this.viewDiffsFileByFile && + !this.isCollapsed && + !this.glFeatures.singleFileFileByFile + ) { this.requestDiff(); } diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 16f45c3ad6a..c3a4897ce78 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -50,6 +50,7 @@ export default { i18n: { ...DIFF_FILE_HEADER, compareButtonLabel: __('Compare submodule commit revisions'), + fileModeTooltip: __('File permissions'), }, props: { discussionPath: { @@ -201,6 +202,9 @@ export default { externalUrlLabel() { return sprintf(__('View on %{url}'), { url: this.diffFile.formatted_external_url }); }, + labelToggleFile() { + return this.expanded ? __('Hide file contents') : __('Show file contents'); + }, }, watch: { 'idState.moreActionsShown': { @@ -287,12 +291,14 @@ export default { @click.self="handleToggleFile" > <div class="file-header-content"> - <gl-icon + <gl-button v-if="collapsible" - ref="collapseIcon" - :name="collapseIcon" - :size="16" - class="diff-toggle-caret gl-mr-2" + ref="collapseButton" + class="gl-mr-2" + category="tertiary" + size="small" + :icon="collapseIcon" + :aria-label="labelToggleFile" @click.stop="handleToggleFile" /> <a @@ -342,7 +348,13 @@ export default { data-track-property="diff_copy_file" /> - <small v-if="isModeChanged" ref="fileMode" class="mr-1"> + <small + v-if="isModeChanged" + ref="fileMode" + v-gl-tooltip.hover + class="mr-1" + :title="$options.i18n.fileModeTooltip" + > {{ diffFile.a_mode }} → {{ diffFile.b_mode }} </small> @@ -364,7 +376,7 @@ export default { v-if="isReviewable && showLocalFileReviews" v-gl-tooltip.hover data-testid="fileReviewCheckbox" - class="gl-mr-5 gl-display-flex gl-align-items-center" + class="gl-mr-5 gl-mb-n3 gl-display-flex gl-align-items-center" :title="$options.i18n.fileReviewTooltip" :checked="reviewed" @change="toggleReview" 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 f63ab1bb067..43ba527dad8 100644 --- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue +++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue @@ -8,7 +8,7 @@ import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import MultilineCommentForm from '~/notes/components/multiline_comment_form.vue'; import { commentLineOptions, formatLineRange } from '~/notes/components/multiline_comment_utils'; import NoteForm from '~/notes/components/note_form.vue'; -import autosave from '~/notes/mixins/autosave'; +import { capitalizeFirstCharacter } from '~/lib/utils/text_utility'; import { DIFF_NOTE_TYPE, INLINE_DIFF_LINES_KEY, @@ -21,7 +21,7 @@ export default { NoteForm, MultilineCommentForm, }, - mixins: [autosave, diffLineNoteFormMixin, glFeatureFlagsMixin()], + mixins: [diffLineNoteFormMixin, glFeatureFlagsMixin()], props: { diffFileHash: { type: String, @@ -146,6 +146,27 @@ export default { return lines; }, + autosaveKey() { + if (!this.isLoggedIn) return ''; + + const { + id, + noteable_type: noteableTypeUnderscored, + noteableType, + diff_head_sha: diffHeadSha, + source_project_id: sourceProjectId, + } = this.noteableData; + + return [ + s__('Autosave|Note'), + capitalizeFirstCharacter(noteableTypeUnderscored || noteableType), + id, + diffHeadSha, + DIFF_NOTE_TYPE, + sourceProjectId, + this.line.line_code, + ].join('/'); + }, }, created() { if (this.range) { @@ -155,17 +176,6 @@ export default { } }, mounted() { - if (this.isLoggedIn) { - const keys = [ - this.noteableData.diff_head_sha, - DIFF_NOTE_TYPE, - this.noteableData.source_project_id, - this.line.line_code, - ]; - - this.initAutoSave(this.noteableData, keys); - } - if (this.selectedCommentPosition) { this.commentLineStart = this.selectedCommentPosition.start; } @@ -196,9 +206,6 @@ export default { lineCode: this.line.line_code, fileHash: this.diffFileHash, }); - this.$nextTick(() => { - this.resetAutoSave(); - }); }), handleSaveNote(note) { return this.saveDiffDiscussion({ note, formData: this.formData }).then(() => @@ -232,6 +239,7 @@ export default { :diff-file="diffFile" :show-suggest-popover="showSuggestPopover" :save-button-title="__('Comment')" + :autosave-key="autosaveKey" class="diff-comment-form gl-mt-3" @handleFormUpdateAddToReview="addToReview" @cancelForm="handleCancelCommentForm" diff --git a/app/assets/javascripts/diffs/components/diff_stats.vue b/app/assets/javascripts/diffs/components/diff_stats.vue index e8b4ff16aec..7de8eff7863 100644 --- a/app/assets/javascripts/diffs/components/diff_stats.vue +++ b/app/assets/javascripts/diffs/components/diff_stats.vue @@ -76,7 +76,7 @@ export default { class="diff-stats-group gl-text-red-500 gl-display-flex gl-align-items-center" :class="{ bold: isCompareVersionsHeader }" > - <span>-</span> + <span>−</span> <span data-testid="js-file-deletion-line">{{ removedLines }}</span> </div> </div> diff --git a/app/assets/javascripts/diffs/components/diff_view.vue b/app/assets/javascripts/diffs/components/diff_view.vue index a2e052e0f93..348d6d1d78d 100644 --- a/app/assets/javascripts/diffs/components/diff_view.vue +++ b/app/assets/javascripts/diffs/components/diff_view.vue @@ -1,7 +1,6 @@ <script> import { mapGetters, mapState, mapActions } from 'vuex'; import { IdState } from 'vendor/vue-virtual-scroller'; -import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import DraftNote from '~/batch_comments/components/draft_note.vue'; import draftCommentsMixin from '~/diffs/mixins/draft_comments'; import { getCommentedLines } from '~/notes/components/multiline_comment_utils'; @@ -21,11 +20,7 @@ export default { DiffCommentCell, DraftNote, }, - mixins: [ - draftCommentsMixin, - IdState({ idProp: (vm) => vm.diffFile.file_hash }), - glFeatureFlagsMixin(), - ], + mixins: [draftCommentsMixin, IdState({ idProp: (vm) => vm.diffFile.file_hash })], props: { diffFile: { type: Object, @@ -265,10 +260,7 @@ export default { @stopdragging="onStopDragging" /> <diff-line - v-if=" - glFeatures.refactorCodeQualityInlineFindings && - codeQualityExpandedLines.includes(getCodeQualityLine(line)) - " + v-if="codeQualityExpandedLines.includes(getCodeQualityLine(line))" :key="line.line_code" :line="line" @hideCodeQualityFindings="hideCodeQualityFindings" diff --git a/app/assets/javascripts/diffs/components/hidden_files_warning.vue b/app/assets/javascripts/diffs/components/hidden_files_warning.vue index f6a8c679f3b..26d37484541 100644 --- a/app/assets/javascripts/diffs/components/hidden_files_warning.vue +++ b/app/assets/javascripts/diffs/components/hidden_files_warning.vue @@ -1,17 +1,18 @@ <script> -import { GlAlert, GlSprintf } from '@gitlab/ui'; +import { GlAlert, GlButton, GlSprintf } from '@gitlab/ui'; import { __ } from '~/locale'; export const i18n = { - title: __('Too many changes to show.'), + title: __('Some changes are not shown.'), plainDiff: __('Plain diff'), - emailPatch: __('Email patch'), + emailPatch: __('Patches'), }; export default { i18n, components: { GlAlert, + GlButton, GlSprintf, }, props: { @@ -38,18 +39,15 @@ export default { <template> <gl-alert variant="warning" + class="gl-mx-5 gl-mb-4 gl-mt-3" :title="$options.i18n.title" - :primary-button-text="$options.i18n.plainDiff" - :primary-button-link="plainDiffPath" - :secondary-button-text="$options.i18n.emailPatch" - :secondary-button-link="emailPatchPath" :dismissible="false" > <gl-sprintf :message=" sprintf( __( - 'To preserve performance only %{strongStart}%{visible} of %{total}%{strongEnd} files are displayed.', + 'For a faster browsing experience, only %{strongStart}%{visible} of %{total}%{strongEnd} files are shown. Download one of the files below to see all changes.', ), { visible, total } /* eslint-disable-line @gitlab/vue-no-new-non-primitive-in-template */, ) @@ -59,5 +57,13 @@ export default { <strong>{{ content }}</strong> </template> </gl-sprintf> + <template #actions> + <gl-button :href="plainDiffPath" class="gl-mr-3 gl-alert-action"> + {{ $options.i18n.plainDiff }} + </gl-button> + <gl-button :href="emailPatchPath" class="gl-alert-action"> + {{ $options.i18n.emailPatch }} + </gl-button> + </template> </gl-alert> </template> diff --git a/app/assets/javascripts/diffs/components/shared/findings_drawer.vue b/app/assets/javascripts/diffs/components/shared/findings_drawer.vue new file mode 100644 index 00000000000..da880c6f3ca --- /dev/null +++ b/app/assets/javascripts/diffs/components/shared/findings_drawer.vue @@ -0,0 +1,110 @@ +<script> +import { GlDrawer, GlIcon, GlLink } from '@gitlab/ui'; +import SafeHtml from '~/vue_shared/directives/safe_html'; +import { s__ } from '~/locale'; +import { DRAWER_Z_INDEX } from '~/lib/utils/constants'; +import { SEVERITY_CLASSES, SEVERITY_ICONS } from '~/ci/reports/codequality_report/constants'; +import { getContentWrapperHeight } from '~/lib/utils/dom_utils'; + +export const i18n = { + severity: s__('FindingsDrawer|Severity:'), + engine: s__('FindingsDrawer|Engine:'), + category: s__('FindingsDrawer|Category:'), + otherLocations: s__('FindingsDrawer|Other locations:'), +}; + +export default { + i18n, + components: { GlDrawer, GlIcon, GlLink }, + directives: { + SafeHtml, + }, + props: { + drawer: { + type: Object, + required: true, + }, + }, + safeHtmlConfig: { + ALLOWED_TAGS: ['a', 'h1', 'h2', 'p'], + ALLOWED_ATTR: ['href', 'rel'], + }, + computed: { + drawerOffsetTop() { + return getContentWrapperHeight('.content-wrapper'); + }, + }, + DRAWER_Z_INDEX, + methods: { + severityClass(severity) { + return SEVERITY_CLASSES[severity] || SEVERITY_CLASSES.unknown; + }, + severityIcon(severity) { + return SEVERITY_ICONS[severity] || SEVERITY_ICONS.unknown; + }, + }, +}; +</script> +<template> + <gl-drawer + :header-height="drawerOffsetTop" + :z-index="$options.DRAWER_Z_INDEX" + class="findings-drawer" + :open="Object.keys(drawer).length !== 0" + @close="$emit('close')" + > + <template #title> + <h2 data-testid="findings-drawer-heading" class="gl-font-size-h2 gl-mt-0 gl-mb-0"> + {{ drawer.description }} + </h2> + </template> + + <template #default> + <ul class="gl-list-style-none gl-border-b-initial gl-mb-0 gl-pb-0!"> + <li data-testid="findings-drawer-severity" class="gl-mb-4"> + <span class="gl-font-weight-bold">{{ $options.i18n.severity }}</span> + <gl-icon + data-testid="findings-drawer-severity-icon" + :size="12" + :name="severityIcon(drawer.severity)" + :class="severityClass(drawer.severity)" + class="codequality-severity-icon" + /> + + {{ drawer.severity }} + </li> + <li data-testid="findings-drawer-engine" class="gl-mb-4"> + <span class="gl-font-weight-bold">{{ $options.i18n.engine }}</span> + {{ drawer.engineName }} + </li> + <li data-testid="findings-drawer-category" class="gl-mb-4"> + <span class="gl-font-weight-bold">{{ $options.i18n.category }}</span> + {{ drawer.categories ? drawer.categories[0] : '' }} + </li> + <li data-testid="findings-drawer-other-locations" class="gl-mb-4"> + <span class="gl-font-weight-bold gl-mb-3 gl-display-block">{{ + $options.i18n.otherLocations + }}</span> + <ul class="gl-pl-6"> + <li + v-for="otherLocation in drawer.otherLocations" + :key="otherLocation.path" + class="gl-mb-1" + > + <gl-link + data-testid="findings-drawer-other-locations-link" + :href="otherLocation.href" + >{{ otherLocation.path }}</gl-link + > + </li> + </ul> + </li> + </ul> + <span + v-safe-html:[$options.safeHtmlConfig]="drawer.content ? drawer.content.body : ''" + data-testid="findings-drawer-body" + class="drawer-body gl-display-block gl-px-3 gl-py-0!" + ></span> + </template> + </gl-drawer> +</template> diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index ab08c72b08f..4f1875e9175 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -5,6 +5,7 @@ import micromatch from 'micromatch'; import { debounce } from 'lodash'; import { getModifierKey } from '~/constants'; import { s__, sprintf } from '~/locale'; +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { RecycleScroller } from 'vendor/vue-virtual-scroller'; import DiffFileRow from './diff_file_row.vue'; @@ -19,6 +20,7 @@ export default { DiffFileRow, RecycleScroller, }, + mixins: [glFeatureFlagsMixin()], props: { hideFileStats: { type: Boolean, @@ -105,7 +107,7 @@ export default { this.resizeObserver.disconnect(); }, methods: { - ...mapActions('diffs', ['toggleTreeOpen', 'scrollToFile']), + ...mapActions('diffs', ['toggleTreeOpen', 'goToFile']), clearSearch() { this.search = ''; }, @@ -128,7 +130,7 @@ export default { > <div class="gl-pb-3 position-relative tree-list-search d-flex"> <div class="flex-fill d-flex"> - <gl-icon name="search" class="position-absolute tree-list-icon" /> + <gl-icon name="search" class="gl-absolute gl-top-5 tree-list-icon" /> <label for="diff-tree-search" class="sr-only">{{ $options.searchPlaceholder }}</label> <input id="diff-tree-search" @@ -154,7 +156,7 @@ export default { <div ref="scrollRoot" :class="{ 'tree-list-blobs': !renderTreeList || search }" - class="gl-flex-grow-1" + class="gl-flex-grow-1 mr-tree-list" > <recycle-scroller v-if="flatFilteredTreeList.length" @@ -172,10 +174,10 @@ export default { :hide-file-stats="hideFileStats" :current-diff-file-id="currentDiffFileId" :style="{ '--level': item.level }" - :class="{ 'tree-list-parent': item.tree.length }" + :class="{ 'tree-list-parent': item.level > 0 }" class="gl-relative" @toggleTreeOpen="toggleTreeOpen" - @clickFile="(path) => scrollToFile({ path })" + @clickFile="(path) => goToFile({ singleFile: glFeatures.singleFileFileByFile, path })" /> </template> <template #after> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index 873c4819669..a459def6b4b 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -112,3 +112,6 @@ export const TRACKING_WHITESPACE_HIDE = 'i_code_review_diff_hide_whitespace'; export const TRACKING_CLICK_SINGLE_FILE_SETTING = 'i_code_review_click_single_file_mode_setting'; export const TRACKING_SINGLE_FILE_MODE = 'i_code_review_diff_single_file'; export const TRACKING_MULTIPLE_FILES_MODE = 'i_code_review_diff_multiple_files'; + +// UI +export const ZERO_CHANGES_ALT_DISPLAY = '-'; diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index 0f44eb06cb3..e233a0cef0a 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -1,6 +1,12 @@ import { __, s__ } from '~/locale'; export const GENERIC_ERROR = __('Something went wrong on our end. Please try again!'); +export const LOAD_SINGLE_DIFF_FAILED = s__( + "MergeRequest|Can't fetch the diff needed to update this view. Please reload this page.", +); +export const DISCUSSION_SINGLE_DIFF_FAILED = s__( + "MergeRequest|Can't fetch the single file diff for the discussion. Please reload this page.", +); export const DIFF_FILE_HEADER = { optionsDropdownTitle: __('Options'), diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 00a08434dac..ad7182024da 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -27,7 +27,7 @@ export default function initDiffsApp(store = notesStore) { store, apolloProvider, provide: { - newSavedRepliesPath: dataset.savedRepliesNewPath, + newCommentTemplatePath: dataset.newCommentTemplatePath, }, data() { return { diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 9236e14beb1..a70c907314b 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -16,6 +16,7 @@ import { __, s__ } from '~/locale'; import notesEventHub from '~/notes/event_hub'; import { generateTreeList } from '~/diffs/utils/tree_worker_utils'; import { sortTree } from '~/ide/stores/utils'; +import { containsSensitiveToken, confirmSensitiveAction } from '~/lib/utils/secret_detection'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE, @@ -49,6 +50,7 @@ import { TRACKING_SINGLE_FILE_MODE, TRACKING_MULTIPLE_FILES_MODE, } from '../constants'; +import { DISCUSSION_SINGLE_DIFF_FAILED, LOAD_SINGLE_DIFF_FAILED } from '../i18n'; import eventHub from '../event_hub'; import { isCollapsed } from '../utils/diff_file'; import { markFileReview, setReviewsForMergeRequest } from '../utils/file_reviews'; @@ -62,6 +64,8 @@ import { idleCallback, allDiscussionWrappersExpanded, prepareLineForRenamedFile, + parseUrlHashAsFileHash, + isUrlHashNoteLink, } from './utils'; export const setBaseConfig = ({ commit }, options) => { @@ -101,6 +105,47 @@ export const setBaseConfig = ({ commit }, options) => { }); }; +export const fetchFileByFile = async ({ state, getters, commit }) => { + const isNoteLink = isUrlHashNoteLink(window?.location?.hash); + const id = parseUrlHashAsFileHash(window?.location?.hash, state.currentDiffFileId); + const treeEntry = id + ? getters.flatBlobsList.find(({ fileHash }) => fileHash === id) + : getters.flatBlobsList[0]; + + if (treeEntry && !treeEntry.diffLoaded && !getters.getDiffFileByHash(id)) { + // Overloading "batch" loading indicators so the UI stays mostly the same + commit(types.SET_BATCH_LOADING_STATE, 'loading'); + commit(types.SET_RETRIEVING_BATCHES, true); + + const urlParams = { + old_path: treeEntry.filePaths.old, + new_path: treeEntry.filePaths.new, + w: state.showWhitespace ? '0' : '1', + view: 'inline', + }; + + axios + .get(mergeUrlParams({ ...urlParams }, state.endpointDiffForPath)) + .then(({ data: diffData }) => { + commit(types.SET_DIFF_DATA_BATCH, { diff_files: diffData.diff_files }); + + if (!isNoteLink && !state.currentDiffFileId) { + commit(types.SET_CURRENT_DIFF_FILE, state.diffFiles[0]?.file_hash || ''); + } + + commit(types.SET_BATCH_LOADING_STATE, 'loaded'); + + eventHub.$emit('diffFilesModified'); + }) + .catch(() => { + commit(types.SET_BATCH_LOADING_STATE, 'error'); + }) + .finally(() => { + commit(types.SET_RETRIEVING_BATCHES, false); + }); + } +}; + export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { let perPage = state.viewDiffsFileByFile ? 1 : 5; let increaseAmount = 1.4; @@ -512,13 +557,20 @@ export const toggleFileDiscussionWrappers = ({ commit }, diff) => { } }; -export const saveDiffDiscussion = ({ state, dispatch }, { note, formData }) => { +export const saveDiffDiscussion = async ({ state, dispatch }, { note, formData }) => { const postData = getNoteFormData({ commit: state.commit, note, ...formData, }); + if (containsSensitiveToken(note)) { + const confirmed = await confirmSensitiveAction(); + if (!confirmed) { + return null; + } + } + return dispatch('saveNote', postData, { root: true }) .then((result) => dispatch('updateDiscussion', result.discussion, { root: true })) .then((discussion) => dispatch('assignDiscussionsToDiff', [discussion])) @@ -539,6 +591,31 @@ export const setCurrentFileHash = ({ commit }, hash) => { commit(types.SET_CURRENT_DIFF_FILE, hash); }; +export const goToFile = ({ state, commit, dispatch, getters }, { path, singleFile }) => { + if (!state.viewDiffsFileByFile || !singleFile) { + dispatch('scrollToFile', { path }); + } else { + if (!state.treeEntries[path]) return; + + const { fileHash } = state.treeEntries[path]; + + commit(types.SET_CURRENT_DIFF_FILE, fileHash); + document.location.hash = fileHash; + + if (!getters.isTreePathLoaded(path)) { + dispatch('fetchFileByFile') + .then(() => { + dispatch('scrollToFile', { path }); + }) + .catch(() => { + createAlert({ + message: LOAD_SINGLE_DIFF_FAILED, + }); + }); + } + } +}; + export const scrollToFile = ({ state, commit, getters }, { path }) => { if (!state.treeEntries[path]) return; @@ -779,13 +856,11 @@ export const setSuggestPopoverDismissed = ({ commit, state }) => }); 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`')); + return Promise.reject(new Error('`state` must already contain a valid `commit`')); // eslint-disable-line @gitlab/require-i18n-strings } - /* 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'); @@ -821,6 +896,24 @@ export function moveToNeighboringCommit({ dispatch, state }, { direction }) { } } +export const rereadNoteHash = ({ state, dispatch }) => { + const urlHash = window?.location?.hash; + + if (isUrlHashNoteLink(urlHash)) { + dispatch('setCurrentDiffFileIdFromNote', urlHash.split('_').pop()) + .then(() => { + if (state.viewDiffsFileByFile) { + dispatch('fetchFileByFile'); + } + }) + .catch(() => { + createAlert({ + message: DISCUSSION_SINGLE_DIFF_FAILED, + }); + }); + } +}; + export const setCurrentDiffFileIdFromNote = ({ commit, getters, rootGetters }, noteId) => { const note = rootGetters.notesById[noteId]; @@ -833,11 +926,18 @@ export const setCurrentDiffFileIdFromNote = ({ commit, getters, rootGetters }, n } }; -export const navigateToDiffFileIndex = ({ commit, getters }, index) => { +export const navigateToDiffFileIndex = ( + { state, getters, commit, dispatch }, + { index, singleFile }, +) => { const { fileHash } = getters.flatBlobsList[index]; document.location.hash = fileHash; commit(types.SET_CURRENT_DIFF_FILE, fileHash); + + if (state.viewDiffsFileByFile && singleFile) { + dispatch('fetchFileByFile'); + } }; export const setFileByFile = ({ state, commit }, { fileByFile }) => { diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 3739ef0cd55..4ca353333b7 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -559,19 +559,19 @@ export const allDiscussionWrappersExpanded = (diff) => { return discussionsExpanded; }; -export function isUrlHashNoteLink(urlHash) { +export function isUrlHashNoteLink(urlHash = '') { const id = urlHash.replace(/^#/, ''); return id.startsWith('note'); } -export function isUrlHashFileHeader(urlHash) { +export function isUrlHashFileHeader(urlHash = '') { const id = urlHash.replace(/^#/, ''); return id.startsWith('diff-content'); } -export function parseUrlHashAsFileHash(urlHash, currentDiffFileId = '') { +export function parseUrlHashAsFileHash(urlHash = '', currentDiffFileId = '') { const isNoteLink = isUrlHashNoteLink(urlHash); let id = urlHash.replace(/^#/, ''); diff --git a/app/assets/javascripts/diffs/utils/merge_request.js b/app/assets/javascripts/diffs/utils/merge_request.js index 43e04a814c5..6847b8900d2 100644 --- a/app/assets/javascripts/diffs/utils/merge_request.js +++ b/app/assets/javascripts/diffs/utils/merge_request.js @@ -1,3 +1,5 @@ +import { ZERO_CHANGES_ALT_DISPLAY } from '../constants'; + const endpointRE = /^(\/?(.+?)\/(.+?)\/-\/merge_requests\/(\d+)).*$/i; function getVersionInfo({ endpoint } = {}) { @@ -13,6 +15,17 @@ function getVersionInfo({ endpoint } = {}) { }; } +export function updateChangesTabCount({ + count, + badge = document.querySelector('.js-diffs-tab .gl-badge'), +} = {}) { + if (badge) { + // The purpose of this function is to assign to this parameter + /* eslint-disable-next-line no-param-reassign */ + badge.textContent = count || ZERO_CHANGES_ALT_DISPLAY; + } +} + export function getDerivedMergeRequestInformation({ endpoint } = {}) { let mrPath; let userOrGroup; |