Commit 801a5093 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Compare codeclimate artifacts on the merge request page

It will compare code issues for merge request base and head and show
output in the merge request widget. Project must have codeclimate job added
to .gitlab-ci.yml according to https://docs.gitlab.com/ce/ci/examples/code_climate.html
and have finished pipeline for base and head commits of the merge
request
Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent b0595414
<script>
import successIcon from 'icons/_icon_status_success.svg';
import errorIcon from 'icons/_icon_status_failed.svg';
import issuesBlock from './mr_widget_code_quality_issues.vue';
import loadingIcon from '../../vue_shared/components/loading_icon.vue';
import '../../lib/utils/text_utility';
export default {
name: 'MRWidgetCodeQuality',
props: {
mr: {
type: Object,
required: true,
},
service: {
type: Object,
required: true,
},
},
components: {
issuesBlock,
loadingIcon,
},
data() {
return {
collapseText: 'Expand',
isCollapsed: true,
isLoading: false,
loadingFailed: false,
};
},
computed: {
stateIcon() {
return this.mr.codeclimateMetrics.newIssues.length ? errorIcon : successIcon;
},
hasNoneIssues() {
const { newIssues, resolvedIssues } = this.mr.codeclimateMetrics;
return !newIssues.length && !resolvedIssues.length;
},
hasIssues() {
const { newIssues, resolvedIssues } = this.mr.codeclimateMetrics;
return newIssues.length || resolvedIssues.length;
},
codeText() {
const { newIssues, resolvedIssues } = this.mr.codeclimateMetrics;
let newIssuesText = '';
let resolvedIssuesText = '';
let text = '';
if (this.hasNoneIssues) {
text = 'No changes to code quality so far.';
} else if (this.hasIssues) {
if (newIssues.length) {
newIssuesText = `degraded on ${newIssues.length} ${this.pointsText(newIssues)}`;
}
if (resolvedIssues.length) {
resolvedIssuesText = `improved on ${resolvedIssues.length} ${this.pointsText(resolvedIssues)}`;
}
const connector = this.hasIssues ? 'and' : '';
text = `Code quality ${resolvedIssuesText} ${connector} ${newIssuesText}.`;
}
return text;
},
},
methods: {
pointsText(issues) {
return gl.text.pluralize('point', issues.length);
},
toggleCollapsed() {
this.isCollapsed = !this.isCollapsed;
const text = this.isCollapsed ? 'Expand' : 'Collapse';
this.collapseText = text;
},
handleError() {
this.isLoading = false;
this.loadingFailed = true;
},
},
created() {
const { head_path, base_path } = this.mr.codeclimate;
this.isLoading = true;
this.service.fetchCodeclimate(head_path)
.then(resp => resp.json())
.then((data) => {
this.mr.setCodeclimateHeadMetrics(data);
this.service.fetchCodeclimate(base_path)
.then(response => response.json())
.then(baseData => this.mr.setCodeclimateBaseMetrics(baseData))
.then(() => this.mr.compareCodeclimateMetrics())
.then(() => {
this.isLoading = false;
})
.catch(() => this.handleError());
})
.catch(() => this.handleError());
},
};
</script>
<template>
<section class="mr-widget-code-quality">
<div
v-if="isLoading"
class="padding-left">
<i
class="fa fa-spinner fa-spin"
aria-hidden="true">
</i>
Loading codeclimate report.
</div>
<div v-else-if="!isLoading && !loadingFailed">
<span
class="padding-left ci-status-icon"
:class="{
'ci-status-icon-failed': mr.codeclimateMetrics.newIssues.length,
'ci-status-icon-passed': mr.codeclimateMetrics.newIssues.length === 0
}"
v-html="stateIcon">
</span>
<span>
{{codeText}}
</span>
<button
type="button"
class="btn-link btn-blank"
v-if="hasIssues"
@click="toggleCollapsed">
{{collapseText}}
</button>
<div
class="code-quality-container"
v-if="hasIssues"
v-show="!isCollapsed">
<issues-block
class="js-mr-code-new-issues"
v-if="mr.codeclimateMetrics.newIssues.length"
type="failed"
:issues="mr.codeclimateMetrics.newIssues"
/>
<issues-block
class="js-mr-code-resolved-issues"
v-if="mr.codeclimateMetrics.resolvedIssues.length"
type="success"
:issues="mr.codeclimateMetrics.resolvedIssues"
/>
</div>
</div>
<div
v-else-if="loadingFailed"
class="padding-left">
Failed to load codeclimate report.
</div>
</section>
</template>
<script>
export default {
name: 'MRWidgetCodeQualityIssues',
props: {
issues: {
type: Array,
required: true,
},
type: {
type: String,
required: true,
},
},
};
</script>
<template>
<ul class="mr-widget-code-quality-list">
<li
class="commit-sha"
:class="{
failed: type === 'failed',
success: type === 'success'
}
"v-for="issue in issues">
<i
class="fa"
:class="{
'fa-minus': type === 'failed',
'fa-plus': type === 'success'
}"
aria-hidden="true">
</i>
<span>
{{issue.check_name}}
{{issue.location.path}}
{{issue.location.positions}}
{{issue.location.lines}}
</span>
</li>
</ul>
</template>
......@@ -16,6 +16,7 @@ export { default as WidgetMergeHelp } from './components/mr_widget_merge_help';
export { default as WidgetPipeline } from './components/mr_widget_pipeline';
export { default as WidgetDeployment } from './components/mr_widget_deployment';
export { default as WidgetRelatedLinks } from './components/mr_widget_related_links';
export { default as WidgetCodeQuality } from './components/mr_widget_code_quality.vue';
export { default as MergedState } from './components/states/mr_widget_merged';
export { default as FailedToMerge } from './components/states/mr_widget_failed_to_merge';
export { default as ClosedState } from './components/states/mr_widget_closed';
......
......@@ -6,6 +6,7 @@ import {
WidgetPipeline,
WidgetDeployment,
WidgetRelatedLinks,
WidgetCodeQuality,
MergedState,
ClosedState,
LockedState,
......@@ -58,6 +59,10 @@ export default {
shouldRenderDeployments() {
return this.mr.deployments.length;
},
shouldRenderCodeQuality() {
const { codeclimate } = this.mr;
return codeclimate && codeclimate.head_path && codeclimate.base_path;
},
},
methods: {
createService(store) {
......@@ -81,13 +86,12 @@ export default {
.then((res) => {
this.mr.setData(res);
this.setFavicon();
if (cb) {
cb.call(null, res);
}
})
.catch(() => {
new Flash('Something went wrong. Please try again.'); // eslint-disable-line
});
.catch(() => new Flash('Something went wrong. Please try again.'));
},
initPolling() {
this.pollingInterval = new gl.SmartInterval({
......@@ -134,9 +138,7 @@ export default {
document.body.appendChild(el);
}
})
.catch(() => {
new Flash('Something went wrong. Please try again.'); // eslint-disable-line
});
.catch(() => new Flash('Something went wrong. Please try again.'));
},
resumePolling() {
this.pollingInterval.resume();
......@@ -213,6 +215,7 @@ export default {
'mr-widget-pipeline-failed': PipelineFailedState,
'mr-widget-merge-when-pipeline-succeeds': MergeWhenPipelineSucceedsState,
'mr-widget-auto-merge-failed': AutoMergeFailed,
'mr-widget-code-quality': WidgetCodeQuality,
},
template: `
<div class="mr-state-widget prepend-top-default">
......@@ -224,6 +227,11 @@ export default {
v-if="shouldRenderDeployments"
:mr="mr"
:service="service" />
<mr-widget-code-quality
v-if="shouldRenderCodeQuality"
:mr="mr"
:service="service"
/>
<component
:is="componentName"
:mr="mr"
......
......@@ -47,6 +47,10 @@ export default class MRWidgetService {
return this.mergeActionsContentResource.get();
}
fetchCodeclimate(endpoint) { // eslint-disable-line
return Vue.http.get(endpoint);
}
static stopEnvironment(url) {
return Vue.http.post(url);
}
......
......@@ -5,6 +5,14 @@ export default class MergeRequestStore {
constructor(data) {
this.sha = data.diff_head_sha;
this.codeclimate = data.codeclimate;
this.codeclimateMetrics = {
headIssues: [],
baseIssues: [],
newIssues: [],
resolvedIssues: [],
};
this.setData(data);
}
......@@ -113,6 +121,25 @@ export default class MergeRequestStore {
}
}
setCodeclimateHeadMetrics(data) {
this.codeclimateMetrics.headIssues = data;
}
setCodeclimateBaseMetrics(data) {
this.codeclimateMetrics.baseIssues = data;
}
compareCodeclimateMetrics() {
const { headIssues, baseIssues } = this.codeclimateMetrics;
this.codeclimateMetrics.newIssues = this.filterByFingerprint(headIssues, baseIssues);
this.codeclimateMetrics.resolvedIssues = this.filterByFingerprint(baseIssues, headIssues);
}
filterByFingerprint(firstArray, secondArray) { // eslint-disable-line
return firstArray.filter(item => !secondArray.find(el => el.fingerprint === item.fingerprint));
}
static getAuthorObject(event) {
if (!event) {
return {};
......
......@@ -905,3 +905,42 @@
}
}
}
.mr-widget-code-quality {
padding-top: $gl-padding-top;
.padding-left {
padding-left: $gl-padding;
}
.ci-status-icon {
vertical-align: sub;
svg {
width: 22px;
height: 22px;
margin-right: 4px;
}
}
.code-quality-container {
border-top: 1px solid $gray-darker;
border-bottom: 1px solid $gray-darker;
padding: $gl-padding-top;
background-color: $gray-light;
.mr-widget-code-quality-list {
list-style: none;
padding: 0 36px;
margin: 0;
li.success {
color: $green-500;
}
li.failed {
color: $red-500;
}
}
}
}
......@@ -35,6 +35,7 @@ module Ci
scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) }
scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) }
scope :manual_actions, ->() { where(when: :manual).relevant }
scope :codeclimate, ->() { where(name: 'codeclimate') }
mount_uploader :artifacts_file, ArtifactUploader
mount_uploader :artifacts_metadata, ArtifactUploader
......
......@@ -396,6 +396,13 @@ module Ci
.fabricate!
end
def codeclimate_artifact
artifacts.codeclimate.find do |artifact|
artifact.options[:artifacts][:paths] == ['codeclimate.json'] &&
artifact.artifacts_metadata?
end
end
private
def pipeline_data
......
......@@ -991,4 +991,21 @@ class MergeRequest < ActiveRecord::Base
true
end
def base_pipeline
@base_pipeline ||= project.pipelines.find_by(sha: merge_request_diff&.base_commit_sha)
end
def codeclimate_artifact
@codeclimate_artifact ||= head_pipeline&.codeclimate_artifact
end
def base_codeclimate_artifact
@base_codeclimate_artifact ||= base_pipeline&.codeclimate_artifact
end
def has_codeclimate_data?
codeclimate_artifact&.success? &&
base_codeclimate_artifact&.success?
end
end
......@@ -196,6 +196,23 @@ class MergeRequestEntity < IssuableEntity
merge_request)
end
# EE-specific
expose :codeclimate, if: lambda { |mr, _| mr.has_codeclimate_data? && can?(current_user, :read_build, mr.project) } do
expose :head_path do |merge_request|
raw_namespace_project_build_artifacts_url(merge_request.project.namespace,
merge_request.project,
merge_request.codeclimate_artifact,
path: 'codeclimate.json')
end
expose :base_path do |merge_request|
raw_namespace_project_build_artifacts_url(merge_request.project.namespace,
merge_request.project,
merge_request.base_codeclimate_artifact,
path: 'codeclimate.json')
end
end
private
delegate :current_user, to: :request
......
---
title: Compare codeclimate artifacts on the merge request page
merge_request: 1984
author:
......@@ -103,7 +103,11 @@
"rebase_path": { "type": ["string", "null"] },
"approved": { "type": "boolean" },
"approvals_path": { "type": ["string", "null"] },
"ff_only_enabled": { "type": "boolean" }
"ff_only_enabled": { "type": "boolean" },
"codeclimate": {
"head_path": { "type": "string" },
"base_path": { "type": "string" }
}
},
"additionalProperties": false
}
import Vue from 'vue';
import mrWidgetCodeQualityIssues from '~/vue_merge_request_widget/components/mr_widget_code_quality_issues.vue';
describe('Merge Request Code Quality Issues', () => {
let vm;
let MRWidgetCodeQualityIssues;
let mountComponent;
beforeEach(() => {
MRWidgetCodeQualityIssues = Vue.extend(mrWidgetCodeQualityIssues);
mountComponent = props => new MRWidgetCodeQualityIssues({ propsData: props }).$mount();
});
describe('Renders provided list of issues', () => {
describe('with positions and lines', () => {
beforeEach(() => {
vm = mountComponent({
type: 'success',
issues: [{
check_name: 'foo',
location: {
path: 'bar',
positions: '81',
lines: '21',
},
}],
});
});
it('should render issue', () => {
expect(
vm.$el.querySelector('li span').textContent.trim().replace(/\s+/g, ''),
).toEqual('foobar8121');
});
});
describe('without positions and lines', () => {
beforeEach(() => {
vm = mountComponent({
type: 'success',
issues: [{
check_name: 'foo',
location: {
path: 'bar',
},
}],
});
});
it('should render issue without position and lines', () => {
expect(
vm.$el.querySelector('li span').textContent.trim().replace(/\s+/g, ''),
).toEqual('foobar');
});
});
describe('for type failed', () => {
beforeEach(() => {
vm = mountComponent({
type: 'failed',
issues: [{
check_name: 'foo',
location: {
path: 'bar',
positions: '81',
lines: '21',
},
}],
});
});
it('should render failed minus icon', () => {
expect(vm.$el.querySelector('li').classList.contains('failed')).toEqual(true);
expect(vm.$el.querySelector('li i').classList.contains('fa-minus')).toEqual(true);
});
});
describe('for type success', () => {
beforeEach(() => {
vm = mountComponent({
type: 'success',
issues: [{
check_name: 'foo',
location: {
path: 'bar',
positions: '81',
lines: '21',
},
}],
});
});
it('should render success plus icon', () => {
expect(vm.$el.querySelector('li').classList.contains('success')).toEqual(true);
expect(vm.$el.querySelector('li i').classList.contains('fa-plus')).toEqual(true);
});
});
});
});
import Vue from 'vue';
import mrWidgetCodeQuality from '~/vue_merge_request_widget/components/mr_widget_code_quality.vue';
import Store from '~/vue_merge_request_widget/stores/mr_widget_store';
import Service from '~/vue_merge_request_widget/services/mr_widget_service';
import mockData, { baseIssues, headIssues } from '../mock_data';
describe('Merge Request Code Quality', () => {
let vm;
let MRWidgetCodeQuality;
let store;
let mountComponent;
let service;
beforeEach(() => {
MRWidgetCodeQuality = Vue.extend(mrWidgetCodeQuality);
store = new Store(mockData);
service = new Service('');
mountComponent = props => new MRWidgetCodeQuality({ propsData: props }).$mount();
});
afterEach(() => {
vm.$destroy();
});
describe('when it is loading', () => {
beforeEach(() => {
vm = mountComponent({
mr: store,
service,
});
});
it('should render loading indicator', () => {
expect(vm.$el.textContent.trim()).toEqual('Loading codeclimate report.');
});
});
describe('with successfull request', () => {
const interceptor = (request, next) => {
if (request.url === 'head.json') {
next(request.respondWith(JSON.stringify(headIssues), {
status: 200,
}));
}
if (request.url === 'base.json') {
next(request.respondWith(JSON.stringify(baseIssues), {
status: 200,
}));
}
};
beforeEach(() => {
Vue.http.interceptors.push(interceptor);
vm = mountComponent({
mr: store,
service,
});
});
afterEach(() => {
Vue.http.interceptors = _.without(Vue.http.interceptors, interceptor);
});
it('should render provided data', (done) => {
setTimeout(() => {
expect(
vm.$el.querySelector('span:nth-child(2)').textContent.trim(),
).toEqual('Code quality improved on 1 point and degraded on 1 point.');
done();
}, 0);
});
describe('toggleCollapsed', () => {
it('toggles issues', (done) => {
setTimeout(() => {
vm.$el.querySelector('button').click();
Vue.nextTick(() => {
expect(
vm.$el.querySelector('.code-quality-container').geAttribute('style'),
).toEqual(null);
expect(
vm.$el.querySelector('button').textContent.trim(),
).toEqual('Collapse');
vm.$el.querySelector('button').click();
Vue.nextTick(() => {
expect(
vm.$el.querySelector('.code-quality-container').geAttribute('style'),
).toEqual('display: none;');
expect(
vm.$el.querySelector('button').textContent.trim(),
).toEqual('Expand');
});
});
done();
}, 0);
});
});
});
describe('with empty successfull request', () => {
const emptyInterceptor = (request, next) => {
if (request.url === 'head.json') {
next(request.respondWith(JSON.stringify([]), {
status: 200,
}));
}
if (request.url === 'base.json') {
next(request.respondWith(JSON.stringify([]), {
status: 200,
}));
}
};
beforeEach(() => {
Vue.http.interceptors.push(emptyInterceptor);
vm = mountComponent({
mr: store,
service,
});
});
afterEach(() => {
Vue.http.interceptors = _.without(Vue.http.interceptors, emptyInterceptor);
});
it('should render provided data', (done) => {
setTimeout(() => {
expect(
vm.$el.querySelector('span:nth-child(2)').textContent.trim(),
).toEqual('No changes to code quality so far.');
done();
}, 0);
});
});
describe('with failed request', () => {
const errorInterceptor = (request, next) => {
if (request.url === 'head.json') {
next(request.respondWith(JSON.stringify([]), {
status: 500,
}));
}
if (request.url === 'base.json') {
next(request.respondWith(JSON.stringify([]), {
status: 500,
}));
}
};
beforeEach(() => {
Vue.http.interceptors.push(errorInterceptor);
vm = mountComponent({
mr: store,
service,
});
});
afterEach(() => {
Vue.http.interceptors = _.without(Vue.http.interceptors, errorInterceptor);
});
it('should render error indicator', (done) => {
setTimeout(() => {
expect(vm.$el.textContent.trim()).toEqual('Failed to load codeclimate report.');
done();
}, 0);
});
});
});
......@@ -210,5 +210,68 @@ export default {
"merge_commit_message_with_description": "Merge branch 'daaaa' into 'master'\n\nUpdate README.md\n\nSee merge request !22",
"diverged_commits_count": 0,
"only_allow_merge_if_pipeline_succeeds": false,
"commit_change_content_path": "/root/acets-app/merge_requests/22/commit_change_content"
"commit_change_content_path": "/root/acets-app/merge_requests/22/commit_change_content",
"codeclimate": {
"head_path": "head.json",
"base_path": "base.json"
}
}
export const headIssues = [
{
"check_name": "Rubocop/Lint/UselessAssignment",
"location": {
"path": "lib/six.rb",
"positions": {
"begin": {
"column": 6,
"line": 59
},
"end": {
"column": 7,
"line": 59
}
}
},
"fingerprint": "e879dd9bbc0953cad5037cde7ff0f627",
},
{
"categories": ["Security"],
"check_name": "Insecure Dependency",
"location": {
"path": "Gemfile.lock",
"lines": {
"begin": 22,
"end": 22
}
},
"fingerprint": "ca2e59451e98ae60ba2f54e3857c50e5",
}
];
export const baseIssues = [
{
"categories": ["Security"],
"check_name": "Insecure Dependency",
"location": {
"path": "Gemfile.lock",
"lines": {
"begin": 22,
"end": 22
}
},
"fingerprint": "ca2e59451e98ae60ba2f54e3857c50e5",
},
{
"categories": ["Security"],
"check_name": "Insecure Dependency",
"location": {
"path": "Gemfile.lock",
"lines": {
"begin": 21,
"end": 21
}
},
"fingerprint": "ca2354534dee94ae60ba2f54e3857c50e5",
}
]
import MergeRequestStore from '~/vue_merge_request_widget/stores/mr_widget_store';
import mockData from '../mock_data';
import mockData, { headIssues, baseIssues } from '../mock_data';
describe('MergeRequestStore', () => {
describe('setData', () => {
let store;
beforeEach(() => {
store = new MergeRequestStore(mockData);
});
describe('setData', () => {
it('should set hasSHAChanged when the diff SHA changes', () => {
store.setData({ ...mockData, diff_head_sha: 'a-different-string' });
expect(store.hasSHAChanged).toBe(true);
......@@ -19,4 +19,45 @@ describe('MergeRequestStore', () => {
expect(store.hasSHAChanged).toBe(false);
});
});
describe('setCodeclimateHeadMetrics', () => {
it('should set defaults', () => {
expect(store.codeclimate).toEqual(mockData.codeclimate);
expect(store.codeclimateMetrics).toEqual({
headIssues: [],
baseIssues: [],
newIssues: [],
resolvedIssues: [],
});
});
it('should set the provided head metrics', () => {
store.setCodeclimateHeadMetrics(headIssues);
expect(store.codeclimateMetrics.headIssues).toEqual(headIssues);
});
});
describe('setCodeclimateBaseMetrics', () => {
it('should set the provided base metrics', () => {
store.setCodeclimateBaseMetrics(baseIssues);
expect(store.codeclimateMetrics.baseIssues).toEqual(baseIssues);
});
});
describe('compareCodeclimateMetrics', () => {
beforeEach(() => {
store.setCodeclimateHeadMetrics(headIssues);
store.setCodeclimateBaseMetrics(baseIssues);
store.compareCodeclimateMetrics();
});
it('should return the new issues', () => {
expect(store.codeclimateMetrics.newIssues[0]).toEqual(headIssues[0]);
});
it('should return the resolved issues', () => {
expect(store.codeclimateMetrics.resolvedIssues[0]).toEqual(baseIssues[1]);
});
});
});
......@@ -1218,4 +1218,24 @@ describe Ci::Pipeline, models: true do
it_behaves_like 'not sending any notification'
end
end
describe '#codeclimate_artifact' do
let!(:build) do
create(
:ci_build,
:artifacts,
name: 'codeclimate',
pipeline: pipeline,
options: {
artifacts: {
paths: ['codeclimate.json']
}
}
)
end
it do
expect(pipeline.codeclimate_artifact).to eq(build)
end
end
end
......@@ -2065,4 +2065,10 @@ describe MergeRequest, models: true do
end
end
end
describe '#base_pipeline' do
let!(:pipeline) { create(:ci_empty_pipeline, project: subject.project, sha: subject.diff_base_sha) }
it { expect(subject.base_pipeline).to eq(pipeline) }
end
end
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