feat: enhance contact data handling by filtering empty strings and allowing null to delete fields

2026-04-27 16:59:18 +02:00
parent 5f741c66c7
commit 10946511b0
3 changed files with 150 additions and 47 deletions
@@ -234,10 +234,22 @@ export class ContactService {
          continue;
        }

+        // Skip empty string values - they don't provide meaningful data
+        // and can cause issues with template rendering and data integrity
+        if (value === '') {
+          continue;
+        }
+
+        // Delete field if null is passed (allows removing fields from contact data)
+        if (value === null) {
+          delete mergedData[key];
+          continue;
+        }
+
        // Validate locale field (special user-settable field)
        // Only validate type - any locale string is accepted since we default to English if unsupported
        if (key === 'locale') {
-          if (value !== null && value !== undefined && typeof value !== 'string') {
+          if (value !== undefined && typeof value !== 'string') {
            throw new HttpException(400, 'Locale must be a string');
          }
        }
@@ -265,33 +277,49 @@ export class ContactService {
      const isSubscriptionChanging = subscribed !== undefined && existing.subscribed !== subscribed;
      const wasSubscribed = existing.subscribed;

-      const updated = await prisma.contact.update({
-        where: {id: existing.id},
-        data: {
-          data: Object.keys(mergedData).length > 0 ? toPrismaJson(mergedData) : Prisma.JsonNull,
-          ...(subscribed !== undefined ? {subscribed} : {}),
-        },
-      });
+      try {
+        const updated = await prisma.contact.update({
+          where: {id: existing.id},
+          data: {
+            data: Object.keys(mergedData).length > 0 ? toPrismaJson(mergedData) : Prisma.JsonNull,
+            ...(subscribed !== undefined ? {subscribed} : {}),
+          },
+        });

-      // Track subscription event if status changed
-      if (isSubscriptionChanging) {
-        if (subscribed && !wasSubscribed) {
-          await EventService.trackEvent(projectId, 'contact.subscribed', updated.id);
-        } else if (!subscribed && wasSubscribed) {
-          await EventService.trackEvent(projectId, 'contact.unsubscribed', updated.id);
+        // Track subscription event if status changed
+        if (isSubscriptionChanging) {
+          if (subscribed && !wasSubscribed) {
+            await EventService.trackEvent(projectId, 'contact.subscribed', updated.id);
+          } else if (!subscribed && wasSubscribed) {
+            await EventService.trackEvent(projectId, 'contact.unsubscribed', updated.id);
+          }
        }
-      }

-      return updated;
+        return updated;
+      } catch (error) {
+        // Provide helpful error message for database/validation issues
+        throw new HttpException(
+          500,
+          `Failed to update contact: ${error instanceof Error ? error.message : 'Unknown error'}`,
+        );
+      }
    } else {
-      return prisma.contact.create({
-        data: {
-          projectId,
-          email,
-          data: Object.keys(mergedData).length > 0 ? toPrismaJson(mergedData) : Prisma.JsonNull,
-          subscribed: subscribed ?? defaultSubscribed,
-        },
-      });
+      try {
+        return await prisma.contact.create({
+          data: {
+            projectId,
+            email,
+            data: Object.keys(mergedData).length > 0 ? toPrismaJson(mergedData) : Prisma.JsonNull,
+            subscribed: subscribed ?? defaultSubscribed,
+          },
+        });
+      } catch (error) {
+        // Provide helpful error message for database/validation issues
+        throw new HttpException(
+          500,
+          `Failed to create contact: ${error instanceof Error ? error.message : 'Unknown error'}`,
+        );
+      }
    }
  }

@@ -336,19 +336,75 @@ describe('ContactService - Duplicate Prevention & Data Merging', () => {
      });
    });

-    it('should handle null values in data', async () => {
+    it('should delete keys when null value is passed', async () => {
      const email = 'test@example.com';

      const contact = await ContactService.upsert(projectId, email, {
        firstName: 'John',
-        middleName: null,
+        middleName: null, // null should delete/not store the key
        lastName: 'Doe',
      });

      expect(contact.data).toHaveProperty('firstName', 'John');
-      expect(contact.data).toHaveProperty('middleName', null);
+      expect(contact.data).not.toHaveProperty('middleName'); // Key should not exist
      expect(contact.data).toHaveProperty('lastName', 'Doe');
    });
+
+    it('should remove existing fields when null is passed', async () => {
+      const email = 'test@example.com';
+
+      // Create contact with data
+      await ContactService.upsert(projectId, email, {
+        firstName: 'John',
+        middleName: 'Michael',
+        lastName: 'Doe',
+      });
+
+      // Update with null to remove middleName
+      const updated = await ContactService.upsert(projectId, email, {
+        middleName: null, // Should delete this field
+      });
+
+      expect(updated.data).toHaveProperty('firstName', 'John'); // Preserved
+      expect(updated.data).not.toHaveProperty('middleName'); // Removed
+      expect(updated.data).toHaveProperty('lastName', 'Doe'); // Preserved
+    });
+
+    it('should filter out empty string values from data', async () => {
+      const email = 'test@example.com';
+
+      const contact = await ContactService.upsert(projectId, email, {
+        firstName: 'John',
+        middleName: '', // Empty string should be filtered out
+        lastName: 'Doe',
+        company: '', // Empty string should be filtered out
+      });
+
+      expect(contact.data).toHaveProperty('firstName', 'John');
+      expect(contact.data).toHaveProperty('lastName', 'Doe');
+      expect(contact.data).not.toHaveProperty('middleName');
+      expect(contact.data).not.toHaveProperty('company');
+    });
+
+    it('should not overwrite existing data with empty strings', async () => {
+      const email = 'test@example.com';
+
+      // Create contact with data
+      await ContactService.upsert(projectId, email, {
+        firstName: 'John',
+        company: 'Acme Inc',
+      });
+
+      // Update with empty string - should preserve existing values
+      const updated = await ContactService.upsert(projectId, email, {
+        firstName: '', // Should be filtered out, preserving "John"
+        lastName: 'Doe',
+      });
+
+      expect(updated.data).toHaveProperty('firstName', 'John'); // Preserved
+      expect(updated.data).toHaveProperty('company', 'Acme Inc'); // Preserved
+      expect(updated.data).toHaveProperty('lastName', 'Doe'); // New value
+    });
  });

  describe('Contact CRUD Operations', () => {