[FIX] pivot: Prevent faulty pivot domain to crash

The getter `getPivotCellFromPosition` parses pivot cells domain args but this step assumes that the domain is valid (e.g. refers to a valid groupby,measure combination). Unfortunately, if the domain is invalid, the parsing will crash. Similarly to the decision made for the getter `evaluateFormula`[^1] we wrap the parsing in a try/catch statement and in case of faulty evaluation, return the same result as for invalid/non-existing pivots, that is an empty Pivotcell. [^1]: See #3371 closes #4805 Task: 4088765 X-original-commit: 693db2b Signed-off-by: Lucas Lefèvre (lul) <[email protected]> Signed-off-by: Rémi Rahir (rar) <[email protected]>
odoo · Aug 9, 2024 · 2367612 · 2367612
1 parent 64f71eb
commit 2367612
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 22 deletions.
diff --git a/src/plugins/ui_core_views/pivot_ui.ts b/src/plugins/ui_core_views/pivot_ui.ts
@@ -207,33 +207,37 @@ export class PivotUIPlugin extends UIPlugin {
       const pivotRow = position.row - mainPosition.row;
       return pivotCells[pivotCol][pivotRow];
     }
-    if (functionName === "PIVOT.HEADER" && args.at(-2) === "measure") {
-      const domain = pivot.parseArgsToPivotDomain(
-        args.slice(1, -2).map((value) => ({ value } as FunctionResultObject))
-      );
-      return {
-        type: "MEASURE_HEADER",
-        domain,
-        measure: args.at(-1)?.toString() || "",
-      };
-    } else if (functionName === "PIVOT.HEADER") {
+    try {
+      if (functionName === "PIVOT.HEADER" && args.at(-2) === "measure") {
+        const domain = pivot.parseArgsToPivotDomain(
+          args.slice(1, -2).map((value) => ({ value } as FunctionResultObject))
+        );
+        return {
+          type: "MEASURE_HEADER",
+          domain,
+          measure: args.at(-1)?.toString() || "",
+        };
+      } else if (functionName === "PIVOT.HEADER") {
+        const domain = pivot.parseArgsToPivotDomain(
+          args.slice(1).map((value) => ({ value } as FunctionResultObject))
+        );
+        return {
+          type: "HEADER",
+          domain,
+        };
+      }
+      const [measure, ...domainArgs] = args.slice(1);
       const domain = pivot.parseArgsToPivotDomain(
-        args.slice(1).map((value) => ({ value } as FunctionResultObject))
+        domainArgs.map((value) => ({ value } as FunctionResultObject))
       );
       return {
-        type: "HEADER",
+        type: "VALUE",
         domain,
+        measure: measure?.toString() || "",
       };
+    } catch (_) {
+      return EMPTY_PIVOT_CELL;
     }
-    const [measure, ...domainArgs] = args.slice(1);
-    const domain = pivot.parseArgsToPivotDomain(
-      domainArgs.map((value) => ({ value } as FunctionResultObject))
-    );
-    return {
-      type: "VALUE",
-      domain,
-      measure: measure?.toString() || "",
-    };
   }
 
   getPivot(pivotId: UID) {

diff --git a/tests/pivots/pivot_plugin.test.ts b/tests/pivots/pivot_plugin.test.ts
@@ -1,4 +1,5 @@
-import { setCellContent } from "../test_helpers/commands_helpers";
+import { EMPTY_PIVOT_CELL } from "../../src/helpers/pivot/table_spreadsheet_pivot";
+import { selectCell, setCellContent } from "../test_helpers/commands_helpers";
 import { createModelFromGrid, toCellPosition } from "../test_helpers/helpers";
 import { addPivot } from "../test_helpers/pivot_helpers";
 
@@ -28,4 +29,23 @@ describe("Pivot plugin", () => {
     setCellContent(model, "G1", "=PIVOT.HEADER(1)");
     expect(isSpillPivotFormula("G1")).toBe(false);
   });
+
+  test("getPivotCellFromPosition doesn't throw with invalid pivot domain", () => {
+    // prettier-ignore
+    const grid = {
+      A1: "Customer", B1: "Price", C1: '=PIVOT.VALUE(1,"Price","5","Bob")',
+      A2: "Alice",    B2: "10",
+      A3: "Bob",      B3: "30",
+    };
+    const model = createModelFromGrid(grid);
+    addPivot(model, "A1:B3", {
+      columns: [],
+      rows: [{ fieldName: "Customer" }],
+      measures: [{ id: "Price:sum", fieldName: "Price", aggregator: "sum" }],
+    });
+    selectCell(model, "C1");
+    expect(model.getters.getPivotCellFromPosition(model.getters.getActivePosition())).toMatchObject(
+      EMPTY_PIVOT_CELL
+    );
+  });
 });