[IMP] Pivot: added a check in the allowdispatch

Spec :
Currently, one could dispatch the command ADD_PIVOT by specifying a pivotId which corresponds to an already existing pivot. This is quite error prone as it could lead to a situation where we erase an existing PIVOT by mistake. 
The goal is to prevent this kind of situation by adding a check in the allowDispatch of the core plugin Pivot which should reject the command ADD_PIVOT if it targets an already exisitng pivot (similarly to the check done for the command INSERT_PIVOT).

closes #7573

Task: 5360591
Signed-off-by: Pierre Rousseau (pro) <[email protected]>

Author: matho-odoo

packages/o-spreadsheet-engine/src/plugins/core/pivot.ts

@@ -46,6 +46,9 @@ export class PivotCorePlugin extends CorePlugin<CoreState> implements CoreState
   allowDispatch(cmd: CoreCommand) {
     switch (cmd.type) {
       case "ADD_PIVOT": {
+        if (cmd.pivotId in this.pivots) {
+          return CommandResult.PivotIdTaken;
+        }
         return this.checkValidations(
           cmd.pivot,
           this.checkDuplicatedMeasureIds,

packages/o-spreadsheet-engine/src/types/commands.ts

@@ -1460,6 +1460,7 @@ export const enum CommandResult {
   SheetIsHidden = "SheetIsHidden",
   InvalidTableResize = "InvalidTableResize",
   PivotIdNotFound = "PivotIdNotFound",
+  PivotIdTaken = "PivotIdTaken",
   PivotInError = "PivotInError",
   EmptyName = "EmptyName",
   ValueCellIsInvalidFormula = "ValueCellIsInvalidFormula",

tests/pivots/pivot_plugin.test.ts

@@ -247,6 +247,14 @@ describe("Pivot plugin", () => {
     expect(updateResult).toBeCancelledBecause(CommandResult.PivotIdNotFound);
   });
 
+  test("cannot add a pivot with an existing id", () => {
+    const model = new Model();
+    const createResult1 = addPivot(model, "A1:A2", {}, "1");
+    expect(createResult1.isSuccessful).toBe(true);
+    const createResult2 = addPivot(model, "A1:A2", {}, "1");
+    expect(createResult2).toBeCancelledBecause(CommandResult.PivotIdTaken);
+  });
+
   test("cannot duplicate a pivot with a wrong id", () => {
     const model = new Model();
     const updateResult = model.dispatch("DUPLICATE_PIVOT", {

tests/pivots/spreadsheet_pivot/spreadsheet_pivot_side_panel.test.ts

@@ -880,9 +880,9 @@ describe("Spreadsheet pivot side panel", () => {
           measures: [{ id: "Amount", fieldName: "Amount", aggregator: "sum" }],
           sortedColumn,
         },
-        "1"
+        "2"
       );
-      env.openSidePanel("PivotSidePanel", { pivotId: "1" });
+      env.openSidePanel("PivotSidePanel", { pivotId: "2" });
       await nextTick();
     });
 
@@ -893,29 +893,29 @@ describe("Spreadsheet pivot side panel", () => {
     });
 
     test("Does not display sorting for pivot with no sorting or invalid sorting ", async () => {
-      updatePivot(model, "1", { sortedColumn: undefined });
-      env.openSidePanel("PivotSidePanel", { pivotId: "1" });
+      updatePivot(model, "2", { sortedColumn: undefined });
+      env.openSidePanel("PivotSidePanel", { pivotId: "2" });
       await nextTick();
       expect(".o-sidePanel .o-pivot-sort").toHaveCount(0);
 
-      updatePivot(model, "1", {
+      updatePivot(model, "2", {
         sortedColumn: { order: "asc", measure: "Yolo", domain: [] },
       });
       await nextTick();
       expect(".o-sidePanel .o-pivot-sort").toHaveCount(0);
     });
 
     test("Pivot sorting is removed when removing the sorted measure", async () => {
-      expect(model.getters.getPivotCoreDefinition("1").sortedColumn).toEqual(sortedColumn);
+      expect(model.getters.getPivotCoreDefinition("2").sortedColumn).toEqual(sortedColumn);
       click(fixture, ".pivot-measure .fa-trash");
-      expect(model.getters.getPivotCoreDefinition("1").sortedColumn).toBeUndefined();
+      expect(model.getters.getPivotCoreDefinition("2").sortedColumn).toBeUndefined();
     });
 
     test("Pivot sorting is removed when removing a column", async () => {
-      expect(model.getters.getPivotCoreDefinition("1").sortedColumn).toEqual(sortedColumn);
+      expect(model.getters.getPivotCoreDefinition("2").sortedColumn).toEqual(sortedColumn);
       const column = fixture.querySelectorAll(".pivot-dimension")[0];
       click(column, ".fa-trash");
-      expect(model.getters.getPivotCoreDefinition("1").sortedColumn).toBeUndefined();
+      expect(model.getters.getPivotCoreDefinition("2").sortedColumn).toBeUndefined();
     });
   });