From a5d96f1534203f10c68d57e0305ff21d1f6eb589 Mon Sep 17 00:00:00 2001
From: Jonathan Labreuille <jonathan@mailpoet.com>
Date: Wed, 25 Nov 2015 18:31:57 +0100
Subject: [PATCH] WP Users list - refactored and fixed listing issues (related
 to Segments) - removed bulk actions from segments - added synchronize methods
 for WP users - Update action in segments only for WP Users list - added
 "type" column to segments (default, wp_users, dynamic...) - added "status"
 column to subscriber_segment table (useful soon)

---
 assets/js/src/forms/list.jsx       |  3 ++
 assets/js/src/listing/listing.jsx  | 49 ++++++++++++---------------
 assets/js/src/newsletters/list.jsx |  3 ++
 assets/js/src/segments/list.jsx    | 37 ++++++++++++---------
 assets/js/src/subscribers/list.jsx | 16 +++++++++
 lib/Config/Hooks.php               | 12 +++----
 lib/Config/Migrator.php            | 30 ++---------------
 lib/Models/Segment.php             | 15 ++++++++-
 lib/Router/Segments.php            | 25 ++++++--------
 lib/{Filters => Segments}/WP.php   | 53 ++++++++++++++++++++++++------
 views/form/editor.html             | 14 +++++---
 11 files changed, 150 insertions(+), 107 deletions(-)
 rename lib/{Filters => Segments}/WP.php (61%)

diff --git a/assets/js/src/forms/list.jsx b/assets/js/src/forms/list.jsx
index c11a6b1f72..b9a62fcf45 100644
--- a/assets/js/src/forms/list.jsx
+++ b/assets/js/src/forms/list.jsx
@@ -105,6 +105,9 @@ const item_actions = [
         refresh();
       });
     }
+  },
+  {
+    name: 'trash'
   }
 ];
 
