From 440d2595547272e134ca62943619f3e667bfd3fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Rahir=20=28rar=29?= Date: Wed, 9 Oct 2024 16:41:47 +0200 Subject: [PATCH] [FIX] Datavalidation: coreView plugin should not dispatch See #5070 - CoreView plugins replay remote revisions and should not take the risk to dispatch during the replay. The code that was assigning arbitrary default values to the cells of a boolean datavalidation has been moved to a UI feature plugin, hence avoiding the problem raised in PR #5070. Task: 4241141 --- src/plugins/index.ts | 4 ++- .../evaluation_data_validation.ts | 15 -------- .../ui_feature/datavalidation_insertion.ts | 35 +++++++++++++++++++ .../collaborative_clipboard.test.ts | 18 ++++++++++ 4 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 src/plugins/ui_feature/datavalidation_insertion.ts diff --git a/src/plugins/index.ts b/src/plugins/index.ts index 89ad429fe3..8590ffef65 100644 --- a/src/plugins/index.ts +++ b/src/plugins/index.ts @@ -36,6 +36,7 @@ import { SortPlugin, UIOptionsPlugin, } from "./ui_feature"; +import { DataValidationInsertionPlugin } from "./ui_feature/datavalidation_insertion"; import { HistoryPlugin } from "./ui_feature/local_history"; import { SplitToColumnsPlugin } from "./ui_feature/split_to_columns"; import { TableAutofillPlugin } from "./ui_feature/table_autofill"; @@ -78,7 +79,8 @@ export const featurePluginRegistry = new Registry() .add("collaborative", CollaborativePlugin) .add("history", HistoryPlugin) .add("data_cleanup", DataCleanupPlugin) - .add("table_autofill", TableAutofillPlugin); + .add("table_autofill", TableAutofillPlugin) + .add("datavalidation_insert", DataValidationInsertionPlugin); // Plugins which have a state, but which should not be shared in collaborative export const statefulUIPluginRegistry = new Registry() diff --git a/src/plugins/ui_core_views/evaluation_data_validation.ts b/src/plugins/ui_core_views/evaluation_data_validation.ts index 6728c49f3a..cfc6ac00d6 100644 --- a/src/plugins/ui_core_views/evaluation_data_validation.ts +++ b/src/plugins/ui_core_views/evaluation_data_validation.ts @@ -56,27 +56,12 @@ export class EvaluationDataValidationPlugin extends UIPlugin { } switch (cmd.type) { case "ADD_DATA_VALIDATION_RULE": - const ranges = cmd.ranges.map((range) => this.getters.getRangeFromRangeData(range)); - if (cmd.rule.criterion.type === "isBoolean") { - this.setContentToBooleanCells({ ...cmd.rule, ranges }); - } - delete this.validationResults[cmd.sheetId]; - break; case "REMOVE_DATA_VALIDATION_RULE": delete this.validationResults[cmd.sheetId]; break; } } - private setContentToBooleanCells(rule: DataValidationRule) { - for (const position of getCellPositionsInRanges(rule.ranges)) { - const evaluatedCell = this.getters.getEvaluatedCell(position); - if (evaluatedCell.type !== CellValueType.boolean) { - this.dispatch("UPDATE_CELL", { ...position, content: "FALSE" }); - } - } - } - isDataValidationInvalid(cellPosition: CellPosition): boolean { return !this.getValidationResultForCell(cellPosition).isValid; } diff --git a/src/plugins/ui_feature/datavalidation_insertion.ts b/src/plugins/ui_feature/datavalidation_insertion.ts new file mode 100644 index 0000000000..60696e39bf --- /dev/null +++ b/src/plugins/ui_feature/datavalidation_insertion.ts @@ -0,0 +1,35 @@ +import { getCellPositionsInRanges, isBoolean } from "../../helpers"; +import { CellValueType, Command, isMatrix } from "../../types/index"; +import { UIPlugin } from "../ui_plugin"; + +export class DataValidationInsertionPlugin extends UIPlugin { + handle(cmd: Command) { + switch (cmd.type) { + case "ADD_DATA_VALIDATION_RULE": + if (cmd.rule.criterion.type === "isBoolean") { + const ranges = cmd.ranges.map((range) => this.getters.getRangeFromRangeData(range)); + for (const position of getCellPositionsInRanges(ranges)) { + const cell = this.getters.getCell(position); + const evaluatedCell = this.getters.getEvaluatedCell(position); + + if (!cell?.content) { + this.dispatch("UPDATE_CELL", { ...position, content: "FALSE" }); + } else if (cell?.content && evaluatedCell.type === CellValueType.empty) { + let value: string | undefined; + if (cell.content.startsWith("=")) { + const result = this.getters.evaluateFormula(position.sheetId, cell.content); + value = (isMatrix(result) ? result[0][0] : result)?.toString(); + } else { + value = cell.content; + } + if (!value || !isBoolean(value)) { + this.dispatch("UPDATE_CELL", { ...position, content: "FALSE" }); + } + } else if (evaluatedCell.type !== CellValueType.boolean) { + this.dispatch("UPDATE_CELL", { ...position, content: "FALSE" }); + } + } + } + } + } +} diff --git a/tests/collaborative/collaborative_clipboard.test.ts b/tests/collaborative/collaborative_clipboard.test.ts index f6ae6be8fb..2c3abd102b 100644 --- a/tests/collaborative/collaborative_clipboard.test.ts +++ b/tests/collaborative/collaborative_clipboard.test.ts @@ -3,6 +3,7 @@ import { LineChartDefinition } from "../../src/types/chart"; import { MockTransportService } from "../__mocks__/transport_service"; import { addColumns, + addDataValidation, copy, createChart, createSheet, @@ -123,4 +124,21 @@ describe("Collaborative range manipulation", () => { "D4" ); }); + + test("Can copy boolean datavalidation while preserving the cell values", () => { + setCellContent(alice, "A1", "TRUE"); + setCellContent(alice, "A2", "FALSE"); + addDataValidation(alice, "A1:A2", "id", { type: "isBoolean", values: [] }); + + copy(alice, "A1:A2"); + paste(alice, "B1"); + expect([alice, bob, charlie]).toHaveSynchronizedValue( + (user) => getCell(user, "B1")?.content, + "TRUE" + ); + expect([alice, bob, charlie]).toHaveSynchronizedValue( + (user) => getCell(user, "B2")?.content, + "FALSE" + ); + }); });