Fix filters comparison (#363)

* Fix filters comparison

Filters comparison was broken in different ways:
- Same filters with different ordering would be seen as different
- Same filters with different *values* ordering would be seen as
  different
- Same filters with different display info would be seen as different
  (when the page is refreshed, the display info on values isn't set)

This commit fixes all of that, and adds tests for each of these
scenarios

* Add missing data-test

web/src/components/filters/filters-chips.tsx

@@ -4,7 +4,7 @@ import * as _ from 'lodash';
 import * as React from 'react';
 import { useTranslation } from 'react-i18next';
 import { navigate } from '../dynamic-loader/dynamic-loader';
-import { Filter, Filters, hasEnabledFilterValues, removeFromFilters } from '../../model/filters';
+import { Filter, Filters, filtersEqual, hasEnabledFilterValues, removeFromFilters } from '../../model/filters';
 import { QuickFilter } from '../../model/quick-filters';
 import { autoCompleteCache } from '../../utils/autocomplete-cache';
 import { getPathWithParams, netflowTrafficPath } from '../../utils/url';
@@ -52,7 +52,7 @@ export const FiltersChips: React.FC<FiltersChipsProps> = ({
   if (_.isEmpty(chipFilters) && _.isEmpty(defaultFilters)) {
     return null;
   }
-  const isDefaultFilters = _.isEqual(chipFilters, defaultFilters);
+  const isDefaultFilters = filtersEqual(chipFilters, defaultFilters);
   const canSwap = canSwapFilters(chipFilters!);
 
   return (

web/src/components/netflow-traffic.tsx

@@ -37,6 +37,7 @@ import {
   DisabledFilters,
   Filter,
   Filters,
+  filtersEqual,
   getDisabledFiltersRecord,
   getEnabledFilters,
   hasIndexFields,
@@ -1033,7 +1034,7 @@ export const NetflowTraffic: React.FC<{
             id: 'reset-filters',
             label: resetText,
             onClick: resetDefaultFilters,
-            enabled: defFilters.length > 0 && !_.isEqual(filters, defFilters)
+            enabled: defFilters.length > 0 && !filtersEqual(filters.list, defFilters)
           },
           {
             id: 'clear-all-filters',

web/src/components/overflow/links-overflow.tsx

@@ -77,7 +77,7 @@ export const LinksOverflow: React.FC<LinksOverflowProps> = ({ id, items }) => {
           }
           isOpen={isOpen}
           dropdownItems={enabledItems.map(item => (
-            <DropdownItem key={item.id} onClick={item.onClick}>
+            <DropdownItem key={item.id} onClick={item.onClick} data-test={item.id + '-button'}>
               {item.label}
             </DropdownItem>
           ))}

web/src/model/__tests__/filters.spec.ts

@@ -1,5 +1,5 @@
 import { findFilter } from '../../utils/filter-definitions';
-import { doesIncludeFilter, Filter } from '../filters';
+import { doesIncludeFilter, Filter, filtersEqual } from '../filters';
 
 describe('doesIncludeFilter', () => {
   const srcNameFilter = findFilter((a: string) => a, 'src_name')!;
@@ -63,3 +63,90 @@ describe('doesIncludeFilter', () => {
     expect(isIncluded).toBeTruthy();
   });
 });
+
+describe('filtersEqual', () => {
+  const f1 = findFilter((a: string) => a, 'src_name')!;
+  const f2 = findFilter((a: string) => a, 'dst_name')!;
+  const values1 = [{ v: 'abc' }, { v: 'def' }];
+  const values2 = [{ v: 'def' }, { v: 'abc' }];
+  const values3 = [{ v: 'abc' }, { v: 'def', display: 'def' }];
+  const values4 = [{ v: 'abc' }];
+
+  it('should be equal with same order', () => {
+    const list1: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f2, not: true, values: values1 }
+    ];
+    const list2: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f2, not: true, values: values1 }
+    ];
+    expect(filtersEqual(list1, list2)).toBe(true);
+    expect(filtersEqual(list2, list1)).toBe(true);
+  });
+
+  it('should be equal with different order', () => {
+    const list1: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f2, not: true, values: values1 }
+    ];
+    const list2: Filter[] = [
+      { def: f2, not: true, values: values1 },
+      { def: f1, not: false, values: values1 }
+    ];
+    expect(filtersEqual(list1, list2)).toBe(true);
+    expect(filtersEqual(list2, list1)).toBe(true);
+  });
+
+  it('should be equal with different values order', () => {
+    const list1: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f2, not: true, values: values1 }
+    ];
+    const list2: Filter[] = [
+      { def: f1, not: false, values: values2 },
+      { def: f2, not: true, values: values2 }
+    ];
+    expect(filtersEqual(list1, list2)).toBe(true);
+    expect(filtersEqual(list2, list1)).toBe(true);
+  });
+
+  it('should be equal with different values display', () => {
+    const list1: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f2, not: true, values: values1 }
+    ];
+    const list2: Filter[] = [
+      { def: f1, not: false, values: values3 },
+      { def: f2, not: true, values: values3 }
+    ];
+    expect(filtersEqual(list1, list2)).toBe(true);
+    expect(filtersEqual(list2, list1)).toBe(true);
+  });
+
+  it('should differ with different keys', () => {
+    const list1: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f2, not: true, values: values1 }
+    ];
+    const list2: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f1, not: true, values: values1 }
+    ];
+    expect(filtersEqual(list1, list2)).toBe(false);
+    expect(filtersEqual(list2, list1)).toBe(false);
+  });
+
+  it('should differ with different values', () => {
+    const list1: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f2, not: true, values: values1 }
+    ];
+    const list2: Filter[] = [
+      { def: f1, not: false, values: values1 },
+      { def: f2, not: true, values: values4 }
+    ];
+    expect(filtersEqual(list1, list2)).toBe(false);
+    expect(filtersEqual(list2, list1)).toBe(false);
+  });
+});

