diff --git a/src/plugins/ui_feature/insert_pivot.ts b/src/plugins/ui_feature/insert_pivot.ts index a964c60519..f86a1c82e3 100644 --- a/src/plugins/ui_feature/insert_pivot.ts +++ b/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; diff --git a/tests/pivots/pivot_plugin.test.ts b/tests/pivots/pivot_plugin.test.ts index b3b94cb060..7d07ad2c09 100644 --- a/tests/pivots/pivot_plugin.test.ts +++ b/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)"); + }); });