Commit 2964a5c7 authored by Daniel Tian's avatar Daniel Tian Committed by Savas Vedova

Use querystring as source of truth for filters on vulnerability report

parent a309f510
......@@ -39,8 +39,6 @@ export default {
// Toggle the option's existence in the array.
this.selectedOptions = xor(this.selectedOptions, [option]);
}
this.updateQuerystring();
},
},
NO_ACTIVITY,
......
......@@ -81,8 +81,6 @@ export default {
this.selectedOptions = options.every((option) => this.selectedSet.has(option))
? without(this.selectedOptions, ...options)
: union(this.selectedOptions, options);
this.updateQuerystring();
},
},
};
......
......@@ -25,13 +25,45 @@ export default {
data() {
return {
searchTerm: '',
selectedOptions: undefined,
};
},
computed: {
options() {
return this.filter.options;
},
querystringIds() {
const ids = this.$route.query[this.filter.id] || [];
return Array.isArray(ids) ? ids : [ids];
},
selectedOptions: {
get() {
const hasAllId = this.querystringIds.includes(this.filter.allOption.id);
// If the querystring IDs includes the All option, return an empty array. We'll do this even
// if there are other IDs because the special All option takes precedence.
if (hasAllId) {
return [];
}
const options = this.options.filter((x) => this.querystringIds.includes(x.id));
// If the querystring IDs didn't match any options, return the default options.
if (!options.length) {
return this.filter.defaultOptions;
}
return options;
},
set(options) {
const selectedOptions = options.length ? options : [this.filter.allOption];
const ids = selectedOptions.map((x) => x.id);
// To avoid a console error, don't update the querystring if it's the same as the current one.
if (isEqual(this.querystringIds, ids)) {
return;
}
const query = { ...this.$route.query, [this.filter.id]: ids };
this.$router.push({ query });
},
},
selectedSet() {
return new Set(this.selectedOptions);
},
......@@ -50,55 +82,29 @@ export default {
option.name.toLowerCase().includes(this.searchTerm.toLowerCase()),
);
},
querystringIds() {
const ids = this.$route?.query[this.filter.id] || [];
return Array.isArray(ids) ? ids : [ids];
},
querystringOptions() {
const options = this.options.filter((x) => this.querystringIds.includes(x.id));
const hasAllId = this.querystringIds.includes(this.filter.allOption.id);
if (options.length && !hasAllId) {
return options;
}
return hasAllId ? [] : this.filter.defaultOptions;
},
showSearchBox() {
return this.options.length >= this.searchBoxShowThreshold;
},
},
watch: {
selectedOptions() {
this.$emit('filter-changed', this.filterObject);
selectedOptions: {
immediate: true,
handler(newOptions, oldOptions) {
// This check is needed because updating the querystring for one filter will trigger an
// update for all filters, even if the querystring for this filter didn't change.
if (!isEqual(newOptions, oldOptions)) {
this.$emit('filter-changed', this.filterObject);
}
},
},
},
created() {
this.selectedOptions = this.querystringOptions;
// When the user clicks the forward/back browser buttons, update the selected options.
window.addEventListener('popstate', () => {
this.selectedOptions = this.querystringOptions;
});
},
methods: {
toggleOption(option) {
// Toggle the option's existence in the array.
this.selectedOptions = xor(this.selectedOptions, [option]);
this.updateQuerystring();
},
deselectAllOptions() {
this.selectedOptions = [];
this.updateQuerystring();
},
updateQuerystring() {
const options = this.selectedOptionsOrAll.map((x) => x.id);
// To avoid a console error, don't update the querystring if it's the same as the current one.
if (!this.$router || isEqual(this.querystringIds, options)) {
return;
}
const query = { ...this.$route.query, [this.filter.id]: options };
this.$router.push({ query });
},
isSelected(option) {
return this.selectedSet.has(option);
......
......@@ -311,6 +311,7 @@ export default {
@vulnerability-updated="deselectVulnerability"
/>
<gl-table
v-if="filters"
:busy="isLoading"
:fields="fields"
:items="filteredVulnerabilities"
......
import { shallowMount } from '@vue/test-utils';
import { createLocalVue, shallowMount } from '@vue/test-utils';
import VueRouter from 'vue-router';
import ActivityFilter from 'ee/security_dashboard/components/filters/activity_filter.vue';
import { activityFilter, activityOptions } from 'ee/security_dashboard/helpers';
const localVue = createLocalVue();
localVue.use(VueRouter);
const router = new VueRouter();
const { NO_ACTIVITY, WITH_ISSUES, NO_LONGER_DETECTED } = activityOptions;
describe('Activity Filter component', () => {
......@@ -22,6 +27,8 @@ describe('Activity Filter component', () => {
const createWrapper = () => {
wrapper = shallowMount(ActivityFilter, {
localVue,
router,
propsData: { filter: activityFilter },
});
};
......@@ -36,6 +43,10 @@ describe('Activity Filter component', () => {
afterEach(() => {
wrapper.destroy();
// Clear out the querystring if one exists, it persists between tests.
if (router.currentRoute.query[activityFilter.id]) {
router.replace('/');
}
});
it('renders the options', () => {
......@@ -51,7 +62,7 @@ describe('Activity Filter component', () => {
`(
'deselects mutually exclusive options when $expectedOption.id is selected',
async ({ selectedOptions, expectedOption }) => {
await wrapper.setData({ selectedOptions });
await selectedOptions.map(clickItem);
expectSelectedItems(selectedOptions);
......@@ -63,10 +74,12 @@ describe('Activity Filter component', () => {
describe('filter-changed event', () => {
it('contains the correct filterObject for the all option', async () => {
// Click on another option first.
await clickItem(NO_ACTIVITY);
await clickItem(activityFilter.allOption);
expect(wrapper.emitted('filter-changed')).toHaveLength(2);
expect(wrapper.emitted('filter-changed')[1][0]).toStrictEqual({
expect(wrapper.emitted('filter-changed')).toHaveLength(3);
expect(wrapper.emitted('filter-changed')[2][0]).toStrictEqual({
hasIssues: undefined,
hasResolution: undefined,
});
......
......@@ -56,10 +56,12 @@ describe('Scanner Filter component', () => {
});
afterEach(() => {
// Clear out the querystring if one exists. It persists between tests.
if (wrapper.vm.$route.query[filter.id]) {
wrapper.vm.$router.push('/');
wrapper.destroy();
// Clear out the querystring if one exists, it persists between tests.
if (router.currentRoute.query[filter.id]) {
router.replace('/');
}
wrapper.destroy();
});
......
......@@ -66,11 +66,10 @@ describe('Standard Filter component', () => {
};
afterEach(() => {
// Clear out the querystring if one exists. It persists between tests.
// Clear out the querystring if one exists, it persists between tests.
if (filterQuery()) {
wrapper.vm.$router.push('/');
router.replace('/');
}
wrapper.destroy();
});
describe('filter options', () => {
......@@ -118,11 +117,17 @@ describe('Standard Filter component', () => {
});
describe('loading prop', () => {
it.each([true, false])(`sets the filter body loading prop to %s`, (loading) => {
createWrapper({}, { loading });
it.each([true, false])(
`sets the filter body loading prop to %s and emits the expected event data`,
(loading) => {
const query = { [filter.id]: optionIdsAt([1, 3, 5]) };
router.replace({ query });
createWrapper({}, { loading });
expect(filterBody().props('loading')).toBe(loading);
});
expect(filterBody().props('loading')).toBe(loading);
expect(wrapper.emitted('filter-changed')[0][0]).toEqual(query);
},
);
});
describe('selecting options', () => {
......@@ -186,12 +191,8 @@ describe('Standard Filter component', () => {
});
describe('filter querystring', () => {
const updateQuerystring = async (ids) => {
// window.history.back() won't change the location nor fire the popstate event, so we need
// to fake it by doing it manually.
const updateQuerystring = (ids) => {
router.replace({ query: { [filter.id]: ids } });
window.dispatchEvent(new Event('popstate'));
await wrapper.vm.$nextTick();
};
describe('clicking on items', () => {
......@@ -328,12 +329,25 @@ describe('Standard Filter component', () => {
const other = ['6', '7', '8'];
const query = { [filter.id]: ids, other };
router.replace({ query });
window.dispatchEvent(new Event('popstate'));
await wrapper.vm.$nextTick();
expectSelectedItems([1, 2, 3]);
expect(wrapper.vm.$route.query.other).toEqual(other);
});
it('does not emit a filter-changed event if the querystring change was not for the current filter', async () => {
const query = { [filter.id]: optionIdsAt([2, 5]) };
router.replace({ query });
createWrapper();
expect(wrapper.emitted('filter-changed')).toHaveLength(1);
query.other = [1, 2, 3];
router.push({ query });
await wrapper.vm.$nextTick();
expect(wrapper.emitted('filter-changed')).toHaveLength(1);
});
});
});
});
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