fix(filters): don't use indexOf NOT_IN_CONTAINS (#262)

- using indexOf returns true whenever the substring is found within a string and that could cause issue, for example "Task1" will be found in "Task123" but that shouldn't be accepted

packages/common/src/enums/operatorType.enum.ts

@@ -2,7 +2,7 @@ export enum OperatorType {
   /** value is empty */
   empty = '',
 
-  /** value contains x */
+  /** value contains in x (search for substring in the string) */
   contains = 'Contains',
 
   /** value not contains x (inversed of contains) */
@@ -51,7 +51,7 @@ export enum OperatorType {
   notIn = 'NOT_IN',
 
   /**
-   * Find a substring contained inside a collection
+   * Find a substring contained inside a collection, note that it has to be a CSV string.
    * For example, this condition would return True with "IN_CONTAINS":: value='Task2,Task3', collection=['Task2','Task3']
    * This would have returned False with "IN" because 'Task2' does not equal 'Task2,Task3'. However 'Task2' is contained in 'Task2,Task3'
    */

packages/common/src/filter-conditions/__tests__/collectionSearchFilterCondition.spec.ts

@@ -58,15 +58,40 @@ describe('collectionSearchFilterCondition method', () => {
     expect(output).toBe(false);
   });
 
+  it('should return True even when Operator is "Not IN" and the cell value is not in search terms', () => {
+    const options1 = { dataKey: '', operator: 'NIN', cellValue: 'bar', fieldType: FieldType.string, searchTerms: ['foo'] } as FilterConditionOption;
+    const options2 = { dataKey: '', operator: 'NOT_IN', cellValue: 'bar', fieldType: FieldType.string, searchTerms: ['foo'] } as FilterConditionOption;
+
+    const output1 = collectionSearchFilterCondition(options1);
+    const output2 = collectionSearchFilterCondition(options2);
+
+    expect(output1).toBe(true);
+    expect(output2).toBe(true);
+  });
+
   it('should return True when input value contains searchTerms content', () => {
     const options = { dataKey: '', operator: 'IN_CONTAINS', cellValue: 'Task2,Task3', fieldType: FieldType.string, searchTerms: ['Task2', 'Task3'] } as FilterConditionOption;
     const output = collectionSearchFilterCondition(options);
     expect(output).toBe(true);
   });
 
+  it('should return True when input value is not found when using "not in contains" searchTerms content', () => {
+    const options1 = { dataKey: '', operator: 'NIN_CONTAINS', cellValue: 'Task11,Task22,Task33', fieldType: FieldType.string, searchTerms: ['Task1', 'Task2', 'Task3'] } as FilterConditionOption;
+    const options2 = { dataKey: '', operator: 'NOT_IN_CONTAINS', cellValue: 'Task11,Task22,Task33', fieldType: FieldType.string, searchTerms: ['Task1', 'Task2', 'Task3'] } as FilterConditionOption;
+    const output1 = collectionSearchFilterCondition(options1);
+    const output2 = collectionSearchFilterCondition(options2);
+
+    expect(output1).toBe(true);
+    expect(output2).toBe(true);
+  });
+
   it('should return False when input value not in contains searchTerms content', () => {
-    const options = { dataKey: '', operator: 'NIN_CONTAINS', cellValue: 'Task2,Task3', fieldType: FieldType.string, searchTerms: ['Task2', 'Task3'] } as FilterConditionOption;
-    const output = collectionSearchFilterCondition(options);
-    expect(output).toBe(false);
+    const options1 = { dataKey: '', operator: 'NIN_CONTAINS', cellValue: 'Task1,Task3', fieldType: FieldType.string, searchTerms: ['Task1', 'Task2', 'Task3'] } as FilterConditionOption;
+    const options2 = { dataKey: '', operator: 'NOT_IN_CONTAINS', cellValue: 'Task1,Task3', fieldType: FieldType.string, searchTerms: ['Task1', 'Task2', 'Task3'] } as FilterConditionOption;
+    const output1 = collectionSearchFilterCondition(options1);
+    const output2 = collectionSearchFilterCondition(options2);
+
+    expect(output1).toBe(false);
+    expect(output2).toBe(false);
   });
 });

packages/common/src/filter-conditions/__tests__/filterUtilities.spec.ts

@@ -188,7 +188,7 @@ describe('filterUtilities', () => {
     });
 
     it('should return False when value1 is not "IN_CONTAINS" value2 collection', () => {
-      const output = testFilterCondition('IN_CONTAINS', 'Task1,Task4', ['Task2', 'Task3']);
+      const output = testFilterCondition('IN_CONTAINS', 'Task11,Task4', ['Task 1', 'Task2', 'Task3']);
       expect(output).toBeFalsy();
     });
 
