Skip to content

Commit 0634a86

Browse files
matho-odoomatho-odoo
committed
[IMP] Data filter: Trim whitespace
[Data filter] Trim whitespace and don't bother of caps vs low case when filtering Changes done in the filters by values and by criterion Task:5375214
1 parent 04b8575 commit 0634a86

File tree

6 files changed

+148
-112
lines changed

6 files changed

+148
-112
lines changed

‎packages/o-spreadsheet-engine/src/helpers/text_helper.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -292,9 +292,9 @@ export function getFontSizeMatchingWidth(
292292
return fontSize;
293293
}
294294

295-
/** Transform a string to lower case. If the string is undefined, return an empty string */
296-
export function toLowerCase(str: string | undefined): string {
297-
return str ? str.toLowerCase() : "";
295+
/** Transform a string to lowercase and removes whitespace from both ends of the string*/
296+
export function toTrimmedLowerCase(str: string | undefined): string {
297+
return str ? str.toLowerCase().trim() : "";
298298
}
299299

300300
/**

‎packages/o-spreadsheet-engine/src/plugins/ui_stateful/filter_evaluation.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { isMultipleElementMatrix, toScalar } from "../../functions/helper_matric
22
import { parseLiteral } from "../../helpers/cells/cell_evaluation";
33
import { toXC } from "../../helpers/coordinates";
44
import { deepCopy, getUniqueText, range } from "../../helpers/misc";
5-
import { toLowerCase } from "../../helpers/text_helper";
5+
import { toTrimmedLowerCase } from "../../helpers/text_helper";
66
import { positions, toZone, zoneToDimension } from "../../helpers/zones";
77
import { criterionEvaluatorRegistry } from "../../registries/criterion_registry";
88
import { Command, CommandResult, LocalCommand, UpdateFilterCommand } from "../../types/commands";
@@ -139,6 +139,11 @@ export class FilterEvaluationPlugin extends UIPlugin {
139139
const id = this.getters.getFilterId({ sheetId, col, row });
140140
if (!id) return;
141141
if (!this.filterValues[sheetId]) this.filterValues[sheetId] = {};
142+
if (value.filterType === "values") {
143+
value.hiddenValues = value.hiddenValues.map(toTrimmedLowerCase);
144+
} else if (value.filterType === "criterion") {
145+
value.values = value.values.map(toTrimmedLowerCase);
146+
}
142147
this.filterValues[sheetId][id] = value;
143148
}
144149

@@ -163,7 +168,7 @@ export class FilterEvaluationPlugin extends UIPlugin {
163168
continue;
164169
}
165170
if (filterValue.filterType === "values") {
166-
const filteredValues = filterValue.hiddenValues?.map(toLowerCase);
171+
const filteredValues = filterValue.hiddenValues;
167172
if (!filteredValues) continue;
168173
const filteredValuesSet = new Set(filteredValues);
169174
for (let row = filteredZone.top; row <= filteredZone.bottom; row++) {
@@ -205,7 +210,7 @@ export class FilterEvaluationPlugin extends UIPlugin {
205210

206211
private getCellValueAsString(sheetId: UID, col: number, row: number): string {
207212
const value = this.getters.getEvaluatedCell({ sheetId, col, row }).formattedValue;
208-
return value.toLowerCase();
213+
return toTrimmedLowerCase(value);
209214
}
210215

211216
exportForExcel(data: ExcelWorkbookData) {
@@ -234,7 +239,7 @@ export class FilterEvaluationPlugin extends UIPlugin {
234239
if (filteredValues.length) {
235240
const xlsxDisplayedValues = valuesInFilterZone
236241
.filter((val) => val)
237-
.filter((val) => !filteredValues.includes(val));
242+
.filter((val) => !filteredValues.includes(toTrimmedLowerCase(val)));
238243
filters.push({
239244
colId: i,
240245
displayedValues: [...new Set(xlsxDisplayedValues)],

‎packages/o-spreadsheet-engine/src/registries/criterion_registry.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { detectLink } from "../helpers/links";
2020
import { localizeContent } from "../helpers/locale";
2121
import { isNumberBetween } from "../helpers/misc";
2222
import { rangeReference } from "../helpers/references";
23+
import { toTrimmedLowerCase } from "../helpers/text_helper";
2324
import { _t } from "../translation";
2425
import { CellValue } from "../types/cells";
2526
import {
@@ -115,8 +116,8 @@ criterionEvaluatorRegistry.add("notContainsText", {
115116
criterionEvaluatorRegistry.add("isEqualText", {
116117
type: "isEqualText",
117118
isValueValid: (value: CellValue, criterion: EvaluatedCriterion) => {
118-
const strValue = String(value);
119-
return strValue.toLowerCase() === String(criterion.values[0]).toLowerCase();
119+
const strValue = toTrimmedLowerCase(String(value));
120+
return strValue === String(criterion.values[0]);
120121
},
121122
getErrorString: (criterion: EvaluatedCriterion) => {
122123
return _t('The value must be exactly "%s"', String(criterion.values[0]));
@@ -663,8 +664,8 @@ criterionEvaluatorRegistry.add("customFormula", {
663664
criterionEvaluatorRegistry.add("beginsWithText", {
664665
type: "beginsWithText",
665666
isValueValid: (value: CellValue, criterion: EvaluatedCriterion) => {
666-
const strValue = String(value);
667-
return strValue.toLowerCase().startsWith(String(criterion.values[0]).toLowerCase());
667+
const strValue = toTrimmedLowerCase(String(value));
668+
return strValue.startsWith(String(criterion.values[0]));
668669
},
669670
getErrorString: (criterion: EvaluatedCriterion) => {
670671
return _t('The value must be a text that begins with "%s"', String(criterion.values[0]));
@@ -679,8 +680,8 @@ criterionEvaluatorRegistry.add("beginsWithText", {
679680
criterionEvaluatorRegistry.add("endsWithText", {
680681
type: "endsWithText",
681682
isValueValid: (value: CellValue, criterion: EvaluatedCriterion) => {
682-
const strValue = String(value);
683-
return strValue.toLowerCase().endsWith(String(criterion.values[0]).toLowerCase());
683+
const strValue = toTrimmedLowerCase(String(value));
684+
return strValue.endsWith(String(criterion.values[0]));
684685
},
685686
getErrorString: (criterion: EvaluatedCriterion) => {
686687
return _t('The value must be a text that ends with "%s"', String(criterion.values[0]));

‎src/components/filters/filter_menu_value_list/filter_menu_value_list.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { SpreadsheetChildEnv } from "@odoo/o-spreadsheet-engine/types/spreadsheet_env";
22
import { Component, onWillUpdateProps, useRef, useState } from "@odoo/owl";
3-
import { deepEquals, positions, toLowerCase } from "../../../helpers";
3+
import { deepEquals, positions, toTrimmedLowerCase } from "../../../helpers";
44
import { fuzzyLookup } from "../../../helpers/search";
55
import { Position } from "../../../types";
66
import { FilterMenuValueItem } from "../filter_menu_item/filter_menu_value_item";
@@ -76,12 +76,12 @@ export class FilterMenuValueList extends Component<Props, SpreadsheetChildEnv> {
7676

7777
const cellValues = cells.map((val) => val.cellValue);
7878
const filterValues = filterValue?.filterType === "values" ? filterValue.hiddenValues : [];
79-
const normalizedFilteredValues = new Set(filterValues.map(toLowerCase));
79+
const normalizedFilteredValues = new Set(filterValues);
8080

8181
const set = new Set<string>();
8282
const values: (Value & { normalizedValue: string })[] = [];
8383
const addValue = (value: string) => {
84-
const normalizedValue = toLowerCase(value);
84+
const normalizedValue = toTrimmedLowerCase(value);
8585
if (!set.has(normalizedValue)) {
8686
values.push({
8787
string: value || "",

‎tests/table/filter_evaluation_plugin.test.ts‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ describe("Simple filter test", () => {
3434
test("Can update a filter", () => {
3535
createTableWithFilter(model, "A1:A5");
3636
updateFilter(model, "A1", ["2", "A"]);
37-
expect(model.getters.getFilterHiddenValues({ sheetId, col: 0, row: 0 })).toEqual(["2", "A"]);
37+
expect(model.getters.getFilterHiddenValues({ sheetId, col: 0, row: 0 })).toEqual(["2", "a"]);
3838
});
3939

4040
test("Can update a filter in readonly mode", () => {
4141
createTableWithFilter(model, "A1:A5");
4242
model.updateMode("readonly");
4343
updateFilter(model, "A1", ["2", "A"]);
44-
expect(model.getters.getFilterHiddenValues({ sheetId, col: 0, row: 0 })).toEqual(["2", "A"]);
44+
expect(model.getters.getFilterHiddenValues({ sheetId, col: 0, row: 0 })).toEqual(["2", "a"]);
4545
});
4646

4747
test("Update filter is correctly rejected when target is not inside a table", () => {
@@ -95,11 +95,11 @@ describe("Simple filter test", () => {
9595
sheetIdTo: sheet2Id,
9696
sheetNameTo: "Copy of Sheet1",
9797
});
98-
expect(getFilterHiddenValues(model, sheet2Id)).toMatchObject([{ zone: "B1:B3", value: ["C"] }]);
98+
expect(getFilterHiddenValues(model, sheet2Id)).toMatchObject([{ zone: "B1:B3", value: ["c"] }]);
9999
deleteColumns(model, ["A"], sheet2Id);
100100

101-
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "B1:B3", value: ["C"] }]);
102-
expect(getFilterHiddenValues(model, sheet2Id)).toMatchObject([{ zone: "A1:A3", value: ["C"] }]);
101+
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "B1:B3", value: ["c"] }]);
102+
expect(getFilterHiddenValues(model, sheet2Id)).toMatchObject([{ zone: "A1:A3", value: ["c"] }]);
103103
});
104104
});
105105

@@ -161,6 +161,15 @@ describe("Filter Evaluation", () => {
161161
expect(model.getters.isRowHidden(sheetId, 2)).toEqual(true);
162162
});
163163

164+
test("Filters ignore whitespaces", () => {
165+
setCellContent(model, "A2", "a");
166+
setCellContent(model, "A3", " a");
167+
setCellContent(model, "A4", "a ");
168+
updateFilter(model, "A2", ["a"]);
169+
expect(model.getters.isRowHidden(sheetId, 1)).toEqual(true);
170+
expect(model.getters.isRowHidden(sheetId, 2)).toEqual(true);
171+
expect(model.getters.isRowHidden(sheetId, 3)).toEqual(true);
172+
});
164173
test("Header is not filtered", () => {
165174
updateFilter(model, "A1", ["A1"]);
166175
expect(model.getters.isRowHidden(sheetId, 0)).toEqual(false);
@@ -195,17 +204,17 @@ describe("Filter Evaluation", () => {
195204

196205
test("Updating a table zone keep the filtered values if the filter header did not move", () => {
197206
updateFilter(model, "A1", ["A2"]);
198-
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A5", value: ["A2"] }]);
207+
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A5", value: ["a2"] }]);
199208
expect(model.getters.isRowHidden(sheetId, 1)).toEqual(true);
200209

201210
updateTableZone(model, "A1:A5", "A1:A6");
202-
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A6", value: ["A2"] }]);
211+
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A6", value: ["a2"] }]);
203212
expect(model.getters.isRowHidden(sheetId, 1)).toEqual(true);
204213
});
205214

206215
test("Updating a table zone drops the filtered values if the filter header moved", () => {
207216
updateFilter(model, "A1", ["A3"]);
208-
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A5", value: ["A3"] }]);
217+
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A5", value: ["a3"] }]);
209218
expect(model.getters.isRowHidden(sheetId, 2)).toEqual(true);
210219

211220
updateTableZone(model, "A1:A5", "A2:A5");
@@ -215,7 +224,7 @@ describe("Filter Evaluation", () => {
215224

216225
test("Updating a table zone updates the hidden rows", () => {
217226
updateFilter(model, "A1", ["A2"]);
218-
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A5", value: ["A2"] }]);
227+
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A5", value: ["a2"] }]);
219228
expect(model.getters.isRowHidden(sheetId, 1)).toEqual(true);
220229

221230
updateTableZone(model, "A1:A5", "E1:E3");
@@ -225,7 +234,7 @@ describe("Filter Evaluation", () => {
225234

226235
test("Removing the filters from a table updates the hidden rows", () => {
227236
updateFilter(model, "A1", ["A2"]);
228-
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A5", value: ["A2"] }]);
237+
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A5", value: ["a2"] }]);
229238
expect(model.getters.isRowHidden(sheetId, 1)).toEqual(true);
230239

231240
updateTableConfig(model, "A1:A5", { hasFilters: false });
@@ -377,6 +386,27 @@ describe("Filter criterion test", () => {
377386
}
378387
);
379388

389+
test.each(["beginsWithText", "endsWithText", "isEqualText"] as const)(
390+
"Can filter based on a text criterion %s ignoring lowercase/uppercase and whitespaces",
391+
(type) => {
392+
const grid = {
393+
A2: "a",
394+
A3: " a",
395+
A4: "a ",
396+
A5: "A",
397+
A6: "b",
398+
};
399+
setGrid(model, grid);
400+
createTableWithFilter(model, "A1:A6");
401+
updateFilterCriterion(model, "A1", {
402+
type,
403+
values: ["a"],
404+
});
405+
406+
expect(getFilteredRows()).toEqual([5]);
407+
}
408+
);
409+
380410
test.each([
381411
["isEqual", ["1"], [2, 3]],
382412
["isNotEqual", ["1"], [1]],

0 commit comments

Comments
 (0)