web/src/model/filters.ts

@@ -1,4 +1,5 @@
 import _ from 'lodash';
+import { isEqual } from '../utils/base-compare';
 
 export type FiltersEncoder = (values: FilterValue[], matchAny: boolean, not: boolean) => string;
 
@@ -142,17 +143,34 @@ export const hasNonIndexFields = (filters: Filter[]) => {
 type FilterKey = Omit<Filter, 'values'>;
 
 export const findFromFilters = (activeFilters: Filter[], search: FilterKey): Filter | undefined => {
-  return activeFilters.find(f => filtersEqual(f, search));
+  return activeFilters.find(f => filterKeyEqual(f, search));
 };
 
 export const removeFromFilters = (activeFilters: Filter[], search: FilterKey): Filter[] => {
-  return activeFilters.filter(f => !filtersEqual(f, search));
+  return activeFilters.filter(f => !filterKeyEqual(f, search));
 };
 
-export const filtersEqual = (f1: FilterKey, f2: FilterKey): boolean => {
+export const filterKeyEqual = (f1: FilterKey, f2: FilterKey): boolean => {
   return f1.def.id === f2.def.id && f1.not == f2.not;
 };
 
+type ComparableFilter = { key: string; values: string[] };
+
+const comparableFilter = (f: Filter): ComparableFilter => {
+  return {
+    key: f.def.id + (f.not ? '!' : ''),
+    values: f.values.map(v => v.v).sort()
+  };
+};
+
+const comparableFilters = (f: Filter[]) => {
+  return f.map(comparableFilter).sort((f1, f2) => f1.key.localeCompare(f2.key));
+};
+
+export const filtersEqual = (f1: Filter[], f2: Filter[]): boolean => {
+  return isEqual(comparableFilters(f1), comparableFilters(f2));
+};
+
 export const doesIncludeFilter = (activeFilters: Filter[], search: FilterKey, values: FilterValue[]): boolean => {
   const found = findFromFilters(activeFilters, search);
   if (found) {

web/src/model/flow-query.ts

@@ -1,4 +1,4 @@
-import { Filter, Filters, filtersEqual } from './filters';
+import { Filter, Filters, filterKeyEqual } from './filters';
 import { swapFilters } from '../components/filters/filters-helper';
 
 export type Reporter = 'source' | 'destination' | 'both';
@@ -93,7 +93,7 @@ const determineOverlap = (orig: Filter[], swapped: Filter[]): { overlaps: Filter
   const overlaps: Filter[] = [];
   orig.forEach(o => {
     if (o.def.overlap) {
-      const valuesFromSwapped = swapped.find(s => filtersEqual(o, s))?.values.map(v => v.v);
+      const valuesFromSwapped = swapped.find(s => filterKeyEqual(o, s))?.values.map(v => v.v);
       const overlap: Filter = valuesFromSwapped
         ? {
             def: o.def,

web/src/utils/base-compare.ts

@@ -1,3 +1,5 @@
+import _ from 'lodash';
+
 export const compareNumbers = (a: number, b: number) => {
   if (!isNaN(a) && !isNaN(b)) {
     return a - b;
@@ -15,3 +17,6 @@ export const compareStrings = (a: string, b: string) => {
   }
   return -1;
 };
+
+// isEqual with type assertion (lodash uses "any")
+export const isEqual = <T>(a: T, b: T) => _.isEqual(a, b);