From 57f1b34990371da3e700f21f57cf922506d415ba Mon Sep 17 00:00:00 2001
From: Savas Vedova <svedova@gitlab.com>
Date: Wed, 17 May 2023 11:49:05 +0000
Subject: [PATCH] Migrate status filter to collapsible listbox
Use GlCollapsibleListbox instead of the deprecated GlDropdown component.
Changelog: changed
EE: true
---
.../shared/filters/status_filter.vue | 94 ++++++++----------
.../components/shared/filters/utils.js | 4 +-
.../shared/filters/status_filter_spec.js | 95 +++++--------------
.../components/shared/filters/utils_spec.js | 7 ++
4 files changed, 74 insertions(+), 126 deletions(-)
diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/filters/status_filter.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/filters/status_filter.vue
index d0528c7289500..44cc6755e1bdb 100644
--- a/ee/app/assets/javascripts/security_dashboard/components/shared/filters/status_filter.vue
+++ b/ee/app/assets/javascripts/security_dashboard/components/shared/filters/status_filter.vue
@@ -1,71 +1,69 @@
<script>
-import { GlDropdown } from '@gitlab/ui';
-import { xor } from 'lodash';
+import { GlCollapsibleListbox } from '@gitlab/ui';
import { s__ } from '~/locale';
import { VULNERABILITY_STATE_OBJECTS } from 'ee/vulnerabilities/constants';
-import DropdownButtonText from './dropdown_button_text.vue';
import QuerystringSync from './querystring_sync.vue';
-import FilterItem from './filter_item.vue';
import { ALL_ID } from './constants';
+import { getSelectedOptionsText } from './utils';
const { detected, confirmed, dismissed, resolved } = VULNERABILITY_STATE_OBJECTS;
// For backwards compatibility with existing bookmarks, the ID needs to be capitalized.
export const DROPDOWN_OPTIONS = [
- { id: detected.state.toUpperCase(), text: detected.buttonText },
- { id: confirmed.state.toUpperCase(), text: confirmed.buttonText },
- { id: dismissed.state.toUpperCase(), text: dismissed.buttonText },
- { id: resolved.state.toUpperCase(), text: resolved.buttonText },
+ { value: ALL_ID, text: s__('SecurityReports|All statuses') },
+ { value: detected.state.toUpperCase(), text: detected.buttonText },
+ { value: confirmed.state.toUpperCase(), text: confirmed.buttonText },
+ { value: dismissed.state.toUpperCase(), text: dismissed.buttonText },
+ { value: resolved.state.toUpperCase(), text: resolved.buttonText },
];
-export const VALID_IDS = [ALL_ID, ...DROPDOWN_OPTIONS.map(({ id }) => id)];
-export const DEFAULT_IDS = [detected.state.toUpperCase(), confirmed.state.toUpperCase()];
+export const VALID_IDS = DROPDOWN_OPTIONS.map(({ value }) => value);
+export const DEFAULT_IDS = [VALID_IDS[1], VALID_IDS[2]];
export default {
- components: { GlDropdown, DropdownButtonText, QuerystringSync, FilterItem },
+ components: {
+ GlCollapsibleListbox,
+ QuerystringSync,
+ },
data: () => ({
selected: DEFAULT_IDS,
}),
computed: {
- selectedIds() {
- return this.selected.length ? this.selected : [ALL_ID];
- },
- selectedItemTexts() {
- const options = DROPDOWN_OPTIONS.filter(({ id }) => this.selected.includes(id));
- // Return the text for selected items, or all items if nothing is selected.
- return options.length ? options.map(({ text }) => text) : [this.$options.i18n.allItemsText];
+ toggleText() {
+ return getSelectedOptionsText(DROPDOWN_OPTIONS, this.selected);
},
},
watch: {
selected: {
immediate: true,
handler() {
- this.$emit('filter-changed', { state: this.selected });
+ this.$emit('filter-changed', {
+ state: this.selected.filter((value) => value !== ALL_ID),
+ });
},
},
},
methods: {
- deselectAll() {
- this.selected = [];
- },
- toggleSelected(id) {
- this.selected = xor(this.selected, [id]);
+ updateSelected(selected) {
+ if (selected.length <= 0 || selected.at(-1) === ALL_ID) {
+ this.selected = [ALL_ID];
+ } else {
+ this.selected = selected.filter((value) => value !== ALL_ID);
+ }
},
- setSelected(ids) {
- if (ids.includes(ALL_ID)) {
- this.selected = [];
- } else if (!ids.length) {
- this.selected = DEFAULT_IDS;
+ updateSelectedFromQS(selected) {
+ if (selected.includes(ALL_ID)) {
+ this.selected = [ALL_ID];
+ } else if (selected.length > 0) {
+ this.selected = selected;
} else {
- this.selected = ids;
+ this.selected = DEFAULT_IDS;
}
},
},
i18n: {
label: s__('SecurityReports|Status'),
- allItemsText: s__('SecurityReports|All statuses'),
},
DROPDOWN_OPTIONS,
VALID_IDS,
- ALL_ID,
};
</script>
@@ -73,29 +71,19 @@ export default {
<div>
<querystring-sync
querystring-key="state"
- :value="selectedIds"
+ :value="selected"
:valid-values="$options.VALID_IDS"
- @input="setSelected"
+ @input="updateSelectedFromQS"
/>
<label class="gl-mb-2">{{ $options.i18n.label }}</label>
- <gl-dropdown :header-text="$options.i18n.label" block toggle-class="gl-mb-0">
- <template #button-text>
- <dropdown-button-text :items="selectedItemTexts" :name="$options.i18n.label" />
- </template>
- <filter-item
- :is-checked="!selected.length"
- :text="$options.i18n.allItemsText"
- :data-testid="$options.ALL_ID"
- @click="deselectAll"
- />
- <filter-item
- v-for="{ id, text } in $options.DROPDOWN_OPTIONS"
- :key="id"
- :data-testid="id"
- :is-checked="selected.includes(id)"
- :text="text"
- @click="toggleSelected(id)"
- />
- </gl-dropdown>
+ <gl-collapsible-listbox
+ :header-text="$options.i18n.label"
+ block
+ multiple
+ :items="$options.DROPDOWN_OPTIONS"
+ :selected="selected"
+ :toggle-text="toggleText"
+ @select="updateSelected"
+ />
</div>
</template>
diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/filters/utils.js b/ee/app/assets/javascripts/security_dashboard/components/shared/filters/utils.js
index 51e83e2a47bfc..213eca941860d 100644
--- a/ee/app/assets/javascripts/security_dashboard/components/shared/filters/utils.js
+++ b/ee/app/assets/javascripts/security_dashboard/components/shared/filters/utils.js
@@ -9,12 +9,12 @@ import { n__ } from '~/locale';
* - If more than one option is selected, returns the text of the first option
* followed by the text "+X more", where X is the number of additional selected options
*
- * @param {Array<{ id: number | string }>} options
+ * @param {Array<{ id: number | string, value: string }>} options
* @param {Array<{ id: number | string }>} selected
* @returns {String}
*/
export const getSelectedOptionsText = (options, selected, placeholder = '') => {
- const selectedOptions = options.filter(({ id }) => selected.includes(id));
+ const selectedOptions = options.filter(({ id, value }) => selected.includes(id || value));
if (selectedOptions.length === 0) {
return placeholder;
diff --git a/ee/spec/frontend/security_dashboard/components/shared/filters/status_filter_spec.js b/ee/spec/frontend/security_dashboard/components/shared/filters/status_filter_spec.js
index 1f00469f64c0e..d81452a343221 100644
--- a/ee/spec/frontend/security_dashboard/components/shared/filters/status_filter_spec.js
+++ b/ee/spec/frontend/security_dashboard/components/shared/filters/status_filter_spec.js
@@ -1,4 +1,4 @@
-import { GlDropdown } from '@gitlab/ui';
+import { GlCollapsibleListbox } from '@gitlab/ui';
import { nextTick } from 'vue';
import StatusFilter, {
DEFAULT_IDS,
@@ -7,11 +7,9 @@ import StatusFilter, {
} from 'ee/security_dashboard/components/shared/filters/status_filter.vue';
import { ALL_ID } from 'ee/security_dashboard/components/shared/filters/constants';
import QuerystringSync from 'ee/security_dashboard/components/shared/filters/querystring_sync.vue';
-import DropdownButtonText from 'ee/security_dashboard/components/shared/filters/dropdown_button_text.vue';
-import FilterItem from 'ee/security_dashboard/components/shared/filters/filter_item.vue';
import { mountExtended } from 'helpers/vue_test_utils_helper';
-const OPTION_IDS = DROPDOWN_OPTIONS.map(({ id }) => id);
+const OPTION_IDS = DROPDOWN_OPTIONS.map(({ value }) => value);
describe('Status Filter component', () => {
let wrapper;
@@ -23,22 +21,13 @@ describe('Status Filter component', () => {
};
const findQuerystringSync = () => wrapper.findComponent(QuerystringSync);
- const findDropdownItems = () => wrapper.findAllComponents(FilterItem);
- const findDropdownItem = (id) => wrapper.findByTestId(id);
+ const findListbox = () => wrapper.findComponent(GlCollapsibleListbox);
- const clickDropdownItem = async (id) => {
- findDropdownItem(id).vm.$emit('click');
+ const clickDropdownItem = async (...ids) => {
+ findListbox().vm.$emit('select', [...ids]);
await nextTick();
};
- const expectSelectedItems = (ids) => {
- const checkedItems = findDropdownItems()
- .wrappers.filter((item) => item.props('isChecked'))
- .map((item) => item.attributes('data-testid'));
-
- expect(checkedItems).toEqual(ids);
- };
-
beforeEach(() => {
createWrapper();
});
@@ -61,13 +50,12 @@ describe('Status Filter component', () => {
it.each`
emitted | expected
${['CONFIRMED', 'RESOLVED']} | ${['CONFIRMED', 'RESOLVED']}
- ${[]} | ${DEFAULT_IDS}
${[ALL_ID]} | ${[ALL_ID]}
`('restores selected items - $emitted', async ({ emitted, expected }) => {
findQuerystringSync().vm.$emit('input', emitted);
await nextTick();
- expectSelectedItems(expected);
+ expect(findListbox().props('selected')).toEqual(expected);
});
});
@@ -77,92 +65,57 @@ describe('Status Filter component', () => {
});
it('shows the dropdown with correct header text', () => {
- expect(wrapper.findComponent(GlDropdown).props('headerText')).toBe(StatusFilter.i18n.label);
+ expect(findListbox().props('headerText')).toBe(StatusFilter.i18n.label);
});
- it('shows the DropdownButtonText component with the correct props', () => {
- expect(wrapper.findComponent(DropdownButtonText).props()).toMatchObject({
- items: ['Needs triage', 'Confirmed'],
- name: StatusFilter.i18n.label,
- });
+ it('shows the placeholder correctly', async () => {
+ await clickDropdownItem('CONFIRMED', 'RESOLVED');
+ expect(findListbox().props('toggleText')).toBe('Confirmed +1 more');
});
});
describe('dropdown items', () => {
it('shows all dropdown items with correct text', () => {
- expect(findDropdownItems()).toHaveLength(DROPDOWN_OPTIONS.length + 1);
-
- expect(findDropdownItem(ALL_ID).text()).toBe(StatusFilter.i18n.allItemsText);
- DROPDOWN_OPTIONS.forEach(({ id, text }) => {
- expect(findDropdownItem(id).text()).toBe(text);
- });
- });
-
- it('allows multiple items to be selected', async () => {
- const ids = [];
- // Deselect everything to begin with.
- clickDropdownItem(ALL_ID);
-
- for await (const id of OPTION_IDS) {
- await clickDropdownItem(id);
- ids.push(id);
-
- expectSelectedItems(ids);
- }
+ expect(findListbox().props('items')).toEqual(DROPDOWN_OPTIONS);
});
it('toggles the item selection when clicked on', async () => {
- // Deselect everything to begin with.
- clickDropdownItem(ALL_ID);
-
- for await (const id of OPTION_IDS) {
- await clickDropdownItem(id);
-
- expectSelectedItems([id]);
-
- await clickDropdownItem(id);
-
- expectSelectedItems([ALL_ID]);
- }
+ await clickDropdownItem('CONFIRMED', 'RESOLVED');
+ expect(findListbox().props('selected')).toEqual(['CONFIRMED', 'RESOLVED']);
+ await clickDropdownItem('DETECTED');
+ expect(findListbox().props('selected')).toEqual(['DETECTED']);
});
it('selects default items when created', () => {
- expectSelectedItems(DEFAULT_IDS);
+ expect(findListbox().props('selected')).toEqual(DEFAULT_IDS);
});
it('selects ALL item and deselects everything else when it is clicked', async () => {
await clickDropdownItem(ALL_ID);
- await clickDropdownItem(ALL_ID); // Click again to verify that it doesn't toggle.
-
- expectSelectedItems([ALL_ID]);
+ expect(findListbox().props('selected')).toEqual([ALL_ID]);
});
it('deselects the ALL item when another item is clicked', async () => {
- await clickDropdownItem(ALL_ID);
- await clickDropdownItem(OPTION_IDS[0]);
-
- expectSelectedItems([OPTION_IDS[0]]);
+ await clickDropdownItem(ALL_ID, 'CONFIRMED');
+ expect(findListbox().props('selected')).toEqual(['CONFIRMED']);
});
});
describe('filter-changed event', () => {
it('emits filter-changed event with default IDs when created', () => {
- expect(wrapper.emitted('filter-changed')[0][0].state).toBe(DEFAULT_IDS);
+ expect(wrapper.emitted('filter-changed')[0][0].state).toEqual(DEFAULT_IDS);
});
it('emits filter-changed event when selected item is changed', async () => {
- const ids = [];
- // Deselect everything to begin with.
await clickDropdownItem(ALL_ID);
expect(wrapper.emitted('filter-changed')[1][0].state).toEqual([]);
- for await (const id of OPTION_IDS) {
- await clickDropdownItem(id);
- ids.push(id);
+ await clickDropdownItem(...OPTION_IDS);
- expect(wrapper.emitted('filter-changed')[ids.length + 1][0].state).toEqual(ids);
- }
+ expect(wrapper.emitted('filter-changed')[2][0].state).toEqual(
+ OPTION_IDS.filter((id) => id !== ALL_ID),
+ );
});
});
});
diff --git a/ee/spec/frontend/security_dashboard/components/shared/filters/utils_spec.js b/ee/spec/frontend/security_dashboard/components/shared/filters/utils_spec.js
index dfc68d01ec922..192fb228d0e2c 100644
--- a/ee/spec/frontend/security_dashboard/components/shared/filters/utils_spec.js
+++ b/ee/spec/frontend/security_dashboard/components/shared/filters/utils_spec.js
@@ -28,6 +28,13 @@ describe('getSelectedOptionsText', () => {
expect(getSelectedOptionsText(options, selected)).toBe('first');
});
+ it('should also work with the value property', () => {
+ const options = [{ value: 1, text: 'first' }];
+ const selected = [options[0].value];
+
+ expect(getSelectedOptionsText(options, selected)).toBe('first');
+ });
+
it.each`
options | expectedText
${[{ id: 1, text: 'first' }, { id: 2, text: 'second' }]} | ${'first +1 more'}
--
GitLab