[FIX] filters: filter can hide all the visible rows of a sheet

It's currently possible to use data filters to hide all the rows in a sheet,
leaving a buggy sheet with no visible rows.

Fixing this is a bit tricky, since the filtered rows are an UI concept, and
thus we cannot rely on the allowDispatch of "HIDE_COLUMNS_ROWS" to prevent
the user from hiding all the rows. Using allowDispatch isn't possible
because the filtered values (and the values of the cells) are different
from an user to another. This means that the allowDispatch could return
a success for an user, and a failure for the other, leading to a
dis-synchronized model state for the 2 user.

Fortunately we can fix this by disabling data filters whose header row is
hidden (by the user or by another filter). By construction of the filters,
it becomes impossible to hide all the rows of a sheet.

This may still becomes an issue in the future if we implement something
like filters being applied to a column header. In that case, we would need
to handle sheets with all their rows hidden.

Odoo task 3205608

closes #2198

X-original-commit: 1eee732
Signed-off-by: Rémi Rahir (rar) <[email protected]>

src/plugins/ui_core_views/filter_evaluation.ts

@@ -73,6 +73,9 @@ export class FilterEvaluationPlugin extends UIPlugin {
       case "CREATE_SHEET":
         this.filterValues[cmd.sheetId] = {};
         break;
+      case "HIDE_COLUMNS_ROWS":
+        this.updateHiddenRows();
+        break;
       case "UPDATE_FILTER":
         this.updateFilter(cmd);
         this.updateHiddenRows();
@@ -190,10 +193,15 @@ export class FilterEvaluationPlugin extends UIPlugin {
 
   private updateHiddenRows() {
     const sheetId = this.getters.getActiveSheetId();
-    const filters = this.getters.getFilters(sheetId);
+    const filters = this.getters
+      .getFilters(sheetId)
+      .sort((filter1, filter2) => filter1.zoneWithHeaders.top - filter2.zoneWithHeaders.top);
 
     const hiddenRows = new Set<number>();
     for (let filter of filters) {
+      // Disable filters whose header are hidden
+      if (this.getters.isRowHiddenByUser(sheetId, filter.zoneWithHeaders.top)) continue;
+      if (hiddenRows.has(filter.zoneWithHeaders.top)) continue;
       const filteredValues = this.filterValues[sheetId]?.[filter.id]?.map(toLowerCase);
       if (!filteredValues || !filter.filteredZone) continue;
       for (let row = filter.filteredZone.top; row <= filter.filteredZone.bottom; row++) {

tests/plugins/filters.test.ts

@@ -10,6 +10,7 @@ import {
   deleteColumns,
   deleteFilter,
   deleteRows,
+  hideRows,
   insertCells,
   merge,
   paste,
@@ -146,6 +147,29 @@ describe("Filters plugin", () => {
       expect(getFilterValues(model, sheetId)).toMatchObject([{ zone: "A1:A3", value: ["C"] }]);
       expect(getFilterValues(model, sheet2Id)).toMatchObject([{ zone: "A1:A3", value: ["D"] }]);
     });
+
+    test("Filter is disabled if its header row is hidden by the user", () => {
+      createFilter(model, "A1:A3");
+      setCellContent(model, "A2", "28");
+      updateFilter(model, "A1", ["28"]);
+      expect(model.getters.isRowHidden(sheetId, 1)).toBe(true);
+
+      hideRows(model, [0]);
+      expect(model.getters.isRowHidden(sheetId, 1)).toBe(false);
+    });
+
+    test("Filter is disabled if its header row is hidden by another filter", () => {
+      createFilter(model, "A2:A3");
+      setCellContent(model, "A3", "15");
+      updateFilter(model, "A2", ["15"]);
+      expect(model.getters.isRowHidden(sheetId, 2)).toBe(true);
+
+      createFilter(model, "B1:B2");
+      setCellContent(model, "B2", "28");
+      updateFilter(model, "B1", ["28"]);
+      expect(model.getters.isRowHidden(sheetId, 1)).toBe(true);
+      expect(model.getters.isRowHidden(sheetId, 2)).toBe(false);
+    });
   });
 
   describe("Filter Table Zone Expansion", () => {