Commit 0000d799 authored by Daniel Tian's avatar Daniel Tian Committed by Savas Vedova

Save sort on querystring for vulnerability report

Changelog: added
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80140
EE: true
parent 35b47bf7
......@@ -19,12 +19,11 @@ import FalsePositiveBadge from 'ee/vulnerabilities/components/false_positive_bad
import RemediatedBadge from 'ee/vulnerabilities/components/remediated_badge.vue';
import { VULNERABILITY_STATES } from 'ee/vulnerabilities/constants';
import { formatDate } from '~/lib/utils/datetime_utility';
import { convertToSnakeCase } from '~/lib/utils/text_utility';
import { FIELDS } from 'ee/security_dashboard/components/shared/vulnerability_report/constants';
import AutoFixHelpText from '../auto_fix_help_text.vue';
import IssuesBadge from '../issues_badge.vue';
import SelectionSummary from '../selection_summary.vue';
import VulnerabilityCommentIcon from '../vulnerability_comment_icon.vue';
import { FIELDS } from './constants';
export default {
components: {
......@@ -82,12 +81,18 @@ export default {
type: String,
required: true,
},
sort: {
type: Object,
required: false,
default: () => ({
sortBy: FIELDS.SEVERITY.key,
sortDesc: true,
}),
},
},
data() {
return {
selectedVulnerabilities: {},
sortBy: 'severity',
sortDesc: true,
};
},
computed: {
......@@ -231,14 +236,8 @@ export default {
useConvertReportType(reportType) {
return convertReportType(reportType);
},
handleSortChange(context) {
const fieldName = convertToSnakeCase(context.sortBy);
const direction = context.sortDesc ? 'desc' : 'asc';
if (!fieldName) {
return;
}
this.$emit('sort-changed', `${fieldName}_${direction}`);
handleSortChange({ sortBy, sortDesc }) {
this.$emit('update:sort', { sortBy, sortDesc });
},
getVulnerabilityState(state = '') {
const stateName = state.toLowerCase();
......@@ -267,8 +266,8 @@ export default {
:fields="displayFields"
:items="vulnerabilities"
:thead-class="theadClass"
:sort-desc="sortDesc"
:sort-by="sortBy"
:sort-desc="sort.sortDesc"
:sort-by="sort.sortBy"
sort-icon-left
no-local-sorting
stacked="sm"
......
......@@ -4,7 +4,10 @@ import { produce } from 'immer';
import createFlash from '~/flash';
import { s__ } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { convertToSnakeCase } from '~/lib/utils/text_utility';
import { parseBoolean } from '~/lib/utils/common_utils';
import VulnerabilityList from './vulnerability_list.vue';
import { FIELDS } from './constants';
const PAGE_SIZE = 20;
......@@ -52,7 +55,6 @@ export default {
data() {
return {
vulnerabilities: [],
sort: undefined,
pageInfo: {},
// The "before" querystring value on page load.
initialBefore: this.$route.query.before,
......@@ -67,7 +69,7 @@ export default {
variables() {
return {
fullPath: this.fullPath,
sort: this.sort,
sort: `${convertToSnakeCase(this.sort.sortBy)}_${this.sort.sortDesc ? 'desc' : 'asc'}`,
vetEnabled: this.canViewFalsePositive,
includeExternalIssueLinks: this.hasJiraVulnerabilitiesIntegrationEnabled,
// If we're using "after" we need to use "first", and if we're using "before" we need to
......@@ -124,6 +126,17 @@ export default {
}
},
},
sort: {
get() {
return {
sortBy: this.$route.query.sortBy || FIELDS.SEVERITY.key,
sortDesc: this.$route.query.sortDesc ? parseBoolean(this.$route.query.sortDesc) : true,
};
},
set({ sortBy, sortDesc }) {
this.pushQuerystring({ sortBy, sortDesc });
},
},
// Used to show the infinite scrolling loading spinner.
isLoadingVulnerabilities() {
return this.$apollo.queries.vulnerabilities.loading;
......@@ -151,19 +164,12 @@ export default {
this.vulnerabilities = [];
}
},
sort() {
// Clear out the vulnerabilities so that the skeleton loader is shown.
this.vulnerabilities = [];
},
},
methods: {
updateSort(sort) {
if (this.shouldUsePagination) {
// Reset the paging whenever the sort is changed.
this.resetPaging();
} else {
// Clear out the vulnerabilities so that the skeleton loader is shown.
this.vulnerabilities = [];
}
this.sort = sort;
},
fetchNextPage() {
this.$apollo.queries.vulnerabilities.fetchMore({
variables: { after: this.pageInfo.endCursor },
......@@ -197,9 +203,9 @@ export default {
:is-loading="shouldShowLoadingSkeleton"
:vulnerabilities="vulnerabilities"
:fields="fields"
:sort.sync="sort"
:should-show-project-namespace="showProjectNamespace"
:portal-name="portalName"
@sort-changed="updateSort"
/>
<div v-if="shouldUsePagination" class="gl-text-center gl-mt-6">
......
import Vue, { nextTick } from 'vue';
import VueApollo from 'vue-apollo';
import { GlIntersectionObserver } from '@gitlab/ui';
import VueRouter from 'vue-router';
import VulnerabilityListGraphql from 'ee/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue';
......@@ -10,6 +9,7 @@ import vulnerabilitiesQuery from 'ee/security_dashboard/graphql/queries/group_vu
import createMockApollo from 'helpers/mock_apollo_helper';
import waitForPromises from 'helpers/wait_for_promises';
import createFlash from '~/flash';
import { FIELDS } from 'ee/security_dashboard/components/shared/vulnerability_report/constants';
jest.mock('~/flash');
......@@ -19,6 +19,9 @@ const router = new VueRouter();
const fullPath = 'path';
const portalName = 'portal-name';
// Sort object used by tests that don't need to care about what the values are.
const SORT_OBJECT = { sortBy: 'state', sortDesc: true };
const DEFAULT_SORT = `${FIELDS.SEVERITY.key}_desc`;
const createVulnerabilitiesRequestHandler = ({ hasNextPage }) =>
jest.fn().mockResolvedValue({
......@@ -76,6 +79,10 @@ describe('Vulnerability list GraphQL component', () => {
afterEach(() => {
wrapper.destroy();
vulnerabilitiesRequestHandler.mockClear();
// Reset querystring.
if (Object.keys(router.currentRoute.query).length) {
router.push({ query: undefined });
}
});
describe('vulnerabilities query', () => {
......@@ -140,18 +147,95 @@ describe('Vulnerability list GraphQL component', () => {
});
});
it('calls the vulnerabilities query with the data from the sort-changed event', async () => {
it('calls the GraphQL query with the expected sort data when the vulnerability list changes the sort', async () => {
createWrapper();
// First call should be undefined, which uses the default sort.
vulnerabilitiesRequestHandler.mockClear();
findVulnerabilityList().vm.$emit('update:sort', SORT_OBJECT);
await nextTick();
expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(
expect.objectContaining({ sort: undefined }),
expect.objectContaining({ sort: 'state_desc' }),
);
});
});
const sort = 'sort';
findVulnerabilityList().vm.$emit('sort-changed', sort);
describe('sorting', () => {
it.each`
sortBy | sortDesc | expected
${'state'} | ${true} | ${'state_desc'}
${'detected'} | ${false} | ${'detected_asc'}
${'description'} | ${true} | ${'description_desc'}
${'reportType'} | ${false} | ${'report_type_asc'}
`(
'reads the querystring sort info and calls the GraphQL query with "$expected" for sort',
({ sortBy, sortDesc, expected }) => {
router.push({ query: { sortBy, sortDesc } });
createWrapper();
// This is important; we want the sort info to be read from the querystring from the
// beginning so that the GraphQL request is only done once, instead of starting with the
// default sort, then immediately reading the querystring value, which will trigger the
// GraphQL request twice.
expect(vulnerabilitiesRequestHandler).toHaveBeenCalledTimes(1);
expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(
expect.objectContaining({ sort: expected }),
);
},
);
it(`uses the default sort if there's no querystring data`, () => {
createWrapper();
expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(
expect.objectContaining({ sort: DEFAULT_SORT }),
);
});
it('passes the sort data to the vulnerability list', () => {
router.push({ query: SORT_OBJECT });
createWrapper();
expect(findVulnerabilityList().props('sort')).toEqual(SORT_OBJECT);
});
it('calls the GraphQL query with the expected sort data when the vulnerability list changes the sort', async () => {
createWrapper();
findVulnerabilityList().vm.$emit('update:sort', SORT_OBJECT);
await nextTick();
expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(
expect.objectContaining({ sort: 'state_desc' }),
);
});
it.each`
sortBy | sortDesc
${'state'} | ${true}
${'detected'} | ${false}
${'description'} | ${true}
${'reportType'} | ${false}
`(
'updates the querystring to sortBy = "$sortBy", sortDesc = "$sortDesc" when the vulnerability list changes the sort',
({ sortBy, sortDesc }) => {
createWrapper();
findVulnerabilityList().vm.$emit('update:sort', { sortBy, sortDesc });
expect(router.currentRoute.query).toMatchObject({ sortBy, sortDesc: sortDesc.toString() });
},
);
// This test is for when the querystring is changed by the user clicking forward/back in the
// browser. When a history state is pushed that only changes the querystring, the page does not
// refresh, so we need to handle the case where the user is stepping through the browser history
// but no page refresh is done.
it('calls the GraphQL query with the expected sort data when the querystring is changed', async () => {
createWrapper();
router.push({ query: { sortBy: 'state', sortDesc: true } });
await nextTick();
expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(expect.objectContaining({ sort }));
expect(vulnerabilitiesRequestHandler).toHaveBeenCalledWith(
expect.objectContaining({ sort: 'state_desc' }),
);
});
});
......
......@@ -571,20 +571,24 @@ describe('Vulnerability list component', () => {
});
});
describe('sort-changed listener', () => {
it('does not emit when sort by data is empty', () => {
createWrapper();
findTable().vm.$emit('sort-changed', { sortBy: '', sortDesc: true });
expect(wrapper.emitted('sort-changed')).toBe(undefined);
describe('sorting', () => {
it('passes the sort prop to the table', () => {
const sort = { sortBy: 'a', sortDesc: true };
createWrapper({ stubs: { GlTable: true }, props: { sort } });
expect(findTable().attributes()).toMatchObject({
'sort-by': sort.sortBy,
'sort-desc': sort.sortDesc.toString(),
});
});
it('emits sort by data in expected format', () => {
it('emits sort data in expected format', () => {
createWrapper();
findTable().vm.$emit('sort-changed', { sortBy: 'state', sortDesc: true });
const sort = { sortBy: 'state', sortDesc: true };
findTable().vm.$emit('sort-changed', sort);
expect(wrapper.emitted('sort-changed')[0][0]).toBe('state_desc');
expect(wrapper.emitted('update:sort')[0][0]).toEqual(sort);
});
});
......
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