Skip to content

Commit

Permalink
[FIX] EditionPlugin: data validation duplicate values
Browse files Browse the repository at this point in the history
Previously, when creating data validation with either a list of values
or a range of values, it resulted in an error being thrown by the
auto-complete dropdown. This occurred because the values were being used
as keys, and by definition, having duplicate keys in a list is nonsensical.

This commit addresses the problem by eliminating duplicate values from
the list.

closes #3843

Taskid: 3768392
Signed-off-by: Adrien Minne (adrm) <adrm@odoo.com>
  • Loading branch information
dhrp-odoo committed Mar 26, 2024
1 parent ed7d3fa commit c2eed35
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 5 deletions.
3 changes: 3 additions & 0 deletions src/plugins/core/data_validation.ts
Expand Up @@ -162,7 +162,10 @@ export class DataValidationPlugin

if (newRule.criterion.type === "isBoolean") {
this.setCenterStyleToBooleanCells(newRule);
} else if (newRule.criterion.type === "isValueInList") {
newRule.criterion.values = Array.from(new Set(newRule.criterion.values));
}

const adaptedRules = this.removeRangesFromRules(sheetId, newRule.ranges, rules);
const ruleIndex = adaptedRules.findIndex((rule) => rule.id === newRule.id);

Expand Down
14 changes: 9 additions & 5 deletions src/plugins/ui_stateful/edition.ts
Expand Up @@ -758,11 +758,15 @@ export class EditionPlugin extends UIPlugin {
values = rule.criterion.values;
} else {
const range = this.getters.getRangeFromSheetXC(this.sheetId, rule.criterion.values[0]);
values = this.getters
.getRangeValues(range)
.filter(isNotNull)
.map((value) => value.toString())
.filter((val) => val !== "");
values = Array.from(
new Set(
this.getters
.getRangeValues(range)
.filter(isNotNull)
.map((value) => value.toString())
.filter((val) => val !== "")
)
);
}
const composerContent = this.getCurrentContent();
if (composerContent && composerContent !== this.getInitialComposerContent()) {
Expand Down
10 changes: 10 additions & 0 deletions tests/data_validation/data_validation_core_plugin.test.ts
Expand Up @@ -80,6 +80,16 @@ describe("Data validation", () => {
]);
});

test("Duplicate values will be filtered out when adding a rule for value in the list", () => {
addDataValidation(model, "A1", "id", {
type: "isValueInList",
values: ["1", "1", "2", "3", "2"],
displayStyle: "arrow",
});

expect(getDataValidationRules(model, sheetId)[0].criterion.values).toEqual(["1", "2", "3"]);
});

test("Adding a rule with an existing id will replace the old one", () => {
addDataValidation(model, "A1", "id", { type: "textContains", values: ["1"] });
addDataValidation(model, "A1:C2", "id", { type: "isBetween", values: ["1", "5"] });
Expand Down
27 changes: 27 additions & 0 deletions tests/data_validation/data_validation_list_component.test.ts
Expand Up @@ -254,6 +254,33 @@ describe("autocomplete in composer", () => {
expect(values[1].textContent).toBe("ok");
expect(values[2].textContent).toBe("thing");
});

test("Duplicate values will be removed before sending proposals to the autocomplete dropdown in data validation with range", async () => {
setCellContent(model, "A2", "ok");
setCellContent(model, "A3", "hello");
setCellContent(model, "A4", "ok");
addDataValidation(model, "A1", "id", {
type: "isValueInRange",
values: ["A2:A4"],
displayStyle: "arrow",
});

({ fixture, parent } = await mountComposerWrapper(model));
await typeInComposer("");
expect(model.getters.getAutoCompleteDataValidationValues()).toMatchObject(["ok", "hello"]);
});

test("Duplicate values will be removed before sending proposals to the autocomplete dropdown in data validation with list", async () => {
addDataValidation(model, "A1", "id", {
type: "isValueInList",
values: ["ok", "hello", "ok", "hello"],
displayStyle: "arrow",
});

({ fixture, parent } = await mountComposerWrapper(model));
await typeInComposer("");
expect(model.getters.getAutoCompleteDataValidationValues()).toMatchObject(["ok", "hello"]);
});
});

describe("Selection arrow icon in grid", () => {
Expand Down

0 comments on commit c2eed35

Please sign in to comment.