diff --git a/packages/database/data-migrations/20240712123456_segments_cleanup/data-migration.ts b/packages/database/data-migrations/20240712123456_segments_cleanup/data-migration.ts new file mode 100644 index 0000000000..45c741aef7 --- /dev/null +++ b/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()); diff --git a/packages/database/package.json b/packages/database/package.json index c9c442065f..20013a5c2e 100644 --- a/packages/database/package.json +++ b/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", diff --git a/packages/lib/segment/service.ts b/packages/lib/segment/service.ts index ba79188aa2..a7f0996596 100644 --- a/packages/lib/segment/service.ts +++ b/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 => { 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 => { 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 })); diff --git a/packages/lib/segment/tests/__mocks__/segment.mock.ts b/packages/lib/segment/tests/__mocks__/segment.mock.ts index 13061d9fe2..3c58e7e67c 100644 --- a/packages/lib/segment/tests/__mocks__/segment.mock.ts +++ b/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"], diff --git a/packages/lib/segment/tests/segment.test.ts b/packages/lib/segment/tests/segment.test.ts index b3d34f839f..68014433fb 100644 --- a/packages/lib/segment/tests/segment.test.ts +++ b/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); }); }); }); diff --git a/packages/lib/survey/service.ts b/packages/lib/survey/service.ts index 127e466883..0245ae5bf3 100644 --- a/packages/lib/survey/service.ts +++ b/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 }); diff --git a/packages/ui/ConfirmDeleteSegmentModal/index.tsx b/packages/ui/ConfirmDeleteSegmentModal/index.tsx index 6378ecc4ee..f37b72b474 100644 --- a/packages/ui/ConfirmDeleteSegmentModal/index.tsx +++ b/packages/ui/ConfirmDeleteSegmentModal/index.tsx @@ -29,38 +29,29 @@ export const ConfirmDeleteSegmentModal = ({
{segmentHasSurveys && (
-

If you delete this segment, this will happen:

-
    -
  • - This segment will be removed from these surveys: -
      - {segment.activeSurveys.map((survey) => ( -
    1. {survey}
    2. - ))} - - {segment.inactiveSurveys.map((survey) => ( -
    3. {survey}
    4. - ))} -
    -
  • -
  • - These surveys will be paused. -
  • -
+

You cannot delete this segment since it’s still used in these surveys:

+
    + {segment.activeSurveys.map((survey) => ( +
  1. {survey}
  2. + ))} + {segment.inactiveSurveys.map((survey) => ( +
  3. {survey}
  4. + ))} +
)} -

This action cannot be undone.

+

+ {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."} +

-