fix: surveys with no segment fixes (#2903)

Co-authored-by: Dhruwang Jariwala <67850763+Dhruwang@users.noreply.github.com>

packages/database/data-migrations/20240712123456_segments_cleanup/data-migration.ts

@@ -0,0 +1,66 @@
+import { PrismaClient } from "@prisma/client";
+
+const prisma = new PrismaClient();
+
+async function main() {
+  await prisma.$transaction(
+    async (tx) => {
+      console.log("starting migration");
+      const segmentsWithNoSurveys = await tx.segment.findMany({
+        where: {
+          surveys: {
+            none: {},
+          },
+        },
+      });
+
+      const surveyIds = segmentsWithNoSurveys.map((segment) => segment.id);
+
+      await tx.segment.deleteMany({
+        where: {
+          id: {
+            in: surveyIds,
+          },
+        },
+      });
+
+      console.log(`Deleted ${segmentsWithNoSurveys.length} segments with no surveys`);
+
+      const appSurveysWithoutSegment = await tx.survey.findMany({
+        where: {
+          type: "app",
+          segmentId: null,
+        },
+      });
+
+      console.log(`Found ${appSurveysWithoutSegment.length} app surveys without a segment`);
+
+      const segmentPromises = [];
+
+      for (const appSurvey of appSurveysWithoutSegment) {
+        // create a new private segment for each app survey
+
+        segmentPromises.push(
+          tx.segment.create({
+            data: {
+              title: appSurvey.id,
+              isPrivate: true,
+              environment: { connect: { id: appSurvey.environmentId } },
+              surveys: { connect: { id: appSurvey.id } },
+            },
+          })
+        );
+      }
+
+      await Promise.all(segmentPromises);
+    },
+    { timeout: 50000 }
+  );
+}
+
+main()
+  .catch(async (e) => {
+    console.error(e);
+    process.exit(1);
+  })
+  .finally(async () => await prisma.$disconnect());

packages/database/package.json

@@ -40,7 +40,8 @@
     "data-migration:product-config": "ts-node ./data-migrations/20240612115151_adds_product_config/data-migration.ts",
     "data-migration:v2.2": "pnpm data-migration:adds_app_and_website_status_indicator && pnpm data-migration:product-config && pnpm data-migration:pricing-v2",
     "data-migration:zh-to-zh-Hans": "ts-node ./data-migrations/20240625101352_update_zh_to_zh-Hans/data-migration.ts",
-    "data-migration:v2.3": "pnpm data-migration:zh-to-zh-Hans"
+    "data-migration:v2.3": "pnpm data-migration:zh-to-zh-Hans",
+    "data-migration:segments-cleanup": "ts-node ./data-migrations/20240712123456_segments_cleanup/data-migration.ts"
   },
   "dependencies": {
     "@prisma/client": "^5.16.2",

packages/lib/segment/service.ts

@@ -3,7 +3,12 @@ import { cache as reactCache } from "react";
 import { prisma } from "@formbricks/database";
 import { ZString } from "@formbricks/types/common";
 import { ZId } from "@formbricks/types/environment";
-import { DatabaseError, ResourceNotFoundError, ValidationError } from "@formbricks/types/errors";
+import {
+  DatabaseError,
+  OperationNotAllowedError,
+  ResourceNotFoundError,
+  ValidationError,
+} from "@formbricks/types/errors";
 import {
   TActionMetric,
   TAllOperators,
@@ -236,6 +241,10 @@ export const deleteSegment = async (segmentId: string): Promise<TSegment> => {
       throw new ResourceNotFoundError("segment", segmentId);
     }
 
+    if (currentSegment.surveys?.length) {
+      throw new OperationNotAllowedError("Cannot delete a segment that is associated with a survey");
+    }
+
     const segment = await prisma.segment.delete({
       where: {
         id: segmentId,
@@ -243,20 +252,6 @@ export const deleteSegment = async (segmentId: string): Promise<TSegment> => {
       select: selectSegment,
     });
 
-    // pause all the running surveys that are using this segment
-    const surveyIds = segment.surveys.map((survey) => survey.id);
-    if (!!surveyIds?.length) {
-      await prisma.survey.updateMany({
-        where: {
-          id: { in: surveyIds },
-          status: "inProgress",
-        },
-        data: {
-          status: "paused",
-        },
-      });
-    }
-
     segmentCache.revalidate({ id: segmentId, environmentId: segment.environmentId });
     segment.surveys.map((survey) => surveyCache.revalidate({ id: survey.id }));
 

packages/lib/segment/tests/__mocks__/segment.mock.ts

@@ -10,6 +10,7 @@ import {
 } from "@formbricks/types/segment";
 
 export const mockSegmentId = "rh2eual2apby2bx0r027ru70";
+export const mockDeleteSegmentId = "to336z1uth9cvyb1sh7k9i77";
 export const mockEnvironmentId = "t7fszh4tsotoe87ppa6lqhie";
 export const mockSurveyId = "phz5mjwvatwc0dqwuip90qpv";
 export const mockFilterGroupId = "wi6zz4ekmcwi08bhv1hmgqcr";
@@ -164,6 +165,18 @@ export const mockSegmentPrisma = {
   surveys: [{ id: mockSurveyId }],
 };
 
+export const mockDeleteSegmentPrisma = {
+  ...mockSegmentPrisma,
+  id: mockDeleteSegmentId,
+  surveys: [],
+};
+
+export const mockDeleteSegment = {
+  ...mockSegment,
+  id: mockDeleteSegmentId,
+  surveys: [],
+};
+
 export const mockSegmentActiveInactiveSurves = {
   activeSurveys: ["Churn Survey"],
   inactiveSurveys: ["NPS Survey"],

packages/lib/segment/tests/segment.test.ts

@@ -1,6 +1,9 @@
 import { prisma } from "../../__mocks__/database";
 import {
   getMockSegmentFilters,
+  mockDeleteSegment,
+  mockDeleteSegmentId,
+  mockDeleteSegmentPrisma,
   mockEnvironmentId,
   mockEvaluateSegmentUserData,
   mockSegment,
@@ -13,7 +16,7 @@ import {
 import { Prisma } from "@prisma/client";
 import { beforeEach, describe, expect, it } from "vitest";
 import { testInputValidation } from "vitestSetup";
-import { DatabaseError, ResourceNotFoundError } from "@formbricks/types/errors";
+import { DatabaseError, OperationNotAllowedError, ResourceNotFoundError } from "@formbricks/types/errors";
 import {
   cloneSegment,
   createSegment,
@@ -255,9 +258,10 @@ describe("Tests for updateSegment service", () => {
 describe("Tests for deleteSegment service", () => {
   describe("Happy Path", () => {
     it("Deletes a user segment", async () => {
-      prisma.segment.delete.mockResolvedValue(mockSegmentPrisma);
-      const result = await deleteSegment(mockSegmentId);
-      expect(result).toEqual(mockSegment);
+      prisma.segment.findUnique.mockResolvedValue(mockDeleteSegmentPrisma);
+      prisma.segment.delete.mockResolvedValue(mockDeleteSegmentPrisma);
+      const result = await deleteSegment(mockDeleteSegmentId);
+      expect(result).toEqual(mockDeleteSegment);
     });
   });
 
@@ -266,7 +270,7 @@ describe("Tests for deleteSegment service", () => {
 
     it("Throws a ResourceNotFoundError error if the user segment does not exist", async () => {
       prisma.segment.findUnique.mockResolvedValue(null);
-      await expect(deleteSegment(mockSegmentId)).rejects.toThrow(ResourceNotFoundError);
+      await expect(deleteSegment(mockDeleteSegmentId)).rejects.toThrow(ResourceNotFoundError);
     });
 
     it("Throws a DatabaseError error if there is a PrismaClientKnownRequestError", async () => {
@@ -276,16 +280,21 @@ describe("Tests for deleteSegment service", () => {
         clientVersion: "0.0.1",
       });
 
+      prisma.segment.findUnique.mockResolvedValue(mockDeleteSegmentPrisma);
       prisma.segment.delete.mockRejectedValue(errToThrow);
 
-      await expect(deleteSegment(mockSegmentId)).rejects.toThrow(DatabaseError);
+      await expect(deleteSegment(mockDeleteSegmentId)).rejects.toThrow(DatabaseError);
+    });
+
+    it("Throws an OperationNotAllowedError if the segment is associated with a survey", async () => {
+      await expect(deleteSegment(mockSegmentId)).rejects.toThrow(OperationNotAllowedError);
     });
 
     it("Throws a generic Error for unexpected exceptions", async () => {
       const mockErrorMessage = "Mock error message";
       prisma.segment.delete.mockRejectedValue(new Error(mockErrorMessage));
 
-      await expect(deleteSegment(mockSegmentId)).rejects.toThrow(Error);
+      await expect(deleteSegment(mockDeleteSegmentId)).rejects.toThrow(Error);
     });
   });
 });

packages/lib/survey/service.ts

@@ -1068,8 +1068,10 @@ export const loadNewSegmentInSurvey = async (surveyId: string, newSegmentId: str
       throw new ResourceNotFoundError("survey", surveyId);
     }
 
-    const currentSegment = await getSegment(newSegmentId);
-    if (!currentSegment) {
+    const currentSurveySegment = currentSurvey.segment;
+
+    const newSegment = await getSegment(newSegmentId);
+    if (!newSegment) {
       throw new ResourceNotFoundError("segment", newSegmentId);
     }
 
@@ -1087,6 +1089,14 @@ export const loadNewSegmentInSurvey = async (surveyId: string, newSegmentId: str
       },
     });
 
+    if (
+      currentSurveySegment &&
+      currentSurveySegment.isPrivate &&
+      currentSurveySegment.title === currentSurvey.id
+    ) {
+      await deleteSegment(currentSurveySegment.id);
+    }
+
     segmentCache.revalidate({ id: newSegmentId });
     surveyCache.revalidate({ id: surveyId });
 

packages/ui/ConfirmDeleteSegmentModal/index.tsx

@@ -29,38 +29,29 @@ export const ConfirmDeleteSegmentModal = ({
       <div className="text-slate-900">
         {segmentHasSurveys && (
           <div className="space-y-2">
-            <p>If you delete this segment, this will happen:</p>
-            <ul className="ml-4 list-disc">
-              <li>
-                This segment will be <b>removed</b> from these surveys:
-                <ol className="my-2 ml-4 list-decimal text-sm">
-                  {segment.activeSurveys.map((survey) => (
-                    <li key={survey}>{survey}</li>
-                  ))}
-
-                  {segment.inactiveSurveys.map((survey) => (
-                    <li key={survey}>{survey}</li>
-                  ))}
-                </ol>
-              </li>
-              <li>
-                These surveys will be <b>paused.</b>
-              </li>
-            </ul>
+            <p>You cannot delete this segment since it’s still used in these surveys:</p>
+            <ol className="my-2 ml-4 list-decimal">
+              {segment.activeSurveys.map((survey) => (
+                <li key={survey}>{survey}</li>
+              ))}
+              {segment.inactiveSurveys.map((survey) => (
+                <li key={survey}>{survey}</li>
+              ))}
+            </ol>
           </div>
         )}
-        <p className="mt-2">This action cannot be undone.</p>
+        <p className="mt-2">
+          {segmentHasSurveys
+            ? "Please remove the segment from these surveys in order to delete it."
+            : "Are you sure you want to delete this segment? This action cannot be undone."}
+        </p>
       </div>
 
       <div className="mt-4 space-x-2 text-right">
         <Button variant="minimal" onClick={() => setOpen(false)}>
           Cancel
         </Button>
-        <Button
-          variant="warn"
-          onClick={() => {
-            handleDelete();
-          }}>
+        <Button variant="warn" onClick={handleDelete} disabled={segmentHasSurveys}>
           Delete
         </Button>
       </div>