fix: row/column deletion in matrix question (#6184)

apps/web/modules/survey/editor/components/matrix-question-form.test.tsx

@@ -2,7 +2,8 @@ import { createI18nString } from "@/lib/i18n/utils";
 import { findOptionUsedInLogic } from "@/modules/survey/editor/lib/utils";
 import { cleanup, render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { afterEach, describe, expect, test, vi } from "vitest";
+import React from "react";
+import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
 import {
   TSurvey,
   TSurveyLanguage,
@@ -12,6 +13,16 @@ import {
 import { TUserLocale } from "@formbricks/types/user";
 import { MatrixQuestionForm } from "./matrix-question-form";
 
+// Mock cuid2 to track CUID generation
+const mockCuids = ["cuid1", "cuid2", "cuid3", "cuid4", "cuid5", "cuid6"];
+let cuidIndex = 0;
+
+vi.mock("@paralleldrive/cuid2", () => ({
+  default: {
+    createId: vi.fn(() => mockCuids[cuidIndex++]),
+  },
+}));
+
 // Mock window.matchMedia - required for useAutoAnimate
 Object.defineProperty(window, "matchMedia", {
   writable: true,
@@ -386,4 +397,223 @@ describe("MatrixQuestionForm", () => {
 
     expect(mockUpdateQuestion).not.toHaveBeenCalled();
   });
+
+  // CUID functionality tests
+  describe("CUID Management", () => {
+    beforeEach(() => {
+      // Reset CUID index before each test
+      cuidIndex = 0;
+    });
+
+    test("generates stable CUIDs for rows and columns on initial render", () => {
+      const { rerender } = render(<MatrixQuestionForm {...defaultProps} />);
+
+      // Check that CUIDs are generated for initial items
+      expect(cuidIndex).toBe(6); // 3 rows + 3 columns
+
+      // Rerender with the same props - no new CUIDs should be generated
+      rerender(<MatrixQuestionForm {...defaultProps} />);
+      expect(cuidIndex).toBe(6); // Should remain the same
+    });
+
+    test("maintains stable CUIDs across rerenders", () => {
+      const TestComponent = ({ question }: { question: TSurveyMatrixQuestion }) => {
+        return <MatrixQuestionForm {...defaultProps} question={question} />;
+      };
+
+      const { rerender } = render(<TestComponent question={mockMatrixQuestion} />);
+
+      // Check initial CUID count
+      expect(cuidIndex).toBe(6); // 3 rows + 3 columns
+
+      // Rerender multiple times
+      rerender(<TestComponent question={mockMatrixQuestion} />);
+      rerender(<TestComponent question={mockMatrixQuestion} />);
+      rerender(<TestComponent question={mockMatrixQuestion} />);
+
+      // CUIDs should remain stable
+      expect(cuidIndex).toBe(6); // Should not increase
+    });
+
+    test("generates new CUIDs only when rows are added", async () => {
+      const user = userEvent.setup();
+
+      // Create a test component that can update its props
+      const TestComponent = () => {
+        const [question, setQuestion] = React.useState(mockMatrixQuestion);
+
+        const handleUpdateQuestion = (_: number, updates: Partial<TSurveyMatrixQuestion>) => {
+          setQuestion((prev) => ({ ...prev, ...updates }));
+        };
+
+        return (
+          <MatrixQuestionForm {...defaultProps} question={question} updateQuestion={handleUpdateQuestion} />
+        );
+      };
+
+      const { getByText } = render(<TestComponent />);
+
+      // Initial render should generate 6 CUIDs (3 rows + 3 columns)
+      expect(cuidIndex).toBe(6);
+
+      // Add a new row
+      const addRowButton = getByText("environments.surveys.edit.add_row");
+      await user.click(addRowButton);
+
+      // Should generate 1 new CUID for the new row
+      expect(cuidIndex).toBe(7);
+    });
+
+    test("generates new CUIDs only when columns are added", async () => {
+      const user = userEvent.setup();
+
+      // Create a test component that can update its props
+      const TestComponent = () => {
+        const [question, setQuestion] = React.useState(mockMatrixQuestion);
+
+        const handleUpdateQuestion = (_: number, updates: Partial<TSurveyMatrixQuestion>) => {
+          setQuestion((prev) => ({ ...prev, ...updates }));
+        };
+
+        return (
+          <MatrixQuestionForm {...defaultProps} question={question} updateQuestion={handleUpdateQuestion} />
+        );
+      };
+
+      const { getByText } = render(<TestComponent />);
+
+      // Initial render should generate 6 CUIDs (3 rows + 3 columns)
+      expect(cuidIndex).toBe(6);
+
+      // Add a new column
+      const addColumnButton = getByText("environments.surveys.edit.add_column");
+      await user.click(addColumnButton);
+
+      // Should generate 1 new CUID for the new column
+      expect(cuidIndex).toBe(7);
+    });
+
+    test("maintains CUID stability when items are deleted", async () => {
+      const user = userEvent.setup();
+      const { findAllByTestId, rerender } = render(<MatrixQuestionForm {...defaultProps} />);
+
+      // Mock that no items are used in logic
+      vi.mocked(findOptionUsedInLogic).mockReturnValue(-1);
+
+      // Initial render: 6 CUIDs generated
+      expect(cuidIndex).toBe(6);
+
+      // Delete a row
+      const deleteButtons = await findAllByTestId("tooltip-renderer");
+      await user.click(deleteButtons[0].querySelector("button") as HTMLButtonElement);
+
+      // No new CUIDs should be generated for deletion
+      expect(cuidIndex).toBe(6);
+
+      // Rerender should not generate new CUIDs
+      rerender(<MatrixQuestionForm {...defaultProps} />);
+      expect(cuidIndex).toBe(6);
+    });
+
+    test("handles mixed operations maintaining CUID stability", async () => {
+      const user = userEvent.setup();
+
+      // Create a test component that can update its props
+      const TestComponent = () => {
+        const [question, setQuestion] = React.useState(mockMatrixQuestion);
+
+        const handleUpdateQuestion = (_: number, updates: Partial<TSurveyMatrixQuestion>) => {
+          setQuestion((prev) => ({ ...prev, ...updates }));
+        };
+
+        return (
+          <MatrixQuestionForm {...defaultProps} question={question} updateQuestion={handleUpdateQuestion} />
+        );
+      };
+
+      const { getByText, findAllByTestId } = render(<TestComponent />);
+
+      // Mock that no items are used in logic
+      vi.mocked(findOptionUsedInLogic).mockReturnValue(-1);
+
+      // Initial: 6 CUIDs
+      expect(cuidIndex).toBe(6);
+
+      // Add a row: +1 CUID
+      const addRowButton = getByText("environments.surveys.edit.add_row");
+      await user.click(addRowButton);
+      expect(cuidIndex).toBe(7);
+
+      // Add a column: +1 CUID
+      const addColumnButton = getByText("environments.surveys.edit.add_column");
+      await user.click(addColumnButton);
+      expect(cuidIndex).toBe(8);
+
+      // Delete a row: no new CUIDs
+      const deleteButtons = await findAllByTestId("tooltip-renderer");
+      await user.click(deleteButtons[0].querySelector("button") as HTMLButtonElement);
+      expect(cuidIndex).toBe(8);
+
+      // Delete a column: no new CUIDs
+      const updatedDeleteButtons = await findAllByTestId("tooltip-renderer");
+      await user.click(updatedDeleteButtons[2].querySelector("button") as HTMLButtonElement);
+      expect(cuidIndex).toBe(8);
+    });
+
+    test("CUID arrays are properly maintained when items are deleted in order", async () => {
+      const user = userEvent.setup();
+      const propsWithManyRows = {
+        ...defaultProps,
+        question: {
+          ...mockMatrixQuestion,
+          rows: [
+            createI18nString("Row 1", ["en"]),
+            createI18nString("Row 2", ["en"]),
+            createI18nString("Row 3", ["en"]),
+            createI18nString("Row 4", ["en"]),
+          ],
+        },
+      };
+
+      const { findAllByTestId } = render(<MatrixQuestionForm {...propsWithManyRows} />);
+
+      // Mock that no items are used in logic
+      vi.mocked(findOptionUsedInLogic).mockReturnValue(-1);
+
+      // Initial: 7 CUIDs (4 rows + 3 columns)
+      expect(cuidIndex).toBe(7);
+
+      // Delete first row
+      const deleteButtons = await findAllByTestId("tooltip-renderer");
+      await user.click(deleteButtons[0].querySelector("button") as HTMLButtonElement);
+
+      // Verify the correct row was deleted (should be Row 2, Row 3, Row 4 remaining)
+      expect(mockUpdateQuestion).toHaveBeenLastCalledWith(0, {
+        rows: [
+          propsWithManyRows.question.rows[1],
+          propsWithManyRows.question.rows[2],
+          propsWithManyRows.question.rows[3],
+        ],
+      });
+
+      // No new CUIDs should be generated
+      expect(cuidIndex).toBe(7);
+    });
+
+    test("CUID generation is consistent across component instances", () => {
+      // Reset CUID index
+      cuidIndex = 0;
+
+      // Render first instance
+      const { unmount } = render(<MatrixQuestionForm {...defaultProps} />);
+      expect(cuidIndex).toBe(6);
+
+      // Unmount and render second instance
+      unmount();
+      render(<MatrixQuestionForm {...defaultProps} />);
+
+      // Should generate 6 more CUIDs for the new instance
+      expect(cuidIndex).toBe(12);
+    });
+  });
 });

apps/web/modules/survey/editor/components/matrix-question-form.tsx

@@ -8,9 +8,10 @@ import { Label } from "@/modules/ui/components/label";
 import { ShuffleOptionSelect } from "@/modules/ui/components/shuffle-option-select";
 import { TooltipRenderer } from "@/modules/ui/components/tooltip";
 import { useAutoAnimate } from "@formkit/auto-animate/react";
+import cuid2 from "@paralleldrive/cuid2";
 import { useTranslate } from "@tolgee/react";
 import { PlusIcon, TrashIcon } from "lucide-react";
-import type { JSX } from "react";
+import { type JSX, useMemo, useRef } from "react";
 import toast from "react-hot-toast";
 import { TI18nString, TSurvey, TSurveyMatrixQuestion } from "@formbricks/types/surveys/types";
 import { TUserLocale } from "@formbricks/types/user";
@@ -39,6 +40,45 @@ export const MatrixQuestionForm = ({
 }: MatrixQuestionFormProps): JSX.Element => {
   const languageCodes = extractLanguageCodes(localSurvey.languages);
   const { t } = useTranslate();
+
+  // Refs to maintain stable CUIDs across renders
+  const cuidRefs = useRef<{
+    rows: string[];
+    columns: string[];
+  }>({
+    rows: [],
+    columns: [],
+  });
+
+  // Generic function to ensure CUIDs are synchronized with the current state
+  const ensureCuids = (type: "rows" | "columns", currentItems: TI18nString[]) => {
+    const currentCuids = cuidRefs.current[type];
+    if (currentCuids.length !== currentItems.length) {
+      if (currentItems.length > currentCuids.length) {
+        // Add new CUIDs for added items
+        const newCuids = Array(currentItems.length - currentCuids.length)
+          .fill(null)
+          .map(() => cuid2.createId());
+        cuidRefs.current[type] = [...currentCuids, ...newCuids];
+      } else {
+        // Remove CUIDs for deleted items (keep the remaining ones in order)
+        cuidRefs.current[type] = currentCuids.slice(0, currentItems.length);
+      }
+    }
+  };
+
+  // Generic function to get items with CUIDs
+  const getItemsWithCuid = (type: "rows" | "columns", items: TI18nString[]) => {
+    ensureCuids(type, items);
+    return items.map((item, index) => ({
+      ...item,
+      id: cuidRefs.current[type][index],
+    }));
+  };
+
+  const rowsWithCuid = useMemo(() => getItemsWithCuid("rows", question.rows), [question.rows]);
+  const columnsWithCuid = useMemo(() => getItemsWithCuid("columns", question.columns), [question.columns]);
+
   // Function to add a new Label input field
   const handleAddLabel = (type: "row" | "column") => {
     if (type === "row") {
@@ -79,6 +119,11 @@ export const MatrixQuestionForm = ({
     }
 
     const updatedLabels = labels.filter((_, idx) => idx !== index);
+
+    // Update the CUID arrays when deleting
+    const cuidType = type === "row" ? "rows" : "columns";
+    cuidRefs.current[cuidType] = cuidRefs.current[cuidType].filter((_, idx) => idx !== index);
+
     if (type === "row") {
       updateQuestion(questionIdx, { rows: updatedLabels });
     } else {
@@ -182,8 +227,8 @@ export const MatrixQuestionForm = ({
           {/* Rows section */}
           <Label htmlFor="rows">{t("environments.surveys.edit.rows")}</Label>
           <div className="mt-2 flex flex-col gap-2" ref={parent}>
-            {question.rows.map((row, index) => (
-              <div className="flex items-center" key={`${row}-${index}`}>
+            {rowsWithCuid.map((row, index) => (
+              <div className="flex items-center" key={row.id}>
                 <QuestionFormInput
                   id={`row-${index}`}
                   label={""}
@@ -232,8 +277,8 @@ export const MatrixQuestionForm = ({
           {/* Columns section */}
           <Label htmlFor="columns">{t("environments.surveys.edit.columns")}</Label>
           <div className="mt-2 flex flex-col gap-2" ref={parent}>
-            {question.columns.map((column, index) => (
-              <div className="flex items-center" key={`${column}-${index}`}>
+            {columnsWithCuid.map((column, index) => (
+              <div className="flex items-center" key={column.id}>
                 <QuestionFormInput
                   id={`column-${index}`}
                   label={""}