Merge pull request #819 from mailpoet/bulk_delete_custom_fields

Deletes custom fields associations when bulk deleting subscribers [MAILPOET-820]

lib/Models/Subscriber.php

@@ -750,7 +750,8 @@ class Subscriber extends Model {
     $count = parent::bulkAction($orm, function($subscriber_ids) {
       // delete all subscriber/segment relationships
       SubscriberSegment::deleteManySubscriptions($subscriber_ids);
-
+      // delete all subscriber/custom field relationships
+      SubscriberCustomField::deleteManySubscriberRelations($subscriber_ids);
       // delete subscribers (except WP Users)
       Subscriber::whereIn('id', $subscriber_ids)
         ->whereNull('wp_user_id')

lib/Models/SubscriberCustomField.php

@@ -89,4 +89,10 @@ class SubscriberCustomField extends Model {
     $relations = self::where('subscriber_id', $subscriber->id);
     return $relations->deleteMany();
   }
+
+  static function deleteManySubscriberRelations(array $subscriber_ids) {
+    if(empty($subscriber_ids)) return false;
+    $relations = self::whereIn('subscriber_id', $subscriber_ids);
+    return $relations->deleteMany();
+  }
 }

tests/unit/Models/SubscriberCustomFieldTest.php

@@ -5,30 +5,31 @@ class SubscriberCustomFieldTest extends MailPoetTest {
   function __construct() {
     $this->data = array(
       array(
-        10, // custom_field_id
+        'custom_field_id' => 10,
-        12, // subscriber_id
+        'subscriber_id' => 12,
-        'Test 1' // value
+        'value' => 'Test 1'
       ),
       array(
-        10, // custom_field_id
+        'custom_field_id' => 10,
-        13, // subscriber_id
+        'subscriber_id' => 13,
-        'Test 2' // value
+        'value' => 'Test 2'
       )
     );
   }
 
   function testItCanBeCreated() {
     $subscriberCustomField = SubscriberCustomField::create();
-    $subscriberCustomField->custom_field_id = $this->data[0][0];
+    $subscriberCustomField->custom_field_id = $this->data[0]['custom_field_id'];
-    $subscriberCustomField->subscriber_id = $this->data[0][1];
+    $subscriberCustomField->subscriber_id = $this->data[0]['subscriber_id'];
-    $subscriberCustomField->value = $this->data[0][2];
+    $subscriberCustomField->value = $this->data[0]['value'];
     $subscriberCustomField->save();
     expect($subscriberCustomField->id())->greaterOrEquals(1);
     expect($subscriberCustomField->getErrors())->false();
   }
 
   function testItCanCreateMultipleRecords() {
-    SubscriberCustomField::createMultiple($this->data);
+    $data = array_map('array_values', $this->data);
+    SubscriberCustomField::createMultiple($data);
     $records = SubscriberCustomField::findArray();
     expect(count($records))->equals(2);
     expect($records[0]['value'])->equals('Test 1');
@@ -36,15 +37,39 @@ class SubscriberCustomFieldTest extends MailPoetTest {
   }
 
   function testItCanUpdateMultipleRecords() {
-    SubscriberCustomField::createMultiple($this->data);
+    $data = array_map('array_values', $this->data);
+    SubscriberCustomField::createMultiple($data);
     $updated_data = $this->data;
-    $updated_data[0][2] = 'Updated';
+    $updated_data[0]['value'] = 'Updated';
+    $updated_data = array_map('array_values', $updated_data);
     SubscriberCustomField::updateMultiple($updated_data);
     $records = SubscriberCustomField::findArray();
     expect($records[0]['value'])->equals('Updated');
     expect($records[1]['value'])->equals('Test 2');
   }
 
+  function testItCanDeleteManySubscriberRelations() {
+    $data = array_map('array_values', $this->data);
+    SubscriberCustomField::createMultiple($data);
+    SubscriberCustomField::deleteManySubscriberRelations(
+      array(
+        $this->data[0]['subscriber_id'],
+        $this->data[1]['subscriber_id']
+      )
+    );
+    $records = SubscriberCustomField::findArray();
+    expect($records)->isEmpty();
+  }
+
+  function testItCanDeleteSubscriberRelations() {
+    $data = array_map('array_values', $this->data);
+    SubscriberCustomField::createMultiple($data);
+    $subscriber = (object)array('id' => $this->data[0]['subscriber_id']);
+    SubscriberCustomField::deleteSubscriberRelations($subscriber);
+    $records = SubscriberCustomField::findArray();
+    expect($records)->count(1);
+  }
+
   function _after() {
     ORM::forTable(SubscriberCustomField::$_table)
       ->deleteMany();

tests/unit/Models/SubscriberTest.php

@@ -732,12 +732,50 @@ class SubscriberTest extends MailPoetTest {
     expect(Subscriber::verifyToken('fake@email.com', $token))->false();
   }
 
-  function testVerifiedTokensOfDifferentLengths() {
+  function testItVerifiesTokensOfDifferentLengths() {
     $token = md5(AUTH_KEY . $this->data['email']);
     expect(strlen($token))->notEquals(Subscriber::SUBSCRIBER_TOKEN_LENGTH);
     expect(Subscriber::verifyToken($this->data['email'], $token))->true();
   }
 
+  function testItBulkDeletesSubscribers() {
+    $segment = Segment::createOrUpdate(
+      array(
+        'name' => 'test'
+      )
+    );
+    $custom_field = CustomField::createOrUpdate(
+      array(
+        'name' => 'name',
+        'type' => 'type',
+        'params' => array(
+          'label' => 'label'
+        ),
+      )
+    );
+    $subscriber_custom_field = SubscriberCustomField::createOrUpdate(
+      array(
+        'subscriber_id' => $this->subscriber->id,
+        'custom_field_id' => $custom_field->id,
+        'value' => 'test',
+      )
+    );
+    expect(SubscriberCustomField::findMany())->count(1);
+    $subscriber_segment = SubscriberSegment::createOrUpdate(
+      array(
+        'subscriber_id' => $this->subscriber->id,
+        'segment_id' => 1
+      )
+    );
+    expect(SubscriberSegment::findMany())->count(1);
+
+    // associated segments and custom fields should be deleted
+    Subscriber::filter('bulkDelete');
+    expect(SubscriberCustomField::findArray())->isEmpty();
+    expect(SubscriberSegment::findArray())->isEmpty();
+    expect(Subscriber::findArray())->isEmpty();
+  }
+
   function _after() {
     ORM::raw_execute('TRUNCATE ' . Subscriber::$_table);
     ORM::raw_execute('TRUNCATE ' . Segment::$_table);