From f535004d4d382de4bd41230b0aed89764a583113 Mon Sep 17 00:00:00 2001
From: "Adrien Minne (adrm)" <adrm@odoo.com>
Date: Mon, 18 Nov 2024 10:53:55 +0100
Subject: [PATCH] [FIX] figure: crash on deleting dragged figure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There was a traceback if a figure was deleted while being dragged.

closes odoo/o-spreadsheet#5223

Task: 4329240
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
---
 .../figure_container/figure_container.ts      | 26 +++++++++++++++----
 src/components/helpers/drag_and_drop.ts       | 15 ++++++++---
 tests/components/figure.test.ts               |  9 +++++++
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/src/components/figures/figure_container/figure_container.ts b/src/components/figures/figure_container/figure_container.ts
index f65c4739be..77c5fa434e 100644
--- a/src/components/figures/figure_container/figure_container.ts
+++ b/src/components/figures/figure_container/figure_container.ts
@@ -1,4 +1,4 @@
-import { Component, onMounted, useState } from "@odoo/owl";
+import { Component, onMounted, onWillUpdateProps, useState } from "@odoo/owl";
 import { MIN_FIG_SIZE } from "../../../constants";
 import { figureRegistry } from "../../../registries";
 import {
@@ -21,6 +21,7 @@ interface DndState {
   y: Pixel;
   width: Pixel;
   height: Pixel;
+  cancelDnd: (() => void) | undefined;
 }
 interface Props {
   sidePanelIsOpen: Boolean;
@@ -104,6 +105,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
     y: 0,
     width: 0,
     height: 0,
+    cancelDnd: undefined,
   });
 
   setup() {
@@ -117,14 +119,28 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
       // new rendering
       this.render();
     });
+    onWillUpdateProps(() => {
+      const sheetId = this.env.model.getters.getActiveSheetId();
+      if (this.dnd.figId && !this.env.model.getters.getFigure(sheetId, this.dnd.figId)) {
+        if (this.dnd.cancelDnd) {
+          this.dnd.cancelDnd();
+        }
+        this.dnd.figId = undefined;
+        this.dnd.cancelDnd = undefined;
+      }
+    });
   }
 
   private getVisibleFigures(): Figure[] {
     const visibleFigures = this.env.model.getters.getVisibleFigures();
     if (this.dnd.figId && !visibleFigures.some((figure) => figure.id === this.dnd.figId)) {
-      visibleFigures.push(
-        this.env.model.getters.getFigure(this.env.model.getters.getActiveSheetId(), this.dnd.figId)!
+      const draggedFigure = this.env.model.getters.getFigure(
+        this.env.model.getters.getActiveSheetId(),
+        this.dnd.figId
       );
+      if (draggedFigure) {
+        visibleFigures.push(draggedFigure);
+      }
     }
     return visibleFigures;
   }
@@ -256,7 +272,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
       this.dnd.figId = undefined;
       this.env.model.dispatch("UPDATE_FIGURE", { sheetId, id: figure.id, x, y });
     };
-    startDnd(onMouseMove, onMouseUp);
+    this.dnd.cancelDnd = startDnd(onMouseMove, onMouseUp);
   }
 
   startResize(figure: Figure, dirX: ResizeDirection, dirY: ResizeDirection, ev: MouseEvent) {
@@ -315,7 +331,7 @@ export class FiguresContainer extends Component<Props, SpreadsheetChildEnv> {
         ...update,
       });
     };
-    startDnd(onMouseMove, onMouseUp);
+    this.dnd.cancelDnd = startDnd(onMouseMove, onMouseUp);
   }
 
   private getDndFigure(): Figure {
diff --git a/src/components/helpers/drag_and_drop.ts b/src/components/helpers/drag_and_drop.ts
index 196ddb19d4..f9cb222c7c 100644
--- a/src/components/helpers/drag_and_drop.ts
+++ b/src/components/helpers/drag_and_drop.ts
@@ -4,20 +4,27 @@ import { HeaderIndex } from "../../types/misc";
 import { gridOverlayPosition } from "./dom_helpers";
 type EventFn = (ev: MouseEvent) => void;
 
+/**
+ * Start listening to pointer events and apply the given callbacks.
+ *
+ * @returns A function to remove the listeners.
+ */
 export function startDnd(
   onMouseMove: EventFn,
   onMouseUp: EventFn,
   onMouseDown: EventFn = () => {}
 ) {
-  const _onMouseUp = (ev: MouseEvent) => {
-    onMouseUp(ev);
-
+  const removeListeners = () => {
     window.removeEventListener("mousedown", onMouseDown);
     window.removeEventListener("mouseup", _onMouseUp);
     window.removeEventListener("dragstart", _onDragStart);
     window.removeEventListener("mousemove", onMouseMove);
     window.removeEventListener("wheel", onMouseMove);
   };
+  const _onMouseUp = (ev: MouseEvent) => {
+    onMouseUp(ev);
+    removeListeners();
+  };
   function _onDragStart(ev: DragEvent) {
     ev.preventDefault();
   }
@@ -26,6 +33,8 @@ export function startDnd(
   window.addEventListener("dragstart", _onDragStart);
   window.addEventListener("mousemove", onMouseMove);
   window.addEventListener("wheel", onMouseMove);
+
+  return removeListeners;
 }
 
 /**
diff --git a/tests/components/figure.test.ts b/tests/components/figure.test.ts
index b9060d5b08..19d22eea84 100644
--- a/tests/components/figure.test.ts
+++ b/tests/components/figure.test.ts
@@ -436,6 +436,15 @@ describe("figures", () => {
         });
       }
     );
+
+    test("Deleting a figure during drag and drop does not crash", async () => {
+      createFigure(model, { id: "someuuid", x: 200, y: 100 });
+      await nextTick();
+      await dragElement(".o-figure", 150, 100, false);
+      model.dispatch("DELETE_FIGURE", { id: "someuuid", sheetId });
+      await nextTick();
+      expect(model.getters.getFigure(sheetId, "someuuid")).toEqual(undefined);
+    });
   });
 
   test("Cannot select/move figure in readonly mode", async () => {