fix: Prioritize deleting than other operations

- Deleting a board root having children and undoing made invalid children status
miyanokomiya · Nov 9, 2024 · 8f276b8 · 8f276b8
1 parent e6e9ba9
commit 8f276b8
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 34 deletions.
diff --git a/src/composables/alignHandler.ts b/src/composables/alignHandler.ts
@@ -918,6 +918,8 @@ export function getModifiedAlignRootIds(
   if (patchInfo.delete) {
     patchInfo.delete.forEach((id) => {
       const shape = shapeMap[id];
+      if (!shape) return;
+
       if (isAlignBoxShape(shape)) {
         deletedRootIdSet.add(id);
       }

diff --git a/src/composables/boardHandler.ts b/src/composables/boardHandler.ts
@@ -402,6 +402,8 @@ export function getModifiedBoardRootIds(
   if (patchInfo.delete) {
     patchInfo.delete.forEach((id) => {
       const shape = shapeMap[id];
+      if (!shape) return;
+
       if (isBoardRootShape(shape)) {
         deletedRootIdSet.add(shape.id);
       } else if (isParentRoot(shape)) {

diff --git a/src/composables/shapeHandlers/treeHandler.ts b/src/composables/shapeHandlers/treeHandler.ts
@@ -807,6 +807,8 @@ export function getModifiedTreeRootIds(srcComposite: ShapeComposite, patchInfo:
   if (patchInfo.update) {
     Object.keys(patchInfo.update).forEach((id) => {
       const shape = srcComposite.shapeMap[id];
+      if (!shape) return;
+
       if (isTreeRootShape(shape)) {
         targetTreeRootIdSet.add(shape.id);
       } else if (isTreeNodeShape(shape) && isValidTreeNode(srcComposite, shape)) {
@@ -818,6 +820,8 @@ export function getModifiedTreeRootIds(srcComposite: ShapeComposite, patchInfo:
   if (patchInfo.delete) {
     patchInfo.delete.forEach((id) => {
       const shape = srcComposite.shapeMap[id];
+      if (!shape) return;
+
       if (isTreeRootShape(shape)) {
         deletedRootIdSet.add(shape.id);
       } else if (isTreeNodeShape(shape) && isValidTreeNode(srcComposite, shape)) {

diff --git a/src/composables/shapeLayoutHandler.spec.ts b/src/composables/shapeLayoutHandler.spec.ts
@@ -48,6 +48,22 @@ describe("getEntityPatchByDelete", () => {
       [line.id]: { pConnection: undefined },
     });
   });
+
+  test("should delete update when it's deleted", () => {
+    const rectangle = createShape(getCommonStruct, "rectangle", {
+      id: "rectangle",
+    });
+    const line = createShape(getCommonStruct, "line", {
+      id: "line",
+    });
+    const shapeComposite = newShapeComposite({
+      shapes: [rectangle, line],
+      getStruct: getCommonStruct,
+    });
+    const result = getEntityPatchByDelete(shapeComposite, [rectangle.id], { [rectangle.id]: { findex: "aA" } });
+    expect(result.delete).toEqual([rectangle.id]);
+    expect(result.update).toStrictEqual({});
+  });
 });
 
 describe("getPatchByLayouts", () => {
@@ -78,32 +94,33 @@ describe("getPatchByLayouts", () => {
 });
 
 describe("getPatchInfoByLayouts", () => {
+  const root = createShape(getCommonStruct, "board_root", {
+    id: "root",
+    findex: generateKeyBetween(null, null),
+  });
+  const column0 = createShape(getCommonStruct, "board_column", {
+    id: "column0",
+    findex: generateKeyBetween(root.findex, null),
+    parentId: root.id,
+  });
+  const column1 = createShape(getCommonStruct, "board_column", {
+    id: "column1",
+    findex: generateKeyBetween(root.findex, null),
+    parentId: root.id,
+  });
+  const lane0 = createShape(getCommonStruct, "board_lane", {
+    id: "lane0",
+    findex: generateKeyBetween(column1.findex, null),
+    parentId: root.id,
+  });
+  const card0 = createShape<BoardCardShape>(getCommonStruct, "board_card", {
+    id: "card0",
+    findex: generateKeyBetween(lane0.findex, null),
+    parentId: root.id,
+    columnId: column0.id,
+  });
+
   test("should delete when a shape can't exist under the updated condition", () => {
-    const root = createShape(getCommonStruct, "board_root", {
-      id: "root",
-      findex: generateKeyBetween(null, null),
-    });
-    const column0 = createShape(getCommonStruct, "board_column", {
-      id: "column0",
-      findex: generateKeyBetween(root.findex, null),
-      parentId: root.id,
-    });
-    const column1 = createShape(getCommonStruct, "board_column", {
-      id: "column1",
-      findex: generateKeyBetween(root.findex, null),
-      parentId: root.id,
-    });
-    const lane0 = createShape(getCommonStruct, "board_lane", {
-      id: "lane0",
-      findex: generateKeyBetween(column1.findex, null),
-      parentId: root.id,
-    });
-    const card0 = createShape<BoardCardShape>(getCommonStruct, "board_card", {
-      id: "card0",
-      findex: generateKeyBetween(lane0.findex, null),
-      parentId: root.id,
-      columnId: column0.id,
-    });
     const shapeComposite = newShapeComposite({
       shapes: [root, column0, column1, card0, lane0],
       getStruct: getCommonStruct,
@@ -118,6 +135,24 @@ describe("getPatchInfoByLayouts", () => {
     const result2 = getPatchInfoByLayouts(shapeComposite, { delete: [card0.id] });
     expect(result2.delete).toEqual([card0.id]);
   });
+
+  test("should delete update when it's deleted", () => {
+    const shapeComposite = newShapeComposite({
+      shapes: [root, column0, card0],
+      getStruct: getCommonStruct,
+    });
+
+    const result0 = getPatchInfoByLayouts(shapeComposite, {
+      add: [lane0],
+      delete: [card0.id, lane0.id],
+      update: {
+        [card0.id]: { findex: "Zz" },
+      },
+    });
+    expect(result0.add).toEqual([]);
+    expect(result0.delete).toEqual([card0.id, lane0.id]);
+    expect(result0.update).not.toHaveProperty(card0.id);
+  });
 });
 
 describe("getPatchAfterLayouts", () => {

diff --git a/src/composables/shapeLayoutHandler.ts b/src/composables/shapeLayoutHandler.ts
@@ -1,6 +1,6 @@
 import { EntityPatchInfo, Shape } from "../models";
 import { refreshShapeRelations } from "../shapes";
-import { isObjectEmpty, mapEach, mergeMap, patchPipe, toList } from "../utils/commons";
+import { isObjectEmpty, mapEach, mapFilter, mergeMap, patchPipe, toList } from "../utils/commons";
 import { mergeEntityPatchInfo, normalizeEntityPatchInfo } from "../utils/entities";
 import { topSortHierarchy } from "../utils/graph";
 import { getAllBranchIds, getTree } from "../utils/tree";
@@ -38,7 +38,8 @@ export function getEntityPatchByDelete(
     availableIdSet,
   );
   const refreshedPatch = patch ? mergeMap(patch, patchByRefreshRelation) : patchByRefreshRelation;
-  return { update: refreshedPatch, delete: deleteAllIds };
+  const deleteAllIdSet = new Set(deleteAllIds);
+  return { update: mapFilter(refreshedPatch, (_, id) => !deleteAllIdSet.has(id)), delete: deleteAllIds };
 }
 
 /**
@@ -92,13 +93,19 @@ export function getPatchInfoByLayouts(
     shouldDeleteIds.length > 0
       ? getNextShapeComposite(updatedCompositeBeforeDelete, { delete: shouldDeleteIds })
       : updatedCompositeBeforeDelete;
-  const adjustedPatchInfo =
-    shouldDeleteIds.length > 0
-      ? {
-          ...patchInfo,
-          delete: [...(patchInfo.delete ?? []), ...shouldDeleteIds],
-        }
-      : patchInfo;
+  const deletedAllIds = [...(patchInfo.delete ?? []), ...shouldDeleteIds];
+  const deletedAllIdSet = new Set(deletedAllIds);
+  const adjustedPatchInfo = {
+    add:
+      deletedAllIdSet.size > 0 && patchInfo.add
+        ? patchInfo.add.filter((s) => !deletedAllIdSet.has(s.id))
+        : patchInfo.add,
+    update:
+      deletedAllIdSet.size > 0 && patchInfo.update
+        ? mapFilter(patchInfo.update, (_, id) => !deletedAllIdSet.has(id))
+        : patchInfo.update,
+    delete: deletedAllIds,
+  };
 
   const patchResult = patchPipe(
     [