fix(category): remove not always working and ignoring dynamic sections (#1939)

apps/nextjs/src/components/board/sections/category/actions/remove-category.ts

@@ -0,0 +1,111 @@
+import type { Board, CategorySection, DynamicSection, EmptySection, Section } from "~/app/[locale]/boards/_types";
+
+export interface RemoveCategoryInput {
+  id: string;
+}
+
+export const removeCategoryCallback =
+  (input: RemoveCategoryInput) =>
+  (previous: Board): Board => {
+    const currentCategory = previous.sections.find(
+      (section): section is CategorySection => section.kind === "category" && section.id === input.id,
+    );
+    if (!currentCategory) {
+      return previous;
+    }
+
+    const emptySectionsAbove = previous.sections.filter(
+      (section): section is EmptySection => section.kind === "empty" && section.yOffset < currentCategory.yOffset,
+    );
+    const aboveSection = emptySectionsAbove.sort((sectionA, sectionB) => sectionB.yOffset - sectionA.yOffset).at(0);
+
+    const emptySectionsBelow = previous.sections.filter(
+      (section): section is EmptySection => section.kind === "empty" && section.yOffset > currentCategory.yOffset,
+    );
+    const removedSection = emptySectionsBelow.sort((sectionA, sectionB) => sectionA.yOffset - sectionB.yOffset).at(0);
+
+    if (!aboveSection || !removedSection) {
+      return previous;
+    }
+
+    // Calculate the yOffset for the items in the currentCategory and removedWrapper to add them with the same offset to the aboveWrapper
+    const aboveYOffset = Math.max(
+      calculateYHeightWithOffsetForItems(aboveSection),
+      calculateYHeightWithOffsetForDynamicSections(previous.sections, aboveSection.id),
+    );
+    const categoryYOffset = Math.max(
+      calculateYHeightWithOffsetForItems(currentCategory),
+      calculateYHeightWithOffsetForDynamicSections(previous.sections, currentCategory.id),
+    );
+
+    const previousCategoryItems = currentCategory.items.map((item) => ({
+      ...item,
+      yOffset: item.yOffset + aboveYOffset,
+    }));
+    const previousBelowWrapperItems = removedSection.items.map((item) => ({
+      ...item,
+      yOffset: item.yOffset + aboveYOffset + categoryYOffset,
+    }));
+
+    return {
+      ...previous,
+      sections: [
+        ...previous.sections.filter((section) => section.yOffset < aboveSection.yOffset && section.kind !== "dynamic"),
+        {
+          ...aboveSection,
+          items: [...aboveSection.items, ...previousCategoryItems, ...previousBelowWrapperItems],
+        },
+        ...previous.sections
+          .filter(
+            (section): section is CategorySection | EmptySection =>
+              section.yOffset > removedSection.yOffset && section.kind !== "dynamic",
+          )
+          .map((section) => ({
+            ...section,
+            position: section.yOffset - 2,
+          })),
+        ...previous.sections
+          .filter((section): section is DynamicSection => section.kind === "dynamic")
+          .map((dynamicSection) => {
+            // Move dynamic sections from removed section to above section with required yOffset
+            if (dynamicSection.parentSectionId === removedSection.id) {
+              return {
+                ...dynamicSection,
+                yOffset: dynamicSection.yOffset + aboveYOffset + categoryYOffset,
+                parentSectionId: aboveSection.id,
+              };
+            }
+
+            // Move dynamic sections from category to above section with required yOffset
+            if (dynamicSection.parentSectionId === currentCategory.id) {
+              return {
+                ...dynamicSection,
+                yOffset: dynamicSection.yOffset + aboveYOffset,
+                parentSectionId: aboveSection.id,
+              };
+            }
+
+            return dynamicSection;
+          }),
+      ],
+    };
+  };
+
+const calculateYHeightWithOffsetForDynamicSections = (sections: Section[], sectionId: string) => {
+  return sections.reduce((acc, section) => {
+    if (section.kind !== "dynamic" || section.parentSectionId !== sectionId) {
+      return acc;
+    }
+
+    const yHeightWithOffset = section.yOffset + section.height;
+    if (yHeightWithOffset > acc) return yHeightWithOffset;
+    return acc;
+  }, 0);
+};
+
+const calculateYHeightWithOffsetForItems = (section: Section) =>
+  section.items.reduce((acc, item) => {
+    const yHeightWithOffset = item.yOffset + item.height;
+    if (yHeightWithOffset > acc) return yHeightWithOffset;
+    return acc;
+  }, 0);

apps/nextjs/src/components/board/sections/category/actions/test/remove-category.spec.ts

@@ -0,0 +1,125 @@
+/* eslint-disable @typescript-eslint/no-non-null-assertion */
+import { describe, expect, test } from "vitest";
+
+import type { DynamicSection, Item, Section } from "~/app/[locale]/boards/_types";
+import { removeCategoryCallback } from "../remove-category";
+
+describe("Remove Category", () => {
+  test.each([
+    [3, [0, 1, 2, 3, 4, 5, 6], [0, 1, 2, 5, 6], [3, 4], 2],
+    [5, [0, 1, 2, 3, 4, 5, 6], [0, 1, 2, 3, 4], [5, 6], 4],
+    [1, [0, 1, 2, 3, 4, 5, 6], [0, 3, 4, 5, 6], [1, 2], 0],
+    [3, [0, 3, 6, 7, 8], [0, 7, 8], [3, 6], 0],
+  ])(
+    "should remove category",
+    (removeId, initialYOffsets, expectedYOffsets, expectedRemovals, expectedLocationOfItems) => {
+      const sections = createSections(initialYOffsets);
+
+      const input = removeId.toString();
+
+      const result = removeCategoryCallback({ id: input })({ sections } as never);
+
+      expect(result.sections.map((section) => parseInt(section.id, 10))).toEqual(expectedYOffsets);
+      expectedRemovals.forEach((expectedRemoval) => {
+        expect(result.sections.find((section) => section.id === expectedRemoval.toString())).toBeUndefined();
+      });
+      const aboveSection = result.sections.find((section) => section.id === expectedLocationOfItems.toString());
+      expect(aboveSection?.items.map((item) => parseInt(item.id, 10))).toEqual(
+        expect.arrayContaining(expectedRemovals),
+      );
+    },
+  );
+
+  test("should correctly move items to above empty section", () => {
+    const initialYOffsets = [0, 1, 2, 3, 4, 5, 6];
+    const sections: Section[] = createSections(initialYOffsets);
+    const aboveSection = sections.find((section) => section.yOffset === 2)!;
+    aboveSection.items = [
+      createItem({ id: "above-1" }),
+      createItem({ id: "above-2", yOffset: 3, xOffset: 2, height: 2 }),
+    ];
+    const removedCategory = sections.find((section) => section.yOffset === 3)!;
+    removedCategory.items = [
+      createItem({ id: "category-1" }),
+      createItem({ id: "category-2", yOffset: 2, xOffset: 4, width: 4 }),
+    ];
+    const removedEmptySection = sections.find((section) => section.yOffset === 4)!;
+    removedEmptySection.items = [
+      createItem({ id: "below-1", xOffset: 5 }),
+      createItem({ id: "below-2", yOffset: 1, xOffset: 1, height: 2 }),
+    ];
+    sections.push(
+      createDynamicSection({
+        id: "7",
+        parentSectionId: "3",
+        yOffset: 7,
+        height: 3,
+        items: [createItem({ id: "dynamic-1" })],
+      }),
+    );
+
+    const input = "3";
+
+    const result = removeCategoryCallback({ id: input })({ sections } as never);
+
+    expect(result.sections.map((section) => parseInt(section.id, 10))).toEqual([0, 1, 2, 5, 6, 7]);
+    const aboveSectionResult = result.sections.find((section) => section.id === "2")!;
+    expect(aboveSectionResult.items).toEqual(
+      expect.arrayContaining([
+        createItem({ id: "above-1" }),
+        createItem({ id: "above-2", yOffset: 3, xOffset: 2, height: 2 }),
+        createItem({ id: "category-1", yOffset: 5 }),
+        createItem({ id: "category-2", yOffset: 7, xOffset: 4, width: 4 }),
+        createItem({ id: "below-1", yOffset: 15, xOffset: 5 }),
+        createItem({ id: "below-2", yOffset: 16, xOffset: 1, height: 2 }),
+      ]),
+    );
+    const dynamicSection = result.sections.find((section): section is DynamicSection => section.id === "7")!;
+    expect(dynamicSection.yOffset).toBe(12);
+    expect(dynamicSection.parentSectionId).toBe("2");
+  });
+});
+
+const createItem = (item: Partial<{ id: string; width: number; height: number; yOffset: number; xOffset: number }>) => {
+  return {
+    id: item.id ?? "0",
+    kind: "app",
+    options: {},
+    advancedOptions: {
+      customCssClasses: [],
+    },
+    height: item.height ?? 1,
+    width: item.width ?? 1,
+    yOffset: item.yOffset ?? 0,
+    xOffset: item.xOffset ?? 0,
+    integrationIds: [],
+  } satisfies Item;
+};
+
+const createDynamicSection = (
+  section: Partial<
+    Pick<DynamicSection, "id" | "height" | "width" | "yOffset" | "xOffset" | "parentSectionId" | "items">
+  >,
+) => {
+  return {
+    id: section.id ?? "0",
+    kind: "dynamic",
+    height: section.height ?? 1,
+    width: section.width ?? 1,
+    yOffset: section.yOffset ?? 0,
+    xOffset: section.xOffset ?? 0,
+    parentSectionId: section.parentSectionId ?? "0",
+    items: section.items ?? [],
+  } satisfies DynamicSection;
+};
+
+const createSections = (initialYOffsets: number[]) => {
+  return initialYOffsets.map((yOffset, index) => ({
+    id: yOffset.toString(),
+    kind: index % 2 === 0 ? "empty" : "category",
+    name: "Category",
+    yOffset,
+    xOffset: 0,
+    items: [createItem({ id: yOffset.toString() })],
+  })) satisfies Section[];
+};

apps/nextjs/src/components/board/sections/category/category-actions.ts

@@ -2,10 +2,12 @@ import { useCallback } from "react";
 
 import { createId } from "@homarr/db/client";
 
-import type { CategorySection, EmptySection, Section } from "~/app/[locale]/boards/_types";
+import type { CategorySection, EmptySection } from "~/app/[locale]/boards/_types";
 import { useUpdateBoard } from "~/app/[locale]/boards/(content)/_client";
 import type { MoveCategoryInput } from "./actions/move-category";
 import { moveCategoryCallback } from "./actions/move-category";
+import type { RemoveCategoryInput } from "./actions/remove-category";
+import { removeCategoryCallback } from "./actions/remove-category";
 
 interface AddCategory {
   name: string;
@@ -17,10 +19,6 @@ interface RenameCategory {
   name: string;
 }
 
-interface RemoveCategory {
-  id: string;
-}
-
 export const useCategoryActions = () => {
   const { updateBoard } = useUpdateBoard();
 
@@ -132,57 +130,8 @@ export const useCategoryActions = () => {
   );
 
   const removeCategory = useCallback(
-    ({ id: categoryId }: RemoveCategory) => {
-      updateBoard((previous) => {
-        const currentCategory = previous.sections.find(
-          (section): section is CategorySection => section.kind === "category" && section.id === categoryId,
-        );
-        if (!currentCategory) return previous;
-
-        const aboveWrapper = previous.sections.find(
-          (section): section is EmptySection =>
-            section.kind === "empty" && section.yOffset === currentCategory.yOffset - 1,
-        );
-
-        const removedWrapper = previous.sections.find(
-          (section): section is EmptySection =>
-            section.kind === "empty" && section.yOffset === currentCategory.yOffset + 1,
-        );
-
-        if (!aboveWrapper || !removedWrapper) return previous;
-
-        // Calculate the yOffset for the items in the currentCategory and removedWrapper to add them with the same offset to the aboveWrapper
-        const aboveYOffset = calculateYHeightWithOffset(aboveWrapper);
-        const categoryYOffset = calculateYHeightWithOffset(currentCategory);
-
-        const previousCategoryItems = currentCategory.items.map((item) => ({
-          ...item,
-          yOffset: item.yOffset + aboveYOffset,
-        }));
-        const previousBelowWrapperItems = removedWrapper.items.map((item) => ({
-          ...item,
-          yOffset: item.yOffset + aboveYOffset + categoryYOffset,
-        }));
-
-        return {
-          ...previous,
-          sections: [
-            ...previous.sections.filter((section) => section.yOffset < currentCategory.yOffset - 1),
-            {
-              ...aboveWrapper,
-              items: [...aboveWrapper.items, ...previousCategoryItems, ...previousBelowWrapperItems],
-            },
-            ...previous.sections
-              .filter(
-                (section): section is CategorySection | EmptySection => section.yOffset >= currentCategory.yOffset + 2,
-              )
-              .map((section) => ({
-                ...section,
-                position: section.yOffset - 2,
-              })),
-          ],
-        };
-      });
+    (input: RemoveCategoryInput) => {
+      updateBoard(removeCategoryCallback(input));
     },
     [updateBoard],
   );
@@ -195,10 +144,3 @@ export const useCategoryActions = () => {
     removeCategory,
   };
 };
-
-const calculateYHeightWithOffset = (section: Section) =>
-  section.items.reduce((acc, item) => {
-    const yHeightWithOffset = item.yOffset + item.height;
-    if (yHeightWithOffset > acc) return yHeightWithOffset;
-    return acc;
-  }, 0);