- Updates based on code review comments

assets/js/lib/jquery.asyncqueue.js

@@ -0,0 +1,79 @@
+/*
+* This file is part of the jquery plugin "asyncQueue".
+*
+* (c) Sebastien Roch <roch.sebastien@gmail.com>
+*
+* For the full copyright and license information, please view the LICENSE
+* file that was distributed with this source code.
+*/
+(function($){
+    $.AsyncQueue = function() {
+        var that = this,
+            queue = [],
+            failureFunc,
+            completeFunc,
+            paused = false,
+            lastCallbackData,
+            _run;
+
+        _run = function() {
+            var f = queue.shift();
+
+            if (f) {
+                f.apply(that, [that]);
+                if (paused === false) {
+                    _run();
+                }
+            } else {
+                if(completeFunc){
+                    completeFunc.apply(that);
+                }
+            }
+        }
+
+        this.onFailure = function(func) {
+            failureFunc = func;
+        }
+
+        this.onComplete = function(func) {
+            completeFunc = func;
+        }
+
+        this.add = function(func) {
+            queue.push(func);
+            return this;
+        }
+
+        this.storeData = function(dataObject) {
+            lastCallbackData = dataObject;
+            return this;
+        }
+
+        this.lastCallbackData = function () {
+            return lastCallbackData;
+        }
+
+        this.run = function() {
+            paused = false;
+            _run();
+        }
+
+        this.pause = function () {
+            paused = true;
+            return this;
+        }
+
+        this.failure = function() {
+            paused = true;
+            if (failureFunc) {
+                var args = [that];
+                for(i = 0; i < arguments.length; i++) {
+                    args.push(arguments[i]);
+                }
+                failureFunc.apply(that, args);
+            }
+        }
+
+        return this;
+    }
+})(jQuery);

assets/js/src/subscribers/importExport/import.js

