diff options
Diffstat (limited to 'app/assets/javascripts/diffs')
| -rw-r--r-- | app/assets/javascripts/diffs/components/app.vue | 32 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/components/diff_file.vue | 20 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/components/diff_file_header.vue | 13 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/components/merge_conflict_warning.vue | 62 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/components/shared/findings_drawer.vue | 103 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/constants.js | 3 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/i18n.js | 3 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/index.js | 4 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/store/actions.js | 31 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/store/mutation_types.js | 1 | ||||
| -rw-r--r-- | app/assets/javascripts/diffs/store/mutations.js | 5 |
11 files changed, 151 insertions, 126 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 54c276c36b1..00fd9f43a4f 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -115,6 +115,11 @@ export default { required: false, default: false, }, + codequalityReportAvailable: { + type: Boolean, + required: false, + default: false, + }, endpointCodequality: { type: String, required: false, @@ -147,6 +152,7 @@ export default { subscribedToVirtualScrollingEvents: false, autoScrolled: false, activeProject: undefined, + hasScannerError: false, }; }, apollo: { @@ -157,26 +163,31 @@ export default { return { fullPath: this.projectPath, iid: this.iid }; }, skip() { - const codeQualityBoolean = Boolean(this.endpointCodequality); + if (this.hasScannerError) { + return true; + } - return !this.sastReportsInInlineDiff || (!codeQualityBoolean && !this.sastReportAvailable); + return ( + !this.sastReportsInInlineDiff || + (!this.codequalityReportAvailable && !this.sastReportAvailable) + ); }, update(data) { - const codeQualityBoolean = Boolean(this.endpointCodequality); const { codequalityReportsComparer, sastReport } = data?.project?.mergeRequest || {}; this.activeProject = data?.project?.mergeRequest?.project; if ( (sastReport?.status === FINDINGS_STATUS_PARSED || !this.sastReportAvailable) && - (!codeQualityBoolean || codequalityReportsComparer.status === FINDINGS_STATUS_PARSED) + (!this.codequalityReportAvailable || + codequalityReportsComparer.status === FINDINGS_STATUS_PARSED) ) { - this.getMRCodequalityAndSecurityReportStopPolling( - this.$apollo.queries.getMRCodequalityAndSecurityReports, - ); + this.$apollo.queries.getMRCodequalityAndSecurityReports.stopPolling(); } if (sastReport?.status === FINDINGS_STATUS_ERROR && this.sastReportAvailable) { this.fetchScannerFindingsError(); + + this.$apollo.queries.getMRCodequalityAndSecurityReports.stopPolling(); } if (codequalityReportsComparer?.report?.newErrors) { @@ -192,6 +203,7 @@ export default { }, error() { this.fetchScannerFindingsError(); + this.$apollo.queries.getMRCodequalityAndSecurityReports.stopPolling(); }, }, }, @@ -432,8 +444,9 @@ export default { this.setDrawer({}); }, fetchScannerFindingsError() { + this.hasScannerError = true; createAlert({ - message: __('Something went wrong fetching the Scanner Findings. Please try again.'), + message: __('Something went wrong fetching the scanner findings. Please try again.'), }); }, subscribeToEvents() { @@ -445,9 +458,6 @@ export default { diffsEventHub.$on(EVT_MR_PREPARED, this.fetchData); diffsEventHub.$on(EVT_DISCUSSIONS_ASSIGNED, this.handleHash); }, - getMRCodequalityAndSecurityReportStopPolling(query) { - query.stopPolling(); - }, unsubscribeFromEvents() { diffsEventHub.$off(EVT_DISCUSSIONS_ASSIGNED, this.handleHash); diffsEventHub.$off(EVT_MR_PREPARED, this.fetchData); diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 8c1cab20ece..82b721da493 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -9,7 +9,7 @@ import DiffContent from 'jh_else_ce/diffs/components/diff_content.vue'; import { createAlert } from '~/alert'; import { hasDiff } from '~/helpers/diffs_helper'; import { diffViewerErrors } from '~/ide/constants'; -import { scrollToElement } from '~/lib/utils/common_utils'; +import { scrollToElement, isElementStuck } from '~/lib/utils/common_utils'; import { sprintf } from '~/locale'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import notesEventHub from '~/notes/event_hub'; @@ -170,6 +170,11 @@ export default { showWarning() { return this.isCollapsed && this.automaticallyCollapsed && !this.viewDiffsFileByFile; }, + expandableWarning() { + return this.file.viewer?.generated + ? this.$options.i18n.autoCollapsedGenerated + : this.$options.i18n.autoCollapsed; + }, showContent() { return !this.isCollapsed && !this.isFileTooLarge; }, @@ -295,8 +300,13 @@ export default { collapsed: collapsingNow, }); - if (collapsingNow && viaUserInteraction && contentElement) { - scrollToElement(contentElement, { duration: 1 }); + if ( + collapsingNow && + viaUserInteraction && + contentElement && + isElementStuck(this.$refs.header.$el) + ) { + scrollToElement(contentElement, { duration: 0 }); } if (!this.hasDiff && !collapsingNow) { @@ -381,6 +391,7 @@ export default { class="diff-file file-holder gl-border-none gl-mb-0! gl-pb-5" > <diff-file-header + ref="header" :can-current-user-fork="canCurrentUserFork" :diff-file="file" :collapsible="true" @@ -419,6 +430,7 @@ export default { <div :id="`diff-content-${file.file_hash}`" :class="hasBodyClasses.contentByHash" + class="diff-content" data-testid="content-area" > <gl-alert @@ -523,7 +535,7 @@ export default { class="collapsed-file-warning gl-p-7 gl-bg-orange-50 gl-text-center gl-rounded-bottom-left-base gl-rounded-bottom-right-base" > <p class="gl-mb-5"> - {{ $options.i18n.autoCollapsed }} + {{ expandableWarning }} </p> <gl-button data-testid="expand-button" @click.prevent="handleToggle"> {{ $options.i18n.expand }} diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index 20f82500a02..e45fd508a5b 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -211,19 +211,6 @@ export default { return this.getNoteableData.current_user.can_create_note; }, }, - watch: { - 'idState.moreActionsShown': { - handler(val) { - const el = this.$el.closest('.vue-recycle-scroller__item-view'); - - if (el) { - // We can't add a style with Vue because of the way the virtual - // scroller library renders the diff files - el.style.zIndex = val ? '1' : null; - } - }, - }, - }, methods: { ...mapActions('diffs', [ 'toggleFileDiscussions', diff --git a/app/assets/javascripts/diffs/components/merge_conflict_warning.vue b/app/assets/javascripts/diffs/components/merge_conflict_warning.vue deleted file mode 100644 index 6cb1ed4cbcf..00000000000 --- a/app/assets/javascripts/diffs/components/merge_conflict_warning.vue +++ /dev/null @@ -1,62 +0,0 @@ -<script> -import { GlButton, GlAlert, GlModalDirective } from '@gitlab/ui'; - -export default { - components: { - GlAlert, - GlButton, - }, - directives: { - GlModalDirective, - }, - props: { - mergeable: { - type: Boolean, - required: true, - }, - resolutionPath: { - type: String, - required: true, - }, - }, -}; -</script> - -<template> - <div> - <gl-alert - :dismissible="false" - :title="__('There are merge conflicts')" - variant="warning" - class="gl-mb-5" - > - <p class="gl-mb-2"> - {{ __('The comparison view may be inaccurate due to merge conflicts.') }} - </p> - <p class="gl-mb-0"> - {{ - __( - 'Resolve these conflicts, or ask someone with write access to this repository to resolve them locally.', - ) - }} - </p> - <template #actions> - <gl-button - v-if="resolutionPath" - :href="resolutionPath" - variant="confirm" - class="gl-mr-3 gl-alert-action" - > - {{ __('Resolve conflicts') }} - </gl-button> - <gl-button - v-if="mergeable" - v-gl-modal-directive="'modal-merge-info'" - class="gl-alert-action" - > - {{ __('Resolve locally') }} - </gl-button> - </template> - </gl-alert> - </div> -</template> diff --git a/app/assets/javascripts/diffs/components/shared/findings_drawer.vue b/app/assets/javascripts/diffs/components/shared/findings_drawer.vue index 2c1a8305935..854f6910fa1 100644 --- a/app/assets/javascripts/diffs/components/shared/findings_drawer.vue +++ b/app/assets/javascripts/diffs/components/shared/findings_drawer.vue @@ -1,9 +1,10 @@ <script> -import { GlBadge, GlDrawer, GlIcon, GlLink } from '@gitlab/ui'; +import { GlBadge, GlDrawer, GlLink, GlButton, GlIcon } from '@gitlab/ui'; import { __, s__ } from '~/locale'; import { DRAWER_Z_INDEX } from '~/lib/utils/constants'; import { getSeverity } from '~/ci/reports/utils'; import { getContentWrapperHeight } from '~/lib/utils/dom_utils'; +import { SAST_FINDING_DISMISSED } from '../../constants'; import DrawerItem from './findings_drawer_item.vue'; export const i18n = { @@ -26,7 +27,7 @@ export const codeQuality = 'codeQuality'; export default { i18n, codeQuality, - components: { GlBadge, GlDrawer, GlIcon, GlLink, DrawerItem }, + components: { GlBadge, GlDrawer, GlLink, GlButton, GlIcon, DrawerItem }, props: { drawer: { type: Object, @@ -38,19 +39,50 @@ export default { default: () => {}, }, }, + data() { + return { + activeIndex: 0, + }; + }, computed: { getDrawerHeaderHeight() { return getContentWrapperHeight(); }, isCodeQuality() { - return this.drawer.scale === this.$options.codeQuality; + return this.activeElement.scale === this.$options.codeQuality; + }, + activeElement() { + return this.drawer.findings[this.activeIndex]; + }, + findingsStatus() { + return this.activeElement.state === SAST_FINDING_DISMISSED ? 'muted' : 'warning'; }, }, DRAWER_Z_INDEX, + watch: { + drawer(newVal) { + this.activeIndex = newVal.index; + }, + }, methods: { getSeverity, + prev() { + if (this.activeIndex === 0) { + this.activeIndex = this.drawer.findings.length - 1; + } else { + this.activeIndex -= 1; + } + }, + next() { + if (this.activeIndex === this.drawer.findings.length - 1) { + this.activeIndex = 0; + } else { + this.activeIndex += 1; + } + }, + concatIdentifierName(name, index) { - return name + (index !== this.drawer.identifiers.length - 1 ? ', ' : ''); + return name + (index !== this.activeElement.identifiers.length - 1 ? ', ' : ''); }, }, }; @@ -64,36 +96,51 @@ export default { @close="$emit('close')" > <template #title> - <h2 class="drawer-heading gl-font-base gl-mt-0 gl-mb-0"> + <h2 class="drawer-heading gl-font-base gl-mt-0 gl-mb-0 gl-w-28"> <gl-icon :size="12" - :name="getSeverity(drawer).name" - :class="getSeverity(drawer).class" + :name="getSeverity(activeElement).name" + :class="getSeverity(activeElement).class" class="inline-findings-severity-icon gl-vertical-align-baseline!" /> - <span class="drawer-heading-severity">{{ drawer.severity }}</span> + <span class="drawer-heading-severity">{{ activeElement.severity }}</span> {{ isCodeQuality ? $options.i18n.codeQualityFinding : $options.i18n.sastFinding }} </h2> + <div v-if="drawer.findings.length > 1"> + <gl-button data-testid="findings-drawer-prev-button" class="gl-p-1!" @click="prev"> + <gl-icon :size="24" name="chevron-left" /> + </gl-button> + <gl-button class="gl-p-1!" @click="next"> + <gl-icon data-testid="findings-drawer-next-button" :size="24" name="chevron-right" /> + </gl-button> + </div> </template> <template #default> <ul class="gl-list-style-none gl-border-b-initial gl-mb-0 gl-pb-0!"> - <drawer-item v-if="drawer.title" :description="$options.i18n.name" :value="drawer.title" /> + <drawer-item + v-if="activeElement.title" + :description="$options.i18n.name" + :value="activeElement.title" + data-testid="findings-drawer-title" + /> - <drawer-item v-if="drawer.state" :description="$options.i18n.status"> + <drawer-item v-if="activeElement.state" :description="$options.i18n.status"> <template #value> - <gl-badge variant="warning" class="text-capitalize">{{ drawer.state }}</gl-badge> + <gl-badge :variant="findingsStatus" class="text-capitalize">{{ + activeElement.state + }}</gl-badge> </template> </drawer-item> <drawer-item - v-if="drawer.description" + v-if="activeElement.description" :description="$options.i18n.description" - :value="drawer.description" + :value="activeElement.description" /> <drawer-item - v-if="project && drawer.scale !== $options.codeQuality" + v-if="project && activeElement.scale !== $options.codeQuality" :description="$options.i18n.project" > <template #value> @@ -101,23 +148,31 @@ export default { </template> </drawer-item> - <drawer-item v-if="drawer.location || drawer.webUrl" :description="$options.i18n.file"> + <drawer-item + v-if="activeElement.location || activeElement.webUrl" + :description="$options.i18n.file" + > <template #value> - <span v-if="drawer.webUrl && drawer.filePath && drawer.line"> - <gl-link :href="drawer.webUrl">{{ drawer.filePath }}:{{ drawer.line }}</gl-link> + <span v-if="activeElement.webUrl && activeElement.filePath && activeElement.line"> + <gl-link :href="activeElement.webUrl" + >{{ activeElement.filePath }}:{{ activeElement.line }}</gl-link + > </span> - <span v-else-if="drawer.location"> - {{ drawer.location.file }}:{{ drawer.location.startLine }} + <span v-else-if="activeElement.location"> + {{ activeElement.location.file }}:{{ activeElement.location.startLine }} </span> </template> </drawer-item> <drawer-item - v-if="drawer.identifiers && drawer.identifiers.length" + v-if="activeElement.identifiers && activeElement.identifiers.length" :description="$options.i18n.identifiers" > <template #value> - <span v-for="(identifier, index) in drawer.identifiers" :key="identifier.externalId"> + <span + v-for="(identifier, index) in activeElement.identifiers" + :key="identifier.externalId" + > <gl-link v-if="identifier.url" :href="identifier.url"> {{ concatIdentifierName(identifier.name, index) }} </gl-link> @@ -129,15 +184,15 @@ export default { </drawer-item> <drawer-item - v-if="drawer.scale" + v-if="activeElement.scale" :description="$options.i18n.tool" :value="isCodeQuality ? $options.i18n.codeQuality : $options.i18n.sast" /> <drawer-item - v-if="drawer.engineName" + v-if="activeElement.engineName" :description="$options.i18n.engine" - :value="drawer.engineName" + :value="activeElement.engineName" /> </ul> </template> diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index e48eb10753c..351df1d1996 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -118,3 +118,6 @@ export const TRACKING_MULTIPLE_FILES_MODE = 'i_code_review_diff_multiple_files'; // UI export const ZERO_CHANGES_ALT_DISPLAY = '-'; + +// SAST Findings +export const SAST_FINDING_DISMISSED = 'dismissed'; diff --git a/app/assets/javascripts/diffs/i18n.js b/app/assets/javascripts/diffs/i18n.js index 15e3893d22a..651cae11c47 100644 --- a/app/assets/javascripts/diffs/i18n.js +++ b/app/assets/javascripts/diffs/i18n.js @@ -22,6 +22,9 @@ export const DIFF_FILE = { fork: __('Fork'), cancel: __('Cancel'), autoCollapsed: __('Files with large changes are collapsed by default.'), + autoCollapsedGenerated: __( + 'Generated files are collapsed by default. This behavior can be overriden via .gitattributes file if required.', + ), expand: __('Expand file'), }; export const START_THREAD = __('Start another thread'); diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 034dd4cf6d2..15e4225f062 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -36,7 +36,8 @@ export default function initDiffsApp(store = notesStore) { iid: dataset.iid || '', endpointCoverage: dataset.endpointCoverage || '', endpointCodequality: dataset.endpointCodequality || '', - sastReportAvailable: dataset.endpointSast, + codequalityReportAvailable: parseBoolean(dataset.codequalityReportAvailable), + sastReportAvailable: parseBoolean(dataset.sastReportAvailable), helpPagePath: dataset.helpPagePath, currentUser: JSON.parse(dataset.currentUserData) || {}, changesEmptyStateIllustration: dataset.changesEmptyStateIllustration, @@ -86,6 +87,7 @@ export default function initDiffsApp(store = notesStore) { iid: this.iid, endpointCoverage: this.endpointCoverage, endpointCodequality: this.endpointCodequality, + codequalityReportAvailable: this.codequalityReportAvailable, sastReportAvailable: this.sastReportAvailable, currentUser: this.currentUser, helpPagePath: this.helpPagePath, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index fcaf8e99b2d..1c0e20183e2 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -15,6 +15,7 @@ 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 { isCollapsed } from '~/diffs/utils/diff_file'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE, @@ -73,6 +74,7 @@ import { prepareLineForRenamedFile, parseUrlHashAsFileHash, isUrlHashNoteLink, + findDiffFile, } from './utils'; export const setBaseConfig = ({ commit }, options) => { @@ -658,18 +660,18 @@ export const goToFile = ({ state, commit, dispatch, getters }, { path }) => { const { fileHash } = state.treeEntries[path]; commit(types.SET_CURRENT_DIFF_FILE, fileHash); - document.location.hash = fileHash; + + const newUrl = new URL(window.location); + newUrl.hash = fileHash; + historyPushState(newUrl, { skipScrolling: true }); + scrollToElement('.diff-files-holder', { duration: 0 }); if (!getters.isTreePathLoaded(path)) { - dispatch('fetchFileByFile') - .then(() => { - dispatch('scrollToFile', { path }); - }) - .catch(() => { - createAlert({ - message: LOAD_SINGLE_DIFF_FAILED, - }); + dispatch('fetchFileByFile').catch(() => { + createAlert({ + message: LOAD_SINGLE_DIFF_FAILED, }); + }); } } }; @@ -1041,8 +1043,15 @@ export function reviewFile({ commit, state }, { file, reviewed = true }) { export const disableVirtualScroller = ({ commit }) => commit(types.DISABLE_VIRTUAL_SCROLLING); -export const toggleFileCommentForm = ({ commit }, filePath) => - commit(types.TOGGLE_FILE_COMMENT_FORM, filePath); +export const toggleFileCommentForm = ({ state, commit }, filePath) => { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + if (isCollapsed(file)) { + commit(types.SET_FILE_COMMENT_FORM, { filePath, expanded: true }); + } else { + commit(types.TOGGLE_FILE_COMMENT_FORM, filePath); + } + commit(types.SET_FILE_COLLAPSED, { filePath, collapsed: false }); +}; export const addDraftToFile = ({ commit }, { filePath, draft }) => commit(types.ADD_DRAFT_TO_FILE, { filePath, draft }); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index c2177bacbcc..b155804c70c 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -53,4 +53,5 @@ export const TOGGLE_LINE_DISCUSSIONS = 'TOGGLE_LINE_DISCUSSIONS'; export const DISABLE_VIRTUAL_SCROLLING = 'DISABLE_VIRTUAL_SCROLLING'; export const TOGGLE_FILE_COMMENT_FORM = 'TOGGLE_FILE_COMMENT_FORM'; +export const SET_FILE_COMMENT_FORM = 'SET_FILE_COMMENT_FORM'; export const ADD_DRAFT_TO_FILE = 'ADD_DRAFT_TO_FILE'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 08c195469e3..bc5ed3c40df 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -394,6 +394,11 @@ export default { file.hasCommentForm = !file.hasCommentForm; }, + [types.SET_FILE_COMMENT_FORM](state, { filePath, expanded }) { + const file = findDiffFile(state.diffFiles, filePath, 'file_path'); + + file.hasCommentForm = expanded; + }, [types.ADD_DRAFT_TO_FILE](state, { filePath, draft }) { const file = findDiffFile(state.diffFiles, filePath, 'file_path'); |