Commit ef1b4bd4 authored by Miranda Fluharty's avatar Miranda Fluharty Committed by Scott Hampton

Show code quality badge on files in MR diff view

parent a3cf10c2
......@@ -84,6 +84,11 @@ export default {
required: false,
default: '',
},
endpointCodequality: {
type: String,
required: false,
default: '',
},
projectPath: {
type: String,
required: true,
......@@ -160,6 +165,7 @@ export default {
plainDiffPath: (state) => state.diffs.plainDiffPath,
emailPatchPath: (state) => state.diffs.emailPatchPath,
retrievingBatches: (state) => state.diffs.retrievingBatches,
codequalityDiff: (state) => state.diffs.codequalityDiff,
}),
...mapState('diffs', [
'showTreeList',
......@@ -173,7 +179,12 @@ export default {
'viewDiffsFileByFile',
'mrReviews',
]),
...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']),
...mapGetters('diffs', [
'whichCollapsedTypes',
'isParallelView',
'currentDiffIndex',
'fileCodequalityDiff',
]),
...mapGetters(['isNotesFetched', 'getNoteableData']),
diffs() {
if (!this.viewDiffsFileByFile) {
......@@ -271,6 +282,7 @@ export default {
endpointMetadata: this.endpointMetadata,
endpointBatch: this.endpointBatch,
endpointCoverage: this.endpointCoverage,
endpointCodequality: this.endpointCodequality,
projectPath: this.projectPath,
dismissEndpoint: this.dismissEndpoint,
showSuggestPopover: this.showSuggestPopover,
......@@ -326,6 +338,7 @@ export default {
'fetchDiffFilesMeta',
'fetchDiffFilesBatch',
'fetchCoverageFiles',
'fetchCodequality',
'startRenderDiffsQueue',
'assignDiscussionsToDiff',
'setHighlightedRow',
......@@ -382,6 +395,10 @@ export default {
this.fetchCoverageFiles();
}
if (this.endpointCodequality) {
this.fetchCodequality();
}
if (!this.isNotesFetched) {
notesEventHub.$emit('fetchNotesData');
}
......@@ -509,6 +526,7 @@ export default {
:help-page-path="helpPagePath"
:can-current-user-fork="canCurrentUserFork"
:view-diffs-file-by-file="viewDiffsFileByFile"
:codequality-diff="fileCodequalityDiff(file.file_path)"
/>
<div
v-if="showFileByFileNavigation"
......
......@@ -67,6 +67,11 @@ export default {
type: Boolean,
required: true,
},
codequalityDiff: {
type: Array,
required: false,
default: () => [],
},
},
data() {
return {
......@@ -148,6 +153,9 @@ export default {
return loggedIn && featureOn;
},
hasCodequalityChanges() {
return this.codequalityDiff.length > 0;
},
},
watch: {
'file.id': {
......@@ -294,6 +302,7 @@ export default {
:add-merge-request-buttons="true"
:view-diffs-file-by-file="viewDiffsFileByFile"
:show-local-file-reviews="showLocalFileReviews"
:has-codequality-changes="hasCodequalityChanges"
class="js-file-title file-title gl-border-1 gl-border-solid gl-border-gray-100"
:class="hasBodyClasses.header"
@toggleFile="handleToggle"
......
......@@ -41,6 +41,7 @@ export default {
GlDropdownDivider,
GlFormCheckbox,
GlLoadingIcon,
CodeQualityBadge: () => import('ee_component/diffs/components/code_quality_badge.vue'),
},
directives: {
GlTooltip: GlTooltipDirective,
......@@ -95,6 +96,11 @@ export default {
required: false,
default: false,
},
hasCodequalityChanges: {
type: Boolean,
required: false,
default: false,
},
},
data() {
return {
......@@ -327,6 +333,8 @@ export default {
data-track-property="diff_copy_file"
/>
<code-quality-badge v-if="hasCodequalityChanges" class="gl-mr-2" />
<small v-if="isModeChanged" ref="fileMode" class="mr-1">
{{ diffFile.a_mode }}{{ diffFile.b_mode }}
</small>
......
......@@ -73,6 +73,7 @@ export default function initDiffsApp(store) {
endpointMetadata: dataset.endpointMetadata || '',
endpointBatch: dataset.endpointBatch || '',
endpointCoverage: dataset.endpointCoverage || '',
endpointCodequality: dataset.endpointCodequality || '',
projectPath: dataset.projectPath,
helpPagePath: dataset.helpPagePath,
currentUser: JSON.parse(dataset.currentUserData) || {},
......@@ -114,6 +115,7 @@ export default function initDiffsApp(store) {
endpointMetadata: this.endpointMetadata,
endpointBatch: this.endpointBatch,
endpointCoverage: this.endpointCoverage,
endpointCodequality: this.endpointCodequality,
currentUser: this.currentUser,
projectPath: this.projectPath,
helpPagePath: this.helpPagePath,
......
import Cookies from 'js-cookie';
import Visibility from 'visibilityjs';
import Vue from 'vue';
import { deprecatedCreateFlash as createFlash } from '~/flash';
import { diffViewerModes } from '~/ide/constants';
......@@ -52,12 +53,15 @@ import {
prepareLineForRenamedFile,
} from './utils';
let eTagPoll;
export const setBaseConfig = ({ commit }, options) => {
const {
endpoint,
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
projectPath,
dismissEndpoint,
showSuggestPopover,
......@@ -70,6 +74,7 @@ export const setBaseConfig = ({ commit }, options) => {
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
projectPath,
dismissEndpoint,
showSuggestPopover,
......@@ -231,6 +236,48 @@ export const fetchCoverageFiles = ({ commit, state }) => {
coveragePoll.makeRequest();
};
export const clearEtagPoll = () => {
eTagPoll = null;
};
export const stopCodequalityPolling = () => {
if (eTagPoll) eTagPoll.stop();
};
export const restartCodequalityPolling = () => {
if (eTagPoll) eTagPoll.restart();
};
export const fetchCodequality = ({ commit, state, dispatch }) => {
eTagPoll = new Poll({
resource: {
getCodequalityDiffReports: (endpoint) => axios.get(endpoint),
},
data: state.endpointCodequality,
method: 'getCodequalityDiffReports',
successCallback: ({ status, data }) => {
if (status === httpStatusCodes.OK) {
commit(types.SET_CODEQUALITY_DATA, data);
eTagPoll.stop();
}
},
errorCallback: () => createFlash(__('Something went wrong on our end. Please try again!')),
});
if (!Visibility.hidden()) {
eTagPoll.makeRequest();
}
Visibility.change(() => {
if (!Visibility.hidden()) {
dispatch('restartCodequalityPolling');
} else {
dispatch('stopCodequalityPolling');
}
});
};
export const setHighlightedRow = ({ commit }, lineCode) => {
const fileHash = lineCode.split('_')[0];
commit(types.SET_HIGHLIGHTED_ROW, lineCode);
......
......@@ -135,6 +135,16 @@ export const fileLineCoverage = (state) => (file, line) => {
return {};
};
/**
* Returns the codequality diff data for a given file
* @param {string} filePath
* @returns {Array}
*/
export const fileCodequalityDiff = (state) => (filePath) => {
if (!state.codequalityDiff.files || !state.codequalityDiff.files[filePath]) return [];
return state.codequalityDiff.files[filePath];
};
/**
* Returns index of a currently selected diff in diffFiles
* @returns {number}
......
......@@ -28,6 +28,7 @@ export default () => ({
startVersion: null, // Null unless a target diff is selected for comparison that is not the "base" diff
diffFiles: [],
coverageFiles: {},
codequalityDiff: {},
mergeRequestDiffs: [],
mergeRequestDiff: null,
diffViewType: viewTypeFromQueryString || viewTypeFromCookie || defaultViewType,
......
......@@ -11,6 +11,7 @@ export const SET_MR_FILE_REVIEWS = 'SET_MR_FILE_REVIEWS';
export const SET_DIFF_VIEW_TYPE = 'SET_DIFF_VIEW_TYPE';
export const SET_COVERAGE_DATA = 'SET_COVERAGE_DATA';
export const SET_CODEQUALITY_DATA = 'SET_CODEQUALITY_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';
......
......@@ -33,6 +33,7 @@ export default {
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
projectPath,
dismissEndpoint,
showSuggestPopover,
......@@ -45,6 +46,7 @@ export default {
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
projectPath,
dismissEndpoint,
showSuggestPopover,
......@@ -87,6 +89,10 @@ export default {
Object.assign(state, { coverageFiles });
},
[types.SET_CODEQUALITY_DATA](state, codequalityDiffData) {
Object.assign(state, { codequalityDiff: codequalityDiffData });
},
[types.RENDER_FILE](state, file) {
renderFile(file);
},
......
......@@ -185,6 +185,27 @@ module MergeRequestsHelper
end
end
def diffs_tab_pane_data(project, merge_request, params)
{
"is-locked": merge_request.discussion_locked?,
endpoint: diffs_project_merge_request_path(project, merge_request, 'json', params),
endpoint_metadata: @endpoint_metadata_url,
endpoint_batch: diffs_batch_project_json_merge_request_path(project, merge_request, 'json', params),
endpoint_coverage: @coverage_path,
help_page_path: help_page_path('user/discussions/index.md', anchor: 'suggest-changes'),
current_user_data: @current_user_data,
update_current_user_path: @update_current_user_path,
project_path: project_path(merge_request.project),
changes_empty_state_illustration: image_path('illustrations/merge_request_changes_empty.svg'),
is_fluid_layout: fluid_layout.to_s,
dismiss_endpoint: user_callouts_path,
show_suggest_popover: show_suggest_popover?.to_s,
show_whitespace_default: @show_whitespace_default.to_s,
file_by_file_default: @file_by_file_default.to_s,
default_suggestion_commit_message: default_suggestion_commit_message
}
end
private
def review_requested_merge_requests_count
......
......@@ -81,22 +81,7 @@
- params = request.query_parameters
- if Feature.enabled?(:default_merge_ref_for_diffs, @project, default_enabled: :yaml)
- params = params.merge(diff_head: true)
= render "projects/merge_requests/tabs/pane", name: "diffs", id: "js-diffs-app", class: "diffs", data: { "is-locked": @merge_request.discussion_locked?,
endpoint: diffs_project_merge_request_path(@project, @merge_request, 'json', params),
endpoint_metadata: @endpoint_metadata_url,
endpoint_batch: diffs_batch_project_json_merge_request_path(@project, @merge_request, 'json', params),
endpoint_coverage: @coverage_path,
help_page_path: suggest_changes_help_path,
current_user_data: @current_user_data,
update_current_user_path: @update_current_user_path,
project_path: project_path(@merge_request.project),
changes_empty_state_illustration: image_path('illustrations/merge_request_changes_empty.svg'),
is_fluid_layout: fluid_layout.to_s,
dismiss_endpoint: user_callouts_path,
show_suggest_popover: show_suggest_popover?.to_s,
show_whitespace_default: @show_whitespace_default.to_s,
file_by_file_default: @file_by_file_default.to_s,
default_suggestion_commit_message: default_suggestion_commit_message }
= render "projects/merge_requests/tabs/pane", name: "diffs", id: "js-diffs-app", class: "diffs", data: diffs_tab_pane_data(@project, @merge_request, params)
.mr-loading-status
.loading.hide
......
<script>
import { GlBadge, GlTooltipDirective } from '@gitlab/ui';
import { __ } from '~/locale';
export default {
components: {
GlBadge,
},
directives: {
GlTooltip: GlTooltipDirective,
},
i18n: {
badgeTitle: __('Code Quality'),
badgeTooltip: __(
'The merge request has been updated, and the number of code quality violations in this file has changed.',
),
},
};
</script>
<template>
<gl-badge
v-gl-tooltip
:title="$options.i18n.badgeTooltip"
icon="information"
class="gl-display-inline-block"
>
{{ $options.i18n.badgeTitle }}
</gl-badge>
</template>
......@@ -2,6 +2,8 @@
module EE
module MergeRequestsHelper
extend ::Gitlab::Utils::Override
def render_items_list(items, separator = "and")
items_cnt = items.size
......@@ -15,5 +17,12 @@ module EE
"#{items.join(", ")} #{separator} #{last_item}"
end
end
override :diffs_tab_pane_data
def diffs_tab_pane_data(project, merge_request, params)
super.merge(
endpoint_codequality: (codequality_mr_diff_reports_project_merge_request_path(@project, @merge_request, 'json') if project.licensed_feature_available?(:inline_codequality) && @merge_request.has_codequality_mr_diff_report?)
)
end
end
end
......@@ -157,6 +157,7 @@ class License < ApplicationRecord
group_ci_cd_analytics
group_level_compliance_dashboard
incident_management
inline_codequality
insights
issuable_health_status
jira_vulnerabilities_integration
......
import { shallowMount, createLocalVue } from '@vue/test-utils';
import Vuex from 'vuex';
import CodeQualityBadge from 'ee/diffs/components/code_quality_badge.vue';
import diffFileMockDataReadable from 'jest/diffs/mock_data/diff_file';
import DiffFileComponent from '~/diffs/components/diff_file.vue';
import createDiffsStore from '~/diffs/store/modules';
const getReadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataReadable));
function createComponent({ first = false, last = false, options = {}, props = {} }) {
const file = getReadableFile();
const localVue = createLocalVue();
localVue.use(Vuex);
const store = new Vuex.Store({
modules: {
diffs: createDiffsStore(),
},
});
store.state.diffs.diffFiles = [file];
const wrapper = shallowMount(DiffFileComponent, {
store,
localVue,
propsData: {
file,
canCurrentUserFork: false,
viewDiffsFileByFile: false,
isFirstFile: first,
isLastFile: last,
...props,
},
...options,
});
return {
localVue,
wrapper,
store,
};
}
describe('EE DiffFile', () => {
let wrapper;
afterEach(() => {
wrapper.destroy();
});
describe('code quality badge', () => {
it('is shown when there is diff data for the file', () => {
({ wrapper } = createComponent({
props: {
codequalityDiff: [
{ line: 1, description: 'Unexpected alert.', severity: 'minor' },
{
line: 3,
description: 'Arrow function has too many statements (52). Maximum allowed is 30.',
severity: 'minor',
},
],
},
}));
expect(wrapper.find(CodeQualityBadge)).toExist();
});
it('is not shown when there is no diff data for the file', () => {
({ wrapper } = createComponent({}));
expect(wrapper.find(CodeQualityBadge)).toExist();
});
});
});
......@@ -30516,6 +30516,9 @@ msgstr ""
msgid "The merge request can now be merged."
msgstr ""
msgid "The merge request has been updated, and the number of code quality violations in this file has changed."
msgstr ""
msgid "The metric must be one of %{metrics}."
msgstr ""
......
......@@ -56,6 +56,7 @@ describe('diffs/components/app', () => {
endpointMetadata: `${TEST_HOST}/diff/endpointMetadata`,
endpointBatch: `${TEST_HOST}/diff/endpointBatch`,
endpointCoverage: `${TEST_HOST}/diff/endpointCoverage`,
endpointCodequality: `${TEST_HOST}/diff/endpointCodequality`,
projectPath: 'namespace/project',
currentUser: {},
changesEmptyStateIllustration: '',
......@@ -141,6 +142,24 @@ describe('diffs/components/app', () => {
});
});
describe('codequality diff', () => {
it('fetches code quality data when endpoint is provided', () => {
createComponent();
jest.spyOn(wrapper.vm, 'fetchCodequality');
wrapper.vm.fetchData(false);
expect(wrapper.vm.fetchCodequality).toHaveBeenCalled();
});
it('does not fetch code quality data when endpoint is blank', async () => {
createComponent({ endpointCodequality: '' });
jest.spyOn(wrapper.vm, 'fetchCodequality');
wrapper.vm.fetchData(false);
expect(wrapper.vm.fetchCodequality).not.toHaveBeenCalled();
});
});
it.each`
props | state | expected
${{ isFluidLayout: true }} | ${{ isParallelView: false }} | ${false}
......
......@@ -17,6 +17,9 @@ import {
fetchDiffFilesBatch,
fetchDiffFilesMeta,
fetchCoverageFiles,
clearEtagPoll,
stopCodequalityPolling,
fetchCodequality,
assignDiscussionsToDiff,
removeDiscussionsFromDiff,
startRenderDiffsQueue,
......@@ -98,6 +101,7 @@ describe('DiffsStoreActions', () => {
const endpointMetadata = '/diffs/set/endpoint/metadata';
const endpointBatch = '/diffs/set/endpoint/batch';
const endpointCoverage = '/diffs/set/coverage_reports';
const endpointCodequality = '/diffs/set/codequality_diff';
const projectPath = '/root/project';
const dismissEndpoint = '/-/user_callouts';
const showSuggestPopover = false;
......@@ -109,6 +113,7 @@ describe('DiffsStoreActions', () => {
endpointBatch,
endpointMetadata,
endpointCoverage,
endpointCodequality,
projectPath,
dismissEndpoint,
showSuggestPopover,
......@@ -118,6 +123,7 @@ describe('DiffsStoreActions', () => {
endpointBatch: '',
endpointMetadata: '',
endpointCoverage: '',
endpointCodequality: '',
projectPath: '',
dismissEndpoint: '',
showSuggestPopover: true,
......@@ -130,6 +136,7 @@ describe('DiffsStoreActions', () => {
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
projectPath,
dismissEndpoint,
showSuggestPopover,
......@@ -297,6 +304,47 @@ describe('DiffsStoreActions', () => {
});
});
describe('fetchCodequality', () => {
let mock;
const endpointCodequality = '/fetch';
beforeEach(() => {
mock = new MockAdapter(axios);
});
afterEach(() => {
stopCodequalityPolling();
clearEtagPoll();
});
it('should commit SET_CODEQUALITY_DATA with received response', (done) => {
const data = {
files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
};
mock.onGet(endpointCodequality).reply(200, { data });
testAction(
fetchCodequality,
{},
{ endpointCodequality },
[{ type: types.SET_CODEQUALITY_DATA, payload: { data } }],
[],
done,
);
});
it('should show flash on API error', (done) => {
mock.onGet(endpointCodequality).reply(400);
testAction(fetchCodequality, {}, { endpointCodequality }, [], [], () => {
expect(createFlash).toHaveBeenCalledTimes(1);
expect(createFlash).toHaveBeenCalledWith(expect.stringMatching('Something went wrong'));
done();
});
});
});
describe('setHighlightedRow', () => {
it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => {
testAction(setHighlightedRow, 'ABC_123', {}, [
......
......@@ -376,6 +376,26 @@ describe('Diffs Module Getters', () => {
});
});
describe('fileCodequalityDiff', () => {
beforeEach(() => {
Object.assign(localState.codequalityDiff, {
files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
});
});
it('returns empty array when no codequality data is available', () => {
Object.assign(localState.codequalityDiff, {});
expect(getters.fileCodequalityDiff(localState)('test.js')).toEqual([]);
});
it('returns array when codequality data is available for given file', () => {
expect(getters.fileCodequalityDiff(localState)('app.js')).toEqual([
{ line: 1, description: 'Unexpected alert.', severity: 'minor' },
]);
});
});
describe('suggestionCommitMessage', () => {
let rootState;
......
......@@ -113,6 +113,19 @@ describe('DiffsStoreMutations', () => {
});
});
describe('SET_CODEQUALITY_DATA', () => {
it('should set codequality data', () => {
const state = { codequalityDiff: {} };
const codequality = {
files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
};
mutations[types.SET_CODEQUALITY_DATA](state, codequality);
expect(state.codequalityDiff).toEqual(codequality);
});
});
describe('SET_DIFF_VIEW_TYPE', () => {
it('should set diff view type properly', () => {
const state = {};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment