From 39d86b70db4c719d318b94eb179c547210d3d25d Mon Sep 17 00:00:00 2001 From: Jurriaan Wijnberg Date: Fri, 9 Oct 2020 17:09:19 +0200 Subject: [PATCH 1/6] Select the option with the current itemsPerPage in the select dropdown --- .../__tests__/slick-pagination.component.spec.ts | 13 ++++++++++++- .../components/slick-pagination.component.html | 4 ++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/app/modules/angular-slickgrid/components/__tests__/slick-pagination.component.spec.ts b/src/app/modules/angular-slickgrid/components/__tests__/slick-pagination.component.spec.ts index 237bf884d..ad6993adc 100644 --- a/src/app/modules/angular-slickgrid/components/__tests__/slick-pagination.component.spec.ts +++ b/src/app/modules/angular-slickgrid/components/__tests__/slick-pagination.component.spec.ts @@ -12,7 +12,7 @@ const paginationServiceStub = { dataTo: 10, pageNumber: 2, pageCount: 1, - itemsPerPage: 5, + itemsPerPage: 10, pageSize: 10, totalItems: 100, availablePageSizes: [5, 10, 15, 20], @@ -125,6 +125,17 @@ describe('App Component', () => { expect(component.totalItems).toBe(100); }); + it('should select the option with the current itemsPerPage in the select dropdown', () => { + fixture.detectChanges(); + + const selectElement = fixture.debugElement.query(By.css('select')).nativeElement as HTMLSelectElement; + + expect(selectElement.value).toBe('10'); + expect(selectElement.selectedIndex).toBe(1); + const optionElement = selectElement.selectedOptions.item(0); + expect(optionElement.value).toBe('10'); + }); + it('should create a the Slick-Pagination component in the DOM and expect different locale when changed', () => { translate.use('en'); fixture.detectChanges(); diff --git a/src/app/modules/angular-slickgrid/components/slick-pagination.component.html b/src/app/modules/angular-slickgrid/components/slick-pagination.component.html index 134639822..f4797ad65 100644 --- a/src/app/modules/angular-slickgrid/components/slick-pagination.component.html +++ b/src/app/modules/angular-slickgrid/components/slick-pagination.component.html @@ -38,8 +38,8 @@ - + {{textItemsPerPage}}, From 5822de1c12133841a2ba563d3969ce4b9c5c6856 Mon Sep 17 00:00:00 2001 From: jr01 Date: Fri, 18 Dec 2020 17:11:45 +0100 Subject: [PATCH 2/6] fix(backend): oData queries with input filter --- .../__tests__/grid-odata.service.spec.ts | 102 +++++++- .../services/grid-odata.service.ts | 227 +++++++++--------- 2 files changed, 218 insertions(+), 111 deletions(-) diff --git a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts index 62ff4f308..ece69c288 100644 --- a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts +++ b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts @@ -957,6 +957,62 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); }); + it('should return a query to filter a search value between an inclusive range of numbers using the 2 dots (..) separator, the "RangeInclusive" operator and the range has an unbounded end', () => { + const expectation = `$top=10&$filter=(Duration ge 5)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['5..'], operator: 'RangeInclusive' }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query to filter a search value between an inclusive range of numbers using the 2 dots (..) separator, the "RangeInclusive" operator and the range has an unbounded begin', () => { + const expectation = `$top=10&$filter=(Duration le 5)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['..5'], operator: 'RangeInclusive' }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query to filter a search value between an inclusive range of numbers using the 2 dots (..) separator, the "RangeExclusive" operator and the range has an unbounded end', () => { + const expectation = `$top=10&$filter=(Duration gt 5)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['5..'], operator: 'RangeExclusive' }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query to filter a search value between an inclusive range of numbers using the 2 dots (..) separator, the "RangeExclusive" operator and the range has an unbounded begin', () => { + const expectation = `$top=10&$filter=(Duration lt 5)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['..5'], operator: 'RangeExclusive' }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + it('should return a query to filter a search value between an exclusive range of numbers using 2 search terms and the "RangeExclusive" operator', () => { const expectation = `$top=10&$filter=(substringof('abc', Company) and (Duration gt 5 and Duration lt 22))`; const mockColumnCompany = { id: 'company', field: 'company' } as Column; @@ -1423,9 +1479,9 @@ describe('GridOdataService', () => { expect(currentFilters).toEqual(presetFilters); }); - it('should return a query with a filter with range of numbers with decimals when the preset is a filter range with 3 dots (...) separator', () => { + it('should return a query with a filter with range of numbers with decimals when the preset is a filter range with 2 dots (..) separator and range ends with a fraction', () => { serviceOptions.columnDefinitions = [{ id: 'company', field: 'company' }, { id: 'gender', field: 'gender' }, { id: 'duration', field: 'duration', type: FieldType.number }]; - const expectation = `$top=10&$filter=(Duration ge 0.5 and Duration le .88)`; + const expectation = `$top=10&$filter=(Duration ge 0.5 and Duration le 0.88)`; const presetFilters = [ { columnId: 'duration', searchTerms: ['0.5...88'] }, ] as CurrentFilter[]; @@ -1539,6 +1595,48 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); expect(currentFilters).toEqual(presetFilters); }); + + it('should return a query to filter a search value with a "decimalSeparator" set', () => { + const expectation = `$filter=(Duration eq 10.22)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number, filter: { params: { decimalSeparator: ',' }} } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['10,22'] }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query to filter a search value with a fraction of a number that is missing a leading 0', () => { + const expectation = `$filter=(Duration eq 0.22)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['.22'] }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); + + it('should return a query to filter a search value with a number that contains invalid characters', () => { + const expectation = `$filter=(Duration eq -22)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['-2a2'] }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); }); }); diff --git a/src/app/modules/angular-slickgrid/services/grid-odata.service.ts b/src/app/modules/angular-slickgrid/services/grid-odata.service.ts index 4631ab71e..5bc703784 100644 --- a/src/app/modules/angular-slickgrid/services/grid-odata.service.ts +++ b/src/app/modules/angular-slickgrid/services/grid-odata.service.ts @@ -259,7 +259,7 @@ export class GridOdataService implements BackendService { let fieldName = (columnDef.filter && columnDef.filter.queryField) || columnDef.queryFieldFilter || columnDef.queryField || columnDef.field || columnDef.name || ''; const fieldType = columnDef.type || FieldType.string; - let searchTerms = (columnFilter ? columnFilter.searchTerms : null) || []; + let searchTerms = (columnFilter && columnFilter.searchTerms ? [...columnFilter.searchTerms] : null) || []; let fieldSearchValue = (Array.isArray(searchTerms) && searchTerms.length === 1) ? searchTerms[0] : ''; if (typeof fieldSearchValue === 'undefined') { fieldSearchValue = ''; @@ -282,16 +282,22 @@ export class GridOdataService implements BackendService { continue; } - if (Array.isArray(searchTerms) && searchTerms.length === 1 && typeof searchTerms[0] === 'string' && searchTerms[0].indexOf('..') > 0) { - searchTerms = searchTerms[0].split('..'); + if (Array.isArray(searchTerms) && searchTerms.length === 1 && typeof searchTerms[0] === 'string' && searchTerms[0].indexOf('..') >= 0) { if (!operator) { - operator = OperatorType.rangeInclusive; + operator = this._gridOptions.defaultFilterRangeOperator; } - } - // escaping the search value - searchValue = searchValue.replace(`'`, `''`); // escape single quotes by doubling them - searchValue = encodeURIComponent(searchValue); // encode URI of the final search value + searchTerms = searchTerms[0].split('..', 2); + if (searchTerms[0] === '') { + operator = operator === OperatorType.rangeInclusive ? '<=' : operator === OperatorType.rangeExclusive ? '<' : operator; + searchTerms = searchTerms.slice(1); + searchValue = searchTerms[0]; + } else if (searchTerms[1] === '') { + operator = operator === OperatorType.rangeInclusive ? '>=' : operator === OperatorType.rangeExclusive ? '>' : operator; + searchTerms = searchTerms.slice(0, 1); + searchValue = searchTerms[0]; + } + } // if we didn't find an Operator but we have a Column Operator inside the Filter (DOM Element), we should use its default Operator // multipleSelect is "IN", while singleSelect is "EQ", else don't map any operator @@ -299,9 +305,19 @@ export class GridOdataService implements BackendService { operator = columnDef.filter.operator; } + // No operator and 2 search terms should lead to default range operator. + if (!operator && Array.isArray(searchTerms) && searchTerms.length === 2 && searchTerms[0] && searchTerms[1]) { + operator = this._gridOptions.defaultFilterRangeOperator; + } + + // Range with 1 searchterm should lead to equals for a date field. + if ((operator === OperatorType.rangeInclusive || OperatorType.rangeExclusive) && Array.isArray(searchTerms) && searchTerms.length === 1 && fieldType === FieldType.date) { + operator = OperatorType.equal; + } + // if we still don't have an operator find the proper Operator to use by it's field type if (!operator) { - operator = mapOperatorByFieldType(columnDef.type || FieldType.string); + operator = mapOperatorByFieldType(fieldType); } // extra query arguments @@ -311,6 +327,15 @@ export class GridOdataService implements BackendService { this.saveColumnFilter(fieldName, fieldSearchValue, searchTerms); } } else { + // Normalize all search values + const decimalSeparator = this.getDecimalSeparator(columnDef); + searchValue = this.normalizeSearchValue(fieldType, searchValue, odataVersion, decimalSeparator); + if (Array.isArray(searchTerms)) { + searchTerms.forEach((part, index) => { + searchTerms[index] = this.normalizeSearchValue(fieldType, searchTerms[index], odataVersion, decimalSeparator); + }); + } + searchBy = ''; // titleCase the fieldName so that it matches the WebApi names @@ -318,63 +343,40 @@ export class GridOdataService implements BackendService { fieldName = titleCase(fieldName || ''); } - if (fieldType === FieldType.date) { - searchBy = this.filterBySearchDate(fieldName, operator, searchTerms, odataVersion); - } else if (searchTerms && searchTerms.length > 1 && (operator === 'IN' || operator === 'NIN' || operator === 'NOTIN' || operator === 'NOT IN' || operator === 'NOT_IN')) { + if (searchTerms && searchTerms.length > 1 && (operator === 'IN' || operator === 'NIN' || operator === 'NOTIN' || operator === 'NOT IN' || operator === 'NOT_IN')) { // when having more than 1 search term (then check if we have a "IN" or "NOT IN" filter search) - const tmpSearchTerms = []; - + const tmpSearchTerms: string[] = []; if (operator === 'IN') { // example:: (Stage eq "Expired" or Stage eq "Renewal") for (let j = 0, lnj = searchTerms.length; j < lnj; j++) { - if (fieldType === FieldType.string || fieldType === FieldType.text || fieldType === FieldType.readonly) { - const searchVal = encodeURIComponent(searchTerms[j].replace(`'`, `''`)); - tmpSearchTerms.push(`${fieldName} eq '${searchVal}'`); - } else { - // Single quote escape is not needed for non string type - tmpSearchTerms.push(`${fieldName} eq ${searchTerms[j]}`); - } + tmpSearchTerms.push(`${fieldName} eq ${searchTerms[j]}`); } searchBy = tmpSearchTerms.join(' or '); - if (!(typeof searchBy === 'string' && searchBy[0] === '(' && searchBy.slice(-1) === ')')) { - searchBy = `(${searchBy})`; - } } else { // example:: (Stage ne "Expired" and Stage ne "Renewal") for (let k = 0, lnk = searchTerms.length; k < lnk; k++) { - const searchVal = encodeURIComponent(searchTerms[k].replace(`'`, `''`)); - tmpSearchTerms.push(`${fieldName} ne '${searchVal}'`); + tmpSearchTerms.push(`${fieldName} ne ${searchTerms[k]}`); } searchBy = tmpSearchTerms.join(' and '); - if (!(typeof searchBy === 'string' && searchBy[0] === '(' && searchBy.slice(-1) === ')')) { - searchBy = `(${searchBy})`; - } + } + if (!(typeof searchBy === 'string' && searchBy[0] === '(' && searchBy.slice(-1) === ')')) { + searchBy = `(${searchBy})`; } } else if (operator === '*' || operator === 'a*' || operator === '*z' || lastValueChar === '*' || operator === OperatorType.startsWith || operator === OperatorType.endsWith) { // first/last character is a '*' will be a startsWith or endsWith - searchBy = (operator === '*' || operator === '*z' || operator === OperatorType.endsWith) ? `endswith(${fieldName}, '${searchValue}')` : `startswith(${fieldName}, '${searchValue}')`; - } else if (fieldType === FieldType.string || fieldType === FieldType.text || fieldType === FieldType.readonly) { - // string field needs to be in single quotes - if (operator === '' || operator === OperatorType.contains || operator === OperatorType.notContains) { - searchBy = this.odataQueryVersionWrapper('substring', odataVersion, fieldName, searchValue); - if (operator === OperatorType.notContains) { - searchBy = `not ${searchBy}`; - } - } else if (operator === OperatorType.rangeExclusive || operator === OperatorType.rangeInclusive) { - // example:: (Duration >= 5 and Duration <= 10) - searchBy = this.filterBySearchTermRange(fieldName, operator, searchTerms); - } else { - searchBy = `${fieldName} ${this.mapOdataOperator(operator)} '${searchValue}'`; + searchBy = (operator === '*' || operator === '*z' || operator === OperatorType.endsWith) ? `endswith(${fieldName}, ${searchValue})` : `startswith(${fieldName}, ${searchValue})`; + } else if (operator === OperatorType.rangeExclusive || operator === OperatorType.rangeInclusive) { + // example:: (Name >= 'Bob' and Name <= 'Jane') + searchBy = this.filterBySearchTermRange(fieldName, operator, searchTerms); + } else if ((operator === '' || operator === OperatorType.contains || operator === OperatorType.notContains) && + (fieldType === FieldType.string || fieldType === FieldType.text || fieldType === FieldType.readonly)) { + searchBy = odataVersion >= 4 ? `contains(${fieldName}, ${searchValue})` : `substringof(${searchValue}, ${fieldName})`; + if (operator === OperatorType.notContains) { + searchBy = `not ${searchBy}`; } } else { - if (operator === OperatorType.rangeExclusive || operator === OperatorType.rangeInclusive) { - // example:: (Duration >= 5 and Duration <= 10) - searchBy = this.filterBySearchTermRange(fieldName, operator, searchTerms); - } else { - // any other field type (or undefined type) - searchValue = (fieldType === FieldType.number || fieldType === FieldType.boolean) ? searchValue : `'${searchValue}'`; - searchBy = `${fieldName} ${this.mapOdataOperator(operator)} ${searchValue}`; - } + // any other field type (or undefined type) + searchBy = `${fieldName} ${this.mapOdataOperator(operator)} ${searchValue}`; } // push to our temp array and also trim white spaces @@ -535,77 +537,84 @@ export class GridOdataService implements BackendService { }); } - private odataQueryVersionWrapper(queryType: 'dateTime' | 'substring', version: number, fieldName: string, searchValue = ''): string { - let query = ''; - switch (queryType) { - case 'dateTime': - query = version >= 4 ? searchValue : `DateTime'${searchValue}'`; - break; - case 'substring': - query = version >= 4 ? `contains(${fieldName}, '${searchValue}')` : `substringof('${searchValue}', ${fieldName})`; - break; - } - return query; - } - - /** - * Filter by a search date, the searchTerms might be a single value or range of dates (2 searchTerms OR 1 string separated by 2 dots "date1..date2") - * Also depending on the OData version number, the output will be different, previous version must wrap dates with DateTime - * - version 2-3:: Finish gt DateTime'2019-08-12T00:00:00Z' - * - version 4:: Finish gt 2019-08-12T00:00:00Z - */ - private filterBySearchDate(fieldName: string, operator: OperatorType | OperatorString, searchTerms: SearchTerm[], version: number): string { - let query = ''; - let searchValues: SearchTerm[] = []; - if (Array.isArray(searchTerms) && searchTerms.length > 1) { - searchValues = searchTerms; - if (operator !== OperatorType.rangeExclusive && operator !== OperatorType.rangeInclusive && this._gridOptions.defaultFilterRangeOperator) { - operator = this._gridOptions.defaultFilterRangeOperator; - } - } - - // single search value - if (searchValues.length === 0 && Array.isArray(searchTerms) && searchTerms.length === 1 && searchTerms[0]) { - const searchValue1 = this.odataQueryVersionWrapper('dateTime', version, fieldName, parseUtcDate(searchTerms[0] as string, true)); - if (searchValue1) { - return `${fieldName} ${this.mapOdataOperator(operator)} ${searchValue1}`; - } - } - - // multiple search value (date range) - if (Array.isArray(searchValues) && searchValues.length === 2 && searchValues[0] && searchValues[1]) { - // date field needs to be UTC and within DateTime function - const searchValue1 = this.odataQueryVersionWrapper('dateTime', version, fieldName, parseUtcDate(searchValues[0] as string, true)); - const searchValue2 = this.odataQueryVersionWrapper('dateTime', version, fieldName, parseUtcDate(searchValues[1] as string, true)); - - if (searchValue1 && searchValue2) { - if (operator === OperatorType.rangeInclusive) { - // example:: (Finish >= DateTime'2019-08-11T00:00:00Z' and Finish <= DateTime'2019-09-12T00:00:00Z') - query = `(${fieldName} ge ${searchValue1} and ${fieldName} le ${searchValue2})`; - } else if (operator === OperatorType.rangeExclusive) { - // example:: (Finish > DateTime'2019-08-11T00:00:00Z' and Finish < DateTime'2019-09-12T00:00:00Z') - query = `(${fieldName} gt ${searchValue1} and ${fieldName} lt ${searchValue2})`; - } - } - } - return query; - } - /** * Filter by a range of searchTerms (2 searchTerms OR 1 string separated by 2 dots "value1..value2") */ private filterBySearchTermRange(fieldName: string, operator: OperatorType | OperatorString, searchTerms: SearchTerm[]) { let query = ''; - if (Array.isArray(searchTerms) && searchTerms.length === 2) { if (operator === OperatorType.rangeInclusive) { // example:: (Duration >= 5 and Duration <= 10) - query = `(${fieldName} ge ${searchTerms[0]} and ${fieldName} le ${searchTerms[1]})`; + query = `(${fieldName} ge ${searchTerms[0]}`; + if (searchTerms[1] !== '') { + query += ` and ${fieldName} le ${searchTerms[1]}`; + } + query += ')'; } else if (operator === OperatorType.rangeExclusive) { // example:: (Duration > 5 and Duration < 10) - query = `(${fieldName} gt ${searchTerms[0]} and ${fieldName} lt ${searchTerms[1]})`; + query = `(${fieldName} gt ${searchTerms[0]}`; + if (searchTerms[1] !== '') { + query += ` and ${fieldName} lt ${searchTerms[1]}`; + } + query += ')'; } } return query; } + + /** + * Normalizes the search value according to field type and oData version. + */ + private normalizeSearchValue(fieldType: FieldType, searchValue: any, version: number, decimalSeparator: string) { + switch (fieldType) { + case FieldType.date: + searchValue = parseUtcDate(searchValue as string, true); + searchValue = version >= 4 ? searchValue : `DateTime'${searchValue}'`; + break; + case FieldType.string: + case FieldType.text: + case FieldType.readonly: + if (typeof searchValue === 'string') { + // escape single quotes by doubling them + searchValue = searchValue.replace(/'/g, `''`); + // encode URI of the final search value + searchValue = encodeURIComponent(searchValue); + // strings need to be quoted. + searchValue = `'${searchValue}'`; + } + break; + case FieldType.integer: + case FieldType.number: + case FieldType.float: + if (typeof searchValue === 'string') { + // Parse a valid decimal from the string. + + // Remove all trailing decimal separators. + searchValue = searchValue.replace(new RegExp(`(\\${decimalSeparator}+$)`, 'g'), ''); + // Replace leading decimal separators with '0{decimalSeparator}' + searchValue = searchValue.replace(new RegExp(`(^\\${decimalSeparator}+)`, 'g'), `0${decimalSeparator}`); + // Remove any non valid decimal character from the search string + searchValue = searchValue.replace(new RegExp(`[^0-9\-\\${decimalSeparator}]`, 'g'), ''); + // Replace the decimal separator from the input with the oData decimal separator. + searchValue = searchValue.replace(decimalSeparator, '.'); + + // if nothing left, search for 0 + if (searchValue === '') { + searchValue = '0'; + } + } + break; + } + + return searchValue; + } + + private getDecimalSeparator(columnDef: Column) { + const optionName = 'decimalSeparator'; + const params = columnDef && columnDef.filter && columnDef.filter.params; + if (params && params.hasOwnProperty(optionName)) { + return params[optionName]; + } + return '.'; + } } From 79619c987a284f599583e98a3f894d1ae678bad1 Mon Sep 17 00:00:00 2001 From: jr01 Date: Fri, 18 Dec 2020 21:04:01 +0100 Subject: [PATCH 3/6] Fix NL number input weirdness --- .../__tests__/grid-odata.service.spec.ts | 4 +- .../services/grid-odata.service.ts | 38 ++++++++++++------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts index ece69c288..9b561282a 100644 --- a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts +++ b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts @@ -1596,9 +1596,9 @@ describe('GridOdataService', () => { expect(currentFilters).toEqual(presetFilters); }); - it('should return a query to filter a search value with a "decimalSeparator" set', () => { + it('should return a query to filter a search value with "decimalInputSeparators" set', () => { const expectation = `$filter=(Duration eq 10.22)`; - const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number, filter: { params: { decimalSeparator: ',' }} } as Column; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number, filter: { params: { decimalInputSeparators: ',.' }} } as Column; const mockColumnFilters = { duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['10,22'] }, } as ColumnFilters; diff --git a/src/app/modules/angular-slickgrid/services/grid-odata.service.ts b/src/app/modules/angular-slickgrid/services/grid-odata.service.ts index 5bc703784..a6046363a 100644 --- a/src/app/modules/angular-slickgrid/services/grid-odata.service.ts +++ b/src/app/modules/angular-slickgrid/services/grid-odata.service.ts @@ -328,11 +328,11 @@ export class GridOdataService implements BackendService { } } else { // Normalize all search values - const decimalSeparator = this.getDecimalSeparator(columnDef); - searchValue = this.normalizeSearchValue(fieldType, searchValue, odataVersion, decimalSeparator); + const decimalInputSeparators = this.getDecimalInputSeparators(columnDef); + searchValue = this.normalizeSearchValue(fieldType, searchValue, odataVersion, decimalInputSeparators); if (Array.isArray(searchTerms)) { searchTerms.forEach((part, index) => { - searchTerms[index] = this.normalizeSearchValue(fieldType, searchTerms[index], odataVersion, decimalSeparator); + searchTerms[index] = this.normalizeSearchValue(fieldType, searchTerms[index], odataVersion, decimalInputSeparators); }); } @@ -565,7 +565,7 @@ export class GridOdataService implements BackendService { /** * Normalizes the search value according to field type and oData version. */ - private normalizeSearchValue(fieldType: FieldType, searchValue: any, version: number, decimalSeparator: string) { + private normalizeSearchValue(fieldType: FieldType, searchValue: any, version: number, decimalInputSeparators: string) { switch (fieldType) { case FieldType.date: searchValue = parseUtcDate(searchValue as string, true); @@ -589,14 +589,19 @@ export class GridOdataService implements BackendService { if (typeof searchValue === 'string') { // Parse a valid decimal from the string. - // Remove all trailing decimal separators. - searchValue = searchValue.replace(new RegExp(`(\\${decimalSeparator}+$)`, 'g'), ''); - // Replace leading decimal separators with '0{decimalSeparator}' - searchValue = searchValue.replace(new RegExp(`(^\\${decimalSeparator}+)`, 'g'), `0${decimalSeparator}`); - // Remove any non valid decimal character from the search string - searchValue = searchValue.replace(new RegExp(`[^0-9\-\\${decimalSeparator}]`, 'g'), ''); - // Replace the decimal separator from the input with the oData decimal separator. - searchValue = searchValue.replace(decimalSeparator, '.'); + // Replace all input decimal separators with the oData decimal separator. + for (const decimalInputSeparator of decimalInputSeparators) { + searchValue = searchValue.replace(decimalInputSeparator, '.'); + } + + // Replace double dots with single dots + searchValue = searchValue.replace(/\.\./g, '.'); + // Remove a trailing dot + searchValue = searchValue.replace(/\.+$/g, ''); + // Predix a leading dot with 0 + searchValue = searchValue.replace(/^\.+/g, '0.'); + // Remove any non valid decimal characters from the search string + searchValue = searchValue.replace(/(?!^\-)[^\d\.]/g, ''); // if nothing left, search for 0 if (searchValue === '') { @@ -609,8 +614,13 @@ export class GridOdataService implements BackendService { return searchValue; } - private getDecimalSeparator(columnDef: Column) { - const optionName = 'decimalSeparator'; + /** + * Returns all allowed decimal separators. + * For example in NL it is common when entering numbers that both ',' and '.' acct as the decimal separator, + * while displaying/formatting the ',' is the decimal separator and the '.' the thousand separator. + */ + private getDecimalInputSeparators(columnDef: Column) { + const optionName = 'decimalInputSeparators'; const params = columnDef && columnDef.filter && columnDef.filter.params; if (params && params.hasOwnProperty(optionName)) { return params[optionName]; From 6cc73960fffa6bee836c508e6825732df63ddb9f Mon Sep 17 00:00:00 2001 From: jr01 Date: Sat, 19 Dec 2020 17:00:08 +0100 Subject: [PATCH 4/6] Code review rework --- .../__tests__/grid-odata.service.spec.ts | 20 +++++++------- .../services/grid-odata.service.ts | 26 +++---------------- 2 files changed, 13 insertions(+), 33 deletions(-) diff --git a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts index 9b561282a..30d4845e1 100644 --- a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts +++ b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts @@ -1596,11 +1596,11 @@ describe('GridOdataService', () => { expect(currentFilters).toEqual(presetFilters); }); - it('should return a query to filter a search value with "decimalInputSeparators" set', () => { - const expectation = `$filter=(Duration eq 10.22)`; - const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number, filter: { params: { decimalInputSeparators: ',.' }} } as Column; + it('should return a query to filter a search value with a fraction of a number that is missing a leading 0', () => { + const expectation = `$filter=(Duration eq 0.22)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; const mockColumnFilters = { - duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['10,22'] }, + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['.22'] }, } as ColumnFilters; service.init(serviceOptions, paginationOptions, gridStub); @@ -1610,11 +1610,11 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); }); - it('should return a query to filter a search value with a fraction of a number that is missing a leading 0', () => { - const expectation = `$filter=(Duration eq 0.22)`; + it('should return a query to filter a search value with a number that contains invalid characters', () => { + const expectation = `$filter=(Duration eq -22)`; const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; const mockColumnFilters = { - duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['.22'] }, + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['-2a2'] }, } as ColumnFilters; service.init(serviceOptions, paginationOptions, gridStub); @@ -1624,11 +1624,11 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); }); - it('should return a query to filter a search value with a number that contains invalid characters', () => { - const expectation = `$filter=(Duration eq -22)`; + it('should return a query to filter a search value with an integer that contains invalid characters', () => { + const expectation = `$filter=(Duration eq 22)`; const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; const mockColumnFilters = { - duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['-2a2'] }, + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['22;'] }, } as ColumnFilters; service.init(serviceOptions, paginationOptions, gridStub); diff --git a/src/app/modules/angular-slickgrid/services/grid-odata.service.ts b/src/app/modules/angular-slickgrid/services/grid-odata.service.ts index a6046363a..4859e9451 100644 --- a/src/app/modules/angular-slickgrid/services/grid-odata.service.ts +++ b/src/app/modules/angular-slickgrid/services/grid-odata.service.ts @@ -328,11 +328,10 @@ export class GridOdataService implements BackendService { } } else { // Normalize all search values - const decimalInputSeparators = this.getDecimalInputSeparators(columnDef); - searchValue = this.normalizeSearchValue(fieldType, searchValue, odataVersion, decimalInputSeparators); + searchValue = this.normalizeSearchValue(fieldType, searchValue, odataVersion); if (Array.isArray(searchTerms)) { searchTerms.forEach((part, index) => { - searchTerms[index] = this.normalizeSearchValue(fieldType, searchTerms[index], odataVersion, decimalInputSeparators); + searchTerms[index] = this.normalizeSearchValue(fieldType, searchTerms[index], odataVersion); }); } @@ -565,7 +564,7 @@ export class GridOdataService implements BackendService { /** * Normalizes the search value according to field type and oData version. */ - private normalizeSearchValue(fieldType: FieldType, searchValue: any, version: number, decimalInputSeparators: string) { + private normalizeSearchValue(fieldType: FieldType, searchValue: any, version: number) { switch (fieldType) { case FieldType.date: searchValue = parseUtcDate(searchValue as string, true); @@ -589,11 +588,6 @@ export class GridOdataService implements BackendService { if (typeof searchValue === 'string') { // Parse a valid decimal from the string. - // Replace all input decimal separators with the oData decimal separator. - for (const decimalInputSeparator of decimalInputSeparators) { - searchValue = searchValue.replace(decimalInputSeparator, '.'); - } - // Replace double dots with single dots searchValue = searchValue.replace(/\.\./g, '.'); // Remove a trailing dot @@ -613,18 +607,4 @@ export class GridOdataService implements BackendService { return searchValue; } - - /** - * Returns all allowed decimal separators. - * For example in NL it is common when entering numbers that both ',' and '.' acct as the decimal separator, - * while displaying/formatting the ',' is the decimal separator and the '.' the thousand separator. - */ - private getDecimalInputSeparators(columnDef: Column) { - const optionName = 'decimalInputSeparators'; - const params = columnDef && columnDef.filter && columnDef.filter.params; - if (params && params.hasOwnProperty(optionName)) { - return params[optionName]; - } - return '.'; - } } From ded54d9f23071ab3beb0e4d94657f4d0c627d2e4 Mon Sep 17 00:00:00 2001 From: jr01 Date: Thu, 24 Dec 2020 08:56:17 +0100 Subject: [PATCH 5/6] Fix parsing negative decimals --- .../services/__tests__/grid-odata.service.spec.ts | 14 ++++++++++++++ .../services/grid-odata.service.ts | 6 ++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts index 30d4845e1..330ebdb44 100644 --- a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts +++ b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts @@ -1637,6 +1637,20 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); }); + + it('should return a query to filter a search value with a number that only has a minus characters', () => { + const expectation = `$filter=(Duration eq 0)`; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnFilters = { + duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['-'] }, + } as ColumnFilters; + + service.init(serviceOptions, paginationOptions, gridStub); + service.updateFilters(mockColumnFilters, false); + const query = service.buildQuery(); + + expect(query).toBe(expectation); + }); }); }); diff --git a/src/app/modules/angular-slickgrid/services/grid-odata.service.ts b/src/app/modules/angular-slickgrid/services/grid-odata.service.ts index 4859e9451..61e1e3548 100644 --- a/src/app/modules/angular-slickgrid/services/grid-odata.service.ts +++ b/src/app/modules/angular-slickgrid/services/grid-odata.service.ts @@ -592,13 +592,15 @@ export class GridOdataService implements BackendService { searchValue = searchValue.replace(/\.\./g, '.'); // Remove a trailing dot searchValue = searchValue.replace(/\.+$/g, ''); - // Predix a leading dot with 0 + // Prefix a leading dot with 0 searchValue = searchValue.replace(/^\.+/g, '0.'); + // Prefix leading dash dot with -0. + searchValue = searchValue.replace(/^\-+\.+/g, '-0.'); // Remove any non valid decimal characters from the search string searchValue = searchValue.replace(/(?!^\-)[^\d\.]/g, ''); // if nothing left, search for 0 - if (searchValue === '') { + if (searchValue === '' || searchValue === '-') { searchValue = '0'; } } From d9aab7628c6864e3da4cfe2b579172cfd4e45e4a Mon Sep 17 00:00:00 2001 From: Jurriaan Wijnberg Date: Tue, 5 Jan 2021 08:11:29 +0100 Subject: [PATCH 6/6] Code review rework --- .../services/__tests__/grid-odata.service.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts index 330ebdb44..27fad64ab 100644 --- a/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts +++ b/src/app/modules/angular-slickgrid/services/__tests__/grid-odata.service.spec.ts @@ -1610,7 +1610,7 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); }); - it('should return a query to filter a search value with a number that contains invalid characters', () => { + it('should return a query without invalid characters to filter a search value with a number that contains invalid characters', () => { const expectation = `$filter=(Duration eq -22)`; const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; const mockColumnFilters = { @@ -1624,9 +1624,9 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); }); - it('should return a query to filter a search value with an integer that contains invalid characters', () => { + it('should return a query without invalid characters to filter a search value with an integer that contains invalid characters', () => { const expectation = `$filter=(Duration eq 22)`; - const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.integer } as Column; const mockColumnFilters = { duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['22;'] }, } as ColumnFilters; @@ -1638,9 +1638,9 @@ describe('GridOdataService', () => { expect(query).toBe(expectation); }); - it('should return a query to filter a search value with a number that only has a minus characters', () => { + it('should return a query with 0 to filter a search value when the search value contains a minus', () => { const expectation = `$filter=(Duration eq 0)`; - const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.number } as Column; + const mockColumnDuration = { id: 'duration', field: 'duration', type: FieldType.float } as Column; const mockColumnFilters = { duration: { columnId: 'duration', columnDef: mockColumnDuration, searchTerms: ['-'] }, } as ColumnFilters;