fix: fixes duplicate userId issue with the contacts UI (#7051)

Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>

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,45 +114,147 @@ describe("updateAttributes", () => {
     expect(result.messages).toContain("The email already exists for this environment and was not updated.");
   });
 
-  test("creates new attributes if under limit", async () => {
-    vi.mocked(getContactAttributeKeys).mockResolvedValue([attributeKeys[0]]);
-    vi.mocked(getContactAttributes).mockResolvedValue({ name: "Jane" });
+  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", newAttr: "val" };
+    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).toEqual([]);
+    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 () => {
+    // Use name and email keys (2 existing keys), MAX is mocked to 2
+    // We update existing attributes, no new ones created
+    vi.mocked(getContactAttributeKeys).mockResolvedValue([attributeKeys[0], attributeKeys[1]]); // name, email
+    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).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" };
+    // Include email to satisfy the "at least one of email or userId" requirement
+    const attributes = { name: "John", email: "john@example.com", newAttr: "val" };
     const result = await updateAttributes(contactId, userId, environmentId, attributes);
     expect(result.success).toBe(true);
     expect(result.messages?.[0]).toMatch(/Could not create 1 new attribute/);
   });
 
-  test("returns success with no attributes to update or create", async () => {
-    vi.mocked(getContactAttributeKeys).mockResolvedValue([]);
-    vi.mocked(getContactAttributes).mockResolvedValue({});
+  test("returns success with only email attribute", async () => {
+    vi.mocked(getContactAttributeKeys).mockResolvedValue([attributeKeys[1]]); // email key
+    vi.mocked(getContactAttributes).mockResolvedValue({ email: "existing@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 = {};
+    const attributes = { email: "updated@example.com" };
     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 () => {
+  test("deletes non-default attributes when deleteRemovedAttributes is true", async () => {
+    // Reset mocks explicitly for this test
+    vi.mocked(prisma.contactAttribute.deleteMany).mockClear();
+
+    vi.mocked(getContactAttributeKeys).mockResolvedValue(attributeKeys);
+    vi.mocked(getContactAttributes).mockResolvedValue({
+      name: "Jane",
+      email: "jane@example.com",
+      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" };
+    // Pass deleteRemovedAttributes: true to enable deletion behavior
+    const result = await updateAttributes(contactId, userId, environmentId, attributes, true);
+    // Only customAttr (key-3) should be deleted, not name (key-1) or email (key-2)
+    expect(prisma.contactAttribute.deleteMany).toHaveBeenCalledWith({
+      where: {
+        contactId,
+        attributeKeyId: {
+          in: ["key-3"],
+        },
+      },
+    });
+    expect(result.success).toBe(true);
+    expect(result.messages).toBeUndefined();
+  });
+
+  test("does not delete attributes when deleteRemovedAttributes is false (default behavior)", async () => {
     // Reset mocks explicitly for this test
     vi.mocked(prisma.contactAttribute.deleteMany).mockClear();
 
@@ -156,27 +266,19 @@ describe("updateAttributes", () => {
     });
     vi.mocked(hasEmailAttribute).mockResolvedValue(false);
     vi.mocked(prisma.$transaction).mockResolvedValue(undefined);
-    vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 1 });
     const attributes = { name: "John", email: "john@example.com" };
+    // Default behavior (deleteRemovedAttributes: false) should NOT delete existing attributes
     const result = await updateAttributes(contactId, userId, environmentId, attributes);
-    // Only customAttr (key-3) should be deleted, not name (key-1) or email (key-2)
-    expect(prisma.contactAttribute.deleteMany).toHaveBeenCalledWith({
-      where: {
-        contactId,
-        attributeKeyId: {
-          in: ["key-3"],
-        },
-      },
-    });
+    // deleteMany should NOT be called since we're merging, not replacing
+    expect(prisma.contactAttribute.deleteMany).not.toHaveBeenCalled();
     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 () => {
+  test("does not delete default attributes even when deleteRemovedAttributes is true", async () => {
     // Reset mocks explicitly for this test
     vi.mocked(prisma.contactAttribute.deleteMany).mockClear();
 
-    // Need to include userId and firstName in attributeKeys for this test
     // Note: DEFAULT_ATTRIBUTES includes: email, userId, firstName, lastName (not "name")
     const attributeKeysWithDefaults: TContactAttributeKey[] = [
       {
@@ -231,13 +333,105 @@ 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" };
-    const result = await updateAttributes(contactId, userId, environmentId, attributes);
+    // Pass deleteRemovedAttributes: true to test that default attributes are still preserved
+    const result = await updateAttributes(contactId, userId, environmentId, attributes, true);
     // Should not delete default attributes (email, userId, firstName) - deleteMany should not be called
     // since all current attributes are default attributes
     expect(prisma.contactAttribute.deleteMany).not.toHaveBeenCalled();
     expect(result.success).toBe(true);
   });
+
+  test("preserves existing email when empty string is submitted", async () => {
+    vi.mocked(getContactAttributeKeys).mockResolvedValue(attributeKeys);
+    vi.mocked(getContactAttributes).mockResolvedValue({ name: "Jane", email: "existing@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 });
+
+    // Attempt to clear email by submitting empty string
+    const attributes = { name: "John", email: "" };
+    const result = await updateAttributes(contactId, userId, environmentId, attributes);
+
+    // Verify that the transaction was called with the preserved email
+    expect(prisma.$transaction).toHaveBeenCalled();
+    const transactionCall = vi.mocked(prisma.$transaction).mock.calls[0][0];
+    // The email should be preserved (existing@example.com), not cleared
+    expect(transactionCall).toHaveLength(2); // name and email
+    expect(result.success).toBe(true);
+  });
+
+  test("allows clearing userId when empty string is submitted", 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: "existing-user-id" });
+    vi.mocked(hasEmailAttribute).mockResolvedValue(false);
+    vi.mocked(hasUserIdAttribute).mockResolvedValue(false);
+    vi.mocked(prisma.$transaction).mockResolvedValue(undefined);
+    vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 });
+
+    // Clear userId by submitting empty string - this should be allowed
+    const attributes = { name: "John", userId: "" };
+    const result = await updateAttributes(contactId, userId, environmentId, attributes);
+
+    // Verify that the transaction was called
+    expect(prisma.$transaction).toHaveBeenCalled();
+    const transactionCall = vi.mocked(prisma.$transaction).mock.calls[0][0];
+    // Only name and userId (empty) should be in the transaction
+    expect(transactionCall).toHaveLength(2); // name and userId (with empty value)
+    expect(result.success).toBe(true);
+  });
+
+  test("preserves existing values when both email and userId would be cleared", async () => {
+    const attributeKeysWithBoth: 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(attributeKeysWithBoth);
+    vi.mocked(getContactAttributes).mockResolvedValue({
+      name: "Jane",
+      email: "existing@example.com",
+      userId: "existing-user-id",
+    });
+    vi.mocked(hasEmailAttribute).mockResolvedValue(false);
+    vi.mocked(hasUserIdAttribute).mockResolvedValue(false);
+    vi.mocked(prisma.$transaction).mockResolvedValue(undefined);
+    vi.mocked(prisma.contactAttribute.deleteMany).mockResolvedValue({ count: 0 });
+
+    // Attempt to clear both email and userId
+    const attributes = { name: "John", email: "", userId: "" };
+    const result = await updateAttributes(contactId, userId, environmentId, attributes);
+
+    expect(result.success).toBe(true);
+    expect(result.messages).toContain(
+      "Either email or userId is required. The existing values were preserved."
+    );
+  });
 });

apps/web/modules/ee/contacts/lib/contact-attributes.test.ts

@@ -1,6 +1,7 @@
 import { beforeEach, describe, expect, test, vi } from "vitest";
 import { prisma } from "@formbricks/database";
-import { getContactAttributes, hasEmailAttribute } from "./contact-attributes";
+import { TContactAttribute } from "@formbricks/types/contact-attribute";
+import { getContactAttributes, hasEmailAttribute, hasUserIdAttribute } from "./contact-attributes";
 
 vi.mock("@formbricks/database", () => ({
   prisma: {
@@ -16,11 +17,12 @@ vi.mock("@/lib/utils/validate", () => ({ validateInputs: vi.fn() }));
 const contactId = "contact-1";
 const environmentId = "env-1";
 const email = "john@example.com";
+const userId = "user-123";
 
 const mockAttributes = [
   { value: "john@example.com", attributeKey: { key: "email", name: "Email" } },
   { value: "John", attributeKey: { key: "name", name: "Name" } },
-];
+] as unknown as TContactAttribute[];
 
 describe("getContactAttributes", () => {
   beforeEach(() => {
@@ -50,7 +52,9 @@ describe("hasEmailAttribute", () => {
   });
 
   test("returns true if email attribute exists", async () => {
-    vi.mocked(prisma.contactAttribute.findFirst).mockResolvedValue({ id: "attr-1" });
+    vi.mocked(prisma.contactAttribute.findFirst).mockResolvedValue({
+      id: "attr-1",
+    } as unknown as TContactAttribute);
     const result = await hasEmailAttribute(email, environmentId, contactId);
     expect(prisma.contactAttribute.findFirst).toHaveBeenCalledWith({
       where: {
@@ -67,3 +71,29 @@ describe("hasEmailAttribute", () => {
     expect(result).toBe(false);
   });
 });
+
+describe("hasUserIdAttribute", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  test("returns true if userId attribute exists on another contact", async () => {
+    vi.mocked(prisma.contactAttribute.findFirst).mockResolvedValue({
+      id: "attr-1",
+    } as unknown as TContactAttribute);
+    const result = await hasUserIdAttribute(userId, environmentId, contactId);
+    expect(prisma.contactAttribute.findFirst).toHaveBeenCalledWith({
+      where: {
+        AND: [{ attributeKey: { key: "userId", environmentId }, value: userId }, { NOT: { contactId } }],
+      },
+      select: { id: true },
+    });
+    expect(result).toBe(true);
+  });
+
+  test("returns false if userId attribute does not exist on another contact", async () => {
+    vi.mocked(prisma.contactAttribute.findFirst).mockResolvedValue(null);
+    const result = await hasUserIdAttribute(userId, environmentId, contactId);
+    expect(result).toBe(false);
+  });
+});

apps/web/modules/ee/contacts/lib/update-contact-attributes.test.ts

@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, vi } from "vitest";
+import { beforeEach, describe, expect, test, vi } from "vitest";
 import { updateAttributes } from "./attributes";
 import { getContactAttributeKeys } from "./contact-attribute-keys";
 import { getContactAttributes } from "./contact-attributes";
@@ -16,7 +16,7 @@ describe("updateContactAttributes", () => {
     vi.clearAllMocks();
   });
 
-  it("should update contact attributes successfully", async () => {
+  test("should update contact attributes with deleteRemovedAttributes: true", async () => {
     const contactId = "contact123";
     const environmentId = "env123";
     const userId = "user123";
@@ -91,13 +91,14 @@ describe("updateContactAttributes", () => {
 
     expect(getContact).toHaveBeenCalledWith(contactId);
     expect(getContactAttributeKeys).toHaveBeenCalledWith(environmentId);
-    expect(updateAttributes).toHaveBeenCalledWith(contactId, userId, environmentId, attributes);
+    // Should call updateAttributes with deleteRemovedAttributes: true for UI form updates
+    expect(updateAttributes).toHaveBeenCalledWith(contactId, userId, environmentId, attributes, true);
     expect(getContactAttributes).toHaveBeenCalledWith(contactId);
     expect(result.updatedAttributes).toEqual(mockUpdatedAttributes);
     expect(result.updatedAttributeKeys).toBeUndefined();
   });
 
-  it("should detect new attribute keys when created", async () => {
+  test("should detect new attribute keys when created", async () => {
     const contactId = "contact123";
     const environmentId = "env123";
     const userId = "user123";
@@ -184,7 +185,7 @@ describe("updateContactAttributes", () => {
     ]);
   });
 
-  it("should handle missing userId with warning message", async () => {
+  test("should handle missing userId gracefully", async () => {
     const contactId = "contact123";
     const environmentId = "env123";
     const attributes = {
@@ -226,13 +227,13 @@ describe("updateContactAttributes", () => {
 
     const result = await updateContactAttributes(contactId, attributes);
 
-    expect(updateAttributes).toHaveBeenCalledWith(contactId, "", environmentId, attributes);
-    expect(result.messages).toContain(
-      "Warning: userId attribute is missing. Some operations may not work correctly."
-    );
+    // When userId is not in attributes, pass empty string to updateAttributes
+    expect(updateAttributes).toHaveBeenCalledWith(contactId, "", environmentId, attributes, true);
+    // No warning message - the backend now gracefully handles missing userId by keeping current value
+    expect(result.messages).toBeUndefined();
   });
 
-  it("should merge messages from updateAttributes", async () => {
+  test("should merge messages from updateAttributes", async () => {
     const contactId = "contact123";
     const environmentId = "env123";
     const userId = "user123";
@@ -279,7 +280,7 @@ describe("updateContactAttributes", () => {
     expect(result.messages).toContain("The email already exists for this environment and was not updated.");
   });
 
-  it("should throw error if contact not found", async () => {
+  test("should throw error if contact not found", async () => {
     const contactId = "contact123";
     const attributes = {
       firstName: "John",