[FIX] sheet: prevent deleting all visible columns/rows

We currently prevent deleting all columns, but we can still delete all
the visible columns, which leads to a broken sheet with no column.

This commit prevent it in the allowDisatch of sheet.ts, and also
disable the button in the header context menu.

Odoo task 3204730

Part-of: #2384

src/helpers/misc.ts

@@ -462,6 +462,11 @@ export function deepEquals<T extends Object>(o1: T, o2: T): boolean {
   return true;
 }
 
+/** Check if the given array contains all the values of the other array. */
+export function includesAll<T>(arr: T[], values: T[]): boolean {
+  return values.every((value) => arr.includes(value));
+}
+
 /**
  * Return an object with all the keys in the object that have a falsy value removed.
  */

src/plugins/core/header_visibility.ts

@@ -1,10 +1,11 @@
-import { deepCopy, getAddHeaderStartIndex } from "../../helpers";
+import { deepCopy, getAddHeaderStartIndex, includesAll, range } from "../../helpers";
 import { Command, CommandResult, ExcelWorkbookData, WorkbookData } from "../../types";
 import { ConsecutiveIndexes, Dimension, HeaderIndex, UID } from "../../types/misc";
 import { CorePlugin } from "../core_plugin";
 
 export class HeaderVisibilityPlugin extends CorePlugin {
   static getters = [
+    "canRemoveHeaders",
     "getHiddenColsGroups",
     "getHiddenRowsGroups",
     "isRowHiddenByUser",
@@ -16,7 +17,7 @@ export class HeaderVisibilityPlugin extends CorePlugin {
   allowDispatch(cmd: Command) {
     switch (cmd.type) {
       case "HIDE_COLUMNS_ROWS": {
-        if (!this.hiddenHeaders[cmd.sheetId]) {
+        if (!this.getters.tryGetSheet(cmd.sheetId)) {
           return CommandResult.InvalidSheetId;
         }
         const hiddenGroup =
@@ -31,6 +32,14 @@ export class HeaderVisibilityPlugin extends CorePlugin {
           ? CommandResult.Success
           : CommandResult.TooManyHiddenElements;
       }
+      case "REMOVE_COLUMNS_ROWS":
+        if (!this.getters.tryGetSheet(cmd.sheetId)) {
+          return CommandResult.InvalidSheetId;
+        }
+        if (!this.canRemoveHeaders(cmd.sheetId, cmd.dimension, cmd.elements)) {
+          return CommandResult.NotEnoughElements;
+        }
+        return CommandResult.Success;
     }
     return CommandResult.Success;
   }
@@ -83,6 +92,11 @@ export class HeaderVisibilityPlugin extends CorePlugin {
     return;
   }
 
+  canRemoveHeaders(sheetId: UID, dimension: Dimension, elements: HeaderIndex[]): boolean {
+    const visibleHeaders = this.getAllVisibleHeaders(sheetId, dimension);
+    return !includesAll(elements, visibleHeaders);
+  }
+
   isRowHiddenByUser(sheetId: UID, index: HeaderIndex): boolean {
     return this.hiddenHeaders[sheetId].ROW[index];
   }
@@ -131,6 +145,12 @@ export class HeaderVisibilityPlugin extends CorePlugin {
     return consecutiveIndexes;
   }
 
+  private getAllVisibleHeaders(sheetId: UID, dimension: Dimension): HeaderIndex[] {
+    return range(0, this.hiddenHeaders[sheetId][dimension].length).filter(
+      (i) => !this.hiddenHeaders[sheetId][dimension][i]
+    );
+  }
+
   import(data: WorkbookData) {
     for (let sheet of data.sheets) {
       this.hiddenHeaders[sheet.id] = { COL: [], ROW: [] };

src/plugins/core/sheet.ts

@@ -118,15 +118,6 @@ export class SheetPlugin extends CorePlugin<SheetState> implements SheetState {
         return this.orderedSheetIds.length > 1
           ? CommandResult.Success
           : CommandResult.NotEnoughSheets;
-      case "REMOVE_COLUMNS_ROWS": {
-        const length =
-          cmd.dimension === "COL"
-            ? this.getNumberCols(cmd.sheetId)
-            : this.getNumberRows(cmd.sheetId);
-        return length > cmd.elements.length
-          ? CommandResult.Success
-          : CommandResult.NotEnoughElements;
-      }
       case "FREEZE_ROWS": {
         return this.checkValidations(
           cmd,

src/registries/menus/col_menu_registry.ts

@@ -72,6 +72,11 @@ colMenuRegistry
     name: ACTIONS.REMOVE_COLUMNS_NAME,
     sequence: 90,
     action: ACTIONS.REMOVE_COLUMNS_ACTION,
+    isVisible: (env: SpreadsheetChildEnv) => {
+      const sheetId = env.model.getters.getActiveSheetId();
+      const selectedCols = env.model.getters.getElementsFromSelection("COL");
+      return env.model.getters.canRemoveHeaders(sheetId, "COL", selectedCols);
+    },
   })
   .add("clear_column", {
     name: ACTIONS.DELETE_CONTENT_COLUMNS_NAME,
@@ -84,11 +89,8 @@ colMenuRegistry
     action: ACTIONS.HIDE_COLUMNS_ACTION,
     isVisible: (env: SpreadsheetChildEnv) => {
       const sheetId = env.model.getters.getActiveSheetId();
-      const hiddenCols = env.model.getters.getHiddenColsGroups(sheetId).flat();
-      return (
-        env.model.getters.getNumberCols(sheetId) >
-        hiddenCols.length + env.model.getters.getElementsFromSelection("COL").length
-      );
+      const selectedCols = env.model.getters.getElementsFromSelection("COL");
+      return env.model.getters.canRemoveHeaders(sheetId, "COL", selectedCols);
     },
     separator: true,
   })

src/registries/menus/row_menu_registry.ts

@@ -55,6 +55,11 @@ rowMenuRegistry
     name: ACTIONS.REMOVE_ROWS_NAME,
     sequence: 70,
     action: ACTIONS.REMOVE_ROWS_ACTION,
+    isVisible: (env: SpreadsheetChildEnv) => {
+      const sheetId = env.model.getters.getActiveSheetId();
+      const selectedRows = env.model.getters.getElementsFromSelection("ROW");
+      return env.model.getters.canRemoveHeaders(sheetId, "ROW", selectedRows);
+    },
   })
   .add("clear_row", {
     name: ACTIONS.DELETE_CONTENT_ROWS_NAME,
@@ -67,11 +72,8 @@ rowMenuRegistry
     action: ACTIONS.HIDE_ROWS_ACTION,
     isVisible: (env: SpreadsheetChildEnv) => {
       const sheetId = env.model.getters.getActiveSheetId();
-      const hiddenRows = env.model.getters.getHiddenRowsGroups(sheetId).flat();
-      return (
-        env.model.getters.getNumberRows(sheetId) >
-        hiddenRows.length + env.model.getters.getElementsFromSelection("ROW").length
-      );
+      const selectedRows = env.model.getters.getElementsFromSelection("ROW");
+      return env.model.getters.canRemoveHeaders(sheetId, "ROW", selectedRows);
     },
     separator: true,
   })

tests/components/grid_manipulation.test.ts

@@ -1,7 +1,14 @@
 import { Spreadsheet } from "../../src";
 import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH } from "../../src/constants";
+import { zoneToXc } from "../../src/helpers";
 import { Model } from "../../src/model";
-import { selectColumn, selectRow } from "../test_helpers/commands_helpers";
+import {
+  hideColumns,
+  hideRows,
+  selectColumn,
+  selectRow,
+  setSelection,
+} from "../test_helpers/commands_helpers";
 import { simulateClick, triggerMouseEvent } from "../test_helpers/dom_helper";
 import { getSelectionAnchorCellXc } from "../test_helpers/getters_helpers";
 import { mountSpreadsheet, nextTick, spyDispatch } from "../test_helpers/helpers";
@@ -95,6 +102,24 @@ describe("Context Menu add/remove row/col", () => {
     });
   });
 
+  test("cannot delete nor hide all cols with contextmenu", async () => {
+    setSelection(model, [zoneToXc(model.getters.getSheetZone(model.getters.getActiveSheetId()))]);
+    simulateContextMenu(".o-col-resizer", COLUMN_D);
+    await nextTick();
+    expect(fixture.querySelector(".o-menu div[data-name='delete_column']")).toBeNull();
+    expect(fixture.querySelector(".o-menu div[data-name='hide_columns']")).toBeNull();
+  });
+
+  test("cannot delete nor hide all non-hidden cols with contextmenu", async () => {
+    const sheetZone = model.getters.getSheetZone(model.getters.getActiveSheetId());
+    setSelection(model, [zoneToXc({ ...sheetZone, left: sheetZone.left + 1 })]);
+    hideColumns(model, ["A"]);
+    simulateContextMenu(".o-col-resizer", COLUMN_D);
+    await nextTick();
+    expect(fixture.querySelector(".o-menu div[data-name='delete_column']")).toBeNull();
+    expect(fixture.querySelector(".o-menu div[data-name='hide_columns']")).toBeNull();
+  });
+
   test("can delete rows with contextmenu", async () => {
     simulateContextMenu(".o-row-resizer", ROW_5);
     await nextTick();
@@ -107,6 +132,24 @@ describe("Context Menu add/remove row/col", () => {
     });
   });
 
+  test("cannot delete nor hide all rows with contextmenu", async () => {
+    setSelection(model, [zoneToXc(model.getters.getSheetZone(model.getters.getActiveSheetId()))]);
+    simulateContextMenu(".o-row-resizer", ROW_5);
+    await nextTick();
+    expect(fixture.querySelector(".o-menu div[data-name='delete_row']")).toBeNull();
+    expect(fixture.querySelector(".o-menu div[data-name='hide_rows']")).toBeNull();
+  });
+
+  test("cannot delete nor hide all non-hidden rows with contextmenu", async () => {
+    const sheetZone = model.getters.getSheetZone(model.getters.getActiveSheetId());
+    setSelection(model, [zoneToXc({ ...sheetZone, top: sheetZone.top + 1 })]);
+    hideRows(model, [0]);
+    simulateContextMenu(".o-row-resizer", ROW_5);
+    await nextTick();
+    expect(fixture.querySelector(".o-menu div[data-name='delete_row']")).toBeNull();
+    expect(fixture.querySelector(".o-menu div[data-name='hide_rows']")).toBeNull();
+  });
+
   test("can add before cols with contextmenu", async () => {
     simulateContextMenu(".o-col-resizer", COLUMN_D);
     await nextTick();

tests/plugins/grid_manipulation.test.ts

@@ -135,6 +135,7 @@ describe("Clear columns", () => {
     expect(getBorder(model, "C2")).toEqual(border);
   });
   test("cannot delete column in invalid sheet", () => {
+    model = new Model();
     expect(deleteColumns(model, ["A"], "INVALID")).toBeCancelledBecause(
       CommandResult.InvalidSheetId
     );
@@ -180,6 +181,7 @@ describe("Clear rows", () => {
     expect(getCell(model, "C2")).toMatchObject({ style });
   });
   test("cannot delete row in invalid sheet", () => {
+    model = new Model();
     expect(deleteRows(model, [0], "INVALID")).toBeCancelledBecause(CommandResult.InvalidSheetId);
   });
 });

tests/plugins/sheets.test.ts

@@ -7,9 +7,11 @@ import {
   createChart,
   createSheet,
   createSheetWithName,
+  deleteColumns,
   deleteRows,
   freezeColumns,
   freezeRows,
+  hideColumns,
   hideRows,
   hideSheet,
   merge,
@@ -879,15 +881,25 @@ describe("sheets", () => {
   });
 
   test("Cannot remove more columns/rows than there are inside the sheet", () => {
-    const model = new Model({
-      sheets: [
-        {
-          colNumber: 1,
-          rowNumber: 3,
-        },
-      ],
-    });
-    expect(deleteRows(model, [1, 2, 3, 4])).toBeCancelledBecause(CommandResult.NotEnoughElements);
+    const model = new Model({ sheets: [{ colNumber: 3, rowNumber: 3 }] });
+    expect(deleteRows(model, [0, 1, 2])).toBeCancelledBecause(CommandResult.NotEnoughElements);
+    expect(deleteColumns(model, ["A", "B", "C"])).toBeCancelledBecause(
+      CommandResult.NotEnoughElements
+    );
+  });
+
+  test("Cannot remove all the non-hidden columns/rows", () => {
+    const model = new Model({ sheets: [{ colNumber: 4, rowNumber: 4 }] });
+    hideRows(model, [1, 3]);
+    hideColumns(model, ["B", "D"]);
+
+    expect(deleteRows(model, [0, 2])).toBeCancelledBecause(CommandResult.NotEnoughElements);
+    expect(deleteRows(model, [0, 1, 2])).toBeCancelledBecause(CommandResult.NotEnoughElements);
+
+    expect(deleteColumns(model, ["A", "C"])).toBeCancelledBecause(CommandResult.NotEnoughElements);
+    expect(deleteColumns(model, ["A", "B", "C"])).toBeCancelledBecause(
+      CommandResult.NotEnoughElements
+    );
   });
 
   test("Cannot have all rows/columns hidden at once", () => {