addressed feedback

apps/web/lib/utils/safe-identifier.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "vitest";
-import { isSafeIdentifier, toSafeIdentifier } from "./safe-identifier";
+import { isSafeIdentifier } from "./safe-identifier";
 
 describe("safe-identifier", () => {
   describe("isSafeIdentifier", () => {
@@ -32,59 +32,4 @@ describe("safe-identifier", () => {
       expect(isSafeIdentifier("")).toBe(false);
     });
   });
-
-  describe("toSafeIdentifier", () => {
-    test("converts valid strings to safe identifiers", () => {
-      expect(toSafeIdentifier("email")).toBe("email");
-      expect(toSafeIdentifier("user_name")).toBe("user_name");
-    });
-
-    test("converts spaces to underscores", () => {
-      expect(toSafeIdentifier("email address")).toBe("email_address");
-      expect(toSafeIdentifier("user name")).toBe("user_name");
-    });
-
-    test("converts special characters to underscores", () => {
-      expect(toSafeIdentifier("user:name")).toBe("user_name");
-      expect(toSafeIdentifier("user-name")).toBe("user_name");
-      expect(toSafeIdentifier("user(name)")).toBe("user_name");
-    });
-
-    test("handles strings starting with numbers", () => {
-      expect(toSafeIdentifier("123attr")).toBe("attr_123attr");
-      expect(toSafeIdentifier("01region")).toBe("attr_01region");
-    });
-
-    test("removes accents and normalizes", () => {
-      expect(toSafeIdentifier("café")).toBe("cafe");
-      expect(toSafeIdentifier("naïve")).toBe("naive");
-      expect(toSafeIdentifier("résumé")).toBe("resume");
-    });
-
-    test("collapses multiple underscores", () => {
-      expect(toSafeIdentifier("user__name")).toBe("user_name");
-      expect(toSafeIdentifier("email___address")).toBe("email_address");
-    });
-
-    test("removes leading and trailing underscores", () => {
-      expect(toSafeIdentifier("_email_")).toBe("email");
-      expect(toSafeIdentifier("__user__")).toBe("user");
-    });
-
-    test("handles empty string", () => {
-      expect(toSafeIdentifier("")).toBe("");
-    });
-
-    test("handles strings that become empty after sanitization", () => {
-      expect(toSafeIdentifier("!!!")).toBe("attr_key");
-      expect(toSafeIdentifier("---")).toBe("attr_key");
-    });
-
-    test("converts to lowercase", () => {
-      expect(toSafeIdentifier("Email")).toBe("email");
-      expect(toSafeIdentifier("USER_NAME")).toBe("user_name");
-      expect(toSafeIdentifier("TestKey123")).toBe("testkey123");
-    });
-  });
 });
-

apps/web/lib/utils/safe-identifier.ts

@@ -13,36 +13,3 @@ export const isSafeIdentifier = (value: string): boolean => {
   // Can only contain lowercase letters, numbers, and underscores
   return /^[a-z0-9_]+$/.test(value);
 };
-
-/**
- * Converts a string to a safe identifier by:
- * - Converting to lowercase
- * - Replacing invalid characters with underscores
- * - Removing leading/trailing underscores
- * - Ensuring it starts with a letter (prepending 'attr_' if it starts with a number)
- */
-export const toSafeIdentifier = (value: string): string => {
-  if (!value) return "";
-
-  // Convert to lowercase and replace invalid characters with underscores
-  let safe = value
-    .toLowerCase()
-    .normalize("NFD")
-    .replaceAll(/[\u0300-\u036f]/g, "") // Remove accents
-    .replaceAll(/[^a-z\d_]/g, "_") // Replace invalid chars with underscore
-    .replaceAll(/_+/g, "_") // Collapse multiple underscores
-    .replace(/^_+/, "") // Remove leading underscores
-    .replace(/_+$/, ""); // Remove trailing underscores
-
-  // If it starts with a number, prepend 'attr_'
-  if (/^\d/.test(safe)) {
-    safe = `attr_${safe}`;
-  }
-
-  // If empty after sanitization, return a default
-  if (!safe) {
-    safe = "attr_key";
-  }
-
-  return safe;
-};

