[IMP] data_validation: add formula autocomplete and validation

This commit adds autocomplete and validation checks for formulas used in data validation rules. closes #5141 Task: 4084829 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
odoo · Nov 20, 2024 · bbc540b · bbc540b
1 parent 022f3b3
commit bbc540b
Show file tree

Hide file tree

Showing 11 changed files with 175 additions and 73 deletions.
diff --git a/src/components/side_panel/data_validation/dv_criterion_form/dv_input/dv_input.ts b/src/components/side_panel/data_validation/dv_criterion_form/dv_input/dv_input.ts
@@ -3,6 +3,7 @@ import { canonicalizeContent } from "../../../../../helpers/locale";
 import { dataValidationEvaluatorRegistry } from "../../../../../registries/data_validation_registry";
 import { _t } from "../../../../../translation";
 import { DataValidationCriterionType, SpreadsheetChildEnv } from "../../../../../types";
+import { StandaloneComposer } from "../../../../composer/standalone_composer/standalone_composer";
 import { css } from "../../../../helpers";
 
 interface Props {
@@ -43,6 +44,7 @@ export class DataValidationInput extends Component<Props, SpreadsheetChildEnv> {
     focused: false,
     onBlur: () => {},
   };
+  static components = { StandaloneComposer: StandaloneComposer };
 
   inputRef = useRef("input");
 
@@ -61,11 +63,6 @@ export class DataValidationInput extends Component<Props, SpreadsheetChildEnv> {
     shouldDisplayError: !!this.props.value, // Don't display error if user inputted nothing yet
   });
 
