[FIX] pivot: remove forbidden chars when duplicating

Steps to reproduce:

- insert a new pivot
- rename the pivot with one of the following character
  in its name: ', *, ?, /, \, [, ]
- duplicate the pivot

=> boom

The command to create the new sheet is rejected (the name contains invalid
characters). But we later tries to activate the sheet (which doesn doesn't
exist since it was never created)

closes #5048

Task: 4221325
X-original-commit: 941317f
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
Signed-off-by: Pierre Rousseau (pro) <[email protected]>

src/plugins/ui_feature/insert_pivot.ts

@@ -1,4 +1,4 @@
-import { PIVOT_TABLE_CONFIG } from "../../constants";
+import { FORBIDDEN_IN_EXCEL_REGEX, PIVOT_TABLE_CONFIG } from "../../constants";
 import { createPivotFormula } from "../../helpers/pivot/pivot_helpers";
 import { SpreadsheetPivotTable } from "../../helpers/pivot/table_spreadsheet_pivot";
 import { getZoneArea, positionToZone } from "../../helpers/zones";
@@ -85,7 +85,7 @@ export class InsertPivotPlugin extends UIPlugin {
     const position = this.getters.getSheetIds().indexOf(activeSheetId) + 1;
     const formulaId = this.getters.getPivotFormulaId(newPivotId);
     const newPivotName = this.getters.getPivotName(newPivotId);
-    this.dispatch("CREATE_SHEET", {
+    const result = this.dispatch("CREATE_SHEET", {
       sheetId: newSheetId,
       name: this.getPivotDuplicateSheetName(
         _t("%(newPivotName)s (Pivot #%(formulaId)s)", {
@@ -95,21 +95,24 @@ export class InsertPivotPlugin extends UIPlugin {
       ),
       position,
     });
-    this.dispatch("ACTIVATE_SHEET", { sheetIdFrom: activeSheetId, sheetIdTo: newSheetId });
-    this.dispatch("UPDATE_CELL", {
-      sheetId: newSheetId,
-      col: 0,
-      row: 0,
-      content: `=PIVOT(${formulaId})`,
-    });
+    if (result.isSuccessful) {
+      this.dispatch("ACTIVATE_SHEET", { sheetIdFrom: activeSheetId, sheetIdTo: newSheetId });
+      this.dispatch("UPDATE_CELL", {
+        sheetId: newSheetId,
+        col: 0,
+        row: 0,
+        content: `=PIVOT(${formulaId})`,
+      });
+    }
   }
 
   private getPivotDuplicateSheetName(pivotName: string) {
     let i = 1;
     const names = this.getters.getSheetIds().map((id) => this.getters.getSheetName(id));
-    let name = pivotName;
+    const sanitizedName = pivotName.replace(new RegExp(FORBIDDEN_IN_EXCEL_REGEX, "g"), " ");
+    let name = sanitizedName;
     while (names.includes(name)) {
-      name = `${pivotName} (${i})`;
+      name = `${sanitizedName} (${i})`;
       i++;
     }
     return name;

tests/pivots/pivot_plugin.test.ts

@@ -1,6 +1,7 @@
 import { CommandResult } from "../../src";
+import { FORBIDDEN_SHEET_CHARS } from "../../src/constants";
 import { EMPTY_PIVOT_CELL } from "../../src/helpers/pivot/table_spreadsheet_pivot";
-import { selectCell, setCellContent } from "../test_helpers/commands_helpers";
+import { renameSheet, selectCell, setCellContent } from "../test_helpers/commands_helpers";
 import { createModelFromGrid, toCellPosition } from "../test_helpers/helpers";
 import { addPivot, updatePivot } from "../test_helpers/pivot_helpers";
 
@@ -163,4 +164,40 @@ describe("Pivot plugin", () => {
     const thirdCalculatedName = model.getters.generateNewCalculatedMeasureName(definition.measures);
     expect(thirdCalculatedName).toBe("Calculated measure 1");
   });
+
+  test("forbidden characters are removed from new sheet name when duplicating a pivot", () => {
+    const grid = {
+      A1: "Customer",
+      A2: "Alice",
+    };
+    const model = createModelFromGrid(grid);
+    addPivot(model, "A1:A2", { name: `forbidden: ${FORBIDDEN_SHEET_CHARS}` }, "pivot1");
+    model.dispatch("DUPLICATE_PIVOT_IN_NEW_SHEET", {
+      newPivotId: "pivot2",
+      newSheetId: "Sheet2",
+      pivotId: "pivot1",
+    });
+    expect(model.getters.getSheetName("Sheet2")).toEqual(
+      "forbidden:  , , , , , ,  (copy) (Pivot #2)"
+    );
+    expect(model.getters.getPivotName("pivot2")).toEqual("forbidden: ',*,?,/,\\,[,] (copy)");
+  });
+
+  test("sheet names with forbidden characters cannot conflict", () => {
+    const grid = {
+      A1: "Customer",
+      A2: "Alice",
+    };
+    const model = createModelFromGrid(grid);
+    const sheetId = model.getters.getActiveSheetId();
+    const name = "forbidden: /";
+    renameSheet(model, sheetId, "forbidden:   (copy) (Pivot #2)");
+    addPivot(model, "A1:A2", { name }, "pivot1");
+    model.dispatch("DUPLICATE_PIVOT_IN_NEW_SHEET", {
+      newPivotId: "pivot2",
+      newSheetId: "Sheet2",
+      pivotId: "pivot1",
+    });
+    expect(model.getters.getSheetName("Sheet2")).toEqual("forbidden:   (copy) (Pivot #2) (1)");
+  });
 });