apps/web/modules/ee/contacts/attributes/components/attributes-table.tsx

@@ -71,6 +71,11 @@ export const AttributesTable = ({
     );
   }, [contactAttributeKeys, searchValue]);
 
+  // Check if all filtered attributes are system attributes
+  const allSystemAttributes = useMemo(() => {
+    return filteredAttributes.length > 0 && filteredAttributes.every((attr) => attr.type === "default");
+  }, [filteredAttributes]);
+
   // Generate columns
   const columns = useMemo(() => {
     return generateAttributeTableColumns(searchValue, isReadOnly, isExpanded ?? false, t, locale);
@@ -136,6 +141,21 @@ export const AttributesTable = ({
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [environmentId]);
 
+  // Hide select column when all attributes are system attributes
+  useEffect(() => {
+    if (!isReadOnly && allSystemAttributes) {
+      setColumnVisibility((prev) => ({
+        ...prev,
+        select: false,
+      }));
+    } else if (!isReadOnly && !allSystemAttributes) {
+      setColumnVisibility((prev) => ({
+        ...prev,
+        select: true,
+      }));
+    }
+  }, [allSystemAttributes, isReadOnly]);
+
   // Save settings to localStorage when they change
   useEffect(() => {
     if (columnOrder.length > 0) {
@@ -179,7 +199,7 @@ export const AttributesTable = ({
       columnVisibility,
       rowSelection,
       columnPinning: {
-        left: ["select", "createdAt"],
+        left: allSystemAttributes ? ["createdAt"] : ["select", "createdAt"],
       },
     },
   });

apps/web/modules/ee/contacts/attributes/components/create-attribute-modal.tsx

@@ -6,7 +6,7 @@ import { useState } from "react";
 import toast from "react-hot-toast";
 import { useTranslation } from "react-i18next";
 import { getFormattedErrorMessage } from "@/lib/utils/helper";
-import { isSafeIdentifier, toSafeIdentifier } from "@/lib/utils/safe-identifier";
+import { isSafeIdentifier } from "@/lib/utils/safe-identifier";
 import { Button } from "@/modules/ui/components/button";
 import {
   Dialog,
@@ -47,17 +47,9 @@ export function CreateAttributeModal({ environmentId }: Readonly<CreateAttribute
   };
 
   const handleNameChange = (value: string) => {
-    setFormData((prev) => {
-      const newName = value;
-      // Auto-suggest key from name if key is empty or matches previous name suggestion
-      let newKey = prev.key;
-      if (!prev.key || prev.key === toSafeIdentifier(prev.name)) {
-        newKey = toSafeIdentifier(newName);
-      }
-      return { ...prev, name: newName, key: newKey };
-    });
-    if (keyError) {
-      validateKey(formData.key || toSafeIdentifier(value));
+    setFormData((prev) => ({ ...prev, name: value }));
+    if (keyError && formData.key) {
+      validateKey(formData.key);
     }
   };
 
@@ -113,6 +105,11 @@ export function CreateAttributeModal({ environmentId }: Readonly<CreateAttribute
     setIsCreating(false);
   };
 
+  const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
+    e.preventDefault();
+    await handleCreate();
+  };
+
   return (
     <>
       <Button onClick={() => setOpen(true)} size="sm">
@@ -127,7 +124,7 @@ export function CreateAttributeModal({ environmentId }: Readonly<CreateAttribute
             handleResetState();
           }
         }}>
-        <DialogContent className="sm:max-w-lg" disableCloseOnOutsideClick>
+        <DialogContent className="sm:max-w-lg">
           <DialogHeader>
             <DialogTitle>{t("environments.contacts.create_new_attribute")}</DialogTitle>
             <DialogDescription>
@@ -135,63 +132,64 @@ export function CreateAttributeModal({ environmentId }: Readonly<CreateAttribute
             </DialogDescription>
           </DialogHeader>
 
-          <DialogBody>
-            <div className="flex flex-col gap-4">
-              <div className="flex flex-col gap-2">
-                <label className="text-sm font-medium text-slate-900">
-                  {t("environments.contacts.attribute_key")}
-                </label>
-                <Input
-                  value={formData.key}
-                  onChange={(e) => handleKeyChange(e.target.value)}
-                  placeholder={t("environments.contacts.attribute_key_placeholder")}
-                  className={keyError ? "border-red-500" : ""}
-                />
-                {keyError && <p className="text-sm text-red-500">{keyError}</p>}
-                <p className="text-xs text-slate-500">{t("environments.contacts.attribute_key_hint")}</p>
-              </div>
+          <form onSubmit={handleSubmit}>
+            <DialogBody>
+              <div className="flex flex-col gap-4">
+                <div className="flex flex-col gap-2">
+                  <label className="text-sm font-medium text-slate-900">
+                    {t("environments.contacts.attribute_key")}
+                  </label>
+                  <Input
+                    value={formData.key}
+                    onChange={(e) => handleKeyChange(e.target.value)}
+                    placeholder={t("environments.contacts.attribute_key_placeholder")}
+                    className={keyError ? "border-red-500" : ""}
+                  />
+                  {keyError && <p className="text-sm text-red-500">{keyError}</p>}
+                  <p className="text-xs text-slate-500">{t("environments.contacts.attribute_key_hint")}</p>
+                </div>
 
-              <div className="flex flex-col gap-2">
-                <label className="text-sm font-medium text-slate-900">
-                  {t("environments.contacts.attribute_label")}
-                </label>
-                <Input
-                  value={formData.name}
-                  onChange={(e) => handleNameChange(e.target.value)}
-                  placeholder={t("environments.contacts.attribute_label_placeholder")}
-                />
-              </div>
+                <div className="flex flex-col gap-2">
+                  <label className="text-sm font-medium text-slate-900">
+                    {t("environments.contacts.attribute_label")}
+                  </label>
+                  <Input
+                    value={formData.name}
+                    onChange={(e) => handleNameChange(e.target.value)}
+                    placeholder={t("environments.contacts.attribute_label_placeholder")}
+                  />
+                </div>
 
-              <div className="flex flex-col gap-2">
-                <label className="text-sm font-medium text-slate-900">
-                  {t("environments.contacts.attribute_description")} ({t("common.optional")})
-                </label>
-                <Input
-                  value={formData.description}
-                  onChange={(e) => setFormData((prev) => ({ ...prev, description: e.target.value }))}
-                  placeholder={t("environments.contacts.attribute_description_placeholder")}
-                />
+                <div className="flex flex-col gap-2">
+                  <label className="text-sm font-medium text-slate-900">
+                    {t("environments.contacts.attribute_description")} ({t("common.optional")})
+                  </label>
+                  <Input
+                    value={formData.description}
+                    onChange={(e) => setFormData((prev) => ({ ...prev, description: e.target.value }))}
+                    placeholder={t("environments.contacts.attribute_description_placeholder")}
+                  />
+                </div>
               </div>
-            </div>
-          </DialogBody>
+            </DialogBody>
 
-          <DialogFooter>
-            <Button
-              onClick={() => {
-                handleResetState();
-              }}
-              type="button"
-              variant="secondary">
-              {t("common.cancel")}
-            </Button>
-            <Button
-              disabled={!formData.key || !!keyError}
-              loading={isCreating}
-              onClick={handleCreate}
-              type="submit">
-              {t("environments.contacts.create_key")}
-            </Button>
-          </DialogFooter>
+            <DialogFooter className="mt-4">
+              <Button
+                onClick={() => {
+                  handleResetState();
+                }}
+                type="button"
+                variant="secondary">
+                {t("common.cancel")}
+              </Button>
+              <Button
+                disabled={!formData.key || !formData.name || !!keyError}
+                loading={isCreating}
+                type="submit">
+                {t("environments.contacts.create_key")}
+              </Button>
+            </DialogFooter>
+          </form>
         </DialogContent>
       </Dialog>
     </>

apps/web/modules/ee/contacts/attributes/components/edit-attribute-modal.tsx

@@ -55,58 +55,64 @@ export function EditAttributeModal({ attribute, open, setOpen }: Readonly<EditAt
     setIsUpdating(false);
   };
 
+  const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => {
+    e.preventDefault();
+    await handleUpdate();
+  };
+
   return (
     <Dialog open={open} onOpenChange={setOpen}>
-      <DialogContent className="sm:max-w-lg" disableCloseOnOutsideClick>
+      <DialogContent className="sm:max-w-lg">
         <DialogHeader>
           <DialogTitle>{t("environments.contacts.edit_attribute")}</DialogTitle>
           <DialogDescription>{t("environments.contacts.edit_attribute_description")}</DialogDescription>
         </DialogHeader>
+        <form onSubmit={handleSubmit}>
+          <DialogBody>
+            <div className="flex flex-col gap-4">
+              <div className="flex flex-col gap-2">
+                <label className="text-sm font-medium text-slate-900">
+                  {t("environments.contacts.attribute_key")}
+                </label>
+                <Input value={attribute.key} disabled className="bg-slate-50" />
+                <p className="text-xs text-slate-500">
+                  {t("environments.contacts.attribute_key_cannot_be_changed")}
+                </p>
+              </div>
 
-        <DialogBody>
-          <div className="flex flex-col gap-4">
-            <div className="flex flex-col gap-2">
-              <label className="text-sm font-medium text-slate-900">
-                {t("environments.contacts.attribute_key")}
-              </label>
-              <Input value={attribute.key} disabled className="bg-slate-50" />
-              <p className="text-xs text-slate-500">
-                {t("environments.contacts.attribute_key_cannot_be_changed")}
-              </p>
+              <div className="flex flex-col gap-2">
+                <label className="text-sm font-medium text-slate-900">
+                  {t("environments.contacts.attribute_label")}
+                </label>
+                <Input
+                  value={formData.name}
+                  onChange={(e) => setFormData((prev) => ({ ...prev, name: e.target.value }))}
+                  placeholder={t("environments.contacts.attribute_label_placeholder")}
+                />
+              </div>
+
+              <div className="flex flex-col gap-2">
+                <label className="text-sm font-medium text-slate-900">
+                  {t("environments.contacts.attribute_description")} ({t("common.optional")})
+                </label>
+                <Input
+                  value={formData.description}
+                  onChange={(e) => setFormData((prev) => ({ ...prev, description: e.target.value }))}
+                  placeholder={t("environments.contacts.attribute_description_placeholder")}
+                />
+              </div>
             </div>
+          </DialogBody>
 
-            <div className="flex flex-col gap-2">
-              <label className="text-sm font-medium text-slate-900">
-                {t("environments.contacts.attribute_label")}
-              </label>
-              <Input
-                value={formData.name}
-                onChange={(e) => setFormData((prev) => ({ ...prev, name: e.target.value }))}
-                placeholder={t("environments.contacts.attribute_label_placeholder")}
-              />
-            </div>
-
-            <div className="flex flex-col gap-2">
-              <label className="text-sm font-medium text-slate-900">
-                {t("environments.contacts.attribute_description")} ({t("common.optional")})
-              </label>
-              <Input
-                value={formData.description}
-                onChange={(e) => setFormData((prev) => ({ ...prev, description: e.target.value }))}
-                placeholder={t("environments.contacts.attribute_description_placeholder")}
-              />
-            </div>
-          </div>
-        </DialogBody>
-
-        <DialogFooter>
-          <Button onClick={() => setOpen(false)} type="button" variant="secondary">
-            {t("common.cancel")}
-          </Button>
-          <Button disabled={!formData.name} loading={isUpdating} onClick={handleUpdate} type="submit">
-            {t("common.save")}
-          </Button>
-        </DialogFooter>
+          <DialogFooter className="mt-4">
+            <Button onClick={() => setOpen(false)} type="button" variant="secondary">
+              {t("common.cancel")}
+            </Button>
+            <Button disabled={!formData.name} loading={isUpdating} type="submit">
+              {t("common.save")}
+            </Button>
+          </DialogFooter>
+        </form>
       </DialogContent>
     </Dialog>
   );