[IMP] Errors: Add error origin position for #SPILL errors

Currently, Errors that spill because they would collide with other cells
content have an error message that references the problematic cell but
the link to jump to that cell is missing.

closes #8238

Task: 5959985
X-original-commit: f08865b
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>

Author: rrahir

src/helpers/cells/cell_evaluation.ts

@@ -2,6 +2,7 @@ import { isEvaluationError, toString } from "../../functions/helpers";
 import {
   BooleanCell,
   Cell,
+  CellPosition,
   CellValue,
   CellValueType,
   DEFAULT_LOCALE,
@@ -88,12 +89,12 @@ function _createEvaluatedCell(
   locale: Locale,
   cell?: Cell
 ): EvaluatedCell {
-  let { value, format, message } = functionResult;
+  let { value, format, message, errorOriginPosition } = functionResult;
   format = cell?.format || format;
 
   const formattedValue = formatValue(value, { format, locale });
   if (isEvaluationError(value)) {
-    return errorCell(value, message);
+    return errorCell(value, message, errorOriginPosition);
   }
   if (value === null) {
     return emptyCell(format);
@@ -185,13 +186,14 @@ function booleanCell(
   };
 }
 
-function errorCell(value: string, message?: string): ErrorCell {
+function errorCell(value: string, message?: string, errorOriginPosition?: CellPosition): ErrorCell {
   return {
     value,
     formattedValue: value,
     message,
     type: CellValueType.error,
     isAutoSummable: false,
     defaultAlign: "center",
+    errorOriginPosition,
   };
 }

src/plugins/ui_core_views/cell_evaluation/evaluator.ts

@@ -6,7 +6,6 @@ import {
   excludeTopLeft,
   lazy,
   positionToZone,
-  toXC,
   union,
 } from "../../../helpers";
 import { createEvaluatedCell, evaluateLiteral } from "../../../helpers/cells";
@@ -360,6 +359,7 @@ export class Evaluator {
     } catch (e) {
       e.value = e?.value || CellErrorType.GenericError;
       e.message = e?.message || implementationErrorMessage;
+      e.errorOriginPosition = e?.errorOriginPosition;
       return createEvaluatedCell(e);
     } finally {
       this.cellsBeingComputed.delete(cellId);
@@ -506,10 +506,8 @@ export class Evaluator {
       ) {
         this.blockedArrayFormulas.add(formulaPosition);
         throw new SplillBlockedError(
-          _t(
-            "Array result was not expanded because it would overwrite data in %s.",
-            toXC(position.col, position.row)
-          )
+          _t("Array result was not expanded because it would overwrite data."),
+          position
         );
       }
       this.blockedArrayFormulas.delete(formulaPosition);

src/types/errors.ts

@@ -1,4 +1,5 @@
 import { _t } from "../translation";
+import { CellPosition } from "./misc";
 
 export const CellErrorType = {
   NotAvailable: "#N/A",
@@ -55,7 +56,10 @@ export class UnknownFunctionError extends EvaluationError {
 }
 
 export class SplillBlockedError extends EvaluationError {
-  constructor(message = _t("Spill range is not empty")) {
+  constructor(
+    message = _t("Spill range is not empty"),
+    readonly errorOriginPosition?: CellPosition
+  ) {
     super(message, CellErrorType.SpilledBlocked);
   }
 }

tests/evaluation/evaluation_formula_array.test.ts

@@ -18,7 +18,7 @@ import {
   unMerge,
 } from "../test_helpers/commands_helpers";
 import { getCellContent, getCellError, getEvaluatedCell } from "../test_helpers/getters_helpers";
-import { addToRegistry } from "../test_helpers/helpers";
+import { addToRegistry, toCellPosition } from "../test_helpers/helpers";
 
 let model: Model;
 let sheetId: UID;
@@ -246,18 +246,25 @@ describe("evaluate formulas that return an array", () => {
 
   describe("result array can collides with other cell", () => {
     test("throw error on the formula when collide with cell having content", () => {
+      const sheetId = model.getters.getActiveSheetId();
       setCellContent(model, "B2", "kikou");
       setCellContent(model, "A1", "=MFILL(2,2, 42)");
       expect(getEvaluatedCell(model, "A1").value).toBe("#SPILL!");
       expect(getCellError(model, "A1")).toBe(
-        "Array result was not expanded because it would overwrite data in B2."
+        "Array result was not expanded because it would overwrite data."
+      );
+      expect(getEvaluatedCell(model, "A1").errorOriginPosition).toStrictEqual(
+        toCellPosition(sheetId, "B2")
       );
 
       setCellContent(model, "A4", "kikou");
       setCellContent(model, "A3", "=MFILL(2,2, 42)");
       expect(getEvaluatedCell(model, "A3").value).toBe("#SPILL!");
       expect(getCellError(model, "A3")).toBe(
-        "Array result was not expanded because it would overwrite data in A4."
+        "Array result was not expanded because it would overwrite data."
+      );
+      expect(getEvaluatedCell(model, "A1").errorOriginPosition).toStrictEqual(
+        toCellPosition(sheetId, "B2")
       );
     });
 
@@ -266,7 +273,10 @@ describe("evaluate formulas that return an array", () => {
       setCellContent(model, "A1", "=MFILL(2,2, 42)");
       expect(getEvaluatedCell(model, "A1").value).toBe("#SPILL!");
       expect(getCellError(model, "A1")).toBe(
-        "Array result was not expanded because it would overwrite data in B2."
+        "Array result was not expanded because it would overwrite data."
+      );
+      expect(getEvaluatedCell(model, "A1").errorOriginPosition).toStrictEqual(
+        toCellPosition(sheetId, "B2")
       );
       expect(getEvaluatedCell(model, "A2").value).toBe(null);
       expect(getEvaluatedCell(model, "B1").value).toBe(null);
@@ -279,7 +289,10 @@ describe("evaluate formulas that return an array", () => {
       setCellContent(model, "A3", "kikou");
       expect(getEvaluatedCell(model, "A1").value).toBe("#SPILL!");
       expect(getCellError(model, "A1")).toBe(
-        "Array result was not expanded because it would overwrite data in A2."
+        "Array result was not expanded because it would overwrite data."
+      );
+      expect(getEvaluatedCell(model, "A1").errorOriginPosition).toStrictEqual(
+        toCellPosition(sheetId, "A2")
       );
     });
 
@@ -289,7 +302,10 @@ describe("evaluate formulas that return an array", () => {
       setCellContent(model, "C1", "kikou");
       expect(getEvaluatedCell(model, "A1").value).toBe("#SPILL!");
       expect(getCellError(model, "A1")).toBe(
-        "Array result was not expanded because it would overwrite data in B1."
+        "Array result was not expanded because it would overwrite data."
+      );
+      expect(getEvaluatedCell(model, "A1").errorOriginPosition).toStrictEqual(
+        toCellPosition(sheetId, "B1")
       );
     });
 
@@ -814,7 +830,10 @@ describe("evaluate formulas that return an array", () => {
       expect(getEvaluatedCell(model, "B1").value).toBe(42);
       expect(getEvaluatedCell(model, "A2").value).toBe("#SPILL!");
       expect(getCellError(model, "A2")).toBe(
-        "Array result was not expanded because it would overwrite data in B2."
+        "Array result was not expanded because it would overwrite data."
+      );
+      expect(getEvaluatedCell(model, "A2").errorOriginPosition).toStrictEqual(
+        toCellPosition(sheetId, "B2")
       );
     });
 
@@ -824,7 +843,10 @@ describe("evaluate formulas that return an array", () => {
       expect(getEvaluatedCell(model, "B1").value).toBe("#SPILL!");
       expect(getEvaluatedCell(model, "A2").value).toBe(42);
       expect(getCellError(model, "B1")).toBe(
-        "Array result was not expanded because it would overwrite data in B2."
+        "Array result was not expanded because it would overwrite data."
+      );
+      expect(getEvaluatedCell(model, "B1").errorOriginPosition).toStrictEqual(
+        toCellPosition(sheetId, "B2")
       );
     });
 
@@ -834,7 +856,10 @@ describe("evaluate formulas that return an array", () => {
       expect(getEvaluatedCell(model, "B1").value).toBe("#SPILL!");
       expect(getEvaluatedCell(model, "A2").value).toBe(42);
       expect(getCellError(model, "B1")).toBe(
-        "Array result was not expanded because it would overwrite data in B2."
+        "Array result was not expanded because it would overwrite data."
+      );
+      expect(getEvaluatedCell(model, "B1").errorOriginPosition).toStrictEqual(
+        toCellPosition(sheetId, "B2")
       );
     });
 

tests/popover/error_tooltip_component.test.ts

@@ -75,6 +75,14 @@ describe("Error tooltip component", () => {
     expect(".fst-italic").toHaveText(" Caused by A1");
   });
 
+  test("can display error origin position on a spill error", async () => {
+    const model = new Model();
+    setCellContent(model, "A1", "=RANDARRAY(2,2)");
+    setCellContent(model, "A2", "2");
+    await mountErrorTooltip(model, "A1");
+    expect(".fst-italic").toHaveText(" Caused by A2");
+  });
+
   test("Do not display error origin position in dashboard", async () => {
     const model = new Model();
     setCellContent(model, "A1", "=1/0");