Merge branch 'revert-6edcb075' into 'master'

Revert 284471-misc-vulnerability-filters-refactor

See merge request gitlab-org/gitlab!61660

ee/app/assets/javascripts/security_dashboard/components/filters/activity_filter.vue

@@ -39,6 +39,8 @@ export default {
         // Toggle the option's existence in the array.
         this.selectedOptions = xor(this.selectedOptions, [option]);
       }
+
+      this.updateQuerystring();
     },
   },
   NO_ACTIVITY,

ee/app/assets/javascripts/security_dashboard/components/filters/scanner_filter.vue

@@ -81,6 +81,8 @@ export default {
       this.selectedOptions = options.every((option) => this.selectedSet.has(option))
         ? without(this.selectedOptions, ...options)
         : union(this.selectedOptions, options);
+
+      this.updateQuerystring();
     },
   },
 };

ee/app/assets/javascripts/security_dashboard/components/filters/standard_filter.vue

@@ -25,45 +25,13 @@ 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);
     },
@@ -82,29 +50,60 @@ 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() {
+      // 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 (this.querystringIds.includes(this.filter.allOption.id)) {
+        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;
+    },
     showSearchBox() {
       return this.options.length >= this.searchBoxShowThreshold;
     },
   },
   watch: {
-    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);
-        }
-      },
+    selectedOptions() {
+      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);

ee/spec/frontend/security_dashboard/components/filters/activity_filter_spec.js

-import { createLocalVue, shallowMount } from '@vue/test-utils';
-import VueRouter from 'vue-router';
+import { shallowMount } from '@vue/test-utils';
 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', () => {
@@ -27,8 +22,6 @@ describe('Activity Filter component', () => {
 
   const createWrapper = () => {
     wrapper = shallowMount(ActivityFilter, {
-      localVue,
-      router,
       propsData: { filter: activityFilter },
     });
   };
@@ -43,10 +36,6 @@ 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', () => {
@@ -74,12 +63,10 @@ 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(3);
-      expect(wrapper.emitted('filter-changed')[2][0]).toStrictEqual({
+      expect(wrapper.emitted('filter-changed')).toHaveLength(2);
+      expect(wrapper.emitted('filter-changed')[1][0]).toStrictEqual({
         hasIssues: undefined,
         hasResolution: undefined,
       });

ee/spec/frontend/security_dashboard/components/filters/scanner_filter_spec.js

@@ -61,8 +61,6 @@ describe('Scanner Filter component', () => {
     if (router.currentRoute.query[filter.id]) {
       router.replace('/');
     }
-
-    wrapper.destroy();
   });
 
   it('shows the correct dropdown items', () => {

ee/spec/frontend/security_dashboard/components/filters/standard_filter_spec.js

@@ -66,9 +66,10 @@ describe('Standard Filter component', () => {
   };
 
   afterEach(() => {
+    wrapper.destroy();
     // Clear out the querystring if one exists, it persists between tests.
-    if (filterQuery()) {
-      router.replace('/');
+    if (router.currentRoute.query[filter.id]) {
+      wrapper.vm.$router.push('/');
     }
   });
 
@@ -117,17 +118,11 @@ describe('Standard Filter component', () => {
   });
 
   describe('loading prop', () => {
-    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(wrapper.emitted('filter-changed')[0][0]).toEqual(query);
-      },
-    );
+    it.each([true, false])(`sets the filter body loading prop to %s`, (loading) => {
+      createWrapper({}, { loading });
+
+      expect(filterBody().props('loading')).toBe(loading);
+    });
   });
 
   describe('selecting options', () => {
@@ -191,8 +186,12 @@ describe('Standard Filter component', () => {
   });
 
   describe('filter querystring', () => {
-    const updateQuerystring = (ids) => {
+    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.
       router.replace({ query: { [filter.id]: ids } });
+      window.dispatchEvent(new Event('popstate'));
+      await wrapper.vm.$nextTick();
     };
 
     describe('clicking on items', () => {
@@ -328,26 +327,13 @@ describe('Standard Filter component', () => {
         const ids = optionIdsAt([1, 2, 3]);
         const other = ['6', '7', '8'];
         const query = { [filter.id]: ids, other };
-        router.replace({ query });
+        router.push({ 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);
-      });
     });
   });
 });