Commit 1d76f120 authored by Scott Hampton's avatar Scott Hampton

Merge branch '267612-codequality-badge-improvements' into 'master'

Fix FOSS specs broken by !58833 [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!59306
parents 1a84885d e84af05e
......@@ -184,12 +184,7 @@ export default {
'viewDiffsFileByFile',
'mrReviews',
]),
...mapGetters('diffs', [
'whichCollapsedTypes',
'isParallelView',
'currentDiffIndex',
'fileCodequalityDiff',
]),
...mapGetters('diffs', ['whichCollapsedTypes', 'isParallelView', 'currentDiffIndex']),
...mapGetters(['isNotesFetched', 'getNoteableData']),
diffs() {
if (!this.viewDiffsFileByFile) {
......@@ -287,7 +282,6 @@ export default {
endpointMetadata: this.endpointMetadata,
endpointBatch: this.endpointBatch,
endpointCoverage: this.endpointCoverage,
endpointCodequality: this.endpointCodequality,
endpointUpdateUser: this.endpointUpdateUser,
projectPath: this.projectPath,
dismissEndpoint: this.dismissEndpoint,
......@@ -297,6 +291,10 @@ export default {
mrReviews: this.rehydratedMrReviews,
});
if (this.endpointCodequality) {
this.setCodequalityEndpoint(this.endpointCodequality);
}
if (this.shouldShow) {
this.fetchData();
}
......@@ -341,6 +339,7 @@ export default {
...mapActions('diffs', [
'moveToNeighboringCommit',
'setBaseConfig',
'setCodequalityEndpoint',
'fetchDiffFilesMeta',
'fetchDiffFilesBatch',
'fetchCoverageFiles',
......@@ -532,7 +531,6 @@ 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,11 +67,6 @@ export default {
type: Boolean,
required: true,
},
codequalityDiff: {
type: Array,
required: false,
default: () => [],
},
},
data() {
return {
......@@ -85,7 +80,7 @@ export default {
genericError: GENERIC_ERROR,
},
computed: {
...mapState('diffs', ['currentDiffFileId']),
...mapState('diffs', ['currentDiffFileId', 'codequalityDiff']),
...mapGetters(['isNotesFetched']),
...mapGetters('diffs', ['getDiffFileDiscussions']),
viewBlobHref() {
......@@ -154,7 +149,9 @@ export default {
return loggedIn && featureOn;
},
hasCodequalityChanges() {
return this.codequalityDiff.length > 0;
return (
this.codequalityDiff?.files && this.codequalityDiff?.files[this.file.file_path]?.length > 0
);
},
},
watch: {
......
import Cookies from 'js-cookie';
import Visibility from 'visibilityjs';
import Vue from 'vue';
import { deprecatedCreateFlash as createFlash } from '~/flash';
import { diffViewerModes } from '~/ide/constants';
......@@ -53,15 +52,12 @@ import {
prepareLineForRenamedFile,
} from './utils';
let eTagPoll;
export const setBaseConfig = ({ commit }, options) => {
const {
endpoint,
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
endpointUpdateUser,
projectPath,
dismissEndpoint,
......@@ -75,7 +71,6 @@ export const setBaseConfig = ({ commit }, options) => {
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
endpointUpdateUser,
projectPath,
dismissEndpoint,
......@@ -238,48 +233,6 @@ 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,16 +135,6 @@ 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}
......
......@@ -30,7 +30,6 @@ 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: getViewTypeFromQueryString() || viewTypeFromCookie || defaultViewType,
......
import * as actions from '../actions';
import * as actions from 'ee_else_ce/diffs/store/actions';
import createState from 'ee_else_ce/diffs/store/modules/diff_state';
import mutations from 'ee_else_ce/diffs/store/mutations';
import * as getters from '../getters';
import mutations from '../mutations';
import createState from './diff_state';
export default () => ({
namespaced: true,
......
......@@ -11,7 +11,6 @@ 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,7 +33,6 @@ export default {
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
endpointUpdateUser,
projectPath,
dismissEndpoint,
......@@ -47,7 +46,6 @@ export default {
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
endpointUpdateUser,
projectPath,
dismissEndpoint,
......@@ -91,10 +89,6 @@ export default {
Object.assign(state, { coverageFiles });
},
[types.SET_CODEQUALITY_DATA](state, codequalityDiffData) {
Object.assign(state, { codequalityDiff: codequalityDiffData });
},
[types.RENDER_FILE](state, file) {
renderFile(file);
},
......
......@@ -12,7 +12,7 @@ export default {
i18n: {
badgeTitle: __('Code Quality'),
badgeTooltip: __(
'The merge request has been updated, and the number of code quality violations in this file has changed.',
'The merge request has made changes to this file that affect the number of code quality violations in it.',
),
},
};
......
import Visibility from 'visibilityjs';
import createFlash from '~/flash';
import axios from '~/lib/utils/axios_utils';
import httpStatusCodes from '~/lib/utils/http_status';
import Poll from '~/lib/utils/poll';
import { __ } from '~/locale';
import * as types from './mutation_types';
export * from '~/diffs/store/actions';
let codequalityPoll;
export const setCodequalityEndpoint = ({ commit }, endpoint) => {
commit(types.SET_CODEQUALITY_ENDPOINT, endpoint);
};
export const clearCodequalityPoll = () => {
codequalityPoll = null;
};
export const stopCodequalityPolling = () => {
if (codequalityPoll) codequalityPoll.stop();
};
export const restartCodequalityPolling = () => {
if (codequalityPoll) codequalityPoll.restart();
};
export const fetchCodequality = ({ commit, state, dispatch }) => {
codequalityPoll = 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);
dispatch('stopCodequalityPolling');
}
},
errorCallback: () =>
createFlash(__('Something went wrong on our end while loading the code quality diff.')),
});
if (!Visibility.hidden()) {
codequalityPoll.makeRequest();
}
Visibility.change(() => {
if (!Visibility.hidden()) {
dispatch('restartCodequalityPolling');
} else {
dispatch('stopCodequalityPolling');
}
});
};
import createStateCE from '~/diffs/store/modules/diff_state';
export default () => ({
...createStateCE(),
endpointCodequality: '',
codequalityDiff: {},
});
export const SET_CODEQUALITY_ENDPOINT = 'SET_CODEQUALITY_ENDPOINT';
export const SET_CODEQUALITY_DATA = 'SET_CODEQUALITY_DATA';
import CEMutations from '~/diffs/store/mutations';
import * as types from './mutation_types';
export default {
...CEMutations,
[types.SET_CODEQUALITY_ENDPOINT](state, endpoint) {
Object.assign(state, { endpointCodequality: endpoint });
},
[types.SET_CODEQUALITY_DATA](state, codequalityDiffData) {
Object.assign(state, { codequalityDiff: codequalityDiffData });
},
};
import { shallowMount } from '@vue/test-utils';
import MockAdapter from 'axios-mock-adapter';
import Vue from 'vue';
import Vuex from 'vuex';
import createDiffsStore from 'jest/diffs/create_diffs_store';
import { TEST_HOST } from 'spec/test_constants';
import App from '~/diffs/components/app.vue';
import axios from '~/lib/utils/axios_utils';
const TEST_ENDPOINT = `${TEST_HOST}/diff/endpoint`;
Vue.use(Vuex);
describe('diffs/components/app', () => {
let store;
let wrapper;
let mock;
function createComponent(props = {}, extendStore = () => {}) {
store = createDiffsStore();
store.state.diffs.isLoading = false;
store.state.diffs.isTreeLoaded = true;
extendStore(store);
wrapper = shallowMount(App, {
propsData: {
endpoint: TEST_ENDPOINT,
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: '',
dismissEndpoint: '',
showSuggestPopover: true,
fileByFileUserPreference: false,
...props,
},
store,
});
}
beforeEach(() => {
mock = new MockAdapter(axios);
mock.onGet(TEST_ENDPOINT).reply(200, {});
});
afterEach(() => {
wrapper.destroy();
});
describe('EE 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();
});
});
});
import { shallowMount, createLocalVue } from '@vue/test-utils';
import { mount, createLocalVue } from '@vue/test-utils';
import Vuex from 'vuex';
import CodeQualityBadge from 'ee/diffs/components/code_quality_badge.vue';
......@@ -9,7 +9,7 @@ import createDiffsStore from '~/diffs/store/modules';
const getReadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataReadable));
function createComponent({ first = false, last = false, options = {}, props = {} }) {
function createComponent({ withCodequality = true }) {
const file = getReadableFile();
const localVue = createLocalVue();
......@@ -23,18 +23,31 @@ function createComponent({ first = false, last = false, options = {}, props = {}
store.state.diffs.diffFiles = [file];
const wrapper = shallowMount(DiffFileComponent, {
if (withCodequality) {
store.state.diffs.codequalityDiff = {
files: {
[file.file_path]: [
{ line: 1, description: 'Unexpected alert.', severity: 'minor' },
{
line: 3,
description: 'Arrow function has too many statements (52). Maximum allowed is 30.',
severity: 'minor',
},
],
},
};
}
const wrapper = mount(DiffFileComponent, {
store,
localVue,
propsData: {
file,
canCurrentUserFork: false,
viewDiffsFileByFile: false,
isFirstFile: first,
isLastFile: last,
...props,
isFirstFile: false,
isLastFile: false,
},
...options,
});
return {
......@@ -52,27 +65,24 @@ describe('EE DiffFile', () => {
});
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();
describe('when there is diff data for the file', () => {
beforeEach(() => {
({ wrapper } = createComponent({ withCodequality: true }));
});
it('shows the code quality badge', () => {
expect(wrapper.find(CodeQualityBadge).exists()).toBe(true);
});
});
it('is not shown when there is no diff data for the file', () => {
({ wrapper } = createComponent({}));
describe('when there is no diff data for the file', () => {
beforeEach(() => {
({ wrapper } = createComponent({ withCodequality: false }));
});
expect(wrapper.find(CodeQualityBadge)).toExist();
it('does not show the code quality badge', () => {
expect(wrapper.find(CodeQualityBadge).exists()).toBe(false);
});
});
});
});
import MockAdapter from 'axios-mock-adapter';
import {
setCodequalityEndpoint,
clearCodequalityPoll,
stopCodequalityPolling,
fetchCodequality,
} from 'ee/diffs/store/actions';
import * as types from 'ee/diffs/store/mutation_types';
import testAction from 'helpers/vuex_action_helper';
import createFlash from '~/flash';
import axios from '~/lib/utils/axios_utils';
jest.mock('~/flash');
describe('EE DiffsStoreActions', () => {
describe('setCodequalityEndpoint', () => {
it('should set given endpoint', (done) => {
const endpoint = '/codequality_mr_diff.json';
testAction(
setCodequalityEndpoint,
{ endpoint },
{},
[{ type: types.SET_CODEQUALITY_ENDPOINT, payload: { endpoint } }],
[],
done,
);
});
});
describe('fetchCodequality', () => {
let mock;
const endpoint = '/codequality_mr_diff.json';
beforeEach(() => {
mock = new MockAdapter(axios);
});
afterEach(() => {
stopCodequalityPolling();
clearCodequalityPoll();
});
it('should commit SET_CODEQUALITY_DATA with received response and stop polling', (done) => {
const data = {
files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
};
mock.onGet(endpoint).reply(200, { data });
testAction(
fetchCodequality,
{},
{ endpointCodequality: endpoint },
[{ type: types.SET_CODEQUALITY_DATA, payload: { data } }],
[{ type: 'stopCodequalityPolling' }],
done,
);
});
it('should show flash on API error', (done) => {
mock.onGet(endpoint).reply(400);
testAction(fetchCodequality, {}, { endpoint }, [], [], () => {
expect(createFlash).toHaveBeenCalledTimes(1);
expect(createFlash).toHaveBeenCalledWith(expect.stringMatching('Something went wrong'));
done();
});
});
});
});
import * as types from 'ee/diffs/store/mutation_types';
import mutations from 'ee/diffs/store/mutations';
describe('EE DiffsStoreMutations', () => {
describe('SET_CODEQUALITY_ENDPOINT', () => {
it('sets the endpoint into state', () => {
const state = {};
const endpoint = '/codequality_mr_diff.json';
mutations[types.SET_CODEQUALITY_ENDPOINT](state, endpoint);
expect(state.endpointCodequality).toEqual(endpoint);
});
});
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);
});
});
});
......@@ -29056,6 +29056,9 @@ msgstr ""
msgid "Something went wrong on our end"
msgstr ""
msgid "Something went wrong on our end while loading the code quality diff."
msgstr ""
msgid "Something went wrong on our end."
msgstr ""
......@@ -31027,7 +31030,7 @@ 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."
msgid "The merge request has made changes to this file that affect the number of code quality violations in it."
msgstr ""
msgid "The metric must be one of %{metrics}."
......
......@@ -56,7 +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`,
endpointCodequality: '',
projectPath: 'namespace/project',
currentUser: {},
changesEmptyStateIllustration: '',
......@@ -143,19 +143,11 @@ describe('diffs/components/app', () => {
});
describe('codequality diff', () => {
it('fetches code quality data when endpoint is provided', () => {
it('does not fetch code quality data on FOSS', async () => {
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();
});
});
......
......@@ -17,9 +17,6 @@ import {
fetchDiffFilesBatch,
fetchDiffFilesMeta,
fetchCoverageFiles,
clearEtagPoll,
stopCodequalityPolling,
fetchCodequality,
assignDiscussionsToDiff,
removeDiscussionsFromDiff,
startRenderDiffsQueue,
......@@ -101,7 +98,6 @@ 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;
......@@ -113,7 +109,6 @@ describe('DiffsStoreActions', () => {
endpointBatch,
endpointMetadata,
endpointCoverage,
endpointCodequality,
projectPath,
dismissEndpoint,
showSuggestPopover,
......@@ -123,7 +118,6 @@ describe('DiffsStoreActions', () => {
endpointBatch: '',
endpointMetadata: '',
endpointCoverage: '',
endpointCodequality: '',
projectPath: '',
dismissEndpoint: '',
showSuggestPopover: true,
......@@ -136,7 +130,6 @@ describe('DiffsStoreActions', () => {
endpointMetadata,
endpointBatch,
endpointCoverage,
endpointCodequality,
projectPath,
dismissEndpoint,
showSuggestPopover,
......@@ -306,47 +299,6 @@ 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,26 +376,6 @@ 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;
......
......@@ -115,19 +115,6 @@ 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