@@ -198,8 +198,8 @@ describe('filterUtilities', () => {
     });
 
     it('should return True when value1 is "NOT_IN_CONTAINS" value2 collection', () => {
-      const output1 = testFilterCondition('NIN_CONTAINS', 'Task1,Task4', ['Task2', 'Task3']);
-      const output2 = testFilterCondition('NOT_IN_CONTAINS', 'Task1,Task4', ['Task2', 'Task3']);
+      const output1 = testFilterCondition('NIN_CONTAINS', 'Task11,Task4', ['Task 1', 'Task2', 'Task3']);
+      const output2 = testFilterCondition('NOT_IN_CONTAINS', 'Task11,Task4', ['Task 1', 'Task2', 'Task3']);
 
       expect(output1).toBeTruthy();
       expect(output2).toBeTruthy();

packages/common/src/filter-conditions/executeMappedCondition.ts

@@ -7,15 +7,14 @@ import { stringFilterCondition } from './stringFilterCondition';
 import { FieldType, OperatorType } from '../enums/index';
 import { FilterCondition, FilterConditionOption } from '../interfaces/index';
 import { mapMomentDateFormatWithFieldType } from './../services/utilities';
-import { testFilterCondition } from './filterUtilities';
+import { isCollectionOperator, testFilterCondition } from './filterUtilities';
 import * as moment_ from 'moment-mini';
 
 const moment = moment_['default'] || moment_; // patch to fix rollup "moment has no default export" issue, document here https://github.com/rollup/rollup/issues/670
 
 export const executeMappedCondition: FilterCondition = (options: FilterConditionOption) => {
   // when using a multi-select ('IN' operator) we will not use the field type but instead go directly with a collection search
-  const operator = options && options.operator && options.operator.toUpperCase();
-  if (operator === 'IN' || operator === 'NIN' || operator === 'IN_CONTAINS' || operator === 'NIN_CONTAINS') {
+  if (isCollectionOperator(options.operator)) {
     return collectionSearchFilterCondition(options);
   }
 

packages/common/src/filter-conditions/filterUtilities.ts

@@ -27,35 +27,58 @@ export function compareObjects(o1: any, o2: any, compareKey?: string): boolean {
   return true;
 }
 
+/** Simple check to see if the given Operator is meant to be used with a collection check */
+export function isCollectionOperator(operator: OperatorString): boolean {
+  const inputOperator = operator && operator.toUpperCase() || '';
+  switch (inputOperator) {
+    case 'IN':
+    case 'NIN':
+    case 'NOT_IN':
+    case 'IN_CONTAINS':
+    case 'NIN_CONTAINS':
+    case 'NOT_IN_CONTAINS':
+      return true;
+    default:
+      return false;
+  }
+}
+
 export const testFilterCondition = (operator: OperatorString, value1: any, value2: any): boolean => {
   switch (operator) {
     case '<':
-    case 'LT': return (value1 < value2);
+    case 'LT':
+      return (value1 < value2);
     case '<=':
-    case 'LE': return (value1 <= value2);
+    case 'LE':
+      return (value1 <= value2);
     case '>':
-    case 'GT': return (value1 > value2);
+    case 'GT':
+      return (value1 > value2);
     case '>=':
-    case 'GE': return (value1 >= value2);
+    case 'GE':
+      return (value1 >= value2);
     case '!=':
     case '<>':
-    case 'NE': return (value1 !== value2);
+    case 'NE':
+      return (value1 !== value2);
     case '=':
     case '==':
-    case 'EQ': return (value1 === value2);
-    case 'IN': return ((value2 && value2.indexOf) ? (value2.indexOf(value1) > -1) : false);
+    case 'EQ':
+      return (value1 === value2);
+    case 'IN':
+      return ((value2 && Array.isArray(value2 as string[])) ? (value2.includes(value1)) : false);
     case 'NIN':
     case 'NOT_IN':
-      return ((value2 && value2.includes) ? (!value2.includes(value1)) : false);
+      return ((value2 && Array.isArray(value2 as string[])) ? (!value2.includes(value1)) : false);
     case 'IN_CONTAINS':
-      if (value2 && Array.isArray(value2) && value2.findIndex) {
-        return ((value2.findIndex((val) => value1.indexOf(val) > -1)) > -1);
+      if (value2 && Array.isArray(value2) && typeof value1 === 'string') {
+        return value2.some(item => value1.split(',').includes(item));
       }
       return false;
     case 'NIN_CONTAINS':
     case 'NOT_IN_CONTAINS':
-      if (value2 && Array.isArray(value2) && value2.findIndex) {
-        return !((value2.findIndex((val) => value1.indexOf(val) > -1)) > -1);
+      if (value2 && Array.isArray(value2) && typeof value1 === 'string') {
+        return !value2.some(item => value1.split(',').includes(item));
       }
       return false;
   }