diff options
Diffstat (limited to 'app')
37 files changed, 425 insertions, 48 deletions
diff --git a/app/assets/javascripts/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue b/app/assets/javascripts/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue index aefb31fe3d5..74b5a62f754 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue +++ b/app/assets/javascripts/create_cluster/eks_cluster/components/eks_cluster_configuration_form.vue @@ -1,5 +1,5 @@ <script> -import { createNamespacedHelpers, mapState, mapActions } from 'vuex'; +import { createNamespacedHelpers, mapState, mapActions, mapGetters } from 'vuex'; import { escape as esc } from 'lodash'; import { GlFormInput, GlFormCheckbox } from '@gitlab/ui'; import { sprintf, s__ } from '~/locale'; @@ -61,6 +61,7 @@ export default { 'gitlabManagedCluster', 'isCreatingCluster', ]), + ...mapGetters(['subnetValid']), ...mapRolesState({ roles: 'items', isLoadingRoles: 'isLoadingItems', @@ -119,7 +120,7 @@ export default { !this.selectedRegion || !this.selectedKeyPair || !this.selectedVpc || - !this.selectedSubnet || + !this.subnetValid || !this.selectedRole || !this.selectedSecurityGroup || !this.selectedInstanceType || @@ -127,6 +128,9 @@ export default { this.isCreatingCluster ); }, + displaySubnetError() { + return Boolean(this.loadingSubnetsError) || this.selectedSubnet?.length === 1; + }, createClusterButtonLabel() { return this.isCreatingCluster ? s__('ClusterIntegration|Creating Kubernetes cluster') @@ -216,6 +220,13 @@ export default { false, ); }, + subnetValidationErrorText() { + if (this.loadingSubnetsError) { + return s__('ClusterIntegration|Could not load subnets for the selected VPC'); + } + + return s__('ClusterIntegration|You should select at least two subnets'); + }, securityGroupDropdownHelpText() { return sprintf( s__( @@ -289,14 +300,14 @@ export default { this.setRegion({ region }); this.setVpc({ vpc: null }); this.setKeyPair({ keyPair: null }); - this.setSubnet({ subnet: null }); + this.setSubnet({ subnet: [] }); this.setSecurityGroup({ securityGroup: null }); this.fetchVpcs({ region }); this.fetchKeyPairs({ region }); }, setVpcAndFetchSubnets(vpc) { this.setVpc({ vpc }); - this.setSubnet({ subnet: null }); + this.setSubnet({ subnet: [] }); this.setSecurityGroup({ securityGroup: null }); this.fetchSubnets({ vpc, region: this.selectedRegion }); this.fetchSecurityGroups({ vpc, region: this.selectedRegion }); @@ -436,8 +447,8 @@ export default { :placeholder="s__('ClusterIntergation|Select a subnet')" :search-field-placeholder="s__('ClusterIntegration|Search subnets')" :empty-text="s__('ClusterIntegration|No subnet found')" - :has-errors="Boolean(loadingSubnetsError)" - :error-message="s__('ClusterIntegration|Could not load subnets for the selected VPC')" + :has-errors="displaySubnetError" + :error-message="subnetValidationErrorText" @input="setSubnet({ subnet: $event })" /> <p class="form-text text-muted" v-html="subnetDropdownHelpText"></p> diff --git a/app/assets/javascripts/create_cluster/eks_cluster/store/getters.js b/app/assets/javascripts/create_cluster/eks_cluster/store/getters.js index e69de29bb2d..bbe4930c191 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/store/getters.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/store/getters.js @@ -0,0 +1,3 @@ +// eslint-disable-next-line import/prefer-default-export +export const subnetValid = ({ selectedSubnet }) => + Array.isArray(selectedSubnet) && selectedSubnet.length >= 2; diff --git a/app/assets/javascripts/create_cluster/eks_cluster/store/state.js b/app/assets/javascripts/create_cluster/eks_cluster/store/state.js index 20434dcce98..d1337e7ea4a 100644 --- a/app/assets/javascripts/create_cluster/eks_cluster/store/state.js +++ b/app/assets/javascripts/create_cluster/eks_cluster/store/state.js @@ -21,7 +21,7 @@ export default () => ({ selectedRole: '', selectedKeyPair: '', selectedVpc: '', - selectedSubnet: '', + selectedSubnet: [], selectedSecurityGroup: '', selectedInstanceType: 'm5.large', nodeCount: '3', diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 77cd2afc106..3ea2a2fbaee 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -50,6 +50,11 @@ export default { type: String, required: true, }, + endpointCoverage: { + type: String, + required: false, + default: '', + }, projectPath: { type: String, required: true, @@ -169,6 +174,7 @@ export default { endpoint: this.endpoint, endpointMetadata: this.endpointMetadata, endpointBatch: this.endpointBatch, + endpointCoverage: this.endpointCoverage, projectPath: this.projectPath, dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, @@ -218,6 +224,7 @@ export default { 'fetchDiffFiles', 'fetchDiffFilesMeta', 'fetchDiffFilesBatch', + 'fetchCoverageFiles', 'startRenderDiffsQueue', 'assignDiscussionsToDiff', 'setHighlightedRow', @@ -292,6 +299,10 @@ export default { }); } + if (this.endpointCoverage) { + this.fetchCoverageFiles(); + } + if (!this.isNotesFetched) { eventHub.$emit('fetchNotesData'); } diff --git a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue index 4eae2e09c08..46ed76450c4 100644 --- a/app/assets/javascripts/diffs/components/diff_expansion_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_expansion_cell.vue @@ -54,7 +54,7 @@ export default { colspan: { type: Number, required: false, - default: 3, + default: 4, }, }, computed: { diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue index a06dbd70ac5..87f0396cf72 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue @@ -51,7 +51,7 @@ export default { <template> <tr v-if="shouldRender" :class="className" class="notes_holder"> - <td class="notes-content" colspan="3"> + <td class="notes-content" colspan="4"> <div class="content"> <diff-discussions v-if="line.discussions.length" diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue index 55a8df43c62..bd99fcb71b8 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -1,5 +1,6 @@ <script> -import { mapActions, mapState } from 'vuex'; +import { mapActions, mapGetters, mapState } from 'vuex'; +import { GlTooltipDirective } from '@gitlab/ui'; import DiffTableCell from './diff_table_cell.vue'; import { MATCH_LINE_TYPE, @@ -15,11 +16,18 @@ export default { components: { DiffTableCell, }, + directives: { + GlTooltip: GlTooltipDirective, + }, props: { fileHash: { type: String, required: true, }, + filePath: { + type: String, + required: true, + }, contextLinesPath: { type: String, required: true, @@ -40,6 +48,7 @@ export default { }; }, computed: { + ...mapGetters('diffs', ['fileLineCoverage']), ...mapState({ isHighlighted(state) { return this.line.line_code !== null && this.line.line_code === state.diffs.highlightedRow; @@ -62,6 +71,9 @@ export default { isMatchLine() { return this.line.type === MATCH_LINE_TYPE; }, + coverageState() { + return this.fileLineCoverage(this.filePath, this.line.new_line); + }, }, created() { this.newLineType = NEW_LINE_TYPE; @@ -114,13 +126,19 @@ export default { class="diff-line-num new_line qa-new-diff-line" /> <td + v-gl-tooltip.hover + :title="coverageState.text" + :class="[line.type, coverageState.class, { hll: isHighlighted }]" + class="line-coverage" + ></td> + <td :class="[ line.type, { hll: isHighlighted, }, ]" - class="line_content" + class="line_content with-coverage" v-html="line.rich_text" ></td> </tr> diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue index 1eb17588376..8b25cdc2887 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_view.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue @@ -48,6 +48,7 @@ export default { <colgroup> <col style="width: 50px;" /> <col style="width: 50px;" /> + <col style="width: 8px;" /> <col /> </colgroup> <tbody> @@ -63,6 +64,7 @@ export default { <inline-diff-table-row :key="`${line.line_code || index}`" :file-hash="diffFile.file_hash" + :file-path="diffFile.file_path" :context-lines-path="diffFile.context_lines_path" :line="line" :is-bottom="index + 1 === diffLinesLength" diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue index 65b41b0e456..b525490f7cc 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -122,7 +122,7 @@ export default { <template> <tr v-if="shouldRender" :class="className" class="notes_holder"> - <td class="notes-content parallel old" colspan="2"> + <td class="notes-content parallel old" colspan="3"> <div v-if="shouldRenderDiscussionsOnLeft" class="content"> <diff-discussions :discussions="line.left.discussions" @@ -147,7 +147,7 @@ export default { </template> </diff-discussion-reply> </td> - <td class="notes-content parallel new" colspan="2"> + <td class="notes-content parallel new" colspan="3"> <div v-if="shouldRenderDiscussionsOnRight" class="content"> <diff-discussions :discussions="line.right.discussions" diff --git a/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue index c1b30eab199..0a80107ced4 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_expansion_row.vue @@ -49,7 +49,7 @@ export default { :line="line.left" :is-top="isTop" :is-bottom="isBottom" - :colspan="4" + :colspan="6" /> </template> </tr> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue index 4c95d618b0f..83d803f42b1 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -1,6 +1,7 @@ <script> -import { mapActions, mapState } from 'vuex'; +import { mapActions, mapGetters, mapState } from 'vuex'; import $ from 'jquery'; +import { GlTooltipDirective } from '@gitlab/ui'; import DiffTableCell from './diff_table_cell.vue'; import { MATCH_LINE_TYPE, @@ -18,11 +19,18 @@ export default { components: { DiffTableCell, }, + directives: { + GlTooltip: GlTooltipDirective, + }, props: { fileHash: { type: String, required: true, }, + filePath: { + type: String, + required: true, + }, contextLinesPath: { type: String, required: true, @@ -44,6 +52,7 @@ export default { }; }, computed: { + ...mapGetters('diffs', ['fileLineCoverage']), ...mapState({ isHighlighted(state) { const lineCode = @@ -82,6 +91,9 @@ export default { isMatchLineRight() { return this.line.right && this.line.right.type === MATCH_LINE_TYPE; }, + coverageState() { + return this.fileLineCoverage(this.filePath, this.line.right.new_line); + }, }, created() { this.newLineType = NEW_LINE_TYPE; @@ -99,7 +111,7 @@ export default { const allCellsInHoveringRow = Array.from(e.currentTarget.children); const hoverIndex = allCellsInHoveringRow.indexOf(hoveringCell); - if (hoverIndex >= 2) { + if (hoverIndex >= 3) { this.isRightHover = isHover; } else { this.isLeftHover = isHover; @@ -143,17 +155,19 @@ export default { line-position="left" class="diff-line-num old_line" /> + <td :class="parallelViewLeftLineType" class="line-coverage left-side"></td> <td :id="line.left.line_code" :class="parallelViewLeftLineType" - class="line_content parallel left-side" + class="line_content with-coverage parallel left-side" @mousedown="handleParallelLineMouseDown" v-html="line.left.rich_text" ></td> </template> <template v-else> <td class="diff-line-num old_line empty-cell"></td> - <td class="line_content parallel left-side empty-cell"></td> + <td class="line-coverage left-side empty-cell"></td> + <td class="line_content with-coverage parallel left-side empty-cell"></td> </template> <template v-if="line.right && !isMatchLineRight"> <diff-table-cell @@ -170,6 +184,12 @@ export default { class="diff-line-num new_line" /> <td + v-gl-tooltip.hover + :title="coverageState.text" + :class="[line.right.type, coverageState.class, { hll: isHighlighted }]" + class="line-coverage right-side" + ></td> + <td :id="line.right.line_code" :class="[ line.right.type, @@ -177,14 +197,15 @@ export default { hll: isHighlighted, }, ]" - class="line_content parallel right-side" + class="line_content with-coverage parallel right-side" @mousedown="handleParallelLineMouseDown" v-html="line.right.rich_text" ></td> </template> <template v-else> <td class="diff-line-num old_line empty-cell"></td> - <td class="line_content parallel right-side empty-cell"></td> + <td class="line-coverage right-side empty-cell"></td> + <td class="line_content with-coverage parallel right-side empty-cell"></td> </template> </tr> </template> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 88baac092a1..d796aad9d06 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -47,8 +47,10 @@ export default { > <colgroup> <col style="width: 50px;" /> + <col style="width: 8px;" /> <col /> <col style="width: 50px;" /> + <col style="width: 8px;" /> <col /> </colgroup> <tbody> @@ -64,6 +66,7 @@ export default { <parallel-diff-table-row :key="line.line_code" :file-hash="diffFile.file_hash" + :file-path="diffFile.file_path" :context-lines-path="diffFile.context_lines_path" :line="line" :is-bottom="index + 1 === diffLinesLength" diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 375ac80021f..ce48e36bfd7 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -69,6 +69,7 @@ export default function initDiffsApp(store) { endpoint: dataset.endpoint, endpointMetadata: dataset.endpointMetadata || '', endpointBatch: dataset.endpointBatch || '', + endpointCoverage: dataset.endpointCoverage || '', projectPath: dataset.projectPath, helpPagePath: dataset.helpPagePath, currentUser: JSON.parse(dataset.currentUserData) || {}, @@ -104,6 +105,7 @@ export default function initDiffsApp(store) { endpoint: this.endpoint, endpointMetadata: this.endpointMetadata, endpointBatch: this.endpointBatch, + endpointCoverage: this.endpointCoverage, currentUser: this.currentUser, projectPath: this.projectPath, helpPagePath: this.helpPagePath, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index bd85105ccb4..18bbdf402ee 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -1,8 +1,10 @@ import Vue from 'vue'; import Cookies from 'js-cookie'; +import Poll from '~/lib/utils/poll'; import axios from '~/lib/utils/axios_utils'; +import httpStatusCodes from '~/lib/utils/http_status'; import createFlash from '~/flash'; -import { s__ } from '~/locale'; +import { __, s__ } from '~/locale'; import { handleLocationHash, historyPushState, scrollToElement } from '~/lib/utils/common_utils'; import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility'; import TreeWorker from '../workers/tree_worker'; @@ -43,6 +45,7 @@ export const setBaseConfig = ({ commit }, options) => { endpoint, endpointMetadata, endpointBatch, + endpointCoverage, projectPath, dismissEndpoint, showSuggestPopover, @@ -52,6 +55,7 @@ export const setBaseConfig = ({ commit }, options) => { endpoint, endpointMetadata, endpointBatch, + endpointCoverage, projectPath, dismissEndpoint, showSuggestPopover, @@ -170,6 +174,26 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { .catch(() => worker.terminate()); }; +export const fetchCoverageFiles = ({ commit, state }) => { + const coveragePoll = new Poll({ + resource: { + getCoverageReports: endpoint => axios.get(endpoint), + }, + data: state.endpointCoverage, + method: 'getCoverageReports', + successCallback: ({ status, data }) => { + if (status === httpStatusCodes.OK) { + commit(types.SET_COVERAGE_DATA, data); + + coveragePoll.stop(); + } + }, + errorCallback: () => createFlash(__('Something went wrong on our end. Please try again!')), + }); + + coveragePoll.makeRequest(); +}; + export const setHighlightedRow = ({ commit }, lineCode) => { const fileHash = lineCode.split('_')[0]; commit(types.SET_HIGHLIGHTED_ROW, lineCode); diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index c4737090a70..3898974638f 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -1,3 +1,4 @@ +import { __, n__ } from '~/locale'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants'; export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE; @@ -99,6 +100,29 @@ export const getCommentFormForDiffFile = state => fileHash => state.commentForms.find(form => form.fileHash === fileHash); /** + * Returns the test coverage hits for a specific line of a given file + * @param {string} file + * @param {number} line + * @returns {number} + */ +export const fileLineCoverage = state => (file, line) => { + if (!state.coverageFiles.files) return {}; + const fileCoverage = state.coverageFiles.files[file]; + if (!fileCoverage) return {}; + const lineCoverage = fileCoverage[String(line)]; + + if (lineCoverage === 0) { + return { text: __('No test coverage'), class: 'no-coverage' }; + } else if (lineCoverage >= 0) { + return { + text: n__('Test coverage: %d hit', 'Test coverage: %d hits', lineCoverage), + class: 'coverage', + }; + } + return {}; +}; + +/** * Returns index of a currently selected diff in diffFiles * @returns {number} */ diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index 011cd24500a..81f1506260c 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -17,6 +17,7 @@ export default () => ({ commit: null, startVersion: null, diffFiles: [], + coverageFiles: {}, mergeRequestDiffs: [], mergeRequestDiff: null, diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType, diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 2097c8d3655..4436935c1ec 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -5,6 +5,7 @@ export const SET_RETRIEVING_BATCHES = 'SET_RETRIEVING_BATCHES'; export const SET_DIFF_DATA = 'SET_DIFF_DATA'; export const SET_DIFF_DATA_BATCH = 'SET_DIFF_DATA_BATCH'; export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE'; +export const SET_COVERAGE_DATA = 'SET_COVERAGE_DATA'; export const SET_MERGE_REQUEST_DIFFS = 'SET_MERGE_REQUEST_DIFFS'; export const TOGGLE_LINE_HAS_FORM = 'TOGGLE_LINE_HAS_FORM'; export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 086a7872a5d..bb4c80b5759 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -16,6 +16,7 @@ export default { endpoint, endpointMetadata, endpointBatch, + endpointCoverage, projectPath, dismissEndpoint, showSuggestPopover, @@ -25,6 +26,7 @@ export default { endpoint, endpointMetadata, endpointBatch, + endpointCoverage, projectPath, dismissEndpoint, showSuggestPopover, @@ -69,6 +71,10 @@ export default { }); }, + [types.SET_COVERAGE_DATA](state, coverageFiles) { + Object.assign(state, { coverageFiles }); + }, + [types.RENDER_FILE](state, file) { Object.assign(file, { renderIt: true, diff --git a/app/assets/stylesheets/highlight/common.scss b/app/assets/stylesheets/highlight/common.scss index bdeac7e97c0..31075b09b83 100644 --- a/app/assets/stylesheets/highlight/common.scss +++ b/app/assets/stylesheets/highlight/common.scss @@ -29,3 +29,15 @@ color: $link; } } + +@mixin line-coverage-border-color($coverage, $no-coverage) { + transition: border-left 0.1s ease-out; + + &.coverage { + border-left: 3px solid $coverage; + } + + &.no-coverage { + border-left: 3px solid $no-coverage; + } +} diff --git a/app/assets/stylesheets/highlight/themes/dark.scss b/app/assets/stylesheets/highlight/themes/dark.scss index cbce0ba3f1e..5ab762a5104 100644 --- a/app/assets/stylesheets/highlight/themes/dark.scss +++ b/app/assets/stylesheets/highlight/themes/dark.scss @@ -24,6 +24,8 @@ $dark-pre-hll-bg: #373b41; $dark-hll-bg: #373b41; $dark-over-bg: #9f9ab5; $dark-expanded-bg: #3e3e3e; +$dark-coverage: #b5bd68; +$dark-no-coverage: #de935f; $dark-c: #969896; $dark-err: #c66; $dark-k: #b294bb; @@ -124,12 +126,18 @@ $dark-il: #de935f; } td.diff-line-num.hll:not(.empty-cell), + td.line-coverage.hll:not(.empty-cell), td.line_content.hll:not(.empty-cell) { background-color: $dark-diff-not-empty-bg; border-color: darken($dark-diff-not-empty-bg, 15%); } + .line-coverage { + @include line-coverage-border-color($dark-coverage, $dark-no-coverage); + } + .diff-line-num.new, + .line-coverage.new, .line_content.new { @include diff-background($dark-new-bg, $dark-new-idiff, $dark-border); @@ -140,6 +148,7 @@ $dark-il: #de935f; } .diff-line-num.old, + .line-coverage.old, .line_content.old { @include diff-background($dark-old-bg, $dark-old-idiff, $dark-border); @@ -168,6 +177,7 @@ $dark-il: #de935f; &:not(.diff-expanded) + .diff-expanded, &.diff-expanded + .line_holder:not(.diff-expanded) { > .diff-line-num, + > .line-coverage, > .line_content { border-top: 1px solid $black; } @@ -175,6 +185,7 @@ $dark-il: #de935f; &.diff-expanded { > .diff-line-num, + > .line-coverage, > .line_content { background: $dark-expanded-bg; border-color: $dark-expanded-bg; diff --git a/app/assets/stylesheets/highlight/themes/monokai.scss b/app/assets/stylesheets/highlight/themes/monokai.scss index 1b61ffa37e3..348ef69cc4f 100644 --- a/app/assets/stylesheets/highlight/themes/monokai.scss +++ b/app/assets/stylesheets/highlight/themes/monokai.scss @@ -17,6 +17,8 @@ $monokai-diff-border: #808080; $monokai-highlight-bg: #ffe792; $monokai-over-bg: #9f9ab5; $monokai-expanded-bg: #3e3e3e; +$monokai-coverage: #a6e22e; +$monokai-no-coverage: #fd971f; $monokai-new-bg: rgba(166, 226, 46, 0.1); $monokai-new-idiff: rgba(166, 226, 46, 0.15); @@ -124,12 +126,18 @@ $monokai-gi: #a6e22e; } td.diff-line-num.hll:not(.empty-cell), + td.line-coverage.hll:not(.empty-cell), td.line_content.hll:not(.empty-cell) { background-color: $monokai-line-empty-bg; border-color: $monokai-line-empty-border; } + .line-coverage { + @include line-coverage-border-color($monokai-coverage, $monokai-no-coverage); + } + .diff-line-num.new, + .line-coverage.new, .line_content.new { @include diff-background($monokai-new-bg, $monokai-new-idiff, $monokai-diff-border); @@ -140,6 +148,7 @@ $monokai-gi: #a6e22e; } .diff-line-num.old, + .line-coverage.old, .line_content.old { @include diff-background($monokai-old-bg, $monokai-old-idiff, $monokai-diff-border); @@ -168,6 +177,7 @@ $monokai-gi: #a6e22e; &:not(.diff-expanded) + .diff-expanded, &.diff-expanded + .line_holder:not(.diff-expanded) { > .diff-line-num, + > .line-coverage, > .line_content { border-top: 1px solid $black; } @@ -175,6 +185,7 @@ $monokai-gi: #a6e22e; &.diff-expanded { > .diff-line-num, + > .line-coverage, > .line_content { background: $monokai-expanded-bg; border-color: $monokai-expanded-bg; diff --git a/app/assets/stylesheets/highlight/themes/none.scss b/app/assets/stylesheets/highlight/themes/none.scss index a7ede266fb5..c8ac3aa8305 100644 --- a/app/assets/stylesheets/highlight/themes/none.scss +++ b/app/assets/stylesheets/highlight/themes/none.scss @@ -51,6 +51,15 @@ @include match-line; } + .line-coverage { + @include line-coverage-border-color($green-500, $orange-500); + + &.old, + &.new { + background-color: $white-normal; + } + } + .diff-line-num { &.old { a { @@ -83,6 +92,7 @@ &:not(.diff-expanded) + .diff-expanded, &.diff-expanded + .line_holder:not(.diff-expanded) { > .diff-line-num, + > .line-coverage, > .line_content { border-top: 1px solid $none-expanded-border; } @@ -90,6 +100,7 @@ &.diff-expanded { > .diff-line-num, + > .line-coverage, > .line_content { background: $none-expanded-bg; border-color: $none-expanded-bg; diff --git a/app/assets/stylesheets/highlight/themes/solarized-dark.scss b/app/assets/stylesheets/highlight/themes/solarized-dark.scss index 6569f3abc8b..f5b36480f18 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-dark.scss +++ b/app/assets/stylesheets/highlight/themes/solarized-dark.scss @@ -21,6 +21,8 @@ $solarized-dark-highlight: #094554; $solarized-dark-hll-bg: #174652; $solarized-dark-over-bg: #9f9ab5; $solarized-dark-expanded-bg: #010d10; +$solarized-dark-coverage: #859900; +$solarized-dark-no-coverage: #cb4b16; $solarized-dark-c: #586e75; $solarized-dark-err: #93a1a1; $solarized-dark-g: #93a1a1; @@ -128,12 +130,18 @@ $solarized-dark-il: #2aa198; } td.diff-line-num.hll:not(.empty-cell), + td.line-coverage.hll:not(.empty-cell), td.line_content.hll:not(.empty-cell) { background-color: $solarized-dark-hll-bg; border-color: darken($solarized-dark-hll-bg, 15%); } + .line-coverage { + @include line-coverage-border-color($solarized-dark-coverage, $solarized-dark-no-coverage); + } + .diff-line-num.new, + .line-coverage.new, .line_content.new { @include diff-background($solarized-dark-new-bg, $solarized-dark-new-idiff, $solarized-dark-border); @@ -144,6 +152,7 @@ $solarized-dark-il: #2aa198; } .diff-line-num.old, + .line-coverage.old, .line_content.old { @include diff-background($solarized-dark-old-bg, $solarized-dark-old-idiff, $solarized-dark-border); @@ -172,6 +181,7 @@ $solarized-dark-il: #2aa198; &:not(.diff-expanded) + .diff-expanded, &.diff-expanded + .line_holder:not(.diff-expanded) { > .diff-line-num, + > .line-coverage, > .line_content { border-top: 1px solid $black; } @@ -179,6 +189,7 @@ $solarized-dark-il: #2aa198; &.diff-expanded { > .diff-line-num, + > .line-coverage, > .line_content { background: $solarized-dark-expanded-bg; border-color: $solarized-dark-expanded-bg; diff --git a/app/assets/stylesheets/highlight/themes/solarized-light.scss b/app/assets/stylesheets/highlight/themes/solarized-light.scss index 4e74a9ea50a..993370642c3 100644 --- a/app/assets/stylesheets/highlight/themes/solarized-light.scss +++ b/app/assets/stylesheets/highlight/themes/solarized-light.scss @@ -23,6 +23,8 @@ $solarized-light-hll-bg: #ddd8c5; $solarized-light-over-bg: #ded7fc; $solarized-light-expanded-border: #d2cdbd; $solarized-light-expanded-bg: #ece6d4; +$solarized-light-coverage: #859900; +$solarized-light-no-coverage: #cb4b16; $solarized-light-c: #93a1a1; $solarized-light-err: #586e75; $solarized-light-g: #586e75; @@ -135,12 +137,18 @@ $solarized-light-il: #2aa198; } td.diff-line-num.hll:not(.empty-cell), + td.line-coverage.hll:not(.empty-cell), td.line_content.hll:not(.empty-cell) { background-color: $solarized-light-hll-bg; border-color: darken($solarized-light-hll-bg, 15%); } + .line-coverage { + @include line-coverage-border-color($solarized-light-coverage, $solarized-light-no-coverage); + } + .diff-line-num.new, + .line-coverage.new, .line_content.new { @include diff-background($solarized-light-new-bg, $solarized-light-new-idiff, $solarized-light-border); @@ -152,6 +160,7 @@ $solarized-light-il: #2aa198; } .diff-line-num.old, + .line-coverage.old, .line_content.old { @include diff-background($solarized-light-old-bg, $solarized-light-old-idiff, $solarized-light-border); @@ -180,6 +189,7 @@ $solarized-light-il: #2aa198; &:not(.diff-expanded) + .diff-expanded, &.diff-expanded + .line_holder:not(.diff-expanded) { > .diff-line-num, + > .line-coverage, > .line_content { border-top: 1px solid $solarized-light-expanded-border; } @@ -187,6 +197,7 @@ $solarized-light-il: #2aa198; &.diff-expanded { > .diff-line-num, + > .line-coverage, > .line_content { background: $solarized-light-expanded-bg; border-color: $solarized-light-expanded-bg; diff --git a/app/assets/stylesheets/highlight/white_base.scss b/app/assets/stylesheets/highlight/white_base.scss index 973f94c63aa..d82a0794d29 100644 --- a/app/assets/stylesheets/highlight/white_base.scss +++ b/app/assets/stylesheets/highlight/white_base.scss @@ -151,6 +151,7 @@ pre.code, &:not(.diff-expanded) + .diff-expanded, &.diff-expanded + .line_holder:not(.diff-expanded) { > .diff-line-num, + > .line-coverage, > .line_content { border-top: 1px solid $white-expanded-border; } @@ -158,6 +159,7 @@ pre.code, &.diff-expanded { > .diff-line-num, + > .line-coverage, > .line_content { background: $white-expanded-bg; border-color: $white-expanded-bg; @@ -197,6 +199,22 @@ pre.code, background-color: $line-select-yellow; } } + + .line-coverage { + @include line-coverage-border-color($green-500, $orange-500); + + &.old { + background-color: $line-removed; + } + + &.new { + background-color: $line-added; + } + + &.hll:not(.empty-cell) { + background-color: $line-select-yellow; + } + } } // highlight line via anchor diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 24c6fec064a..0c043e4f3fb 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -514,6 +514,10 @@ table.code { position: absolute; left: 0.5em; } + + &.with-coverage::before { + left: 0; + } } &.new { @@ -522,6 +526,10 @@ table.code { position: absolute; left: 0.5em; } + + &.with-coverage::before { + left: 0; + } } } } diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index af185887a8c..e87f1728cbb 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -14,7 +14,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo skip_before_action :merge_request, only: [:index, :bulk_update] before_action :whitelist_query_limiting, only: [:assign_related_issues, :update] before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort] - before_action :authorize_read_actual_head_pipeline!, only: [:test_reports, :exposed_artifacts] + before_action :authorize_read_actual_head_pipeline!, only: [:test_reports, :exposed_artifacts, :coverage_reports] before_action :set_issuables_index, only: [:index] before_action :authenticate_user!, only: [:assign_related_issues] before_action :check_user_can_push_to_source_branch!, only: [:rebase] @@ -63,6 +63,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo @issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar') @current_user_data = UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json @show_whitespace_default = current_user.nil? || current_user.show_whitespace_in_diffs + @coverage_path = coverage_reports_project_merge_request_path(@project, @merge_request, format: :json) if @merge_request.has_coverage_reports? set_pipeline_variables @@ -131,6 +132,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo reports_response(@merge_request.compare_test_reports) end + def coverage_reports + if @merge_request.has_coverage_reports? + reports_response(@merge_request.find_coverage_reports) + else + head :no_content + end + end + def exposed_artifacts if @merge_request.has_exposed_artifacts? reports_response(@merge_request.find_exposed_artifacts) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 4762740ee2f..74a1985ca50 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -916,6 +916,14 @@ module Ci end end + def collect_coverage_reports!(coverage_report) + each_report(Ci::JobArtifact::COVERAGE_REPORT_FILE_TYPES) do |file_type, blob| + Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, coverage_report) + end + + coverage_report + end + def report_artifacts job_artifacts.with_reports end diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 38730357593..ae57da9c546 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -11,6 +11,7 @@ module Ci NotSupportedAdapterError = Class.new(StandardError) TEST_REPORT_FILE_TYPES = %w[junit].freeze + COVERAGE_REPORT_FILE_TYPES = %w[cobertura].freeze NON_ERASABLE_FILE_TYPES = %w[trace].freeze DEFAULT_FILE_NAMES = { archive: nil, @@ -29,7 +30,8 @@ module Ci performance: 'performance.json', metrics: 'metrics.txt', lsif: 'lsif.json', - dotenv: '.env' + dotenv: '.env', + cobertura: 'cobertura-coverage.xml' }.freeze INTERNAL_TYPES = { @@ -45,6 +47,7 @@ module Ci network_referee: :gzip, lsif: :gzip, dotenv: :gzip, + cobertura: :gzip, # All these file formats use `raw` as we need to store them uncompressed # for Frontend to fetch the files and do analysis @@ -92,6 +95,10 @@ module Ci with_file_types(TEST_REPORT_FILE_TYPES) end + scope :coverage_reports, -> do + with_file_types(COVERAGE_REPORT_FILE_TYPES) + end + scope :erasable, -> do types = self.file_types.reject { |file_type| NON_ERASABLE_FILE_TYPES.include?(file_type) }.values @@ -121,7 +128,8 @@ module Ci metrics_referee: 13, ## runner referees network_referee: 14, ## runner referees lsif: 15, # LSIF data for code navigation - dotenv: 16 + dotenv: 16, + cobertura: 17 } enum file_format: { diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index b7bab9bc226..61b28a3e712 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -820,6 +820,14 @@ module Ci end end + def coverage_reports + Gitlab::Ci::Reports::CoverageReports.new.tap do |coverage_reports| + builds.latest.with_reports(Ci::JobArtifact.coverage_reports).each do |build| + build.collect_coverage_reports!(coverage_reports) + end + end + end + def has_exposed_artifacts? complete? && builds.latest.with_exposed_artifacts.exists? end diff --git a/app/models/event.rb b/app/models/event.rb index 606c4d8302f..18682bd694c 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -135,29 +135,11 @@ class Event < ApplicationRecord super(presenter_class: ::EventPresenter) end - # rubocop:disable Metrics/CyclomaticComplexity - # rubocop:disable Metrics/PerceivedComplexity def visible_to_user?(user = nil) - if push_action? || commit_note? - Ability.allowed?(user, :download_code, project) - elsif membership_changed? - Ability.allowed?(user, :read_project, project) - elsif created_project_action? - Ability.allowed?(user, :read_project, project) - elsif issue? || issue_note? - Ability.allowed?(user, :read_issue, note? ? note_target : target) - elsif merge_request? || merge_request_note? - Ability.allowed?(user, :read_merge_request, note? ? note_target : target) - elsif personal_snippet_note? || project_snippet_note? - Ability.allowed?(user, :read_snippet, note_target) - elsif milestone? - Ability.allowed?(user, :read_milestone, project) - else - false # No other event types are visible - end + return false unless capability.present? + + Ability.allowed?(user, capability, permission_object) end - # rubocop:enable Metrics/PerceivedComplexity - # rubocop:enable Metrics/CyclomaticComplexity def resource_parent project || group @@ -364,8 +346,38 @@ class Event < ApplicationRecord Event._to_partial_path end + protected + + def capability + @capability ||= begin + if push_action? || commit_note? + :download_code + elsif membership_changed? || created_project_action? + :read_project + elsif issue? || issue_note? + :read_issue + elsif merge_request? || merge_request_note? + :read_merge_request + elsif personal_snippet_note? || project_snippet_note? + :read_snippet + elsif milestone? + :read_milestone + end + end + end + private + def permission_object + if note? + note_target + elsif target_id.present? + target + else + project + end + end + def push_action_name if new_ref? "pushed new" diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index d165c88fd48..412d0fa4ec8 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -567,6 +567,10 @@ class MergeRequest < ApplicationRecord diffs.modified_paths end + def new_paths + diffs.diff_files.map(&:new_path) + end + def diff_base_commit if merge_request_diff.persisted? merge_request_diff.base_commit @@ -1295,6 +1299,24 @@ class MergeRequest < ApplicationRecord compare_reports(Ci::CompareTestReportsService) end + def has_coverage_reports? + return false unless Feature.enabled?(:coverage_report_view, project) + + actual_head_pipeline&.has_reports?(Ci::JobArtifact.coverage_reports) + end + + # TODO: this method and compare_test_reports use the same + # result type, which is handled by the controller's #reports_response. + # we should minimize mistakes by isolating the common parts. + # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 + def find_coverage_reports + unless has_coverage_reports? + return { status: :error, status_reason: 'This merge request does not have coverage reports' } + end + + compare_reports(Ci::GenerateCoverageReportsService) + end + def has_exposed_artifacts? return false unless Feature.enabled?(:ci_expose_arbitrary_artifacts_in_mr, default_enabled: true) @@ -1318,7 +1340,7 @@ class MergeRequest < ApplicationRecord # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 def compare_reports(service_class, current_user = nil) with_reactive_cache(service_class.name, current_user&.id) do |data| - unless service_class.new(project, current_user) + unless service_class.new(project, current_user, id: id) .latest?(base_pipeline, actual_head_pipeline, data) raise InvalidateReactiveCache end @@ -1335,7 +1357,7 @@ class MergeRequest < ApplicationRecord raise NameError, service_class unless service_class < Ci::CompareReportsBaseService current_user = User.find_by(id: current_user_id) - service_class.new(project, current_user).execute(base_pipeline, actual_head_pipeline) + service_class.new(project, current_user, id: id).execute(base_pipeline, actual_head_pipeline) end def all_commits diff --git a/app/models/user.rb b/app/models/user.rb index 17850d13e48..0c7dfac5776 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1672,6 +1672,16 @@ class User < ApplicationRecord callouts.any? end + def gitlab_employee? + strong_memoize(:gitlab_employee) do + if Gitlab.com? + Mail::Address.new(email).domain == "gitlab.com" + else + false + end + end + end + # @deprecated alias_method :owned_or_masters_groups, :owned_or_maintainers_groups diff --git a/app/presenters/projects/import_export/project_export_presenter.rb b/app/presenters/projects/import_export/project_export_presenter.rb new file mode 100644 index 00000000000..8f3fc53af10 --- /dev/null +++ b/app/presenters/projects/import_export/project_export_presenter.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Projects + module ImportExport + class ProjectExportPresenter < Gitlab::View::Presenter::Delegated + include ActiveModel::Serializers::JSON + + presents :project + + def project_members + super + converted_group_members + end + + def description + self.respond_to?(:override_description) ? override_description : super + end + + private + + def converted_group_members + group_members.each do |group_member| + group_member.source_type = 'Project' # Make group members project members of the future import + end + end + + # rubocop: disable CodeReuse/ActiveRecord + def group_members + return [] unless current_user.can?(:admin_group, project.group) + + # We need `.where.not(user_id: nil)` here otherwise when a group has an + # invitee, it would make the following query return 0 rows since a NULL + # user_id would be present in the subquery + # See http://stackoverflow.com/questions/129077/not-in-clause-and-null-values + non_null_user_ids = project.project_members.where.not(user_id: nil).select(:user_id) + GroupMembersFinder.new(project.group).execute.where.not(user_id: non_null_user_ids) + end + # rubocop: enable CodeReuse/ActiveRecord + end + end +end diff --git a/app/services/ci/generate_coverage_reports_service.rb b/app/services/ci/generate_coverage_reports_service.rb new file mode 100644 index 00000000000..ebd1eaf0bad --- /dev/null +++ b/app/services/ci/generate_coverage_reports_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Ci + # TODO: a couple of points with this approach: + # + reuses existing architecture and reactive caching + # - it's not a report comparison and some comparing features must be turned off. + # see CompareReportsBaseService for more notes. + # issue: https://gitlab.com/gitlab-org/gitlab/issues/34224 + class GenerateCoverageReportsService < CompareReportsBaseService + def execute(base_pipeline, head_pipeline) + merge_request = MergeRequest.find_by_id(params[:id]) + { + status: :parsed, + key: key(base_pipeline, head_pipeline), + data: head_pipeline.coverage_reports.pick(merge_request.new_paths) + } + rescue => e + Gitlab::ErrorTracking.track_exception(e, project_id: project.id) + { + status: :error, + key: key(base_pipeline, head_pipeline), + status_reason: _('An error occurred while fetching coverage reports.') + } + end + + def latest?(base_pipeline, head_pipeline, data) + data&.fetch(:key, nil) == key(base_pipeline, head_pipeline) + end + end +end diff --git a/app/services/projects/import_export/export_service.rb b/app/services/projects/import_export/export_service.rb index 74d5af41a04..f4214410226 100644 --- a/app/services/projects/import_export/export_service.rb +++ b/app/services/projects/import_export/export_service.rb @@ -54,7 +54,16 @@ module Projects end def project_tree_saver - Gitlab::ImportExport::Project::TreeSaver.new(project: project, current_user: current_user, shared: shared, params: params) + tree_saver_class.new(project: project, current_user: current_user, shared: shared, params: params) + end + + def tree_saver_class + if ::Feature.enabled?(:streaming_serializer, project) + Gitlab::ImportExport::Project::TreeSaver + else + # Once we remove :streaming_serializer feature flag, Project::LegacyTreeSaver should be removed as well + Gitlab::ImportExport::Project::LegacyTreeSaver + end end def uploads_saver diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index d65c874f245..4304a18558e 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -78,6 +78,7 @@ endpoint: diffs_project_merge_request_path(@project, @merge_request, 'json', request.query_parameters), endpoint_metadata: diffs_metadata_project_json_merge_request_path(@project, @merge_request, 'json', request.query_parameters), endpoint_batch: diffs_batch_project_json_merge_request_path(@project, @merge_request, 'json', request.query_parameters), + endpoint_coverage: @coverage_path, help_page_path: suggest_changes_help_path, current_user_data: @current_user_data, project_path: project_path(@merge_request.project), |