[FIX] collaborative: recompute selection after concurrent sheet modif…

…ications

See newly added test to see the steps to reproduce the issue.

Issue Summary:
- When Charlie applies his local revision, his active sheet correctly
updates to `Sheet2`, as it is the only visible sheet.
- Upon applying Bob's revision, Charlie's revision is initially rejected
due to `allowDispatch`, since only **one** sheet remains. As a result, his
active sheet switches back to `Sheet1` in the `dispatchToHandlers` step of the
`remote-revision-received` event.
- When Alice's revision is applied, Charlie's revision can now be
accepted, as there are multiple sheets again. However, Charlie's active
sheet remains `Sheet1`, and the selection plugin only detects Alice's
`CREATE_SHEET` command, without acknowledging that Charlie's deletion was
eventually applied.

This results in a crash in the selection plugin, as `Sheet1` does not
exist anymore.

To fix this, we need to ensure that the `activeSheet` exists in the
`finalize`.

closes #5692

Task: 4559104
X-original-commit: d1e311f
Signed-off-by: Rémi Rahir (rar) <[email protected]>
Signed-off-by: Pierre Rousseau (pro) <[email protected]>
Co-authored-by: Lucas Lefèvre <[email protected]>
Co-authored-by: Pierre Rousseau <[email protected]>

src/plugins/ui_stateful/selection.ts

@@ -232,25 +232,7 @@ export class GridSelectionPlugin extends UIPlugin {
             gridSelection: deepCopy(gridSelection),
           };
         }
-        if (!this.getters.tryGetSheet(this.getters.getActiveSheetId())) {
-          const currentSheetIds = this.getters.getVisibleSheetIds();
-          this.activeSheet = this.getters.getSheet(currentSheetIds[0]);
-          if (this.activeSheet.id in this.sheetsData) {
-            const { anchor } = this.clipSelection(
-              this.activeSheet.id,
-              this.sheetsData[this.activeSheet.id].gridSelection
-            );
-            this.selectCell(anchor.cell.col, anchor.cell.row);
-          } else {
-            this.selectCell(0, 0);
-          }
-          const { col, row } = this.gridSelection.anchor.cell;
-          this.moveClient({
-            sheetId: this.getters.getActiveSheetId(),
-            col,
-            row,
-          });
-        }
+        this.fallbackToVisibleSheet();
         const sheetId = this.getters.getActiveSheetId();
         this.gridSelection.zones = this.gridSelection.zones.map((z) =>
           this.getters.expandZone(sheetId, z)
@@ -266,6 +248,7 @@ export class GridSelectionPlugin extends UIPlugin {
   }
 
   finalize(): void {
+    this.fallbackToVisibleSheet();
     /** Any change to the selection has to be  reflected in the selection processor. */
     this.selection.resetDefaultAnchor(this, deepCopy(this.gridSelection.anchor));
   }
@@ -627,6 +610,28 @@ export class GridSelectionPlugin extends UIPlugin {
     return CommandResult.Success;
   }
 
+  private fallbackToVisibleSheet() {
+    if (!this.getters.tryGetSheet(this.getters.getActiveSheetId())) {
+      const currentSheetIds = this.getters.getVisibleSheetIds();
+      this.activeSheet = this.getters.getSheet(currentSheetIds[0]);
+      if (this.activeSheet.id in this.sheetsData) {
+        const { anchor } = this.clipSelection(
+          this.activeSheet.id,
+          this.sheetsData[this.activeSheet.id].gridSelection
+        );
+        this.selectCell(anchor.cell.col, anchor.cell.row);
+      } else {
+        this.selectCell(0, 0);
+      }
+      const { col, row } = this.gridSelection.anchor.cell;
+      this.moveClient({
+        sheetId: this.getters.getActiveSheetId(),
+        col,
+        row,
+      });
+    }
+  }
+
   //-------------------------------------------
   // Helpers for extensions
   // ------------------------------------------

tests/collaborative/collaborative_history.test.ts

@@ -726,6 +726,17 @@ describe("Collaborative local history", () => {
     expect(all).toHaveSynchronizedValue((user) => getCellContent(user, "F1"), "hello");
   });
 
+  test("Active sheet is correctly recomputed after concurrent sheet modifications", () => {
+    const firstSheetId = alice.getters.getActiveSheetId();
+    createSheet(bob, { sheetId: "sheet2", name: "Sheet2", position: 1 });
+    network.concurrent(() => {
+      deleteSheet(bob, "sheet2");
+      createSheet(alice, { sheetId: "sheet3", position: 1, name: "Sheet3" });
+      deleteSheet(charlie, firstSheetId);
+    });
+    expect([alice, bob, charlie]).toHaveSynchronizedExportedData();
+  });
+
   test("local history is cleared and cannot repeat last command after snapshot", () => {
     setCellContent(alice, "A1", "hello");
     setCellContent(alice, "A2", "hello");