- Update import to autodetect dates (UI) and convert them to datetime

format (backend) - Fixes unit test - Fixes code formatting in Date class
2016-08-11 23:02:53 -04:00
parent f8fea75130
commit 81c277ca93
5 changed files with 105 additions and 65 deletions
--- a/assets/js/src/subscribers/importExport/import.js
+++ b/assets/js/src/subscribers/importExport/import.js
@@ -51,6 +51,12 @@ define(
         *  STEP 1 (upload or copy/paste)
         */
        router.on('route:step1', function () {
+          // set or reset temporary validation rule on all columns
+          mailpoetColumns = jQuery.map(mailpoetColumns, function (column, columnIndex) {
+            column.validation_rule = false;
+            return column;
+          });
+
          if (typeof (importData.step1) !== 'undefined') {
            showCurrentStep();
            return;
@@ -797,6 +803,7 @@ define(
                              })
                          });
                        jQuery(selectElement).data('column-id', new_column_data.id);
+                        jQuery(selectElement).data('validation-rule', false);
                        filterSubscribers();
                        // close popup
                        MailPoet.Modal.close();
@@ -847,22 +854,22 @@ define(
                .remove();
            var subscribersClone = jQuery.extend(true, {}, subscribers),
                preventNextStep = false,
-                displayedColumnsIds = jQuery.map(
-                    jQuery('.mailpoet_subscribers_column_data_match'), function (data) {
-                      var columnId = jQuery(data).data('column-id');
-                      jQuery(data).val(columnId).trigger('change');
-                      return columnId;
+                displayedColumns = jQuery.map(
+                    jQuery('.mailpoet_subscribers_column_data_match'), function (element, elementIndex) {
+                      var columnId = jQuery(element).data('column-id');
+                      var validationRule = jQuery(element).data('validation-rule');
+                      jQuery(element).val(columnId).trigger('change');
+                      return { id: columnId, index: elementIndex, validationRule: validationRule, element: element };
                    });
            // iterate through the object of mailpoet columns
-            jQuery.map(mailpoetColumns, function (column) {
+            jQuery.map(mailpoetColumns, function (column, columnIndex) {
              // check if the column id matches the selected id of one of the
              // subscriber's data columns
-              var matchedColumn = jQuery.inArray(column.id, displayedColumnsIds);
-
-              // EMAIL filter: if the last value in the column doesn't have a valid
+              var matchedColumn = _.find(displayedColumns, function(data) { return data.id === column.id; });
+              // EMAIL filter: if the first value in the column doesn't have a valid
              // email, hide the next button
-              if (column.id === "email") {
-                if (!emailRegex.test(subscribersClone.subscribers[0][matchedColumn])) {
+              if (column.id === 'email') {
+                if (!emailRegex.test(subscribersClone.subscribers[0][matchedColumn.index])) {
                  preventNextStep = true;
                  if (!jQuery('[data-id="notice_invalidEmail"]').length) {
                    MailPoet.Notice.error(MailPoet.I18n.t('columnContainsInvalidElement'), {
@@ -878,39 +885,63 @@ define(
                }
              }
              // DATE filter: if column type is date, check if we can recognize it
-              if (column.type === 'date' && matchedColumn !== -1) {
-                jQuery.map(subscribersClone.subscribers, function (data, position) {
-                  var rowData = data[matchedColumn];
-                  var dateFormat = column.params.date_format.toUpperCase();
-                  var date = Moment(rowData, dateFormat, true);
-                  if (position !== fillerPosition) {
+              if (column.type === 'date' && matchedColumn) {
+                var allowedDateFormats = [
+                  Moment.ISO_8601,
+                  'YYYY/MM/DD',
+                  'MM/DD/YYYY',
+                  'DD/MM/YYYY',
+                  'YYYY/MM/DD',
+                  'YYYY/DD/MM',
+                  'MM/YYYY',
+                  'YYYY/MM',
+                  'YYYY'
+                ];
+                var firstRowData = subscribersClone.subscribers[0][matchedColumn.index];
+                var validationRule = false;
                // check if date exists
-                    if (rowData.trim() === '') {
-                      data[matchedColumn] =
+                if (firstRowData.trim() === '') {
+                  subscribersClone.subscribers[0][matchedColumn.index] =
                    '<span class="mailpoet_data_match mailpoet_import_error" title="'
                    + MailPoet.I18n.t('noDateFieldMatch') + '">'
                    + MailPoet.I18n.t('emptyDate')
                    + '</span>';
                  preventNextStep = true;
-                      return;
                }
-                    // check if date is valid
+                else {
+                  for (var format in allowedDateFormats) {
+                    var testedFormat = allowedDateFormats[format]
+                    if (Moment(firstRowData, testedFormat, true).isValid()) {
+                      var validationRule = (typeof(testedFormat) === 'function') ?
+                        'datetime' :
+                        testedFormat
+                      // set validation on the column element
+                      jQuery(matchedColumn.element).data('validation-rule', validationRule);
+                      break;
+                    }
+                    if (validationRule === 'datetime') validationRule = Moment.ISO_8601;
+                  }
+                }
+                jQuery.map(subscribersClone.subscribers, function (data, index) {
+                  if (index === fillerPosition) return;
+                  var rowData = data[matchedColumn.index];
+                  var date = Moment(rowData, testedFormat, true);
+                  // validate date:
                  if (date.isValid()) {
-                      data[matchedColumn] +=
+                    data[matchedColumn.index] +=
                        '<span class="mailpoet_data_match" title="'
                        + MailPoet.I18n.t('verifyDateMatch') + '">'
                        + MailPoet.Date.format(date)
                        + '</span>';
                  }
                  else {
-                      data[matchedColumn] +=
+                    data[matchedColumn.index] +=
                      '<span class="mailpoet_data_match mailpoet_import_error" title="'
                      + MailPoet.I18n.t('noDateFieldMatch') + '">'
-                          + MailPoet.I18n.t('dateMatchError') + ' (' + dateFormat + ') '
+                      + MailPoet.I18n.t('dateMatchError')
                      + '</span>';
                    preventNextStep = true;
-                    }
-                  }
+                  };
                });
                if (preventNextStep && !jQuery('.mailpoet_invalidDate').length) {
                  MailPoet.Notice.error(MailPoet.I18n.t('columnContainsInvalidDate'), {
@@ -980,10 +1011,11 @@ define(
            _.each(jQuery('select.mailpoet_subscribers_column_data_match'),
              function (column, columnIndex) {
                var columnId = jQuery(column).data('column-id');
+                var validationRule = jQuery(column).data('validation-rule');
                if (columnId === 'ignore') {
                  return;
                }
-                columns[columnId] = columnIndex;
+                columns[columnId] = { index: columnIndex, validation_rule: validationRule };
              });

            _.each(subscribers, function () {
--- a/lib/Form/Block/Date.php
+++ b/lib/Form/Block/Date.php
@@ -197,10 +197,9 @@ class Date extends Base {

  static function convertDateToDatetime($date, $date_format) {
    $datetime = false;
-    if ($date_format === 'datetime') {
+    if($date_format === 'datetime') {
      $datetime = $date;
-    }
-    else {
+    } else {
      $parsed_date = explode('/', $date);
      $parsed_date_format = explode('/', $date_format);
      $year_position = array_search('YYYY', $parsed_date_format);
--- a/tests/unit/Models/SubscriberTest.php
+++ b/tests/unit/Models/SubscriberTest.php
@@ -169,7 +169,8 @@ class SubscriberTest extends MailPoetTest {
      'name' => 'Birthday',
      'type' => 'date',
      'params' => array(
-        'date_type' => 'year_month_day'
+        'date_type' => 'year_month_day',
+        'date_format' => 'MM/DD/YYYY'
      )
    ));

@@ -177,7 +178,8 @@ class SubscriberTest extends MailPoetTest {
      'name' => 'Registered on',
      'type' => 'date',
      'params' => array(
-        'date_type' => 'year_month'
+        'date_type' => 'year_month',
+        'date_format' => 'MM/YYYY'
      )
    ));

@@ -199,7 +201,7 @@ class SubscriberTest extends MailPoetTest {
    expect($subscriber->email)->equals('user.with.cf@mailpoet.com');
    expect($subscriber->{'cf_'.$custom_field->id})->equals('Paris');
    // date specified as array gets converted to string
-    expect($subscriber->{'cf_'.$custom_field_2->id})->equals('1984-03-09');
+    expect($subscriber->{'cf_'.$custom_field_2->id})->equals('1984-03-09 00:00:00');
    // date specified as string is stored as is
    expect($subscriber->{'cf_'.$custom_field_3->id})->equals('2013-07');
  }
--- a/tests/unit/Subscribers/ImportExport/Import/ImportTest.php
+++ b/tests/unit/Subscribers/ImportExport/Import/ImportTest.php
@@ -1,5 +1,6 @@
 <?php

+use MailPoet\Models\CustomField;
 use MailPoet\Models\Subscriber;
 use MailPoet\Models\Segment;
 use MailPoet\Models\SubscriberCustomField;
@@ -9,6 +10,13 @@ use MailPoet\Util\Helpers;

 class ImportTest extends MailPoetTest {
  function _before() {
+    $custom_field = CustomField::create();
+    $custom_field->name = 'country';
+    $custom_field->type = 'text';
+    $custom_field->save();
+    $this->subscriber_custom_fields = array((string)$custom_field->id);
+    $this->segment_1 = Segment::createOrUpdate(array('name' => 'Segment 1'));
+    $this->segment_2 = Segment::createOrUpdate(array('name' => 'Segment 2'));
    $this->data = array(
      'subscribers' => array(
        array(
@@ -25,13 +33,13 @@ class ImportTest extends MailPoetTest {
        )
      ),
      'columns' => array(
-        'first_name' => 0,
-        'last_name' => 1,
-        'email' => 2,
-        777 => 3
+        'first_name' => array('index' => 0),
+        'last_name' => array('index' => 1),
+        'email' => array('index' => 2),
+        (string)$custom_field->id => array('index' => 3)
      ),
      'segments' => array(
-        195
+        $this->segment_1->id
      ),
      'timestamp' => time(),
      'updateSubscribers' => true
@@ -41,10 +49,6 @@ class ImportTest extends MailPoetTest {
      'last_name',
      'email'
    );
-    $this->segment_1 = Segment::createOrUpdate(array('name' => 'Segment 1'));
-    $this->segment_2 = Segment::createOrUpdate(array('name' => 'Segment 2'));
-
-    $this->subscriber_custom_fields = array(777);
    $this->import = new Import($this->data);
    $this->subscribers_data = $this->import->transformSubscribersData(
      $this->data['subscribers'],
@@ -63,13 +67,14 @@ class ImportTest extends MailPoetTest {
  }

  function testItCanTransformSubscribers() {
+    $custom_field = $this->subscriber_custom_fields[0];
    expect($this->import->subscribers_data['first_name'][0])
      ->equals($this->data['subscribers'][0][0]);
    expect($this->import->subscribers_data['last_name'][0])
      ->equals($this->data['subscribers'][0][1]);
    expect($this->import->subscribers_data['email'][0])
      ->equals($this->data['subscribers'][0][2]);
-    expect($this->import->subscribers_data['777'][0])
+    expect($this->import->subscribers_data[$custom_field][0])
      ->equals($this->data['subscribers'][0][3]);
  }

@@ -242,6 +247,7 @@ class ImportTest extends MailPoetTest {

  function testItCanCreateOrUpdateCustomFields() {
    $subscribers_data = $this->subscribers_data;
+    $custom_field = $this->subscriber_custom_fields[0];
    $this->import->createOrUpdateSubscribers(
      'create',
      $subscribers_data,
@@ -266,8 +272,8 @@ class ImportTest extends MailPoetTest {
    $subscriber_custom_fields = SubscriberCustomField::findArray();
    expect(count($subscriber_custom_fields))->equals(2);
    expect($subscriber_custom_fields[0]['value'])
-      ->equals($subscribers_data[777][0]);
-    $subscribers_data[777][1] = 'Rio';
+      ->equals($subscribers_data[$custom_field][0]);
+    $subscribers_data[$custom_field][1] = 'Rio';
    $this->import->createOrUpdateCustomFields(
      'update',
      $db_subscribers,
@@ -276,7 +282,7 @@ class ImportTest extends MailPoetTest {
    );
    $subscriber_custom_fields = SubscriberCustomField::findArray();
    expect($subscriber_custom_fields[1]['value'])
-      ->equals($subscribers_data[777][1]);
+      ->equals($subscribers_data[$custom_field][1]);
  }


@@ -345,6 +351,7 @@ class ImportTest extends MailPoetTest {
    ORM::raw_execute('TRUNCATE ' . Subscriber::$_table);
    ORM::raw_execute('TRUNCATE ' . Segment::$_table);
    ORM::raw_execute('TRUNCATE ' . SubscriberSegment::$_table);
+    ORM::raw_execute('TRUNCATE ' . CustomField::$_table);
    ORM::raw_execute('TRUNCATE ' . SubscriberCustomField::$_table);
  }
 }
--- a/views/subscribers/importExport/import/step2.html
+++ b/views/subscribers/importExport/import/step2.html
@@ -92,7 +92,7 @@
      {{#show_and_match_columns .}}
      {{#.}}
      <th>
-        <select class="mailpoet_subscribers_column_data_match" id="{{column_id}}" data-column-id="{{column_id}}" id="column_{{@index}}">
+        <select class="mailpoet_subscribers_column_data_match" data-column-id="{{column_id}}" data-validation-rule="false" id="column_{{@index}}">
      </th>
      {{/.}}
      {{/show_and_match_columns}}