Renamed method names for better clarity + refactoring

- renamed getWPUsers() to getWPSegment()
- renamed SubscriberSegment methods

lib/Config/Populator.php

@@ -109,18 +109,18 @@ class Populator {
 
   private function createDefaultSegments() {
     // WP Users segment
-    $wp_users_segment = Segment::getWPUsers();
+    $wp_segment = Segment::getWPSegment();
 
-    if($wp_users_segment === false) {
+    if($wp_segment === false) {
       // create the wp users list
-      $wp_users_segment = Segment::create();
-      $wp_users_segment->hydrate(array(
+      $wp_segment = Segment::create();
+      $wp_segment->hydrate(array(
         'name' => __('WordPress Users'),
         'description' =>
           __('The list containing all of your WordPress users.'),
         'type' => 'wp_users'
       ));
-      $wp_users_segment->save();
+      $wp_segment->save();
     }
 
     // Synchronize WP Users

lib/Models/Segment.php

@@ -93,7 +93,7 @@ class Segment extends Model {
     return $this;
   }
 
-  static function getWPUsers() {
+  static function getWPSegment() {
     return self::where('type', 'wp_users')->findOne();
   }
 

lib/Models/Subscriber.php

@@ -43,7 +43,7 @@ class Subscriber extends Model {
 
   function delete() {
     // WP Users cannot be deleted
-    if($this->wp_user_id !== null) {
+    if($this->isWPUser()) {
       return false;
     } else {
       // delete all relations to segments
@@ -55,13 +55,17 @@ class Subscriber extends Model {
 
   function trash() {
     // WP Users cannot be trashed
-    if($this->wp_user_id !== null) {
+    if($this->isWPUser()) {
       return false;
     } else {
       return parent::trash();
     }
   }
 
+  function isWPUser() {
+    return ($this->wp_user_id !== null);
+  }
+
   function sendConfirmationEmail() {
     if($this->status === self::STATUS_UNCONFIRMED) {
       $signup_confirmation = Setting::getValue('signup_confirmation');
@@ -173,7 +177,7 @@ class Subscriber extends Model {
 
       if($subscriber->save()) {
         // link subscriber to segments
-        SubscriberSegment::addSubscriptions($subscriber, $segment_ids);
+        SubscriberSegment::subscribeToSegments($subscriber, $segment_ids);
 
         // signup confirmation
         if($subscriber->status !== self::STATUS_SUBSCRIBED) {
@@ -435,10 +439,10 @@ class Subscriber extends Model {
           ($new_status === self::STATUS_UNSUBSCRIBED)
       ) {
         // make sure we unsubscribe the user from all segments
-        SubscriberSegment::removeSubscriptions($subscriber);
+        SubscriberSegment::unsubscribeFromSegments($subscriber);
       } else {
         if($segment_ids !== false) {
-          SubscriberSegment::setSubscriptions($subscriber, $segment_ids);
+          SubscriberSegment::resetSubscriptions($subscriber, $segment_ids);
         }
       }
     }
@@ -502,11 +506,8 @@ class Subscriber extends Model {
       $subscribers_count = 0;
       $subscribers = $orm->findResultSet();
       foreach($subscribers as $subscriber) {
-        try {
-          SubscriberSegment::setSubscriptions($subscriber, array($segment->id));
-          $subscribers_count++;
-        } catch(Exception $e) {
-        }
+        SubscriberSegment::resetSubscriptions($subscriber, array($segment->id));
+        $subscribers_count++;
       }
       return array(
         'subscribers' => $subscribers_count,
@@ -526,11 +527,8 @@ class Subscriber extends Model {
       // delete relations with segment
       $subscribers = $orm->findResultSet();
       foreach($subscribers as $subscriber) {
-        try {
-          SubscriberSegment::removeSubscriptions($subscriber, array($segment->id));
-          $subscribers_count++;
-        } catch(Exception $e) {
-        }
+        SubscriberSegment::unsubscribeFromSegments($subscriber, array($segment->id));
+        $subscribers_count++;
       }
 
       return array(
@@ -545,12 +543,8 @@ class Subscriber extends Model {
     $subscribers = $orm->findResultSet();
     $subscribers_count = 0;
     foreach($subscribers as $subscriber) {
-      try {
-        SubscriberSegment::removeSubscriptions($subscriber);
-        $subscribers_count++;
-      } catch(Exception $e) {
-        continue;
-      }
+      SubscriberSegment::unsubscribeFromSegments($subscriber);
+      $subscribers_count++;
     }
 
     return $subscribers_count;
@@ -582,7 +576,7 @@ class Subscriber extends Model {
       $subscribers = $orm->findMany();
       foreach($subscribers as $subscriber) {
         try {
-          SubscriberSegment::addSubscriptions($subscriber, array($segment->id));
+          SubscriberSegment::subscribeToSegments($subscriber, array($segment->id));
           $subscribers_count++;
         } catch(Exception $e) {
         }
@@ -609,20 +603,12 @@ class Subscriber extends Model {
   }
 
   static function bulkDelete($orm) {
-    return parent::bulkAction($orm, function($ids) {
-      $wp_users_segment = Segment::getWPUsers();
-
-      // delete subscribers' relations to segments (except WP Users' segment)
-      $subscriptions = SubscriberSegment::whereIn('subscriber_id', $ids);
-      if($wp_users_segment !== false) {
-        $subscriptions = $subscriptions->whereNotEqual(
-          'segment_id', $wp_users_segment->id
-        );
-      }
-      $subscriptions->deleteMany();
+    return parent::bulkAction($orm, function($subscriber_ids) {
+      // delete all subscriber/segment relationships
+      SubscriberSegment::deleteSubscriptionsForAll($subscriber_ids);
 
       // delete subscribers (except WP Users)
-      Subscriber::whereIn('id', $ids)
+      Subscriber::whereIn('id', $subscriber_ids)
         ->whereNull('wp_user_id')
         ->deleteMany();
     });

lib/Models/SubscriberSegment.php

@@ -16,18 +16,19 @@ class SubscriberSegment extends Model {
     return $this->has_one(__NAMESPACE__.'\Subscriber', 'id', 'subscriber_id');
   }
 
-  static function removeSubscriptions($subscriber, $segment_ids = array()) {
-    $wp_users_segment = Segment::getWPUsers();
+  static function unsubscribeFromSegments($subscriber, $segment_ids = array()) {
+    if($subscriber !== false && $subscriber->id > 0) {
+
+      $wp_segment = Segment::getWPSegment();
 
-    if($subscriber->id > 0) {
       if(!empty($segment_ids)) {
         // unsubscribe from segments
         foreach($segment_ids as $segment_id) {
 
           // do not remove subscriptions to the WP Users segment
           if(
-            $wp_users_segment !== false
-            && ($wp_users_segment->id === (int)$segment_id)
+            $wp_segment !== false
+            && ($wp_segment->id === (int)$segment_id)
           ) {
             continue;
           }
@@ -43,22 +44,23 @@ class SubscriberSegment extends Model {
         return true;
       } else {
         // unsubscribe from all segments (except the WP users segment)
-        $subscritpions = SubscriberSegment::where('subscriber_id', $subscriber->id);
+        $subscriptions = self::where('subscriber_id', $subscriber->id);
 
-        if($wp_users_segment !== false) {
-          $subscritpions = $subscritpions->whereNotEqual(
-            'segment_id', $wp_users_segment->id
+        if($wp_segment !== false) {
+          $subscriptions = $subscriptions->whereNotEqual(
+            'segment_id', $wp_segment->id
           );
         }
 
-        $subscritpions->findResultSet()
+        return $subscriptions->findResultSet()
           ->set('status', Subscriber::STATUS_UNSUBSCRIBED)
           ->save();
       }
     }
+    return false;
   }
 
-  static function addSubscriptions($subscriber, $segment_ids = array()) {
+  static function subscribeToSegments($subscriber, $segment_ids = array()) {
     if($subscriber->id > 0) {
       if(!empty($segment_ids)) {
         // subscribe to segments
@@ -75,17 +77,51 @@ class SubscriberSegment extends Model {
         return true;
       } else {
         // subscribe to all segments
-        return SubscriberSegment::where('subscriber_id', $subscriber->id)
+        return self::where('subscriber_id', $subscriber->id)
           ->findResultSet()
           ->set('status', Subscriber::STATUS_SUBSCRIBED)
           ->save();
       }
     }
+    return false;
   }
 
-  static function setSubscriptions($subscriber, $segment_ids = array()) {
-    self::removeSubscriptions($subscriber);
-    self::addSubscriptions($subscriber, $segment_ids);
+  static function resetSubscriptions($subscriber, $segment_ids = array()) {
+    self::unsubscribeFromSegments($subscriber);
+    return self::subscribeToSegments($subscriber, $segment_ids);
+  }
+
+  static function deleteSubscriptionsForAll($subscriber_ids = array()) {
+    if(!empty($subscriber_ids)) {
+      // delete subscribers' relations to segments (except WP Users' segment)
+      $subscriptions = SubscriberSegment::whereIn('subscriber_id', $subscriber_ids);
+
+      $wp_segment = Segment::getWPSegment();
+      if($wp_segment !== false) {
+        $subscriptions = $subscriptions->whereNotEqual(
+          'segment_id', $wp_segment->id
+        );
+      }
+      return $subscriptions->deleteMany();
+    }
+    return false;
+  }
+
+  static function deleteSubscriptions($subscriber) {
+    if($subscriber !== false && $subscriber->id > 0) {
+      // delete all relationships to segments (except the WP users segment)
+      $subscriptions = self::where('subscriber_id', $subscriber->id);
+
+      $wp_segment = Segment::getWPSegment();
+      if($wp_segment !== false) {
+        $subscriptions = $subscriptions->whereNotEqual(
+          'segment_id', $wp_segment->id
+        );
+      }
+
+      return $subscriptions->delete();
+    }
+    return false;
   }
 
   static function subscribed($orm) {

lib/Segments/WP.php

@@ -8,7 +8,7 @@ use MailPoet\Newsletter\Scheduler\Scheduler;
 class WP {
   static function synchronizeUser($wp_user_id, $old_wp_user_data = false) {
     $wp_user = \get_userdata($wp_user_id);
-    $wp_users_segment = Segment::getWPUsers();
+    $wp_users_segment = Segment::getWPSegment();
 
     if($wp_user === false or $wp_users_segment === false) return;
 
@@ -53,7 +53,7 @@ class WP {
         $subscriber = Subscriber::createOrUpdate($data);
         if($subscriber->getErrors() === false && $subscriber->id > 0) {
           // add subscriber to the WP Users segment
-          SubscriberSegment::addSubscriptions(
+          SubscriberSegment::subscribeToSegments(
             $subscriber,
             array($wp_users_segment->id)
           );
@@ -73,7 +73,7 @@ class WP {
 
   static function synchronizeUsers() {
     // get wordpress users list
-    $wp_users_segment = Segment::getWPUsers();
+    $wp_users_segment = Segment::getWPSegment();
 
     // fetch all wp users id
     $wp_users = \get_users(array(

lib/Subscription/Pages.php

@@ -69,7 +69,7 @@ class Pages {
       if($this->subscriber->status !== Subscriber::STATUS_UNSUBSCRIBED) {
         $this->subscriber->status = Subscriber::STATUS_UNSUBSCRIBED;
         $this->subscriber->save();
-        SubscriberSegment::removeSubscriptions($this->subscriber);
+        SubscriberSegment::unsubscribeFromSegments($this->subscriber);
       }
     }
   }