From 4831cc8e34beba296553a38e99cf95bbb58e1dec Mon Sep 17 00:00:00 2001 From: pandeymangg Date: Wed, 31 Dec 2025 18:43:34 +0530 Subject: [PATCH] fixes duplicate userId issue with the contacts UI --- .../ee/contacts/lib/attributes.test.ts | 89 +++++++++++++++++-- .../web/modules/ee/contacts/lib/attributes.ts | 67 ++++++++++---- .../ee/contacts/lib/contact-attributes.ts | 30 ++++++- 3 files changed, 162 insertions(+), 24 deletions(-) diff --git a/apps/web/modules/ee/contacts/lib/attributes.test.ts b/apps/web/modules/ee/contacts/lib/attributes.test.ts index b932d9dbda..db9aaf3462 100644 --- a/apps/web/modules/ee/contacts/lib/attributes.test.ts +++ b/apps/web/modules/ee/contacts/lib/attributes.test.ts @@ -2,7 +2,11 @@ import { beforeEach, describe, expect, test, vi } from "vitest"; import { prisma } from "@formbricks/database"; import { TContactAttributeKey } from "@formbricks/types/contact-attribute-key"; import { getContactAttributeKeys } from "@/modules/ee/contacts/lib/contact-attribute-keys"; -import { getContactAttributes, hasEmailAttribute } from "@/modules/ee/contacts/lib/contact-attributes"; +import { + getContactAttributes, + hasEmailAttribute, + hasUserIdAttribute, +} from "@/modules/ee/contacts/lib/contact-attributes"; import { updateAttributes } from "./attributes"; vi.mock("@/lib/constants", () => ({ @@ -20,6 +24,7 @@ vi.mock("@/modules/ee/contacts/lib/contact-attributes", async () => { ...actual, getContactAttributes: vi.fn(), hasEmailAttribute: vi.fn(), + hasUserIdAttribute: vi.fn(), }; }); vi.mock("@formbricks/database", () => ({ @@ -75,6 +80,7 @@ describe("updateAttributes", () => { vi.clearAllMocks(); // Set default mock return values - these will be overridden in individual tests vi.mocked(getContactAttributes).mockResolvedValue({}); + vi.mocked(hasUserIdAttribute).mockResolvedValue(false); vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); vi.mocked(prisma.$transaction).mockResolvedValue(undefined); }); @@ -83,19 +89,21 @@ describe("updateAttributes", () => { vi.mocked(getContactAttributeKeys).mockResolvedValue(attributeKeys); vi.mocked(getContactAttributes).mockResolvedValue({ name: "Jane", email: "jane@example.com" }); vi.mocked(hasEmailAttribute).mockResolvedValue(false); + vi.mocked(hasUserIdAttribute).mockResolvedValue(false); vi.mocked(prisma.$transaction).mockResolvedValue(undefined); vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); const attributes = { name: "John", email: "john@example.com" }; const result = await updateAttributes(contactId, userId, environmentId, attributes); expect(prisma.$transaction).toHaveBeenCalled(); expect(result.success).toBe(true); - expect(result.messages).toEqual([]); + expect(result.messages).toBeUndefined(); }); test("skips updating email if it already exists", async () => { vi.mocked(getContactAttributeKeys).mockResolvedValue(attributeKeys); vi.mocked(getContactAttributes).mockResolvedValue({ name: "Jane", email: "jane@example.com" }); vi.mocked(hasEmailAttribute).mockResolvedValue(true); + vi.mocked(hasUserIdAttribute).mockResolvedValue(false); vi.mocked(prisma.$transaction).mockResolvedValue(undefined); vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); const attributes = { name: "John", email: "john@example.com" }; @@ -106,10 +114,77 @@ describe("updateAttributes", () => { expect(result.messages).toContain("The email already exists for this environment and was not updated."); }); + test("skips updating userId if it already exists", async () => { + const attributeKeysWithUserId: TContactAttributeKey[] = [ + ...attributeKeys, + { + id: "key-4", + key: "userId", + createdAt: new Date(), + updatedAt: new Date(), + isUnique: true, + name: "User ID", + description: null, + type: "default", + environmentId, + }, + ]; + vi.mocked(getContactAttributeKeys).mockResolvedValue(attributeKeysWithUserId); + vi.mocked(getContactAttributes).mockResolvedValue({ name: "Jane", userId: "old-user-id" }); + vi.mocked(hasEmailAttribute).mockResolvedValue(false); + vi.mocked(hasUserIdAttribute).mockResolvedValue(true); + vi.mocked(prisma.$transaction).mockResolvedValue(undefined); + vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); + const attributes = { name: "John", userId: "duplicate-user-id" }; + const result = await updateAttributes(contactId, userId, environmentId, attributes); + expect(prisma.$transaction).toHaveBeenCalled(); + + expect(result.success).toBe(true); + expect(result.messages).toContain("The userId already exists for this environment and was not updated."); + expect(result.ignoreUserIdAttribute).toBe(true); + }); + + test("skips updating both email and userId if both already exist", async () => { + const attributeKeysWithUserId: TContactAttributeKey[] = [ + ...attributeKeys, + { + id: "key-4", + key: "userId", + createdAt: new Date(), + updatedAt: new Date(), + isUnique: true, + name: "User ID", + description: null, + type: "default", + environmentId, + }, + ]; + vi.mocked(getContactAttributeKeys).mockResolvedValue(attributeKeysWithUserId); + vi.mocked(getContactAttributes).mockResolvedValue({ + name: "Jane", + email: "old@example.com", + userId: "old-user-id", + }); + vi.mocked(hasEmailAttribute).mockResolvedValue(true); + vi.mocked(hasUserIdAttribute).mockResolvedValue(true); + vi.mocked(prisma.$transaction).mockResolvedValue(undefined); + vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); + const attributes = { name: "John", email: "duplicate@example.com", userId: "duplicate-user-id" }; + const result = await updateAttributes(contactId, userId, environmentId, attributes); + expect(prisma.$transaction).toHaveBeenCalled(); + + expect(result.success).toBe(true); + expect(result.messages).toContain("The email already exists for this environment and was not updated."); + expect(result.messages).toContain("The userId already exists for this environment and was not updated."); + expect(result.ignoreEmailAttribute).toBe(true); + expect(result.ignoreUserIdAttribute).toBe(true); + }); + test("creates new attributes if under limit", async () => { vi.mocked(getContactAttributeKeys).mockResolvedValue([attributeKeys[0]]); vi.mocked(getContactAttributes).mockResolvedValue({ name: "Jane" }); vi.mocked(hasEmailAttribute).mockResolvedValue(false); + vi.mocked(hasUserIdAttribute).mockResolvedValue(false); vi.mocked(prisma.$transaction).mockResolvedValue(undefined); vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); const attributes = { name: "John", newAttr: "val" }; @@ -117,13 +192,14 @@ describe("updateAttributes", () => { expect(prisma.$transaction).toHaveBeenCalled(); expect(result.success).toBe(true); - expect(result.messages).toEqual([]); + expect(result.messages).toBeUndefined(); }); test("does not create new attributes if over the limit", async () => { vi.mocked(getContactAttributeKeys).mockResolvedValue(attributeKeys); vi.mocked(getContactAttributes).mockResolvedValue({ name: "Jane", email: "jane@example.com" }); vi.mocked(hasEmailAttribute).mockResolvedValue(false); + vi.mocked(hasUserIdAttribute).mockResolvedValue(false); vi.mocked(prisma.$transaction).mockResolvedValue(undefined); vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); const attributes = { name: "John", newAttr: "val" }; @@ -136,12 +212,13 @@ describe("updateAttributes", () => { vi.mocked(getContactAttributeKeys).mockResolvedValue([]); vi.mocked(getContactAttributes).mockResolvedValue({}); vi.mocked(hasEmailAttribute).mockResolvedValue(false); + vi.mocked(hasUserIdAttribute).mockResolvedValue(false); vi.mocked(prisma.$transaction).mockResolvedValue(undefined); vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); const attributes = {}; const result = await updateAttributes(contactId, userId, environmentId, attributes); expect(result.success).toBe(true); - expect(result.messages).toEqual([]); + expect(result.messages).toBeUndefined(); }); test("deletes non-default attributes that are removed from payload", async () => { @@ -155,6 +232,7 @@ describe("updateAttributes", () => { customAttr: "oldValue", }); vi.mocked(hasEmailAttribute).mockResolvedValue(false); + vi.mocked(hasUserIdAttribute).mockResolvedValue(false); vi.mocked(prisma.$transaction).mockResolvedValue(undefined); vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 1 }); const attributes = { name: "John", email: "john@example.com" }; @@ -169,7 +247,7 @@ describe("updateAttributes", () => { }, }); expect(result.success).toBe(true); - expect(result.messages).toEqual([]); + expect(result.messages).toBeUndefined(); }); test("does not delete default attributes even if removed from payload", async () => { @@ -231,6 +309,7 @@ describe("updateAttributes", () => { firstName: "John", }); vi.mocked(hasEmailAttribute).mockResolvedValue(false); + vi.mocked(hasUserIdAttribute).mockResolvedValue(false); vi.mocked(prisma.$transaction).mockResolvedValue(undefined); vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 }); const attributes = { customAttr: "value" }; diff --git a/apps/web/modules/ee/contacts/lib/attributes.ts b/apps/web/modules/ee/contacts/lib/attributes.ts index a16de0a3c0..3718a0c1b0 100644 --- a/apps/web/modules/ee/contacts/lib/attributes.ts +++ b/apps/web/modules/ee/contacts/lib/attributes.ts @@ -5,7 +5,11 @@ import { TContactAttributeKey } from "@formbricks/types/contact-attribute-key"; import { MAX_ATTRIBUTE_CLASSES_PER_ENVIRONMENT } from "@/lib/constants"; import { validateInputs } from "@/lib/utils/validate"; import { getContactAttributeKeys } from "@/modules/ee/contacts/lib/contact-attribute-keys"; -import { getContactAttributes, hasEmailAttribute } from "@/modules/ee/contacts/lib/contact-attributes"; +import { + getContactAttributes, + hasEmailAttribute, + hasUserIdAttribute, +} from "@/modules/ee/contacts/lib/contact-attributes"; // Default/system attributes that should not be deleted even if missing from payload const DEFAULT_ATTRIBUTES = new Set(["email", "userId", "firstName", "lastName"]); @@ -52,7 +56,12 @@ export const updateAttributes = async ( userId: string, environmentId: string, contactAttributesParam: TContactAttributes -): Promise<{ success: boolean; messages?: string[]; ignoreEmailAttribute?: boolean }> => { +): Promise<{ + success: boolean; + messages?: string[]; + ignoreEmailAttribute?: boolean; + ignoreUserIdAttribute?: boolean; +}> => { validateInputs( [contactId, ZId], [userId, ZString], @@ -61,20 +70,39 @@ export const updateAttributes = async ( ); let ignoreEmailAttribute = false; + let ignoreUserIdAttribute = false; - // Fetch current attributes, contact attribute keys, and email check in parallel - const [currentAttributes, contactAttributeKeys, existingEmailAttribute] = await Promise.all([ - getContactAttributes(contactId), - getContactAttributeKeys(environmentId), - contactAttributesParam.email - ? hasEmailAttribute(contactAttributesParam.email, environmentId, contactId) - : Promise.resolve(null), - ]); + // Fetch current attributes, contact attribute keys, and email/userId checks in parallel + const [currentAttributes, contactAttributeKeys, existingEmailAttribute, existingUserIdAttribute] = + await Promise.all([ + getContactAttributes(contactId), + getContactAttributeKeys(environmentId), + contactAttributesParam.email + ? hasEmailAttribute(contactAttributesParam.email, environmentId, contactId) + : Promise.resolve(null), + contactAttributesParam.userId + ? hasUserIdAttribute(contactAttributesParam.userId, environmentId, contactId) + : Promise.resolve(null), + ]); - // Process email existence early - const { email, ...remainingAttributes } = contactAttributesParam; - const contactAttributes = existingEmailAttribute ? remainingAttributes : contactAttributesParam; + // Process email and userId existence early const emailExists = !!existingEmailAttribute; + const userIdExists = !!existingUserIdAttribute; + + // Remove email and/or userId from attributes if they already exist on another contact + let contactAttributes = { ...contactAttributesParam }; + + if (emailExists) { + const { email: _email, ...rest } = contactAttributes; + contactAttributes = rest; + ignoreEmailAttribute = true; + } + + if (userIdExists) { + const { userId: _userId, ...rest } = contactAttributes; + contactAttributes = rest; + ignoreUserIdAttribute = true; + } // Delete attributes that were removed (using the deleteAttributes service) await deleteAttributes(contactId, currentAttributes, contactAttributesParam, contactAttributeKeys); @@ -99,12 +127,14 @@ export const updateAttributes = async ( } ); - let messages: string[] = emailExists - ? ["The email already exists for this environment and was not updated."] - : []; + const messages: string[] = []; if (emailExists) { - ignoreEmailAttribute = true; + messages.push("The email already exists for this environment and was not updated."); + } + + if (userIdExists) { + messages.push("The userId already exists for this environment and was not updated."); } // Update all existing attributes @@ -159,7 +189,8 @@ export const updateAttributes = async ( return { success: true, - messages, + messages: messages.length > 0 ? messages : undefined, ignoreEmailAttribute, + ignoreUserIdAttribute, }; }; diff --git a/apps/web/modules/ee/contacts/lib/contact-attributes.ts b/apps/web/modules/ee/contacts/lib/contact-attributes.ts index bf9d0bad08..fbd19658c5 100644 --- a/apps/web/modules/ee/contacts/lib/contact-attributes.ts +++ b/apps/web/modules/ee/contacts/lib/contact-attributes.ts @@ -1,7 +1,7 @@ import { Prisma } from "@prisma/client"; import { cache as reactCache } from "react"; import { prisma } from "@formbricks/database"; -import { ZId } from "@formbricks/types/common"; +import { ZId, ZString } from "@formbricks/types/common"; import { TContactAttributes } from "@formbricks/types/contact-attribute"; import { DatabaseError } from "@formbricks/types/errors"; import { ZUserEmail } from "@formbricks/types/user"; @@ -68,3 +68,31 @@ export const hasEmailAttribute = reactCache( return !!contactAttribute; } ); + +export const hasUserIdAttribute = reactCache( + async (userId: string, environmentId: string, contactId: string): Promise => { + validateInputs([userId, ZString], [environmentId, ZId], [contactId, ZId]); + + const contactAttribute = await prisma.contactAttribute.findFirst({ + where: { + AND: [ + { + attributeKey: { + key: "userId", + environmentId, + }, + value: userId, + }, + { + NOT: { + contactId, + }, + }, + ], + }, + select: { id: true }, + }); + + return !!contactAttribute; + } +);