diff options
Diffstat (limited to 'app')
21 files changed, 440 insertions, 247 deletions
diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index c07850b1a4f..c6d32ffef34 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -6,6 +6,7 @@ import { __ } from '~/locale'; import createFlash from '~/flash'; import PanelResizer from '~/vue_shared/components/panel_resizer.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import { isSingleViewStyle } from '~/helpers/diffs_helper'; import eventHub from '../../notes/event_hub'; import CompareVersions from './compare_versions.vue'; import DiffFile from './diff_file.vue'; @@ -145,6 +146,9 @@ export default { }, watch: { diffViewType() { + if (this.needsReload() || this.needsFirstLoad()) { + this.refetchDiffData(); + } this.adjustView(); }, shouldShow() { @@ -224,6 +228,16 @@ export default { { timeout: 1000 }, ); }, + needsReload() { + return ( + this.glFeatures.singleMrDiffView && + this.diffFiles.length && + isSingleViewStyle(this.diffFiles[0]) + ); + }, + needsFirstLoad() { + return this.glFeatures.singleMrDiffView && !this.diffFiles.length; + }, fetchData(toggleTree = true) { if (this.glFeatures.diffsBatchLoad) { this.fetchDiffFilesMeta() @@ -237,6 +251,13 @@ export default { }); this.fetchDiffFilesBatch() + .then(() => { + // 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(); + }) .then(() => this.startDiffRendering()) .catch(() => { createFlash(__('Something went wrong on our end. Please try again!')); @@ -250,6 +271,7 @@ export default { requestIdleCallback( () => { + this.setDiscussions(); this.startRenderDiffsQueue(); }, { timeout: 1000 }, diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 0dbff4ffcec..f5051748f10 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -4,6 +4,7 @@ import _ from 'underscore'; import { GlLoadingIcon } from '@gitlab/ui'; import { __, sprintf } from '~/locale'; import createFlash from '~/flash'; +import { hasDiff } from '~/helpers/diffs_helper'; import eventHub from '../../notes/event_hub'; import DiffFileHeader from './diff_file_header.vue'; import DiffContent from './diff_content.vue'; @@ -55,12 +56,7 @@ export default { return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed); }, hasDiff() { - return ( - (this.file.highlighted_diff_lines && - this.file.parallel_diff_lines && - this.file.parallel_diff_lines.length > 0) || - !this.file.blob.readable_text - ); + return hasDiff(this.file); }, isFileTooLarge() { return this.file.viewer.error === diffViewerErrors.too_large; diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 6f2467ddf71..6714f4e62b8 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -65,6 +65,10 @@ export const fetchDiffFiles = ({ state, commit }) => { w: state.showWhitespace ? '0' : '1', }; + if (state.useSingleDiffStyle) { + urlParams.view = state.diffViewType; + } + commit(types.SET_LOADING, true); worker.addEventListener('message', ({ data }) => { @@ -90,13 +94,22 @@ export const fetchDiffFiles = ({ state, commit }) => { }; export const fetchDiffFilesBatch = ({ commit, state }) => { + const urlParams = { + per_page: DIFFS_PER_PAGE, + w: state.showWhitespace ? '0' : '1', + }; + + if (state.useSingleDiffStyle) { + urlParams.view = state.diffViewType; + } + commit(types.SET_BATCH_LOADING, true); commit(types.SET_RETRIEVING_BATCHES, true); const getBatch = page => axios .get(state.endpointBatch, { - params: { page, per_page: DIFFS_PER_PAGE, w: state.showWhitespace ? '0' : '1' }, + params: { ...urlParams, page }, }) .then(({ data: { pagination, diff_files } }) => { commit(types.SET_DIFF_DATA_BATCH, { diff_files }); @@ -150,7 +163,10 @@ export const assignDiscussionsToDiff = ( { commit, state, rootState }, discussions = rootState.notes.discussions, ) => { - const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles); + const diffPositionByLineCode = getDiffPositionByLineCode( + state.diffFiles, + state.useSingleDiffStyle, + ); const hash = getLocationHash(); discussions @@ -339,24 +355,23 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => { export const toggleFileDiscussionWrappers = ({ commit }, diff) => { const discussionWrappersExpanded = allDiscussionWrappersExpanded(diff); - let linesWithDiscussions; - if (diff.highlighted_diff_lines) { - linesWithDiscussions = diff.highlighted_diff_lines.filter(line => line.discussions.length); - } - if (diff.parallel_diff_lines) { - linesWithDiscussions = diff.parallel_diff_lines.filter( - line => - (line.left && line.left.discussions.length) || - (line.right && line.right.discussions.length), - ); - } - - if (linesWithDiscussions.length) { - linesWithDiscussions.forEach(line => { + const lineCodesWithDiscussions = new Set(); + const { parallel_diff_lines: parallelLines, highlighted_diff_lines: inlineLines } = diff; + const allLines = inlineLines.concat( + parallelLines.map(line => line.left), + parallelLines.map(line => line.right), + ); + const lineHasDiscussion = line => Boolean(line?.discussions.length); + const registerDiscussionLine = line => lineCodesWithDiscussions.add(line.line_code); + + allLines.filter(lineHasDiscussion).forEach(registerDiscussionLine); + + if (lineCodesWithDiscussions.size) { + Array.from(lineCodesWithDiscussions).forEach(lineCode => { commit(types.TOGGLE_LINE_DISCUSSIONS, { fileHash: diff.file_hash, - lineCode: line.line_code, expanded: !discussionWrappersExpanded, + lineCode, }); }); } diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index a4986e26966..8cfdded1f9b 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -45,26 +45,28 @@ export default { }, [types.SET_DIFF_DATA](state, data) { + let files = state.diffFiles; + if ( - !( - gon && - gon.features && - gon.features.diffsBatchLoad && - window.location.search.indexOf('diff_id') === -1 - ) + !(gon?.features?.diffsBatchLoad && window.location.search.indexOf('diff_id') === -1) && + data.diff_files ) { - prepareDiffData(data); + files = prepareDiffData(data, files); } Object.assign(state, { ...convertObjectPropsToCamelCase(data), + diffFiles: files, }); }, [types.SET_DIFF_DATA_BATCH](state, data) { - prepareDiffData(data); + const files = prepareDiffData(data, state.diffFiles); - state.diffFiles.push(...data.diff_files); + Object.assign(state, { + ...convertObjectPropsToCamelCase(data), + diffFiles: files, + }); }, [types.RENDER_FILE](state, file) { @@ -88,11 +90,11 @@ export default { if (!diffFile) return; - if (diffFile.highlighted_diff_lines) { + if (diffFile.highlighted_diff_lines.length) { diffFile.highlighted_diff_lines.find(l => l.line_code === lineCode).hasForm = hasForm; } - if (diffFile.parallel_diff_lines) { + if (diffFile.parallel_diff_lines.length) { const line = diffFile.parallel_diff_lines.find(l => { const { left, right } = l; @@ -153,13 +155,13 @@ export default { }, [types.EXPAND_ALL_FILES](state) { - state.diffFiles = state.diffFiles.map(file => ({ - ...file, - viewer: { - ...file.viewer, - collapsed: false, - }, - })); + state.diffFiles.forEach(file => { + Object.assign(file, { + viewer: Object.assign(file.viewer, { + collapsed: false, + }), + }); + }); }, [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, hash }) { @@ -197,29 +199,29 @@ export default { }; }; - state.diffFiles = state.diffFiles.map(diffFile => { - if (diffFile.file_hash === fileHash) { - const file = { ...diffFile }; - - if (file.highlighted_diff_lines) { - file.highlighted_diff_lines = file.highlighted_diff_lines.map(line => - setDiscussionsExpanded(lineCheck(line) ? mapDiscussions(line) : line), - ); + state.diffFiles.forEach(file => { + if (file.file_hash === fileHash) { + if (file.highlighted_diff_lines.length) { + file.highlighted_diff_lines.forEach(line => { + Object.assign( + line, + setDiscussionsExpanded(lineCheck(line) ? mapDiscussions(line) : line), + ); + }); } - if (file.parallel_diff_lines) { - file.parallel_diff_lines = file.parallel_diff_lines.map(line => { + if (file.parallel_diff_lines.length) { + file.parallel_diff_lines.forEach(line => { const left = line.left && lineCheck(line.left); const right = line.right && lineCheck(line.right); if (left || right) { - return { - ...line, + Object.assign(line, { left: line.left ? setDiscussionsExpanded(mapDiscussions(line.left)) : null, right: line.right ? setDiscussionsExpanded(mapDiscussions(line.right, () => !left)) : null, - }; + }); } return line; @@ -227,15 +229,15 @@ export default { } if (!file.parallel_diff_lines || !file.highlighted_diff_lines) { - file.discussions = (file.discussions || []) + const newDiscussions = (file.discussions || []) .filter(d => d.id !== discussion.id) .concat(discussion); - } - return file; + Object.assign(file, { + discussions: newDiscussions, + }); + } } - - return diffFile; }); }, @@ -259,9 +261,9 @@ export default { [types.TOGGLE_LINE_DISCUSSIONS](state, { fileHash, lineCode, expanded }) { const selectedFile = state.diffFiles.find(f => f.file_hash === fileHash); - updateLineInFile(selectedFile, lineCode, line => - Object.assign(line, { discussionsExpanded: expanded }), - ); + updateLineInFile(selectedFile, lineCode, line => { + Object.assign(line, { discussionsExpanded: expanded }); + }); }, [types.TOGGLE_FOLDER_OPEN](state, path) { diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 281a0de1fc2..b379f1fabef 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -185,6 +185,7 @@ export function addContextLines(options) { * Trims the first char of the `richText` property when it's either a space or a diff symbol. * @param {Object} line * @returns {Object} + * @deprecated */ export function trimFirstCharOfLineContent(line = {}) { // eslint-disable-next-line no-param-reassign @@ -212,79 +213,171 @@ function getLineCode({ left, right }, index) { return index; } -// This prepares and optimizes the incoming diff data from the server -// by setting up incremental rendering and removing unneeded data -export function prepareDiffData(diffData) { - const filesLength = diffData.diff_files.length; - let showingLines = 0; - for (let i = 0; i < filesLength; i += 1) { - const file = diffData.diff_files[i]; - - if (file.parallel_diff_lines) { - const linesLength = file.parallel_diff_lines.length; - for (let u = 0; u < linesLength; u += 1) { - const line = file.parallel_diff_lines[u]; - - line.line_code = getLineCode(line, u); - if (line.left) { - line.left = trimFirstCharOfLineContent(line.left); - line.left.discussions = []; - line.left.hasForm = false; - } - if (line.right) { - line.right = trimFirstCharOfLineContent(line.right); - line.right.discussions = []; - line.right.hasForm = false; - } +function diffFileUniqueId(file) { + return `${file.content_sha}-${file.file_hash}`; +} + +function combineDiffFilesWithPriorFiles(files, prior = []) { + files.forEach(file => { + const id = diffFileUniqueId(file); + const oldMatch = prior.find(oldFile => diffFileUniqueId(oldFile) === id); + + if (oldMatch) { + const missingInline = !file.highlighted_diff_lines; + const missingParallel = !file.parallel_diff_lines; + + if (missingInline) { + Object.assign(file, { + highlighted_diff_lines: oldMatch.highlighted_diff_lines, + }); } - } - if (file.highlighted_diff_lines) { - const linesLength = file.highlighted_diff_lines.length; - for (let u = 0; u < linesLength; u += 1) { - const line = file.highlighted_diff_lines[u]; - Object.assign(line, { - ...trimFirstCharOfLineContent(line), - discussions: [], - hasForm: false, + if (missingParallel) { + Object.assign(file, { + parallel_diff_lines: oldMatch.parallel_diff_lines, }); } - showingLines += file.parallel_diff_lines.length; + } + }); + + return files; +} + +function ensureBasicDiffFileLines(file) { + const missingInline = !file.highlighted_diff_lines; + const missingParallel = !file.parallel_diff_lines; + + Object.assign(file, { + highlighted_diff_lines: missingInline ? [] : file.highlighted_diff_lines, + parallel_diff_lines: missingParallel ? [] : file.parallel_diff_lines, + }); + + return file; +} + +function cleanRichText(text) { + return text ? text.replace(/^[+ -]/, '') : undefined; +} + +function prepareLine(line) { + return Object.assign(line, { + rich_text: cleanRichText(line.rich_text), + discussionsExpanded: true, + discussions: [], + hasForm: false, + text: undefined, + }); +} + +function prepareDiffFileLines(file) { + const inlineLines = file.highlighted_diff_lines; + const parallelLines = file.parallel_diff_lines; + let parallelLinesCount = 0; + + inlineLines.forEach(prepareLine); + + parallelLines.forEach((line, index) => { + Object.assign(line, { line_code: getLineCode(line, index) }); + + if (line.left) { + parallelLinesCount += 1; + prepareLine(line.left); } - const name = (file.viewer && file.viewer.name) || diffViewerModes.text; + if (line.right) { + parallelLinesCount += 1; + prepareLine(line.right); + } Object.assign(file, { - renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, - collapsed: name === diffViewerModes.text && showingLines > MAX_LINES_TO_BE_RENDERED, - isShowingFullFile: false, - isLoadingFullFile: false, - discussions: [], - renderingLines: false, + inlineLinesCount: inlineLines.length, + parallelLinesCount, }); - } + }); + + return file; } -export function getDiffPositionByLineCode(diffFiles) { - return diffFiles.reduce((acc, diffFile) => { - // We can only use highlightedDiffLines to create the map of diff lines because - // highlightedDiffLines will also include every parallel diff line in it. - if (diffFile.highlighted_diff_lines) { +function getVisibleDiffLines(file) { + return Math.max(file.inlineLinesCount, file.parallelLinesCount); +} + +function finalizeDiffFile(file) { + const name = (file.viewer && file.viewer.name) || diffViewerModes.text; + const lines = getVisibleDiffLines(file); + + Object.assign(file, { + renderIt: lines < LINES_TO_BE_RENDERED_DIRECTLY, + collapsed: name === diffViewerModes.text && lines > MAX_LINES_TO_BE_RENDERED, + isShowingFullFile: false, + isLoadingFullFile: false, + discussions: [], + renderingLines: false, + }); + + return file; +} + +export function prepareDiffData(diffData, priorFiles) { + return combineDiffFilesWithPriorFiles(diffData.diff_files, priorFiles) + .map(ensureBasicDiffFileLines) + .map(prepareDiffFileLines) + .map(finalizeDiffFile); +} + +export function getDiffPositionByLineCode(diffFiles, useSingleDiffStyle) { + let lines = []; + const hasInlineDiffs = diffFiles.some(file => file.highlighted_diff_lines.length > 0); + + if (!useSingleDiffStyle || hasInlineDiffs) { + // In either of these cases, we can use `highlighted_diff_lines` because + // that will include all of the parallel diff lines, too + + lines = diffFiles.reduce((acc, diffFile) => { diffFile.highlighted_diff_lines.forEach(line => { - if (line.line_code) { - acc[line.line_code] = { - base_sha: diffFile.diff_refs.base_sha, - head_sha: diffFile.diff_refs.head_sha, - start_sha: diffFile.diff_refs.start_sha, - new_path: diffFile.new_path, - old_path: diffFile.old_path, - old_line: line.old_line, - new_line: line.new_line, - line_code: line.line_code, - position_type: 'text', - }; + acc.push({ file: diffFile, line }); + }); + + return acc; + }, []); + } else { + // If we're in single diff view mode and the inline lines haven't been + // loaded yet, we need to parse the parallel lines + + lines = diffFiles.reduce((acc, diffFile) => { + diffFile.parallel_diff_lines.forEach(pair => { + // It's possible for a parallel line to have an opposite line that doesn't exist + // For example: *deleted* lines will have `null` right lines, while + // *added* lines will have `null` left lines. + // So we have to check each line before we push it onto the array so we're not + // pushing null line diffs + + if (pair.left) { + acc.push({ file: diffFile, line: pair.left }); + } + + if (pair.right) { + acc.push({ file: diffFile, line: pair.right }); } }); + + return acc; + }, []); + } + + return lines.reduce((acc, { file, line }) => { + if (line.line_code) { + acc[line.line_code] = { + base_sha: file.diff_refs.base_sha, + head_sha: file.diff_refs.head_sha, + start_sha: file.diff_refs.start_sha, + new_path: file.new_path, + old_path: file.old_path, + old_line: line.old_line, + new_line: line.new_line, + line_code: line.line_code, + position_type: 'text', + }; } return acc; @@ -462,47 +555,47 @@ export const convertExpandLines = ({ export const idleCallback = cb => requestIdleCallback(cb); -export const updateLineInFile = (selectedFile, lineCode, updateFn) => { - if (selectedFile.parallel_diff_lines) { - const targetLine = selectedFile.parallel_diff_lines.find( - line => - (line.left && line.left.line_code === lineCode) || - (line.right && line.right.line_code === lineCode), - ); - if (targetLine) { - const side = targetLine.left && targetLine.left.line_code === lineCode ? 'left' : 'right'; +function getLinesFromFileByLineCode(file, lineCode) { + const parallelLines = file.parallel_diff_lines; + const inlineLines = file.highlighted_diff_lines; + const matchesCode = line => line.line_code === lineCode; - updateFn(targetLine[side]); - } - } - if (selectedFile.highlighted_diff_lines) { - const targetInlineLine = selectedFile.highlighted_diff_lines.find( - line => line.line_code === lineCode, - ); + return [ + ...parallelLines.reduce((acc, line) => { + if (line.left) { + acc.push(line.left); + } - if (targetInlineLine) { - updateFn(targetInlineLine); - } - } + if (line.right) { + acc.push(line.right); + } + + return acc; + }, []), + ...inlineLines, + ].filter(matchesCode); +} + +export const updateLineInFile = (selectedFile, lineCode, updateFn) => { + getLinesFromFileByLineCode(selectedFile, lineCode).forEach(updateFn); }; export const allDiscussionWrappersExpanded = diff => { - const discussionsExpandedArray = []; - if (diff.parallel_diff_lines) { - diff.parallel_diff_lines.forEach(line => { - if (line.left && line.left.discussions.length) { - discussionsExpandedArray.push(line.left.discussionsExpanded); - } - if (line.right && line.right.discussions.length) { - discussionsExpandedArray.push(line.right.discussionsExpanded); - } - }); - } else if (diff.highlighted_diff_lines) { - diff.highlighted_diff_lines.forEach(line => { - if (line.discussions.length) { - discussionsExpandedArray.push(line.discussionsExpanded); - } - }); - } - return discussionsExpandedArray.every(el => el); + let discussionsExpanded = true; + const changeExpandedResult = line => { + if (line && line.discussions.length) { + discussionsExpanded = discussionsExpanded && line.discussionsExpanded; + } + }; + + diff.parallel_diff_lines.forEach(line => { + changeExpandedResult(line.left); + changeExpandedResult(line.right); + }); + + diff.highlighted_diff_lines.forEach(line => { + changeExpandedResult(line); + }); + + return discussionsExpanded; }; diff --git a/app/assets/javascripts/error_tracking/components/error_details.vue b/app/assets/javascripts/error_tracking/components/error_details.vue index d4562d4a9a5..23a1ec4e367 100644 --- a/app/assets/javascripts/error_tracking/components/error_details.vue +++ b/app/assets/javascripts/error_tracking/components/error_details.vue @@ -1,6 +1,7 @@ <script> import { mapActions, mapGetters, mapState } from 'vuex'; import dateFormat from 'dateformat'; +import createFlash from '~/flash'; import { GlFormInput, GlLink, GlLoadingIcon, GlBadge } from '@gitlab/ui'; import { __, sprintf, n__ } from '~/locale'; import LoadingButton from '~/vue_shared/components/loading_button.vue'; @@ -11,6 +12,8 @@ import TrackEventDirective from '~/vue_shared/directives/track_event'; import timeagoMixin from '~/vue_shared/mixins/timeago'; import { trackClickErrorLinkToSentryOptions } from '../utils'; +import query from '../queries/details.query.graphql'; + export default { components: { LoadingButton, @@ -27,6 +30,14 @@ export default { }, mixins: [timeagoMixin], props: { + issueId: { + type: String, + required: true, + }, + projectPath: { + type: String, + required: true, + }, issueDetailsPath: { type: String, required: true, @@ -44,8 +55,28 @@ export default { required: true, }, }, + apollo: { + GQLerror: { + query, + variables() { + return { + fullPath: this.projectPath, + errorId: `gid://gitlab/Gitlab::ErrorTracking::DetailedError/${this.issueId}`, + }; + }, + pollInterval: 2000, + update: data => data.project.sentryDetailedError, + error: () => createFlash(__('Failed to load error details from Sentry.')), + result(res) { + if (res.data.project?.sentryDetailedError) { + this.$apollo.queries.GQLerror.stopPolling(); + } + }, + }, + }, data() { return { + GQLerror: null, issueCreationInProgress: false, }; }, @@ -56,26 +87,28 @@ export default { return sprintf( __('Reported %{timeAgo} by %{reportedBy}'), { - reportedBy: `<strong>${this.error.culprit}</strong>`, + reportedBy: `<strong>${this.GQLerror.culprit}</strong>`, timeAgo: this.timeFormatted(this.stacktraceData.date_received), }, false, ); }, firstReleaseLink() { - return `${this.error.external_base_url}/releases/${this.error.first_release_short_version}`; + return `${this.error.external_base_url}/releases/${this.GQLerror.firstReleaseShortVersion}`; }, lastReleaseLink() { - return `${this.error.external_base_url}releases/${this.error.last_release_short_version}`; + return `${this.error.external_base_url}releases/${this.GQLerror.lastReleaseShortVersion}`; }, showDetails() { - return Boolean(!this.loading && this.error && this.error.id); + return Boolean( + !this.loading && !this.$apollo.queries.GQLerror.loading && this.error && this.GQLerror, + ); }, showStacktrace() { return Boolean(!this.loadingStacktrace && this.stacktrace && this.stacktrace.length); }, issueTitle() { - return this.error.title; + return this.GQLerror.title; }, issueDescription() { return sprintf( @@ -84,13 +117,13 @@ export default { ), { description: '# Error Details:\n', - errorUrl: `${this.error.external_url}\n`, - firstSeen: `\n${this.error.first_seen}\n`, - lastSeen: `${this.error.last_seen}\n`, - countLabel: n__('- Event', '- Events', this.error.count), - count: `${this.error.count}\n`, - userCountLabel: n__('- User', '- Users', this.error.user_count), - userCount: `${this.error.user_count}\n`, + errorUrl: `${this.GQLerror.externalUrl}\n`, + firstSeen: `\n${this.GQLerror.firstSeen}\n`, + lastSeen: `${this.GQLerror.lastSeen}\n`, + countLabel: n__('- Event', '- Events', this.GQLerror.count), + count: `${this.GQLerror.count}\n`, + userCountLabel: n__('- User', '- Users', this.GQLerror.userCount), + userCount: `${this.GQLerror.userCount}\n`, }, false, ); @@ -119,7 +152,7 @@ export default { <template> <div> - <div v-if="loading" class="py-3"> + <div v-if="$apollo.queries.GQLerror.loading || loading" class="py-3"> <gl-loading-icon :size="3" /> </div> <div v-else-if="showDetails" class="error-details"> @@ -129,7 +162,7 @@ export default { <gl-form-input class="hidden" name="issue[title]" :value="issueTitle" /> <input name="issue[description]" :value="issueDescription" type="hidden" /> <gl-form-input - :value="error.id" + :value="GQLerror.id" class="hidden" name="issue[sentry_issue_attributes][sentry_issue_identifier]" /> @@ -145,16 +178,16 @@ export default { </form> </div> <div> - <tooltip-on-truncate :title="error.title" truncate-target="child" placement="top"> - <h2 class="text-truncate">{{ error.title }}</h2> + <tooltip-on-truncate :title="GQLerror.title" truncate-target="child" placement="top"> + <h2 class="text-truncate">{{ GQLerror.title }}</h2> </tooltip-on-truncate> <template v-if="error.tags"> - <gl-badge v-if="error.tags.level" variant="danger" class="rounded-pill mr-2">{{ - errorLevel - }}</gl-badge> - <gl-badge v-if="error.tags.logger" variant="light" class="rounded-pill">{{ - error.tags.logger - }}</gl-badge> + <gl-badge v-if="error.tags.level" variant="danger" class="rounded-pill mr-2" + >{{ errorLevel }} + </gl-badge> + <gl-badge v-if="error.tags.logger" variant="light" class="rounded-pill" + >{{ error.tags.logger }} + </gl-badge> </template> <h3>{{ __('Error details') }}</h3> @@ -168,35 +201,35 @@ export default { <li> <span class="bold">{{ __('Sentry event') }}:</span> <gl-link - v-track-event="trackClickErrorLinkToSentryOptions(error.external_url)" - :href="error.external_url" + v-track-event="trackClickErrorLinkToSentryOptions(GQLerror.externalUrl)" + :href="GQLerror.externalUrl" target="_blank" > - <span class="text-truncate">{{ error.external_url }}</span> + <span class="text-truncate">{{ GQLerror.externalUrl }}</span> <icon name="external-link" class="ml-1 flex-shrink-0" /> </gl-link> </li> - <li v-if="error.first_release_short_version"> + <li v-if="GQLerror.firstReleaseShortVersion"> <span class="bold">{{ __('First seen') }}:</span> - {{ formatDate(error.first_seen) }} + {{ formatDate(GQLerror.firstSeen) }} <gl-link :href="firstReleaseLink" target="_blank"> - <span>{{ __('Release') }}: {{ error.first_release_short_version }}</span> + <span>{{ __('Release') }}: {{ GQLerror.firstReleaseShortVersion }}</span> </gl-link> </li> - <li v-if="error.last_release_short_version"> + <li v-if="GQLerror.lastReleaseShortVersion"> <span class="bold">{{ __('Last seen') }}:</span> - {{ formatDate(error.last_seen) }} + {{ formatDate(GQLerror.lastSeen) }} <gl-link :href="lastReleaseLink" target="_blank"> - <span>{{ __('Release') }}: {{ error.last_release_short_version }}</span> + <span>{{ __('Release') }}: {{ GQLerror.lastReleaseShortVersion }}</span> </gl-link> </li> <li> <span class="bold">{{ __('Events') }}:</span> - <span>{{ error.count }}</span> + <span>{{ GQLerror.count }}</span> </li> <li> <span class="bold">{{ __('Users') }}:</span> - <span>{{ error.user_count }}</span> + <span>{{ GQLerror.userCount }}</span> </li> </ul> diff --git a/app/assets/javascripts/error_tracking/details.js b/app/assets/javascripts/error_tracking/details.js index 872cb8868a2..b9761cdf2c1 100644 --- a/app/assets/javascripts/error_tracking/details.js +++ b/app/assets/javascripts/error_tracking/details.js @@ -1,22 +1,39 @@ import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; import store from './store'; import ErrorDetails from './components/error_details.vue'; import csrf from '~/lib/utils/csrf'; +Vue.use(VueApollo); + export default () => { + const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), + }); + // eslint-disable-next-line no-new new Vue({ el: '#js-error_details', + apolloProvider, components: { ErrorDetails, }, store, render(createElement) { const domEl = document.querySelector(this.$options.el); - const { issueDetailsPath, issueStackTracePath, projectIssuesPath } = domEl.dataset; + const { + issueId, + projectPath, + issueDetailsPath, + issueStackTracePath, + projectIssuesPath, + } = domEl.dataset; return createElement('error-details', { props: { + issueId, + projectPath, issueDetailsPath, issueStackTracePath, projectIssuesPath, diff --git a/app/assets/javascripts/error_tracking/queries/details.query.graphql b/app/assets/javascripts/error_tracking/queries/details.query.graphql new file mode 100644 index 00000000000..f65bdb9b968 --- /dev/null +++ b/app/assets/javascripts/error_tracking/queries/details.query.graphql @@ -0,0 +1,18 @@ +query errorDetails($fullPath: ID!, $errorId: ID!) { + project(fullPath: $fullPath) { + sentryDetailedError(id: $errorId) { + id + sentryId + title + userCount + count + firstSeen + lastSeen + message + culprit + externalUrl + firstReleaseShortVersion + lastReleaseShortVersion + } + } +} diff --git a/app/assets/javascripts/helpers/diffs_helper.js b/app/assets/javascripts/helpers/diffs_helper.js new file mode 100644 index 00000000000..9695d01ad3d --- /dev/null +++ b/app/assets/javascripts/helpers/diffs_helper.js @@ -0,0 +1,19 @@ +export function hasInlineLines(diffFile) { + return diffFile?.highlighted_diff_lines?.length > 0; /* eslint-disable-line camelcase */ +} + +export function hasParallelLines(diffFile) { + return diffFile?.parallel_diff_lines?.length > 0; /* eslint-disable-line camelcase */ +} + +export function isSingleViewStyle(diffFile) { + return !hasParallelLines(diffFile) || !hasInlineLines(diffFile); +} + +export function hasDiff(diffFile) { + return ( + hasInlineLines(diffFile) || + hasParallelLines(diffFile) || + !diffFile?.blob?.readable_text /* eslint-disable-line camelcase */ + ); +} diff --git a/app/assets/javascripts/monitoring/stores/actions.js b/app/assets/javascripts/monitoring/stores/actions.js index 1cb82ce0083..fce89b450e4 100644 --- a/app/assets/javascripts/monitoring/stores/actions.js +++ b/app/assets/javascripts/monitoring/stores/actions.js @@ -39,7 +39,7 @@ export const requestMetricsDashboard = ({ commit }) => { }; export const receiveMetricsDashboardSuccess = ({ commit, dispatch }, { response, params }) => { commit(types.SET_ALL_DASHBOARDS, response.all_dashboards); - commit(types.RECEIVE_METRICS_DATA_SUCCESS, response.dashboard.panel_groups); + commit(types.RECEIVE_METRICS_DATA_SUCCESS, response.dashboard); return dispatch('fetchPrometheusMetrics', params); }; export const receiveMetricsDashboardFailure = ({ commit }, error) => { diff --git a/app/assets/javascripts/monitoring/stores/mutations.js b/app/assets/javascripts/monitoring/stores/mutations.js index 16a34a6c026..0b848de9562 100644 --- a/app/assets/javascripts/monitoring/stores/mutations.js +++ b/app/assets/javascripts/monitoring/stores/mutations.js @@ -84,23 +84,26 @@ export default { state.emptyState = 'loading'; state.showEmptyState = true; }, - [types.RECEIVE_METRICS_DATA_SUCCESS](state, groupData) { - state.dashboard.panel_groups = groupData.map((group, i) => { - const key = `${slugify(group.group || 'default')}-${i}`; - let { panels = [] } = group; - - // each panel has metric information that needs to be normalized - panels = panels.map(panel => ({ - ...panel, - metrics: normalizePanelMetrics(panel.metrics, panel.y_label), - })); - - return { - ...group, - panels, - key, - }; - }); + [types.RECEIVE_METRICS_DATA_SUCCESS](state, dashboard) { + state.dashboard = { + ...dashboard, + panel_groups: dashboard.panel_groups.map((group, i) => { + const key = `${slugify(group.group || 'default')}-${i}`; + let { panels = [] } = group; + + // each panel has metric information that needs to be normalized + panels = panels.map(panel => ({ + ...panel, + metrics: normalizePanelMetrics(panel.metrics, panel.y_label), + })); + + return { + ...group, + panels, + key, + }; + }), + }; if (!state.dashboard.panel_groups.length) { state.emptyState = 'noData'; diff --git a/app/helpers/projects/error_tracking_helper.rb b/app/helpers/projects/error_tracking_helper.rb index 2ec7fa91c86..f2d16c30fb4 100644 --- a/app/helpers/projects/error_tracking_helper.rb +++ b/app/helpers/projects/error_tracking_helper.rb @@ -18,9 +18,11 @@ module Projects::ErrorTrackingHelper opts = [project, issue_id, { format: :json }] { - 'project-issues-path' => project_issues_path(project), + 'issue-id' => issue_id, + 'project-path' => project.full_path, 'issue-details-path' => details_project_error_tracking_index_path(*opts), 'issue-update-path' => update_project_error_tracking_index_path(*opts), + 'project-issues-path' => project_issues_path(project), 'issue-stack-trace-path' => stack_trace_project_error_tracking_index_path(*opts) } end diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb index 68548bd2fdc..85cb3f5b46a 100644 --- a/app/models/ci/trigger.rb +++ b/app/models/ci/trigger.rb @@ -11,7 +11,7 @@ module Ci has_many :trigger_requests validates :token, presence: true, uniqueness: true - validates :owner, presence: true, unless: :supports_legacy_tokens? + validates :owner, presence: true before_validation :set_default_values @@ -31,17 +31,8 @@ module Ci token[0...4] if token.present? end - def legacy? - self.owner_id.blank? - end - - def supports_legacy_tokens? - Feature.enabled?(:use_legacy_pipeline_triggers, project) - end - def can_access_project? - supports_legacy_tokens? && legacy? || - Ability.allowed?(self.owner, :create_build, project) + Ability.allowed?(self.owner, :create_build, project) end end end diff --git a/app/policies/ci/trigger_policy.rb b/app/policies/ci/trigger_policy.rb index 578301d7f7e..e26f96a4b2b 100644 --- a/app/policies/ci/trigger_policy.rb +++ b/app/policies/ci/trigger_policy.rb @@ -5,13 +5,12 @@ module Ci delegate { @subject.project } with_options scope: :subject, score: 0 - condition(:legacy) { @subject.supports_legacy_tokens? && @subject.legacy? } with_score 0 condition(:is_owner) { @user && @subject.owner_id == @user.id } rule { ~can?(:admin_build) }.prevent :admin_trigger - rule { legacy | is_owner }.enable :admin_trigger + rule { is_owner }.enable :admin_trigger rule { can?(:admin_build) }.enable :manage_trigger end diff --git a/app/views/errors/_footer.html.haml b/app/views/errors/_footer.html.haml index e67a3a142f6..bb9edc54b4b 100644 --- a/app/views/errors/_footer.html.haml +++ b/app/views/errors/_footer.html.haml @@ -4,7 +4,7 @@ = link_to s_('Nav|Home'), root_path %li - if current_user - = link_to s_('Nav|Sign out and sign in with a different account'), destroy_user_session_path + = link_to s_('Nav|Sign out and sign in with a different account'), destroy_user_session_path, method: :post - else = link_to s_('Nav|Sign In / Register'), new_session_path(:user, redirect_to_referer: 'yes') %li diff --git a/app/views/layouts/header/_current_user_dropdown.html.haml b/app/views/layouts/header/_current_user_dropdown.html.haml index 88803f982e8..84906c305a7 100644 --- a/app/views/layouts/header/_current_user_dropdown.html.haml +++ b/app/views/layouts/header/_current_user_dropdown.html.haml @@ -47,4 +47,4 @@ - if current_user_menu?(:sign_out) %li.divider %li - = link_to _("Sign out"), destroy_user_session_path, class: "sign-out-link", data: { qa_selector: 'sign_out_link' } + = link_to _("Sign out"), destroy_user_session_path, method: :post, class: "sign-out-link", data: { qa_selector: 'sign_out_link' } diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index 9ae06ae77f1..379ba976040 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -55,7 +55,7 @@ - if Feature.enabled?(:user_mode_in_session) - if header_link?(:admin_mode) = nav_link(controller: 'admin/sessions') do - = link_to destroy_admin_session_path, class: 'd-lg-none lock-open-icon' do + = link_to destroy_admin_session_path, method: :post, class: 'd-lg-none lock-open-icon' do = _('Leave Admin Mode') - elsif current_user.admin? = nav_link(controller: 'admin/sessions') do diff --git a/app/views/projects/triggers/_content.html.haml b/app/views/projects/triggers/_content.html.haml deleted file mode 100644 index e686068657c..00000000000 --- a/app/views/projects/triggers/_content.html.haml +++ /dev/null @@ -1,9 +0,0 @@ -- if Feature.enabled?(:use_legacy_pipeline_triggers, @project) - %p.append-bottom-default - Triggers with the - %span.badge.badge-primary legacy - label do not have an associated user and only have access to the current project. - %br - = succeed '.' do - Learn more in the - = link_to 'triggers documentation', help_page_path('ci/triggers/README'), target: '_blank' diff --git a/app/views/projects/triggers/_index.html.haml b/app/views/projects/triggers/_index.html.haml index a559ce41e57..55a9234f01a 100644 --- a/app/views/projects/triggers/_index.html.haml +++ b/app/views/projects/triggers/_index.html.haml @@ -1,6 +1,5 @@ .row.prepend-top-default.append-bottom-default.triggers-container .col-lg-12 - = render "projects/triggers/content" .card .card-header Manage your project's triggers diff --git a/app/views/projects/triggers/_trigger.html.haml b/app/views/projects/triggers/_trigger.html.haml index 60de3630bb5..d80248f7e80 100644 --- a/app/views/projects/triggers/_trigger.html.haml +++ b/app/views/projects/triggers/_trigger.html.haml @@ -7,12 +7,7 @@ %span= trigger.short_token .label-container - - if trigger.legacy? - - if trigger.supports_legacy_tokens? - %span.badge.badge-primary.has-tooltip{ title: "Trigger makes use of deprecated functionality" } legacy - - else - %span.badge.badge-danger.has-tooltip{ title: "Trigger is invalid due to being a legacy trigger. We recommend replacing it with a new trigger" } invalid - - elsif !trigger.can_access_project? + - unless trigger.can_access_project? %span.badge.badge-danger.has-tooltip{ title: "Trigger user has insufficient permissions to project" } invalid %td diff --git a/app/views/projects/triggers/edit.html.haml b/app/views/projects/triggers/edit.html.haml index c35df322b9d..0f74d733c06 100644 --- a/app/views/projects/triggers/edit.html.haml +++ b/app/views/projects/triggers/edit.html.haml @@ -1,9 +1,7 @@ - page_title "Trigger" .row.prepend-top-default.append-bottom-default - .col-lg-3 - = render "content" - .col-lg-9 + .col-lg-12 %h4.prepend-top-0 Update trigger = render "form", btn_text: "Save trigger" |