[FIX] sheet: duplicate sheet name problem when renaming

This commit fixes the problem that when renaming the sheet and the new sheet name is the same
as the old one, but the case is different, an error will occur. The
sheet name should be case insensitive.

When detecting duplicate names, the sheet itself will be excluded.

task 3370632

closes #2610

Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>

Author: Chenyun Yang

src/helpers/ui/sheet_interactive.ts

@@ -8,9 +8,6 @@ export function interactiveRenameSheet(
   name: string,
   errorCallback: () => void
 ) {
-  const originalSheetName = env.model.getters.getSheetName(sheetId);
-  if (name === null || name === originalSheetName) return;
-
   const result = env.model.dispatch("RENAME_SHEET", { sheetId, name });
   if (result.reasons.includes(CommandResult.MissingSheetName)) {
     env.raiseError(_lt("The sheet name cannot be empty."), errorCallback);

src/plugins/core/sheet.ts

@@ -639,10 +639,16 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
   }
 
   private checkSheetName(cmd: RenameSheetCommand | CreateSheetCommand): CommandResult {
+    const originalSheetName = this.getters.tryGetSheetName(cmd.sheetId);
+    if (originalSheetName !== undefined && cmd.name === originalSheetName) {
+      return CommandResult.UnchangedSheetName;
+    }
+
     const { orderedSheetIds, sheets } = this;
     const name = cmd.name && cmd.name.trim().toLowerCase();
-
-    if (orderedSheetIds.find((id) => sheets[id]?.name.toLowerCase() === name)) {
+    if (
+      orderedSheetIds.find((id) => sheets[id]?.name.toLowerCase() === name && id !== cmd.sheetId)
+    ) {
       return CommandResult.DuplicatedSheetName;
     }
     if (FORBIDDEN_IN_EXCEL_REGEX.test(name!)) {
@@ -703,8 +709,8 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
     const oldName = sheet.name;
     this.history.update("sheets", sheet.id, "name", name.trim());
     const sheetIdsMapName = Object.assign({}, this.sheetIdsMapName);
-    sheetIdsMapName[name] = sheet.id;
     delete sheetIdsMapName[oldName];
+    sheetIdsMapName[name] = sheet.id;
     this.history.update("sheetIdsMapName", sheetIdsMapName);
   }
 

src/types/commands.ts

@@ -1096,6 +1096,7 @@ export const enum CommandResult {
   NotEnoughElements,
   NotEnoughSheets,
   MissingSheetName,
+  UnchangedSheetName,
   DuplicatedSheetName,
   DuplicatedSheetId,
   ForbiddenCharactersInSheetName,

tests/components/bottom_bar.test.ts

@@ -321,6 +321,17 @@ describe("BottomBar component", () => {
       expect(window.getSelection()?.toString()).toEqual("ThisIsASheet");
       expect(document.activeElement).toEqual(sheetName);
     });
+
+    test("Sheet name is case insensitive", async () => {
+      const sheetName = fixture.querySelector<HTMLElement>(".o-sheet-name")!;
+      expect(sheetName.textContent).toEqual("Sheet1");
+      triggerMouseEvent(sheetName, "dblclick");
+      await nextTick();
+      sheetName.textContent = "SHEET1";
+      await keyDown({ key: "Enter" });
+      expect(raiseError).not.toHaveBeenCalled();
+      expect(sheetName.textContent).toEqual("SHEET1");
+    });
   });
 
   test("Can duplicate a sheet", async () => {

tests/plugins/sheets.test.ts

@@ -154,6 +154,21 @@ describe("sheets", () => {
     );
   });
 
+  test("Rename command won't be dispatched if the name is unchanged (case sensitive)", () => {
+    const model = new Model({ sheets: [{ id: "11", name: "Sheet1" }] });
+    expect(renameSheet(model, "11", "Sheet1")).toBeCancelledBecause(
+      CommandResult.UnchangedSheetName
+    );
+  });
+
+  test("Can change sheet name case", () => {
+    const sheetId = "11";
+    const model = new Model({ sheets: [{ id: sheetId, name: "Sheet1" }] });
+    expect(model.getters.getSheetName(sheetId)).toBe("Sheet1");
+    renameSheet(model, "11", "SHEET1");
+    expect(model.getters.getSheetName(sheetId)).toBe("SHEET1");
+  });
+
   test("Cannot create a sheet with a position > length of sheets", () => {
     const model = new Model();
     expect(model.dispatch("CREATE_SHEET", { sheetId: "42", position: 54 })).toBeCancelledBecause(