- Simplifies code that prepares subscriber custom field data for

creation/update
- Updates tests

lib/Models/SubscriberCustomField.php

@@ -52,7 +52,6 @@ class SubscriberCustomField extends Model {
   }
 
   static function createMultiple($values) {
-    $values = array_map('array_values', $values);
     return self::rawExecute(
       'INSERT IGNORE INTO `' . self::$_table . '` ' .
       '(custom_field_id, subscriber_id, value) ' .
@@ -67,8 +66,6 @@ class SubscriberCustomField extends Model {
   }
 
   static function updateMultiple($values) {
-    self::createMultiple($values);
-    $values = array_map('array_values', $values);
     self::rawExecute(
       'UPDATE `' . self::$_table . '` ' .
       'SET value = ' .

lib/Subscribers/ImportExport/Import/Import.php

@@ -210,10 +210,12 @@ class Import {
       );
     $existing_subscribers =
       array_map(function($subscriber) use ($new_records) {
-        return array_values( // reindex array
+        return array_filter(
+          array_values( // reindex array
             array_map(function($index, $data) use ($new_records) {
               if(!in_array($index, $new_records)) return $data;
             }, array_keys($subscriber), $subscriber)
+          )
         );
       }, $subscribers_data);
     return array(
@@ -359,7 +361,7 @@ class Import {
       $query->where('created_at', $this->created_at);
     $result = Helpers::arrayColumn(
       $query->findArray(),
-      'email', 'id'
+      'id'
     );
     if($subscriber_custom_fields) {
       $this->createOrUpdateCustomFields(
@@ -378,37 +380,36 @@ class Import {
 
   function createOrUpdateCustomFields(
     $action,
-    $db_subscribers,
+    $db_subscribers_ids,
     $subscribers_data,
-    $subscriber_custom_fields
+    $subscriber_custom_fields_ids
   ) {
     // check if custom fields exist in the database
-    $subscriber_custom_fields = Helpers::flattenArray(
+    $subscriber_custom_fields_ids = Helpers::flattenArray(
-      CustomField::whereIn('id', $subscriber_custom_fields)->select('id')->findArray()
+      CustomField::whereIn('id', $subscriber_custom_fields_ids)
+        ->select('id')
+        ->findArray()
     );
-    if(!$subscriber_custom_fields) return;
+    if(!$subscriber_custom_fields_ids) return;
-    $subscribers = array_map(function($column) use ($db_subscribers, $subscribers_data) {
+    $subscriber_custom_fields_data = array();
-      $count = range(0, count($subscribers_data[$column]) - 1);
+    foreach($subscribers_data as $field_id => $subscriber_data) {
-      return array_filter( // remove null values
+      // exclude non-custom fields
-        array_map(function($index, $value) use ($db_subscribers, $subscribers_data, $column) {
+      if(!is_int($field_id)) continue;
-          $subscriber_id = array_search(
+      $subscriber_index = 0;
-            $subscribers_data['email'][$index],
+      foreach($subscriber_data as $value) {
-            $db_subscribers
+        // assemble an array: custom_field_id, subscriber_id, value
-          );
+        $subscriber_custom_fields_data[] = array(
-          // if subscriber does not not exist, return a null value
+          (int)$field_id,
-          return ($subscriber_id) ?
+          (int)$db_subscribers_ids[$subscriber_index],
-            array($column, $subscriber_id, $value) :
+          $value
-            null;
-        }, $count, $subscribers_data[$column])
-      );
-    }, $subscriber_custom_fields);
-    $subscribers[0] = array_filter($subscribers[0]);
-    foreach(array_chunk($subscribers[0], 200) as $data) {
-      if($action === 'create') {
-        SubscriberCustomField::createMultiple(
-          $data
         );
+        $subscriber_index++;
       }
+    }
+    foreach(array_chunk($subscriber_custom_fields_data, 200) as $data) {
+      SubscriberCustomField::createMultiple(
+        $data
+      );
       if($action === 'update') {
         SubscriberCustomField::updateMultiple(
           $data

tests/unit/Models/SubscriberCustomFieldTest.php

@@ -5,36 +5,44 @@ class SubscriberCustomFieldTest extends MailPoetTest {
   function __construct() {
     $this->data = array(
       array(
-        'custom_field_id' => 10,
+        10, // custom_field_id
-        'subscriber_id' => 12,
+        12, // subscriber_id
-        'value' => 'Test 1'
+        'Test 1' // value
       ),
       array(
-        'custom_field_id' => 10,
+        10, // custom_field_id
-        'subscriber_id' => 13,
+        13, // subscriber_id
-        'value' => 'Test 2'
+        'Test 2' // value
       )
     );
   }
 
   function testItCanBeCreated() {
     $subscriberCustomField = SubscriberCustomField::create();
-    $subscriberCustomField->hydrate($this->data[0]);
+    $subscriberCustomField->custom_field_id = $this->data[0][0];
+    $subscriberCustomField->subscriber_id = $this->data[0][1];
+    $subscriberCustomField->value = $this->data[0][2];
     $subscriberCustomField->save();
-    expect($subscriberCustomField->id() > 0)->true();
+    expect($subscriberCustomField->id())->greaterOrEquals(1);
     expect($subscriberCustomField->getErrors())->false();
   }
 
-  function testItCanCreateOrUpdateMultipleRecords() {
+  function testItCanCreateMultipleRecords() {
     SubscriberCustomField::createMultiple($this->data);
     $records = SubscriberCustomField::findArray();
     expect(count($records))->equals(2);
-    expect($records[1]['value'])->equals($this->data[1]['value']);
+    expect($records[0]['value'])->equals('Test 1');
-    $updatedData = $this->data;
+    expect($records[1]['value'])->equals('Test 2');
-    $updatedData[0]['value'] = 'updated';
+  }
-    SubscriberCustomField::updateMultiple($updatedData);
+
+  function testItCanUpdateMultipleRecords() {
+    SubscriberCustomField::createMultiple($this->data);
+    $updated_data = $this->data;
+    $updated_data[0][2] = 'Updated';
+    SubscriberCustomField::updateMultiple($updated_data);
     $records = SubscriberCustomField::findArray();
-    expect($records[0]['value'])->equals($updatedData[0]['value']);
+    expect($records[0]['value'])->equals('Updated');
+    expect($records[1]['value'])->equals('Test 2');
   }
 
   function _after() {

tests/unit/Subscribers/ImportExport/Import/ImportTest.php

@@ -279,18 +279,18 @@ class ImportTest extends MailPoetTest {
       $this->subscriber_fields,
       false
     );
-    $db_subscribers = Helpers::arrayColumn(
+    $db_subscribers_ids = Helpers::arrayColumn(
       Subscriber::selectMany(
         array(
           'id',
           'email'
         ))
         ->findArray(),
-      'email', 'id'
+      'id'
     );
     $this->import->createOrUpdateCustomFields(
       'create',
-      $db_subscribers,
+      $db_subscribers_ids,
       $subscribers_data,
       $this->subscriber_custom_fields
     );
@@ -301,7 +301,7 @@ class ImportTest extends MailPoetTest {
     $subscribers_data[$custom_field][1] = 'Rio';
     $this->import->createOrUpdateCustomFields(
       'update',
-      $db_subscribers,
+      $db_subscribers_ids,
       $subscribers_data,
       $this->subscriber_custom_fields
     );