Commit 0530cc86 authored by GitLab Bot's avatar GitLab Bot

Automatic merge of gitlab-org/gitlab master

parents b2aad714 fb56d1ef
......@@ -91,9 +91,9 @@ export default {
<h3 class="page-title gl-m-0">{{ title }}</h3>
</div>
<div v-if="error.length" class="alert alert-danger">
<gl-alert v-if="error.length" variant="warning" class="gl-mb-5" :dismissible="false">
<p v-for="(message, index) in error" :key="index" class="gl-mb-0">{{ message }}</p>
</div>
</gl-alert>
<feature-flag-form
:name="name"
......
<script>
import { GlAlert } from '@gitlab/ui';
import { mapState, mapActions } from 'vuex';
import axios from '~/lib/utils/axios_utils';
import FeatureFlagForm from './form.vue';
......@@ -10,6 +11,7 @@ import featureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
export default {
components: {
FeatureFlagForm,
GlAlert,
},
mixins: [featureFlagsMixin()],
inject: {
......@@ -61,9 +63,9 @@ export default {
<div>
<h3 class="page-title">{{ s__('FeatureFlags|New feature flag') }}</h3>
<div v-if="error.length" class="alert alert-danger">
<p v-for="(message, index) in error" :key="index" class="mb-0">{{ message }}</p>
</div>
<gl-alert v-if="error.length" variant="warning" class="gl-mb-5" :dismissible="false">
<p v-for="(message, index) in error" :key="index" class="gl-mb-0">{{ message }}</p>
</gl-alert>
<feature-flag-form
:cancel-path="path"
......
......@@ -2,6 +2,7 @@
import { mapState, mapActions, mapGetters } from 'vuex';
import { componentNames } from '~/reports/components/issue_body';
import { s__, sprintf } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import ReportSection from '~/reports/components/report_section.vue';
import createStore from './store';
......@@ -11,6 +12,7 @@ export default {
components: {
ReportSection,
},
mixins: [glFeatureFlagsMixin()],
props: {
headPath: {
type: String,
......@@ -30,6 +32,11 @@ export default {
required: false,
default: null,
},
codequalityReportsPath: {
type: String,
required: false,
default: '',
},
codequalityHelpPath: {
type: String,
required: true,
......@@ -37,7 +44,7 @@ export default {
},
componentNames,
computed: {
...mapState(['newIssues', 'resolvedIssues']),
...mapState(['newIssues', 'resolvedIssues', 'hasError', 'statusReason']),
...mapGetters([
'hasCodequalityIssues',
'codequalityStatus',
......@@ -51,10 +58,11 @@ export default {
headPath: this.headPath,
baseBlobPath: this.baseBlobPath,
headBlobPath: this.headBlobPath,
reportsPath: this.codequalityReportsPath,
helpPath: this.codequalityHelpPath,
});
this.fetchReports();
this.fetchReports(this.glFeatures.codequalityMrDiff);
},
methods: {
...mapActions(['fetchReports', 'setPaths']),
......@@ -80,5 +88,7 @@ export default {
:popover-options="codequalityPopover"
:show-report-section-status-icon="false"
class="js-codequality-widget mr-widget-border-top mr-report"
/>
>
<template v-if="hasError" #sub-heading>{{ statusReason }}</template>
</report-section>
</template>
......@@ -4,9 +4,20 @@ import { parseCodeclimateMetrics, doCodeClimateComparison } from './utils/codequ
export const setPaths = ({ commit }, paths) => commit(types.SET_PATHS, paths);
export const fetchReports = ({ state, dispatch, commit }) => {
export const fetchReports = ({ state, dispatch, commit }, diffFeatureFlagEnabled) => {
commit(types.REQUEST_REPORTS);
if (diffFeatureFlagEnabled) {
return axios
.get(state.reportsPath)
.then(({ data }) => {
return dispatch('receiveReportsSuccess', {
newIssues: parseCodeclimateMetrics(data.new_errors, state.headBlobPath),
resolvedIssues: parseCodeclimateMetrics(data.resolved_errors, state.baseBlobPath),
});
})
.catch((error) => dispatch('receiveReportsError', error));
}
if (!state.basePath) {
return dispatch('receiveReportsError');
}
......@@ -18,13 +29,13 @@ export const fetchReports = ({ state, dispatch, commit }) => {
),
)
.then((data) => dispatch('receiveReportsSuccess', data))
.catch(() => dispatch('receiveReportsError'));
.catch((error) => dispatch('receiveReportsError', error));
};
export const receiveReportsSuccess = ({ commit }, data) => {
commit(types.RECEIVE_REPORTS_SUCCESS, data);
};
export const receiveReportsError = ({ commit }) => {
commit(types.RECEIVE_REPORTS_ERROR);
export const receiveReportsError = ({ commit }, error) => {
commit(types.RECEIVE_REPORTS_ERROR, error);
};
......@@ -6,6 +6,7 @@ export default {
state.headPath = paths.headPath;
state.baseBlobPath = paths.baseBlobPath;
state.headBlobPath = paths.headBlobPath;
state.reportsPath = paths.reportsPath;
state.helpPath = paths.helpPath;
},
[types.REQUEST_REPORTS](state) {
......@@ -13,12 +14,14 @@ export default {
},
[types.RECEIVE_REPORTS_SUCCESS](state, data) {
state.hasError = false;
state.statusReason = '';
state.isLoading = false;
state.newIssues = data.newIssues;
state.resolvedIssues = data.resolvedIssues;
},
[types.RECEIVE_REPORTS_ERROR](state) {
[types.RECEIVE_REPORTS_ERROR](state, error) {
state.isLoading = false;
state.hasError = true;
state.statusReason = error?.response?.data?.status_reason;
},
};
export default () => ({
basePath: null,
headPath: null,
reportsPath: null,
baseBlobPath: null,
headBlobPath: null,
isLoading: false,
hasError: false,
statusReason: '',
newIssues: [],
resolvedIssues: [],
......
......@@ -3,8 +3,10 @@ import CodeQualityComparisonWorker from '../../workers/codequality_comparison_wo
export const parseCodeclimateMetrics = (issues = [], path = '') => {
return issues.map((issue) => {
const parsedIssue = {
...issue,
name: issue.description,
path: issue.file_path,
urlPath: `${path}/${issue.file_path}#L${issue.line}`,
...issue,
};
if (issue?.location?.path) {
......
......@@ -464,6 +464,7 @@ export default {
:head-path="mr.codeclimate.head_path"
:head-blob-path="mr.headBlobPath"
:base-blob-path="mr.baseBlobPath"
:codequality-reports-path="mr.codequalityReportsPath"
:codequality-help-path="mr.codequalityHelpPath"
/>
......
......@@ -241,10 +241,11 @@ export default class MergeRequestStore {
this.isDismissedSuggestPipeline = data.is_dismissed_suggest_pipeline;
this.securityReportsDocsPath = data.security_reports_docs_path;
// codeclimate
// code quality
const blobPath = data.blob_path || {};
this.headBlobPath = blobPath.head_path || '';
this.baseBlobPath = blobPath.base_path || '';
this.codequalityReportsPath = data.codequality_reports_path;
this.codequalityHelpPath = data.codequality_help_path;
this.codeclimate = data.codeclimate;
......
......@@ -31,11 +31,10 @@ module AvatarsHelper
end
def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true)
if user
user.avatar_url(size: size, only_path: only_path) || default_avatar
else
gravatar_icon(nil, size, scale)
end
return gravatar_icon(nil, size, scale) unless user
return default_avatar if user.blocked?
user.avatar_url(size: size, only_path: only_path) || default_avatar
end
def gravatar_icon(user_email = '', size = nil, scale = 2)
......
---
title: Remove avatar of the blocked user
merge_request: 52051
author: Yogi (@yo)
type: fixed
---
title: Update styling of validation messages in New Feature Flag form
merge_request: 52217
author:
type: changed
---
title: Add `gl-button` to promotion buttons on issue sidebar
merge_request: 51287
author: Yogi (@yo)
type: other
......@@ -278,6 +278,7 @@ export default {
:head-path="mr.codeclimate.head_path"
:head-blob-path="mr.headBlobPath"
:base-blob-path="mr.baseBlobPath"
:codequality-reports-path="mr.codequalityReportsPath"
:codequality-help-path="mr.codequalityHelpPath"
/>
<grouped-browser-performance-reports-app
......
......@@ -20,9 +20,9 @@
%p
= s_('Promotions|Epics let you manage your portfolio of projects more efficiently and with less effort by tracking groups of issues that share a theme, across projects and milestones.')
= link_to s_('Read more'), 'https://docs.gitlab.com/ee/user/group/epics/', class: 'btn-link', target: '_blank'
.gl-display-flex.gl-flex-wrap
.gl-flex-wrap
= render 'shared/promotions/promotion_link_project', short_form: true
= link_to s_("Promotions|Don't show me this again"), '#', class: 'btn js-close js-close-callout gl-mt-2'
= link_to s_("Promotions|Don't show me this again"), '#', class: 'gl-button btn js-close js-close-callout gl-mt-2'
.hide-collapsed
= s_('Promotions|This feature is locked.')
......
......@@ -35,6 +35,6 @@
%div
= render 'shared/promotions/promotion_link_project', short_form: true, target_blank: !trial_cta, trial_cta: trial_cta
- if session_dismissal
= link_to s_("Promotions|Not now, thanks!"), '#', class: ['btn', 'js-close', 'js-close-callout', 'gl-mt-3', 'js-close-session', 'tr-issue-weights-not-now-cta'], data: { track_event: 'click_notnow' }.merge(tracking_options)
= link_to s_("Promotions|Not now, thanks!"), '#', class: ['gl-button', 'btn', 'js-close', 'js-close-callout', 'gl-mt-3', 'js-close-session', 'tr-issue-weights-not-now-cta'], data: { track_event: 'click_notnow' }.merge(tracking_options)
- else
= link_to s_("Promotions|Don't show me this again"), '#', class: ['btn', 'js-close', 'js-close-callout', 'gl-mt-3', 'tr-issue-weights-dont-show-me-again'], data: { track_event: 'click_dontshow' }.merge(tracking_options)
= link_to s_("Promotions|Don't show me this again"), '#', class: ['gl-button', 'btn', 'js-close', 'js-close-callout', 'gl-mt-3', 'tr-issue-weights-dont-show-me-again'], data: { track_event: 'click_dontshow' }.merge(tracking_options)
......@@ -20,6 +20,6 @@
- if License.current&.expired?
= link_to (!short_form ? s_('Promotions|Buy GitLab Enterprise Edition') : s_('Promotions|Buy EE')), ::EE::SUBSCRIPTIONS_PLANS_URL, class: 'btn btn-primary'
- else
= link_to s_('Promotions|Start GitLab Ultimate trial'), new_trial_url, class: 'btn btn-primary btn-block'
= link_to s_('Promotions|Start GitLab Ultimate trial'), new_trial_url, class: 'gl-button btn btn-confirm gl-mb-3'
- else
%p= s_('Promotions|Contact your Administrator to upgrade your license.')
......@@ -75,6 +75,8 @@ describe('Edit feature flag form', () => {
});
const findAlert = () => wrapper.find(GlAlert);
const findWarningGlAlert = () =>
wrapper.findAll(GlAlert).filter((c) => c.props('variant') === 'warning');
it('should display the iid', () => {
expect(wrapper.find('h3').text()).toContain('^5');
......@@ -88,7 +90,7 @@ describe('Edit feature flag form', () => {
expect(wrapper.find(GlToggle).props('value')).toBe(true);
});
it('should not alert users that feature flags are changing soon', () => {
it('should alert users the flag is read only', () => {
expect(findAlert().text()).toContain('GitLab is moving to a new way of managing feature flags');
});
......@@ -96,8 +98,9 @@ describe('Edit feature flag form', () => {
it('should render the error', () => {
store.dispatch('receiveUpdateFeatureFlagError', { message: ['The name is required'] });
return wrapper.vm.$nextTick(() => {
expect(wrapper.find('.alert-danger').exists()).toEqual(true);
expect(wrapper.find('.alert-danger').text()).toContain('The name is required');
const warningGlAlert = findWarningGlAlert();
expect(warningGlAlert.at(1).exists()).toEqual(true);
expect(warningGlAlert.at(1).text()).toContain('The name is required');
});
});
});
......
......@@ -41,6 +41,9 @@ describe('New feature flag form', () => {
});
};
const findWarningGlAlert = () =>
wrapper.findAll(GlAlert).filter((c) => c.props('variant') === 'warning');
beforeEach(() => {
factory();
});
......@@ -53,8 +56,9 @@ describe('New feature flag form', () => {
it('should render the error', () => {
store.dispatch('receiveCreateFeatureFlagError', { message: ['The name is required'] });
return wrapper.vm.$nextTick(() => {
expect(wrapper.find('.alert').exists()).toEqual(true);
expect(wrapper.find('.alert').text()).toContain('The name is required');
const warningGlAlert = findWarningGlAlert();
expect(warningGlAlert.at(0).exists()).toBe(true);
expect(warningGlAlert.at(0).text()).toContain('The name is required');
});
});
});
......@@ -81,10 +85,6 @@ describe('New feature flag form', () => {
expect(wrapper.find(Form).props('scopes')).toContainEqual(defaultScope);
});
it('should not alert users that feature flags are changing soon', () => {
expect(wrapper.find(GlAlert).exists()).toBe(false);
});
it('has an all users strategy by default', () => {
const strategies = wrapper.find(Form).props('strategies');
......
......@@ -88,3 +88,53 @@ export const issueDiff = [
urlPath: 'headPath/lib/six.rb#L6',
},
];
export const reportIssues = {
status: 'failed',
new_errors: [
{
description:
'Method `long_if` has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.',
severity: 'minor',
file_path: 'codequality.rb',
line: 5,
},
],
resolved_errors: [
{
description: 'Insecure Dependency',
severity: 'major',
file_path: 'lib/six.rb',
line: 22,
},
],
existing_errors: [],
summary: { total: 3, resolved: 0, errored: 3 },
};
export const parsedReportIssues = {
newIssues: [
{
description:
'Method `long_if` has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.',
file_path: 'codequality.rb',
line: 5,
name:
'Method `long_if` has a Cognitive Complexity of 10 (exceeds 5 allowed). Consider refactoring.',
path: 'codequality.rb',
severity: 'minor',
urlPath: 'null/codequality.rb#L5',
},
],
resolvedIssues: [
{
description: 'Insecure Dependency',
file_path: 'lib/six.rb',
line: 22,
name: 'Insecure Dependency',
path: 'lib/six.rb',
severity: 'major',
urlPath: 'null/lib/six.rb#L22',
},
],
};
......@@ -5,7 +5,14 @@ import axios from '~/lib/utils/axios_utils';
import * as actions from '~/reports/codequality_report/store/actions';
import * as types from '~/reports/codequality_report/store/mutation_types';
import createStore from '~/reports/codequality_report/store';
import { headIssues, baseIssues, mockParsedHeadIssues, mockParsedBaseIssues } from '../mock_data';
import {
headIssues,
baseIssues,
mockParsedHeadIssues,
mockParsedBaseIssues,
reportIssues,
parsedReportIssues,
} from '../mock_data';
// mock codequality comparison worker
jest.mock('~/reports/codequality_report/workers/codequality_comparison_worker', () =>
......@@ -39,6 +46,7 @@ describe('Codequality Reports actions', () => {
headPath: 'headPath',
baseBlobPath: 'baseBlobPath',
headBlobPath: 'headBlobPath',
reportsPath: 'reportsPath',
helpPath: 'codequalityHelpPath',
};
......@@ -55,68 +63,119 @@ describe('Codequality Reports actions', () => {
describe('fetchReports', () => {
let mock;
let diffFeatureFlagEnabled;
beforeEach(() => {
localState.headPath = `${TEST_HOST}/head.json`;
localState.basePath = `${TEST_HOST}/base.json`;
mock = new MockAdapter(axios);
});
describe('with codequalityMrDiff feature flag enabled', () => {
beforeEach(() => {
diffFeatureFlagEnabled = true;
localState.reportsPath = `${TEST_HOST}/codequality_reports.json`;
mock = new MockAdapter(axios);
});
afterEach(() => {
mock.restore();
});
afterEach(() => {
mock.restore();
});
describe('on success', () => {
it('commits REQUEST_REPORTS and dispatches receiveReportsSuccess', (done) => {
mock.onGet(`${TEST_HOST}/head.json`).reply(200, headIssues);
mock.onGet(`${TEST_HOST}/base.json`).reply(200, baseIssues);
testAction(
actions.fetchReports,
null,
localState,
[{ type: types.REQUEST_REPORTS }],
[
{
payload: {
newIssues: [mockParsedHeadIssues[0]],
resolvedIssues: [mockParsedBaseIssues[0]],
describe('on success', () => {
it('commits REQUEST_REPORTS and dispatches receiveReportsSuccess', (done) => {
mock.onGet(`${TEST_HOST}/codequality_reports.json`).reply(200, reportIssues);
testAction(
actions.fetchReports,
diffFeatureFlagEnabled,
localState,
[{ type: types.REQUEST_REPORTS }],
[
{
payload: parsedReportIssues,
type: 'receiveReportsSuccess',
},
type: 'receiveReportsSuccess',
},
],
done,
);
],
done,
);
});
});
});
describe('on error', () => {
it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
mock.onGet(`${TEST_HOST}/head.json`).reply(500);
testAction(
actions.fetchReports,
null,
localState,
[{ type: types.REQUEST_REPORTS }],
[{ type: 'receiveReportsError' }],
done,
);
describe('on error', () => {
it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
mock.onGet(`${TEST_HOST}/codequality_reports.json`).reply(500);
testAction(
actions.fetchReports,
diffFeatureFlagEnabled,
localState,
[{ type: types.REQUEST_REPORTS }],
[{ type: 'receiveReportsError', payload: expect.any(Error) }],
done,
);
});
});
});
describe('with no base path', () => {
it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
localState.basePath = null;
testAction(
actions.fetchReports,
null,
localState,
[{ type: types.REQUEST_REPORTS }],
[{ type: 'receiveReportsError' }],
done,
);
describe('with codequalityMrDiff feature flag disabled', () => {
beforeEach(() => {
diffFeatureFlagEnabled = false;
localState.headPath = `${TEST_HOST}/head.json`;
localState.basePath = `${TEST_HOST}/base.json`;
mock = new MockAdapter(axios);
});
afterEach(() => {
mock.restore();
});
describe('on success', () => {
it('commits REQUEST_REPORTS and dispatches receiveReportsSuccess', (done) => {
mock.onGet(`${TEST_HOST}/head.json`).reply(200, headIssues);
mock.onGet(`${TEST_HOST}/base.json`).reply(200, baseIssues);
testAction(
actions.fetchReports,
diffFeatureFlagEnabled,
localState,
[{ type: types.REQUEST_REPORTS }],
[
{
payload: {
newIssues: [mockParsedHeadIssues[0]],
resolvedIssues: [mockParsedBaseIssues[0]],
},
type: 'receiveReportsSuccess',
},
],
done,
);
});
});
describe('on error', () => {
it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
mock.onGet(`${TEST_HOST}/head.json`).reply(500);
testAction(
actions.fetchReports,
diffFeatureFlagEnabled,
localState,
[{ type: types.REQUEST_REPORTS }],
[{ type: 'receiveReportsError', payload: expect.any(Error) }],
done,
);
});
});
describe('with no base path', () => {
it('commits REQUEST_REPORTS and dispatches receiveReportsError', (done) => {
localState.basePath = null;
testAction(
actions.fetchReports,
diffFeatureFlagEnabled,
localState,
[{ type: types.REQUEST_REPORTS }],
[{ type: 'receiveReportsError' }],
done,
);
});
});
});
});
......@@ -142,7 +201,7 @@ describe('Codequality Reports actions', () => {
actions.receiveReportsError,
null,
localState,
[{ type: types.RECEIVE_REPORTS_ERROR }],
[{ type: types.RECEIVE_REPORTS_ERROR, payload: null }],
[],
done,
);
......
......@@ -55,6 +55,12 @@ describe('Codequality Reports mutations', () => {
expect(localState.hasError).toEqual(false);
});
it('clears statusReason', () => {
mutations.RECEIVE_REPORTS_SUCCESS(localState, {});
expect(localState.statusReason).toEqual('');
});
it('sets newIssues and resolvedIssues from response data', () => {
const data = { newIssues: [{ id: 1 }], resolvedIssues: [{ id: 2 }] };
mutations.RECEIVE_REPORTS_SUCCESS(localState, data);
......@@ -76,5 +82,13 @@ describe('Codequality Reports mutations', () => {
expect(localState.hasError).toEqual(true);
});
it('sets statusReason to string from error response data', () => {
const data = { status_reason: 'This merge request does not have codequality reports' };
const error = { response: { data } };
mutations.RECEIVE_REPORTS_ERROR(localState, error);
expect(localState.statusReason).toEqual(data.status_reason);
});
});
});
......@@ -2,7 +2,13 @@ import {
parseCodeclimateMetrics,
doCodeClimateComparison,
} from '~/reports/codequality_report/store/utils/codequality_comparison';
import { baseIssues, mockParsedHeadIssues, mockParsedBaseIssues } from '../../mock_data';
import {
baseIssues,
mockParsedHeadIssues,
mockParsedBaseIssues,
reportIssues,
parsedReportIssues,
} from '../../mock_data';
jest.mock('~/reports/codequality_report/workers/codequality_comparison_worker', () => {
let mockPostMessageCallback;
......@@ -34,7 +40,7 @@ describe('Codequality report store utils', () => {
let result;
describe('parseCodeclimateMetrics', () => {
it('should parse the received issues', () => {
it('should parse the issues from codeclimate artifacts', () => {
[result] = parseCodeclimateMetrics(baseIssues, 'path');
expect(result.name).toEqual(baseIssues[0].check_name);
......@@ -42,6 +48,14 @@ describe('Codequality report store utils', () => {
expect(result.line).toEqual(baseIssues[0].location.lines.begin);
});
it('should parse the issues from backend codequality diff', () => {
[result] = parseCodeclimateMetrics(reportIssues.new_errors, 'path');
expect(result.name).toEqual(parsedReportIssues.newIssues[0].name);
expect(result.path).toEqual(parsedReportIssues.newIssues[0].path);
expect(result.line).toEqual(parsedReportIssues.newIssues[0].line);
});
describe('when an issue has no location or path', () => {
const issue = { description: 'Insecure Dependency' };
......
......@@ -135,6 +135,15 @@ RSpec.describe AvatarsHelper do
helper.avatar_icon_for_user(nil, 20, 2)
end
end
context 'for a blocked user' do
let(:user) { create(:user, :blocked) }
it 'returns the default avatar' do
expect(helper.avatar_icon_for_user(user).to_s)
.to eq(helper.default_avatar)
end
end
end
describe '#gravatar_icon' do
......
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