diff --git a/apps/web/modules/ee/contacts/segments/lib/filter/prisma-query.test.ts b/apps/web/modules/ee/contacts/segments/lib/filter/prisma-query.test.ts index c013f66e02f4..9a0a96368d61 100644 --- a/apps/web/modules/ee/contacts/segments/lib/filter/prisma-query.test.ts +++ b/apps/web/modules/ee/contacts/segments/lib/filter/prisma-query.test.ts @@ -5,10 +5,14 @@ import { getSegment } from "../segments"; import { segmentFilterToPrismaQuery } from "./prisma-query"; const mockQueryRawUnsafe = vi.fn(); +const mockFindFirst = vi.fn(); vi.mock("@formbricks/database", () => ({ prisma: { $queryRawUnsafe: (...args: unknown[]) => mockQueryRawUnsafe(...args), + contactAttribute: { + findFirst: (...args: unknown[]) => mockFindFirst(...args), + }, }, })); @@ -26,7 +30,9 @@ describe("segmentFilterToPrismaQuery", () => { beforeEach(() => { vi.clearAllMocks(); - // Default mock: number filter raw SQL returns one matching contact + // Default: backfill is complete, no un-migrated rows + mockFindFirst.mockResolvedValue(null); + // Fallback path mock: raw SQL returns one matching contact when un-migrated rows exist mockQueryRawUnsafe.mockResolvedValue([{ contactId: "mock-contact-1" }]); }); @@ -145,7 +151,16 @@ describe("segmentFilterToPrismaQuery", () => { }, }, ], - OR: [{ id: { in: ["mock-contact-1"] } }], + OR: [ + { + attributes: { + some: { + attributeKey: { key: "age" }, + valueNumber: { gt: 30 }, + }, + }, + }, + ], }); } }); @@ -757,7 +772,12 @@ describe("segmentFilterToPrismaQuery", () => { }); expect(subgroup.AND[0].AND[2]).toStrictEqual({ - id: { in: ["mock-contact-1"] }, + attributes: { + some: { + attributeKey: { key: "age" }, + valueNumber: { gte: 18 }, + }, + }, }); // Segment inclusion @@ -1158,10 +1178,23 @@ describe("segmentFilterToPrismaQuery", () => { }, }); - // Second subgroup (numeric operators - now use raw SQL subquery returning contact IDs) + // Second subgroup (numeric operators - uses clean Prisma filter post-backfill) const secondSubgroup = whereClause.AND?.[0]; expect(secondSubgroup.AND[1].AND).toContainEqual({ - id: { in: ["mock-contact-1"] }, + attributes: { + some: { + attributeKey: { key: "loginCount" }, + valueNumber: { gt: 5 }, + }, + }, + }); + expect(secondSubgroup.AND[1].AND).toContainEqual({ + attributes: { + some: { + attributeKey: { key: "purchaseAmount" }, + valueNumber: { lte: 1000 }, + }, + }, }); // Third subgroup (negation operators in OR clause) @@ -1196,6 +1229,103 @@ describe("segmentFilterToPrismaQuery", () => { } }); + test("number filter falls back to raw SQL when un-migrated rows exist", async () => { + mockFindFirst.mockResolvedValue({ id: "unmigrated-row-1" }); + mockQueryRawUnsafe.mockResolvedValue([{ contactId: "mock-contact-1" }]); + + const filters: TBaseFilters = [ + { + id: "filter_1", + connector: null, + resource: { + id: "attr_1", + root: { + type: "attribute" as const, + contactAttributeKey: "age", + }, + value: 25, + qualifier: { + operator: "greaterThan", + }, + }, + }, + ]; + + const result = await segmentFilterToPrismaQuery(mockSegmentId, filters, mockEnvironmentId); + + expect(result.ok).toBe(true); + if (result.ok) { + const filterClause = result.data.whereClause.AND?.[1] as any; + expect(filterClause.AND[0]).toEqual({ + OR: [ + { + attributes: { + some: { + attributeKey: { key: "age" }, + valueNumber: { gt: 25 }, + }, + }, + }, + { id: { in: ["mock-contact-1"] } }, + ], + }); + } + + expect(mockFindFirst).toHaveBeenCalledWith({ + where: { + attributeKey: { + key: "age", + environmentId: mockEnvironmentId, + }, + valueNumber: null, + }, + select: { id: true }, + }); + + expect(mockQueryRawUnsafe).toHaveBeenCalled(); + const sqlCall = mockQueryRawUnsafe.mock.calls[0]; + expect(sqlCall[0]).toContain('cak."environmentId" = $4'); + expect(sqlCall[4]).toBe(mockEnvironmentId); + }); + + test("number filter uses clean Prisma query when backfill is complete", async () => { + const filters: TBaseFilters = [ + { + id: "filter_1", + connector: null, + resource: { + id: "attr_1", + root: { + type: "attribute" as const, + contactAttributeKey: "score", + }, + value: 100, + qualifier: { + operator: "lessEqual", + }, + }, + }, + ]; + + const result = await segmentFilterToPrismaQuery(mockSegmentId, filters, mockEnvironmentId); + + expect(result.ok).toBe(true); + if (result.ok) { + const filterClause = result.data.whereClause.AND?.[1] as any; + expect(filterClause.AND[0]).toEqual({ + attributes: { + some: { + attributeKey: { key: "score" }, + valueNumber: { lte: 100 }, + }, + }, + }); + } + + expect(mockFindFirst).toHaveBeenCalled(); + expect(mockQueryRawUnsafe).not.toHaveBeenCalled(); + }); + // ========================================== // DATE FILTER TESTS // ========================================== @@ -1638,8 +1768,15 @@ describe("segmentFilterToPrismaQuery", () => { mode: "insensitive", }); - // Number filter uses raw SQL subquery (transition code) returning contact IDs - expect(andConditions[1]).toEqual({ id: { in: ["mock-contact-1"] } }); + // Number filter uses clean Prisma filter post-backfill + expect(andConditions[1]).toEqual({ + attributes: { + some: { + attributeKey: { key: "purchaseCount" }, + valueNumber: { gt: 5 }, + }, + }, + }); // Date filter uses OR fallback with 'valueDate' and string 'value' expect((andConditions[2] as unknown as any).attributes.some.OR[0].valueDate).toHaveProperty("gte"); diff --git a/apps/web/modules/ee/contacts/segments/lib/filter/prisma-query.ts b/apps/web/modules/ee/contacts/segments/lib/filter/prisma-query.ts index c825aab4e98c..230e3d4fc32b 100644 --- a/apps/web/modules/ee/contacts/segments/lib/filter/prisma-query.ts +++ b/apps/web/modules/ee/contacts/segments/lib/filter/prisma-query.ts @@ -116,59 +116,100 @@ const buildDateAttributeFilterWhereClause = (filter: TSegmentAttributeFilter): P /** * Builds a Prisma where clause for number attribute filters. - * Uses a raw SQL subquery to handle both migrated rows (valueNumber populated) - * and un-migrated rows (valueNumber NULL, value contains numeric string). - * This is transition code for the deferred value backfill. + * Uses a clean Prisma query when all rows have valueNumber populated (post-backfill). + * Falls back to a raw SQL subquery for un-migrated rows (valueNumber NULL, value contains numeric string). * * TODO: After the backfill script has been run and all valueNumber columns are populated, - * revert this to the clean Prisma-only version that queries valueNumber directly. + * remove the un-migrated fallback path entirely. */ const buildNumberAttributeFilterWhereClause = async ( - filter: TSegmentAttributeFilter + filter: TSegmentAttributeFilter, + environmentId: string ): Promise => { const { root, qualifier, value } = filter; const { contactAttributeKey } = root; const { operator } = qualifier; const numericValue = typeof value === "number" ? value : Number(value); - const sqlOp = SQL_OPERATORS[operator]; - if (!sqlOp) { - return {}; + let valueNumberCondition: Prisma.FloatNullableFilter; + + switch (operator) { + case "greaterThan": + valueNumberCondition = { gt: numericValue }; + break; + case "greaterEqual": + valueNumberCondition = { gte: numericValue }; + break; + case "lessThan": + valueNumberCondition = { lt: numericValue }; + break; + case "lessEqual": + valueNumberCondition = { lte: numericValue }; + break; + default: + return {}; } - const matchingContactIds = await prisma.$queryRawUnsafe<{ contactId: string }[]>( + const migratedFilter: Prisma.ContactWhereInput = { + attributes: { + some: { + attributeKey: { key: contactAttributeKey }, + valueNumber: valueNumberCondition, + }, + }, + }; + + const hasUnmigratedRows = await prisma.contactAttribute.findFirst({ + where: { + attributeKey: { + key: contactAttributeKey, + environmentId, + }, + valueNumber: null, + }, + select: { id: true }, + }); + + if (!hasUnmigratedRows) { + return migratedFilter; + } + + const sqlOp = SQL_OPERATORS[operator]; + const unmigratedMatchingIds = await prisma.$queryRawUnsafe<{ contactId: string }[]>( ` SELECT DISTINCT ca."contactId" FROM "ContactAttribute" ca JOIN "ContactAttributeKey" cak ON ca."attributeKeyId" = cak.id WHERE cak.key = $1 - AND ( - (ca."valueNumber" IS NOT NULL AND ca."valueNumber" ${sqlOp} $2) - OR - (ca."valueNumber" IS NULL AND ca.value ~ $3 AND ca.value::double precision ${sqlOp} $2) - ) + AND cak."environmentId" = $4 + AND ca."valueNumber" IS NULL + AND ca.value ~ $3 + AND ca.value::double precision ${sqlOp} $2 `, contactAttributeKey, numericValue, - NUMBER_PATTERN_SQL + NUMBER_PATTERN_SQL, + environmentId ); - const contactIds = matchingContactIds.map((r) => r.contactId); - - if (contactIds.length === 0) { - // Return an impossible condition so the filter correctly excludes all contacts - return { id: "__NUMBER_FILTER_NO_MATCH__" }; + if (unmigratedMatchingIds.length === 0) { + return migratedFilter; } - return { id: { in: contactIds } }; + const contactIds = unmigratedMatchingIds.map((r) => r.contactId); + + return { + OR: [migratedFilter, { id: { in: contactIds } }], + }; }; /** * Builds a Prisma where clause from a segment attribute filter */ const buildAttributeFilterWhereClause = async ( - filter: TSegmentAttributeFilter + filter: TSegmentAttributeFilter, + environmentId: string ): Promise => { const { root, qualifier, value } = filter; const { contactAttributeKey } = root; @@ -215,7 +256,7 @@ const buildAttributeFilterWhereClause = async ( // Handle number operators if (["greaterThan", "greaterEqual", "lessThan", "lessEqual"].includes(operator)) { - return await buildNumberAttributeFilterWhereClause(filter); + return await buildNumberAttributeFilterWhereClause(filter, environmentId); } // For string operators, ensure value is a primitive (not an object or array) @@ -253,7 +294,8 @@ const buildAttributeFilterWhereClause = async ( * Builds a Prisma where clause from a person filter */ const buildPersonFilterWhereClause = async ( - filter: TSegmentPersonFilter + filter: TSegmentPersonFilter, + environmentId: string ): Promise => { const { personIdentifier } = filter.root; @@ -265,7 +307,7 @@ const buildPersonFilterWhereClause = async ( contactAttributeKey: personIdentifier, }, }; - return await buildAttributeFilterWhereClause(personFilter); + return await buildAttributeFilterWhereClause(personFilter, environmentId); } return {}; @@ -314,6 +356,7 @@ const buildDeviceFilterWhereClause = ( const buildSegmentFilterWhereClause = async ( filter: TSegmentSegmentFilter, segmentPath: Set, + environmentId: string, deviceType?: "phone" | "desktop" ): Promise => { const { root } = filter; @@ -337,7 +380,7 @@ const buildSegmentFilterWhereClause = async ( const newPath = new Set(segmentPath); newPath.add(segmentId); - return processFilters(segment.filters, newPath, deviceType); + return processFilters(segment.filters, newPath, environmentId, deviceType); }; /** @@ -346,19 +389,25 @@ const buildSegmentFilterWhereClause = async ( const processSingleFilter = async ( filter: TSegmentFilter, segmentPath: Set, + environmentId: string, deviceType?: "phone" | "desktop" ): Promise => { const { root } = filter; switch (root.type) { case "attribute": - return await buildAttributeFilterWhereClause(filter as TSegmentAttributeFilter); + return await buildAttributeFilterWhereClause(filter as TSegmentAttributeFilter, environmentId); case "person": - return await buildPersonFilterWhereClause(filter as TSegmentPersonFilter); + return await buildPersonFilterWhereClause(filter as TSegmentPersonFilter, environmentId); case "device": return buildDeviceFilterWhereClause(filter as TSegmentDeviceFilter, deviceType); case "segment": - return await buildSegmentFilterWhereClause(filter as TSegmentSegmentFilter, segmentPath, deviceType); + return await buildSegmentFilterWhereClause( + filter as TSegmentSegmentFilter, + segmentPath, + environmentId, + deviceType + ); default: return {}; } @@ -370,6 +419,7 @@ const processSingleFilter = async ( const processFilters = async ( filters: TBaseFilters, segmentPath: Set, + environmentId: string, deviceType?: "phone" | "desktop" ): Promise => { if (filters.length === 0) return {}; @@ -386,10 +436,10 @@ const processFilters = async ( // Process the resource based on its type if (isResourceFilter(resource)) { // If it's a single filter, process it directly - whereClause = await processSingleFilter(resource, segmentPath, deviceType); + whereClause = await processSingleFilter(resource, segmentPath, environmentId, deviceType); } else { // If it's a group of filters, process it recursively - whereClause = await processFilters(resource, segmentPath, deviceType); + whereClause = await processFilters(resource, segmentPath, environmentId, deviceType); } if (Object.keys(whereClause).length === 0) continue; @@ -432,7 +482,7 @@ export const segmentFilterToPrismaQuery = reactCache( // Initialize an empty stack for tracking the current evaluation path const segmentPath = new Set([segmentId]); - const filtersWhereClause = await processFilters(filters, segmentPath, deviceType); + const filtersWhereClause = await processFilters(filters, segmentPath, environmentId, deviceType); const whereClause = { AND: [baseWhereClause, filtersWhereClause],