Commit f380bd62 authored by Paul Gascou-Vaillancourt's avatar Paul Gascou-Vaillancourt Committed by Kushal Pandya

Display proper error messages on vulnerabilities fetch failure

- Save the response's error code to the store
- When fetching vulnerabilities fails with one of the supported error
codes (currently 401 & 403), show an empty state with more information
on the error and how to circumvent it
- Update tests
parent d90648aa
......@@ -7,6 +7,7 @@ import SecurityDashboardTable from './security_dashboard_table.vue';
import VulnerabilityChart from './vulnerability_chart.vue';
import VulnerabilityCountList from './vulnerability_count_list.vue';
import VulnerabilitySeverity from './vulnerability_severity.vue';
import LoadingError from './loading_error.vue';
export default {
name: 'SecurityDashboardApp',
......@@ -17,6 +18,7 @@ export default {
VulnerabilityChart,
VulnerabilityCountList,
VulnerabilitySeverity,
LoadingError,
},
props: {
vulnerabilitiesEndpoint: {
......@@ -55,8 +57,9 @@ export default {
},
},
computed: {
...mapState('vulnerabilities', ['modal', 'pageInfo']),
...mapState('vulnerabilities', ['modal', 'pageInfo', 'loadingVulnerabilitiesErrorCode']),
...mapGetters('filters', ['activeFilters']),
...mapGetters('vulnerabilities', ['loadingVulnerabilitiesFailedWithRecognizedErrorCode']),
canCreateIssue() {
const path = this.vulnerability.create_vulnerability_feedback_issue_path;
return Boolean(path);
......@@ -138,48 +141,54 @@ export default {
<template>
<section>
<header>
<filters />
</header>
<loading-error
v-if="loadingVulnerabilitiesFailedWithRecognizedErrorCode"
:error-code="loadingVulnerabilitiesErrorCode"
/>
<template v-else>
<header>
<filters />
</header>
<vulnerability-count-list v-if="shouldShowCountList" class="mb-0" />
<vulnerability-count-list v-if="shouldShowCountList" class="mb-0" />
<div class="row mt-4">
<article class="col" :class="{ 'col-xl-7': !isLockedToProject }">
<security-dashboard-table>
<template #emptyState>
<slot name="emptyState"></slot>
</template>
</security-dashboard-table>
</article>
<div class="row mt-4">
<article class="col" :class="{ 'col-xl-7': !isLockedToProject }">
<security-dashboard-table>
<template #emptyState>
<slot name="emptyState"></slot>
</template>
</security-dashboard-table>
</article>
<aside v-if="shouldShowAside" class="col-xl-5">
<vulnerability-chart v-if="shouldShowChart" class="mb-3" />
<vulnerability-severity
v-if="shouldShowVulnerabilitySeverities"
:endpoint="vulnerableProjectsEndpoint"
/>
</aside>
</div>
<aside v-if="shouldShowAside" class="col-xl-5">
<vulnerability-chart v-if="shouldShowChart" class="mb-3" />
<vulnerability-severity
v-if="shouldShowVulnerabilitySeverities"
:endpoint="vulnerableProjectsEndpoint"
/>
</aside>
</div>
<issue-modal
:modal="modal"
:vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
:can-create-issue="canCreateIssue"
:can-create-merge-request="canCreateMergeRequest"
:can-dismiss-vulnerability="canDismissVulnerability"
@addDismissalComment="addDismissalComment({ vulnerability, comment: $event })"
@editVulnerabilityDismissalComment="openDismissalCommentBox()"
@showDismissalDeleteButtons="showDismissalDeleteButtons"
@hideDismissalDeleteButtons="hideDismissalDeleteButtons"
@deleteDismissalComment="deleteDismissalComment({ vulnerability })"
@closeDismissalCommentBox="closeDismissalCommentBox()"
@createMergeRequest="createMergeRequest({ vulnerability })"
@createNewIssue="createIssue({ vulnerability })"
@dismissVulnerability="dismissVulnerability({ vulnerability, comment: $event })"
@openDismissalCommentBox="openDismissalCommentBox()"
@revertDismissVulnerability="undoDismiss({ vulnerability })"
@downloadPatch="downloadPatch({ vulnerability })"
/>
<issue-modal
:modal="modal"
:vulnerability-feedback-help-path="vulnerabilityFeedbackHelpPath"
:can-create-issue="canCreateIssue"
:can-create-merge-request="canCreateMergeRequest"
:can-dismiss-vulnerability="canDismissVulnerability"
@addDismissalComment="addDismissalComment({ vulnerability, comment: $event })"
@editVulnerabilityDismissalComment="openDismissalCommentBox()"
@showDismissalDeleteButtons="showDismissalDeleteButtons"
@hideDismissalDeleteButtons="hideDismissalDeleteButtons"
@deleteDismissalComment="deleteDismissalComment({ vulnerability })"
@closeDismissalCommentBox="closeDismissalCommentBox()"
@createMergeRequest="createMergeRequest({ vulnerability })"
@createNewIssue="createIssue({ vulnerability })"
@dismissVulnerability="dismissVulnerability({ vulnerability, comment: $event })"
@openDismissalCommentBox="openDismissalCommentBox()"
@revertDismissVulnerability="undoDismiss({ vulnerability })"
@downloadPatch="downloadPatch({ vulnerability })"
/>
</template>
</section>
</template>
<script>
import { GlEmptyState } from '@gitlab/ui';
import { s__, __ } from '~/locale';
import { LOADING_VULNERABILITIES_ERROR_CODES as ERROR_CODES } from '../store/modules/vulnerabilities/constants';
import { imagePath } from '~/lib/utils/common_utils';
const description = s__(
'Security Reports|Security reports can only be accessed by authorized users.',
);
export default {
emptyStatePropsMap: {
[ERROR_CODES.UNAUTHORIZED]: {
title: s__('Security Reports|You must sign in as an authorized user to see this report'),
description,
primaryButtonText: __('Sign in'),
primaryButtonLink: '/users/sign_in',
svgPath: imagePath('illustrations/user-not-logged-in.svg'),
},
[ERROR_CODES.FORBIDDEN]: {
title: s__('Security Reports|You do not have sufficient permissions to access this report'),
description,
svgPath: imagePath('illustrations/lock_promotion.svg'),
},
},
components: {
GlEmptyState,
},
props: {
errorCode: {
type: Number,
required: true,
validator: value => Object.values(ERROR_CODES).includes(value),
},
},
};
</script>
<template>
<gl-empty-state v-bind="$options.emptyStatePropsMap[errorCode]" />
</template>
......@@ -81,8 +81,8 @@ export const fetchVulnerabilities = ({ state, dispatch }, params = {}) => {
const { headers, data } = response;
dispatch('receiveVulnerabilitiesSuccess', { headers, data });
})
.catch(() => {
dispatch('receiveVulnerabilitiesError');
.catch(error => {
dispatch('receiveVulnerabilitiesError', error.response.status);
});
};
......@@ -98,8 +98,8 @@ export const receiveVulnerabilitiesSuccess = ({ commit }, { headers, data }) =>
commit(types.RECEIVE_VULNERABILITIES_SUCCESS, { pageInfo, vulnerabilities });
};
export const receiveVulnerabilitiesError = ({ commit }) => {
commit(types.RECEIVE_VULNERABILITIES_ERROR);
export const receiveVulnerabilitiesError = ({ commit }, errorCode) => {
commit(types.RECEIVE_VULNERABILITIES_ERROR, errorCode);
};
export const openModal = ({ commit }, payload = {}) => {
......
import httpStatusCodes from '~/lib/utils/http_status';
export const CRITICAL = 'critical';
export const HIGH = 'high';
export const MEDIUM = 'medium';
......@@ -10,3 +12,8 @@ export const DAYS = {
SIXTY: 60,
NINETY: 90,
};
export const LOADING_VULNERABILITIES_ERROR_CODES = {
UNAUTHORIZED: httpStatusCodes.UNAUTHORIZED,
FORBIDDEN: httpStatusCodes.FORBIDDEN,
};
import { LOADING_VULNERABILITIES_ERROR_CODES } from './constants';
export const dashboardError = state =>
state.errorLoadingVulnerabilities && state.errorLoadingVulnerabilitiesCount;
export const dashboardListError = state =>
......@@ -5,6 +7,12 @@ export const dashboardListError = state =>
export const dashboardCountError = state =>
!state.errorLoadingVulnerabilities && state.errorLoadingVulnerabilitiesCount;
export const loadingVulnerabilitiesFailedWithRecognizedErrorCode = state =>
state.errorLoadingVulnerabilities &&
Object.values(LOADING_VULNERABILITIES_ERROR_CODES).includes(
state.loadingVulnerabilitiesErrorCode,
);
export const getVulnerabilityHistoryByName = state => name =>
state.vulnerabilitiesHistory[name.toLowerCase()];
......
......@@ -19,15 +19,17 @@ export default {
[types.REQUEST_VULNERABILITIES](state) {
state.isLoadingVulnerabilities = true;
state.errorLoadingVulnerabilities = false;
state.loadingVulnerabilitiesErrorCode = null;
},
[types.RECEIVE_VULNERABILITIES_SUCCESS](state, payload) {
state.isLoadingVulnerabilities = false;
state.pageInfo = payload.pageInfo;
state.vulnerabilities = payload.vulnerabilities;
},
[types.RECEIVE_VULNERABILITIES_ERROR](state) {
[types.RECEIVE_VULNERABILITIES_ERROR](state, errorCode = null) {
state.isLoadingVulnerabilities = false;
state.errorLoadingVulnerabilities = true;
state.loadingVulnerabilitiesErrorCode = errorCode;
},
[types.SET_VULNERABILITIES_COUNT_ENDPOINT](state, payload) {
state.vulnerabilitiesCountEndpoint = payload;
......
......@@ -3,6 +3,7 @@ import { __, s__ } from '~/locale';
export default () => ({
isLoadingVulnerabilities: true,
errorLoadingVulnerabilities: false,
loadingVulnerabilitiesErrorCode: null,
vulnerabilities: [],
isLoadingVulnerabilitiesCount: true,
errorLoadingVulnerabilitiesCount: false,
......
---
title: Display proper error messages on vulnerabilities fetch failure
merge_request: 23812
author:
type: changed
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`LoadingError component with error code 401 empty state has correct props 1`] = `
Object {
"compact": false,
"description": "Security reports can only be accessed by authorized users.",
"primaryButtonLink": "/users/sign_in",
"primaryButtonText": "Sign in",
"secondaryButtonLink": null,
"secondaryButtonText": null,
"svgPath": "/assets/illustrations/user-not-logged-in.svg",
"title": "You must sign in as an authorized user to see this report",
}
`;
exports[`LoadingError component with error code 403 empty state has correct props 1`] = `
Object {
"compact": false,
"description": "Security reports can only be accessed by authorized users.",
"primaryButtonLink": null,
"primaryButtonText": null,
"secondaryButtonLink": null,
"secondaryButtonText": null,
"svgPath": "/assets/illustrations/lock_promotion.svg",
"title": "You do not have sufficient permissions to access this report",
}
`;
......@@ -8,6 +8,7 @@ import SecurityDashboardTable from 'ee/security_dashboard/components/security_da
import VulnerabilityChart from 'ee/security_dashboard/components/vulnerability_chart.vue';
import VulnerabilityCountList from 'ee/security_dashboard/components/vulnerability_count_list.vue';
import VulnerabilitySeverity from 'ee/security_dashboard/components/vulnerability_severity.vue';
import LoadingError from 'ee/security_dashboard/components/loading_error.vue';
import createStore from 'ee/security_dashboard/store';
import { getParameterValues } from '~/lib/utils/url_utility';
......@@ -156,14 +157,9 @@ describe('Security Dashboard app', () => {
describe('dismissed vulnerabilities', () => {
beforeEach(() => {
getParameterValues.mockImplementation(() => [true]);
setup();
});
afterEach(() => {
getParameterValues.mockRestore();
});
it.each`
description | getParameterValuesReturnValue | expected
${'hides dismissed vulnerabilities by default'} | ${[]} | ${true}
......@@ -175,4 +171,25 @@ describe('Security Dashboard app', () => {
expect(wrapper.vm.$store.state.filters.hideDismissed).toBe(expected);
});
});
describe('on error', () => {
beforeEach(() => {
setup();
createComponent();
});
it.each([401, 403])('displays an error on error %s', errorCode => {
store.dispatch('vulnerabilities/receiveVulnerabilitiesError', errorCode);
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.find(LoadingError).exists()).toBe(true);
});
});
it.each([404, 500])('does not display an error on error %s', errorCode => {
store.dispatch('vulnerabilities/receiveVulnerabilitiesError', errorCode);
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.find(LoadingError).exists()).toBe(false);
});
});
});
});
import { shallowMount } from '@vue/test-utils';
import { GlEmptyState } from '@gitlab/ui';
import LoadingError from 'ee/security_dashboard/components/loading_error.vue';
describe('LoadingError component', () => {
let wrapper;
const createWrapper = errorCode => {
wrapper = shallowMount(LoadingError, {
propsData: {
errorCode,
},
});
};
afterEach(() => {
wrapper.destroy();
wrapper = null;
});
describe.each([401, 403])('with error code %s', errorCode => {
beforeEach(() => {
createWrapper(errorCode);
});
it('renders an empty state', () => {
expect(wrapper.find(GlEmptyState).exists()).toBe(true);
});
it('empty state has correct props', () => {
expect(wrapper.find(GlEmptyState).props()).toMatchSnapshot();
});
});
});
......@@ -290,8 +290,10 @@ describe('vulnerabilities actions', () => {
});
describe('on error', () => {
const errorCode = 404;
beforeEach(() => {
mock.onGet(state.vulnerabilitiesEndpoint).replyOnce(404, {});
mock.onGet(state.vulnerabilitiesEndpoint).replyOnce(errorCode, {});
});
it('should dispatch the request and error actions', done => {
......@@ -300,7 +302,10 @@ describe('vulnerabilities actions', () => {
{},
state,
[],
[{ type: 'requestVulnerabilities' }, { type: 'receiveVulnerabilitiesError' }],
[
{ type: 'requestVulnerabilities' },
{ type: 'receiveVulnerabilitiesError', payload: errorCode },
],
done,
);
});
......@@ -337,11 +342,13 @@ describe('vulnerabilities actions', () => {
describe('receiveVulnerabilitiesError', () => {
it('should commit the error mutation', done => {
const errorCode = 403;
testAction(
actions.receiveVulnerabilitiesError,
{},
errorCode,
state,
[{ type: types.RECEIVE_VULNERABILITIES_ERROR }],
[{ type: types.RECEIVE_VULNERABILITIES_ERROR, payload: errorCode }],
[],
done,
);
......
......@@ -53,6 +53,7 @@ describe('vulnerabilities module mutations', () => {
describe('REQUEST_VULNERABILITIES', () => {
beforeEach(() => {
state.errorLoadingVulnerabilities = true;
state.loadingVulnerabilitiesErrorCode = 403;
mutations[types.REQUEST_VULNERABILITIES](state);
});
......@@ -63,6 +64,10 @@ describe('vulnerabilities module mutations', () => {
it('should set `errorLoadingVulnerabilities` to `false`', () => {
expect(state.errorLoadingVulnerabilities).toBeFalsy();
});
it('should reset `loadingVulnerabilitiesErrorCode`', () => {
expect(state.loadingVulnerabilitiesErrorCode).toBe(null);
});
});
describe('RECEIVE_VULNERABILITIES_SUCCESS', () => {
......@@ -90,11 +95,19 @@ describe('vulnerabilities module mutations', () => {
});
describe('RECEIVE_VULNERABILITIES_ERROR', () => {
it('should set `isLoadingVulnerabilities` to `false`', () => {
mutations[types.RECEIVE_VULNERABILITIES_ERROR](state);
const errorCode = 403;
beforeEach(() => {
mutations[types.RECEIVE_VULNERABILITIES_ERROR](state, errorCode);
});
it('should set `isLoadingVulnerabilities` to `false`', () => {
expect(state.isLoadingVulnerabilities).toBeFalsy();
});
it('should set `loadingVulnerabilitiesErrorCode`', () => {
expect(state.loadingVulnerabilitiesErrorCode).toBe(errorCode);
});
});
describe('SET_VULNERABILITIES_COUNT_ENDPOINT', () => {
......
......@@ -16758,6 +16758,9 @@ msgstr ""
msgid "Security Reports|Oops, something doesn't seem right."
msgstr ""
msgid "Security Reports|Security reports can only be accessed by authorized users."
msgstr ""
msgid "Security Reports|There was an error adding the comment."
msgstr ""
......@@ -16782,6 +16785,12 @@ msgstr ""
msgid "Security Reports|Undo dismiss"
msgstr ""
msgid "Security Reports|You do not have sufficient permissions to access this report"
msgstr ""
msgid "Security Reports|You must sign in as an authorized user to see this report"
msgstr ""
msgid "Security configuration help link"
msgstr ""
......
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