Commit 27d69590 authored by Sam Beckham's avatar Sam Beckham Committed by Paul Slaughter

Move the vuex logic out of the filter component

- Pulls all vuex logic out of the filter component and moves it into
either computed properties, or up to the filters component that
encompasses it.
- Moves the filters spec to Jest
- Removes the store based testing in the filter component
- Adds extra tests around the filter component

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27216
parent 8cf11c3e
<script> <script>
import { mapGetters, mapActions } from 'vuex';
import { GlDropdown, GlSearchBoxByType } from '@gitlab/ui'; import { GlDropdown, GlSearchBoxByType } from '@gitlab/ui';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
...@@ -10,8 +9,8 @@ export default { ...@@ -10,8 +9,8 @@ export default {
Icon, Icon,
}, },
props: { props: {
filterId: { filter: {
type: String, type: Object,
required: true, required: true,
}, },
}, },
...@@ -21,15 +20,17 @@ export default { ...@@ -21,15 +20,17 @@ export default {
}; };
}, },
computed: { computed: {
...mapGetters('filters', ['getFilter', 'getSelectedOptions', 'getSelectedOptionNames']), filterId() {
filter() { return this.filter.id;
return this.getFilter(this.filterId);
}, },
selection() { selection() {
return this.getFilter(this.filterId).selection; return this.filter.selection;
}, },
selectedOptionText() { firstSelectedOption() {
return this.getSelectedOptionNames(this.filterId) || '-'; return this.filter.options.find(option => this.selection.has(option.id))?.name || '-';
},
extraOptionCount() {
return this.selection.size - 1;
}, },
filteredOptions() { filteredOptions() {
return this.filter.options.filter(option => return this.filter.options.filter(option =>
...@@ -41,12 +42,8 @@ export default { ...@@ -41,12 +42,8 @@ export default {
}, },
}, },
methods: { methods: {
...mapActions('filters', ['setFilter']),
clickFilter(option) { clickFilter(option) {
this.setFilter({ this.$emit('setFilter', { filterId: this.filterId, optionId: option.id });
filterId: this.filterId,
optionId: option.id,
});
}, },
isSelected(option) { isSelected(option) {
return this.selection.has(option.id); return this.selection.has(option.id);
...@@ -64,12 +61,11 @@ export default { ...@@ -64,12 +61,11 @@ export default {
<gl-dropdown ref="dropdown" class="d-block mt-1" menu-class="dropdown-extended-height"> <gl-dropdown ref="dropdown" class="d-block mt-1" menu-class="dropdown-extended-height">
<template slot="button-content"> <template slot="button-content">
<span class="text-truncate" :data-qa-selector="qaSelector"> <span class="text-truncate" :data-qa-selector="qaSelector">
{{ selectedOptionText.firstOption }} {{ firstSelectedOption }}
</span> </span>
<span v-if="selectedOptionText.extraOptionCount" class="flex-grow-1 ml-1"> <span v-if="extraOptionCount" class="flex-grow-1 ml-1">
{{ selectedOptionText.extraOptionCount }} {{ n__('+%d more', '+%d more', extraOptionCount) }}
</span> </span>
<i class="fa fa-chevron-down" aria-hidden="true"></i> <i class="fa fa-chevron-down" aria-hidden="true"></i>
</template> </template>
......
<script> <script>
import { mapGetters } from 'vuex'; import { mapGetters, mapActions } from 'vuex';
import DashboardFilter from './filter.vue'; import DashboardFilter from './filter.vue';
import GlToggleVuex from '~/vue_shared/components/gl_toggle_vuex.vue'; import GlToggleVuex from '~/vue_shared/components/gl_toggle_vuex.vue';
...@@ -9,9 +9,10 @@ export default { ...@@ -9,9 +9,10 @@ export default {
GlToggleVuex, GlToggleVuex,
}, },
computed: { computed: {
...mapGetters({ ...mapGetters('filters', ['visibleFilters']),
filters: 'filters/visibleFilters', },
}), methods: {
...mapActions('filters', ['setFilter']),
}, },
}; };
</script> </script>
...@@ -20,10 +21,11 @@ export default { ...@@ -20,10 +21,11 @@ export default {
<div class="dashboard-filters border-bottom bg-gray-light"> <div class="dashboard-filters border-bottom bg-gray-light">
<div class="row mx-0 p-2"> <div class="row mx-0 p-2">
<dashboard-filter <dashboard-filter
v-for="filter in filters" v-for="filter in visibleFilters"
:key="filter.id" :key="filter.id"
class="col-sm-6 col-md-4 col-lg-2 p-2 js-filter" class="col-sm-6 col-md-4 col-lg-2 p-2 js-filter"
:filter-id="filter.id" :filter="filter"
@setFilter="setFilter"
/> />
<div class="ml-lg-auto p-2"> <div class="ml-lg-auto p-2">
<strong>{{ s__('SecurityDashboard|Hide dismissed') }}</strong> <strong>{{ s__('SecurityDashboard|Hide dismissed') }}</strong>
......
import { sprintf, __ } from '~/locale';
import { isBaseFilterOption } from './utils'; import { isBaseFilterOption } from './utils';
export const getFilter = state => filterId => state.filters.find(filter => filter.id === filterId);
export const getSelectedOptions = (state, getters) => filterId => {
const filter = getters.getFilter(filterId);
return filter.options.filter(option => filter.selection.has(option.id));
};
export const getSelectedOptionNames = (state, getters) => filterId => {
const selectedOptions = getters.getSelectedOptions(filterId);
const extraOptionCount = selectedOptions.length - 1;
const firstOption = selectedOptions.map(option => option.name)[0];
return {
firstOption,
extraOptionCount: extraOptionCount
? sprintf(__('+%{extraOptionCount} more'), { extraOptionCount })
: '',
};
};
/** /**
* Loops through all the filters and returns all the active ones * Loops through all the filters and returns all the active ones
* stripping out base filter options. * stripping out base filter options.
......
import Vuex from 'vuex';
import Filter from 'ee/security_dashboard/components/filter.vue'; import Filter from 'ee/security_dashboard/components/filter.vue';
import createStore from 'ee/security_dashboard/store'; import { mount } from '@vue/test-utils';
import { mount, createLocalVue } from '@vue/test-utils';
import stubChildren from 'helpers/stub_children'; import stubChildren from 'helpers/stub_children';
import { trimText } from 'helpers/text_helper';
const localVue = createLocalVue(); const generateOption = index => ({
localVue.use(Vuex); name: `Option ${index}`,
id: `option-${index}`,
});
const generateOptions = length => {
return Array.from({ length }).map((_, i) => generateOption(i));
};
describe('Filter component', () => { describe('Filter component', () => {
let wrapper; let wrapper;
let store;
const createWrapper = propsData => { const createWrapper = propsData => {
wrapper = mount(Filter, { wrapper = mount(Filter, {
...@@ -19,7 +23,6 @@ describe('Filter component', () => { ...@@ -19,7 +23,6 @@ describe('Filter component', () => {
GlSearchBoxByType: false, GlSearchBoxByType: false,
}, },
propsData, propsData,
store,
attachToDocument: true, attachToDocument: true,
}); });
}; };
...@@ -34,37 +37,36 @@ describe('Filter component', () => { ...@@ -34,37 +37,36 @@ describe('Filter component', () => {
return toggleButton.attributes('aria-expanded') === 'true'; return toggleButton.attributes('aria-expanded') === 'true';
} }
function setProjectsCount(count) {
const projects = new Array(count).fill(null).map((_, i) => ({
name: i.toString(),
id: i.toString(),
}));
store.dispatch('filters/setFilterOptions', {
filterId: 'project_id',
options: projects,
});
}
beforeEach(() => {
store = createStore();
});
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
}); });
describe('severity', () => { describe('severity', () => {
let options;
beforeEach(() => { beforeEach(() => {
createWrapper({ filterId: 'severity' }); options = generateOptions(8);
const filter = {
name: 'Severity',
id: 'severity',
options,
selection: new Set([options[0].id, options[1].id, options[2].id]),
};
createWrapper({ filter });
}); });
it('should display all 8 severity options', () => { it('should display all 8 severity options', () => {
expect(dropdownItemsCount()).toEqual(8); expect(dropdownItemsCount()).toEqual(8);
}); });
it('should display a check next to only the selected item', () => { it('should display a check next to only the selected items', () => {
expect(wrapper.findAll('.dropdown-item .js-check').length).toEqual(1); expect(wrapper.findAll('.dropdown-item .js-check').length).toEqual(3);
});
it('should correctly display the selected text', () => {
const selectedText = trimText(wrapper.find('.dropdown-toggle').text());
expect(selectedText).toBe(`${options[0].name} +2 more`);
}); });
it('should display "Severity" as the option name', () => { it('should display "Severity" as the option name', () => {
...@@ -107,11 +109,18 @@ describe('Filter component', () => { ...@@ -107,11 +109,18 @@ describe('Filter component', () => {
describe('Project', () => { describe('Project', () => {
describe('when there are lots of projects', () => { describe('when there are lots of projects', () => {
const lots = 30; const LOTS = 30;
beforeEach(() => { beforeEach(() => {
createWrapper({ filterId: 'project_id', dashboardDocumentation: '' }); const options = generateOptions(LOTS);
setProjectsCount(lots); const filter = {
return wrapper.vm.$nextTick(); name: 'Project',
id: 'project',
options,
selection: new Set([options[0].id]),
};
createWrapper({ filter });
}); });
it('should display a search box', () => { it('should display a search box', () => {
...@@ -119,7 +128,7 @@ describe('Filter component', () => { ...@@ -119,7 +128,7 @@ describe('Filter component', () => {
}); });
it(`should show all projects`, () => { it(`should show all projects`, () => {
expect(dropdownItemsCount()).toBe(lots); expect(dropdownItemsCount()).toBe(LOTS);
}); });
it('should show only matching projects when a search term is entered', () => { it('should show only matching projects when a search term is entered', () => {
......
import createState from 'ee/security_dashboard/store/modules/filters/state'; import createState from 'ee/security_dashboard/store/modules/filters/state';
import * as getters from 'ee/security_dashboard/store/modules/filters/getters'; import * as getters from 'ee/security_dashboard/store/modules/filters/getters';
import { BASE_FILTERS } from 'ee/security_dashboard/store/modules/filters/constants';
describe('filters module getters', () => { describe('filters module getters', () => {
const mockedGetters = state => {
const getFilter = filterId => getters.getFilter(state)(filterId);
const getSelectedOptions = filterId =>
getters.getSelectedOptions(state, { getFilter })(filterId);
return {
getFilter,
getSelectedOptions,
};
};
let state; let state;
beforeEach(() => { beforeEach(() => {
state = createState(); state = createState();
}); });
describe('getFilter', () => {
it('should return the type filter information', () => {
const typeFilter = getters.getFilter(state)('report_type');
expect(typeFilter.name).toEqual('Report type');
});
});
describe('getSelectedOptions', () => {
describe('with one selected option', () => {
it('should return the base filter as the selected option', () => {
const selectedOptions = getters.getSelectedOptions(state, mockedGetters(state))(
'report_type',
);
expect(selectedOptions).toHaveLength(1);
expect(selectedOptions[0].name).toBe(BASE_FILTERS.report_type.name);
});
});
describe('with multiple selected options', () => {
it('should return both "High" and "Critical" ', () => {
state = {
filters: [
{
id: 'severity',
options: [{ id: 'critical' }, { id: 'high' }],
selection: new Set(['critical', 'high']),
},
],
};
const selectedOptions = getters.getSelectedOptions(state, mockedGetters(state))('severity');
expect(selectedOptions).toHaveLength(2);
});
});
});
describe('getSelectedOptionNames', () => {
it('should return the base filter as the selected option', () => {
const selectedOptionNames = getters.getSelectedOptionNames(state, mockedGetters(state))(
'severity',
);
expect(selectedOptionNames.firstOption).toBe(BASE_FILTERS.severity.name);
expect(selectedOptionNames.extraOptionCount).toBe('');
});
it('should return the correct message when multiple filters are selected', () => {
state = {
filters: [
{
id: 'severity',
options: [{ name: 'Critical', id: 1 }, { name: 'High', id: 2 }],
selection: new Set([1, 2]),
},
],
};
const selectedOptionNames = getters.getSelectedOptionNames(state, mockedGetters(state))(
'severity',
);
expect(selectedOptionNames).toEqual({ firstOption: 'Critical', extraOptionCount: '+1 more' });
});
});
describe('activeFilters', () => { describe('activeFilters', () => {
it('should return no severity filters', () => { it('should return no severity filters', () => {
const activeFilters = getters.activeFilters(state, mockedGetters(state)); const activeFilters = getters.activeFilters(state);
expect(activeFilters.severity).toHaveLength(0); expect(activeFilters.severity).toHaveLength(0);
}); });
...@@ -99,7 +22,7 @@ describe('filters module getters', () => { ...@@ -99,7 +22,7 @@ describe('filters module getters', () => {
selection: new Set(['one', 'two']), selection: new Set(['one', 'two']),
}; };
state.filters.push(dummyFilter); state.filters.push(dummyFilter);
const activeFilters = getters.activeFilters(state, mockedGetters(state)); const activeFilters = getters.activeFilters(state);
expect(activeFilters.dummy).toHaveLength(2); expect(activeFilters.dummy).toHaveLength(2);
}); });
......
...@@ -581,10 +581,12 @@ msgstr "" ...@@ -581,10 +581,12 @@ msgstr ""
msgid "+ %{numberOfHiddenAssignees} more" msgid "+ %{numberOfHiddenAssignees} more"
msgstr "" msgstr ""
msgid "+%{approvers} more approvers" msgid "+%d more"
msgstr "" msgid_plural "+%d more"
msgstr[0] ""
msgstr[1] ""
msgid "+%{extraOptionCount} more" msgid "+%{approvers} more approvers"
msgstr "" msgstr ""
msgid "+%{tags} more" msgid "+%{tags} more"
......
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