[FIX] Datavalidation: coreView plugin should not dispatch

See #5216 - 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 #5216.

closes #5217

Task: 4241141
X-original-commit: fd31d5f
Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Signed-off-by: Rémi Rahir (rar) <[email protected]>

Author: rrahir

src/plugins/index.ts

@@ -42,6 +42,7 @@ import {
   UIOptionsPlugin,
 } from "./ui_feature";
 import { CellComputedStylePlugin } from "./ui_feature/cell_computed_style";
+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";
@@ -89,7 +90,8 @@ export const featurePluginRegistry = new Registry<UIPluginConstructor>()
   .add("history", HistoryPlugin)
   .add("data_cleanup", DataCleanupPlugin)
   .add("table_autofill", TableAutofillPlugin)
-  .add("table_ui_resize", TableResizeUI);
+  .add("table_ui_resize", TableResizeUI)
+  .add("datavalidation_insert", DataValidationInsertionPlugin);
 
 // Plugins which have a state, but which should not be shared in collaborative
 export const statefulUIPluginRegistry = new Registry<UIPluginConstructor>()

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;
   }

src/plugins/ui_feature/datavalidation_insertion.ts

@@ -0,0 +1,37 @@
+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" });
+              // In this case, a cell has been updated in the core plugin but
+              // not yet evaluated. This can occur after a paste operation.
+            } 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" });
+            }
+          }
+        }
+    }
+  }
+}

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,45 @@ describe("Collaborative range manipulation", () => {
       "D4"
     );
   });
+
+  test("Can copy boolean datavalidation while preserving the cell values", () => {
+    setCellContent(alice, "A1", "TRUE");
+    setCellContent(alice, "A3", "not a boolean");
+    setCellContent(alice, "A4", "=TRANSPOSE(A1)");
+    setCellContent(alice, "A5", "=TEXT(5)");
+    setCellContent(alice, "A6", "=NOT(A1)");
+    setCellContent(alice, "A7", "7");
+    addDataValidation(alice, "A1:A7", "id", { type: "isBoolean", values: [] });
+
+    copy(alice, "A1:A7");
+    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" // A2 was empty, which is falsy
+    );
+    expect([alice, bob, charlie]).toHaveSynchronizedValue(
+      (user) => getCell(user, "B3")?.content,
+      "FALSE" // text is not a boolean -> falsy
+    );
+    expect([alice, bob, charlie]).toHaveSynchronizedValue(
+      (user) => getCell(user, "B4")?.content,
+      "=TRANSPOSE(B1)" // is truthy
+    );
+    expect([alice, bob, charlie]).toHaveSynchronizedValue(
+      (user) => getCell(user, "B5")?.content,
+      "FALSE" // text is not a boolean -> falsy
+    );
+    expect([alice, bob, charlie]).toHaveSynchronizedValue(
+      (user) => getCell(user, "B6")?.content,
+      "=NOT(B1)"
+    );
+    expect([alice, bob, charlie]).toHaveSynchronizedValue(
+      (user) => getCell(user, "B7")?.content,
+      "FALSE" // a number which does not represent a boolean is falsy
+    );
+  });
 });