@@ -22,7 +22,7 @@ define(
         return;
       }
       jQuery(document).ready(function () {
-        console.log = function() {};
+        var noticeTimeout = 3000;
         jQuery('input[name="select_method"]').attr('checked', false);
         // configure router
         router = new (Backbone.Router.extend({
@@ -127,7 +127,7 @@ define(
             var pasteSize = encodeURI(pasteInputElement.val()).split(/%..|./).length - 1;
             if (pasteSize > maxPostSizeBytes) {
               MailPoet.Notice.error(MailPoetI18n.maxPostSizeNotice, {
-                timeout: 3000,
+                timeout: noticeTimeout,
               });
               return;
             }
@@ -147,7 +147,7 @@ define(
               if (ext === null || ext[1].toLowerCase() !== 'csv') {
                 this.value = '';
                 MailPoet.Notice.error(MailPoetI18n.wrongFileFormat, {
-                  timeout: 3000,
+                  timeout: noticeTimeout,
                 });
               }
 
@@ -197,7 +197,7 @@ define(
               if (response.result === false) {
                 MailPoet.Notice.hide();
                 MailPoet.Notice.error(response.errors, {
-                  timeout: 3000,
+                  timeout: noticeTimeout,
                 });
                 jQuery('.mailpoet_mailchimp-key-status')
                     .removeClass()
@@ -222,7 +222,7 @@ define(
               MailPoet.Modal.loading(false);
               MailPoet.Notice.error(
                   MailPoetI18n.serverError + error.statusText.toLowerCase() + '.', {
-                    timeout: 3000,
+                    timeout: noticeTimeout,
                   }
               );
             });
@@ -249,7 +249,7 @@ define(
               else {
                 MailPoet.Notice.hide();
                 MailPoet.Notice.error(response.errors, {
-                  timeout: 3000,
+                  timeout: noticeTimeout,
                 });
               }
               MailPoet.Modal.loading(false);
@@ -257,7 +257,7 @@ define(
               MailPoet.Modal.loading(false);
               MailPoet.Notice.error(
                   MailPoetI18n.serverError + result.statusText.toLowerCase() + '.', {
-                    timeout: 3000,
+                    timeout: noticeTimeout,
                   }
               );
             });
@@ -349,7 +349,7 @@ define(
               error: function () {
                 MailPoet.Notice.hide();
                 MailPoet.Notice.error(MailPoetI18n.dataProcessingError, {
-                  timeout: 3000,
+                  timeout: noticeTimeout,
                 });
               },
               complete: function (CSV) {
@@ -433,7 +433,7 @@ define(
                   errorNotice = errorNotice.replace('[link]', MailPoetI18n.csvKBLink);
                   errorNotice = errorNotice.replace('[/link]', '</a>');
                   MailPoet.Notice.error(errorNotice, {
-                    timeout: 3000,
+                    timeout: noticeTimeout,
                   });
                 }
               }
@@ -564,7 +564,7 @@ define(
                     if (!segmentSelectionNotice.length) {
                       MailPoet.Notice.error(MailPoetI18n.segmentSelectionRequired, {
                         static: true,
-                        timeout: 3000,
+                        timeout: noticeTimeout,
                         scroll: true,
                         id: 'segmentSelection',
                         hideClose: true
@@ -643,7 +643,7 @@ define(
                         MailPoet.Modal.close();
                         MailPoet.Notice.error(
                             MailPoetI18n.segmentCreateError + response.message + '.', {
-                              timeout: 3000,
+                              timeout: noticeTimeout,
                             }
                         );
                       }
@@ -652,7 +652,7 @@ define(
                       MailPoet.Modal.close();
                       MailPoet.Notice.error(
                           MailPoetI18n.serverError + error.statusText.toLowerCase() + '.', {
-                            timeout: 3000
+                            timeout: noticeTimeout
                           }
                       );
                     });
@@ -859,7 +859,7 @@ define(
                             }
                             else {
                               MailPoet.Notice.error(MailPoetI18n.customFieldCreateError, {
-                                timeout: 3000,
+                                timeout: noticeTimeout,
                               });
                             }
                             MailPoet.Modal.loading(false);
@@ -868,7 +868,7 @@ define(
                             MailPoet.Modal.loading(false);
                             MailPoet.Notice.error(
                                 MailPoetI18n.serverError + error.statusText.toLowerCase() + '.', {
-                                  timeout: 3000,
+                                  timeout: noticeTimeout,
                                 }
                             );
                           });
@@ -933,7 +933,7 @@ define(
                   if (!jQuery('[data-id="notice_invalidEmail"]').length) {
                     MailPoet.Notice.error(MailPoetI18n.columnContainsInvalidElement, {
                       static: true,
-                      timeout: 3000,
+                      timeout: noticeTimeout,
                       scroll: true,
                       hideClose: true,
                       id: 'invalidEmail'
@@ -1013,7 +1013,7 @@ define(
                 if (preventNextStep && !jQuery('.mailpoet_invalidDate').length) {
                   MailPoet.Notice.error(MailPoetI18n.columnContainsInvalidDate, {
                     static: true,
-                    timeout: 3000,
+                    timeout: noticeTimeout,
                     scroll: true,
                     hideClose: true,
                     id: 'invalidDate'
@@ -1055,7 +1055,9 @@ define(
 
             var columns = {},
               queue = new jQuery.AsyncQueue(),
-              batch = 0,
+              batchNumber = 0,
+              batchSize = 500,
+              timestamp = Date.now() / 1000,
               subscribers = [],
               importResults = {
                 'created': 0,
@@ -1072,7 +1074,7 @@ define(
                   return res;
                 }, []);
               },
-              subscribers = splitSubscribers(importData.step1.subscribers, 500);
+              subscribers = splitSubscribers(importData.step1.subscribers, batchSize);
 
             _.each(jQuery('select.mailpoet_subscribers_column_data_match'),
               function (column, columnIndex) {
@@ -1092,8 +1094,8 @@ define(
                     action: 'processImport',
                     data: JSON.stringify({
                       columns: columns,
-                      subscribers: subscribers[batch],
+                      subscribers: subscribers[batchNumber],
-                      length: subscribers[batch].length,
+                      timestamp: timestamp,
                       segments: segmentSelectElement.val(),
                       updateSubscribers: (jQuery(':radio[name="subscriber_update_option"]:checked').val() === 'yes') ? true : false
                     })
@@ -1102,8 +1104,8 @@ define(
                     if (response.result === false) {
                       importResults.errors.push(response.errors);
                     } else {
-                      importResults.created += response.data.created;
+                      importResults.created = response.data.created;
-                      importResults.updated += response.data.updated;
+                      importResults.updated = response.data.updated;
                       importResults.segments = response.data.segments;
                     }
                     queue.run();
@@ -1114,7 +1116,7 @@ define(
                     );
                     queue.run();
                   });
-                batch++;
+                batchNumber++;
               })
             });
 
@@ -1124,7 +1126,7 @@ define(
               MailPoet.Modal.loading(false);
               if (importResults.errors.length > 0 && !importResults.updated && !importResults.created) {
                 MailPoet.Notice.error(_.flatten(importResults.errors), {
-                    timeout: 3000,
+                    timeout: noticeTimeout,
                   }
                 );
               }
@@ -1155,7 +1157,7 @@ define(
 
           if (importData.step2.errors.length > 0) {
             MailPoet.Notice.error(_.flatten(importData.step2.errors), {
-              timeout: 3000,
+              timeout: noticeTimeout,
             });
           }
 

lib/Config/Migrator.php

@@ -57,7 +57,6 @@ class Migrator {
       'last_name tinytext NOT NULL,',
       'email varchar(150) NOT NULL,',
       'status varchar(12) NOT NULL DEFAULT "unconfirmed",',
-      'import_batch varchar(12) NULL,',
       'created_at TIMESTAMP NOT NULL DEFAULT 0,',
       'deleted_at TIMESTAMP NULL DEFAULT NULL,',
       'updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,',

lib/Models/Subscriber.php

@@ -498,7 +498,7 @@ class Subscriber extends Model {
     );
   }
 
-  static function updateMultiple($columns, $subscribers, $import_batch = false) {
+  static function updateMultiple($columns, $subscribers, $updated_at = false) {
     $ignoreColumnsOnUpdate = array(
       'email',
       'created_at'
@@ -538,7 +538,7 @@ class Subscriber extends Model {
     return self::rawExecute(
       'UPDATE `' . self::$_table . '` ' .
       'SET ' . implode(', ', $sql('statement')) . ' '.
-      (($import_batch) ? ', import_batch = "' . $import_batch . '" ' : '') .
+      (($updated_at) ? ', updated_at = "' . $updated_at . '" ' : '') .
         'WHERE email IN ' .
         '(' . rtrim(str_repeat('?,', count($subscribers)), ',') . ')',
       array_merge(

lib/Subscribers/ImportExport/Import/Import.php

@@ -6,7 +6,6 @@ use MailPoet\Models\SubscriberCustomField;
 use MailPoet\Models\SubscriberSegment;
 use MailPoet\Subscribers\ImportExport\BootStrapMenu;
 use MailPoet\Util\Helpers;
-use MailPoet\Util\Security;
 
 class Import {
   public $subscribers_data;
@@ -15,8 +14,8 @@ class Import {
   public $subscriber_fields;
   public $subscriber_custom_fields;
   public $subscribers_count;
-  public $import_time;
+  public $created_at;
-  public $import_batch;
+  public $updated_at;
   public $profiler_start;
 
   public function __construct($data) {
@@ -33,8 +32,8 @@ class Import {
       array_keys($data['columns'])
     );
     $this->subscribers_count = count(reset($this->subscribers_data));
-    $this->import_time = date('Y-m-d H:i:s');
+    $this->created_at = date('Y-m-d H:i:s', (int) $data['timestamp']);
-    $this->import_batch = Security::generateRandomString();
+    $this->updated_at = date('Y-m-d H:i:s', (int) $data['timestamp'] + 1);
     $this->profiler_start = microtime(true);
   }
 
@@ -84,7 +83,6 @@ class Import {
         'updated' => count($updated_subscribers),
         'segments' => $segments->getSegments()
       ),
-      'time' => date('Y-m-d H:i:s'),
       'profiler' => $this->timeExecution()
     );
   }
@@ -97,13 +95,14 @@ class Import {
   }
 
   function filterExistingAndNewSubscribers($subscribers_data) {
+    $chunk_size = 200;
     $existing_records = array_filter(
       array_map(function($subscriber_emails) {
         return Subscriber::selectMany(array('email'))
           ->whereIn('email', $subscriber_emails)
           ->whereNull('deleted_at')
           ->findArray();
-      }, array_chunk($subscribers_data['email'], 200))
+      }, array_chunk($subscribers_data['email'], $chunk_size))
     );
     if(!$existing_records) {
       return array(
@@ -150,17 +149,19 @@ class Import {
   }
 
   function deleteExistingTrashedSubscribers($subscribers_data) {
+    $chunk_size = 200;
     $existing_trashed_records = array_filter(
       array_map(function($subscriber_emails) {
         return Subscriber::selectMany(array('id'))
           ->whereIn('email', $subscriber_emails)
           ->whereNotNull('deleted_at')
           ->findArray();
-      }, array_chunk($subscribers_data['email'], 200))
+      }, array_chunk($subscribers_data['email'], $chunk_size))
     );
     if(!$existing_trashed_records) return;
     $existing_trashed_records = Helpers::flattenArray($existing_trashed_records);
-    foreach(array_chunk($existing_trashed_records, 200) as $subscriber_ids) {
+    foreach(array_chunk($existing_trashed_records, $chunk_size) as
+            $subscriber_ids) {
       Subscriber::whereIn('id', $subscriber_ids)
         ->deleteMany();
       SubscriberSegment::whereIn('subscriber_id', $subscriber_ids)
@@ -170,16 +171,8 @@ class Import {
 
   function extendSubscribersAndFields($subscribers_data, $subscriber_fields) {
     $subscribers_data['created_at'] =
-      array_fill(0, $this->subscribers_count, $this->import_time);
+      array_fill(0, $this->subscribers_count, $this->created_at);
-    $subscribers_data['import_batch'] =
+    $subscriber_fields[] = 'created_at';
-      array_fill(0, $this->subscribers_count, $this->import_batch);
-    $subscriber_fields = array_merge(
-      $subscriber_fields,
-      array(
-        'created_at',
-        'import_batch'
-      )
-    );
     return array(
       $subscribers_data,
       $subscriber_fields
@@ -260,14 +253,14 @@ class Import {
     $subscriber_fields,
     $subscriber_custom_fields
   ) {
+    $chunk_size = 100;
     $subscribers_count = count(reset($subscribers_data)) - 1;
     $subscribers = array_map(function($index) use ($subscribers_data, $subscriber_fields) {
       return array_map(function($field) use ($index, $subscribers_data) {
         return $subscribers_data[$field][$index];
       }, $subscriber_fields);
     }, range(0, $subscribers_count));
-    $batch = ($action === 'update') ? Security::generateRandomString() : $this->import_batch;
+    foreach(array_chunk($subscribers, $chunk_size) as $data) {
-    foreach(array_chunk($subscribers, 100) as $data) {
       if($action == 'create') {
         Subscriber::createMultiple(
           $subscriber_fields,
@@ -278,18 +271,20 @@ class Import {
         Subscriber::updateMultiple(
           $subscriber_fields,
           $data,
-          $batch
+          $this->updated_at
         );
       }
     }
-    $result = Helpers::arrayColumn( // return id=>email array of results
+    $query = Subscriber::selectMany(
-      Subscriber::selectMany(
       array(
         'id',
         'email'
-        ))
+      ));
-        ->where('import_batch', $batch)
+    $query = ($action === 'update') ?
-        ->findArray(),
+      $query->where('updated_at', $this->updated_at) :
+      $query->where('created_at', $this->created_at);
+    $result = Helpers::arrayColumn(
+      $query->findArray(),
       'email', 'id'
     );
     if($subscriber_custom_fields) {

package.json

@@ -5,8 +5,7 @@
   },
   "napa": {
     "blob": "eligrey/Blob.js.git",
-    "filesaver": "eligrey/FileSaver.js.git",
+    "filesaver": "eligrey/FileSaver.js.git"
-    "asyncqueue": "mjward/Jquery-Async-queue.git"
   },
   "dependencies": {
     "backbone": "1.2.3",

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

@@ -7,7 +7,7 @@ use MailPoet\Subscribers\ImportExport\Import\Import;
 use MailPoet\Util\Helpers;
 
 class ImportCest {
-  function __construct() {
+  function _before() {
     $this->data = array(
       'subscribers' => array(
         array(
@@ -32,6 +32,7 @@ class ImportCest {
       'segments' => array(
         195
       ),
+      'timestamp' => time(),
       'updateSubscribers' => true
     );
     $this->subscriber_fields = array(
@@ -54,9 +55,8 @@ class ImportCest {
     expect(is_array($this->import->subscriber_fields))->true();
     expect(is_array($this->import->subscriber_custom_fields))->true();
     expect($this->import->subscribers_count)->equals(2);
-    expect($this->import->import_batch)->notEmpty();
+    expect($this->import->created_at)->notEmpty();
-    expect($this->import->import_time)->notEmpty();
+    expect($this->import->updated_at)->notEmpty();
-    expect($this->import->import_batch)->notEmpty();
   }
 
   function itCanTransformSubscribers() {
@@ -313,24 +313,27 @@ class ImportCest {
     );
   }
 
-  function itCanProcess() {
+  function itCanUpdateSubscribers() {
-    $import = clone($this->import);
+    $result = $this->import->process();
-    $result = $import->process();
-    expect($result['data']['created'])->equals(2);
     expect($result['data']['updated'])->equals(0);
-    $result = $import->process();
+    $result = $this->import->process();
-    expect($result['data']['created'])->equals(0);
     expect($result['data']['updated'])->equals(2);
+    $this->import->update_subscribers = false;
+    $result = $this->import->process();
+    expect($result['data']['updated'])->equals(0);
+  }
+
+  function itCanProcess() {
+    $result = $this->import->process();
+    expect($result['data']['created'])->equals(2);
     Subscriber::where('email', 'mary@jane.com')
       ->findOne()
       ->delete();
-    $result = $import->process();
+    $timestamp = time() + 1;
+    $this->import->created_at = date('Y-m-d H:i:s', $timestamp);
+    $this->import->updated_at = date('Y-m-d H:i:s', $timestamp + 1);
+    $result = $this->import->process();
     expect($result['data']['created'])->equals(1);
-    expect($result['data']['updated'])->equals(1);
-    $import->update_subscribers = false;
-    $result = $import->process();
-    expect($result['data']['created'])->equals(0);
-    expect($result['data']['updated'])->equals(0);
   }
 
   function _after() {

webpack.config.js

@@ -17,7 +17,8 @@ baseConfig = {
   resolve: {
     modulesDirectories: [
       'node_modules',
-      'assets/js/src'
+      'assets/js/src',
+      'assets/js/lib'
     ],
     alias: {
       'handlebars': 'handlebars/dist/handlebars.js',
@@ -31,7 +32,7 @@ baseConfig = {
       'papaparse': 'papaparse/papaparse.min.js',
       'helpscout': 'helpscout.js',
       'html2canvas': 'html2canvas/dist/html2canvas.js',
-      'asyncqueue': 'asyncqueue/jquery.asyncqueue.js'
+      'asyncqueue': 'jquery.asyncqueue.js'
     },
   },
   node: {