Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,10 @@ const validateResponse = (
...responseUpdateInput.data,
};

const isFinished = responseUpdateInput.finished ?? false;

const validationErrors = validateResponseData(
survey.blocks,
mergedData,
responseUpdateInput.language ?? response.language ?? "en",
isFinished,
survey.questions
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ const validateResponse = (responseInputData: TResponseInput, survey: TSurvey) =>
survey.blocks,
responseInputData.data,
responseInputData.language ?? "en",
responseInputData.finished,
survey.questions
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ export const PUT = withV1ApiWrapper({
result.survey.blocks,
responseUpdate.data,
responseUpdate.language ?? "en",
responseUpdate.finished,
result.survey.questions
);

Expand Down
1 change: 0 additions & 1 deletion apps/web/app/api/v1/management/responses/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ export const POST = withV1ApiWrapper({
surveyResult.survey.blocks,
responseInput.data,
responseInput.language ?? "en",
responseInput.finished,
surveyResult.survey.questions
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ export const POST = async (request: Request, context: Context): Promise<Response
survey.blocks,
responseInputData.data,
responseInputData.language ?? "en",
responseInputData.finished,
survey.questions
);

Expand Down
61 changes: 47 additions & 14 deletions apps/web/modules/api/lib/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe("validateResponseData", () => {
mockGetElementsFromBlocks.mockReturnValue(mockElements);
mockValidateBlockResponses.mockReturnValue({});

validateResponseData([], mockResponseData, "en", true, mockQuestions);
validateResponseData([], mockResponseData, "en", mockQuestions);

expect(mockTransformQuestionsToBlocks).toHaveBeenCalledWith(mockQuestions, []);
expect(mockGetElementsFromBlocks).toHaveBeenCalledWith(transformedBlocks);
Expand All @@ -105,15 +105,15 @@ describe("validateResponseData", () => {
mockGetElementsFromBlocks.mockReturnValue(mockElements);
mockValidateBlockResponses.mockReturnValue({});

validateResponseData(mockBlocks, mockResponseData, "en", true, mockQuestions);
validateResponseData(mockBlocks, mockResponseData, "en", mockQuestions);

expect(mockTransformQuestionsToBlocks).not.toHaveBeenCalled();
});

test("should return null when both blocks and questions are empty", () => {
expect(validateResponseData([], mockResponseData, "en", true, [])).toBeNull();
expect(validateResponseData(null, mockResponseData, "en", true, [])).toBeNull();
expect(validateResponseData(undefined, mockResponseData, "en", true, null)).toBeNull();
expect(validateResponseData([], mockResponseData, "en", [])).toBeNull();
expect(validateResponseData(null, mockResponseData, "en", [])).toBeNull();
expect(validateResponseData(undefined, mockResponseData, "en", null)).toBeNull();
});

test("should use default language code", () => {
Expand All @@ -125,25 +125,58 @@ describe("validateResponseData", () => {
expect(mockValidateBlockResponses).toHaveBeenCalledWith(mockElements, mockResponseData, "en");
});

test("should validate only present fields when finished is false", () => {
test("should validate only fields present in responseData", () => {
const partialResponseData: TResponseData = { element1: "test" };
const partialElements = [mockElements[0]];
const elementsToValidate = [mockElements[0]];
mockGetElementsFromBlocks.mockReturnValue(mockElements);
mockValidateBlockResponses.mockReturnValue({});

validateResponseData(mockBlocks, partialResponseData, "en", false);
validateResponseData(mockBlocks, partialResponseData, "en");

expect(mockValidateBlockResponses).toHaveBeenCalledWith(partialElements, partialResponseData, "en");
expect(mockValidateBlockResponses).toHaveBeenCalledWith(elementsToValidate, partialResponseData, "en");
});

test("should validate all fields when finished is true", () => {
const partialResponseData: TResponseData = { element1: "test" };
mockGetElementsFromBlocks.mockReturnValue(mockElements);
test("should never validate elements not in responseData", () => {
const blocksWithTwoElements: TSurveyBlock[] = [
...mockBlocks,
{
id: "block2",
name: "Block 2",
elements: [
{
id: "element2",
type: TSurveyElementTypeEnum.OpenText,
headline: { default: "Q2" },
required: true,
inputType: "text",
charLimit: { enabled: false },
},
],
},
];
const allElements = [
...mockElements,
{
id: "element2",
type: TSurveyElementTypeEnum.OpenText,
headline: { default: "Q2" },
required: true,
inputType: "text",
charLimit: { enabled: false },
},
];
const responseDataWithOnlyElement1: TResponseData = { element1: "test" };
mockGetElementsFromBlocks.mockReturnValue(allElements);
mockValidateBlockResponses.mockReturnValue({});

validateResponseData(mockBlocks, partialResponseData, "en", true);
validateResponseData(blocksWithTwoElements, responseDataWithOnlyElement1, "en");

expect(mockValidateBlockResponses).toHaveBeenCalledWith(mockElements, partialResponseData, "en");
// Only element1 should be validated, not element2 (even though it's required)
expect(mockValidateBlockResponses).toHaveBeenCalledWith(
[allElements[0]],
responseDataWithOnlyElement1,
"en"
);
});
});

Expand Down
14 changes: 5 additions & 9 deletions apps/web/modules/api/lib/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,20 @@ import { getElementsFromBlocks } from "@/lib/survey/utils";
import { ApiErrorDetails } from "@/modules/api/v2/types/api-error";

/**
* Validates response data against survey validation rules
* Handles partial responses (in-progress) by only validating present fields when finished is false
* Validates response data against survey validation rules.
* Only validates elements that have data in responseData - never validates
* all survey elements regardless of completion status.
*
* @param blocks - Survey blocks containing elements with validation rules (preferred)
* @param responseData - Response data to validate (keyed by element ID)
* @param languageCode - Language code for error messages (defaults to "en")
* @param finished - Whether the response is finished (defaults to true for management APIs)
* @param questions - Survey questions (legacy format, used as fallback if blocks are empty)
* @returns Validation error map keyed by element ID, or null if validation passes
*/
export const validateResponseData = (
blocks: TSurveyBlock[] | undefined | null,
responseData: TResponseData,
languageCode: string = "en",
finished: boolean = true,
questions?: TSurveyQuestion[] | undefined | null
): TValidationErrorMap | null => {
// Use blocks if available, otherwise transform questions to blocks
Expand All @@ -42,11 +41,8 @@ export const validateResponseData = (
// Extract elements from blocks
const allElements = getElementsFromBlocks(blocksToUse);

// If response is not finished, only validate elements that are present in the response data
// This prevents "required" errors for fields the user hasn't reached yet
const elementsToValidate = finished
? allElements
: allElements.filter((element) => Object.keys(responseData).includes(element.id));
// Always validate only elements that are present in responseData
const elementsToValidate = allElements.filter((element) => Object.keys(responseData).includes(element.id));

// Validate selected elements
const errorMap = validateBlockResponses(elementsToValidate, responseData, languageCode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ export const PUT = (request: Request, props: { params: Promise<{ responseId: str
questionsResponse.data.blocks,
body.data,
body.language ?? "en",
body.finished,
questionsResponse.data.questions
);

Expand Down
1 change: 0 additions & 1 deletion apps/web/modules/api/v2/management/responses/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ export const POST = async (request: Request) =>
surveyQuestions.data.blocks,
body.data,
body.language ?? "en",
body.finished,
surveyQuestions.data.questions
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1276,6 +1276,7 @@ describe("segmentFilterToPrismaQuery", () => {
attributeKey: {
key: "age",
environmentId: mockEnvironmentId,
dataType: "number",
},
valueNumber: null,
},
Expand Down Expand Up @@ -1362,7 +1363,7 @@ describe("segmentFilterToPrismaQuery", () => {
{
attributes: {
some: {
attributeKey: { key: "purchaseDate" },
attributeKey: { key: "purchaseDate", dataType: "date" },
OR: [
{ valueDate: { lt: new Date(targetDate) } },
{ valueDate: null, value: { lt: new Date(targetDate).toISOString() } },
Expand Down Expand Up @@ -1406,7 +1407,7 @@ describe("segmentFilterToPrismaQuery", () => {
{
attributes: {
some: {
attributeKey: { key: "signupDate" },
attributeKey: { key: "signupDate", dataType: "date" },
OR: [
{ valueDate: { gt: new Date(targetDate) } },
{ valueDate: null, value: { gt: new Date(targetDate).toISOString() } },
Expand Down Expand Up @@ -1451,7 +1452,7 @@ describe("segmentFilterToPrismaQuery", () => {
{
attributes: {
some: {
attributeKey: { key: "lastActivityDate" },
attributeKey: { key: "lastActivityDate", dataType: "date" },
OR: [
{ valueDate: { gte: new Date(startDate), lte: new Date(endDate) } },
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ const buildDateAttributeFilterWhereClause = (filter: TSegmentAttributeFilter): P
return {
attributes: {
some: {
attributeKey: { key: contactAttributeKey },
attributeKey: { key: contactAttributeKey, dataType: "date" },
OR: [{ valueDate: dateCondition }, { valueDate: null, value: stringDateCondition }],
},
},
Expand Down Expand Up @@ -165,6 +165,7 @@ const buildNumberAttributeFilterWhereClause = async (
attributeKey: {
key: contactAttributeKey,
environmentId,
dataType: "number",
},
valueNumber: null,
},
Expand All @@ -183,6 +184,7 @@ const buildNumberAttributeFilterWhereClause = async (
JOIN "ContactAttributeKey" cak ON ca."attributeKeyId" = cak.id
WHERE cak.key = $1
AND cak."environmentId" = $4
AND cak."dataType" = 'number'
AND ca."valueNumber" IS NULL
AND ca.value ~ $3
AND ca.value::double precision ${sqlOp} $2
Expand Down
68 changes: 68 additions & 0 deletions apps/web/modules/ee/contacts/segments/lib/segments.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ vi.mock("@formbricks/database", () => ({
create: vi.fn(),
delete: vi.fn(),
update: vi.fn(),
upsert: vi.fn(),
findFirst: vi.fn(),
},
survey: {
Expand Down Expand Up @@ -206,6 +207,73 @@ describe("Segment Service Tests", () => {
vi.mocked(prisma.segment.create).mockRejectedValue(new Error("DB error"));
await expect(createSegment(mockSegmentCreateInput)).rejects.toThrow(Error);
});

test("should upsert a private segment without surveyId", async () => {
const privateInput: TSegmentCreateInput = {
...mockSegmentCreateInput,
isPrivate: true,
};
const privateSegmentPrisma = { ...mockSegmentPrisma, isPrivate: true };
vi.mocked(prisma.segment.upsert).mockResolvedValue(privateSegmentPrisma);
const segment = await createSegment(privateInput);
expect(segment).toEqual({ ...mockSegment, isPrivate: true });
expect(prisma.segment.upsert).toHaveBeenCalledWith({
where: {
environmentId_title: {
environmentId,
title: privateInput.title,
},
},
create: {
environmentId,
title: privateInput.title,
description: undefined,
isPrivate: true,
filters: [],
},
update: {
description: undefined,
filters: [],
},
select: selectSegment,
});
expect(prisma.segment.create).not.toHaveBeenCalled();
});

test("should upsert a private segment with surveyId", async () => {
const privateInputWithSurvey: TSegmentCreateInput = {
...mockSegmentCreateInput,
isPrivate: true,
surveyId,
};
const privateSegmentPrisma = { ...mockSegmentPrisma, isPrivate: true };
vi.mocked(prisma.segment.upsert).mockResolvedValue(privateSegmentPrisma);
const segment = await createSegment(privateInputWithSurvey);
expect(segment).toEqual({ ...mockSegment, isPrivate: true });
expect(prisma.segment.upsert).toHaveBeenCalledWith({
where: {
environmentId_title: {
environmentId,
title: privateInputWithSurvey.title,
},
},
create: {
environmentId,
title: privateInputWithSurvey.title,
description: undefined,
isPrivate: true,
filters: [],
surveys: { connect: { id: surveyId } },
},
update: {
description: undefined,
filters: [],
surveys: { connect: { id: surveyId } },
},
select: selectSegment,
});
expect(prisma.segment.create).not.toHaveBeenCalled();
});
});

describe("cloneSegment", () => {
Expand Down
Loading
Loading