diff --git a/assets/js/src/listing/listing.jsx b/assets/js/src/listing/listing.jsx
index 804f13c82b..d7495133b1 100644
--- a/assets/js/src/listing/listing.jsx
+++ b/assets/js/src/listing/listing.jsx
@@ -79,33 +79,48 @@ define(
 
         if(custom_actions.length > 0) {
           item_actions = custom_actions.map(function(action, index) {
-            if(action.onFilter !== undefined) {
-              if(action.onFilter(this.props.item) === false) {
+            if(action.display !== undefined) {
+              if(action.display(this.props.item) === false) {
                 return;
               }
             }
 
-            if(action.refresh) {
+            if(action.name === 'trash') {
+              return (
+                <span key={ 'action-'+index } className="trash">
+                  {(index > 0) ? ' | ' : ''}
+                  <a
+                    href="javascript:;"
+                    onClick={ this.handleTrashItem.bind(
+                      null,
+                      this.props.item.id
+                    ) }>
+                    Trash
+                  </a>
+                </span>
+              );
+            } else if(action.refresh) {
               return (
                 <span
                   onClick={ this.props.onRefreshItems }
                   key={ 'action-'+index } className={ action.name }>
+                  {(index > 0) ? ' | ' : ''}
                   { action.link(this.props.item) }
-                  {(index < (custom_actions.length - 1)) ? ' | ' : ''}
                 </span>
               );
             } else if(action.link) {
               return (
                 <span
                   key={ 'action-'+index } className={ action.name }>
+                  {(index > 0) ? ' | ' : ''}
                   { action.link(this.props.item) }
-                  {(index < (custom_actions.length - 1)) ? ' | ' : ''}
                 </span>
               );
             } else {
               return (
                 <span
                   key={ 'action-'+index } className={ action.name }>
+                  {(index > 0) ? ' | ' : ''}
                   <a href="javascript:;" onClick={
                     (action.onClick !== undefined)
                     ? action.onClick.bind(null,
@@ -114,7 +129,6 @@ define(
                       )
                     : false
                   }>{ action.label }</a>
-                  {(index < (custom_actions.length - 1)) ? ' | ' : ''}
                 </span>
               );
             }
@@ -127,12 +141,6 @@ define(
           );
         }
 
-        var is_readonly = (
-          (this.props.onFilterItem !== undefined)
-          ? this.props.onFilterItem(this.props.item)
-          : false
-        );
-
         if(this.props.group === 'trash') {
            var actions = (
             <div>
@@ -170,19 +178,6 @@ define(
             <div>
               <div className="row-actions">
                 { item_actions }
-                { (is_readonly) ? '' : (
-                  <span className="trash">
-                    { ' | ' }
-                    <a
-                      href="javascript:;"
-                      onClick={ this.handleTrashItem.bind(
-                        null,
-                        this.props.item.id
-                      ) }>
-                      Trash
-                    </a>
-                  </span>
-                )}
               </div>
               <button
                 onClick={ this.handleToggleItem.bind(null, this.props.item.id) }
@@ -272,7 +267,6 @@ define(
                     columns={ this.props.columns }
                     onSelectItem={ this.props.onSelectItem }
                     onRenderItem={ this.props.onRenderItem }
-                    onFilterItem={ this.props.onFilterItem }
                     onDeleteItem={ this.props.onDeleteItem }
                     onRestoreItem={ this.props.onRestoreItem }
                     onTrashItem={ this.props.onTrashItem }
@@ -652,7 +646,7 @@ define(
         // bulk actions
         var bulk_actions = this.props.bulk_actions || [];
 
-        if(this.state.group === 'trash') {
+        if(this.state.group === 'trash' && bulk_actions.length > 0) {
           bulk_actions = [
             {
               name: 'restore',
@@ -736,7 +730,6 @@ define(
 
               <ListingItems
                 onRenderItem={ this.handleRenderItem }
-                onFilterItem={ this.props.onFilterItem }
                 onDeleteItem={ this.handleDeleteItem }
                 onRestoreItem={ this.handleRestoreItem }
                 onTrashItem={ this.handleTrashItem }
diff --git a/assets/js/src/newsletters/list.jsx b/assets/js/src/newsletters/list.jsx
index bc17b093b1..603d0e72a0 100644
--- a/assets/js/src/newsletters/list.jsx
+++ b/assets/js/src/newsletters/list.jsx
@@ -112,6 +112,9 @@ define(
             </a>
           );
         }
+      },
+      {
+        name: 'trash'
       }
     ];
 
diff --git a/assets/js/src/segments/list.jsx b/assets/js/src/segments/list.jsx
index b952074f64..981e8b684c 100644
--- a/assets/js/src/segments/list.jsx
+++ b/assets/js/src/segments/list.jsx
@@ -122,19 +122,29 @@ const item_actions = [
         refresh();
       });
     },
-    onFilter: function(segment) {
-      return (segment.filters.length === 0);
+    display: function(segment) {
+      return (segment.type !== 'wp_users');
     }
   },
   {
-    name: 'sync_segment',
+    name: 'synchronize_segment',
     label: 'Update',
     className: 'update',
     onClick: function(item, refresh) {
-      console.log(item);
+      return MailPoet.Ajax.post({
+        endpoint: 'segments',
+        action: 'synchronize'
+      }).done(function(response) {
+        if(response === true) {
+          MailPoet.Notice.success(
+            ('List "%$1s" has been synchronized.').replace('%$1s', item.name)
+          );
+          refresh();
+        }
+      });
     },
-    onFilter: function(segment) {
-      return (segment.filters.length > 0);
+    display: function(segment) {
+      return (segment.type === 'wp_users');
     }
   },
   {
@@ -144,21 +154,19 @@ const item_actions = [
         <a href={ item.subscribers_url }>View subscribers</a>
       );
     }
+  },
+  {
+    name: 'trash',
+    display: function(segment) {
+      return (segment.type !== 'wp_users');
+    }
   }
 ];
 
 const bulk_actions = [
-  {
-    name: 'trash',
-    label: 'Trash',
-    onSuccess: messages.onTrash
-  }
 ];
 
 const SegmentList = React.createClass({
-  filterItem: function(segment) {
-    return !!(segment.filters.length > 0);
-  },
   renderItem: function(segment, actions) {
     var rowClasses = classNames(
       'manage-column',
@@ -206,7 +214,6 @@ const SegmentList = React.createClass({
           limit={ 1000 }
           endpoint="segments"
           onRenderItem={ this.renderItem }
-          onFilterItem={ this.filterItem }
           columns={ columns }
           bulk_actions={ bulk_actions }
           item_actions={ item_actions }
diff --git a/assets/js/src/subscribers/list.jsx b/assets/js/src/subscribers/list.jsx
index 9bde39a23d..0b4d454d0b 100644
--- a/assets/js/src/subscribers/list.jsx
+++ b/assets/js/src/subscribers/list.jsx
@@ -206,6 +206,21 @@ const bulk_actions = [
   }
 ];
 
+const item_actions = [
+  {
+    name: 'edit',
+    label: 'Edit',
+    link: function(item) {
+      return (
+        <Link to={ `/edit/${item.id}` }>Edit</Link>
+      );
+    }
+  },
+  {
+    name: 'trash'
+  }
+];
+
 const SubscriberList = React.createClass({
   renderItem: function(subscriber, actions) {
     let row_classes = classNames(
@@ -295,6 +310,7 @@ const SubscriberList = React.createClass({
           onRenderItem={ this.renderItem }
           columns={ columns }
           bulk_actions={ bulk_actions }
+          item_actions={ item_actions }
           messages={ messages }
           onGetItems={ this.onGetItems }
         />
diff --git a/lib/Config/Hooks.php b/lib/Config/Hooks.php
index f61868246a..a23444ce77 100644
--- a/lib/Config/Hooks.php
+++ b/lib/Config/Hooks.php
@@ -9,33 +9,33 @@ class Hooks {
     // WP Users synchronization
     add_action(
       'user_register',
-      '\MailPoet\Filters\WP::synchronizeUser',
+      '\MailPoet\Segments\WP::synchronizeUser',
       1
     );
     add_action(
       'added_existing_user',
-      '\MailPoet\Filters\WP::synchronizeUser',
+      '\MailPoet\Segments\WP::synchronizeUser',
       1
     );
     add_action(
       'profile_update',
-      '\MailPoet\Filters\WP::synchronizeUser',
+      '\MailPoet\Segments\WP::synchronizeUser',
       1
     );
     add_action(
       'delete_user',
-      '\MailPoet\Filters\WP::synchronizeUser',
+      '\MailPoet\Segments\WP::synchronizeUser',
       1
     );
     // multisite
     add_action(
       'deleted_user',
-      '\MailPoet\Filters\WP::synchronizeUser',
+      '\MailPoet\Segments\WP::synchronizeUser',
       1
     );
     add_action(
       'remove_user_from_blog',
-      '\MailPoet\Filters\WP::synchronizeUser',
+      '\MailPoet\Segments\WP::synchronizeUser',
       1
     );
   }
diff --git a/lib/Config/Migrator.php b/lib/Config/Migrator.php
index cc4f36753c..25f5051995 100644
--- a/lib/Config/Migrator.php
+++ b/lib/Config/Migrator.php
@@ -15,8 +15,6 @@ class Migrator {
       'newsletters',
       'newsletter_templates',
       'segments',
-      'filters',
-      'segment_filter',
       'subscriber_segment',
       'newsletter_segment',
       'custom_fields',
@@ -111,6 +109,7 @@ class Migrator {
     $attributes = array(
       'id mediumint(9) NOT NULL AUTO_INCREMENT,',
       'name varchar(90) NOT NULL,',
+      'type varchar(90) NOT NULL DEFAULT "default",',
       'description varchar(250) NOT NULL,',
       'created_at TIMESTAMP NOT NULL DEFAULT 0,',
       'deleted_at TIMESTAMP NULL DEFAULT NULL,',
@@ -121,37 +120,12 @@ class Migrator {
     return $this->sqlify(__FUNCTION__, $attributes);
   }
 
-  function filters() {
-    $attributes = array(
-      'id mediumint(9) NOT NULL AUTO_INCREMENT,',
-      'name varchar(90) NOT NULL,',
-      'created_at TIMESTAMP NOT NULL DEFAULT 0,',
-      'updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,',
-      'PRIMARY KEY  (id),',
-      'UNIQUE KEY name (name)'
-    );
-    return $this->sqlify(__FUNCTION__, $attributes);
-  }
-
-  function segment_filter() {
-    $attributes = array(
-      'id mediumint(9) NOT NULL AUTO_INCREMENT,',
-      'segment_id mediumint(9) NOT NULL,',
-      'filter_id mediumint(9) NOT NULL,',
-      'params longtext NULL,',
-      'created_at TIMESTAMP NOT NULL DEFAULT 0,',
-      'updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,',
-      'PRIMARY KEY  (id),',
-      'UNIQUE KEY segment_filter (segment_id,filter_id)'
-    );
-    return $this->sqlify(__FUNCTION__, $attributes);
-  }
-
   function subscriber_segment() {
     $attributes = array(
       'id mediumint(9) NOT NULL AUTO_INCREMENT,',
       'subscriber_id mediumint(9) NOT NULL,',
       'segment_id mediumint(9) NOT NULL,',
+      'status varchar(12) NOT NULL DEFAULT "subscribed",',
       'created_at TIMESTAMP NOT NULL DEFAULT 0,',
       'updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,',
       'PRIMARY KEY  (id),',
diff --git a/lib/Models/Segment.php b/lib/Models/Segment.php
index 6d92eb19d6..364b253e8f 100644
--- a/lib/Models/Segment.php
+++ b/lib/Models/Segment.php
@@ -77,7 +77,20 @@ class Segment extends Model {
   }
 
   static function getWPUsers() {
-    return self::findOne(1);
+    $segment = self::where('type', 'wp_users')->findOne();
+
+    if($segment === false) {
+      // create the wp users list
+      $segment = self::create();
+      $segment->hydrate(array(
+        'name' => __('WordPress Users'),
+        'type' => 'wp_users'
+      ));
+      $segment->save();
+      return self::findOne($segment->id());
+    }
+
+    return $segment;
   }
 
   static function search($orm, $search = '') {
diff --git a/lib/Router/Segments.php b/lib/Router/Segments.php
index 97c1a9103e..00b3490761 100644
--- a/lib/Router/Segments.php
+++ b/lib/Router/Segments.php
@@ -4,6 +4,7 @@ use \MailPoet\Models\Segment;
 use \MailPoet\Models\SubscriberSegment;
 use \MailPoet\Models\SegmentFilter;
 use \MailPoet\Listing;
+use \MailPoet\Segments\WP;
 
 if(!defined('ABSPATH')) exit;
 
@@ -32,18 +33,6 @@ class Segments {
 
     // fetch segments relations for each returned item
     foreach($listing_data['items'] as &$item) {
-      $item['filters'] = SegmentFilter::table_alias('relation')
-        ->where(
-          'relation.segment_id',
-          $item['id']
-        )
-        ->join(
-          MP_FILTERS_TABLE,
-          'filters.id = relation.filter_id',
-          'filters'
-        )
-        ->findArray();
-
       $stats = SubscriberSegment::table_alias('relation')
         ->where(
           'relation.segment_id',
@@ -55,15 +44,15 @@ class Segments {
           'subscribers'
         )
         ->select_expr(
-          'SUM(CASE status WHEN "subscribed" THEN 1 ELSE 0 END)',
+          'SUM(CASE subscribers.status WHEN "subscribed" THEN 1 ELSE 0 END)',
           'subscribed'
         )
         ->select_expr(
-          'SUM(CASE status WHEN "unsubscribed" THEN 1 ELSE 0 END)',
+          'SUM(CASE subscribers.status WHEN "unsubscribed" THEN 1 ELSE 0 END)',
           'unsubscribed'
         )
         ->select_expr(
-          'SUM(CASE status WHEN "unconfirmed" THEN 1 ELSE 0 END)',
+          'SUM(CASE subscribers.status WHEN "unconfirmed" THEN 1 ELSE 0 END)',
           'unconfirmed'
         )
         ->findOne()->asArray();
@@ -148,6 +137,12 @@ class Segments {
     wp_send_json($result);
   }
 
+  function synchronize() {
+    $result = WP::synchronizeUsers();
+
+    wp_send_json($result);
+  }
+
   function bulkAction($data = array()) {
     $bulk_action = new Listing\BulkAction(
       '\MailPoet\Models\Segment',
diff --git a/lib/Filters/WP.php b/lib/Segments/WP.php
similarity index 61%
rename from lib/Filters/WP.php
rename to lib/Segments/WP.php
index eb76fd5626..adb98c7a51 100644
--- a/lib/Filters/WP.php
+++ b/lib/Segments/WP.php
@@ -1,8 +1,7 @@
 <?php
-namespace MailPoet\Filters;
+namespace MailPoet\Segments;
 use \MailPoet\Models\Subscriber;
 use \MailPoet\Models\Segment;
-use \MailPoet\Models\SubscriberSegment;
 
 class WP {
   static function synchronizeUser($wp_user_id) {
@@ -13,9 +12,18 @@ class WP {
       ->findOne();
 
     switch(current_filter()) {
+      case 'delete_user':
+      case 'deleted_user':
+      case 'remove_user_from_blog':
+        if($subscriber !== false && $subscriber->id()) {
+          $subscriber->delete();
+        }
+      break;
+
       case 'user_register':
       case 'added_existing_user':
       case 'profile_update':
+      default:
         // get first name & last name
         $first_name = $wpUser->first_name;
         $last_name = $wpUser->last_name;
@@ -42,14 +50,39 @@ class WP {
           $segment->addSubscriber($subscriber->id());
         }
       break;
-
-      case 'delete_user':
-      case 'deleted_user':
-      case 'remove_user_from_blog':
-        if($subscriber !== false && $subscriber->id()) {
-          $subscriber->delete();
-        }
-      break;
     }
   }
+
+  static function synchronizeUsers() {
+    // get wordpress users list
+    $segment = Segment::getWPUsers();
+
+    // count WP users
+    $users_count = \count_users()['total_users'];
+    $linked_subscribers_count = $segment->subscribers()->count();
+
+    if($users_count !== $linked_subscribers_count) {
+      $linked_subscribers = Subscriber::select('wp_user_id')
+        ->whereNotNull('wp_user_id')
+        ->findArray();
+
+      $exclude_ids = array();
+      if(!empty($linked_subscribers)) {
+        $exclude_ids = array_map(function($subscriber) {
+          return $subscriber['wp_user_id'];
+        }, $linked_subscribers);
+      }
+
+      $users = \get_users(array(
+        'count_total'  => false,
+        'fields' => 'ID',
+        'exclude' => $exclude_ids
+      ));
+
+      foreach($users as $user) {
+        static::synchronizeUser($user);
+      }
+    }
+    return true;
+  }
 }
\ No newline at end of file
diff --git a/views/form/editor.html b/views/form/editor.html
index cf39d8a946..31fca1db67 100644
--- a/views/form/editor.html
+++ b/views/form/editor.html
@@ -4,9 +4,15 @@
   <h2 class="title">
     <span id="mailpoet_form_name"><%= form.name %></span>
     <input id="mailpoet_form_name_input" type="text" value="" style="display:none;" />
-    <span>
-      <a id="mailpoet_form_edit_name" class="button" href="javascript:;"><%= __('Edit name' ) %></a>
-    </span>
+    <a
+      id="mailpoet_form_edit_name"
+      class="add-new-h2"
+      href="javascript:;"
+    ><%= __('Edit name' ) %></a>
+    <a
+      href="<%= admin_url('admin.php?page=mailpoet-forms') | raw %>"
+      class="add-new-h2"
+    ><%= __('List of forms' ) %></a>
   </h2>
 <% endblock %>
 
@@ -42,7 +48,7 @@
             <div id="mailpoet_settings_segment_selection">
               <!-- Form settings: list selection -->
               <p>
-              <strong><%= __('This form adds subscribers to these lists:') %></strong>
+                <strong><%= __('This form adds subscribers to these lists:') %></strong>
               </p>
               <select
                 id="mailpoet_form_segments"