Commit dcec1415 authored by Clement Ho's avatar Clement Ho

Merge branch...

Merge branch '11818-code-quality-mr-widget-blocks-the-whole-mr-detail-web-ui-while-loading' into 'master'

Un-block UI interactions while Code Quality MR widget is loading

Closes #11818

See merge request gitlab-org/gitlab-ee!14323
parents 2709ff73 c8df337a
...@@ -159,13 +159,15 @@ export default { ...@@ -159,13 +159,15 @@ export default {
this.isLoadingCodequality = true; this.isLoadingCodequality = true;
Promise.all([this.service.fetchReport(head_path), this.service.fetchReport(base_path)]) Promise.all([this.service.fetchReport(head_path), this.service.fetchReport(base_path)])
.then(values => { .then(values =>
this.mr.compareCodeclimateMetrics( this.mr.compareCodeclimateMetrics(
values[0], values[0],
values[1], values[1],
this.mr.headBlobPath, this.mr.headBlobPath,
this.mr.baseBlobPath, this.mr.baseBlobPath,
); ),
)
.then(() => {
this.isLoadingCodequality = false; this.isLoadingCodequality = false;
}) })
.catch(() => { .catch(() => {
......
import CEMergeRequestStore from '~/vue_merge_request_widget/stores/mr_widget_store'; import CEMergeRequestStore from '~/vue_merge_request_widget/stores/mr_widget_store';
import { filterByKey } from 'ee/vue_shared/security_reports/store/utils';
import { mapApprovalsResponse, mapApprovalRulesResponse } from '../mappers'; import { mapApprovalsResponse, mapApprovalRulesResponse } from '../mappers';
import CodeQualityComparisonWorker from '../workers/code_quality_comparison_worker';
export default class MergeRequestStore extends CEMergeRequestStore { export default class MergeRequestStore extends CEMergeRequestStore {
constructor(data) { constructor(data) {
...@@ -94,19 +94,27 @@ export default class MergeRequestStore extends CEMergeRequestStore { ...@@ -94,19 +94,27 @@ export default class MergeRequestStore extends CEMergeRequestStore {
}; };
} }
static doCodeClimateComparison(headIssues, baseIssues) {
// Do these comparisons in worker threads to avoid blocking the main thread
return new Promise(resolve => {
const worker = new CodeQualityComparisonWorker();
worker.addEventListener('message', ({ data }) => resolve(data));
worker.postMessage({
headIssues,
baseIssues,
});
});
}
compareCodeclimateMetrics(headIssues, baseIssues, headBlobPath, baseBlobPath) { compareCodeclimateMetrics(headIssues, baseIssues, headBlobPath, baseBlobPath) {
const parsedHeadIssues = MergeRequestStore.parseCodeclimateMetrics(headIssues, headBlobPath); const parsedHeadIssues = MergeRequestStore.parseCodeclimateMetrics(headIssues, headBlobPath);
const parsedBaseIssues = MergeRequestStore.parseCodeclimateMetrics(baseIssues, baseBlobPath); const parsedBaseIssues = MergeRequestStore.parseCodeclimateMetrics(baseIssues, baseBlobPath);
this.codeclimateMetrics.newIssues = filterByKey( return MergeRequestStore.doCodeClimateComparison(parsedHeadIssues, parsedBaseIssues).then(
parsedHeadIssues, response => {
parsedBaseIssues, this.codeclimateMetrics.newIssues = response.newIssues;
'fingerprint', this.codeclimateMetrics.resolvedIssues = response.resolvedIssues;
); },
this.codeclimateMetrics.resolvedIssues = filterByKey(
parsedBaseIssues,
parsedHeadIssues,
'fingerprint',
); );
} }
......
import filterByKey from 'ee/vue_shared/security_reports/store/utils/filter_by_key';
const keyToFilterBy = 'fingerprint';
// eslint-disable-next-line no-restricted-globals
self.addEventListener('message', e => {
const { data } = e;
if (data === undefined) {
return;
}
// eslint-disable-next-line no-restricted-globals
self.postMessage({
newIssues: filterByKey(data.parsedHeadIssues, data.parsedBaseIssues, keyToFilterBy),
resolvedIssues: filterByKey(data.parsedBaseIssues, data.parsedHeadIssues, keyToFilterBy),
});
// eslint-disable-next-line no-restricted-globals
self.close();
});
...@@ -3,12 +3,12 @@ import * as types from './mutation_types'; ...@@ -3,12 +3,12 @@ import * as types from './mutation_types';
import { import {
parseSastIssues, parseSastIssues,
parseDependencyScanningIssues, parseDependencyScanningIssues,
filterByKey,
getDastSite, getDastSite,
parseDastIssues, parseDastIssues,
getUnapprovedVulnerabilities, getUnapprovedVulnerabilities,
findIssueIndex, findIssueIndex,
} from './utils'; } from './utils';
import filterByKey from './utils/filter_by_key';
import { parseSastContainer } from './utils/container_scanning'; import { parseSastContainer } from './utils/container_scanning';
import { visitUrl } from '~/lib/utils/url_utility'; import { visitUrl } from '~/lib/utils/url_utility';
......
...@@ -254,17 +254,6 @@ export const parseDastIssues = (issues = [], feedback = []) => ...@@ -254,17 +254,6 @@ export const parseDastIssues = (issues = [], feedback = []) =>
}; };
}); });
/**
* Compares two arrays by the given key and returns the difference
*
* @param {Array} firstArray
* @param {Array} secondArray
* @param {String} key
* @returns {Array}
*/
export const filterByKey = (firstArray = [], secondArray = [], key = '') =>
firstArray.filter(item => !secondArray.find(el => el[key] === item[key]));
export const getUnapprovedVulnerabilities = (issues = [], unapproved = []) => export const getUnapprovedVulnerabilities = (issues = [], unapproved = []) =>
issues.filter(item => unapproved.find(el => el === item.vulnerability)); issues.filter(item => unapproved.find(el => el === item.vulnerability));
......
/**
* Compares two arrays by the given key and returns the difference
*
* @param {Array} firstArray
* @param {Array} secondArray
* @param {String} key
* @returns {Array}
*/
const filterByKey = (firstArray = [], secondArray = [], key = '') =>
firstArray.filter(item => !secondArray.find(el => el[key] === item[key]));
export default filterByKey;
---
title: Un-block UI interactions while Code Quality MR widget is loading
merge_request: 14323
author:
type: fixed
...@@ -6,12 +6,12 @@ import { ...@@ -6,12 +6,12 @@ import {
parseDependencyScanningIssues, parseDependencyScanningIssues,
getDastSite, getDastSite,
parseDastIssues, parseDastIssues,
filterByKey,
getUnapprovedVulnerabilities, getUnapprovedVulnerabilities,
groupedTextBuilder, groupedTextBuilder,
statusIcon, statusIcon,
countIssues, countIssues,
} from 'ee/vue_shared/security_reports/store/utils'; } from 'ee/vue_shared/security_reports/store/utils';
import filterByKey from 'ee/vue_shared/security_reports/store/utils/filter_by_key';
import { import {
formatContainerScanningDescription, formatContainerScanningDescription,
formatContainerScanningMessage, formatContainerScanningMessage,
......
...@@ -4,6 +4,7 @@ import axios from '~/lib/utils/axios_utils'; ...@@ -4,6 +4,7 @@ import axios from '~/lib/utils/axios_utils';
import mrWidgetOptions from 'ee/vue_merge_request_widget/mr_widget_options.vue'; import mrWidgetOptions from 'ee/vue_merge_request_widget/mr_widget_options.vue';
import MRWidgetService from 'ee/vue_merge_request_widget/services/mr_widget_service'; import MRWidgetService from 'ee/vue_merge_request_widget/services/mr_widget_service';
import MRWidgetStore from 'ee/vue_merge_request_widget/stores/mr_widget_store'; import MRWidgetStore from 'ee/vue_merge_request_widget/stores/mr_widget_store';
import filterByKey from 'ee/vue_shared/security_reports/store/utils/filter_by_key';
import mountComponent from 'spec/helpers/vue_mount_component_helper'; import mountComponent from 'spec/helpers/vue_mount_component_helper';
import { TEST_HOST } from 'spec/test_constants'; import { TEST_HOST } from 'spec/test_constants';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
...@@ -14,6 +15,8 @@ import mockData, { ...@@ -14,6 +15,8 @@ import mockData, {
headIssues, headIssues,
basePerformance, basePerformance,
headPerformance, headPerformance,
parsedBaseIssues,
parsedHeadIssues,
} from 'ee_spec/vue_mr_widget/mock_data'; } from 'ee_spec/vue_mr_widget/mock_data';
import { import {
...@@ -322,6 +325,14 @@ describe('ee merge request widget options', () => { ...@@ -322,6 +325,14 @@ describe('ee merge request widget options', () => {
mock.onGet('head.json').reply(200, headIssues); mock.onGet('head.json').reply(200, headIssues);
mock.onGet('base.json').reply(200, baseIssues); mock.onGet('base.json').reply(200, baseIssues);
vm = mountComponent(Component); vm = mountComponent(Component);
// mock worker response
spyOn(MRWidgetStore, 'doCodeClimateComparison').and.callFake(() =>
Promise.resolve({
newIssues: filterByKey(parsedHeadIssues, parsedBaseIssues, 'fingerprint'),
resolvedIssues: filterByKey(parsedBaseIssues, parsedHeadIssues, 'fingerprint'),
}),
);
}); });
it('should render provided data', done => { it('should render provided data', done => {
...@@ -372,6 +383,14 @@ describe('ee merge request widget options', () => { ...@@ -372,6 +383,14 @@ describe('ee merge request widget options', () => {
mock.onGet('head.json').reply(200, []); mock.onGet('head.json').reply(200, []);
mock.onGet('base.json').reply(200, []); mock.onGet('base.json').reply(200, []);
vm = mountComponent(Component); vm = mountComponent(Component);
// mock worker response
spyOn(MRWidgetStore, 'doCodeClimateComparison').and.callFake(() =>
Promise.resolve({
newIssues: filterByKey([], [], 'fingerprint'),
resolvedIssues: filterByKey([], [], 'fingerprint'),
}),
);
}); });
afterEach(() => { afterEach(() => {
...@@ -390,6 +409,28 @@ describe('ee merge request widget options', () => { ...@@ -390,6 +409,28 @@ describe('ee merge request widget options', () => {
}); });
}); });
describe('with codeclimate comparison worker rejection', () => {
beforeEach(() => {
mock.onGet('head.json').reply(200, headIssues);
mock.onGet('base.json').reply(200, baseIssues);
vm = mountComponent(Component);
// mock worker rejection
spyOn(MRWidgetStore, 'doCodeClimateComparison').and.callFake(() => Promise.reject());
});
it('should render error indicator', done => {
setTimeout(() => {
expect(
removeBreakLine(
vm.$el.querySelector('.js-codequality-widget .js-code-text').textContent,
),
).toEqual('Failed to load codeclimate report');
done();
}, 0);
});
});
describe('with failed request', () => { describe('with failed request', () => {
beforeEach(() => { beforeEach(() => {
mock.onGet('head.json').reply(500, []); mock.onGet('head.json').reply(500, []);
......
import MergeRequestStore from 'ee/vue_merge_request_widget/stores/mr_widget_store'; import MergeRequestStore from 'ee/vue_merge_request_widget/stores/mr_widget_store';
import filterByKey from 'ee/vue_shared/security_reports/store/utils/filter_by_key';
import { stateKey } from '~/vue_merge_request_widget/stores/state_maps'; import { stateKey } from '~/vue_merge_request_widget/stores/state_maps';
import mockData, { import mockData, {
headIssues, headIssues,
...@@ -16,7 +17,15 @@ describe('MergeRequestStore', () => { ...@@ -16,7 +17,15 @@ describe('MergeRequestStore', () => {
describe('compareCodeclimateMetrics', () => { describe('compareCodeclimateMetrics', () => {
beforeEach(() => { beforeEach(() => {
store.compareCodeclimateMetrics(headIssues, baseIssues, 'headPath', 'basePath'); // mock worker response
spyOn(MergeRequestStore, 'doCodeClimateComparison').and.callFake(() =>
Promise.resolve({
newIssues: filterByKey(parsedHeadIssues, parsedBaseIssues, 'fingerprint'),
resolvedIssues: filterByKey(parsedBaseIssues, parsedHeadIssues, 'fingerprint'),
}),
);
return store.compareCodeclimateMetrics(headIssues, baseIssues, 'headPath', 'basePath');
}); });
it('should return the new issues', () => { it('should return the new issues', () => {
......
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