Skip to content

Commit a14af9c

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 a14af9c

File tree

6 files changed

+154
-112
lines changed

6 files changed

+154
-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: 16 additions & 6 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,9 @@ 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+
}
142145
this.filterValues[sheetId][id] = value;
143146
}
144147

@@ -163,7 +166,7 @@ export class FilterEvaluationPlugin extends UIPlugin {
163166
continue;
164167
}
165168
if (filterValue.filterType === "values") {
166-
const filteredValues = filterValue.hiddenValues?.map(toLowerCase);
169+
const filteredValues = filterValue.hiddenValues;
167170
if (!filteredValues) continue;
168171
const filteredValuesSet = new Set(filteredValues);
169172
for (let row = filteredZone.top; row <= filteredZone.bottom; row++) {
@@ -178,7 +181,7 @@ export class FilterEvaluationPlugin extends UIPlugin {
178181

179182
const evaluatedCriterionValues = filterValue.values.map((value) => {
180183
if (!value.startsWith("=")) {
181-
return parseLiteral(value, DEFAULT_LOCALE);
184+
return parseLiteral(toTrimmedLowerCase(value), DEFAULT_LOCALE);
182185
}
183186
return this.getters.evaluateFormula(sheetId, value) ?? "";
184187
});
@@ -194,7 +197,14 @@ export class FilterEvaluationPlugin extends UIPlugin {
194197
for (let row = filteredZone.top; row <= filteredZone.bottom; row++) {
195198
const position = { sheetId, col: filter.col, row };
196199
const value = this.getters.getEvaluatedCell(position).value ?? "";
197-
if (!evaluator.isValueValid(value, evaluatedCriterion, this.getters, sheetId)) {
200+
if (
201+
!evaluator.isValueValid(
202+
toTrimmedLowerCase(value.toString()),
203+
evaluatedCriterion,
204+
this.getters,
205+
sheetId
206+
)
207+
) {
198208
hiddenRows.add(row);
199209
}
200210
}
@@ -205,7 +215,7 @@ export class FilterEvaluationPlugin extends UIPlugin {
205215

206216
private getCellValueAsString(sheetId: UID, col: number, row: number): string {
207217
const value = this.getters.getEvaluatedCell({ sheetId, col, row }).formattedValue;
208-
return value.toLowerCase();
218+
return toTrimmedLowerCase(value);
209219
}
210220

211221
exportForExcel(data: ExcelWorkbookData) {
@@ -234,7 +244,7 @@ export class FilterEvaluationPlugin extends UIPlugin {
234244
if (filteredValues.length) {
235245
const xlsxDisplayedValues = valuesInFilterZone
236246
.filter((val) => val)
237-
.filter((val) => !filteredValues.includes(val));
247+
.filter((val) => !filteredValues.includes(toTrimmedLowerCase(val)));
238248
filters.push({
239249
colId: i,
240250
displayedValues: [...new Set(xlsxDisplayedValues)],

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

Lines changed: 5 additions & 4 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]));
@@ -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: 41 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,16 @@ 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+
});
173+
164174
test("Header is not filtered", () => {
165175
updateFilter(model, "A1", ["A1"]);
166176
expect(model.getters.isRowHidden(sheetId, 0)).toEqual(false);
@@ -195,17 +205,17 @@ describe("Filter Evaluation", () => {
195205

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

201211
updateTableZone(model, "A1:A5", "A1:A6");
202-
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A6", value: ["A2"] }]);
212+
expect(getFilterHiddenValues(model, sheetId)).toMatchObject([{ zone: "A1:A6", value: ["a2"] }]);
203213
expect(model.getters.isRowHidden(sheetId, 1)).toEqual(true);
204214
});
205215

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

211221
updateTableZone(model, "A1:A5", "A2:A5");
@@ -215,7 +225,7 @@ describe("Filter Evaluation", () => {
215225

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

221231
updateTableZone(model, "A1:A5", "E1:E3");
@@ -225,7 +235,7 @@ describe("Filter Evaluation", () => {
225235

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

231241
updateTableConfig(model, "A1:A5", { hasFilters: false });
@@ -377,6 +387,27 @@ describe("Filter criterion test", () => {
377387
}
378388
);
379389

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

0 commit comments

Comments
 (0)