-  onValueChanged(ev: Event) {
-    this.state.shouldDisplayError = true;
-    this.props.onValueChanged((ev.target as HTMLInputElement).value);
-  }
-
   get placeholder(): string {
     const evaluator = dataValidationEvaluatorRegistry.get(this.props.criterionType);
 
@@ -78,6 +75,31 @@ export class DataValidationInput extends Component<Props, SpreadsheetChildEnv> {
     return _t("Value or formula");
   }
 
+  get allowedValues(): string {
+    const evaluator = dataValidationEvaluatorRegistry.get(this.props.criterionType);
+    return evaluator.allowedValues ?? "any";
+  }
+
+  onInputValueChanged(ev: Event) {
+    this.state.shouldDisplayError = true;
+    this.props.onValueChanged((ev.target as HTMLInputElement).value);
+  }
+
+  onChangeComposerValue(str: string) {
+    this.state.shouldDisplayError = true;
+    this.props.onValueChanged(str);
+  }
+
+  getDataValidationRuleInputComposerProps(): StandaloneComposer["props"] {
+    return {
+      onConfirm: (str: string) => this.onChangeComposerValue(str),
+      composerContent: this.props.value,
+      placeholder: this.placeholder,
+      class: "o-sidePanel-composer",
+      defaultRangeSheetId: this.env.model.getters.getActiveSheetId(),
+    };
+  }
+
   get errorMessage(): string | undefined {
     if (!this.state.shouldDisplayError) {
       return undefined;

diff --git a/src/components/side_panel/data_validation/dv_criterion_form/dv_input/dv_input.xml b/src/components/side_panel/data_validation/dv_criterion_form/dv_input/dv_input.xml
@@ -1,20 +1,25 @@
 <templates>
   <t t-name="o-spreadsheet-DataValidationInput">
-    <div class="o-dv-input position-relative w-100">
-      <input
-        type="text"
-        t-ref="input"
-        t-on-input="onValueChanged"
-        t-att-value="props.value"
-        class="o-input"
-        t-att-class="{
-            'o-invalid border-danger position-relative': errorMessage,
-          }"
-        t-att-title="errorMessage"
-        t-att-placeholder="placeholder"
-        t-on-keydown="props.onKeyDown"
-        t-on-blur="props.onBlur"
-      />
+    <div class="o-dv-input position-relative w-100 p-1">
+      <t t-if="allowedValues === 'onlyLiterals'">
+        <input
+          type="text"
+          t-ref="input"
+          t-on-input="onInputValueChanged"
+          t-att-value="props.value"
+          class="o-input"
+          t-att-class="{
+              'o-invalid border-danger position-relative': errorMessage,
+            }"
+          t-att-title="errorMessage"
+          t-att-placeholder="placeholder"
+          t-on-keydown="props.onKeyDown"
+          t-on-blur="props.onBlur"
+        />
+      </t>
+      <t t-else="">
+        <StandaloneComposer t-props="getDataValidationRuleInputComposerProps()"/>
+      </t>
       <span
         t-if="errorMessage"
         class="error-icon text-danger position-absolute d-flex align-items-center"

diff --git a/src/components/side_panel/data_validation/dv_editor/dv_editor.ts b/src/components/side_panel/data_validation/dv_editor/dv_editor.ts
@@ -5,13 +5,16 @@ import { canonicalizeContent } from "../../../../helpers/locale";
 import { dataValidationEvaluatorRegistry } from "../../../../registries/data_validation_registry";
 import {
   AddDataValidationCommand,
+  CancelledReason,
   DataValidationCriterion,
   DataValidationCriterionType,
   DataValidationRule,
   DataValidationRuleData,
   SpreadsheetChildEnv,
 } from "../../../../types";
 import { SelectionInput } from "../../../selection_input/selection_input";
+import { DVTerms } from "../../../translations_terms";
+import { ValidationMessages } from "../../../validation_messages/validation_messages";
 import { Section } from "../../components/section/section";
 import { SelectMenu } from "../../select_menu/select_menu";
 import {
@@ -27,18 +30,19 @@ interface Props {
 
 interface State {
   rule: DataValidationRuleData;
+  errors: CancelledReason[];
 }
 
 export class DataValidationEditor extends Component<Props, SpreadsheetChildEnv> {
   static template = "o-spreadsheet-DataValidationEditor";
-  static components = { SelectionInput, SelectMenu, Section };
+  static components = { SelectionInput, SelectMenu, Section, ValidationMessages };
   static props = {
     rule: { type: Object, optional: true },
     onExit: Function,
     onCloseSidePanel: { type: Function, optional: true },
   };
 
-  state = useState<State>({ rule: this.defaultDataValidationRule });
+  state = useState<State>({ rule: this.defaultDataValidationRule, errors: [] });
 
   setup() {
     if (this.props.rule) {
@@ -71,16 +75,14 @@ export class DataValidationEditor extends Component<Props, SpreadsheetChildEnv>
   }
 
   onSave() {
-    if (!this.canSave) {
-      return;
+    if (this.state.rule) {
+      const result = this.env.model.dispatch("ADD_DATA_VALIDATION_RULE", this.dispatchPayload);
+      if (!result.isSuccessful) {
+        this.state.errors = result.reasons;
+      } else {
+        this.props.onExit();
+      }
     }
-    this.env.model.dispatch("ADD_DATA_VALIDATION_RULE", this.dispatchPayload);
-    this.props.onExit();
-  }
-
-  get canSave(): boolean {
-    return this.env.model.canDispatch("ADD_DATA_VALIDATION_RULE", this.dispatchPayload)
-      .isSuccessful;
   }
 
   get dispatchPayload(): Omit<AddDataValidationCommand, "type"> {
@@ -130,4 +132,8 @@ export class DataValidationEditor extends Component<Props, SpreadsheetChildEnv>
   get criterionComponent(): ComponentConstructor | undefined {
     return dataValidationPanelCriteriaRegistry.get(this.state.rule.criterion.type).component;
   }
+
+  get errorMessages(): string[] {
+    return this.state.errors.map((error) => DVTerms.Errors[error] || DVTerms.Errors.Unexpected);
+  }
 }
diff --git a/src/components/side_panel/data_validation/dv_editor/dv_editor.xml b/src/components/side_panel/data_validation/dv_editor/dv_editor.xml
@@ -37,14 +37,12 @@
       <Section>
         <div class="o-sidePanelButtons">
           <button t-on-click="props.onExit" class="o-dv-cancel o-button">Cancel</button>
-          <button
-            t-on-click="onSave"
-            class="o-dv-save o-button primary"
-            t-att-class="{'o-disabled': !canSave }">
-            Save
-          </button>
+          <button t-on-click="onSave" class="o-dv-save o-button primary">Save</button>
         </div>
       </Section>
+      <Section>
+        <ValidationMessages messages="errorMessages" msgType="'error'"/>
+      </Section>
     </div>
   </t>
 </templates>
diff --git a/src/components/translations_terms.ts b/src/components/translations_terms.ts
@@ -144,6 +144,17 @@ export const DVTerms = {
     numberValue: _t("The value must be a number"),
     dateValue: _t("The value must be a date"),
     validRange: _t("The value must be a valid range"),
+    validFormula: _t("The formula must be valid"),
+  },
+  Errors: {
+    [CommandResult.InvalidRange]: _t("The range is invalid."),
+    [CommandResult.InvalidDataValidationCriterionValue]: _t(
+      "One or more of the provided criteria values are invalid. Please review and correct them."
+    ),
+    [CommandResult.InvalidNumberOfCriterionValues]: _t(
+      "One or more of the provided criteria values are missing."
+    ),
+    Unexpected: _t("The rule is invalid for an unknown reason."),
   },
 };
 

diff --git a/src/plugins/core/data_validation.ts b/src/plugins/core/data_validation.ts
@@ -1,3 +1,4 @@
+import { compile } from "../../formulas";
 import {
   copyRangeWithNewSheetId,
   deepCopy,
@@ -80,6 +81,7 @@ export class DataValidationPlugin
           cmd,
           this.chainValidations(
             this.checkEmptyRange,
+            this.checkValidRange,
             this.checkCriterionTypeIsValid,
             this.checkCriterionHasValidNumberOfValues,
             this.checkCriterionValuesAreValid
@@ -285,19 +287,27 @@ export class DataValidationPlugin
   private checkCriterionValuesAreValid(cmd: AddDataValidationCommand): CommandResult {
     const criterion = cmd.rule.criterion;
     const evaluator = dataValidationEvaluatorRegistry.get(criterion.type);
-    if (
-      criterion.values.some((value) => {
-        if (value.startsWith("=")) {
-          return evaluator.allowedValues === "onlyLiterals";
-        } else if (evaluator.allowedValues === "onlyFormulas") {
-          return true;
-        } else {
-          return !evaluator.isCriterionValueValid(value);
-        }
-      })
-    ) {
+    const isInvalid = (value: string) => {
+      if (evaluator.allowedValues === "onlyFormulas" && !value.startsWith("=")) {
+        return true;
+      }
+      if (value.startsWith("=")) {
+        return evaluator.allowedValues === "onlyLiterals" || compile(value).isBadExpression;
+      }
+      return !evaluator.isCriterionValueValid(value);
+    };
+    if (criterion.values.some(isInvalid)) {
       return CommandResult.InvalidDataValidationCriterionValue;
     }
     return CommandResult.Success;
   }
+
+  private checkValidRange(cmd: AddDataValidationCommand): CommandResult {
+    const ranges = cmd.ranges.map((range) => this.getters.getRangeFromRangeData(range));
+    const stringRanges = ranges.map((range) => this.getters.getRangeString(range, cmd.sheetId));
+    if (stringRanges.some((xc) => !this.getters.isRangeValid(xc))) {
+      return CommandResult.InvalidRange;
+    }
+    return CommandResult.Success;
+  }
 }
diff --git a/src/plugins/ui_core_views/evaluation_data_validation.ts b/src/plugins/ui_core_views/evaluation_data_validation.ts
@@ -1,3 +1,4 @@
+import { DVTerms } from "../../components/translations_terms";
 import { compile } from "../../formulas";
 import { getCellPositionsInRanges, isInside, lazy } from "../../helpers";
 import { dataValidationEvaluatorRegistry } from "../../registries/data_validation_registry";
@@ -82,9 +83,10 @@ export class EvaluationDataValidationPlugin extends UIPlugin {
   ): string | undefined {
     const evaluator = dataValidationEvaluatorRegistry.get(criterionType);
     if (value.startsWith("=")) {
-      return evaluator.allowedValues === "onlyLiterals"
-        ? _t("The value must not be a formula")
-        : undefined;
+      if (evaluator.allowedValues === "onlyLiterals") {
+        return _t("The value must not be a formula");
+      }
+      return this.isValidFormula(value) ? undefined : DVTerms.CriterionError.validFormula;
     } else if (evaluator.allowedValues === "onlyFormulas") {
       return _t("The value must be a formula");
     }
@@ -119,6 +121,10 @@ export class EvaluationDataValidationPlugin extends UIPlugin {
     return error ? { error, rule, isValid: false } : VALID_RESULT;
   }
 
+  private isValidFormula(value: string): boolean {
+    return !compile(value).isBadExpression;
+  }
+
   private getValidationResultForCell(cellPosition: CellPosition): ValidationResult {
     const { col, row, sheetId } = cellPosition;
     if (!this.validationResults[sheetId]) {

diff --git a/src/registries/data_validation_registry.ts b/src/registries/data_validation_registry.ts
@@ -78,7 +78,6 @@ export type DataValidationCriterionEvaluator = {
    * The value should be in canonical form (non-localized).
    */
   isCriterionValueValid: (value: string) => boolean;
-
   /** Return the number of values that the criterion must contains. Return undefined if the criterion can have any number of values */
   numberOfValues: (criterion: DataValidationCriterion) => number | undefined;
   name: string;