Checking invalid emails on the input [MAILPOET-1288]

assets/js/src/newsletters/send/notification.jsx

@@ -92,6 +92,9 @@ define(
             name: 'reply_to_address',
             type: 'text',
             placeholder: MailPoet.I18n.t('replyToAddressPlaceholder'),
+            validation: {
+              'data-parsley-type': 'email',
+            },
           },
         ],
       },

assets/js/src/newsletters/send/standard.jsx

@@ -398,6 +398,9 @@ define(
             name: 'reply_to_address',
             type: 'text',
             placeholder: MailPoet.I18n.t('replyToAddressPlaceholder'),
+            validation: {
+              'data-parsley-type': 'email',
+            },
           },
         ],
       },

assets/js/src/newsletters/send/welcome.jsx

@@ -65,6 +65,9 @@ define(
             name: 'reply_to_address',
             type: 'text',
             placeholder: MailPoet.I18n.t('replyToAddressPlaceholder'),
+            validation: {
+              'data-parsley-type': 'email',
+            },
           },
         ],
       },

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

@@ -209,7 +209,7 @@ define(
                       for (column in rowData) {
                         emailAddress = detectAndCleanupEmail(rowData[column]);
                         if (emailColumnPosition === null
-                          && window.emailRegex.test(emailAddress)) {
+                          && window.mailpoet_email_regex.test(emailAddress)) {
                           emailColumnPosition = column;
                           // add current e-mail to an object index
                           parsedEmails[emailAddress] = true;
@@ -229,7 +229,7 @@ define(
                       if (_.has(parsedEmails, email)) {
                         duplicateEmails.push(email);
                       }
-                      else if (!window.emailRegex.test(email)) {
+                      else if (!window.mailpoet_email_regex.test(email)) {
                         invalidEmails.push(rowData[emailColumnPosition]);
                       }
                       // if we haven't yet processed this e-mail and it passed
@@ -257,7 +257,7 @@ define(
                   // since we assume that the header line is always present, we need
                   // to detect the header by checking if it contains a valid e-mail address
                   window.importData.step1 = {
-                    header: (!window.emailRegex.test(
+                    header: (!window.mailpoet_email_regex.test(
                         processedSubscribers[0][emailColumnPosition])
                     ) ? processedSubscribers.shift() : null,
                     subscribers: processedSubscribers,
@@ -690,7 +690,7 @@ define(
                   columnData = helperSubscribers.subscribers[0][i];
                   columnId = 'ignore'; // set default column type
                   // if the column is not undefined and has a valid e-mail, set type as email
-                  if (columnData % 1 !== 0 && window.emailRegex.test(columnData)) {
+                  if (columnData % 1 !== 0 && window.mailpoet_email_regex.test(columnData)) {
                     columnId = 'email';
                   } else if (helperSubscribers.header) {
                     headerName = helperSubscribers.header[i];
@@ -791,7 +791,9 @@ define(
               // EMAIL filter: if the first value in the column doesn't have a valid
               // email, hide the next button
               if (column.id === 'email') {
-                if (!window.emailRegex.test(subscribersClone.subscribers[0][matchedColumn.index])) {
+                if (!window.mailpoet_email_regex.test(
+                  subscribersClone.subscribers[0][matchedColumn.index])
+                ) {
                   preventNextStep = true;
                   if (!jQuery('[data-id="notice_invalidEmail"]').length) {
                     MailPoet.Notice.error(MailPoet.I18n.t('columnContainsInvalidElement'), {

lib/Models/ModelValidator.php

@@ -32,7 +32,7 @@ class ModelValidator extends \Sudzy\Engine {
 
   function validateEmail($email) {
     $permitted_length = (strlen($email) >= self::EMAIL_MIN_LENGTH && strlen($email) <= self::EMAIL_MAX_LENGTH);
-    $valid_email = (is_email($email) !== false);
+    $valid_email = is_email($email) !== false && parent::_isEmail($email, null);
     return ($permitted_length && $valid_email);
   }
 

lib/Segments/WP.php

@@ -1,6 +1,7 @@
 <?php
 namespace MailPoet\Segments;
 
+use MailPoet\Models\ModelValidator;
 use MailPoet\Models\Subscriber;
 use MailPoet\Models\Segment;
 use MailPoet\Models\SubscriberSegment;
@@ -80,8 +81,9 @@ class WP {
 
   static function synchronizeUsers() {
 
-    self::updateSubscribersEmails();
+    $updated_users_emails = self::updateSubscribersEmails();
-    self::insertSubscribers();
+    $inserted_users_emails = self::insertSubscribers();
+    self::removeUpdatedSubscribersWithInvalidEmail(array_merge($updated_users_emails, $inserted_users_emails));
     self::removeFromTrash();
     self::updateFirstNames();
     self::updateLastNames();
@@ -92,20 +94,48 @@ class WP {
     return true;
   }
 
+  private static function removeUpdatedSubscribersWithInvalidEmail($updated_emails) {
+    $validator = new ModelValidator();
+    $invalid_wp_user_ids = array_map(function($item) {
+      return $item['id'];
+    },
+    array_filter($updated_emails, function($updated_email) use($validator) {
+      return !$validator->validateEmail($updated_email['email']);
+    }));
+    if(!$invalid_wp_user_ids) {
+      return;
+    }
+    \ORM::for_table(Subscriber::$_table)->whereIn('wp_user_id', $invalid_wp_user_ids)->delete_many();
+  }
+
   private static function updateSubscribersEmails() {
     global $wpdb;
     $subscribers_table = Subscriber::$_table;
+    $changed_email_user_ids = \ORM::for_table(Subscriber::$_table)->raw_query(sprintf(
+      'SELECT %1$s.wp_user_id as id, wu.user_email as email FROM %1$s
+        INNER JOIN %2$s AS wu ON %1$s.wp_user_id = wu.id
+        WHERE wu.user_email != %1$s.email
+      ', $subscribers_table, $wpdb->users))->findArray();
+
     Subscriber::raw_execute(sprintf('
       UPDATE IGNORE %1$s
-        JOIN %2$s as wu ON %1$s.wp_user_id = wu.id
+        INNER JOIN %2$s as wu ON %1$s.wp_user_id = wu.id
       SET %1$s.email = wu.user_email
-        WHERE %1$s.wp_user_id IS NOT NULL AND wu.user_email != ""
+        WHERE wu.user_email != %1$s.email AND wu.user_email != ""
     ', $subscribers_table, $wpdb->users));
+    return $changed_email_user_ids;
   }
 
   private static function insertSubscribers() {
     global $wpdb;
     $subscribers_table = Subscriber::$_table;
+
+    $inserterd_user_ids = \ORM::for_table($wpdb->users)->raw_query(sprintf(
+      'SELECT %2$s.id, %2$s.user_email as email FROM %2$s
+        LEFT JOIN %1$s AS mps ON mps.wp_user_id = %2$s.id
+        WHERE mps.wp_user_id IS NULL AND %2$s.user_email != ""
+      ', $subscribers_table, $wpdb->users))->findArray();
+
     Subscriber::raw_execute(sprintf('
       INSERT IGNORE INTO %1$s(wp_user_id, email, status, created_at)
         SELECT wu.id, wu.user_email, "subscribed", CURRENT_TIMESTAMP() FROM %2$s wu
@@ -113,6 +143,8 @@ class WP {
           WHERE mps.wp_user_id IS NULL AND wu.user_email != ""
       ON DUPLICATE KEY UPDATE wp_user_id = wu.id
     ', $subscribers_table, $wpdb->users));
+
+    return $inserterd_user_ids;
   }
 
   private static function updateFirstNames() {

lib/Subscribers/ImportExport/Import/Import.php

@@ -3,6 +3,7 @@ namespace MailPoet\Subscribers\ImportExport\Import;
 
 use MailPoet\Form\Block\Date;
 use MailPoet\Models\CustomField;
+use MailPoet\Models\ModelValidator;
 use MailPoet\Models\Newsletter;
 use MailPoet\Models\Subscriber;
 use MailPoet\Models\SubscriberCustomField;
@@ -148,12 +149,13 @@ class Import {
 
   function validateSubscribersData($subscribers_data, $validation_rules) {
     $invalid_records = array();
+    $validator = new ModelValidator();
     foreach($subscribers_data as $column => &$data) {
       $validation_rule = $validation_rules[$column];
       if($validation_rule === 'email') {
         $data = array_map(
-          function($index, $email) use(&$invalid_records) {
+          function($index, $email) use(&$invalid_records, $validator) {
-            if(!is_email($email)) {
+            if(!$validator->validateEmail($email)) {
               $invalid_records[] = $index;
             }
             return strtolower($email);

tests/unit/Segments/WPTest.php

@@ -54,13 +54,21 @@ class WPTest extends \MailPoetTest  {
     expect($subscriber->email)->equals('user-sync-test-xx@email.com');
   }
 
-  function testItDoesNotSynchronizeEmptyEmailsForExistingUsers() {
+  function testRemovesUsersWithEmptyEmailsFromSunscribersDuringSynchronization() {
     $id = $this->insertUser();
     WP::synchronizeUsers();
     $this->updateWPUserEmail($id, '');
     WP::synchronizeUsers();
-    $subscriber = Subscriber::where('wp_user_id', $id)->findOne();
+    expect(Subscriber::where('wp_user_id', $id)->count())->equals(0);
-    expect($subscriber->email)->notEmpty();
+    $this->deleteWPUser($id);
+  }
+
+  function testRemovesUsersWithInvalidEmailsFromSunscribersDuringSynchronization() {
+    $id = $this->insertUser();
+    WP::synchronizeUsers();
+    $this->updateWPUserEmail($id, 'ivalid.@email.com');
+    WP::synchronizeUsers();
+    expect(Subscriber::where('wp_user_id', $id)->count())->equals(0);
     $this->deleteWPUser($id);
   }
 

views/layout.html

@@ -48,6 +48,8 @@ jQuery('.toplevel_page_mailpoet-newsletters.menu-top-last')
   var mailpoet_premium_version = <%= json_encode(mailpoet_premium_version()) %>;
   var mailpoet_analytics_enabled = <%= is_analytics_enabled() | json_encode %>;
   var mailpoet_analytics_data = <%= json_encode(get_analytics_data()) %>;
+  // RFC 5322 standard; http://emailregex.com/ combined with https://google.github.io/closure-library/api/goog.format.EmailAddress.html#isValid
+  var mailpoet_email_regex = /(?=^[+a-zA-Z0-9_.!#$%&'*\/=?^`{|}~-]+@([a-zA-Z0-9-]+\.)+[a-zA-Z0-9]{2,63}$)(?=^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,})))/;
 </script>
 
 <!-- javascripts -->

views/settings.html

@@ -64,6 +64,17 @@
       $(function() {
         // on form submission
         $('#mailpoet_settings_form').on('submit', function() {
+          // Check if filled emails are valid
+          var invalidEmails = $.map($('#mailpoet_settings_form')[0].elements, function(el) {
+            return el.type === 'email' && el.value && !window.mailpoet_email_regex.test(el.value) ? el.value : null;
+          }).filter(function(val) { return !!val; });
+          if (invalidEmails.length) {
+            MailPoet.Notice.error(
+              "<%= __('Invalid email addresses: ') | escape('js') %>" + invalidEmails.join(', '),
+              { scroll: true }
+            );
+            return false;
+          }
           // if reCAPTCHA is enabled but keys are emty, show error
           var enabled = $('input[name="re_captcha[enabled]"]:checked').val(),
               site_key = $('input[name="re_captcha[site_token]"]').val().trim(),

views/subscribers/importExport/import.html

@@ -24,9 +24,7 @@
     importData = {},
     mailpoetColumnsSelect2 = <%= subscriberFieldsSelect2|raw %>,
     mailpoetColumns = <%= subscriberFields|raw %>,
-    mailpoetSegments = <%= segments|raw %>,
+    mailpoetSegments = <%= segments|raw %>
-    // RFC 5322 standard; http://emailregex.com/
-    emailRegex = /^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
 </script>
 <% endblock %>