Skip to content

Commit

Permalink
fix(filters): string <> should be Not Contains instead of Not Equal (#…
Browse files Browse the repository at this point in the history
…276)

* fix(filters): string <> should be Not Contains instead of Not Equal
- prefixing a text search with `<>` should be equivalent to "Not Contains" instead of "Not Equal", we can keep the `!=` as "Not Equal" so this way we have both available
  • Loading branch information
ghiscoding authored Mar 5, 2021
1 parent e02f746 commit 960884d
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 7 deletions.
1 change: 1 addition & 0 deletions examples/webpack-demo-vanilla-bundle/assets/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"LAST_UPDATE": "Last Update",
"LESS_THAN": "Less than",
"LESS_THAN_OR_EQUAL_TO": "Less than or equal to",
"NOT_CONTAINS": "Not contains",
"NOT_EQUAL_TO": "Not equal to",
"NOT_IN_COLLECTION_SEPERATED_BY_COMMA": "Search items not in a collection, must be separated by a comma (a,b)",
"OF": "of",
Expand Down
1 change: 1 addition & 0 deletions examples/webpack-demo-vanilla-bundle/assets/i18n/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"LAST_UPDATE": "Dernière mise à jour",
"LESS_THAN": "Plus petit que",
"LESS_THAN_OR_EQUAL_TO": "Plus petit ou égal à",
"NOT_CONTAINS": "Ne contient pas",
"NOT_EQUAL_TO": "Non égal à",
"NOT_IN_COLLECTION_SEPERATED_BY_COMMA": "Recherche excluant certain éléments d'une collection, doit être séparé par une virgule (a,b)",
"OF": "de",
Expand Down
1 change: 1 addition & 0 deletions packages/common/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export class Constants {
TEXT_LAST_UPDATE: 'Last Update',
TEXT_LESS_THAN: 'Less than',
TEXT_LESS_THAN_OR_EQUAL_TO: 'Less than or equal to',
TEXT_NOT_CONTAINS: 'Not contains',
TEXT_NOT_EQUAL_TO: 'Not equal to',
TEXT_PAGE: 'Page',
TEXT_REFRESH_DATASET: 'Refresh Dataset',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,20 @@ describe('executeStringFilterCondition method', () => {
expect(output).toBe(true);
});

it('should return False when search term is a substring of the cell value and the operator is "<>" (not contains)', () => {
const searchTerms = ['bost'];
const options = { dataKey: '', operator: '<>', cellValue: 'abbostford', fieldType: FieldType.string, searchTerms } as FilterConditionOption;
const output = executeStringFilterCondition(options, getFilterParsedText(searchTerms));
expect(output).toBe(false);
});

it('should return True when search term is a substring of the cell value and the operator is "!=" (not contains) because "!=" compares agains the entire string', () => {
const searchTerms = ['bost'];
const options = { dataKey: '', operator: '!=', cellValue: 'abbostford', fieldType: FieldType.string, searchTerms } as FilterConditionOption;
const output = executeStringFilterCondition(options, getFilterParsedText(searchTerms));
expect(output).toBe(true);
});

it('should return True when input value provided starts with same substring and the operator is empty string', () => {
const searchTerms = ['abb'];
const options = { dataKey: '', operator: '', cellValue: 'abbostford', fieldType: FieldType.string, searchTerms } as FilterConditionOption;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export const executeStringFilterCondition: FilterCondition = ((options: FilterCo
return cellValue.startsWith(parsedSearchValue);
} else if (options.operator === '' || options.operator === OperatorType.contains) {
return (cellValue.indexOf(parsedSearchValue) > -1);
} else if (options.operator === '<>' || options.operator === OperatorType.notContains) {
return (cellValue.indexOf(parsedSearchValue) === -1);
}
return testFilterCondition(options.operator || '==', cellValue, parsedSearchValue);
}) as FilterCondition;
Expand Down
18 changes: 11 additions & 7 deletions packages/common/src/filters/__tests__/compoundInputFilter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('CompoundInputFilter', () => {
let divContainer: HTMLDivElement;
let filter: CompoundInputFilter;
let filterArguments: FilterArguments;
let spyGetHeaderRow;
let spyGetHeaderRow: jest.SpyInstance;
let mockColumn: Column;

beforeEach(() => {
Expand Down Expand Up @@ -262,9 +262,11 @@ describe('CompoundInputFilter', () => {

expect(filterInputElm.value).toBe('xyz');
expect(removeExtraSpaces(filterOperatorElm[0][0].textContent!)).toBe(' Contains');
expect(removeExtraSpaces(filterOperatorElm[0][1].textContent!)).toBe('= Equals');
expect(removeExtraSpaces(filterOperatorElm[0][2].textContent!)).toBe('a* Starts With');
expect(removeExtraSpaces(filterOperatorElm[0][3].textContent!)).toBe('*z Ends With');
expect(removeExtraSpaces(filterOperatorElm[0][1].textContent!)).toBe('<> Not contains');
expect(removeExtraSpaces(filterOperatorElm[0][2].textContent!)).toBe('= Equals');
expect(removeExtraSpaces(filterOperatorElm[0][3].textContent!)).toBe('!= Not equal to');
expect(removeExtraSpaces(filterOperatorElm[0][4].textContent!)).toBe('a* Starts With');
expect(removeExtraSpaces(filterOperatorElm[0][5].textContent!)).toBe('*z Ends With');
});

it('should trigger a callback with the clear filter set when calling the "clear" method', () => {
Expand Down Expand Up @@ -330,9 +332,11 @@ describe('CompoundInputFilter', () => {

expect(filterInputElm.value).toBe('xyz');
expect(removeExtraSpaces(filterOperatorElm[0][0].textContent!)).toBe(' Contient');
expect(removeExtraSpaces(filterOperatorElm[0][1].textContent!)).toBe('= Égale');
expect(removeExtraSpaces(filterOperatorElm[0][2].textContent!)).toBe('a* Commence par');
expect(removeExtraSpaces(filterOperatorElm[0][3].textContent!)).toBe('*z Se termine par');
expect(removeExtraSpaces(filterOperatorElm[0][1].textContent!)).toBe('<> Ne contient pas');
expect(removeExtraSpaces(filterOperatorElm[0][2].textContent!)).toBe('= Égale');
expect(removeExtraSpaces(filterOperatorElm[0][3].textContent!)).toBe('!= Non égal à');
expect(removeExtraSpaces(filterOperatorElm[0][4].textContent!)).toBe('a* Commence par');
expect(removeExtraSpaces(filterOperatorElm[0][5].textContent!)).toBe('*z Se termine par');
});
});
});
2 changes: 2 additions & 0 deletions packages/common/src/filters/compoundInputFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ export class CompoundInputFilter implements Filter {
case FieldType.password:
optionValues = [
{ operator: '' as OperatorString, description: this.getOutputText('CONTAINS', 'TEXT_CONTAINS', 'Contains') },
{ operator: '<>' as OperatorString, description: this.getOutputText('NOT_CONTAINS', 'TEXT_NOT_CONTAINS', 'Not Contains') },
{ operator: '=' as OperatorString, description: this.getOutputText('EQUALS', 'TEXT_EQUALS', 'Equals') },
{ operator: '!=' as OperatorString, description: this.getOutputText('NOT_EQUAL_TO', 'TEXT_NOT_EQUAL_TO', 'Not equal to') },
{ operator: 'a*' as OperatorString, description: this.getOutputText('STARTS_WITH', 'TEXT_STARTS_WITH', 'Starts with') },
{ operator: '*z' as OperatorString, description: this.getOutputText('ENDS_WITH', 'TEXT_ENDS_WITH', 'Ends with') },
];
Expand Down
3 changes: 3 additions & 0 deletions packages/common/src/interfaces/locale.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ export interface Locale {
/** Text "Less than or equal to" shown in Compound Editors/Filters as an Operator */
TEXT_LESS_THAN_OR_EQUAL_TO: string;

/** Text "Not contains" shown in Compound Editors/Filters as an Operator */
TEXT_NOT_CONTAINS: string;

/** Text "Not equal to" shown in Compound Editors/Filters as an Operator */
TEXT_NOT_EQUAL_TO: string;

Expand Down
1 change: 1 addition & 0 deletions test/translateServiceStub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export class TranslateServiceStub implements TranslaterService {
case 'MALE': output = this._locale === 'en' ? 'Male' : 'Mâle'; break;
case 'ITEMS': output = this._locale === 'en' ? 'items' : 'éléments'; break;
case 'ITEMS_PER_PAGE': output = this._locale === 'en' ? 'items per page' : 'éléments par page'; break;
case 'NOT_CONTAINS': output = this._locale === 'en' ? 'Not contains' : 'Ne contient pas'; break;
case 'NOT_EQUAL_TO': output = this._locale === 'en' ? 'Not equal to' : 'Non égal à'; break;
case 'OF': output = this._locale === 'en' ? 'of' : 'de'; break;
case 'OK': output = this._locale === 'en' ? 'OK' : 'Terminé'; break;
Expand Down

0 comments on commit 960884d

Please sign in to comment.